Re: [PATCH net-next v2 1/4] eth: mpnic: count packets, bytes, and drops per ring

[Daniel Zahka]

On Mon Oct 26, 2026 at 6:11 AM EDT, Sebastian Andrzej Siewior wrote:
> On 2026-10-20 14:50:38 [-0700], Jakub Kicinski wrote:
>> On Tue, 20 Oct 2026 23:21:55 +0200 Sebastian Andrzej Siewior wrote:
>> > > > Is there any other driver doing this?
>> > > > There is ::get_sset_count() and so on for the per-queue stats.  
>> > > 
>> > > You may be mixing up queue stats (netlink) with the old ethtool -S
>> > > kitchen sink / garbage dump.  
>> > 
>> > The statistics is folded on device-down into a global counter. So on
>> > device-up the statistics is still available. At least that is my
>> > understanding. This means the per-queue stats are lost and global stats
>> > don't match the per queue stats anymore.
>> 
>> Yes, that's intentional and recommended by the docs for queue stats.
>
> I guess this is Documentation/networking/statistics.rst that I need to
> study.
>
>> > So if this never happens, it might not be needed to preserve this. That
>> > force-RCU might not be needed. This does happen on down and I would
>> > assume that the device is no longer processing packets.
>> 
>> The (old) ndo for stats reading runs under RCU, only.
>> AI will immediately point that out if we remove the sync.
>> 
>> May I ask why you care? Is there another driver you're trying 
>> to implement stats in? I'm not sure where you're coming from
>> and therefore what to explain to you.
>
> I tried to be useful and do so some reviews and this stats part is what
> I did not expect to be "expected". I would expect per-queue stats which
> are folded into device stats on query time. Anyway. Let me study what is
> recommended before making noise here again.
> One last thing: If you replace RCU with SRCU then the logic should
> remain the same. On the plus side you would have some domain separation
> and the "synchronize" part will not force/ wait for the global grace
> period but be limited to the local one.
>
> Sebastian

I do appreciate the review. Thanks for the SRCU suggestion. I hadn't
considered that. Optimizing the writer side performance will become more
important when we implement ethtool ringparam/channel reconfiguration.
Another idea we had was to heap allocate the base stats and use
call_rcu() to free instead. I decided against that for now, because I
thought it was more complicated, and the writer side is limted to just
ndo_open/ndo_stop for now.