[PATCH v3 net 0/6] xsk: fix AF_XDP multi-buffer Tx descriptor reclaim

[PATCH v3 net 0/6] xsk: fix AF_XDP multi-buffer Tx descriptor reclaim

[Maciej Fijalkowski]

v2: https://lore.kernel.org/netdev/20260710194424.84844-1-maciej.fijalkowski@intel.com/
v2->v3:

* Added a preceding patch that sizes the pool-wide temporary Tx descriptor
  array to the larger of the first Tx ring and the device's
  xdp_zc_max_segs capability. This guarantees that the shared-UMEM path can
  inspect one maximum-sized valid packet even when the socket that creates
  the pool has a smaller Tx ring. Consequently, this patch now records the
  actual allocated size in pool->tx_descs_nentries rather than the first
  socket's Tx ring size.

* Fixed a possible infinite retry loop when a shared-UMEM socket contains
  an incomplete multi-buffer packet. Pass the original descriptor budget
  to xskq_cons_read_desc_batch() instead of the number of descriptors
  currently available, so the parser can distinguish producer exhaustion
  from actual budget exhaustion.

* Moved the completion-ring space check to the common batched Tx path, so
  it is performed exactly once for both singular and shared-SG pools. Keep
  the legacy shared non-SG fallback outside this handling, as it reserves
  completion entries one descriptor at a time.

* Reworked xsk_tx_peek_release_desc_batch() to use a common singular/shared
  batched flow. Resolve an empty Tx socket list before checking CQ space,
  select the shared-SG walker through an explicit shared-pool condition,
  and commit the resulting batch through one common path.

* Simplified xsk_tx_commit_batch() by moving the cached CQ producer
  snapshot into the helper instead of passing it from each caller.

v1: https://lore.kernel.org/netdev/20260623133240.1048434-1-maciej.fijalkowski@intel.com/
v1->v2:

* Reduced the series from seven to five patches by squashing the three
  generic Tx drain and reclaim changes into a single patch. The resulting
  patch handles overflow, invalid descriptors in the middle of a packet,
  and reclaim of the offending descriptor as one coherent change. This
  so it will be less likely to have things reported by Sashiko that are
  fixed in later commits;

* Reworked the zero-copy implementation substantially:
  * removed the bind-transition mechanism, including tx_share_pending,
    xp_prepare_xsk_tx_share(), xp_finish_xsk_tx_share(),
    synchronize_net(), and the transient bind() -EAGAIN behavior;
  * added packet-framed parsing for shared-UMEM SG pools, allowing
    per-socket drain state to be resumed by both singular and shared Tx
    paths;
  * retained the legacy one-descriptor fallback for shared non-SG pools;
  * preserved the existing per-socket fairness quota while allowing the
    shared walker to consume multiple complete packets and continue
    filling the requested batch across fairness rounds;
  * made the fairness quota large enough to process one maximum-sized
    valid multi-buffer packet;
  * extended the parser result with consumed-descriptor and
    budget-limited accounting needed by the shared walker;
  * recorded the size of the pool's temporary Tx descriptor array and
    capped batch processing at that size;
  * kept reclaim-only descriptors ordered after preceding driver-visible
    descriptors and protected the delayed-reclaim state with
    READ_ONCE()/WRITE_ONCE().

* Rewrote the zero-copy patch description to cover oversized packets,
  continuation draining across calls, shared-UMEM SG handling, and CQ
  publication ordering.

* Corrected the too-many-frags selftest description to state that the
  invalid packet contains max_frags + 1 fragments and terminates at an
  explicit packet boundary.


Hi,

This series fixes several AF_XDP multi-buffer Tx paths where descriptors
consumed from the Tx ring are not consistently returned to userspace
through the completion ring when the packet is later dropped as invalid.

The affected cases are invalid or oversized multi-buffer Tx packets in
both the generic and zero-copy paths. In these cases, the kernel can
consume one or more Tx descriptors while building or validating a
multi-buffer packet, then drop the packet before it reaches the device.
Userspace still owns the UMEM buffers only after the corresponding
addresses are returned through the CQ. Missing completions therefore
make userspace lose track of those buffers.

The generic path fixes cover following related cases:
* partially built multi-buffer skbs dropped by xsk_drop_skb();
  continuation descriptors left in the Tx ring after xsk_build_skb()
  reports overflow;
* invalid descriptors encountered in the middle of a multi-buffer
  packet, including the offending invalid descriptor itself.

The zero-copy path is handled separately. The batched Tx parser now
distinguishes descriptors that can be passed to the driver from
descriptors that are consumed only because they belong to an invalid
multi-buffer packet. Reclaim-only descriptors are written to the CQ
address area and published in completion order, after any earlier
driver-visible Tx descriptors.

The last two patches update xskxceiver so the tests account invalid
multi-buffer Tx packets as descriptors that must be reclaimed, while
still not expecting those invalid packets on the Rx side.

This is a follow-up to Jason's changes [0] which were addressing generic
xmit only and this set allows me to pass full xskxceiver test suite run
against ice driver.

Thanks,
Maciej

[0]: https://lore.kernel.org/netdev/20260520004244.55663-1-kerneljasonxing@gmail.com/


Jason Xing (2):
  xsk: fix buffer leak in xsk_drop_skb() for AF_XDP multi-buffer Tx
  xsk: drain continuation descs after overflow in xsk_build_skb()

Maciej Fijalkowski (4):
  xsk: provide sufficient space in pool->tx_descs
  xsk: reclaim invalid multi-buffer Tx descs in ZC path
  selftests/xsk: fix too-many-frags multi-buffer Tx test
  selftests/xsk: account invalid multi-buffer Tx descriptors

 include/net/xdp_sock.h                        |   1 +
 include/net/xsk_buff_pool.h                   |   9 +-
 net/xdp/xsk.c                                 | 254 ++++++++++++++++--
 net/xdp/xsk_buff_pool.c                       |  13 +-
 net/xdp/xsk_queue.h                           |  76 ++++--
 .../selftests/bpf/prog_tests/test_xsk.c       |  48 ++--
 6 files changed, 329 insertions(+), 72 deletions(-)

-- 
2.43.0

[PATCH v3 net 1/6] xsk: fix buffer leak in xsk_drop_skb() for AF_XDP multi-buffer Tx

[Maciej Fijalkowski]

From: Jason Xing <kernelxing@tencent.com>

This patch is inspired by the check[1] from sashiko. It says when
overflow happens, the address of cq to be published is invalid.
Actually the severer thing is the whole process of publishing the
address of cq in this particular case is not right: it should truely
publish the address and advance the cached_prod in cq as long as it
reads descriptors from txq.

The following is the full analysis.
xsk_drop_skb() is called in three places, which all discard a partially
built multi-buffer skb:
1) xsk_build_skb() -EOVERFLOW error path: packet exceeds MAX_SKB_FRAGS
2) __xsk_generic_xmit() post-loop cleanup: an invalid descriptor in
   the TX ring prevents the partial packet from completing
3) xsk_release(): socket close while xs->skb holds an incomplete packet

In all three cases, the TX descriptors for the already-processed frags
have been consumed from the TX ring (xskq_cons_release), and CQ slots
have been reserved. However, xsk_drop_skb() calls xsk_consume_skb()
which cancels the CQ reservations via xsk_cq_cancel_locked(). Since
the buffer addresses never appear in the completion queue, userspace
permanently loses track of these buffers.

Fix this by letting consume_skb() trigger the existing xsk_destruct_skb
destructor, which already submits buffer addresses to the CQ via
xsk_cq_submit_addr_locked().

Note that cancelling the descriptors back to the TX ring (via
xskq_cons_cancel_n) is not a appropriate option because an oversized
packet that always exceeds MAX_SKB_FRAGS would be retried indefinitely,
which is an obviously deadlock bug in the TX path.

Also move the desc->addr assignment in xsk_build_skb() above the
overflow check so that the current descriptor's address is recorded
before a potential -EOVERFLOW jump to free_err, consistent with the
zerocopy path in xsk_build_skb_zerocopy().

[1]: https://lore.kernel.org/all/20260425041726.85FB3C2BCB2@smtp.kernel.org/

Fixes: cf24f5a5feea ("xsk: add support for AF_XDP multi-buffer on Tx path")
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 net/xdp/xsk.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index b970f30ea9b9..a7a83dc4546a 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -794,8 +794,11 @@ static void xsk_consume_skb(struct sk_buff *skb)
 
 static void xsk_drop_skb(struct sk_buff *skb)
 {
-	xdp_sk(skb->sk)->tx->invalid_descs += xsk_get_num_desc(skb);
-	xsk_consume_skb(skb);
+	struct xdp_sock *xs = xdp_sk(skb->sk);
+
+	xs->tx->invalid_descs += xsk_get_num_desc(skb);
+	consume_skb(skb);
+	xs->skb = NULL;
 }
 
 static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
@@ -877,7 +880,7 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
 			return ERR_PTR(-ENOMEM);
 
 		/* in case of -EOVERFLOW that could happen below,
-		 * xsk_consume_skb() will release this node as whole skb
+		 * xsk_drop_skb() will release this node as whole skb
 		 * would be dropped, which implies freeing all list elements
 		 */
 		xsk_addr->addrs[xsk_addr->num_descs] = desc->addr;
@@ -969,6 +972,8 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
 				goto free_err;
 			}
 
+			xsk_addr->addrs[xsk_addr->num_descs] = desc->addr;
+
 			if (unlikely(nr_frags == (MAX_SKB_FRAGS - 1) && xp_mb_desc(desc))) {
 				err = -EOVERFLOW;
 				goto free_err;
@@ -986,8 +991,6 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
 
 			skb_add_rx_frag(skb, nr_frags, page, 0, len, PAGE_SIZE);
 			refcount_add(PAGE_SIZE, &xs->sk.sk_wmem_alloc);
-
-			xsk_addr->addrs[xsk_addr->num_descs] = desc->addr;
 		}
 	}
 
-- 
2.43.0

[PATCH v3 net 2/6] xsk: drain continuation descs after overflow in xsk_build_skb()

[Maciej Fijalkowski]

From: Jason Xing <kernelxing@tencent.com>

Fix generic xmit path multi-buffer logic when packets are either too big
(count of descriptors exceed MAX_SKB_FRAGS) or an invalid descriptor is
included in fragmented packet. Introduce xdp_sock::drain_cont and act
upon this flag - when it is set, keep on consuming descriptors from
AF_XDP Tx ring and put them directly onto Cq. Previously these
descriptors were silently lost and could never be reached again.

Fixes: cf24f5a5feea ("xsk: add support for AF_XDP multi-buffer on Tx path")
Closes: https://lore.kernel.org/all/20260425041726.85FB3C2BCB2@smtp.kernel.org/
Co-developed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> # wrapped cq addr submission onto routine
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 include/net/xdp_sock.h |  1 +
 net/xdp/xsk.c          | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index ebac60a3d8a1..8b51876efbed 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -80,6 +80,7 @@ struct xdp_sock {
 	 * call of __xsk_generic_xmit().
 	 */
 	struct sk_buff *skb;
+	bool drain_cont;
 
 	struct list_head map_list;
 	/* Protects map_list */
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index a7a83dc4546a..43791647cf18 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -737,6 +737,19 @@ static void xsk_cq_submit_addr_locked(struct xsk_buff_pool *pool,
 	spin_unlock_irqrestore(&pool->cq_prod_lock, flags);
 }
 
+static void xsk_cq_submit_addr_single_locked(struct xsk_buff_pool *pool,
+					     struct xdp_desc *desc)
+{
+	unsigned long flags;
+	u32 idx;
+
+	spin_lock_irqsave(&pool->cq_prod_lock, flags);
+	idx = xskq_get_prod(pool->cq);
+	xskq_prod_write_addr(pool->cq, idx, desc->addr);
+	xskq_prod_submit_n(pool->cq, 1);
+	spin_unlock_irqrestore(&pool->cq_prod_lock, flags);
+}
+
 static void xsk_cq_cancel_locked(struct xsk_buff_pool *pool, u32 n)
 {
 	spin_lock(&pool->cq->cq_cached_prod_lock);
@@ -1063,11 +1076,22 @@ static int __xsk_generic_xmit(struct sock *sk)
 			goto out;
 		}
 
+		if (unlikely(xs->drain_cont)) {
+			xsk_cq_submit_addr_single_locked(xs->pool, &desc);
+
+			xs->tx->invalid_descs++;
+			xskq_cons_release(xs->tx);
+			xs->drain_cont = xp_mb_desc(&desc);
+			continue;
+		}
+
 		skb = xsk_build_skb(xs, &desc);
 		if (IS_ERR(skb)) {
 			err = PTR_ERR(skb);
 			if (err != -EOVERFLOW)
 				goto out;
+			if (xp_mb_desc(&desc))
+				xs->drain_cont = true;
 			err = 0;
 			continue;
 		}
@@ -1101,9 +1125,24 @@ static int __xsk_generic_xmit(struct sock *sk)
 	}
 
 	if (xskq_has_descs(xs->tx)) {
+		bool reclaim_desc = xs->skb || xs->drain_cont;
+
+		if (reclaim_desc) {
+			err = xsk_cq_reserve_locked(xs->pool);
+			if (err) {
+				err = -EAGAIN;
+				goto out;
+			}
+		}
+
 		if (xs->skb)
 			xsk_drop_skb(xs->skb);
+
+		if (reclaim_desc)
+			xsk_cq_submit_addr_single_locked(xs->pool, &desc);
+
 		xskq_cons_release(xs->tx);
+		xs->drain_cont = xp_mb_desc(&desc);
 	}
 
 out:
-- 
2.43.0

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

[Maciej Fijalkowski]

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>
---
 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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Heap buffer overflow in AF_XDP TX batch processing due to undersized `pool->tx_descs` in shared UMEM configurations.
--

--- Patch [3]: [PATCH v3 net 3/6] xsk: provide sufficient space in pool->tx_descs ---
commit 6f635cabf694299ed3f7be4ce0e5f328e6bd90e0
Author: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

xsk: provide sufficient space in pool->tx_descs

The temporary Tx descriptor array in an XSK buffer pool is currently
sized from the Tx ring of the socket that creates the pool.

[ ... ]

> 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.

[Severity: High]
This is a pre-existing issue, but is the statement that larger shared Tx
rings do not require further resizing accurate?

If a secondary socket shares the UMEM and has a much larger TX ring than
the first socket, pool->tx_descs is not resized, locking its size to
max(first_socket_nentries, dev->xdp_zc_max_segs).

However, the batch reading functions don't seem to bound their reads to
the allocated size of the array. They appear to read up to the driver's
budget (e.g., 64) and the current socket's available elements. 

Could this cause a heap buffer overflow when writing into descs?

net/xdp/xsk.c:xsk_tx_peek_release_fallback() {
    ...
	while (nb_pkts < max_entries && xsk_tx_peek_desc(pool, &descs[nb_pkts]))
		nb_pkts++;
    ...
}

net/xdp/xsk_queue.h:xskq_cons_read_desc_batch() {
    ...
	while (cached_cons != q->cached_prod && nb_entries < max) {
		...
		descs[nb_entries] = ring->desc[idx];
		cached_cons++;
    ...
}

If a user binds a minimal first socket (e.g. 2 entries) and a large shared
second socket, the softirq batch process could write past the end of the
array. Since ring->desc is memory-mapped to userspace, could an attacker
control the overwritten heap memory?

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 43791647cf182..385a3f4a1b320 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -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);

This isn't a bug introduced by this patch, but 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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714140722.111645-1-maciej.fijalkowski@intel.com?part=3

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

[Maciej Fijalkowski]

On Wed, Jul 15, 2026 at 02:08:44PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Heap buffer overflow in AF_XDP TX batch processing due to undersized `pool->tx_descs` in shared UMEM configurations.
> --
> 
> --- Patch [3]: [PATCH v3 net 3/6] xsk: provide sufficient space in pool->tx_descs ---
> commit 6f635cabf694299ed3f7be4ce0e5f328e6bd90e0
> Author: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> 
> xsk: provide sufficient space in pool->tx_descs
> 
> The temporary Tx descriptor array in an XSK buffer pool is currently
> sized from the Tx ring of the socket that creates the pool.
> 
> [ ... ]
> 
> > 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.
> 
> [Severity: High]
> This is a pre-existing issue, but is the statement that larger shared Tx
> rings do not require further resizing accurate?
> 
> If a secondary socket shares the UMEM and has a much larger TX ring than
> the first socket, pool->tx_descs is not resized, locking its size to
> max(first_socket_nentries, dev->xdp_zc_max_segs).
> 
> However, the batch reading functions don't seem to bound their reads to
> the allocated size of the array. They appear to read up to the driver's
> budget (e.g., 64) and the current socket's available elements. 
> 
> Could this cause a heap buffer overflow when writing into descs?

This is clamped on later patch:

+	nb_pkts = min(nb_pkts, pool->tx_descs_nentries);
+	if (!nb_pkts)
+		goto out;
+
+	umem_shared = !list_is_singular(&pool->xsk_tx_list);
+
+	if (umem_shared && !(pool->umem->flags & XDP_UMEM_SG_FLAG)) {
+		rcu_read_unlock();
+		return xsk_tx_peek_release_fallback(pool, nb_pkts);
+	}

> 
> net/xdp/xsk.c:xsk_tx_peek_release_fallback() {
>     ...
> 	while (nb_pkts < max_entries && xsk_tx_peek_desc(pool, &descs[nb_pkts]))
> 		nb_pkts++;
>     ...
> }
> 
> net/xdp/xsk_queue.h:xskq_cons_read_desc_batch() {
>     ...
> 	while (cached_cons != q->cached_prod && nb_entries < max) {
> 		...
> 		descs[nb_entries] = ring->desc[idx];
> 		cached_cons++;
>     ...
> }
> 
> If a user binds a minimal first socket (e.g. 2 entries) and a large shared
> second socket, the softirq batch process could write past the end of the
> array. Since ring->desc is memory-mapped to userspace, could an attacker
> control the overwritten heap memory?
> 
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index 43791647cf182..385a3f4a1b320 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -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);
> 
> This isn't a bug introduced by this patch, but 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?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260714140722.111645-1-maciej.fijalkowski@intel.com?part=3

[PATCH v3 net 4/6] xsk: reclaim invalid multi-buffer Tx descs in ZC path

[Maciej Fijalkowski]

The zero-copy Tx batch parser stops when it encounters an invalid
descriptor. If this happens after one or more continuation descriptors,
the Tx consumer can be advanced past fragments that are neither submitted
to the driver nor returned to userspace through the completion ring.

A similar problem occurs when a packet exceeds xdp_zc_max_segs. The
descriptors consumed up to the limit are released without completion, and
the remaining continuation descriptors can subsequently be interpreted
as the beginning of another packet.

Parse Tx batches in packet units and distinguish descriptors belonging to
complete valid packets from descriptors consumed while draining an
invalid or oversized packet. Return the former to the driver and append
the latter to the CQ address area so userspace can reclaim their UMEM
frames.

Once draining starts, continue until the packet's end-of-packet
descriptor is consumed. Preserve the drain state on the socket when EOP
has not yet been supplied, so draining can continue during a later call.
Leave incomplete but otherwise valid packets on the Tx ring. Keep the
existing handling of standalone invalid descriptors unchanged.

Shared-UMEM pools using multi-buffer Tx also need packet-framed parsing.
Walk their Tx sockets one packet at a time, preserving the existing
per-socket fairness scheme, instead of using the legacy one-descriptor
fallback. Keep that fallback for shared pools that do not use
multi-buffer Tx. Since the drain state is maintained per socket and both
the singular and shared paths can resume an interrupted drain, changing
the socket list from singular to shared requires no special bind-time
transition.

CQ entries are positional, and drivers may complete only part of the Tx
work returned by xsk_tx_peek_release_desc_batch(). Therefore, reclaim-only
entries cannot be published immediately when earlier driver-visible
descriptors are still outstanding.

Track the number of driver-visible CQ entries preceding the reclaim
entries. Let xsk_tx_completed() publish partial real Tx completions, and
publish the reclaim entries only after every earlier Tx descriptor has
completed. Complete a reclaim-only batch immediately when there is no
driver-visible work in front of it, and prevent another Tx batch from
being appended while reclaim entries remain pending.

Also cap batch processing by the size of the pool's temporary descriptor
array, as Tx rings belonging to sockets sharing a UMEM may have different
sizes.

This ensures that every descriptor consumed as part of an invalid
multi-buffer packet is eventually returned to userspace without exposing
the dropped packet to the driver or violating CQ completion ordering.

Fixes: cf24f5a5feea ("xsk: add support for AF_XDP multi-buffer on Tx path")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 include/net/xsk_buff_pool.h |   3 +
 net/xdp/xsk.c               | 192 ++++++++++++++++++++++++++++++++----
 net/xdp/xsk_buff_pool.c     |   1 +
 net/xdp/xsk_queue.h         |  76 ++++++++++----
 4 files changed, 232 insertions(+), 40 deletions(-)

diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
index f5e737a83055..2bb1d122b1bc 100644
--- a/include/net/xsk_buff_pool.h
+++ b/include/net/xsk_buff_pool.h
@@ -78,6 +78,9 @@ struct xsk_buff_pool {
 	u32 chunk_size;
 	u32 chunk_shift;
 	u32 frame_len;
+	u32 tx_descs_nentries;
+	u32 reclaim_descs;
+	u32 tx_zc_pending_descs;
 	u32 xdp_zc_max_segs;
 	u8 tx_metadata_len; /* inherited from umem */
 	u8 cached_need_wakeup;
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 385a3f4a1b32..2909a0ec6837 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -499,6 +499,23 @@ void __xsk_map_flush(struct list_head *flush_list)
 
 void xsk_tx_completed(struct xsk_buff_pool *pool, u32 nb_entries)
 {
+	u32 reclaim_descs = READ_ONCE(pool->reclaim_descs);
+
+	if (unlikely(reclaim_descs)) {
+		u32 pending_descs = READ_ONCE(pool->tx_zc_pending_descs);
+
+		if (nb_entries < pending_descs) {
+			WRITE_ONCE(pool->tx_zc_pending_descs,
+				   pending_descs - nb_entries);
+			xskq_prod_submit_n(pool->cq, nb_entries);
+			return;
+		}
+
+		WRITE_ONCE(pool->tx_zc_pending_descs, 0);
+		nb_entries += reclaim_descs;
+		WRITE_ONCE(pool->reclaim_descs, 0);
+	}
+
 	xskq_prod_submit_n(pool->cq, nb_entries);
 }
 EXPORT_SYMBOL(xsk_tx_completed);
@@ -574,24 +591,162 @@ static u32 xsk_tx_peek_release_fallback(struct xsk_buff_pool *pool, u32 max_entr
 	return nb_pkts;
 }
 
+static void xsk_tx_commit_batch(struct xsk_buff_pool *pool,
+				struct xsk_tx_batch *batch)
+{
+	u32 nb_descs = xsk_tx_batch_cq_descs(batch);
+	u32 cq_cached_prod;
+
+	if (!nb_descs)
+		return;
+
+	cq_cached_prod = pool->cq->cached_prod;
+	xskq_prod_write_addr_batch(pool->cq, pool->tx_descs, nb_descs);
+
+	if (unlikely(batch->reclaim_descs)) {
+		u32 cq_pending_descs;
+
+		/* CQ is positional. Descriptors already written but not
+		 * submitted must complete before any reclaim-only descriptors
+		 * appended below.
+		 */
+		cq_pending_descs = cq_cached_prod - xskq_get_prod(pool->cq);
+
+		WRITE_ONCE(pool->tx_zc_pending_descs,
+			   batch->tx_descs + cq_pending_descs);
+		WRITE_ONCE(pool->reclaim_descs, batch->reclaim_descs);
+		if (unlikely(!pool->tx_zc_pending_descs))
+			xsk_tx_completed(pool, 0);
+	}
+}
+
+static struct xsk_tx_batch
+__xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, struct xdp_sock *xs,
+				 struct xdp_desc *descs, u32 max_descs)
+{
+	struct xsk_tx_batch batch = {};
+	u32 entries;
+
+	entries = xskq_cons_nb_entries(xs->tx, max_descs);
+	if (!entries)
+		return batch;
+
+	batch = xskq_cons_read_desc_batch(xs, pool, descs, max_descs);
+	if (!xsk_tx_batch_cq_descs(&batch)) {
+		xs->tx->queue_empty_descs++;
+		if (batch.consumed_descs) {
+			__xskq_cons_release(xs->tx);
+			xs->sk.sk_write_space(&xs->sk);
+		}
+		return batch;
+	}
+
+	__xskq_cons_release(xs->tx);
+	xs->sk.sk_write_space(&xs->sk);
+	return batch;
+}
+
+static struct xsk_tx_batch
+xsk_tx_peek_release_shared_desc_batch(struct xsk_buff_pool *pool, u32 max_descs)
+{
+	u32 cq_descs_before, cq_descs_after;
+	struct xsk_tx_batch sum_batch = {};
+	bool budget_exhausted;
+	u32 per_socket_budget;
+	struct xdp_sock *xs;
+
+	/* The fairness quota must allow one maximum-sized valid packet. */
+	per_socket_budget = max_t(u32, MAX_PER_SOCKET_BUDGET,
+				  pool->xdp_zc_max_segs);
+
+again:
+	budget_exhausted = false;
+	cq_descs_before = xsk_tx_batch_cq_descs(&sum_batch);
+	list_for_each_entry_rcu(xs, &pool->xsk_tx_list, tx_list) {
+		u32 budget, budget_left, offset, remaining;
+		struct xsk_tx_batch curr_batch;
+
+		/* Once reclaim-only descriptors have been appended to the CQ
+		 * address area, do not append driver-visible Tx descriptors
+		 * from another socket after them. xsk_tx_completed() relies on
+		 * all driver-visible descriptors preceding all reclaim-only
+		 * descriptors in CQ order.
+		 */
+		if (sum_batch.reclaim_descs)
+			break;
+
+		/* be gentle when playing with pool->tx_descs */
+		offset = xsk_tx_batch_cq_descs(&sum_batch);
+		if (offset >= max_descs)
+			break;
+
+		if (xs->tx_budget_spent >= per_socket_budget) {
+			if (xskq_cons_nb_entries(xs->tx, 1))
+				budget_exhausted = true;
+			continue;
+		}
+
+		budget_left = per_socket_budget - xs->tx_budget_spent;
+		remaining = max_descs - offset;
+		budget = min(remaining, budget_left);
+
+		curr_batch = __xsk_tx_peek_release_desc_batch(pool, xs,
+							      pool->tx_descs + offset,
+							      budget);
+		if (!xsk_tx_batch_cq_descs(&curr_batch)) {
+			if (curr_batch.budget_limited && budget_left < remaining)
+				budget_exhausted = true;
+			xs->tx_budget_spent += curr_batch.consumed_descs;
+			continue;
+		}
+
+		xs->tx_budget_spent += curr_batch.consumed_descs;
+		sum_batch.tx_descs += curr_batch.tx_descs;
+		sum_batch.reclaim_descs += curr_batch.reclaim_descs;
+	}
+
+	cq_descs_after = xsk_tx_batch_cq_descs(&sum_batch);
+
+	if (sum_batch.reclaim_descs || cq_descs_after >= max_descs)
+		return sum_batch;
+
+	/* Continue filling the batch while this pass made progress */
+	if (cq_descs_before != cq_descs_after)
+		goto again;
+
+	if (!budget_exhausted)
+		return sum_batch;
+
+	list_for_each_entry_rcu(xs, &pool->xsk_tx_list, tx_list)
+		xs->tx_budget_spent = 0;
+	goto again;
+}
+
 u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 nb_pkts)
 {
+	struct xsk_tx_batch batch = {};
 	struct xdp_sock *xs;
+	bool umem_shared;
 
 	rcu_read_lock();
-	if (!list_is_singular(&pool->xsk_tx_list)) {
-		/* Fallback to the non-batched version */
-		rcu_read_unlock();
-		return xsk_tx_peek_release_fallback(pool, nb_pkts);
-	}
+	if (unlikely(READ_ONCE(pool->reclaim_descs)))
+		goto out;
 
-	xs = list_first_or_null_rcu(&pool->xsk_tx_list, struct xdp_sock, tx_list);
-	if (!xs) {
-		nb_pkts = 0;
+	xs = list_first_or_null_rcu(&pool->xsk_tx_list, struct xdp_sock,
+				    tx_list);
+	if (!xs)
 		goto out;
-	}
 
-	nb_pkts = xskq_cons_nb_entries(xs->tx, nb_pkts);
+	nb_pkts = min(nb_pkts, pool->tx_descs_nentries);
+	if (!nb_pkts)
+		goto out;
+
+	umem_shared = !list_is_singular(&pool->xsk_tx_list);
+
+	if (umem_shared && !(pool->umem->flags & XDP_UMEM_SG_FLAG)) {
+		rcu_read_unlock();
+		return xsk_tx_peek_release_fallback(pool, nb_pkts);
+	}
 
 	/* This is the backpressure mechanism for the Tx path. Try to
 	 * reserve space in the completion queue for all packets, but
@@ -603,19 +758,16 @@ u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 nb_pkts)
 	if (!nb_pkts)
 		goto out;
 
-	nb_pkts = xskq_cons_read_desc_batch(xs->tx, pool, nb_pkts);
-	if (!nb_pkts) {
-		xs->tx->queue_empty_descs++;
-		goto out;
-	}
-
-	__xskq_cons_release(xs->tx);
-	xskq_prod_write_addr_batch(pool->cq, pool->tx_descs, nb_pkts);
-	xs->sk.sk_write_space(&xs->sk);
+	batch = umem_shared ?
+		xsk_tx_peek_release_shared_desc_batch(pool, nb_pkts) :
+		__xsk_tx_peek_release_desc_batch(pool, xs,
+						 pool->tx_descs,
+						 nb_pkts);
+	xsk_tx_commit_batch(pool, &batch);
 
 out:
 	rcu_read_unlock();
-	return nb_pkts;
+	return batch.tx_descs;
 }
 EXPORT_SYMBOL(xsk_tx_peek_release_desc_batch);
 
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 12c9fb29af05..a4089480b22b 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -51,6 +51,7 @@ int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs,
 	if (!pool->tx_descs)
 		return -ENOMEM;
 
+	pool->tx_descs_nentries = nentries;
 	return 0;
 }
 
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index 3e3fbb73d23e..a15ff1929db6 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -58,6 +58,18 @@ struct parsed_desc {
 	u32 valid;
 };
 
+struct xsk_tx_batch {
+	u32 tx_descs;
+	u32 reclaim_descs;
+	u32 consumed_descs;
+	bool budget_limited;
+};
+
+static inline u32 xsk_tx_batch_cq_descs(const struct xsk_tx_batch *batch)
+{
+	return batch->tx_descs + batch->reclaim_descs;
+}
+
 /* The structure of the shared state of the rings are a simple
  * circular buffer, as outlined in
  * Documentation/core-api/circular-buffers.rst. For the Rx and
@@ -263,17 +275,18 @@ static inline void parse_desc(struct xsk_queue *q, struct xsk_buff_pool *pool,
 	parsed->mb = xp_mb_desc(desc);
 }
 
-static inline
-u32 xskq_cons_read_desc_batch(struct xsk_queue *q, struct xsk_buff_pool *pool,
-			      u32 max)
+static inline struct xsk_tx_batch
+xskq_cons_read_desc_batch(struct xdp_sock *xs, struct xsk_buff_pool *pool,
+			  struct xdp_desc *descs, u32 max)
 {
-	u32 cached_cons = q->cached_cons, nb_entries = 0;
-	struct xdp_desc *descs = pool->tx_descs;
-	u32 total_descs = 0, nr_frags = 0;
+	bool drain = READ_ONCE(xs->drain_cont);
+	u32 cached_cons, nb_entries = 0, released;
+	struct xsk_tx_batch batch = {};
+	struct xsk_queue *q = xs->tx;
+	u32 nr_frags = 0;
+
+	cached_cons = q->cached_cons;
 
-	/* track first entry, if stumble upon *any* invalid descriptor, rewind
-	 * current packet that consists of frags and stop the processing
-	 */
 	while (cached_cons != q->cached_prod && nb_entries < max) {
 		struct xdp_rxtx_ring *ring = (struct xdp_rxtx_ring *)q->ring;
 		u32 idx = cached_cons & q->ring_mask;
@@ -282,26 +295,49 @@ u32 xskq_cons_read_desc_batch(struct xsk_queue *q, struct xsk_buff_pool *pool,
 		descs[nb_entries] = ring->desc[idx];
 		cached_cons++;
 		parse_desc(q, pool, &descs[nb_entries], &parsed);
-		if (unlikely(!parsed.valid))
-			break;
+		if (unlikely(!parsed.valid)) {
+			if (!drain && !nr_frags && !parsed.mb)
+				break;
+
+			drain = true;
+		}
+
+		nr_frags++;
+		nb_entries++;
 
 		if (likely(!parsed.mb)) {
-			total_descs += (nr_frags + 1);
-			nr_frags = 0;
-		} else {
-			nr_frags++;
-			if (nr_frags == pool->xdp_zc_max_segs) {
+			if (unlikely(drain)) {
+				batch.reclaim_descs = nr_frags;
+				WRITE_ONCE(xs->drain_cont, false);
 				nr_frags = 0;
 				break;
 			}
+
+			batch.tx_descs += nr_frags;
+			nr_frags = 0;
+			continue;
+		}
+
+		if (nr_frags == pool->xdp_zc_max_segs)
+			drain = true;
+	}
+
+	if (nr_frags) {
+		if (drain) {
+			batch.reclaim_descs = nr_frags;
+			WRITE_ONCE(xs->drain_cont, true);
+		} else {
+			if (nb_entries == max)
+				batch.budget_limited = true;
+			cached_cons -= nr_frags;
 		}
-		nb_entries++;
 	}
 
-	cached_cons -= nr_frags;
+	released = cached_cons - q->cached_cons;
 	/* Release valid plus any invalid entries */
-	xskq_cons_release_n(q, cached_cons - q->cached_cons);
-	return total_descs;
+	xskq_cons_release_n(q, released);
+	batch.consumed_descs = released;
+	return batch;
 }
 
 /* Functions for consumers */
-- 
2.43.0

Re: [PATCH v3 net 4/6] xsk: reclaim invalid multi-buffer Tx descs in ZC path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Processing an invalid descriptor or draining an invalid multi-buffer packet at the beginning of a Tx batch stalls the Tx queue.
--

--- Patch [4]: [PATCH v3 net 4/6] xsk: reclaim invalid multi-buffer Tx descs in ZC path ---
commit f71b54a29f7fa6021e0f05cf7b9e1dfd86287c21
Author: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

xsk: reclaim invalid multi-buffer Tx descs in ZC path

This commit updates the zero-copy Tx batch parser to process descriptors in
packet units rather than individually. It ensures that descriptors belonging
to invalid or oversized multi-buffer packets are properly drained and
returned to the completion ring for reclamation without being sent to the
driver.

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 385a3f4a1b320..2909a0ec68373 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c

[ ... ]

> @@ -603,19 +758,16 @@ u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 nb_pkts)
>  	if (!nb_pkts)
>  		goto out;
>  
> -	nb_pkts = xskq_cons_read_desc_batch(xs->tx, pool, nb_pkts);
> -	if (!nb_pkts) {
> -		xs->tx->queue_empty_descs++;
> -		goto out;
> -	}
> -
> -	__xskq_cons_release(xs->tx);
> -	xskq_prod_write_addr_batch(pool->cq, pool->tx_descs, nb_pkts);
> -	xs->sk.sk_write_space(&xs->sk);
> +	batch = umem_shared ?
> +		xsk_tx_peek_release_shared_desc_batch(pool, nb_pkts) :
> +		__xsk_tx_peek_release_desc_batch(pool, xs,
> +						 pool->tx_descs,
> +						 nb_pkts);
> +	xsk_tx_commit_batch(pool, &batch);
>  
>  out:
>  	rcu_read_unlock();
> -	return nb_pkts;
> +	return batch.tx_descs;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but returning 0 to the driver
here from xsk_tx_peek_release_desc_batch when the batch consists solely of
reclaimed descriptors signals that the Tx ring is empty.

Since user space typically waits for hardware completions before manually
triggering sendmsg again, does returning 0 here result in a permanent
Tx queue stall until an unrelated Rx packet arrives to trigger an interrupt?

> diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
> index 3e3fbb73d23e2..a15ff1929db62 100644
> --- a/net/xdp/xsk_queue.h
> +++ b/net/xdp/xsk_queue.h

[ ... ]

> @@ -282,26 +295,49 @@ u32 xskq_cons_read_desc_batch(struct xsk_queue *q, struct xsk_buff_pool *pool,
>  		descs[nb_entries] = ring->desc[idx];
>  		cached_cons++;
>  		parse_desc(q, pool, &descs[nb_entries], &parsed);
> -		if (unlikely(!parsed.valid))
> -			break;
> +		if (unlikely(!parsed.valid)) {
> +			if (!drain && !nr_frags && !parsed.mb)
> +				break;

[Severity: High]
This is a pre-existing issue, but when xskq_cons_read_desc_batch encounters
a standalone invalid descriptor at the very start of a batch, it breaks
early out of this processing loop.

If no valid descriptors were accumulated yet, doesn't this result in
returning an empty batch to the driver, causing NAPI polling to halt
prematurely and stranding any subsequent valid packets on the ring?

> +
> +			drain = true;
> +		}
> +
> +		nr_frags++;
> +		nb_entries++;
>  
>  		if (likely(!parsed.mb)) {
> -			total_descs += (nr_frags + 1);
> -			nr_frags = 0;
> -		} else {
> -			nr_frags++;
> -			if (nr_frags == pool->xdp_zc_max_segs) {
> +			if (unlikely(drain)) {
> +				batch.reclaim_descs = nr_frags;
> +				WRITE_ONCE(xs->drain_cont, false);
>  				nr_frags = 0;
>  				break;
>  			}

[Severity: High]
Similarly, this is a pre-existing issue, but breaking early here when
finishing a drained packet will leave batch.tx_descs as 0 if no valid
packets were seen first.

Could this stall the Tx queue, since no valid packets are submitted to the
hardware to generate the Tx completion interrupts that user space needs
to resume operation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714140722.111645-1-maciej.fijalkowski@intel.com?part=4

Re: [PATCH v3 net 4/6] xsk: reclaim invalid multi-buffer Tx descs in ZC path

[Maciej Fijalkowski]

On Wed, Jul 15, 2026 at 02:08:45PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Processing an invalid descriptor or draining an invalid multi-buffer packet at the beginning of a Tx batch stalls the Tx queue.
> --

For these issues a single invalid descriptor will be reflected at
batch.consumed_descs and based on that global consumer pointer will be
updated so user space will see reclaimed descriptor. The stall could
happen tho if ZC Tx driver would be misbehaving with respect to wakeup
flag. I believe the correct semantics are:

- clear tx wakeup at the tx zc napi side entry
- do the processing (or not)
- set tx wakeup flag back at the exit

which tells us that i40e and ice need massage. Libeth got it right from
what I saw.

> 
> --- Patch [4]: [PATCH v3 net 4/6] xsk: reclaim invalid multi-buffer Tx descs in ZC path ---
> commit f71b54a29f7fa6021e0f05cf7b9e1dfd86287c21
> Author: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> 
> xsk: reclaim invalid multi-buffer Tx descs in ZC path
> 
> This commit updates the zero-copy Tx batch parser to process descriptors in
> packet units rather than individually. It ensures that descriptors belonging
> to invalid or oversized multi-buffer packets are properly drained and
> returned to the completion ring for reclamation without being sent to the
> driver.
> 
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index 385a3f4a1b320..2909a0ec68373 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> 
> [ ... ]
> 
> > @@ -603,19 +758,16 @@ u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 nb_pkts)
> >  	if (!nb_pkts)
> >  		goto out;
> >  
> > -	nb_pkts = xskq_cons_read_desc_batch(xs->tx, pool, nb_pkts);
> > -	if (!nb_pkts) {
> > -		xs->tx->queue_empty_descs++;
> > -		goto out;
> > -	}
> > -
> > -	__xskq_cons_release(xs->tx);
> > -	xskq_prod_write_addr_batch(pool->cq, pool->tx_descs, nb_pkts);
> > -	xs->sk.sk_write_space(&xs->sk);
> > +	batch = umem_shared ?
> > +		xsk_tx_peek_release_shared_desc_batch(pool, nb_pkts) :
> > +		__xsk_tx_peek_release_desc_batch(pool, xs,
> > +						 pool->tx_descs,
> > +						 nb_pkts);
> > +	xsk_tx_commit_batch(pool, &batch);
> >  
> >  out:
> >  	rcu_read_unlock();
> > -	return nb_pkts;
> > +	return batch.tx_descs;
> >  }
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but returning 0 to the driver
> here from xsk_tx_peek_release_desc_batch when the batch consists solely of
> reclaimed descriptors signals that the Tx ring is empty.
> 
> Since user space typically waits for hardware completions before manually
> triggering sendmsg again, does returning 0 here result in a permanent
> Tx queue stall until an unrelated Rx packet arrives to trigger an interrupt?
> 
> > diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
> > index 3e3fbb73d23e2..a15ff1929db62 100644
> > --- a/net/xdp/xsk_queue.h
> > +++ b/net/xdp/xsk_queue.h
> 
> [ ... ]
> 
> > @@ -282,26 +295,49 @@ u32 xskq_cons_read_desc_batch(struct xsk_queue *q, struct xsk_buff_pool *pool,
> >  		descs[nb_entries] = ring->desc[idx];
> >  		cached_cons++;
> >  		parse_desc(q, pool, &descs[nb_entries], &parsed);
> > -		if (unlikely(!parsed.valid))
> > -			break;
> > +		if (unlikely(!parsed.valid)) {
> > +			if (!drain && !nr_frags && !parsed.mb)
> > +				break;
> 
> [Severity: High]
> This is a pre-existing issue, but when xskq_cons_read_desc_batch encounters
> a standalone invalid descriptor at the very start of a batch, it breaks
> early out of this processing loop.
> 
> If no valid descriptors were accumulated yet, doesn't this result in
> returning an empty batch to the driver, causing NAPI polling to halt
> prematurely and stranding any subsequent valid packets on the ring?
> 
> > +
> > +			drain = true;
> > +		}
> > +
> > +		nr_frags++;
> > +		nb_entries++;
> >  
> >  		if (likely(!parsed.mb)) {
> > -			total_descs += (nr_frags + 1);
> > -			nr_frags = 0;
> > -		} else {
> > -			nr_frags++;
> > -			if (nr_frags == pool->xdp_zc_max_segs) {
> > +			if (unlikely(drain)) {
> > +				batch.reclaim_descs = nr_frags;
> > +				WRITE_ONCE(xs->drain_cont, false);
> >  				nr_frags = 0;
> >  				break;
> >  			}
> 
> [Severity: High]
> Similarly, this is a pre-existing issue, but breaking early here when
> finishing a drained packet will leave batch.tx_descs as 0 if no valid
> packets were seen first.
> 
> Could this stall the Tx queue, since no valid packets are submitted to the
> hardware to generate the Tx completion interrupts that user space needs
> to resume operation?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260714140722.111645-1-maciej.fijalkowski@intel.com?part=4

[PATCH v3 net 5/6] selftests/xsk: fix too-many-frags multi-buffer Tx test

[Maciej Fijalkowski]

The too-many-frags test describes a packet that is valid from the Tx
ring ownership point of view, but invalid for transmission because it
exceeds the supported number of fragments.

Keep the generated Tx descriptors valid so that __send_pkts() accounts
them as outstanding descriptors that must be reclaimed through the CQ.
Then mark the corresponding Rx packet invalid so the test still does
not expect the oversized packet to appear on the receive side.

Add a valid synchronization packet after the oversized packet so the
test can verify that the Tx path drains the bad packet and resumes at
the next packet boundary.

Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 .../selftests/bpf/prog_tests/test_xsk.c       | 24 ++++++++++++-------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
index 6eb9096d084c..de17dd48f176 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
@@ -2270,7 +2270,7 @@ int testapp_too_many_frags(struct test_spec *test)
 		max_frags += 1;
 	}
 
-	pkts = calloc(2 * max_frags + 2, sizeof(struct pkt));
+	pkts = calloc(2 * max_frags + 3, sizeof(struct pkt));
 	if (!pkts)
 		return TEST_FAILURE;
 
@@ -2288,24 +2288,30 @@ int testapp_too_many_frags(struct test_spec *test)
 	}
 	pkts[max_frags].options = 0;
 
-	/* An invalid packet with the max amount of frags but signals packet
-	 * continues on the last frag
-	 */
-	for (i = max_frags + 1; i < 2 * max_frags + 1; i++) {
+	/* An invalid packet with the max + 1 amount of frags */
+	for (i = max_frags + 1; i < 2 * max_frags + 2; i++) {
 		pkts[i].len = MIN_PKT_SIZE;
 		pkts[i].options = XDP_PKT_CONTD;
-		pkts[i].valid = false;
+		pkts[i].valid = true;
 	}
+	pkts[2 * max_frags + 1].options = 0;
 
 	/* Valid packet for synch */
-	pkts[2 * max_frags + 1].len = MIN_PKT_SIZE;
-	pkts[2 * max_frags + 1].valid = true;
+	pkts[2 * max_frags + 2].len = MIN_PKT_SIZE;
+	pkts[2 * max_frags + 2].valid = true;
 
-	if (pkt_stream_generate_custom(test, pkts, 2 * max_frags + 2)) {
+	if (pkt_stream_generate_custom(test, pkts, 2 * max_frags + 3)) {
 		free(pkts);
 		return TEST_FAILURE;
 	}
 
+	/* The generated Tx stream must keep the too-big packet valid so that
+	 * __send_pkts() accounts its descriptors in outstanding_tx. The Rx
+	 * stream, however, must not expect this packet on the wire.
+	 */
+	test->ifobj_rx->xsk->pkt_stream->pkts[2].valid = false;
+	test->ifobj_rx->xsk->pkt_stream->nb_valid_entries--;
+
 	ret = testapp_validate_traffic(test);
 	free(pkts);
 	return ret;
-- 
2.43.0

Re: [PATCH v3 net 5/6] selftests/xsk: fix too-many-frags multi-buffer Tx test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] A newly added multi-line comment uses the deprecated kernel style where text is placed on the same line as the opening `/*`.
--

--- Patch [5]: [PATCH v3 net 5/6] selftests/xsk: fix too-many-frags multi-buffer Tx test ---
commit 41dfcc558e2b05136aed2feb3d3534323946fd99
Author: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

selftests/xsk: fix too-many-frags multi-buffer Tx test

This commit updates the too-many-frags test to keep oversized Tx descriptors
valid so they can be accounted and reclaimed, while marking the corresponding
Rx packet invalid. It also adds a synchronization packet to verify the Tx
path recovers at the next packet boundary.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> index 6eb9096d084c0..de17dd48f1764 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
[ ... ]
> @@ -2288,24 +2288,30 @@ int testapp_too_many_frags(struct test_spec *test)
[ ... ]
>  	if (pkt_stream_generate_custom(test, pkts, 2 * max_frags + 3)) {
>  		free(pkts);
>  		return TEST_FAILURE;
>  	}
>  
> +	/* The generated Tx stream must keep the too-big packet valid so that
> +	 * __send_pkts() accounts its descriptors in outstanding_tx. The Rx
> +	 * stream, however, must not expect this packet on the wire.
> +	 */

[Severity: Low]
This isn't a bug, but could this multi-line comment be formatted to match
the preferred BPF subsystem style?

The BPF guidelines specify that multi-line comments should have the opening
/* on its own line, rather than having the comment text start on the same
line.

> +	test->ifobj_rx->xsk->pkt_stream->pkts[2].valid = false;
> +	test->ifobj_rx->xsk->pkt_stream->nb_valid_entries--;
>  
>  	ret = testapp_validate_traffic(test);
>  	free(pkts);
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714140722.111645-1-maciej.fijalkowski@intel.com?part=5

[PATCH v3 net 6/6] selftests/xsk: account invalid multi-buffer Tx descriptors

[Maciej Fijalkowski]

Invalid descriptors in the middle of a multi-buffer packet still belong
to the packet being consumed from the Tx ring. The tests should therefore
count the whole invalid packet as outstanding in verbatim mode, even
though the packet must not be expected on the Rx side.

Make fragment counting follow the packet boundary instead of stopping at
the first invalid fragment. Update custom stream generation so invalid
middle fragments terminate the generated Rx packet while Tx accounting
still covers all descriptors consumed from the invalid multi-buffer
packet.

Also add explicit end fragments after invalid middle descriptors. This
exercises the kernel drain logic and verifies that subsequent valid
packets are not interpreted as continuations of the invalid packet.

Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 .../selftests/bpf/prog_tests/test_xsk.c       | 24 ++++++++++++-------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
index de17dd48f176..7abb4935edf9 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
@@ -427,14 +427,14 @@ static u32 pkt_nb_frags(u32 frame_size, struct pkt_stream *pkt_stream, struct pk
 	}
 
 	/* Search for the end of the packet in verbatim mode */
-	if (!pkt_continues(pkt->options) || !pkt->valid)
+	if (!pkt_continues(pkt->options))
 		return nb_frags;
 
 	next_frag = pkt_stream->current_pkt_nb;
 	pkt++;
 	while (next_frag++ < pkt_stream->nb_pkts) {
 		nb_frags++;
-		if (!pkt_continues(pkt->options) || !pkt->valid)
+		if (!pkt_continues(pkt->options))
 			break;
 		pkt++;
 	}
@@ -665,11 +665,11 @@ static struct pkt_stream *__pkt_stream_generate_custom(struct ifobject *ifobj, s
 			if (!frame->valid || !pkt_continues(frame->options))
 				payload++;
 		} else {
-			if (frame->valid)
+			if (frame->valid) {
 				len += frame->len;
-			if (frame->valid && pkt_continues(frame->options))
-				continue;
-
+				if (pkt_continues(frame->options))
+					continue;
+			}
 			pkt->pkt_nb = pkt_nb;
 			pkt->len = len;
 			pkt->valid = frame->valid;
@@ -1209,6 +1209,7 @@ static int __send_pkts(struct ifobject *ifobject, struct xsk_socket_info *xsk,
 	for (i = 0; i < xsk->batch_size; i++) {
 		struct pkt *pkt = pkt_stream_get_next_tx_pkt(pkt_stream);
 		u32 nb_frags_left, nb_frags, bytes_written = 0;
+		struct pkt *first_pkt = pkt;
 
 		if (!pkt)
 			break;
@@ -1253,6 +1254,8 @@ static int __send_pkts(struct ifobject *ifobject, struct xsk_socket_info *xsk,
 		if (pkt && pkt->valid) {
 			valid_pkts++;
 			valid_frags += nb_frags;
+		} else if (pkt_stream->verbatim && pkt_continues(first_pkt->options)) {
+			valid_frags += nb_frags;
 		}
 	}
 
@@ -2099,13 +2102,16 @@ int testapp_invalid_desc_mb(struct test_spec *test)
 		{0, 0, 0, false, 0},
 		/* Invalid address in the second frame */
 		{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
-		{umem_sz, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
+		{umem_sz * 2, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
+		{0, MIN_PKT_SIZE, 0, false, 0},
 		/* Invalid len in the middle */
 		{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
 		{0, XSK_UMEM__INVALID_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
+		{0, MIN_PKT_SIZE, 0, false, 0},
 		/* Invalid options in the middle */
 		{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
 		{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XSK_DESC__INVALID_OPTION},
+		{0, MIN_PKT_SIZE, 0, false, 0},
 		/* Transmit 2 frags, receive 3 */
 		{0, XSK_UMEM__MAX_FRAME_SIZE, 0, true, XDP_PKT_CONTD},
 		{0, XSK_UMEM__MAX_FRAME_SIZE, 0, true, 0},
@@ -2117,8 +2123,8 @@ int testapp_invalid_desc_mb(struct test_spec *test)
 
 	if (umem->unaligned_mode) {
 		/* Crossing a chunk boundary allowed */
-		pkts[12].valid = true;
-		pkts[13].valid = true;
+		pkts[15].valid = true;
+		pkts[16].valid = true;
 	}
 
 	test->mtu = MAX_ETH_JUMBO_SIZE;
-- 
2.43.0

Re: [PATCH v3 net 0/6] xsk: fix AF_XDP multi-buffer Tx descriptor reclaim

[Maciej Fijalkowski]

On Tue, Jul 14, 2026 at 04:07:16PM +0200, Maciej Fijalkowski wrote:

Top posting!

I took a look at sashiko reports for this revision. I believe we can
continue the process and get human interview now (or everyone including me
are sick of this code). I would say the AI review evil has been
suppressed.

The need_wakeup issues which I brought up at 4/6 can be send as a
follow-up.

> v2: https://lore.kernel.org/netdev/20260710194424.84844-1-maciej.fijalkowski@intel.com/
> v2->v3:
> 
> * Added a preceding patch that sizes the pool-wide temporary Tx descriptor
>   array to the larger of the first Tx ring and the device's
>   xdp_zc_max_segs capability. This guarantees that the shared-UMEM path can
>   inspect one maximum-sized valid packet even when the socket that creates
>   the pool has a smaller Tx ring. Consequently, this patch now records the
>   actual allocated size in pool->tx_descs_nentries rather than the first
>   socket's Tx ring size.
> 
> * Fixed a possible infinite retry loop when a shared-UMEM socket contains
>   an incomplete multi-buffer packet. Pass the original descriptor budget
>   to xskq_cons_read_desc_batch() instead of the number of descriptors
>   currently available, so the parser can distinguish producer exhaustion
>   from actual budget exhaustion.
> 
> * Moved the completion-ring space check to the common batched Tx path, so
>   it is performed exactly once for both singular and shared-SG pools. Keep
>   the legacy shared non-SG fallback outside this handling, as it reserves
>   completion entries one descriptor at a time.
> 
> * Reworked xsk_tx_peek_release_desc_batch() to use a common singular/shared
>   batched flow. Resolve an empty Tx socket list before checking CQ space,
>   select the shared-SG walker through an explicit shared-pool condition,
>   and commit the resulting batch through one common path.
> 
> * Simplified xsk_tx_commit_batch() by moving the cached CQ producer
>   snapshot into the helper instead of passing it from each caller.
> 
> v1: https://lore.kernel.org/netdev/20260623133240.1048434-1-maciej.fijalkowski@intel.com/
> v1->v2:
> 
> * Reduced the series from seven to five patches by squashing the three
>   generic Tx drain and reclaim changes into a single patch. The resulting
>   patch handles overflow, invalid descriptors in the middle of a packet,
>   and reclaim of the offending descriptor as one coherent change. This
>   so it will be less likely to have things reported by Sashiko that are
>   fixed in later commits;
> 
> * Reworked the zero-copy implementation substantially:
>   * removed the bind-transition mechanism, including tx_share_pending,
>     xp_prepare_xsk_tx_share(), xp_finish_xsk_tx_share(),
>     synchronize_net(), and the transient bind() -EAGAIN behavior;
>   * added packet-framed parsing for shared-UMEM SG pools, allowing
>     per-socket drain state to be resumed by both singular and shared Tx
>     paths;
>   * retained the legacy one-descriptor fallback for shared non-SG pools;
>   * preserved the existing per-socket fairness quota while allowing the
>     shared walker to consume multiple complete packets and continue
>     filling the requested batch across fairness rounds;
>   * made the fairness quota large enough to process one maximum-sized
>     valid multi-buffer packet;
>   * extended the parser result with consumed-descriptor and
>     budget-limited accounting needed by the shared walker;
>   * recorded the size of the pool's temporary Tx descriptor array and
>     capped batch processing at that size;
>   * kept reclaim-only descriptors ordered after preceding driver-visible
>     descriptors and protected the delayed-reclaim state with
>     READ_ONCE()/WRITE_ONCE().
> 
> * Rewrote the zero-copy patch description to cover oversized packets,
>   continuation draining across calls, shared-UMEM SG handling, and CQ
>   publication ordering.
> 
> * Corrected the too-many-frags selftest description to state that the
>   invalid packet contains max_frags + 1 fragments and terminates at an
>   explicit packet boundary.
> 
> 
> Hi,
> 
> This series fixes several AF_XDP multi-buffer Tx paths where descriptors
> consumed from the Tx ring are not consistently returned to userspace
> through the completion ring when the packet is later dropped as invalid.
> 
> The affected cases are invalid or oversized multi-buffer Tx packets in
> both the generic and zero-copy paths. In these cases, the kernel can
> consume one or more Tx descriptors while building or validating a
> multi-buffer packet, then drop the packet before it reaches the device.
> Userspace still owns the UMEM buffers only after the corresponding
> addresses are returned through the CQ. Missing completions therefore
> make userspace lose track of those buffers.
> 
> The generic path fixes cover following related cases:
> * partially built multi-buffer skbs dropped by xsk_drop_skb();
>   continuation descriptors left in the Tx ring after xsk_build_skb()
>   reports overflow;
> * invalid descriptors encountered in the middle of a multi-buffer
>   packet, including the offending invalid descriptor itself.
> 
> The zero-copy path is handled separately. The batched Tx parser now
> distinguishes descriptors that can be passed to the driver from
> descriptors that are consumed only because they belong to an invalid
> multi-buffer packet. Reclaim-only descriptors are written to the CQ
> address area and published in completion order, after any earlier
> driver-visible Tx descriptors.
> 
> The last two patches update xskxceiver so the tests account invalid
> multi-buffer Tx packets as descriptors that must be reclaimed, while
> still not expecting those invalid packets on the Rx side.
> 
> This is a follow-up to Jason's changes [0] which were addressing generic
> xmit only and this set allows me to pass full xskxceiver test suite run
> against ice driver.
> 
> Thanks,
> Maciej
> 
> [0]: https://lore.kernel.org/netdev/20260520004244.55663-1-kerneljasonxing@gmail.com/
> 
> 
> Jason Xing (2):
>   xsk: fix buffer leak in xsk_drop_skb() for AF_XDP multi-buffer Tx
>   xsk: drain continuation descs after overflow in xsk_build_skb()
> 
> Maciej Fijalkowski (4):
>   xsk: provide sufficient space in pool->tx_descs
>   xsk: reclaim invalid multi-buffer Tx descs in ZC path
>   selftests/xsk: fix too-many-frags multi-buffer Tx test
>   selftests/xsk: account invalid multi-buffer Tx descriptors
> 
>  include/net/xdp_sock.h                        |   1 +
>  include/net/xsk_buff_pool.h                   |   9 +-
>  net/xdp/xsk.c                                 | 254 ++++++++++++++++--
>  net/xdp/xsk_buff_pool.c                       |  13 +-
>  net/xdp/xsk_queue.h                           |  76 ++++--
>  .../selftests/bpf/prog_tests/test_xsk.c       |  48 ++--
>  6 files changed, 329 insertions(+), 72 deletions(-)
> 
> -- 
> 2.43.0
> 

Re: [PATCH v3 net 0/6] xsk: fix AF_XDP multi-buffer Tx descriptor reclaim

[Jason Xing]

On Wed, Jul 15, 2026 at 9:06 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Tue, Jul 14, 2026 at 04:07:16PM +0200, Maciej Fijalkowski wrote:
>
> Top posting!
>
> I took a look at sashiko reports for this revision. I believe we can
> continue the process and get human interview now (or everyone including me
> are sick of this code). I would say the AI review evil has been
> suppressed.

Thank you, Maciej! I'll finish review all of them in these two days as
quickly as possible.

Thanks,
Jason

>
> The need_wakeup issues which I brought up at 4/6 can be send as a
> follow-up.
>
> > v2: https://lore.kernel.org/netdev/20260710194424.84844-1-maciej.fijalkowski@intel.com/
> > v2->v3:
> >
> > * Added a preceding patch that sizes the pool-wide temporary Tx descriptor
> >   array to the larger of the first Tx ring and the device's
> >   xdp_zc_max_segs capability. This guarantees that the shared-UMEM path can
> >   inspect one maximum-sized valid packet even when the socket that creates
> >   the pool has a smaller Tx ring. Consequently, this patch now records the
> >   actual allocated size in pool->tx_descs_nentries rather than the first
> >   socket's Tx ring size.
> >
> > * Fixed a possible infinite retry loop when a shared-UMEM socket contains
> >   an incomplete multi-buffer packet. Pass the original descriptor budget
> >   to xskq_cons_read_desc_batch() instead of the number of descriptors
> >   currently available, so the parser can distinguish producer exhaustion
> >   from actual budget exhaustion.
> >
> > * Moved the completion-ring space check to the common batched Tx path, so
> >   it is performed exactly once for both singular and shared-SG pools. Keep
> >   the legacy shared non-SG fallback outside this handling, as it reserves
> >   completion entries one descriptor at a time.
> >
> > * Reworked xsk_tx_peek_release_desc_batch() to use a common singular/shared
> >   batched flow. Resolve an empty Tx socket list before checking CQ space,
> >   select the shared-SG walker through an explicit shared-pool condition,
> >   and commit the resulting batch through one common path.
> >
> > * Simplified xsk_tx_commit_batch() by moving the cached CQ producer
> >   snapshot into the helper instead of passing it from each caller.
> >
> > v1: https://lore.kernel.org/netdev/20260623133240.1048434-1-maciej.fijalkowski@intel.com/
> > v1->v2:
> >
> > * Reduced the series from seven to five patches by squashing the three
> >   generic Tx drain and reclaim changes into a single patch. The resulting
> >   patch handles overflow, invalid descriptors in the middle of a packet,
> >   and reclaim of the offending descriptor as one coherent change. This
> >   so it will be less likely to have things reported by Sashiko that are
> >   fixed in later commits;
> >
> > * Reworked the zero-copy implementation substantially:
> >   * removed the bind-transition mechanism, including tx_share_pending,
> >     xp_prepare_xsk_tx_share(), xp_finish_xsk_tx_share(),
> >     synchronize_net(), and the transient bind() -EAGAIN behavior;
> >   * added packet-framed parsing for shared-UMEM SG pools, allowing
> >     per-socket drain state to be resumed by both singular and shared Tx
> >     paths;
> >   * retained the legacy one-descriptor fallback for shared non-SG pools;
> >   * preserved the existing per-socket fairness quota while allowing the
> >     shared walker to consume multiple complete packets and continue
> >     filling the requested batch across fairness rounds;
> >   * made the fairness quota large enough to process one maximum-sized
> >     valid multi-buffer packet;
> >   * extended the parser result with consumed-descriptor and
> >     budget-limited accounting needed by the shared walker;
> >   * recorded the size of the pool's temporary Tx descriptor array and
> >     capped batch processing at that size;
> >   * kept reclaim-only descriptors ordered after preceding driver-visible
> >     descriptors and protected the delayed-reclaim state with
> >     READ_ONCE()/WRITE_ONCE().
> >
> > * Rewrote the zero-copy patch description to cover oversized packets,
> >   continuation draining across calls, shared-UMEM SG handling, and CQ
> >   publication ordering.
> >
> > * Corrected the too-many-frags selftest description to state that the
> >   invalid packet contains max_frags + 1 fragments and terminates at an
> >   explicit packet boundary.
> >
> >
> > Hi,
> >
> > This series fixes several AF_XDP multi-buffer Tx paths where descriptors
> > consumed from the Tx ring are not consistently returned to userspace
> > through the completion ring when the packet is later dropped as invalid.
> >
> > The affected cases are invalid or oversized multi-buffer Tx packets in
> > both the generic and zero-copy paths. In these cases, the kernel can
> > consume one or more Tx descriptors while building or validating a
> > multi-buffer packet, then drop the packet before it reaches the device.
> > Userspace still owns the UMEM buffers only after the corresponding
> > addresses are returned through the CQ. Missing completions therefore
> > make userspace lose track of those buffers.
> >
> > The generic path fixes cover following related cases:
> > * partially built multi-buffer skbs dropped by xsk_drop_skb();
> >   continuation descriptors left in the Tx ring after xsk_build_skb()
> >   reports overflow;
> > * invalid descriptors encountered in the middle of a multi-buffer
> >   packet, including the offending invalid descriptor itself.
> >
> > The zero-copy path is handled separately. The batched Tx parser now
> > distinguishes descriptors that can be passed to the driver from
> > descriptors that are consumed only because they belong to an invalid
> > multi-buffer packet. Reclaim-only descriptors are written to the CQ
> > address area and published in completion order, after any earlier
> > driver-visible Tx descriptors.
> >
> > The last two patches update xskxceiver so the tests account invalid
> > multi-buffer Tx packets as descriptors that must be reclaimed, while
> > still not expecting those invalid packets on the Rx side.
> >
> > This is a follow-up to Jason's changes [0] which were addressing generic
> > xmit only and this set allows me to pass full xskxceiver test suite run
> > against ice driver.
> >
> > Thanks,
> > Maciej
> >
> > [0]: https://lore.kernel.org/netdev/20260520004244.55663-1-kerneljasonxing@gmail.com/
> >
> >
> > Jason Xing (2):
> >   xsk: fix buffer leak in xsk_drop_skb() for AF_XDP multi-buffer Tx
> >   xsk: drain continuation descs after overflow in xsk_build_skb()
> >
> > Maciej Fijalkowski (4):
> >   xsk: provide sufficient space in pool->tx_descs
> >   xsk: reclaim invalid multi-buffer Tx descs in ZC path
> >   selftests/xsk: fix too-many-frags multi-buffer Tx test
> >   selftests/xsk: account invalid multi-buffer Tx descriptors
> >
> >  include/net/xdp_sock.h                        |   1 +
> >  include/net/xsk_buff_pool.h                   |   9 +-
> >  net/xdp/xsk.c                                 | 254 ++++++++++++++++--
> >  net/xdp/xsk_buff_pool.c                       |  13 +-
> >  net/xdp/xsk_queue.h                           |  76 ++++--
> >  .../selftests/bpf/prog_tests/test_xsk.c       |  48 ++--
> >  6 files changed, 329 insertions(+), 72 deletions(-)
> >
> > --
> > 2.43.0
> >