[PATCH net v2] net/mlx5e: Prevent stale XSK buffer release on refill retry

[PATCH net v2] net/mlx5e: Prevent stale XSK buffer release on refill retry

[Jerome Tollet]

When an XDP redirect to an AF_XDP socket fails because its RX ring is
full, the XSK core frees the buffer. During the subsequent batched refill
of a legacy cyclic RQ, mlx5e also releases the WQE's XSK buffer before
allocating a replacement. If that refill succeeds only partially, a WQE
left without a replacement retains its old buffer pointer.

The buffer can meanwhile be allocated to another WQE. A later refill
retry can then free the live buffer through the stale pointer and publish
the same UMEM frame twice.

Mark the WQE as released immediately after the driver-side free. The flag
is already cleared when a replacement buffer is assigned, so refill
retries no longer release stale pointers.

The failure is silent and produces no kernel warning or splat. A
standalone legacy cyclic-RQ zero-copy libxsk reproducer, using 64-byte UDP
traffic offered at 12 Mpps, detected it: stock stopped after 2,854,914
packets in 4.094 seconds, with 4,542 xdp_rx_ring_full events and 64
ownership/double-publication errors. With this change it processed
356,904,225 packets in 30 seconds despite 571,405 xdp_rx_ring_full events,
with no ownership or data errors.

Fixes: 3f93f82988bc ("net/mlx5e: RX, Defer page release in legacy rq for better recycling")
Cc: stable@vger.kernel.org
Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Jerome Tollet <jtollet@cisco.com>
---
Changes in v2:
- Reorder and tighten the problem statement.
- State explicitly that the failure produces no warning or splat.
- Add Dragos' Reviewed-by tag.

v1: https://lore.kernel.org/netdev/20260819151320.64178-1-jtollet@cisco.com/

 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 6fc6605d2..7967a2737 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -410,8 +410,11 @@ static inline void mlx5e_free_rx_wqe(struct mlx5e_rq *rq,
 
 static void mlx5e_xsk_free_rx_wqe(struct mlx5e_wqe_frag_info *wi)
 {
-	if (!(wi->flags & BIT(MLX5E_WQE_FRAG_SKIP_RELEASE)))
-		xsk_buff_free(*wi->xskp);
+	if (wi->flags & BIT(MLX5E_WQE_FRAG_SKIP_RELEASE))
+		return;
+
+	xsk_buff_free(*wi->xskp);
+	wi->flags |= BIT(MLX5E_WQE_FRAG_SKIP_RELEASE);
 }
 
 static void mlx5e_dealloc_rx_wqe(struct mlx5e_rq *rq, u16 ix)
-- 
2.55.0

[PATCH net v3 0/2] net/mlx5e: Prevent stale XSK buffer release on refill retries

[Jerome Tollet]

Prevent duplicate XSK buffer release when a deferred RX refill fails and
the same WQE is retried.

Patch 1 fixes legacy cyclic RQ. It is unchanged from v2 and retains
Dragos' Reviewed-by tag.

Patch 2 fixes the analogous striding-RQ MPWQE path pointed out by the
Sashiko automated review. Targeted fault injection forced three
consecutive allocation failures for one MPWQE. Stock freed the same 16
XSK buffer pointers three times; the fix freed each pointer only once,
made subsequent releases no-ops, and preserved the successful
allocation path's bitmap reset.

Changes in v3:
- Turn the submission into a two-patch series.
- Add the striding-RQ MPWQE fix and its dedicated Fixes tag.
- Rebase onto net/main at 4e15e89faac9.

v2: https://lore.kernel.org/netdev/20260820151558.11015-1-jtollet@cisco.com/

Jerome Tollet (2):
  net/mlx5e: Prevent stale XSK buffer release on refill retry
  net/mlx5e: Prevent stale XSK buffer release on MPWQE refill retry

 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

-- 
2.55.0

[PATCH net v3 1/2] net/mlx5e: Prevent stale XSK buffer release on refill retry

[Jerome Tollet]

When an XDP redirect to an AF_XDP socket fails because its RX ring is
full, the XSK core frees the buffer. During the subsequent batched refill
of a legacy cyclic RQ, mlx5e also releases the WQE's XSK buffer before
allocating a replacement. If that refill succeeds only partially, a WQE
left without a replacement retains its old buffer pointer.

The buffer can meanwhile be allocated to another WQE. A later refill
retry can then free the live buffer through the stale pointer and publish
the same UMEM frame twice.

Mark the WQE as released immediately after the driver-side free. The flag
is already cleared when a replacement buffer is assigned, so refill
retries no longer release stale pointers.

The failure is silent and produces no kernel warning or splat. A
standalone legacy cyclic-RQ zero-copy libxsk reproducer, using 64-byte UDP
traffic offered at 12 Mpps, detected it: stock stopped after 2,854,914
packets in 4.094 seconds, with 4,542 xdp_rx_ring_full events and 64
ownership/double-publication errors. With this change it processed
356,904,225 packets in 30 seconds despite 571,405 xdp_rx_ring_full events,
with no ownership or data errors.

Fixes: 3f93f82988bc ("net/mlx5e: RX, Defer page release in legacy rq for better recycling")
Cc: stable@vger.kernel.org
Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Jerome Tollet <jtollet@cisco.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 206cf9db3..7bd0606a5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -410,8 +410,11 @@ static inline void mlx5e_free_rx_wqe(struct mlx5e_rq *rq,
 
 static void mlx5e_xsk_free_rx_wqe(struct mlx5e_wqe_frag_info *wi)
 {
-	if (!(wi->flags & BIT(MLX5E_WQE_FRAG_SKIP_RELEASE)))
-		xsk_buff_free(*wi->xskp);
+	if (wi->flags & BIT(MLX5E_WQE_FRAG_SKIP_RELEASE))
+		return;
+
+	xsk_buff_free(*wi->xskp);
+	wi->flags |= BIT(MLX5E_WQE_FRAG_SKIP_RELEASE);
 }
 
 static void mlx5e_dealloc_rx_wqe(struct mlx5e_rq *rq, u16 ix)
-- 
2.55.0

[PATCH net v3 2/2] net/mlx5e: Prevent stale XSK buffer release on MPWQE refill retry

[Jerome Tollet]

With AF_XDP on a striding RQ, mlx5e defers releasing XSK buffers until
an MPWQE is refilled. If XSK allocation then returns -ENOMEM,
actual_wq_head is not advanced and a later NAPI poll retries the same
WQE.

mlx5e_free_rx_mpwqe() currently leaves each released slot marked as
releasable. On retry it can therefore call xsk_buff_free() again through
stale pointers after the frames have returned to the XSK pool and been
reallocated.

Mark each slot as released immediately after xsk_buff_free(). The
successful allocation path already clears the bitmap after replacing
all buffers, so retries become idempotent without changing the success
path.

Fault injection forced three consecutive allocation failures for one
selected MPWQE in AF_XDP zero-copy mode. Stock freed the same 16 XSK
buffer pointers on all three attempts, for 48 frees. With this change,
only the initial 16 frees occurred; retries observed a full bitmap, and
a later successful allocation cleared it.

Fixes: 4c2a13236807 ("net/mlx5e: RX, Defer page release in striding rq for better recycling")
Cc: stable@vger.kernel.org
Signed-off-by: Jerome Tollet <jtollet@cisco.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 7bd0606a5..5fe17d295 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -609,9 +609,12 @@ mlx5e_free_rx_mpwqe(struct mlx5e_rq *rq, struct mlx5e_mpw_info *wi)
 		 * is no way to return the page to userspace when the interface
 		 * goes down.
 		 */
-		for (i = 0; i < rq->mpwqe.pages_per_wqe; i++)
-			if (no_xdp_xmit || !test_bit(i, wi->skip_release_bitmap))
+		for (i = 0; i < rq->mpwqe.pages_per_wqe; i++) {
+			if (no_xdp_xmit || !test_bit(i, wi->skip_release_bitmap)) {
 				xsk_buff_free(xsk_buffs[i]);
+				__set_bit(i, wi->skip_release_bitmap);
+			}
+		}
 	} else {
 		for (i = 0; i < rq->mpwqe.pages_per_wqe; i++) {
 			if (no_xdp_xmit || !test_bit(i, wi->skip_release_bitmap)) {
-- 
2.55.0

Re: [PATCH net v3 2/2] net/mlx5e: Prevent stale XSK buffer release on MPWQE refill retry

[Dragos Tatulea]

On 21.08.26 23:37, Jerome Tollet wrote:
> With AF_XDP on a striding RQ, mlx5e defers releasing XSK buffers until
> an MPWQE is refilled. If XSK allocation then returns -ENOMEM,
> actual_wq_head is not advanced and a later NAPI poll retries the same
> WQE.
> 
> mlx5e_free_rx_mpwqe() currently leaves each released slot marked as
> releasable. On retry it can therefore call xsk_buff_free() again through
> stale pointers after the frames have returned to the XSK pool and been
> reallocated.
> 
> Mark each slot as released immediately after xsk_buff_free(). The
> successful allocation path already clears the bitmap after replacing
> all buffers, so retries become idempotent without changing the success
> path.
> 
> Fault injection forced three consecutive allocation failures for one
> selected MPWQE in AF_XDP zero-copy mode. Stock freed the same 16 XSK
> buffer pointers on all three attempts, for 48 frees. With this change,
> only the initial 16 frees occurred; retries observed a full bitmap, and
> a later successful allocation cleared it.
> 
> Fixes: 4c2a13236807 ("net/mlx5e: RX, Defer page release in striding rq for better recycling")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jerome Tollet <jtollet@cisco.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> index 7bd0606a5..5fe17d295 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> @@ -609,9 +609,12 @@ mlx5e_free_rx_mpwqe(struct mlx5e_rq *rq, struct mlx5e_mpw_info *wi)
>  		 * is no way to return the page to userspace when the interface
>  		 * goes down.
>  		 */
> -		for (i = 0; i < rq->mpwqe.pages_per_wqe; i++)
> -			if (no_xdp_xmit || !test_bit(i, wi->skip_release_bitmap))
> +		for (i = 0; i < rq->mpwqe.pages_per_wqe; i++) {
> +			if (no_xdp_xmit || !test_bit(i, wi->skip_release_bitmap)) {
>  				xsk_buff_free(xsk_buffs[i]);
> +				__set_bit(i, wi->skip_release_bitmap);
> +			}
> +		}
>  	} else {
>  		for (i = 0; i < rq->mpwqe.pages_per_wqe; i++) {
>  			if (no_xdp_xmit || !test_bit(i, wi->skip_release_bitmap)) {
I'd change this to be consistent with mlx5e_alloc_rx_mpwqe(): set skip_release_bitmap
bits in error path of mlx5e_xsk_alloc_rx_mpwqe.

Thanks,
Dragos