Re: [net-next v5 09/12] net: bnxt: Add SW GSO completion and teardown support

[Paolo Abeni]

On 3/23/26 7:38 PM, Joe Damato wrote:
> Update __bnxt_tx_int and bnxt_free_one_tx_ring_skbs to handle SW GSO
> segments:
> 
> - MID segments: adjust tx_pkts/tx_bytes accounting and skip skb free
>   (the skb is shared across all segments and freed only once)
> 
> - LAST segments: if the DMA IOVA path was used, use dma_iova_destroy to
>   tear down the contiguous mapping. On the fallback path, payload DMA
>   unmapping is handled by the existing per-BD dma_unmap_len walk.
> 
> Both MID and LAST completions advance tx_inline_cons to release the
> segment's inline header slot back to the ring.
> 
> is_sw_gso is initialized to zero, so the new code paths are not run.
> 
> Suggested-by: Jakub Kicinski <kuba@xxxxxxxxxx>
> Reviewed-by: Pavan Chebbi <pavan.chebbi@xxxxxxxxxxxx>
> Signed-off-by: Joe Damato <joe@xxxxxxx>
> ---
>  v5:
>    - Added Pavan's Reviewed-by. No functional changes.
> 
>  v3:
>    - completion paths updated to use DMA IOVA APIs to teardown mappings.
> 
>  rfcv2:
>    - Update the shared header buffer consumer on TX completion.
> 
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 82 +++++++++++++++++--
>  .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 19 ++++-
>  2 files changed, 91 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 2759a4e2b148..40a16f96feba 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -74,6 +74,8 @@
>  #include "bnxt_debugfs.h"
>  #include "bnxt_coredump.h"
>  #include "bnxt_hwmon.h"
> +#include "bnxt_gso.h"
> +#include <net/tso.h>
>  
>  #define BNXT_TX_TIMEOUT		(5 * HZ)
>  #define BNXT_DEF_MSG_ENABLE	(NETIF_MSG_DRV | NETIF_MSG_HW | \
> @@ -817,12 +819,13 @@ static bool __bnxt_tx_int(struct bnxt *bp, struct bnxt_tx_ring_info *txr,
>  	bool rc = false;
>  
>  	while (RING_TX(bp, cons) != hw_cons) {
> -		struct bnxt_sw_tx_bd *tx_buf;
> +		struct bnxt_sw_tx_bd *tx_buf, *head_buf;
>  		struct sk_buff *skb;
>  		bool is_ts_pkt;
>  		int j, last;
>  
>  		tx_buf = &txr->tx_buf_ring[RING_TX(bp, cons)];
> +		head_buf = tx_buf;
>  		skb = tx_buf->skb;
>  
>  		if (unlikely(!skb)) {
> @@ -869,6 +872,23 @@ static bool __bnxt_tx_int(struct bnxt *bp, struct bnxt_tx_ring_info *txr,
>  							    DMA_TO_DEVICE, 0);
>  			}
>  		}
> +
> +		if (unlikely(head_buf->is_sw_gso)) {
> +			txr->tx_inline_cons++;
> +			if (head_buf->is_sw_gso == BNXT_SW_GSO_LAST) {
> +				if (dma_use_iova(&head_buf->iova_state))

I'm likely lost, but AFAICS the previous patch/bnxt_sw_udp_gso_xmit()
initialize head_buf->iova_state only when
`dma_use_iova(&head_buf->iova_state) == true`. I.e. in fallback scenario
the previous iova_state is maintained.

Additionally AFAICS dma_iova_destroy does not clear `head_buf->iova_state`.

It looks like that 2 consecutive skb hitting the same slot use a
different dma mapping strategy (fallback vs iova) bat things will
happen?!? should the previous patch always initializing
head_buf->iova_state?

/P