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

[Daniel Zahka]

On Tue Oct 20, 2026 at 5:21 PM EDT, Sebastian Andrzej Siewior wrote:
> On 2026-10-19 19:05:43 [-0700], Jakub Kicinski wrote:
>> On Mon, 19 Oct 2026 23:54:05 +0200 Sebastian Andrzej Siewior wrote:
>> > On 2026-10-19 09:04:38 [-0400], Daniel Zahka wrote:
>> > > > Why do you have the two mpnic_base_stats and fold the stats into into
>> > > > it? My guess would be that on stop the counters are reset and you want
>> > > > to preserve the stats.
>> > > >  
>> > > 
>> > > Not sure I understand. I think you are asking about two things:
>> > > 
>> > > 1. Why have two mpnic_base_stats?
>> > > 
>> > > To avoid a separate heap allocation that can fail. Handling an
>> > > allocation failure in the ndo_stop path when doing the aggregation into
>> > > the base stats is tricky.  
>> > 
>> > Right. But looking at hinic3_get_stats64() for instance (I just grabbed
>> > a random one) it simply does query the stats for each queue, add up,
>> > return. It probably does not drop the queues on down, just reallocates
>> > them while changing the number of queues (and maybe MTU).
>> 
>> I don't think pre-allocating stats for chips which supports thousands
>> of queues is reasonable.
>
> I don't mean to pre-allocate stats. If device-down removes queues then
> the stats gone, so what. The queues could remain.
>

hinic3 preallocates stats state for the max supported queues at
registration time. That's why it doesn't keep aggregated base stats. It
also doesn't implement the netdev_stat_ops API, and if it did, it would
need a bit of a redesign to be able to report historical device level
counters separately from live per-queue counters.

>> > > 2. Why split stats into base stats and per queue stats, and why reset
>> > > the per queue counts when the device transitions down?
>> > > 
>> > > This is the way the stats in netdev.yaml are exposed depending on the
>> > > 'scope' request attribute. My reading of the relevant kernel docs is
>> > > that the per queue counters are counting events for the current live
>> > > queue, so it makes sense to reset them and fold those counts into the
>> > > base stats when the interface is transitioned down.  
>> > 
>> > 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.
>

The per queue stats are kept in per-napi state that is freed after
folding in the down path and then zero allocated on the subsequent up.