On Tue, Jan 17, 2017 at 02:53:55PM +0000, Mcnamara, John wrote: > > > > -----Original Message----- > > From: Jerin Jacob [mailto:jerin.ja...@caviumnetworks.com] > > Sent: Tuesday, January 17, 2017 12:34 PM > > To: Mcnamara, John <john.mcnam...@intel.com> > > Cc: Horton, Remy <remy.hor...@intel.com>; dev@dpdk.org; Pattan, Reshma > > <reshma.pat...@intel.com>; Thomas Monjalon <thomas.monja...@6wind.com>; > > olivier.m...@6wind.com > > Subject: Re: [dpdk-dev] [PATCH v7 5/6] lib: added new library for latency > > stats > > > > On Tue, Jan 17, 2017 at 11:19:24AM +0000, Mcnamara, John wrote: > > > > -----Original Message----- > > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Jerin Jacob > > > > Sent: Tuesday, January 17, 2017 4:30 AM > > > > To: Horton, Remy <remy.hor...@intel.com> > > > > Cc: dev@dpdk.org; Pattan, Reshma <reshma.pat...@intel.com>; Thomas > > > > Monjalon <thomas.monja...@6wind.com> > > > > Subject: Re: [dpdk-dev] [PATCH v7 5/6] lib: added new library for > > > > latency stats > > > > > > > > On Mon, Jan 16, 2017 at 04:19:32PM +0000, Remy Horton wrote: > > > > > From: Reshma Pattan <reshma.pat...@intel.com> > > > > > > > > > > Add a library designed to calculate latency statistics and report > > > > > them to the application when queried. The library measures > > > > > minimum, average and maximum latencies, and jitter in nano > > > > > seconds. The current implementation supports global latency stats, > > > > > i.e. per application > > > > stats. > > > > > > > > > > Signed-off-by: Reshma Pattan <reshma.pat...@intel.com> > > > > > Signed-off-by: Remy Horton <remy.hor...@intel.com> > > > > > --- > > > > > MAINTAINERS | 4 + > > > > > config/common_base | 5 + > > > > > doc/api/doxy-api-index.md | 1 + > > > > > doc/api/doxy-api.conf | 1 + > > > > > doc/guides/rel_notes/release_17_02.rst | 5 + > > > > > lib/Makefile | 1 + > > > > > lib/librte_latencystats/Makefile | 57 +++ > > > > > lib/librte_latencystats/rte_latencystats.c | 389 > > > > +++++++++++++++++++++ > > > > > lib/librte_latencystats/rte_latencystats.h | 146 ++++++++ > > > > > .../rte_latencystats_version.map | 10 + > > > > > lib/librte_mbuf/rte_mbuf.h | 3 + > > > > > > > > It is a value added feature for DPDK. But what is the plan for > > > > incorporating the mbuf change? I have 8 month old mbuf change for > > > > ARM for natural alignment. If we are accepting any mbuf change then > > > > we need to include outstanding mbuf changes to avoid future ABI > > breakage. > > > > > > > > http://dpdk.org/dev/patchwork/patch/12878/ > > > > > > > > > > Hi Jerin, > > > > Hi John, > > > > > > > > As far as I know the plan was to reach some sort of consensus on the > > > mbuf structure at the DPDK Userspace 2016, during and after Olivier's > > > presentation and then to make those changes during 17.02. > > > > > > However, I believe Olivier had other work commitments in this release > > > and wasn't able to work on the mbuf changes. > > > > > > The above mbuf change (and addition at the end of the struct) should > > > have gone into that mbuf rework, along with your changes. > > > > > > However, since the mbuf rework didn't happen we need to add the field > > > in this release. > > > > So we don't care the mbuf ABI breakage in the next release. This wasn't > > the message I got earlier for ARM's mbuf change. > > > > http://dpdk.org/dev/patchwork/patch/12878/ > > > Hi Jerin, > > We do care about ABI breakage but I was under the impression that the > timestamp change wasn't breaking the ABI since it was at the end of the > struct. I also ran the ABI validator against the change and it didn't show any > breakage. > > http://dpdk.org/doc/guides/contributing/versioning.html#running-the-abi-validator > > The rearm_data alignment patch, on the other hand, does break ABI. I think > that is the main difference between the two patches. > > If the timestamp change does break ABI then it should also wait until the mbuf > restructuring.
I agree on ABI part. If understand it correctly, Oliver would like to group all the mbuf modification in one version and postponed the rearm_data change. Here is the email --------------------------------------------------------- Changing the mbuf topology is something that should happen as rarely as possible, so I think we should group all mbuf modifications in one version. Your issue (mbuf->rearm alignment), the removing of uneeded fields (port id, maybe nb_segs), and possibly other things should be addressed for next version (16.11). I'll send a deprecation notice before the 16.07 is out if there is no opposition. --------------------------------------------------------- > > > > ... > > > > There is nothing against you or this feature. The only part concerns me > > that some set of patches can always override any rule and include in the > > release (even as marking as EXPERIMENTAL) because of its important for > > some set of consumers. > > Another set has to wait in the queue because its not important for some > > people. > > For me, it is not a sign of vendor neutral open source project. > > To be fair I don't think we are trying to override any rule here. > > Also, we aren't the only vendor looking for a timestamp in the mbuf. > Mellanox also submitted a patch: > > http://dpdk.org/ml/archives/dev/2016-October/048809.html We don't have any issue in adding timestamp in mbuf either. The point, I was trying to make some changes like rearm_data only need for ARM architecture.In those cases, postponing the changes due to some other non direct dependency change is not good a specific architecture/vendor. > > However, it is also fair to acknowledge that the rearm_data alignment patch > shouldn't have had to wait so long. I can't really answer for that directly. > My feeling is that it was targeted for the mbuf rework but got forgotten > when that work slipped. Oliver, Could you please suggest how to proceed further? > > John > >
