Re: [PATCH net-next 2/8] net: designate XSK pool pointers in queues as "ops protected"

Joe Damato
2025-04-08T02:17:40Z

On Mon, Apr 07, 2025 at 12:01:11PM -0700, Jakub Kicinski wrote:
> Read accesses go via xsk_get_pool_from_qid(), the call coming
> from the core and gve look safe (other "ops locked" drivers
> don't support XSK).
>
> Write accesses go via xsk_reg_pool_at_qid() and xsk_clear_pool_at_qid().
> Former is already under the ops lock, latter needs to be locked when
> coming from the workqueue via xp_clear_dev().
>
> Acked-by: Stanislav Fomichev <sdf@fomichev.me>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
> include/linux/netdevice.h | 1 +
> include/net/netdev_rx_queue.h | 6 +++---
> net/xdp/xsk.c | 2 ++
> net/xdp/xsk_buff_pool.c | 7 ++++++-
> 4 files changed, 12 insertions(+), 4 deletions(-)

[...]

> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 25a76c5ce0f1..c7e50fd86c6a 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -279,9 +279,14 @@ static void xp_release_deferred(struct work_struct *work)
> {
> struct xsk_buff_pool *pool = container_of(work, struct xsk_buff_pool,
> work);
> + struct net_device *netdev = pool->netdev;
>
> rtnl_lock();
> - xp_clear_dev(pool);
> + if (netdev) {
> + netdev_lock_ops(netdev);
> + xp_clear_dev(pool);
> + netdev_unlock_ops(netdev);
> + }
> rtnl_unlock();

Is it actually possible for netdev to be NULL here?

I feel like it probably isn't, but if it were possible we'd need an
else case here to xp_clear_dev(pool) without the netdev_lock_ops?