Re: [dpdk-dev] [EXT] Re: [PATCH v2 2/5] eal: add accessor functions for lcore_config

[Jerin Jacob Kollanukkaran <jerinj@marvell.com>]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, May 1, 2019 2:23 AM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: dev@dpdk.org
> Subject: [EXT] Re: [dpdk-dev] [PATCH v2 2/5] eal: add accessor functions for
> lcore_config
> On Tue, 16 Apr 2019 17:03:47 +0000
> Jerin Jacob Kollanukkaran <jerinj@marvell.com> wrote:
> 
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Stephen Hemminger
> > > Sent: Wednesday, April 10, 2019 10:46 PM
> > > To: dev@dpdk.org
> > > Cc: Stephen Hemminger <stephen@networkplumber.org>
> > > Subject: [dpdk-dev] [PATCH v2 2/5] eal: add accessor functions for
> > > lcore_config
> > >
> > > The fields of the internal EAL core configuration are currently laid
> > > bare as part of the API. This is not good practice and limits fixing issues with
> layout and sizes.
> > >
> > > Make new accessor functions for the fields used by current drivers
> > > and examples. Mark return code functions as experimental since this
> > > value might change in the future and probably shouldn't have been
> > > used by non EAL code anyway.
> > >
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > Reviewed-by: David Marchand <david.marchand@redhat.com>  Shared
> > > Library Versions
> > >  -----------------------
> > > diff --git a/lib/librte_eal/common/eal_common_lcore.c
> > > b/lib/librte_eal/common/eal_common_lcore.c
> > > index 1cbac42286ba..6cf4d7abb0bd 100644
> > > --- a/lib/librte_eal/common/eal_common_lcore.c
> > > +++ b/lib/librte_eal/common/eal_common_lcore.c
> > > @@ -16,6 +16,45 @@
> > >  #include "eal_private.h"
> > >  #include "eal_thread.h"
> > >
> > > +int rte_lcore_index(int lcore_id)
> > > +{
> > > +	if (unlikely(lcore_id >= RTE_MAX_LCORE))
> > > +		return -1;
> > > +
> > > +	if (lcore_id < 0)
> > > +		lcore_id = (int)rte_lcore_id();
> > > +
> > > +	return lcore_config[lcore_id].core_index;
> > > +}
> >
> > If I understand it correctly, We are planning to do this scheme only for slow
> path functions. Right?
> > Is rte_lcore_* functions comes in slow path category? I thought a few
> > of them can be used In fast path too.
> > I am bit concerned about a low end arm cores where function invocation
> overhead significant vs inline.
> > ODP has taken a route of introducing a compile time config to choose
> > inline vs separate function to address the performance regression . I am not
> sure what would be the correct way to handle this.
> 
> The lcore config itself is not accessed anywhere in fastpath of any of the
> examples.
> It is usually is part of the setup process. The rte_lcore_index is only used by
> lthread in a log message.

Makes sense.

> 
> The main fastpath is rte_lcore_id() which is already done by per-thread variable
> and is unchanged by these patches.
> 

How about creating a tag like __rte_experimental, say __rte_fastpath
for fastpath functions to avoid confusion between what is  fastpath and what is not
between developers and end users. By creating new section in linker, will
make all fast path function in one area.


> I don't have facilities to do any deep dive performance testing on ARM.