From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH v7 5/6] lib: added new library for latency stats Date: Tue, 17 Jan 2017 21:55:16 +0530 Message-ID: <20170117162515.GA28925@localhost.localdomain> References: <1484583573-30163-1-git-send-email-remy.horton@intel.com> <1484583573-30163-6-git-send-email-remy.horton@intel.com> <20170117042935.GA32676@localhost.localdomain> <20170117123418.GA2611@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Cc: "Horton, Remy" , "dev@dpdk.org" , "Pattan, Reshma" , Thomas Monjalon , "olivier.matz@6wind.com" , "Richardson, Bruce" To: "Mcnamara, John" Return-path: Received: from NAM02-CY1-obe.outbound.protection.outlook.com (mail-cys01nam02on0059.outbound.protection.outlook.com [104.47.37.59]) by dpdk.org (Postfix) with ESMTP id A9274108F for ; Tue, 17 Jan 2017 17:25:38 +0100 (CET) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Jan 17, 2017 at 02:53:55PM +0000, Mcnamara, John wrote: > > > > -----Original Message----- > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > > Sent: Tuesday, January 17, 2017 12:34 PM > > To: Mcnamara, John > > Cc: Horton, Remy ; dev@dpdk.org; Pattan, Reshma > > ; Thomas Monjalon ; > > olivier.matz@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-bounces@dpdk.org] On Behalf Of Jerin Jacob > > > > Sent: Tuesday, January 17, 2017 4:30 AM > > > > To: Horton, Remy > > > > Cc: dev@dpdk.org; Pattan, Reshma ; Thomas > > > > Monjalon > > > > 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 > > > > > > > > > > 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 > > > > > Signed-off-by: Remy Horton > > > > > --- > > > > > 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 > >