Hi Ben,
Much Thanks for your initial reveiw comments.
We have addressed almost all problems and re-submitted the patch except the
below two lines.
"checkpatch" reports:
warning: 1 line adds whitespace errors.
WARNING: Line length is >79-characters long
#137 FILE: include/openvswitch/ofp-msgs.h:656:
OFPTYPE_OXS_AGGREGATE_STATS_REQUEST, /*
OFPRAW_OFPST15_OXS_AGGREGATE_REQUEST. */
WARNING: Line length is >79-characters long
#138 FILE: include/openvswitch/ofp-msgs.h:657:
OFPTYPE_OXS_AGGREGATE_STATS_REPLY, /*
OFPRAW_OFPST15_OXS_AGGREGATE_REPLY. */
We tried to address the above two lines also, but we are getting "unexpected
syntax within OFPTYPE_ definition" error as below, if we are trying to adjust
them.
PYTHONPATH=./python":"$PYTHONPATH PYTHONDONTWRITEBYTECODE=yes /usr/bin/python
./build-aux/extract-ofp-msgs \
./include/openvswitch/ofp-msgs.h lib/ofp-msgs.inc >
lib/ofp-msgs.inc.tmp && mv lib/ofp-msgs.inc.tmp lib/ofp-msgs.inc
./include/openvswitch/ofp-msgs.h:659: unexpected syntax within OFPTYPE_
definition
make[2]: *** [lib/ofp-msgs.inc] Error 1
make[2]: Leaving directory `/home/tcs/freshgitjune15/ovsfinalpatch/ovs'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/tcs/freshgitjune15/ovsfinalpatch/ovs'
make: *** [all] Error 2
Thanks & Regards
Satya Valli
Tata Consultancy Services
Mailto: [email protected]
Website: http://www.tcs.com
____________________________________________
Experience certainty. IT Services
Business Solutions
Consulting
____________________________________________
-----Ben Pfaff <[email protected]> wrote: -----
To: SatyaValli <[email protected]>
From: Ben Pfaff <[email protected]>
Date: 06/15/2017 02:38AM
Cc: [email protected], SatyaValli <[email protected]>
Subject: Re: [ovs-dev] [PATCH] OF1.5/EXT-334 OXS/Extensible Flow Entry
Statistics Support
On Fri, Jun 09, 2017 at 01:26:51PM +0530, SatyaValli wrote:
> commit 49a3592b2878a33033e5fd2e6e5ce82ebccef780
> Author: Satya Valli <[email protected]>
>
> OpenVswitch: OF1.5/EXT-334 Extensible Flow Entry Statistics
>
> OpenFlow 1.5 introduces the Extensible Statistics (OXS) by redefining the
> existing
> flow entry statistics with OXS Fields.
> This Patch provides implementation for OXS fields encoding in TLV format.
>
> To support this implementation below two messages are newly added
>
> OFPST_OXS_FLOW_STATS_REQUEST
> OFPST_OXS_FLOW_STATS_REPLY
> OFPST_OXS_AGGREGATE_STATS_REQUEST
> OFPST_OXS_AGGREGATE_STATS_REPLY
> OFPST_FLOW_REMOVED
>
> As per the openflow specification-1.5, this enhancement should take place
> on the existing flow entry statistics with the OXS fields on all the messages
> that carries flow entry statistics.
>
> The current commit adds support for the new feature in flow statistics
> multipart messages,
> aggregate multipart messages and OXS flow statistics support for flow removal
> message.
>
> Some more fields are added to ovs-ofctl dump-flows command to support
> OpenFlow15 OXS stats.
> Below are Commands to display OXS stats field wise
>
> Flow Statistics Multipart
> ovs-ofctl dump-flows -O OpenFlow15 <bridge> oxs-duration
> ovs-ofctl dump-flows -O OpenFlow15 <bridge> oxs-idle_time
> ovs-ofctl dump-flows -O OpenFlow15 <bridge> oxs-packet_count
> ovs-ofctl dump-flows -O OpenFlow15 <bridge> oxs-byte_count
>
> Aggregate Flow Statistics Multipart
> ovs-ofctl dump-aggregate -O OpenFlow15 <bridge> oxs-packet_count
> ovs-ofctl dump-aggregate -O OpenFlow15 <bridge> oxs-byte_count
> ovs-ofctl dump-aggregate -O OpenFlow15 <bridge> oxs-flow_count
>
> Signed-off-by: Satya Valli <[email protected]>
> Co-authored-by: Lavanya Harivelam <[email protected]> and Surya
> Muttamsetty <[email protected]>
Thank you for working on Open vSwitch.
"Co-authored-by" takes only one name. Please use multiple
"Co-authored-by" tags if you have more than one coauthor.
"checkpatch" reports:
warning: 1 line adds whitespace errors.
WARNING: Line length is >79-characters long
#137 FILE: include/openvswitch/ofp-msgs.h:656:
OFPTYPE_OXS_AGGREGATE_STATS_REQUEST, /*
OFPRAW_OFPST15_OXS_AGGREGATE_REQUEST. */
WARNING: Line length is >79-characters long
#138 FILE: include/openvswitch/ofp-msgs.h:657:
OFPTYPE_OXS_AGGREGATE_STATS_REPLY, /*
OFPRAW_OFPST15_OXS_AGGREGATE_REPLY. */
ERROR: Inappropriate bracing around statement
#483 FILE: lib/ofp-util.c:3331:
if (error)
ERROR: Improper whitespace around control block
#1114 FILE: lib/ox-stat.c:534:
HMAP_FOR_EACH_IN_BUCKET(oxfs, header_node, hash_int(header_no_len, 0),
ERROR: Improper whitespace around control block
#1135 FILE: lib/ox-stat.c:555:
LIST_FOR_EACH(oxfs, ox_node, &oxs_ox_map[id]) {
ERROR: Improper whitespace around control block
#1298 FILE: lib/ox-stat.c:718:
if(fs) {
ERROR: Improper whitespace around control block
#1308 FILE: lib/ox-stat.c:728:
if(fs) {
ERROR: Improper whitespace around control block
#1318 FILE: lib/ox-stat.c:738:
if(fs) {
Clang reports:
../lib/ofp-util.c:3466:42: error: implicit conversion from enumeration type
'enum ofputil_protocol' to different enumeration type 'enum ofp_version'
[-Werror,-Wenum-conversion]
"sparse" reports:
../lib/ox-stat.c:282:45: warning: constant 0xFFFFFFFF00000000 is so big it
is unsigned long long
../lib/ox-stat.c:291:42: warning: constant 0xFFFFFFFF00000000 is so big it
is unsigned long long
../lib/ox-stat.c:127:9: warning: symbol 'oxs_field_set' was not declared.
Should it be static?
../lib/ox-stat.c:280:31: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:280:31: expected restricted ovs_be64 [usertype]
<noident>
../lib/ox-stat.c:280:31: got unsigned long long [unsigned] [addressable]
[usertype] duration
../lib/ox-stat.c:290:32: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:290:32: expected restricted ovs_be64 [usertype]
<noident>
../lib/ox-stat.c:290:32: got unsigned long long [unsigned] [addressable]
[usertype] idle_time
../lib/ox-stat.c:298:39: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:298:39: expected restricted ovs_be64 [usertype]
<noident>
../lib/ox-stat.c:298:39: got unsigned long long [unsigned] [addressable]
[usertype] packet_count
../lib/ox-stat.c:305:37: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:305:37: expected restricted ovs_be64 [usertype]
<noident>
../lib/ox-stat.c:305:37: got unsigned long long [unsigned] [addressable]
[usertype] byte_count
../lib/ox-stat.c:443:40: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:443:40: expected restricted ovs_be32 [usertype] x
../lib/ox-stat.c:443:40: got unsigned int [unsigned] [addressable]
[usertype] flow_count
../lib/ox-stat.c:450:43: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:450:43: expected restricted ovs_be64 [usertype]
<noident>
../lib/ox-stat.c:450:43: got unsigned long long [unsigned] [addressable]
[usertype] packet_count
../lib/ox-stat.c:457:41: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:457:41: expected restricted ovs_be64 [usertype]
<noident>
../lib/ox-stat.c:457:41: got unsigned long long [unsigned] [addressable]
[usertype] byte_count
../lib/ox-stat.c:926:49: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:926:49: expected restricted ovs_be32 [usertype] x
../lib/ox-stat.c:926:49: got unsigned long long
../lib/ox-stat.c:927:39: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:927:39: expected restricted ovs_be32 [usertype] x
../lib/ox-stat.c:927:39: got unsigned int [unsigned] [usertype] <noident>
../lib/ox-stat.c:934:39: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:934:39: expected restricted ovs_be64 [usertype]
<noident>
../lib/ox-stat.c:934:39: got unsigned long long [unsigned] [addressable]
[usertype] packet_count
../lib/ox-stat.c:942:37: warning: incorrect type in argument 1 (different
base types)
../lib/ox-stat.c:942:37: expected restricted ovs_be64 [usertype]
<noident>
../lib/ox-stat.c:942:37: got unsigned long long [unsigned] [addressable]
[usertype] byte_count
../lib/ox-stat.c:720:18: warning: incorrect type in assignment (different
base types)
../lib/ox-stat.c:720:18: expected unsigned int [unsigned] [assigned]
[usertype] flow_count
../lib/ox-stat.c:720:18: got restricted ovs_be32
../lib/ox-stat.c:730:19: warning: incorrect type in assignment (different
base types)
../lib/ox-stat.c:730:19: expected unsigned long long [unsigned]
[assigned] [usertype] pkt_count
../lib/ox-stat.c:730:19: got restricted ovs_be64
../lib/ox-stat.c:740:20: warning: incorrect type in assignment (different
base types)
../lib/ox-stat.c:740:20: expected unsigned long long [unsigned]
[assigned] [usertype] byte_count
../lib/ox-stat.c:740:20: got restricted ovs_be64
../lib/ox-stat.c:788:18: warning: incorrect type in assignment (different
base types)
../lib/ox-stat.c:788:18: expected unsigned long long [unsigned]
[assigned] [usertype] duration
../lib/ox-stat.c:788:18: got restricted ovs_be64
../lib/ox-stat.c:804:19: warning: incorrect type in assignment (different
base types)
../lib/ox-stat.c:804:19: expected unsigned long long [unsigned]
[assigned] [usertype] pkt_count
../lib/ox-stat.c:804:19: got restricted ovs_be64
../lib/ox-stat.c:820:20: warning: incorrect type in assignment (different
base types)
../lib/ox-stat.c:820:20: expected unsigned long long [unsigned]
[assigned] [usertype] byte_count
../lib/ox-stat.c:820:20: got restricted ovs_be64
../lib/ox-stat.c:619:22: warning: incorrect type in assignment (different
base types)
../lib/ox-stat.c:619:22: expected unsigned long long [unsigned]
[assigned] [usertype] duration
../lib/ox-stat.c:619:22: got restricted ovs_be64
../lib/ox-stat.c:629:22: warning: incorrect type in assignment (different
base types)
../lib/ox-stat.c:629:22: expected unsigned long long [unsigned]
[assigned] [usertype] idl_time
../lib/ox-stat.c:629:22: got restricted ovs_be64
../lib/ox-stat.c:645:23: warning: incorrect type in assignment (different
base types)
../lib/ox-stat.c:645:23: expected unsigned long long [unsigned]
[assigned] [usertype] pkt_count
../lib/ox-stat.c:645:23: got restricted ovs_be64
../lib/ox-stat.c:655:24: warning: incorrect type in assignment (different
base types)
../lib/ox-stat.c:655:24: expected unsigned long long [unsigned]
[assigned] [usertype] byte_count
../lib/ox-stat.c:655:24: got restricted ovs_be64
../lib/ofp-util.c:3466:42: warning: mixing different enum types
../lib/ofp-util.c:3466:42: int enum ofputil_protocol versus
../lib/ofp-util.c:3466:42: int enum ofp_version
Please fix all of these problems and resubmit your patch.
=====-----=====-----=====
Notice: The information contained in this e-mail
message and/or attachments to it may contain
confidential or privileged information. If you are
not the intended recipient, any dissemination, use,
review, distribution, printing or copying of the
information contained in this e-mail message
and/or attachments to it are strictly prohibited. If
you have received this communication in error,
please notify us by reply e-mail or telephone and
immediately and permanently delete the message
and any attachments. Thank you
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
