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

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

[Jerome Tollet]

When an XDP redirect to an AF_XDP socket fails because the RX ring is
full, the XSK core frees the buffer. mlx5e later visits the cyclic WQE
and frees its XSK buffer before trying to refill the slot.

If a batched refill succeeds only partially, a missing WQE keeps its
old buffer pointer. The buffer may meanwhile be allocated to another
WQE, so a later refill retry can free a 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.

A standalone legacy cyclic-RQ zero-copy libxsk reproducer, using
64-byte UDP traffic offered at 12 Mpps, stopped on stock 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>
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 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

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

[Dragos Tatulea]

Hi,

Thanks for fixing this! Just some commit message nits.

On 19.08.26 17:13, Jerome Tollet wrote:
> When an XDP redirect to an AF_XDP socket fails because the RX ring is
> full, the XSK core frees the buffer. mlx5e later visits the cyclic WQE
> and frees its XSK buffer before trying to refill the slot.
> 
Is this necessary? You explain very well the circumstances in the last
paragraph.

> If a batched refill succeeds only partially, a missing WQE keeps its
> old buffer pointer. The buffer may meanwhile be allocated to another
> WQE, so a later refill retry can free a live buffer through the stale
> pointer and publish the same UMEM frame twice.
>
This should be the first paragraph. With a bit of extra context added
(XDP redirect with AF_XDP).

> 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.
> 
> A standalone legacy cyclic-RQ zero-copy libxsk reproducer, using
> 64-byte UDP traffic offered at 12 Mpps, stopped on stock 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.
>
There's no splat, right?

> 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>
> 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 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);
>  }
>

When you send the v2, feel free to add:

Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>

Thanks,
Dragos