Netdev List
 help / color / mirror / Atom feed
From: Michael Chan <michael.chan@broadcom.com>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, andrew+netdev@lunn.ch,
	pavan.chebbi@broadcom.com, andrew.gospodarek@broadcom.com
Subject: [PATCH net 1/4] bnxt_en: Fix ring accounting underflow when rings are constrained
Date: Sun, 30 Aug 2026 19:43:39 -0700	[thread overview]
Message-ID: <20260831024342.2161156-2-michael.chan@broadcom.com> (raw)
In-Reply-To: <20260831024342.2161156-1-michael.chan@broadcom.com>

When __bnxt_reserve_rings() reserves fewer TX rings than requested,
and an XDP program is attached, bnxt_adj_tx_rings() blindly subtracts
bp->tx_nr_rings_xdp from bp->tx_nr_rings, potentially causing the
result to be negative (large value).  The large value will propagate
and cause unpredictable failures.

bnxt_adj_tx_rings() should scale down the TX rings for XDP and TCs
evenly when there is a shortage of TX rings to be correct.  Because
XDP requires a 1:1 mapping with RX rings in combined channel mode,
bp->tx_nr_rings_xdp must be equal to bp->tx_nr_rings_per_tc.  Any
leftover rings after integer division is intentionally left unused.
This will fix the underflow resulting in a negative (large) value.

Additionally, update bnxt_rings_ok() to require a minimum number of
TX rings based on the active configuration (at least 1 ring per TC,
plus 1 XDP ring if XDP is enabled). This guarantees that
bnxt_adj_tx_rings() always has enough rings to satisfy the minimum
viable configuration, gracefully failing the reservation otherwise.

The bnxt_rings_ok() check in __bnxt_reserve_rings() is moved earlier
to return -ENOMEM if we don't have the bare minimum resources before
we commit and update the software state.  Also add a check for
bnxt_trim_rings() failure earlier in the same function for the
same purpose.

Now that we have the proper bnxt_rings_ok() check for the bare
minimum and a more robust bnxt_adj_tx_rings() to handle fewer rings
than requested, we can remove the error path at the end of
bnxt_reserve_rings() that would abort if the rings could not
satisfy the TC requirements.

This existing issue was detetced by Sashiko when reviewing the
new kTLS patchset (patch #3 of 15):

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260810051358.1244418-7-michael.chan@broadcom.com

Fixes: 1ee581c24dfd ("bnxt_en: Adjust TX rings if reservation is less than requested")
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 45 ++++++++++++-----------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d59bcca73a2b..219a6f551f1d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -8135,8 +8135,14 @@ static void bnxt_copy_reserved_rings(struct bnxt *bp, struct bnxt_hw_rings *hwr)
 
 static bool bnxt_rings_ok(struct bnxt *bp, struct bnxt_hw_rings *hwr)
 {
-	return hwr->tx && hwr->rx && hwr->cp && hwr->grp && hwr->vnic &&
-	       hwr->stat && (hwr->cp_p5 || !(bp->flags & BNXT_FLAG_CHIP_P5_PLUS));
+	int min_tx = bp->num_tc ? bp->num_tc : 1;
+
+	if (bp->tx_nr_rings_xdp)
+		min_tx++;
+
+	return hwr->tx >= min_tx && hwr->rx && hwr->cp && hwr->grp &&
+	       hwr->vnic && hwr->stat &&
+	       (hwr->cp_p5 || !(bp->flags & BNXT_FLAG_CHIP_P5_PLUS));
 }
 
 static int bnxt_get_avail_msix(struct bnxt *bp, int num);
@@ -8211,10 +8217,16 @@ static int __bnxt_reserve_rings(struct bnxt *bp)
 		hwr.stat -= bnxt_get_ulp_stat_ctxs(bp);
 	hwr.cp = min_t(int, hwr.cp, hwr.stat);
 	rc = bnxt_trim_rings(bp, &rx_rings, &hwr.tx, hwr.cp, sh);
+	if (rc)
+		return rc;
 	if (bp->flags & BNXT_FLAG_AGG_RINGS)
 		hwr.rx = rx_rings << 1;
 	tx_cp = bnxt_num_tx_to_cp(bp, hwr.tx);
 	hwr.cp = sh ? max_t(int, tx_cp, rx_rings) : tx_cp + rx_rings;
+
+	if (!bnxt_rings_ok(bp, &hwr))
+		return -ENOMEM;
+
 	if (hwr.tx != bp->tx_nr_rings) {
 		netdev_warn(bp->dev,
 			    "Able to reserve only %d out of %d requested TX rings\n",
@@ -8243,9 +8255,6 @@ static int __bnxt_reserve_rings(struct bnxt *bp)
 	    hwr.rss_ctx < bnxt_get_total_rss_ctxs(bp, &hwr))
 		bp->rss_cap &= ~BNXT_RSS_CAP_LARGE_RSS_CTX;
 
-	if (!bnxt_rings_ok(bp, &hwr))
-		return -ENOMEM;
-
 	if (old_rx_rings != bp->hw_resc.resv_rx_rings &&
 	    !netif_is_rxfh_configured(bp->dev))
 		bnxt_set_dflt_rss_indir_tbl(bp, NULL);
@@ -11663,7 +11672,6 @@ int bnxt_reserve_rings(struct bnxt *bp, bool irq_re_init)
 	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
 	bool irq_cleared = false;
 	bool irq_change = false;
-	int tcs = bp->num_tc;
 	int irqs_required;
 	int rc;
 
@@ -11701,17 +11709,6 @@ int bnxt_reserve_rings(struct bnxt *bp, bool irq_re_init)
 		netdev_err(bp->dev, "ring reservation/IRQ init failure rc: %d\n", rc);
 		return rc;
 	}
-	if (tcs && (bp->tx_nr_rings_per_tc * tcs !=
-		    bp->tx_nr_rings - bp->tx_nr_rings_xdp)) {
-		netdev_err(bp->dev, "tx ring reservation failure\n");
-		netdev_reset_tc(bp->dev);
-		bp->num_tc = 0;
-		if (bp->tx_nr_rings_xdp)
-			bp->tx_nr_rings_per_tc = bp->tx_nr_rings_xdp;
-		else
-			bp->tx_nr_rings_per_tc = bp->tx_nr_rings;
-		return -ENOMEM;
-	}
 	return 0;
 }
 
@@ -13216,11 +13213,17 @@ static void bnxt_set_xdp_tx_rings(struct bnxt *bp)
 
 static void bnxt_adj_tx_rings(struct bnxt *bp)
 {
+	int tcs = bp->num_tc ? bp->num_tc : 1;
+
 	/* Make adjustments if reserved TX rings are less than requested */
-	bp->tx_nr_rings -= bp->tx_nr_rings_xdp;
-	bp->tx_nr_rings_per_tc = bnxt_tx_nr_rings_per_tc(bp);
-	if (bp->tx_nr_rings_xdp)
-		bnxt_set_xdp_tx_rings(bp);
+	if (bp->tx_nr_rings_xdp) {
+		tcs++;
+		bp->tx_nr_rings_per_tc = bp->tx_nr_rings / tcs;
+		bp->tx_nr_rings_xdp = bp->tx_nr_rings_per_tc;
+	} else {
+		bp->tx_nr_rings_per_tc = bnxt_tx_nr_rings_per_tc(bp);
+	}
+	bp->tx_nr_rings = bp->tx_nr_rings_per_tc * tcs;
 }
 
 static int __bnxt_open_nic(struct bnxt *bp, bool irq_re_init, bool link_re_init)
-- 
2.51.0


  reply	other threads:[~2026-08-31  2:44 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31  2:43 [PATCH net 0/4] bnxt_en: Bug fixes Michael Chan
2026-08-31  2:43 ` Michael Chan [this message]
2026-09-02  5:46   ` [net,1/4] bnxt_en: Fix ring accounting underflow when rings are constrained netdev-bot+sashiko
2026-08-31  2:43 ` [PATCH net 2/4] bnxt_en: Add bnxt_clear_bars() helper Michael Chan
2026-09-01 22:09   ` Joe Damato
2026-08-31  2:43 ` [PATCH net 3/4] bnxt_en: Fix driver init in kdump kernel Michael Chan
2026-09-01 22:20   ` Joe Damato
2026-09-02  5:46   ` [net,3/4] " netdev-bot+sashiko
2026-08-31  2:43 ` [PATCH net 4/4] bnxt_en: Re-write the BARs following any type of PCIe errors Michael Chan
2026-09-01 22:15   ` Joe Damato
2026-09-02  5:46   ` [net,4/4] " netdev-bot+sashiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260831024342.2161156-2-michael.chan@broadcom.com \
    --to=michael.chan@broadcom.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=andrew.gospodarek@broadcom.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pavan.chebbi@broadcom.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox