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
next prev parent 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