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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.