[RFC v2] net/af_packet: make stats reset reliable

[Mattias Rönnblom]

On 2024-05-07 16:51, Stephen Hemminger wrote:
> On Tue, 7 May 2024 14:49:19 +0100
> Ferruh Yigit <ferruh.yigit at 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.yigit at amd.com>
>>>>> ---
>>>>> Cc: Mattias Rönnblom <mattias.ronnblom at ericsson.com>
>>>>> Cc: Stephen Hemminger <stephen at networkplumber.org>
>>>>> Cc: Morten Brørup <mb at smartsharesystems.com>
>>>>>
>>>>> This update triggered by mail list discussion [1].
>>>>>
>>>>> [1]
>>>>> https://inbox.dpdk.org/dev/3b2cf48e-2293-4226-b6cd-5f4dd3969f99@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.