diff mbox series

[v1,net,4/4] net: ena: Set tx_info->xdpf value to NULL

Message ID 20240410091358.16289-5-darinzon@amazon.com (mailing list archive)
State Accepted
Commit 36a1ca01f0452f2549420e7279c2588729bd94df
Delegated to: Netdev Maintainers
Headers show
Series ENA driver bug fixes | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 942 this patch: 942
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 7 maintainers not CCed: pabeni@redhat.com daniel@iogearbox.net hawk@kernel.org edumazet@google.com ast@kernel.org john.fastabend@gmail.com bpf@vger.kernel.org
netdev/build_clang success Errors and warnings before: 953 this patch: 953
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 953 this patch: 953
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 17 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-04-11--00-00 (tests: 959)

Commit Message

Arinzon, David April 10, 2024, 9:13 a.m. UTC 
From: David Arinzon <darinzon@amazon.com>

The patch mentioned in the `Fixes` tag removed the explicit assignment
of tx_info->xdpf to NULL with the justification that there's no need
to set tx_info->xdpf to NULL and tx_info->num_of_bufs to 0 in case
of a mapping error. Both values won't be used once the mapping function
returns an error, and their values would be overridden by the next
transmitted packet.

While both values do indeed get overridden in the next transmission
call, the value of tx_info->xdpf is also used to check whether a TX
descriptor's transmission has been completed (i.e. a completion for it
was polled).

An example scenario:
1. Mapping failed, tx_info->xdpf wasn't set to NULL
2. A VF reset occurred leading to IO resource destruction and
   a call to ena_free_tx_bufs() function
3. Although the descriptor whose mapping failed was freed by the
   transmission function, it still passes the check
     if (!tx_info->skb)

   (skb and xdp_frame are in a union)
4. The xdp_frame associated with the descriptor is freed twice

This patch returns the assignment of NULL to tx_info->xdpf to make the
cleaning function knows that the descriptor is already freed.

Fixes: 504fd6a5390c ("net: ena: fix DMA mapping function issues in XDP")
Signed-off-by: Shay Agroskin <shayagr@amazon.com>
Signed-off-by: David Arinzon <darinzon@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_xdp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Nelson, Shannon April 10, 2024, 9:53 p.m. UTC | #1 
On 4/10/2024 2:13 AM, darinzon@amazon.com wrote:
> From: David Arinzon <darinzon@amazon.com>
> 
> The patch mentioned in the `Fixes` tag removed the explicit assignment
> of tx_info->xdpf to NULL with the justification that there's no need
> to set tx_info->xdpf to NULL and tx_info->num_of_bufs to 0 in case
> of a mapping error. Both values won't be used once the mapping function
> returns an error, and their values would be overridden by the next
> transmitted packet.
> 
> While both values do indeed get overridden in the next transmission
> call, the value of tx_info->xdpf is also used to check whether a TX
> descriptor's transmission has been completed (i.e. a completion for it
> was polled).
> 
> An example scenario:
> 1. Mapping failed, tx_info->xdpf wasn't set to NULL
> 2. A VF reset occurred leading to IO resource destruction and
>     a call to ena_free_tx_bufs() function
> 3. Although the descriptor whose mapping failed was freed by the
>     transmission function, it still passes the check
>       if (!tx_info->skb)
> 
>     (skb and xdp_frame are in a union)
> 4. The xdp_frame associated with the descriptor is freed twice
> 
> This patch returns the assignment of NULL to tx_info->xdpf to make the
> cleaning function knows that the descriptor is already freed.
> 
> Fixes: 504fd6a5390c ("net: ena: fix DMA mapping function issues in XDP")
> Signed-off-by: Shay Agroskin <shayagr@amazon.com>
> Signed-off-by: David Arinzon <darinzon@amazon.com>


Reviewed-by: Shannon Nelson <shannon.nelson@amd.com>


> ---
>   drivers/net/ethernet/amazon/ena/ena_xdp.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_xdp.c b/drivers/net/ethernet/amazon/ena/ena_xdp.c
> index 337c435d..5b175e7e 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_xdp.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_xdp.c
> @@ -89,7 +89,7 @@ int ena_xdp_xmit_frame(struct ena_ring *tx_ring,
> 
>          rc = ena_xdp_tx_map_frame(tx_ring, tx_info, xdpf, &ena_tx_ctx);
>          if (unlikely(rc))
> -               return rc;
> +               goto err;
> 
>          ena_tx_ctx.req_id = req_id;
> 
> @@ -112,7 +112,9 @@ int ena_xdp_xmit_frame(struct ena_ring *tx_ring,
> 
>   error_unmap_dma:
>          ena_unmap_tx_buff(tx_ring, tx_info);
> +err:
>          tx_info->xdpf = NULL;
> +
>          return rc;
>   }
> 
> --
> 2.40.1
> 
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/amazon/ena/ena_xdp.c b/drivers/net/ethernet/amazon/ena/ena_xdp.c
index 337c435d..5b175e7e 100644
--- a/drivers/net/ethernet/amazon/ena/ena_xdp.c
+++ b/drivers/net/ethernet/amazon/ena/ena_xdp.c
@@ -89,7 +89,7 @@  int ena_xdp_xmit_frame(struct ena_ring *tx_ring,
 
 	rc = ena_xdp_tx_map_frame(tx_ring, tx_info, xdpf, &ena_tx_ctx);
 	if (unlikely(rc))
-		return rc;
+		goto err;
 
 	ena_tx_ctx.req_id = req_id;
 
@@ -112,7 +112,9 @@  int ena_xdp_xmit_frame(struct ena_ring *tx_ring,
 
 error_unmap_dma:
 	ena_unmap_tx_buff(tx_ring, tx_info);
+err:
 	tx_info->xdpf = NULL;
+
 	return rc;
 }