hi, yuanhan: Thanks a lot for your clear comments in detail. Patch v3 will be sent later.
-zhiyong- > -----Original Message----- > From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com] > Sent: Tuesday, September 6, 2016 8:28 PM > To: Yang, Zhiyong <zhiyong.yang at intel.com> > Cc: dev at dpdk.org; Xie, Huawei <huawei.xie at intel.com> > Subject: Re: [PATCH v2] net/virtio: fix xstats name issue > > On Tue, Sep 06, 2016 at 03:50:12PM +0800, Zhiyong Yang wrote: > > The patch fixes some xstats name issues > > Please, state **clearly** what the issue is: it's far away from being enough > just mentioning "fix a issue" without actually telling what it is. > > For this case, you could describe the issue like: > > We have a stats named "size_1024_1517_packets", while the code > actually counts the range "[1024, 1518]", which is obviously wrong. > > You could even add the related code piece here (you see the context is > missing in the patch, which is harder for reviewer to see what's actually > going > wrong): > > else if (s < 1519) > stats->size_bins[6]++; > > > and make the xstats name conform > > to the definition of etherStatsPkts1024to1518Octets in rfc2819 page 23. > > It's a bit abrupt to metion the RFC here, without some background. It could > be better if we mention something like: (just followed by above issue > description). > > We could either fix it by correcting the "if" check in the code, > or fix it by just renaming the stats to conform to the code. The > later solution is taken because that's what the RFC2819 suggests. > > > > Fixes: 76d4c652e07d ("virtio: add extended stats") > > > > --- > > Besides that, the three dash "---" means the end of your commit log, > therefore, it should go ---> > > > > > Changes in v2: > > > > modify commit summary. > > add the fixline. > > > > Signed-off-by: Zhiyong Yang <zhiyong.yang at intel.com> > > ... here > > Otherwise, your SoB will be chopped off while apply. > > Another thing to note is, it's a good candidate to me for stable branch, thus, > you may need add following in the commit log before you SoB: > > Cc: <stable at dpdk.org> > > So, mind to send v3, with above fixes? > > Thanks. > > --yliu
