On 2024-05-07 16:51, Stephen Hemminger wrote:
On Tue, 7 May 2024 14:49:19 +0100
Ferruh Yigit <ferruh.yi...@amd.com> wrote:

On 5/7/2024 8:23 AM, Mattias Rönnblom wrote:
On 2024-04-28 17:11, Mattias Rönnblom wrote:
On 2024-04-26 16:38, Ferruh Yigit wrote:
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.

volatile wouldn't help you if you had multiple writers, so that can't
be the reason for its removal. It would be more accurate to say it
should be replaced with atomic updates. If you don't use volatile and
don't use atomics, you have to consider if the compiler can reach the
conclusion that it does not need to store the counter value for future
use *for that thread*. Since otherwise, I don't think the store
actually needs to occur. Since DPDK statistics tend to work, it's
pretty obvious that current compilers tend not to reach this conclusion.

If this should be done 100% properly, the update operation should be a
non-atomic load, non-atomic add, and an atomic store. Similarly, for
the reset, the offset store should be atomic.

Considered the state of the rest of the DPDK code base, I think a
non-atomic, non-volatile solution is also fine.

(That said, I think we're better off just deprecating stats reset
altogether, and returning -ENOTSUP here meanwhile.)
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/

I would prefer that the SW statistics be handled generically by ethdev
layers and used by all such drivers.

The most complete version of SW stats now is in the virtio driver.
If reset needs to be reliable (debatable), then it needs to be done without
atomics.

Why it needs to be done without atomics? Whatever that means.

In what sense should they be unreliable, needs to be documented.

Reply via email to