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: satyavalli.r...@tcs.com
Website: http://www.tcs.com
____________________________________________
Experience certainty.   IT Services
Business Solutions
Consulting
____________________________________________


-----Ben Pfaff <b...@ovn.org> wrote: -----
To: SatyaValli <satyavalli.r...@gmail.com>
From: Ben Pfaff <b...@ovn.org>
Date: 06/15/2017 02:38AM
Cc: d...@openvswitch.org, SatyaValli <satyavalli.r...@tcs.com>
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 <satyavalli.r...@tcs.com>
> 
> 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 <satyavalli.r...@tcs.com>
> Co-authored-by: Lavanya Harivelam <harivelam.lava...@tcs.com> and Surya 
> Muttamsetty <muttamsetty.su...@tcs.com>

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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to