Re: [dpdk-dev] [PATCH v2 5/5] examples/kni: improve zeroing statistics

[Dan Gora <dg@adax.com>]

On Thu, Sep 27, 2018 at 8:40 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >> Not sure about introducing an extra check to datapath for possible error on
> >> stats zero. I am for dropping this patch, what do you think?
> >
> > This is literally adding one instruction to the datapath.  Not even an
> > atomic instruction.  There is no effect on the performance caused by
> > this change.
> >
> > Is that not better than the user (like me who experienced this)
> > wondering why they cannot zero the counters even when there is no
> > traffic?
>
> Can we have something like, stop the forwarding, clear the stats and start
> forwarding back?

This is what is broken.  The stats do not zero because you have the
two worker threads who are constantly performing:

 kni_stats[port_id].rx_packets += num;

and

 kni_stats[port_id].tx_packets += nb_tx;

You know how read/inc/write races work, right?

These are not atomic increments, so the other thread zeroing these
counters is _always_ going to race with these worker threads
overwriting the counters with the old values.

With no traffic it's worse because the worker threads perform these
increments even more often!

> Yes effect is small but it is for each packet, it doesn't make sense to me to
> add extra check for each packet for the rare case of stats reset.

Its not rare at all!  You cannot zero the statistics around 80% of the
time on my machine when there is no traffic.  It's trivial to
reproduce this.  Just run a little traffic, stop the traffic, zero the
stats and check the stats.

If you cannot zero the statistics reliably under any circumstance then
the statistics themselves are worthless and should be removed.  It's
better to have no stats than completely unreliable ones.