[dpdk-dev] [PATCH v2] examples/ipsec-secgw: add per core packet stats

[Ananyev, Konstantin]

> > > @@ -1159,6 +1243,19 @@ ipsec_poll_mode_worker(void)
> > >  			drain_tx_buffers(qconf);
> > >  			drain_crypto_buffers(qconf);
> > >  			prev_tsc = cur_tsc;
> > > +#ifdef ENABLE_STATS
> > > +			if (lcore_id == rte_get_master_lcore()) {
> > > +				/* advance the timer */
> > > +				timer_tsc += diff_tsc;
> > > +
> > > +				/* if timer has reached its timeout */
> > > +				if (unlikely(timer_tsc >= timer_period)) {
> > > +					print_stats();
> > > +					/* reset the timer */
> > > +					timer_tsc = 0;
> > > +				}
> > > +			}
> > > +#endif /* ENABLE_STATS */
> >
> >
> > Why to do stats collection/display inside data-path?
> > Why not use rte_timer/rte_alarm and make it happen in control thread?
> > Another option - make stats printing at some signal to the process.
> > In that case we don't need to bother with time period at all - it will be user to
> > decide.
> > Again if we remove that print_stats() from data-path it might become cheap
> > enough to always collect it, and we will not need ENABLE_STATS macro at all.
> 
> [Anoob] The stats collection will not be cheap because we are updating a common structure from multiple cores.

What I am saying - per-lcore stats collection (updating lcore stats by lcore itself) - should be cheap enough
and can stay in data-path (probably even always, not only when particular compile flag is enabled). 
Global stats collection and display is quite expensive operation,
so we shouldn't keep it in data-path.
We have a control thread within dpdk (for alarms, signals, timers, etc.), 
why not to call print_stats from there?

> And the idea is not to
> have a very efficient stats mechanism. When we have more cores engaged, IPsec performance is directly dependent on ability of RSS to
> split traffic to multiple cores. Such stats would come handy in understanding per core distribution of incoming traffic.
> 
> >