Hello, Thank you for the comments.
"I'm confused by the tentative choice of vendor ID, because it doesn't seem to be an Ethernet OUI for Intel." [MW] To be honest I didn't put too much attention to VENDOR_ID because my initial plan was to re-submit patch with all relevant RFC2819 counters when Vendor ID for Intel would be approved. That is also why I've put it into ofp-util.c, so no special reason behind this. I will choose Intel OUI and put it into openflow header. "There is a lot of code duplication here among all of the netdev..." At the beginning I did the initialization in netdev_get_stats (setting buffer to all "1s") before counters are passed to particular netdev "client", that approach makes sense to me, because: * a lot of code is simplified, * clients care only for filling counters which should be reported. However I found it problematic because netdev-linux is "appending" Values: stats->rx_errors += dev_stats.rx_errors; .... But it seems that I didn't analyse it carefully enough, and it is initialized In other location. I will check that once again and I will try to put initialization in netdev_get_stats. "This version seems quite reasonable." [MW] Thank you, I think that right now I can proceed with final patch, I will re-work the patch based on your comments, I will add all Missing counters relevant for RFC2819, and I will apply this patch in separate mailing thread (not RFC patch). Br, Michal. -----Original Message----- From: Ben Pfaff [mailto:b...@ovn.org] Sent: Monday, November 30, 2015 1:05 AM To: Weglicki, MichalX <michalx.wegli...@intel.com> Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH RFC v2] ofproto: RFC extended statistics patch. On Mon, Nov 23, 2015 at 09:10:40AM +0000, mweglicx wrote: > Implementation of new statistics extension: > - new counters definition based on RFC2819, > - new statistics are retrieved using experimenter code and > are printed as a result to ofctl dump-ports, > - new statistics are printed to output via ofctl only if those > are present in reply message, > - new file header created: include/openflow/intel-ext.h which > contains new statistics definition, > - new extended statistics calculation is implemented only for > dpdk-vhost-enabled ports. > > Please note that this is just feature proposal: > - experimenter code is hardcoded in ofp-util.c, it will be > changed when VENDOR_ID for intel will be defined, > - final patch will include also more counters based on > RFC2863, RFC3635 and RFC2819. > > v2: > - protocol extension in ofproto has been removed, existing > mechanism with experimenter codes has been used, > - additional option in ofctl dump-ports-ext has been removed, > new counters are retrieved via dump-ports. > > Signed-off-by: Michal Weglicki <michalx.wegli...@intel.com> > Signed-off-by: Timo Puha <timox.p...@intel.com> This version seems quite reasonable. I'm confused by the tentative choice of vendor ID, because it doesn't seem to be an Ethernet OUI for Intel. Rather, it seems to belong to: 00-15-1D (hex) M2I CORPORATION 00151D (base 16) M2I CORPORATION Kyonggi Venture Anyang technical center, 13th Floor, 572-5, Anyang 8-Dong, Manan-Gu Anyang-Shi Kyonggi-Do 430-731 KR It seems like you could just choose any Ethernet OUI for Intel and put it in the openflow header; not sure why you're putting it in ofp-util.c instead. There is a lot of code duplication here among all of the netdev implementations, to initialize the added statistics to "unknown" values. It would be better to avoid the duplication. One way would be to add a global helper function; another would be for netdev_get_stats() to initialize the whole structure to 0xff before passing it to the netdev implementation. I'll look forward to the next revision. -------------------------------------------------------------- Intel Research and Development Ireland Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
