Sounds good!
On Wed, Dec 02, 2015 at 11:11:41AM +0000, Weglicki, MichalX wrote: > 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
