Re: [dpdk-dev] [PATCH v3] pipeline: add statistics for librte_pipeline ports and tables

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 26 May 2015 21:35:22 +0000
"Dumitrescu, Cristian" <cristian.dumitrescu@intel.com> wrote:

> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen
> > Hemminger
> > Sent: Tuesday, May 26, 2015 3:57 PM
> > To: Gajdzica, MaciejX T
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v3] pipeline: add statistics for librte_pipeline
> > ports and tables
> > 
> > On Tue, 26 May 2015 15:39:18 +0200
> > Maciej Gajdzica <maciejx.t.gajdzica@intel.com> wrote:
> > 
> > > +#if RTE_LOG_LEVEL == RTE_LOG_DEBUG
> > > +#define RTE_PIPELINE_STATS_ADD(counter, val) \
> > > +	({ (counter) += (val); })
> > > +
> > > +#define RTE_PIPELINE_STATS_ADD_M(counter, mask) \
> > > +	({ (counter) += __builtin_popcountll(mask); })
> > > +#else
> > > +#define RTE_PIPELINE_STATS_ADD(counter, val)
> > > +#define RTE_PIPELINE_STATS_ADD_M(counter, mask)
> > > +#endif
> > 
> > This is worse idea than the previous one.
> > I want statistics done on a per lcore basis, and always available
> > because real users on production system want statistics (but they
> > don't want log spam).
> 
> Stephen,
> 
> Thomas and myself converged towards this solution, Thomas asked if anybody objects, you did not (http://www.dpdk.org/ml/archives/dev/2015-May/018099.html) . You say this idea is bad, but what exactly is your objection? Do you have an alternative proposal?

Yes. Always keep statistics.

We use functions like this internally.

struct xxx_stats {
	uint64_t mib[XXX_MIB_MAX];
};
extern struct xxx_stats xxx_stats[RTE_MAX_LCORE];

#define XXXSTAT_INC(type)	xxxstats[rte_lcore_id()].mibs[type]++


> You already mentioned in the previous thread you would like to have per lcore statistics. I was kindly asking you to describe your idea, but you did not do this yet (http://www.dpdk.org/ml/archives/dev/2015-May/017956.html ). Can you please describe it? Each port instance has its own statistics counters. Each lcore can run one or more pipeline instances, therefore each lcore typically runs several port instances of the same or different type (each port instance with its own statistics), so how is "per lcore stats" requirement applicable here?

I thought you were familiar with technique of having per-cpu structures and variables
widely used in Linux kernel to prevent cache thrashing. Although you call it false sharing,
it isn't false., it happens when same counter ping-pongs between multiple threads for
no added benefit.


> You also reiterate that you would like to have the stats always enabled. You can definitely do this, it is one of the available choices, but why not also accommodate the users that want to pick the opposite choice? Why force apps to spend cycles on stats if the app either does not want these counters (library counters not relevant for that app, maybe the app is only interested in maintaining some other stats that it implements itself) or do not want them anymore (maybe they only needed them during debug phase), etc? Jay asked this question, and I did my best in my reply to describe our motivation (http://www.dpdk.org/ml/archives/dev/2015-May/017992.html). Maybe you missed that post, it would be good to get your reply on this one too.

I want to see DPDK get out of the config madness.
This is real code, not an Intel benchmark special.