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

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

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 <remy.horton@intel.com>
> > Cc: dev@dpdk.org; Pattan, Reshma <reshma.pattan@intel.com>; Thomas
> > Monjalon <thomas.monjalon@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@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@intel.com>
> > > Signed-off-by: Remy Horton <remy.horton@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/

> 
> I guess the difference between the above change and your change is that
> the latter is more complex and potentially affect performance, and as such
> makes more sense as part of a rework.

The mbuf natural alignment is a not complex change, it just moving the field and
it does not have any performance impact on IA nor nobody reported any
performance regression on IA.

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.

Jerin

> 
> Perhaps we, as a community, should commit to the mbuf rework in 17.05
> and make sure it gets done.
> 
> John
> 
> 
>  
> 
> 
>