Thanks for the review. I checked both findings against the v6 base,
47a144672573, and the rebased v7 base, d8674294aefe.
> Does this create a use-after-free race with lockless readers like
> mana_get_stats64()?
I do not think the reported unregister ordering is correct. The call from
mana_remove() takes the immediate, non-batched unregister path:
unregister_netdevice(ndev)
-> unregister_netdevice_queue(ndev, NULL)
-> unregister_netdevice_many()
-> unregister_netdevice_many_notify()
That path unlists the device and calls synchronize_net(), removes the
netdev's sysfs object, and calls synchronize_net() again before returning.
These steps are not deferred until the later rtnl_unlock()/netdev_run_todo()
processing.
The /proc/net/dev walk holds RCU protection through dev_seq_start()/stop().
The sysfs statistics read is also RCU-protected and checks netif_is_alive().
Consequently, the reported readers are drained before unregister_netdevice()
returns and mana_remove() calls mana_free_queue_stats(). This ordering is
present in both bases.
I have therefore not added another grace period or moved the statistics
arrays to a deferred destructor in v7 for the reported before-rtnl_unlock()
window.
> Can this result in lost statistic increments due to a data race?
I agree that the xdp_xmit update can lose increments. Different CPUs can
select the same TX queue, and u64_stats_sync does not exclude concurrent
writers on the supported 64-bit configurations.
However, the same unlocked "tx_stats->xdp_xmit += count" is already present
in mana_xdp_xmit() on the unmodified v6 base. This patch changes the stats
storage from an embedded object to a pointer; it does not introduce that
unlocked update.
I have recorded this as a real, independent statistics bug and am keeping
its fix separate from the queue-set revision. I have not identified a
queue-set regression or a blocking dependency that requires including it
here. The race remains unfixed in v7; it is not being dismissed as a false
positive.
Thanks,
Wei
