[PATCH net] bnxt_en: Prevent queue stop with deferred completions

[PATCH net] bnxt_en: Prevent queue stop with deferred completions

[Joe Damato]

When the driver receives a burst of packets, it can mark a BD with the
NO_CMPL bit to defer completions. The expectation is that the last
packet in the ring will have this bit unset and the completion generated
by that packet will cleanup that packet and the ones preceding it. This
helps to reduce the number of completions fired.

The suppressed completions are controlled by the driver and the number
of packets with suppressed completions scales with the size of the ring.
SW USO packets, on the other hand, have an upper bound on the maximum
number of BDs which can be consumed which does not scale with the ring
size.

So, for small rings it is possible that: a burst of packets is handed to
the driver, the driver defers completions for all of the packets because
the number of free descriptors stays above the threshold in the driver.
Then, a USO packet arrives, but the number of BDs available is not
enough and the USO code exits early with NETDEV_TX_BUSY.

In this case, you end up in a state where the ring is full of packets
with their completions suppressed, which can cause the queue to stop and
never be restarted.

Assuming default CONFIG_MAX_SKB_FRAGS, this is only possible for small
rings (<= 457 descriptors, below the driver default value) when
a burst of packets fills the ring, followed by a large USO packet that
can't fit. For larger rings, the delta between the completion
suppression threshold and the BDs required for SW USO is large enough
that completions will fire and this case is unreachable.

This issue was pointed out by Sashiko and while it seems fairly unlikely
given that the queue size must be small to trigger this, it is indeed
possible.

Fix this by tracking the last BD which deferred completions. If SW USO
exits early and there is a doorbell pending, enable completions for the
last packet in the ring with disabled completions. This ensures that a
completion will be generated and avoids stopping the queue with no way
to start it again.

Fixes: cc5d90667db8 ("net: bnxt: Implement software USO")
Cc: <stable@vger.kernel.org> # v7.1+: 4e15e89faac9: net: bnxt: ring the doorbell when SW USO exits early
Signed-off-by: Joe Damato <joe@dama.to>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  1 +
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  1 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c | 17 +++++++++++++++--
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d59bcca73a2b..8ab5acd5bee6 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -757,6 +757,7 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		if (free_size >= bp->tx_wake_thresh)
 			txbd0->tx_bd_len_flags_type |=
 				cpu_to_le32(TX_BD_FLAGS_NO_CMPL);
+		txr->kick_prod = txr->tx_prod - (last_frag + 2);
 		txr->kick_pending = 1;
 	}
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index ab894f8addef..e2fa90740d61 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -993,6 +993,7 @@ struct bnxt_tx_ring_info {
 	u16			txq_index;
 	u8			tx_napi_idx;
 	u8			kick_pending;
+	u16			kick_prod;
 	struct bnxt_db_info	tx_db;
 
 	struct tx_bd		*tx_desc_ring[MAX_TX_PAGES];
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c
index f7e18bea0fb8..f9bf77bffea7 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c
@@ -69,7 +69,7 @@ netdev_tx_t bnxt_sw_udp_gso_xmit(struct bnxt *bp,
 	if (unlikely(bnxt_tx_avail(bp, txr) < bds_needed)) {
 		netif_txq_try_stop(txq, bnxt_tx_avail(bp, txr),
 				   bp->tx_wake_thresh);
-		return NETDEV_TX_BUSY;
+		goto tx_busy;
 	}
 
 	/* BD backpressure alone cannot prevent overwriting in-flight
@@ -77,7 +77,7 @@ netdev_tx_t bnxt_sw_udp_gso_xmit(struct bnxt *bp,
 	 */
 	if (!netif_txq_maybe_stop(txq, bnxt_inline_avail(txr),
 				  num_segs, num_segs))
-		return NETDEV_TX_BUSY;
+		goto tx_busy;
 
 	if (unlikely(tso_dma_map_init(&map, &pdev->dev, skb, hdr_len)))
 		goto drop;
@@ -235,4 +235,17 @@ netdev_tx_t bnxt_sw_udp_gso_xmit(struct bnxt *bp,
 	dev_kfree_skb_any(skb);
 	dev_core_stats_tx_dropped_inc(bp->dev);
 	return NETDEV_TX_OK;
+
+tx_busy:
+	if (txr->kick_pending) {
+		u16 kick_prod = txr->kick_prod;
+		struct tx_bd *txbd0;
+
+		txbd0 = &txr->tx_desc_ring[TX_RING(bp, kick_prod)]
+					  [TX_IDX(kick_prod)];
+		txbd0->tx_bd_len_flags_type &=
+			cpu_to_le32(~TX_BD_FLAGS_NO_CMPL);
+	}
+
+	return NETDEV_TX_BUSY;
 }

base-commit: e2a6641e3bfde58f2284f9859c2b0fdcc6d1c0da
-- 
2.53.0-Meta

Re: [PATCH net] bnxt_en: Prevent queue stop with deferred completions

[Xuanqiang Luo]

> +	if (txr->kick_pending) {
> +		u16 kick_prod = txr->kick_prod;
> +		struct tx_bd *txbd0;
> +

Please follow netdev’s reverse-xmas-tree convention:

https://docs.kernel.org/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs

Thanks,
Xuanqiang

Re: [PATCH net] bnxt_en: Prevent queue stop with deferred completions

[Eric Dumazet]

On Fri, Aug 28, 2026 at 1:09 AM Joe Damato <joe@dama.to> wrote:
>
> When the driver receives a burst of packets, it can mark a BD with the
> NO_CMPL bit to defer completions. The expectation is that the last
> packet in the ring will have this bit unset and the completion generated
> by that packet will cleanup that packet and the ones preceding it. This
> helps to reduce the number of completions fired.
>
> The suppressed completions are controlled by the driver and the number
> of packets with suppressed completions scales with the size of the ring.
> SW USO packets, on the other hand, have an upper bound on the maximum
> number of BDs which can be consumed which does not scale with the ring
> size.
>
> So, for small rings it is possible that: a burst of packets is handed to
> the driver, the driver defers completions for all of the packets because
> the number of free descriptors stays above the threshold in the driver.
> Then, a USO packet arrives, but the number of BDs available is not
> enough and the USO code exits early with NETDEV_TX_BUSY.
>
> In this case, you end up in a state where the ring is full of packets
> with their completions suppressed, which can cause the queue to stop and
> never be restarted.
>
> Assuming default CONFIG_MAX_SKB_FRAGS, this is only possible for small
> rings (<= 457 descriptors, below the driver default value) when
> a burst of packets fills the ring, followed by a large USO packet that
> can't fit. For larger rings, the delta between the completion
> suppression threshold and the BDs required for SW USO is large enough
> that completions will fire and this case is unreachable.
>
> This issue was pointed out by Sashiko and while it seems fairly unlikely
> given that the queue size must be small to trigger this, it is indeed
> possible.
>
> Fix this by tracking the last BD which deferred completions. If SW USO
> exits early and there is a doorbell pending, enable completions for the
> last packet in the ring with disabled completions. This ensures that a
> completion will be generated and avoids stopping the queue with no way
> to start it again.
>
> Fixes: cc5d90667db8 ("net: bnxt: Implement software USO")
> Cc: <stable@vger.kernel.org> # v7.1+: 4e15e89faac9: net: bnxt: ring the doorbell when SW USO exits early
> Signed-off-by: Joe Damato <joe@dama.to>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH net] bnxt_en: Prevent queue stop with deferred completions

[Pavan Chebbi]

[-- Attachment #1: Type: text/plain, Size: 1563 bytes --]

>         if (unlikely(bnxt_tx_avail(bp, txr) < bds_needed)) {
>                 netif_txq_try_stop(txq, bnxt_tx_avail(bp, txr),
>                                    bp->tx_wake_thresh);
> -               return NETDEV_TX_BUSY;
> +               goto tx_busy;
>         }
>
>         /* BD backpressure alone cannot prevent overwriting in-flight
> @@ -77,7 +77,7 @@ netdev_tx_t bnxt_sw_udp_gso_xmit(struct bnxt *bp,
>          */
>         if (!netif_txq_maybe_stop(txq, bnxt_inline_avail(txr),
>                                   num_segs, num_segs))
> -               return NETDEV_TX_BUSY;
> +               goto tx_busy;
>
>         if (unlikely(tso_dma_map_init(&map, &pdev->dev, skb, hdr_len)))
>                 goto drop;
> @@ -235,4 +235,17 @@ netdev_tx_t bnxt_sw_udp_gso_xmit(struct bnxt *bp,
>         dev_kfree_skb_any(skb);
>         dev_core_stats_tx_dropped_inc(bp->dev);
>         return NETDEV_TX_OK;
> +
> +tx_busy:
> +       if (txr->kick_pending) {
> +               u16 kick_prod = txr->kick_prod;
> +               struct tx_bd *txbd0;
> +
> +               txbd0 = &txr->tx_desc_ring[TX_RING(bp, kick_prod)]
> +                                         [TX_IDX(kick_prod)];
> +               txbd0->tx_bd_len_flags_type &=
> +                       cpu_to_le32(~TX_BD_FLAGS_NO_CMPL);

Should we not do a bnxt_txr_db_kick() also? It maybe a good idea to do
it in success case also instead of bnxt_db_write..

> +       }
> +
> +       return NETDEV_TX_BUSY;
>  }
>
> base-commit: e2a6641e3bfde58f2284f9859c2b0fdcc6d1c0da
> --
> 2.53.0-Meta
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

Re: [PATCH net] bnxt_en: Prevent queue stop with deferred completions

[Joe Damato]

On Sat, Aug 29, 2026 at 02:57:59PM +0530, Pavan Chebbi wrote:
> >         if (unlikely(bnxt_tx_avail(bp, txr) < bds_needed)) {
> >                 netif_txq_try_stop(txq, bnxt_tx_avail(bp, txr),
> >                                    bp->tx_wake_thresh);
> > -               return NETDEV_TX_BUSY;
> > +               goto tx_busy;
> >         }
> >
> >         /* BD backpressure alone cannot prevent overwriting in-flight
> > @@ -77,7 +77,7 @@ netdev_tx_t bnxt_sw_udp_gso_xmit(struct bnxt *bp,
> >          */
> >         if (!netif_txq_maybe_stop(txq, bnxt_inline_avail(txr),
> >                                   num_segs, num_segs))
> > -               return NETDEV_TX_BUSY;
> > +               goto tx_busy;
> >
> >         if (unlikely(tso_dma_map_init(&map, &pdev->dev, skb, hdr_len)))
> >                 goto drop;
> > @@ -235,4 +235,17 @@ netdev_tx_t bnxt_sw_udp_gso_xmit(struct bnxt *bp,
> >         dev_kfree_skb_any(skb);
> >         dev_core_stats_tx_dropped_inc(bp->dev);
> >         return NETDEV_TX_OK;
> > +
> > +tx_busy:
> > +       if (txr->kick_pending) {
> > +               u16 kick_prod = txr->kick_prod;
> > +               struct tx_bd *txbd0;
> > +
> > +               txbd0 = &txr->tx_desc_ring[TX_RING(bp, kick_prod)]
> > +                                         [TX_IDX(kick_prod)];
> > +               txbd0->tx_bd_len_flags_type &=
> > +                       cpu_to_le32(~TX_BD_FLAGS_NO_CMPL);
> 
> Should we not do a bnxt_txr_db_kick() also? It maybe a good idea to do
> it in success case also instead of bnxt_db_write..

When this function returns, the caller writes the doorbell, so we should be
good.