diff mbox series

[bpf-next,v4,05/14] xsk: move queue_id, dev and need_wakeup to buffer pool

Message ID 1595307848-20719-6-git-send-email-magnus.karlsson@intel.com
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series xsk: support shared umems between devices and queues | expand

Commit Message

Magnus Karlsson July 21, 2020, 5:03 a.m. UTC
Move queue_id, dev, and need_wakeup from the umem to the
buffer pool. This so that we in a later commit can share the umem
between multiple HW queues. There is one buffer pool per dev and
queue id, so these variables should belong to the buffer pool, not
the umem. Need_wakeup is also something that is set on a per napi
level, so there is usually one per device and queue id. So move
this to the buffer pool too.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 include/net/xdp_sock.h      |  3 ---
 include/net/xsk_buff_pool.h |  4 ++++
 net/xdp/xdp_umem.c          | 19 +------------------
 net/xdp/xdp_umem.h          |  4 ----
 net/xdp/xsk.c               | 40 +++++++++++++++-------------------------
 net/xdp/xsk_buff_pool.c     | 39 ++++++++++++++++++++++-----------------
 net/xdp/xsk_diag.c          |  4 ++--
 7 files changed, 44 insertions(+), 69 deletions(-)

Comments

Björn Töpel July 28, 2020, 7:09 a.m. UTC | #1
On 2020-07-21 07:03, Magnus Karlsson wrote:
> Move queue_id, dev, and need_wakeup from the umem to the
> buffer pool. This so that we in a later commit can share the umem
> between multiple HW queues. There is one buffer pool per dev and
> queue id, so these variables should belong to the buffer pool, not
> the umem. Need_wakeup is also something that is set on a per napi
> level, so there is usually one per device and queue id. So move
> this to the buffer pool too.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>   include/net/xdp_sock.h      |  3 ---
>   include/net/xsk_buff_pool.h |  4 ++++
>   net/xdp/xdp_umem.c          | 19 +------------------
>   net/xdp/xdp_umem.h          |  4 ----
>   net/xdp/xsk.c               | 40 +++++++++++++++-------------------------
>   net/xdp/xsk_buff_pool.c     | 39 ++++++++++++++++++++++-----------------
>   net/xdp/xsk_diag.c          |  4 ++--
>   7 files changed, 44 insertions(+), 69 deletions(-)
> 
[...]
>   		}
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 36287d2..436648a 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -95,10 +95,9 @@ void xp_set_rxq_info(struct xsk_buff_pool *pool, struct xdp_rxq_info *rxq)
>   }
>   EXPORT_SYMBOL(xp_set_rxq_info);
>   
> -int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
> +int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *netdev,
>   		  u16 queue_id, u16 flags)
>   {
> -	struct xdp_umem *umem = pool->umem;
>   	bool force_zc, force_copy;
>   	struct netdev_bpf bpf;
>   	int err = 0;
> @@ -111,27 +110,30 @@ int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
>   	if (force_zc && force_copy)
>   		return -EINVAL;
>   
> -	if (xsk_get_pool_from_qid(dev, queue_id))
> +	if (xsk_get_pool_from_qid(netdev, queue_id))
>   		return -EBUSY;
>   
> -	err = xsk_reg_pool_at_qid(dev, pool, queue_id);
> +	err = xsk_reg_pool_at_qid(netdev, pool, queue_id);
>   	if (err)
>   		return err;
>   
>   	if (flags & XDP_USE_NEED_WAKEUP) {
> -		umem->flags |= XDP_UMEM_USES_NEED_WAKEUP;
> +		pool->uses_need_wakeup = true;
>   		/* Tx needs to be explicitly woken up the first time.
>   		 * Also for supporting drivers that do not implement this
>   		 * feature. They will always have to call sendto().
>   		 */
> -		umem->need_wakeup = XDP_WAKEUP_TX;
> +		pool->cached_need_wakeup = XDP_WAKEUP_TX;
>   	}
>   
> +	dev_hold(netdev);
> +

You have a reference leak here for the error case.


Björn
Maxim Mikityanskiy July 28, 2020, 9:21 a.m. UTC | #2
On 2020-07-21 08:03, Magnus Karlsson wrote:
> Move queue_id, dev, and need_wakeup from the umem to the
> buffer pool. This so that we in a later commit can share the umem
> between multiple HW queues. There is one buffer pool per dev and
> queue id, so these variables should belong to the buffer pool, not
> the umem. Need_wakeup is also something that is set on a per napi
> level, so there is usually one per device and queue id. So move
> this to the buffer pool too.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>   include/net/xdp_sock.h      |  3 ---
>   include/net/xsk_buff_pool.h |  4 ++++
>   net/xdp/xdp_umem.c          | 19 +------------------
>   net/xdp/xdp_umem.h          |  4 ----
>   net/xdp/xsk.c               | 40 +++++++++++++++-------------------------
>   net/xdp/xsk_buff_pool.c     | 39 ++++++++++++++++++++++-----------------
>   net/xdp/xsk_diag.c          |  4 ++--
>   7 files changed, 44 insertions(+), 69 deletions(-)
> 
> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index 2a284e1..b052f1c 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -26,11 +26,8 @@ struct xdp_umem {
>   	refcount_t users;
>   	struct page **pgs;
>   	u32 npgs;
> -	u16 queue_id;
> -	u8 need_wakeup;
>   	u8 flags;
>   	int id;
> -	struct net_device *dev;
>   	bool zc;
>   	spinlock_t xsk_tx_list_lock;
>   	struct list_head xsk_tx_list;
> diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
> index 380d9ae..2d94890 100644
> --- a/include/net/xsk_buff_pool.h
> +++ b/include/net/xsk_buff_pool.h
> @@ -43,11 +43,15 @@ struct xsk_buff_pool {
>   	u32 headroom;
>   	u32 chunk_size;
>   	u32 frame_len;
> +	u16 queue_id;
> +	u8 cached_need_wakeup;
> +	bool uses_need_wakeup;
>   	bool dma_need_sync;
>   	bool unaligned;
>   	struct xdp_umem *umem;
>   	void *addrs;
>   	struct device *dev;
> +	struct net_device *netdev;
>   	refcount_t users;
>   	struct work_struct work;
>   	struct xdp_buff_xsk *free_heads[];
> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index 7d86a63..b1699d0 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c
> @@ -63,26 +63,9 @@ static void xdp_umem_unaccount_pages(struct xdp_umem *umem)
>   	}
>   }
>   
> -void xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
> -			 u16 queue_id)
> -{
> -	umem->dev = dev;
> -	umem->queue_id = queue_id;
> -
> -	dev_hold(dev);
> -}
> -
> -void xdp_umem_clear_dev(struct xdp_umem *umem)
> -{
> -	dev_put(umem->dev);
> -	umem->dev = NULL;
> -	umem->zc = false;
> -}
> -
>   static void xdp_umem_release(struct xdp_umem *umem)
>   {
> -	xdp_umem_clear_dev(umem);
> -
> +	umem->zc = false;
>   	ida_simple_remove(&umem_ida, umem->id);
>   
>   	xdp_umem_unpin_pages(umem);
> diff --git a/net/xdp/xdp_umem.h b/net/xdp/xdp_umem.h
> index 93e96be..67bf3f3 100644
> --- a/net/xdp/xdp_umem.h
> +++ b/net/xdp/xdp_umem.h
> @@ -8,10 +8,6 @@
>   
>   #include <net/xdp_sock_drv.h>
>   
> -void xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
> -			 u16 queue_id);
> -void xdp_umem_clear_dev(struct xdp_umem *umem);
> -bool xdp_umem_validate_queues(struct xdp_umem *umem);
>   void xdp_get_umem(struct xdp_umem *umem);
>   void xdp_put_umem(struct xdp_umem *umem);
>   void xdp_add_sk_umem(struct xdp_umem *umem, struct xdp_sock *xs);
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index ee04887..624d0fc 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -41,67 +41,61 @@ bool xsk_is_setup_for_bpf_map(struct xdp_sock *xs)
>   
>   void xsk_set_rx_need_wakeup(struct xsk_buff_pool *pool)
>   {
> -	struct xdp_umem *umem = pool->umem;
> -
> -	if (umem->need_wakeup & XDP_WAKEUP_RX)
> +	if (pool->cached_need_wakeup & XDP_WAKEUP_RX)
>   		return;
>   
>   	pool->fq->ring->flags |= XDP_RING_NEED_WAKEUP;
> -	umem->need_wakeup |= XDP_WAKEUP_RX;
> +	pool->cached_need_wakeup |= XDP_WAKEUP_RX;
>   }
>   EXPORT_SYMBOL(xsk_set_rx_need_wakeup);
>   
>   void xsk_set_tx_need_wakeup(struct xsk_buff_pool *pool)
>   {
> -	struct xdp_umem *umem = pool->umem;
>   	struct xdp_sock *xs;
>   
> -	if (umem->need_wakeup & XDP_WAKEUP_TX)
> +	if (pool->cached_need_wakeup & XDP_WAKEUP_TX)
>   		return;
>   
>   	rcu_read_lock();
> -	list_for_each_entry_rcu(xs, &umem->xsk_tx_list, list) {
> +	list_for_each_entry_rcu(xs, &xs->umem->xsk_tx_list, list) {

I know this is going to be fixed in the next patch, but it breaks 
bisectability: this patch is broken because xs is not assigned. I don't 
think this change is actually needed here, it doesn't belong to this 
patch, and you can convert it to &pool->xsk_tx_list in one step in the 
next patch.

>   		xs->tx->ring->flags |= XDP_RING_NEED_WAKEUP;
>   	}
>   	rcu_read_unlock();
>   
> -	umem->need_wakeup |= XDP_WAKEUP_TX;
> +	pool->cached_need_wakeup |= XDP_WAKEUP_TX;
>   }
>   EXPORT_SYMBOL(xsk_set_tx_need_wakeup);
>   
>   void xsk_clear_rx_need_wakeup(struct xsk_buff_pool *pool)
>   {
> -	struct xdp_umem *umem = pool->umem;
> -
> -	if (!(umem->need_wakeup & XDP_WAKEUP_RX))
> +	if (!(pool->cached_need_wakeup & XDP_WAKEUP_RX))
>   		return;
>   
>   	pool->fq->ring->flags &= ~XDP_RING_NEED_WAKEUP;
> -	umem->need_wakeup &= ~XDP_WAKEUP_RX;
> +	pool->cached_need_wakeup &= ~XDP_WAKEUP_RX;
>   }
>   EXPORT_SYMBOL(xsk_clear_rx_need_wakeup);
>   
>   void xsk_clear_tx_need_wakeup(struct xsk_buff_pool *pool)
>   {
> -	struct xdp_umem *umem = pool->umem;
>   	struct xdp_sock *xs;
>   
> -	if (!(umem->need_wakeup & XDP_WAKEUP_TX))
> +	if (!(pool->cached_need_wakeup & XDP_WAKEUP_TX))
>   		return;
>   
>   	rcu_read_lock();
> -	list_for_each_entry_rcu(xs, &umem->xsk_tx_list, list) {
> +	list_for_each_entry_rcu(xs, &xs->umem->xsk_tx_list, list) {

Same here.

>   		xs->tx->ring->flags &= ~XDP_RING_NEED_WAKEUP;
>   	}
>   	rcu_read_unlock();
>   
> -	umem->need_wakeup &= ~XDP_WAKEUP_TX;
> +	pool->cached_need_wakeup &= ~XDP_WAKEUP_TX;
>   }
>   EXPORT_SYMBOL(xsk_clear_tx_need_wakeup);
>   
>   bool xsk_uses_need_wakeup(struct xsk_buff_pool *pool)
>   {
> -	return pool->umem->flags & XDP_UMEM_USES_NEED_WAKEUP;
> +	return pool->uses_need_wakeup;
>   }
>   EXPORT_SYMBOL(xsk_uses_need_wakeup);
>   
> @@ -478,16 +472,16 @@ static __poll_t xsk_poll(struct file *file, struct socket *sock,
>   	__poll_t mask = datagram_poll(file, sock, wait);
>   	struct sock *sk = sock->sk;
>   	struct xdp_sock *xs = xdp_sk(sk);
> -	struct xdp_umem *umem;
> +	struct xsk_buff_pool *pool;
>   
>   	if (unlikely(!xsk_is_bound(xs)))
>   		return mask;
>   
> -	umem = xs->umem;
> +	pool = xs->pool;
>   
> -	if (umem->need_wakeup) {
> +	if (pool->cached_need_wakeup) {
>   		if (xs->zc)
> -			xsk_wakeup(xs, umem->need_wakeup);
> +			xsk_wakeup(xs, pool->cached_need_wakeup);
>   		else
>   			/* Poll needs to drive Tx also in copy mode */
>   			__xsk_sendmsg(sk);
> @@ -731,11 +725,9 @@ static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
>   		goto out_unlock;
>   	} else {
>   		/* This xsk has its own umem. */
> -		xdp_umem_assign_dev(xs->umem, dev, qid);
>   		xs->pool = xp_create_and_assign_umem(xs, xs->umem);
>   		if (!xs->pool) {
>   			err = -ENOMEM;
> -			xdp_umem_clear_dev(xs->umem);
>   			goto out_unlock;
>   		}
>   
> @@ -743,7 +735,6 @@ static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
>   		if (err) {
>   			xp_destroy(xs->pool);
>   			xs->pool = NULL;
> -			xdp_umem_clear_dev(xs->umem);
>   			goto out_unlock;
>   		}
>   	}
> @@ -1089,7 +1080,6 @@ static int xsk_notifier(struct notifier_block *this,
>   
>   				/* Clear device references. */
>   				xp_clear_dev(xs->pool);
> -				xdp_umem_clear_dev(xs->umem);
>   			}
>   			mutex_unlock(&xs->mutex);
>   		}
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 36287d2..436648a 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -95,10 +95,9 @@ void xp_set_rxq_info(struct xsk_buff_pool *pool, struct xdp_rxq_info *rxq)
>   }
>   EXPORT_SYMBOL(xp_set_rxq_info);
>   
> -int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
> +int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *netdev,
>   		  u16 queue_id, u16 flags)
>   {
> -	struct xdp_umem *umem = pool->umem;
>   	bool force_zc, force_copy;
>   	struct netdev_bpf bpf;
>   	int err = 0;
> @@ -111,27 +110,30 @@ int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
>   	if (force_zc && force_copy)
>   		return -EINVAL;
>   
> -	if (xsk_get_pool_from_qid(dev, queue_id))
> +	if (xsk_get_pool_from_qid(netdev, queue_id))
>   		return -EBUSY;
>   
> -	err = xsk_reg_pool_at_qid(dev, pool, queue_id);
> +	err = xsk_reg_pool_at_qid(netdev, pool, queue_id);
>   	if (err)
>   		return err;
>   
>   	if (flags & XDP_USE_NEED_WAKEUP) {
> -		umem->flags |= XDP_UMEM_USES_NEED_WAKEUP;

XDP_UMEM_USES_NEED_WAKEUP definition should be removed, along with its 
remaining usage in xdp_umem_reg.

> +		pool->uses_need_wakeup = true;
>   		/* Tx needs to be explicitly woken up the first time.
>   		 * Also for supporting drivers that do not implement this
>   		 * feature. They will always have to call sendto().
>   		 */
> -		umem->need_wakeup = XDP_WAKEUP_TX;
> +		pool->cached_need_wakeup = XDP_WAKEUP_TX;
>   	}
>   
> +	dev_hold(netdev);
> +
>   	if (force_copy)
>   		/* For copy-mode, we are done. */
>   		return 0;
>   
> -	if (!dev->netdev_ops->ndo_bpf || !dev->netdev_ops->ndo_xsk_wakeup) {
> +	if (!netdev->netdev_ops->ndo_bpf ||
> +	    !netdev->netdev_ops->ndo_xsk_wakeup) {
>   		err = -EOPNOTSUPP;
>   		goto err_unreg_pool;
>   	}
> @@ -140,44 +142,47 @@ int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
>   	bpf.xsk.pool = pool;
>   	bpf.xsk.queue_id = queue_id;
>   
> -	err = dev->netdev_ops->ndo_bpf(dev, &bpf);
> +	err = netdev->netdev_ops->ndo_bpf(netdev, &bpf);
>   	if (err)
>   		goto err_unreg_pool;
>   
> -	umem->zc = true;
> +	pool->netdev = netdev;
> +	pool->queue_id = queue_id;
> +	pool->umem->zc = true;
>   	return 0;
>   
>   err_unreg_pool:
>   	if (!force_zc)
>   		err = 0; /* fallback to copy mode */
>   	if (err)
> -		xsk_clear_pool_at_qid(dev, queue_id);
> +		xsk_clear_pool_at_qid(netdev, queue_id);
>   	return err;
>   }
>   
>   void xp_clear_dev(struct xsk_buff_pool *pool)
>   {
> -	struct xdp_umem *umem = pool->umem;
>   	struct netdev_bpf bpf;
>   	int err;
>   
>   	ASSERT_RTNL();
>   
> -	if (!umem->dev)
> +	if (!pool->netdev)
>   		return;
>   
> -	if (umem->zc) {
> +	if (pool->umem->zc) {
>   		bpf.command = XDP_SETUP_XSK_POOL;
>   		bpf.xsk.pool = NULL;
> -		bpf.xsk.queue_id = umem->queue_id;
> +		bpf.xsk.queue_id = pool->queue_id;
>   
> -		err = umem->dev->netdev_ops->ndo_bpf(umem->dev, &bpf);
> +		err = pool->netdev->netdev_ops->ndo_bpf(pool->netdev, &bpf);
>   
>   		if (err)
> -			WARN(1, "failed to disable umem!\n");
> +			WARN(1, "Failed to disable zero-copy!\n");
>   	}
>   
> -	xsk_clear_pool_at_qid(umem->dev, umem->queue_id);
> +	xsk_clear_pool_at_qid(pool->netdev, pool->queue_id);
> +	dev_put(pool->netdev);
> +	pool->netdev = NULL;
>   }
>   
>   static void xp_release_deferred(struct work_struct *work)
> diff --git a/net/xdp/xsk_diag.c b/net/xdp/xsk_diag.c
> index 52675ea..5bd8ea9 100644
> --- a/net/xdp/xsk_diag.c
> +++ b/net/xdp/xsk_diag.c
> @@ -59,8 +59,8 @@ static int xsk_diag_put_umem(const struct xdp_sock *xs, struct sk_buff *nlskb)
>   	du.num_pages = umem->npgs;
>   	du.chunk_size = umem->chunk_size;
>   	du.headroom = umem->headroom;
> -	du.ifindex = umem->dev ? umem->dev->ifindex : 0;
> -	du.queue_id = umem->queue_id;
> +	du.ifindex = pool->netdev ? pool->netdev->ifindex : 0;
> +	du.queue_id = pool->queue_id;
>   	du.flags = 0;
>   	if (umem->zc)
>   		du.flags |= XDP_DU_F_ZEROCOPY;
>
Magnus Karlsson July 29, 2020, 1:20 p.m. UTC | #3
On Tue, Jul 28, 2020 at 9:10 AM Björn Töpel <bjorn.topel@intel.com> wrote:
>
>
>
> On 2020-07-21 07:03, Magnus Karlsson wrote:
> > Move queue_id, dev, and need_wakeup from the umem to the
> > buffer pool. This so that we in a later commit can share the umem
> > between multiple HW queues. There is one buffer pool per dev and
> > queue id, so these variables should belong to the buffer pool, not
> > the umem. Need_wakeup is also something that is set on a per napi
> > level, so there is usually one per device and queue id. So move
> > this to the buffer pool too.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >   include/net/xdp_sock.h      |  3 ---
> >   include/net/xsk_buff_pool.h |  4 ++++
> >   net/xdp/xdp_umem.c          | 19 +------------------
> >   net/xdp/xdp_umem.h          |  4 ----
> >   net/xdp/xsk.c               | 40 +++++++++++++++-------------------------
> >   net/xdp/xsk_buff_pool.c     | 39 ++++++++++++++++++++++-----------------
> >   net/xdp/xsk_diag.c          |  4 ++--
> >   7 files changed, 44 insertions(+), 69 deletions(-)
> >
> [...]
> >               }
> > diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> > index 36287d2..436648a 100644
> > --- a/net/xdp/xsk_buff_pool.c
> > +++ b/net/xdp/xsk_buff_pool.c
> > @@ -95,10 +95,9 @@ void xp_set_rxq_info(struct xsk_buff_pool *pool, struct xdp_rxq_info *rxq)
> >   }
> >   EXPORT_SYMBOL(xp_set_rxq_info);
> >
> > -int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
> > +int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *netdev,
> >                 u16 queue_id, u16 flags)
> >   {
> > -     struct xdp_umem *umem = pool->umem;
> >       bool force_zc, force_copy;
> >       struct netdev_bpf bpf;
> >       int err = 0;
> > @@ -111,27 +110,30 @@ int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
> >       if (force_zc && force_copy)
> >               return -EINVAL;
> >
> > -     if (xsk_get_pool_from_qid(dev, queue_id))
> > +     if (xsk_get_pool_from_qid(netdev, queue_id))
> >               return -EBUSY;
> >
> > -     err = xsk_reg_pool_at_qid(dev, pool, queue_id);
> > +     err = xsk_reg_pool_at_qid(netdev, pool, queue_id);
> >       if (err)
> >               return err;
> >
> >       if (flags & XDP_USE_NEED_WAKEUP) {
> > -             umem->flags |= XDP_UMEM_USES_NEED_WAKEUP;
> > +             pool->uses_need_wakeup = true;
> >               /* Tx needs to be explicitly woken up the first time.
> >                * Also for supporting drivers that do not implement this
> >                * feature. They will always have to call sendto().
> >                */
> > -             umem->need_wakeup = XDP_WAKEUP_TX;
> > +             pool->cached_need_wakeup = XDP_WAKEUP_TX;
> >       }
> >
> > +     dev_hold(netdev);
> > +
>
> You have a reference leak here for the error case.

Thanks. Will fix.

/Magnus

>
> Björn
Magnus Karlsson July 29, 2020, 1:21 p.m. UTC | #4
On Tue, Jul 28, 2020 at 11:22 AM Maxim Mikityanskiy
<maximmi@mellanox.com> wrote:
>
> On 2020-07-21 08:03, Magnus Karlsson wrote:
> > Move queue_id, dev, and need_wakeup from the umem to the
> > buffer pool. This so that we in a later commit can share the umem
> > between multiple HW queues. There is one buffer pool per dev and
> > queue id, so these variables should belong to the buffer pool, not
> > the umem. Need_wakeup is also something that is set on a per napi
> > level, so there is usually one per device and queue id. So move
> > this to the buffer pool too.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >   include/net/xdp_sock.h      |  3 ---
> >   include/net/xsk_buff_pool.h |  4 ++++
> >   net/xdp/xdp_umem.c          | 19 +------------------
> >   net/xdp/xdp_umem.h          |  4 ----
> >   net/xdp/xsk.c               | 40 +++++++++++++++-------------------------
> >   net/xdp/xsk_buff_pool.c     | 39 ++++++++++++++++++++++-----------------
> >   net/xdp/xsk_diag.c          |  4 ++--
> >   7 files changed, 44 insertions(+), 69 deletions(-)
> >
> > diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> > index 2a284e1..b052f1c 100644
> > --- a/include/net/xdp_sock.h
> > +++ b/include/net/xdp_sock.h
> > @@ -26,11 +26,8 @@ struct xdp_umem {
> >       refcount_t users;
> >       struct page **pgs;
> >       u32 npgs;
> > -     u16 queue_id;
> > -     u8 need_wakeup;
> >       u8 flags;
> >       int id;
> > -     struct net_device *dev;
> >       bool zc;
> >       spinlock_t xsk_tx_list_lock;
> >       struct list_head xsk_tx_list;
> > diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
> > index 380d9ae..2d94890 100644
> > --- a/include/net/xsk_buff_pool.h
> > +++ b/include/net/xsk_buff_pool.h
> > @@ -43,11 +43,15 @@ struct xsk_buff_pool {
> >       u32 headroom;
> >       u32 chunk_size;
> >       u32 frame_len;
> > +     u16 queue_id;
> > +     u8 cached_need_wakeup;
> > +     bool uses_need_wakeup;
> >       bool dma_need_sync;
> >       bool unaligned;
> >       struct xdp_umem *umem;
> >       void *addrs;
> >       struct device *dev;
> > +     struct net_device *netdev;
> >       refcount_t users;
> >       struct work_struct work;
> >       struct xdp_buff_xsk *free_heads[];
> > diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> > index 7d86a63..b1699d0 100644
> > --- a/net/xdp/xdp_umem.c
> > +++ b/net/xdp/xdp_umem.c
> > @@ -63,26 +63,9 @@ static void xdp_umem_unaccount_pages(struct xdp_umem *umem)
> >       }
> >   }
> >
> > -void xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
> > -                      u16 queue_id)
> > -{
> > -     umem->dev = dev;
> > -     umem->queue_id = queue_id;
> > -
> > -     dev_hold(dev);
> > -}
> > -
> > -void xdp_umem_clear_dev(struct xdp_umem *umem)
> > -{
> > -     dev_put(umem->dev);
> > -     umem->dev = NULL;
> > -     umem->zc = false;
> > -}
> > -
> >   static void xdp_umem_release(struct xdp_umem *umem)
> >   {
> > -     xdp_umem_clear_dev(umem);
> > -
> > +     umem->zc = false;
> >       ida_simple_remove(&umem_ida, umem->id);
> >
> >       xdp_umem_unpin_pages(umem);
> > diff --git a/net/xdp/xdp_umem.h b/net/xdp/xdp_umem.h
> > index 93e96be..67bf3f3 100644
> > --- a/net/xdp/xdp_umem.h
> > +++ b/net/xdp/xdp_umem.h
> > @@ -8,10 +8,6 @@
> >
> >   #include <net/xdp_sock_drv.h>
> >
> > -void xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
> > -                      u16 queue_id);
> > -void xdp_umem_clear_dev(struct xdp_umem *umem);
> > -bool xdp_umem_validate_queues(struct xdp_umem *umem);
> >   void xdp_get_umem(struct xdp_umem *umem);
> >   void xdp_put_umem(struct xdp_umem *umem);
> >   void xdp_add_sk_umem(struct xdp_umem *umem, struct xdp_sock *xs);
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index ee04887..624d0fc 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -41,67 +41,61 @@ bool xsk_is_setup_for_bpf_map(struct xdp_sock *xs)
> >
> >   void xsk_set_rx_need_wakeup(struct xsk_buff_pool *pool)
> >   {
> > -     struct xdp_umem *umem = pool->umem;
> > -
> > -     if (umem->need_wakeup & XDP_WAKEUP_RX)
> > +     if (pool->cached_need_wakeup & XDP_WAKEUP_RX)
> >               return;
> >
> >       pool->fq->ring->flags |= XDP_RING_NEED_WAKEUP;
> > -     umem->need_wakeup |= XDP_WAKEUP_RX;
> > +     pool->cached_need_wakeup |= XDP_WAKEUP_RX;
> >   }
> >   EXPORT_SYMBOL(xsk_set_rx_need_wakeup);
> >
> >   void xsk_set_tx_need_wakeup(struct xsk_buff_pool *pool)
> >   {
> > -     struct xdp_umem *umem = pool->umem;
> >       struct xdp_sock *xs;
> >
> > -     if (umem->need_wakeup & XDP_WAKEUP_TX)
> > +     if (pool->cached_need_wakeup & XDP_WAKEUP_TX)
> >               return;
> >
> >       rcu_read_lock();
> > -     list_for_each_entry_rcu(xs, &umem->xsk_tx_list, list) {
> > +     list_for_each_entry_rcu(xs, &xs->umem->xsk_tx_list, list) {
>
> I know this is going to be fixed in the next patch, but it breaks
> bisectability: this patch is broken because xs is not assigned. I don't
> think this change is actually needed here, it doesn't belong to this
> patch, and you can convert it to &pool->xsk_tx_list in one step in the
> next patch.

Thanks. Will fix this and the other comment you had below.

/Magnus

> >               xs->tx->ring->flags |= XDP_RING_NEED_WAKEUP;
> >       }
> >       rcu_read_unlock();
> >
> > -     umem->need_wakeup |= XDP_WAKEUP_TX;
> > +     pool->cached_need_wakeup |= XDP_WAKEUP_TX;
> >   }
> >   EXPORT_SYMBOL(xsk_set_tx_need_wakeup);
> >
> >   void xsk_clear_rx_need_wakeup(struct xsk_buff_pool *pool)
> >   {
> > -     struct xdp_umem *umem = pool->umem;
> > -
> > -     if (!(umem->need_wakeup & XDP_WAKEUP_RX))
> > +     if (!(pool->cached_need_wakeup & XDP_WAKEUP_RX))
> >               return;
> >
> >       pool->fq->ring->flags &= ~XDP_RING_NEED_WAKEUP;
> > -     umem->need_wakeup &= ~XDP_WAKEUP_RX;
> > +     pool->cached_need_wakeup &= ~XDP_WAKEUP_RX;
> >   }
> >   EXPORT_SYMBOL(xsk_clear_rx_need_wakeup);
> >
> >   void xsk_clear_tx_need_wakeup(struct xsk_buff_pool *pool)
> >   {
> > -     struct xdp_umem *umem = pool->umem;
> >       struct xdp_sock *xs;
> >
> > -     if (!(umem->need_wakeup & XDP_WAKEUP_TX))
> > +     if (!(pool->cached_need_wakeup & XDP_WAKEUP_TX))
> >               return;
> >
> >       rcu_read_lock();
> > -     list_for_each_entry_rcu(xs, &umem->xsk_tx_list, list) {
> > +     list_for_each_entry_rcu(xs, &xs->umem->xsk_tx_list, list) {
>
> Same here.
>
> >               xs->tx->ring->flags &= ~XDP_RING_NEED_WAKEUP;
> >       }
> >       rcu_read_unlock();
> >
> > -     umem->need_wakeup &= ~XDP_WAKEUP_TX;
> > +     pool->cached_need_wakeup &= ~XDP_WAKEUP_TX;
> >   }
> >   EXPORT_SYMBOL(xsk_clear_tx_need_wakeup);
> >
> >   bool xsk_uses_need_wakeup(struct xsk_buff_pool *pool)
> >   {
> > -     return pool->umem->flags & XDP_UMEM_USES_NEED_WAKEUP;
> > +     return pool->uses_need_wakeup;
> >   }
> >   EXPORT_SYMBOL(xsk_uses_need_wakeup);
> >
> > @@ -478,16 +472,16 @@ static __poll_t xsk_poll(struct file *file, struct socket *sock,
> >       __poll_t mask = datagram_poll(file, sock, wait);
> >       struct sock *sk = sock->sk;
> >       struct xdp_sock *xs = xdp_sk(sk);
> > -     struct xdp_umem *umem;
> > +     struct xsk_buff_pool *pool;
> >
> >       if (unlikely(!xsk_is_bound(xs)))
> >               return mask;
> >
> > -     umem = xs->umem;
> > +     pool = xs->pool;
> >
> > -     if (umem->need_wakeup) {
> > +     if (pool->cached_need_wakeup) {
> >               if (xs->zc)
> > -                     xsk_wakeup(xs, umem->need_wakeup);
> > +                     xsk_wakeup(xs, pool->cached_need_wakeup);
> >               else
> >                       /* Poll needs to drive Tx also in copy mode */
> >                       __xsk_sendmsg(sk);
> > @@ -731,11 +725,9 @@ static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
> >               goto out_unlock;
> >       } else {
> >               /* This xsk has its own umem. */
> > -             xdp_umem_assign_dev(xs->umem, dev, qid);
> >               xs->pool = xp_create_and_assign_umem(xs, xs->umem);
> >               if (!xs->pool) {
> >                       err = -ENOMEM;
> > -                     xdp_umem_clear_dev(xs->umem);
> >                       goto out_unlock;
> >               }
> >
> > @@ -743,7 +735,6 @@ static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
> >               if (err) {
> >                       xp_destroy(xs->pool);
> >                       xs->pool = NULL;
> > -                     xdp_umem_clear_dev(xs->umem);
> >                       goto out_unlock;
> >               }
> >       }
> > @@ -1089,7 +1080,6 @@ static int xsk_notifier(struct notifier_block *this,
> >
> >                               /* Clear device references. */
> >                               xp_clear_dev(xs->pool);
> > -                             xdp_umem_clear_dev(xs->umem);
> >                       }
> >                       mutex_unlock(&xs->mutex);
> >               }
> > diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> > index 36287d2..436648a 100644
> > --- a/net/xdp/xsk_buff_pool.c
> > +++ b/net/xdp/xsk_buff_pool.c
> > @@ -95,10 +95,9 @@ void xp_set_rxq_info(struct xsk_buff_pool *pool, struct xdp_rxq_info *rxq)
> >   }
> >   EXPORT_SYMBOL(xp_set_rxq_info);
> >
> > -int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
> > +int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *netdev,
> >                 u16 queue_id, u16 flags)
> >   {
> > -     struct xdp_umem *umem = pool->umem;
> >       bool force_zc, force_copy;
> >       struct netdev_bpf bpf;
> >       int err = 0;
> > @@ -111,27 +110,30 @@ int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
> >       if (force_zc && force_copy)
> >               return -EINVAL;
> >
> > -     if (xsk_get_pool_from_qid(dev, queue_id))
> > +     if (xsk_get_pool_from_qid(netdev, queue_id))
> >               return -EBUSY;
> >
> > -     err = xsk_reg_pool_at_qid(dev, pool, queue_id);
> > +     err = xsk_reg_pool_at_qid(netdev, pool, queue_id);
> >       if (err)
> >               return err;
> >
> >       if (flags & XDP_USE_NEED_WAKEUP) {
> > -             umem->flags |= XDP_UMEM_USES_NEED_WAKEUP;
>
> XDP_UMEM_USES_NEED_WAKEUP definition should be removed, along with its
> remaining usage in xdp_umem_reg.
>
> > +             pool->uses_need_wakeup = true;
> >               /* Tx needs to be explicitly woken up the first time.
> >                * Also for supporting drivers that do not implement this
> >                * feature. They will always have to call sendto().
> >                */
> > -             umem->need_wakeup = XDP_WAKEUP_TX;
> > +             pool->cached_need_wakeup = XDP_WAKEUP_TX;
> >       }
> >
> > +     dev_hold(netdev);
> > +
> >       if (force_copy)
> >               /* For copy-mode, we are done. */
> >               return 0;
> >
> > -     if (!dev->netdev_ops->ndo_bpf || !dev->netdev_ops->ndo_xsk_wakeup) {
> > +     if (!netdev->netdev_ops->ndo_bpf ||
> > +         !netdev->netdev_ops->ndo_xsk_wakeup) {
> >               err = -EOPNOTSUPP;
> >               goto err_unreg_pool;
> >       }
> > @@ -140,44 +142,47 @@ int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
> >       bpf.xsk.pool = pool;
> >       bpf.xsk.queue_id = queue_id;
> >
> > -     err = dev->netdev_ops->ndo_bpf(dev, &bpf);
> > +     err = netdev->netdev_ops->ndo_bpf(netdev, &bpf);
> >       if (err)
> >               goto err_unreg_pool;
> >
> > -     umem->zc = true;
> > +     pool->netdev = netdev;
> > +     pool->queue_id = queue_id;
> > +     pool->umem->zc = true;
> >       return 0;
> >
> >   err_unreg_pool:
> >       if (!force_zc)
> >               err = 0; /* fallback to copy mode */
> >       if (err)
> > -             xsk_clear_pool_at_qid(dev, queue_id);
> > +             xsk_clear_pool_at_qid(netdev, queue_id);
> >       return err;
> >   }
> >
> >   void xp_clear_dev(struct xsk_buff_pool *pool)
> >   {
> > -     struct xdp_umem *umem = pool->umem;
> >       struct netdev_bpf bpf;
> >       int err;
> >
> >       ASSERT_RTNL();
> >
> > -     if (!umem->dev)
> > +     if (!pool->netdev)
> >               return;
> >
> > -     if (umem->zc) {
> > +     if (pool->umem->zc) {
> >               bpf.command = XDP_SETUP_XSK_POOL;
> >               bpf.xsk.pool = NULL;
> > -             bpf.xsk.queue_id = umem->queue_id;
> > +             bpf.xsk.queue_id = pool->queue_id;
> >
> > -             err = umem->dev->netdev_ops->ndo_bpf(umem->dev, &bpf);
> > +             err = pool->netdev->netdev_ops->ndo_bpf(pool->netdev, &bpf);
> >
> >               if (err)
> > -                     WARN(1, "failed to disable umem!\n");
> > +                     WARN(1, "Failed to disable zero-copy!\n");
> >       }
> >
> > -     xsk_clear_pool_at_qid(umem->dev, umem->queue_id);
> > +     xsk_clear_pool_at_qid(pool->netdev, pool->queue_id);
> > +     dev_put(pool->netdev);
> > +     pool->netdev = NULL;
> >   }
> >
> >   static void xp_release_deferred(struct work_struct *work)
> > diff --git a/net/xdp/xsk_diag.c b/net/xdp/xsk_diag.c
> > index 52675ea..5bd8ea9 100644
> > --- a/net/xdp/xsk_diag.c
> > +++ b/net/xdp/xsk_diag.c
> > @@ -59,8 +59,8 @@ static int xsk_diag_put_umem(const struct xdp_sock *xs, struct sk_buff *nlskb)
> >       du.num_pages = umem->npgs;
> >       du.chunk_size = umem->chunk_size;
> >       du.headroom = umem->headroom;
> > -     du.ifindex = umem->dev ? umem->dev->ifindex : 0;
> > -     du.queue_id = umem->queue_id;
> > +     du.ifindex = pool->netdev ? pool->netdev->ifindex : 0;
> > +     du.queue_id = pool->queue_id;
> >       du.flags = 0;
> >       if (umem->zc)
> >               du.flags |= XDP_DU_F_ZEROCOPY;
> >
>
diff mbox series

Patch

diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index 2a284e1..b052f1c 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -26,11 +26,8 @@  struct xdp_umem {
 	refcount_t users;
 	struct page **pgs;
 	u32 npgs;
-	u16 queue_id;
-	u8 need_wakeup;
 	u8 flags;
 	int id;
-	struct net_device *dev;
 	bool zc;
 	spinlock_t xsk_tx_list_lock;
 	struct list_head xsk_tx_list;
diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
index 380d9ae..2d94890 100644
--- a/include/net/xsk_buff_pool.h
+++ b/include/net/xsk_buff_pool.h
@@ -43,11 +43,15 @@  struct xsk_buff_pool {
 	u32 headroom;
 	u32 chunk_size;
 	u32 frame_len;
+	u16 queue_id;
+	u8 cached_need_wakeup;
+	bool uses_need_wakeup;
 	bool dma_need_sync;
 	bool unaligned;
 	struct xdp_umem *umem;
 	void *addrs;
 	struct device *dev;
+	struct net_device *netdev;
 	refcount_t users;
 	struct work_struct work;
 	struct xdp_buff_xsk *free_heads[];
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 7d86a63..b1699d0 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -63,26 +63,9 @@  static void xdp_umem_unaccount_pages(struct xdp_umem *umem)
 	}
 }
 
-void xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
-			 u16 queue_id)
-{
-	umem->dev = dev;
-	umem->queue_id = queue_id;
-
-	dev_hold(dev);
-}
-
-void xdp_umem_clear_dev(struct xdp_umem *umem)
-{
-	dev_put(umem->dev);
-	umem->dev = NULL;
-	umem->zc = false;
-}
-
 static void xdp_umem_release(struct xdp_umem *umem)
 {
-	xdp_umem_clear_dev(umem);
-
+	umem->zc = false;
 	ida_simple_remove(&umem_ida, umem->id);
 
 	xdp_umem_unpin_pages(umem);
diff --git a/net/xdp/xdp_umem.h b/net/xdp/xdp_umem.h
index 93e96be..67bf3f3 100644
--- a/net/xdp/xdp_umem.h
+++ b/net/xdp/xdp_umem.h
@@ -8,10 +8,6 @@ 
 
 #include <net/xdp_sock_drv.h>
 
-void xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
-			 u16 queue_id);
-void xdp_umem_clear_dev(struct xdp_umem *umem);
-bool xdp_umem_validate_queues(struct xdp_umem *umem);
 void xdp_get_umem(struct xdp_umem *umem);
 void xdp_put_umem(struct xdp_umem *umem);
 void xdp_add_sk_umem(struct xdp_umem *umem, struct xdp_sock *xs);
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index ee04887..624d0fc 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -41,67 +41,61 @@  bool xsk_is_setup_for_bpf_map(struct xdp_sock *xs)
 
 void xsk_set_rx_need_wakeup(struct xsk_buff_pool *pool)
 {
-	struct xdp_umem *umem = pool->umem;
-
-	if (umem->need_wakeup & XDP_WAKEUP_RX)
+	if (pool->cached_need_wakeup & XDP_WAKEUP_RX)
 		return;
 
 	pool->fq->ring->flags |= XDP_RING_NEED_WAKEUP;
-	umem->need_wakeup |= XDP_WAKEUP_RX;
+	pool->cached_need_wakeup |= XDP_WAKEUP_RX;
 }
 EXPORT_SYMBOL(xsk_set_rx_need_wakeup);
 
 void xsk_set_tx_need_wakeup(struct xsk_buff_pool *pool)
 {
-	struct xdp_umem *umem = pool->umem;
 	struct xdp_sock *xs;
 
-	if (umem->need_wakeup & XDP_WAKEUP_TX)
+	if (pool->cached_need_wakeup & XDP_WAKEUP_TX)
 		return;
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(xs, &umem->xsk_tx_list, list) {
+	list_for_each_entry_rcu(xs, &xs->umem->xsk_tx_list, list) {
 		xs->tx->ring->flags |= XDP_RING_NEED_WAKEUP;
 	}
 	rcu_read_unlock();
 
-	umem->need_wakeup |= XDP_WAKEUP_TX;
+	pool->cached_need_wakeup |= XDP_WAKEUP_TX;
 }
 EXPORT_SYMBOL(xsk_set_tx_need_wakeup);
 
 void xsk_clear_rx_need_wakeup(struct xsk_buff_pool *pool)
 {
-	struct xdp_umem *umem = pool->umem;
-
-	if (!(umem->need_wakeup & XDP_WAKEUP_RX))
+	if (!(pool->cached_need_wakeup & XDP_WAKEUP_RX))
 		return;
 
 	pool->fq->ring->flags &= ~XDP_RING_NEED_WAKEUP;
-	umem->need_wakeup &= ~XDP_WAKEUP_RX;
+	pool->cached_need_wakeup &= ~XDP_WAKEUP_RX;
 }
 EXPORT_SYMBOL(xsk_clear_rx_need_wakeup);
 
 void xsk_clear_tx_need_wakeup(struct xsk_buff_pool *pool)
 {
-	struct xdp_umem *umem = pool->umem;
 	struct xdp_sock *xs;
 
-	if (!(umem->need_wakeup & XDP_WAKEUP_TX))
+	if (!(pool->cached_need_wakeup & XDP_WAKEUP_TX))
 		return;
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(xs, &umem->xsk_tx_list, list) {
+	list_for_each_entry_rcu(xs, &xs->umem->xsk_tx_list, list) {
 		xs->tx->ring->flags &= ~XDP_RING_NEED_WAKEUP;
 	}
 	rcu_read_unlock();
 
-	umem->need_wakeup &= ~XDP_WAKEUP_TX;
+	pool->cached_need_wakeup &= ~XDP_WAKEUP_TX;
 }
 EXPORT_SYMBOL(xsk_clear_tx_need_wakeup);
 
 bool xsk_uses_need_wakeup(struct xsk_buff_pool *pool)
 {
-	return pool->umem->flags & XDP_UMEM_USES_NEED_WAKEUP;
+	return pool->uses_need_wakeup;
 }
 EXPORT_SYMBOL(xsk_uses_need_wakeup);
 
@@ -478,16 +472,16 @@  static __poll_t xsk_poll(struct file *file, struct socket *sock,
 	__poll_t mask = datagram_poll(file, sock, wait);
 	struct sock *sk = sock->sk;
 	struct xdp_sock *xs = xdp_sk(sk);
-	struct xdp_umem *umem;
+	struct xsk_buff_pool *pool;
 
 	if (unlikely(!xsk_is_bound(xs)))
 		return mask;
 
-	umem = xs->umem;
+	pool = xs->pool;
 
-	if (umem->need_wakeup) {
+	if (pool->cached_need_wakeup) {
 		if (xs->zc)
-			xsk_wakeup(xs, umem->need_wakeup);
+			xsk_wakeup(xs, pool->cached_need_wakeup);
 		else
 			/* Poll needs to drive Tx also in copy mode */
 			__xsk_sendmsg(sk);
@@ -731,11 +725,9 @@  static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
 		goto out_unlock;
 	} else {
 		/* This xsk has its own umem. */
-		xdp_umem_assign_dev(xs->umem, dev, qid);
 		xs->pool = xp_create_and_assign_umem(xs, xs->umem);
 		if (!xs->pool) {
 			err = -ENOMEM;
-			xdp_umem_clear_dev(xs->umem);
 			goto out_unlock;
 		}
 
@@ -743,7 +735,6 @@  static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
 		if (err) {
 			xp_destroy(xs->pool);
 			xs->pool = NULL;
-			xdp_umem_clear_dev(xs->umem);
 			goto out_unlock;
 		}
 	}
@@ -1089,7 +1080,6 @@  static int xsk_notifier(struct notifier_block *this,
 
 				/* Clear device references. */
 				xp_clear_dev(xs->pool);
-				xdp_umem_clear_dev(xs->umem);
 			}
 			mutex_unlock(&xs->mutex);
 		}
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 36287d2..436648a 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -95,10 +95,9 @@  void xp_set_rxq_info(struct xsk_buff_pool *pool, struct xdp_rxq_info *rxq)
 }
 EXPORT_SYMBOL(xp_set_rxq_info);
 
-int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
+int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *netdev,
 		  u16 queue_id, u16 flags)
 {
-	struct xdp_umem *umem = pool->umem;
 	bool force_zc, force_copy;
 	struct netdev_bpf bpf;
 	int err = 0;
@@ -111,27 +110,30 @@  int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
 	if (force_zc && force_copy)
 		return -EINVAL;
 
-	if (xsk_get_pool_from_qid(dev, queue_id))
+	if (xsk_get_pool_from_qid(netdev, queue_id))
 		return -EBUSY;
 
-	err = xsk_reg_pool_at_qid(dev, pool, queue_id);
+	err = xsk_reg_pool_at_qid(netdev, pool, queue_id);
 	if (err)
 		return err;
 
 	if (flags & XDP_USE_NEED_WAKEUP) {
-		umem->flags |= XDP_UMEM_USES_NEED_WAKEUP;
+		pool->uses_need_wakeup = true;
 		/* Tx needs to be explicitly woken up the first time.
 		 * Also for supporting drivers that do not implement this
 		 * feature. They will always have to call sendto().
 		 */
-		umem->need_wakeup = XDP_WAKEUP_TX;
+		pool->cached_need_wakeup = XDP_WAKEUP_TX;
 	}
 
+	dev_hold(netdev);
+
 	if (force_copy)
 		/* For copy-mode, we are done. */
 		return 0;
 
-	if (!dev->netdev_ops->ndo_bpf || !dev->netdev_ops->ndo_xsk_wakeup) {
+	if (!netdev->netdev_ops->ndo_bpf ||
+	    !netdev->netdev_ops->ndo_xsk_wakeup) {
 		err = -EOPNOTSUPP;
 		goto err_unreg_pool;
 	}
@@ -140,44 +142,47 @@  int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
 	bpf.xsk.pool = pool;
 	bpf.xsk.queue_id = queue_id;
 
-	err = dev->netdev_ops->ndo_bpf(dev, &bpf);
+	err = netdev->netdev_ops->ndo_bpf(netdev, &bpf);
 	if (err)
 		goto err_unreg_pool;
 
-	umem->zc = true;
+	pool->netdev = netdev;
+	pool->queue_id = queue_id;
+	pool->umem->zc = true;
 	return 0;
 
 err_unreg_pool:
 	if (!force_zc)
 		err = 0; /* fallback to copy mode */
 	if (err)
-		xsk_clear_pool_at_qid(dev, queue_id);
+		xsk_clear_pool_at_qid(netdev, queue_id);
 	return err;
 }
 
 void xp_clear_dev(struct xsk_buff_pool *pool)
 {
-	struct xdp_umem *umem = pool->umem;
 	struct netdev_bpf bpf;
 	int err;
 
 	ASSERT_RTNL();
 
-	if (!umem->dev)
+	if (!pool->netdev)
 		return;
 
-	if (umem->zc) {
+	if (pool->umem->zc) {
 		bpf.command = XDP_SETUP_XSK_POOL;
 		bpf.xsk.pool = NULL;
-		bpf.xsk.queue_id = umem->queue_id;
+		bpf.xsk.queue_id = pool->queue_id;
 
-		err = umem->dev->netdev_ops->ndo_bpf(umem->dev, &bpf);
+		err = pool->netdev->netdev_ops->ndo_bpf(pool->netdev, &bpf);
 
 		if (err)
-			WARN(1, "failed to disable umem!\n");
+			WARN(1, "Failed to disable zero-copy!\n");
 	}
 
-	xsk_clear_pool_at_qid(umem->dev, umem->queue_id);
+	xsk_clear_pool_at_qid(pool->netdev, pool->queue_id);
+	dev_put(pool->netdev);
+	pool->netdev = NULL;
 }
 
 static void xp_release_deferred(struct work_struct *work)
diff --git a/net/xdp/xsk_diag.c b/net/xdp/xsk_diag.c
index 52675ea..5bd8ea9 100644
--- a/net/xdp/xsk_diag.c
+++ b/net/xdp/xsk_diag.c
@@ -59,8 +59,8 @@  static int xsk_diag_put_umem(const struct xdp_sock *xs, struct sk_buff *nlskb)
 	du.num_pages = umem->npgs;
 	du.chunk_size = umem->chunk_size;
 	du.headroom = umem->headroom;
-	du.ifindex = umem->dev ? umem->dev->ifindex : 0;
-	du.queue_id = umem->queue_id;
+	du.ifindex = pool->netdev ? pool->netdev->ifindex : 0;
+	du.queue_id = pool->queue_id;
 	du.flags = 0;
 	if (umem->zc)
 		du.flags |= XDP_DU_F_ZEROCOPY;