Re: [PATCH v3 net 3/6] xsk: provide sufficient space in pool->tx_descs

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Thu, Jul 16, 2026 at 11:29:49AM +0200, Jason Xing wrote:
> On Tue, Jul 14, 2026 at 4:08 PM Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > The temporary Tx descriptor array in an XSK buffer pool is currently
> > sized from the Tx ring of the socket that creates the pool.
> >
> > This is insufficient for shared-UMEM Tx. A later socket may have a
> > larger Tx ring and submit a valid multi-buffer packet containing more
> > descriptors than the first socket's ring, while still remaining within
> > the device's xdp_zc_max_segs limit.
> >
> > A packet-framed batch parser bounded by the temporary array cannot reach
> > the end-of-packet descriptor in that case. It leaves the packet on the
> > Tx ring and encounters the same packet on every subsequent attempt,
> > stalling Tx processing for that socket.
> >
> > Size the temporary descriptor array to the larger of the first Tx ring
> > and the device's xdp_zc_max_segs capability. This keeps the array large
> > enough to inspect one maximum-sized valid packet. Larger shared Tx rings
> > do not require further resizing, as they can be processed over multiple
> > batches.
> >
> > Following commit will actually address the data path side.
> >
> > Fixes: d5581966040f ("xsk: support ZC Tx multi-buffer in batch API")
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> 
> Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>
> 
> I noticed there is one interesting comment[1] from sashiko, which
> actually I think is valid. Probably we don't need to do it in the fix,
> but we might need to target -next branch. Now the tx_descs is limited
> by the first tx ring.
> 
> [1]:
> "...since tx_descs is not
> reallocated if it already exists, the array size is permanently frozen
> to the first socket's size.
> 
> Should the array be resized to accommodate the largest shared ring, or
> should the batch readers be updated to cap their reads to the array's
> actual size?"

My take here is that this set will not break the user space by providing
at least pool->xdp_zc_max_segs at pool->tx_descs. Let us rely at least a
bit on user's intelligence here - if you would attach one short ring for
whatever reason among with bigger ring to the very same umem and whine
your performance suck, then I'd say it's on you :P

Hope this makes sense!

> 
> Thanks,
> Jason
> 
> > ---
> >  include/net/xsk_buff_pool.h |  6 ++++--
> >  net/xdp/xsk.c               | 10 +++++++---
> >  net/xdp/xsk_buff_pool.c     | 12 ++++++++----
> >  3 files changed, 19 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
> > index ccb3b350001f..f5e737a83055 100644
> > --- a/include/net/xsk_buff_pool.h
> > +++ b/include/net/xsk_buff_pool.h
> > @@ -102,12 +102,14 @@ struct xsk_buff_pool {
> >
> >  /* AF_XDP core. */
> >  struct xsk_buff_pool *xp_create_and_assign_umem(struct xdp_sock *xs,
> > -                                               struct xdp_umem *umem);
> > +                                               struct xdp_umem *umem,
> > +                                               u32 max_segs);
> >  int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
> >                   u16 queue_id, u16 flags);
> >  int xp_assign_dev_shared(struct xsk_buff_pool *pool, struct xdp_sock *umem_xs,
> >                          struct net_device *dev, u16 queue_id);
> > -int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs);
> > +int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs,
> > +                     u32 max_segs);
> >  void xp_destroy(struct xsk_buff_pool *pool);
> >  void xp_get_pool(struct xsk_buff_pool *pool);
> >  bool xp_put_pool(struct xsk_buff_pool *pool);
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index 43791647cf18..385a3f4a1b32 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -1525,7 +1525,8 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
> >                          * and/or device.
> >                          */
> >                         xs->pool = xp_create_and_assign_umem(xs,
> > -                                                            umem_xs->umem);
> > +                                                            umem_xs->umem,
> > +                                                            dev->xdp_zc_max_segs);
> >                         if (!xs->pool) {
> >                                 err = -ENOMEM;
> >                                 sockfd_put(sock);
> > @@ -1557,7 +1558,8 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
> >                          * utilizes
> >                          */
> >                         if (xs->tx && !xs->pool->tx_descs) {
> > -                               err = xp_alloc_tx_descs(xs->pool, xs);
> > +                               err = xp_alloc_tx_descs(xs->pool, xs,
> > +                                                       dev->xdp_zc_max_segs);
> >                                 if (err) {
> >                                         xp_put_pool(xs->pool);
> >                                         xs->pool = NULL;
> > @@ -1575,7 +1577,9 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
> >                 goto out_unlock;
> >         } else {
> >                 /* This xsk has its own umem. */
> > -               xs->pool = xp_create_and_assign_umem(xs, xs->umem);
> > +               xs->pool = xp_create_and_assign_umem(xs, xs->umem,
> > +                                                    dev->xdp_zc_max_segs);
> > +
> >                 if (!xs->pool) {
> >                         err = -ENOMEM;
> >                         goto out_unlock;
> > diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> > index 1f28a9641571..12c9fb29af05 100644
> > --- a/net/xdp/xsk_buff_pool.c
> > +++ b/net/xdp/xsk_buff_pool.c
> > @@ -42,9 +42,12 @@ void xp_destroy(struct xsk_buff_pool *pool)
> >         kvfree(pool);
> >  }
> >
> > -int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs)
> > +int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs,
> > +                     u32 max_segs)
> >  {
> > -       pool->tx_descs = kvzalloc_objs(*pool->tx_descs, xs->tx->nentries);
> > +       u32 nentries = max(xs->tx->nentries, max_segs);
> > +
> > +       pool->tx_descs = kvzalloc_objs(*pool->tx_descs, nentries);
> >         if (!pool->tx_descs)
> >                 return -ENOMEM;
> >
> > @@ -52,7 +55,8 @@ int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs)
> >  }
> >
> >  struct xsk_buff_pool *xp_create_and_assign_umem(struct xdp_sock *xs,
> > -                                               struct xdp_umem *umem)
> > +                                               struct xdp_umem *umem,
> > +                                               u32 max_segs)
> >  {
> >         bool unaligned = umem->flags & XDP_UMEM_UNALIGNED_CHUNK_FLAG;
> >         struct xsk_buff_pool *pool;
> > @@ -69,7 +73,7 @@ struct xsk_buff_pool *xp_create_and_assign_umem(struct xdp_sock *xs,
> >                 goto out;
> >
> >         if (xs->tx)
> > -               if (xp_alloc_tx_descs(pool, xs))
> > +               if (xp_alloc_tx_descs(pool, xs, max_segs))
> >                         goto out;
> >
> >         pool->chunk_mask = ~((u64)umem->chunk_size - 1);
> > --
> > 2.43.0
> >