On Tue, Jul 28, 2020 at 9:10 AM Björn Töpel <bjorn.topel@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> > 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
