Re: [PATCH net v2] net: ntb_netdev: Fix statistics races

[Koichiro Den]

On Fri, Aug 28, 2026 at 02:10:48PM -0700, Jakub Kicinski wrote:
> On Fri, 28 Aug 2026 13:49:57 -0700 Jakub Kicinski wrote:
> > On Sat, 29 Aug 2026 00:41:22 +0900 Koichiro Den wrote:
> > > Note that transport callbacks may still complete after
> > > unregister_netdev(), so tie the tstats lifetime to the client device.  
> > 
> > We shouldn't really call devm_netdev_alloc_pcpu_stats() for types
> > of stats declared by netdev core. We should set the pointer type
> > like you did in v1.

Thanks for pointing that out, I wasn't aware of that.

> > 
> > Could you instead fix the fact that the Tx callback runs on 
> > an unregistered device? Maybe just wrap the whole body of
> > ntb_netdev_tx_handler() in RCU and check 
> > 
> > 	READ_ONCE(ndev->reg_state) == NETREG_REGISTERED
> > 
> > ?
> 
> Maybe it's over-complicating. IDK.

I think the RCU approach is clean enough. RCU plus the NETREG_REGISTERED
check should address the stats lifetime issue without moving transport
teardown under RTNL.

I'll respin and let the core manage tstats by setting
NETDEV_PCPU_STAT_TSTATS, as v1 did for dstats. Late completions will still
consume the skb, but skip accounting and queue wake.

Also, looking again at the commit message paragraph you quoted, I realized
it was poorly written and confusing. What I meant was:

    Tie tstats to the client device so they remain alive until transport
    queue teardown. Moving that teardown into .ndo_uninit() would put
    potentially long DMA waits under RTNL, so leave it after
    unregister_netdev(). TX completions may therefore still update stats in
    between.

After reworking it as you suggested, the last paragraph in v3 will say:

    Let the core manage tstats while keeping transport teardown after
    unregister_netdev(), outside RTNL. RCU lets unregister wait for a
    completion already updating stats, while later completions only consume
    the skb and skip accounting and queue wake.

Best regards,
Koichiro