[dpdk-dev] [PATCH v7 5/6] lib: added new library for latency stats

Mcnamara, John john.mcnamara at intel.com
Tue Jan 17 15:53:55 CET 2017



> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
> Sent: Tuesday, January 17, 2017 12:34 PM
> To: Mcnamara, John <john.mcnamara at intel.com>
> Cc: Horton, Remy <remy.horton at intel.com>; dev at dpdk.org; Pattan, Reshma
> <reshma.pattan at intel.com>; Thomas Monjalon <thomas.monjalon at 6wind.com>;
> olivier.matz at 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 at dpdk.org] On Behalf Of Jerin Jacob
> > > Sent: Tuesday, January 17, 2017 4:30 AM
> > > To: Horton, Remy <remy.horton at intel.com>
> > > Cc: dev at dpdk.org; Pattan, Reshma <reshma.pattan at intel.com>; Thomas
> > > Monjalon <thomas.monjalon at 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.pattan at 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.pattan at intel.com>
> > > > Signed-off-by: Remy Horton <remy.horton at 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.


> ...
> 
> 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

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.

John




More information about the dev mailing list