> From: Ferruh Yigit [mailto:ferruh.yi...@amd.com]
> Sent: Friday, 3 May 2024 17.46
>
> For stats reset, use an offset instead of zeroing out actual stats values,
> get_stats() displays diff between stats and offset.
> This way stats only updated in datapath and offset only updated in stats
> reset function. This makes stats reset function more reliable.
>
> As stats only written by single thread, we can remove 'volatile' qualifier
> which should improve the performance in datapath.
>
> While updating around, 'igb_stats' parameter renamed as 'stats'.
>
> Signed-off-by: Ferruh Yigit <ferruh.yi...@amd.com>
> ---
> Cc: Mattias Rönnblom <mattias.ronnb...@ericsson.com>
> Cc: Stephen Hemminger <step...@networkplumber.org>
> Cc: Morten Brørup <m...@smartsharesystems.com>
>
> This update triggered by mail list discussion [1].
>
> [1]
> https://inbox.dpdk.org/dev/3b2cf48e-2293-4226-b6cd-
> 5f4dd3969...@lysator.liu.se/
>
> v2:
> * Remove wrapping check for stats
>
> v3:
> * counter and offset put into same struct per stats
> * Use atomic load / store for stats values
> ---
Note: My comments below relate to software PMDs only.
Design for the following invariants:
1. "counter" may increase at any time. (So stopping forwarding is not required.)
2. "counter" may not decrease.
3. "offset" is always <= "counter".
So:
Stats_get() must read "offset" before "counter"; if "counter" races to increase
in the mean time, it doesn't hurt. Stats_get() is a relatively "cold" function,
so barriers etc. are acceptable.
Assuming that stats_add() lazy-writes "counter"; if stats_get() reads an old
value, its result will be slightly off, but not negative.
Similarly for stats_reset(), which obviously reads "counter" before writing
"offset"; if "counter" races to increase in the mean time, the too low "offset"
will not cause negative stats from stats_get().
And a requested change for performance:
> +struct stats {
> + uint64_t counter;
> + uint64_t offset;
> +};
The "offset" is cold.
Stats_add(), which is the only hot function, only touches "counter".
Instead of having a struct with {counter, offset}, I strongly prefer having
them separate.
E.g. as a struct defining the set of statistics (e.g. pkts, bytes, errors),
instantiated once for the counters (in a hot zone of the device data structure)
and once for the offsets (in a cold zone of the device data structure).
There could be variants of this "set of statistics" struct, e.g. one for RX and
a different one for TX. (Each variant would be instantiated twice, once for
counters, and once for offsets.)
