[PATCH net v5 0/6] Fix a variety of TPA bugs

[PATCH net v5 0/6] Fix a variety of TPA bugs

[Joe Damato]

Greetings:

I am sending this series as an extension to my v4 [1] which was just 1 patch.

Note that patch 5 of this series can now cause the device to fail closed if
memory is tight; bnxt_init_nic propagates an error that was previously
swallowed and fails closed instead of succeeding in a degraded state. If the
maintainers want the device to come up with a partially populated rx_tpa[],
then patch 5 can be dropped and this series can still be applied
and will otherwise work as intended.

This series addresses a variety of bugs orbiting the TPA code in the bnxt
driver that Sashiko (or Clashiko or whatever) pointed out and the series ends
with the patch from the v4 [1].

A lot of the noise generated by the AIs while reviewing my v4 are unrelated
bugs with different fixes tags that, IMHO, distract a bit from the crash at
boot that is currently occurring with Thor2 hardware on recent kernels.

That said, I've tried to wrangle this series together which I hope will solve
most of the important bugs the AIs are feeling something about.

I do not know what other rabbit holes the AIs will find when I submit this
series, but if there is some reasonable stop-gap that we can get applied to
fix the crashes on Thor2 (while I iterate on the rest of the bugs at the
pleasure of the AIs) that would be excellent.

I boot tested this on a Thor1 and a Thor2 machine and there were no crashes at
boot.

Thanks,
Joe

[1]: https://lore.kernel.org/all/20260828190900.1767611-1-joe@dama.to/

Joe Damato (6):
  bnxt_en: Only restore LRO if the device supports TPA
  bnxt_en: Don't free the live ring's TPA state on queue restart failure
  bnxt_en: Propagate TPA buffer allocation failures in
    bnxt_queue_mem_alloc()
  bnxt_en: Handle buffer allocation failure in bnxt_rx_ring_reset()
  bnxt_en: Propagate RX ring init failures in bnxt_init_nic()
  bnxt_en: Bound SW TPA IDs to prevent crashes

 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 57 +++++++++++++++++------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  2 +-
 2 files changed, 43 insertions(+), 16 deletions(-)


base-commit: 1d2929d0850fff683b8aff051275945e65f082c8
-- 
2.53.0-Meta

[PATCH net v5 1/6] bnxt_en: Only restore LRO if the device supports TPA

[Joe Damato]

With a P5+ device with firmware that reports max_aggs_supported == 0, it is
possible to make LRO settable by attaching and detaching an XDP program
even though the device does not support TPA.

Fix this by testing BNXT_SUPPORTS_TPA before restoring the feature bit.

Fixes: f0aa6a37a3db ("eth: bnxt: always recalculate features after XDP clearing, fix null-deref")
Reported-by: Sashiko <sashiko-bot+sashiko@kernel.org>
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260828190900.1767611-1-joe%40dama.to
Cc: stable@vger.kernel.org
Signed-off-by: Joe Damato <joe@dama.to>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d59bcca73a2b..0e5c2a48f313 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -5006,7 +5006,8 @@ void bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode)
 		bnxt_get_max_rings(bp, &rx, &tx, true);
 		if (rx > 1) {
 			bp->flags &= ~BNXT_FLAG_NO_AGG_RINGS;
-			bp->dev->hw_features |= NETIF_F_LRO;
+			if (BNXT_SUPPORTS_TPA(bp))
+				bp->dev->hw_features |= NETIF_F_LRO;
 		}
 	}
 
-- 
2.53.0-Meta

[PATCH net v5 2/6] bnxt_en: Don't free the live ring's TPA state on queue restart failure

[Joe Damato]

bnxt_queue_mem_alloc() shallow copies the live RX ring into the clone:

  memcpy(clone, rxr, sizeof(*rxr));

the code currently clears pointers that the clone owns (such as
rx_agg_bmap), but rx_tpa and rx_tpa_idx_map are left pointing at memory
of the live ring that was cloned.

If an allocation failure happens later and the err_free_tpa_info label
is taken, the live ring's memory can be freed while still in use.

Fix this by initializing the clone's pointers to NULL to prevent live
ring state from being freed inadvertently.

Fixes: bd649c5cc958 ("bnxt_en: handle tpa_info in queue API implementation")
Reported-by: Sashiko <sashiko-bot+sashiko@kernel.org>
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260828190900.1767611-1-joe%40dama.to
Cc: stable@vger.kernel.org
Signed-off-by: Joe Damato <joe@dama.to>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 0e5c2a48f313..f44a23c8a590 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -16333,6 +16333,8 @@ static int bnxt_queue_mem_alloc(struct net_device *dev,
 	clone->need_head_pool = false;
 	clone->rx_page_size = qcfg->rx_page_size;
 	clone->rx_agg_bmap = NULL;
+	clone->rx_tpa = NULL;
+	clone->rx_tpa_idx_map = NULL;
 
 	rc = bnxt_alloc_rx_page_pool(bp, clone, rxr->page_pool->p.nid);
 	if (rc)
-- 
2.53.0-Meta

[PATCH net v5 3/6] bnxt_en: Propagate TPA buffer allocation failures in bnxt_queue_mem_alloc()

[Joe Damato]

bnxt_alloc_one_tpa_info_data() returns -ENOMEM as soon as one allocation
fails. This leaves the remaining rxr->rx_tpa[] entries zeroed.

bnxt_queue_mem_alloc() discards that return value, so the partially
initialized ring is installed by bnxt_queue_start().

Since the agg_id is picked by the hardware and bnxt_alloc_agg_idx maps
it to a SW index in rxr->rx_tpa[], it is possible that an uninitialized
slot can be chosen which would hand a zero DMA address to the device.

Fix this by checking the return value of bnxt_alloc_one_tpa_info_data
and unwinding, freeing the ring buffers.

Fixes: bd649c5cc958 ("bnxt_en: handle tpa_info in queue API implementation")
Reported-by: Sashiko <sashiko-bot+sashiko@kernel.org>
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260828190900.1767611-1-joe%40dama.to
Cc: stable@vger.kernel.org
Signed-off-by: Joe Damato <joe@dama.to>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index f44a23c8a590..3755a30f8d40 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -16378,11 +16378,16 @@ static int bnxt_queue_mem_alloc(struct net_device *dev,
 	bnxt_alloc_one_rx_ring_skb(bp, clone, idx);
 	if (bp->flags & BNXT_FLAG_AGG_RINGS)
 		bnxt_alloc_one_rx_ring_netmem(bp, clone, idx);
-	if (bp->flags & BNXT_FLAG_TPA)
-		bnxt_alloc_one_tpa_info_data(bp, clone);
+	if (bp->flags & BNXT_FLAG_TPA) {
+		rc = bnxt_alloc_one_tpa_info_data(bp, clone);
+		if (rc)
+			goto err_free_rx_ring_skbs;
+	}
 
 	return 0;
 
+err_free_rx_ring_skbs:
+	bnxt_free_one_rx_ring_skbs(bp, clone);
 err_free_tpa_info:
 	bnxt_free_one_tpa_info(bp, clone);
 err_free_rx_agg_ring:
-- 
2.53.0-Meta

[PATCH net v5 4/6] bnxt_en: Handle buffer allocation failure in bnxt_rx_ring_reset()

[Joe Damato]

bnxt_rx_ring_reset() frees the ring buffers and then reallocates them,
ignoring the result.

bnxt_alloc_one_rx_ring() can fail in bnxt_alloc_one_tpa_info_data(), which
returns -ENOMEM on the first failed allocation and leaves the remaining
rxr->rx_tpa[] entries zeroed.

The error isn't propagated up, so the loop in bnxt_rx_ring_reset
continues and at the end the code re-enables TPA with partially
unallocated rx_tpa array.

This means that when the agg_id from hardware is mapped to a SW index in
rxr->rx_tpa[], an uninitialized slot can be chosen which would hand a
zero DMA address to the device.

Fix this by falling back to a global reset, which is what the existing
code already does when other functions fail, but unlike the other
failure cases this particular failure has to return because TPA can't
be re-enabled since the allocation failed.

Fixes: 8fbf58e17dce ("bnxt_en: Implement RX ring reset in response to buffer errors.")
Reported-by: Sashiko <sashiko-bot+sashiko@kernel.org>
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260828190900.1767611-1-joe%40dama.to
Cc: stable@vger.kernel.org
Signed-off-by: Joe Damato <joe@dama.to>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 3755a30f8d40..a8e5fdfcdf59 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -14604,7 +14604,14 @@ static void bnxt_rx_ring_reset(struct bnxt *bp)
 		rxr->rx_sw_agg_prod = 0;
 		rxr->rx_next_cons = 0;
 		rxr->bnapi->in_reset = false;
-		bnxt_alloc_one_rx_ring(bp, i);
+		rc = bnxt_alloc_one_rx_ring(bp, i);
+		if (rc) {
+			netdev_warn(bp->dev, "RX ring reset failed to allocate buffers, rc = %d, falling back to global reset\n",
+				    rc);
+			bnxt_reset_task(bp, true);
+			bnxt_rtnl_unlock_sp(bp);
+			return;
+		}
 		cpr = &rxr->bnapi->cp_ring;
 		cpr->sw_stats->rx.rx_resets++;
 		if (bp->flags & BNXT_FLAG_AGG_RINGS)
-- 
2.53.0-Meta

[PATCH net v5 5/6] bnxt_en: Propagate RX ring init failures in bnxt_init_nic()

[Joe Damato]

bnxt_init_rx_rings() returns an error when bnxt_alloc_one_rx_ring()
fails, but bnxt_init_nic() discards that return value and calls
bnxt_init_chip(), which enables TPA.

If an allocation fails, this could leave rxr->rx_tpa[] partially zeroed
and TPA would be enabled over an array with zeroed entries. This would
lead to a zeroed DMA address being handed out if the agg_idx is
translated to a SW index at a zeroed entry.

Fix this by propagating the error out of bnxt_init_nic(). Both callers
already check its return value and unwind with bnxt_free_skbs() and
bnxt_free_mem(), which tolerate a partially initialized RX ring.

Fixes: c0c050c58d84 ("bnxt_en: New Broadcom ethernet driver.")
Reported-by: Sashiko <sashiko-bot+sashiko@kernel.org>
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260828190900.1767611-1-joe%40dama.to
Cc: stable@vger.kernel.org
Signed-off-by: Joe Damato <joe@dama.to>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index a8e5fdfcdf59..d2943de1b62a 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -11341,8 +11341,13 @@ static int bnxt_shutdown_nic(struct bnxt *bp, bool irq_re_init)
 
 static int bnxt_init_nic(struct bnxt *bp, bool irq_re_init)
 {
+	int rc;
+
 	bnxt_init_cp_rings(bp);
-	bnxt_init_rx_rings(bp);
+	rc = bnxt_init_rx_rings(bp);
+	if (rc)
+		return rc;
+
 	bnxt_init_tx_rings(bp);
 	bnxt_init_ring_grps(bp, irq_re_init);
 	bnxt_init_vnics(bp);
-- 
2.53.0-Meta

[PATCH net v5 6/6] bnxt_en: Bound SW TPA IDs to prevent crashes

[Joe Damato]

FW supports up to 1024 concurrent TPAs, so the FW TPA ID is in the range
0..1023 (see commit ec4d8e7cf024 ("bnxt_en: Add TPA ID mapping logic for
57500 chips.")). bnxt_alloc_agg_idx is intended to wrap the FW ID down to a
software ID which is used to index rxr->rx_tpa, and to generate a mapping
between FW IDs and the wrapped software ID.

On a 57608 with firmware version 233, the firmware advertises 32
concurrent TPAs. As of the commit under fixes, bp->max_tpa on this NIC
is set to 32.

If the software ID from bnxt_alloc_agg_idx is above 31, this results in
an invalid address being loaded on this line:

  tpa_info = &rxr->rx_tpa[agg_id];

because rx_tpa is allocated with only bp->max_tpa (32) entries. Writes
to tpa_info later in the code are out of bounds.

This bug results in a crash at boot:

Oops: general protection fault, kernel NULL pointer dereference 0x8: 0000 [#1] SMP NOPTI
RIP: 0010:bnxt_rx_pkt+0xc0/0x1560
RSP: 0018:ffffc900009b8c78 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000048 RCX: 0000000206682516
RDX: ffffc900009b8db4 RSI: 0000000000000000 RDI: 01ffffff038fe1c0
RBP: ffffc9006e687480 R08: ffffc9006e687000 R09: 0000000000003048
R10: 0000000000000480 R11: ffff8881c6083900 R12: 0000000006682516
R13: ffff8881c6095400 R14: 0000000000000016 R15: ffff8881c6b66680
FS:  0000000000000000(0000) GS:ffff88fef3c77000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fc8bda40584 CR3: 000000807c812001 CR4: 0000000008772ef0
PKRU: 55555554
Call Trace:
 <IRQ>
 ? __netif_receive_skb_list_core+0x1ca/0x250
 __bnxt_poll_work+0x152/0x280
 bnxt_poll_p5+0x1cd/0x480
 __napi_poll+0x30/0x180
 net_rx_action+0x20b/0x3b0
 ? note_gp_changes+0x53/0xe0
 ? tick_setup_sched_timer+0x180/0x180
 ? __napi_schedule+0x9a/0xb0
 ? bnxt_msix+0x24/0x30
 handle_softirqs+0xdd/0x2c0
 __irq_exit_rcu.llvm.3171231171502365008+0x47/0xf0
 common_interrupt+0x85/0x90
 </IRQ>
 <TASK>
 asm_common_interrupt+0x22/0x40

This stack trace is from a crash triggered when an out of bounds rx_tpa
is dereferenced. The invalid write mentioned above is silent in this
particular crash.

Fix this by allocating rx_tpa with bp->max_tpa rounded up to the next
power of 2 (bp->max_tpa_roundup_size) entries and masking the FW TPA ID
with that size, so the wrapped ID can never index past the end of the
array.

Fixes: 54c28fab2fa5 ("bnxt_en: Set bp->max_tpa according to what the FW supports")
Reported-by: Raphael Cardoso Fernandes <raphaelcf@meta.com>
Suggested-by: Michael Chan <michael.chan@broadcom.com>
Cc: stable@vger.kernel.org
Signed-off-by: Joe Damato <joe@dama.to>
---
v5:
  - Updated the commit message to mention where the 0..1023 range comes from.
  - Updated the comment in the code now that a previous commit in this series
    addresses the LRO issue.

v4: https://lore.kernel.org/all/20260828190900.1767611-1-joe@dama.to/
  - Moved bp->max_tpa_roundup_size init out of the early return path and
    documented that TPA is unsupported there, as suggested by Michael.

v3: https://lore.kernel.org/netdev/20260827185700.2157164-1-joe@dama.to/
  - Addressed an issue Sashiko pointed out, where max_tpa_roundup_size may be
    left unset if bnxt_alloc_tpa_info returns early, which would lead to out
    of bounds access.
  - The other pre-existing issues Sashiko pointed out are unrelated to this
    patch and would need different Fixes tags, so they are better served with
    separate patches in the future.

v2: https://lore.kernel.org/netdev/20260825001842.2501798-1-joe@dama.to/
  - Followed Michael's suggestion on the v1 to increase the size of the tpa
    array so that wrapping indexes into the array is a simple mask.
  - Add Suggested-by because the approach was suggested by Michael.
  - Add a Reported-by so that Raphael gets credit for reporting this bug.
  - Boot tested on a machine with a 57608 and the crash did not reproduce.

v1: https://lore.kernel.org/netdev/20260821233549.3134699-1-joe@dama.to/

 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 27 ++++++++++++++---------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  2 +-
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d2943de1b62a..b2ce5b4e5e6a 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1514,14 +1514,16 @@ static int bnxt_discard_rx(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
 	return 0;
 }
 
-static u16 bnxt_alloc_agg_idx(struct bnxt_rx_ring_info *rxr, u16 agg_id)
+static u16 bnxt_alloc_agg_idx(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
+			      u16 agg_id)
 {
 	struct bnxt_tpa_idx_map *map = rxr->rx_tpa_idx_map;
-	u16 idx = agg_id & MAX_TPA_P5_MASK;
+	u16 idx = agg_id & (bp->max_tpa_roundup_size - 1);
 
 	if (test_bit(idx, map->agg_idx_bmap)) {
-		idx = find_first_zero_bit(map->agg_idx_bmap, MAX_TPA_P5);
-		if (idx >= MAX_TPA_P5)
+		idx = find_first_zero_bit(map->agg_idx_bmap,
+					  bp->max_tpa_roundup_size);
+		if (idx >= bp->max_tpa_roundup_size)
 			return INVALID_HW_RING_ID;
 	}
 	__set_bit(idx, map->agg_idx_bmap);
@@ -1586,7 +1588,7 @@ static void bnxt_tpa_start(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
 
 	if (bp->flags & BNXT_FLAG_CHIP_P5_PLUS) {
 		agg_id = TPA_START_AGG_ID_P5(tpa_start);
-		agg_id = bnxt_alloc_agg_idx(rxr, agg_id);
+		agg_id = bnxt_alloc_agg_idx(bp, rxr, agg_id);
 		if (unlikely(agg_id == INVALID_HW_RING_ID)) {
 			netdev_warn(bp->dev, "Unable to allocate agg ID for ring %d, agg 0x%x\n",
 				    rxr->bnapi->index,
@@ -3584,7 +3586,7 @@ static void bnxt_free_one_tpa_info_data(struct bnxt *bp,
 {
 	int i;
 
-	for (i = 0; i < bp->max_tpa; i++) {
+	for (i = 0; i < bp->max_tpa_roundup_size; i++) {
 		struct bnxt_tpa_info *tpa_info = &rxr->rx_tpa[i];
 		u8 *data = tpa_info->data;
 
@@ -3781,7 +3783,7 @@ static void bnxt_free_one_tpa_info(struct bnxt *bp,
 	kfree(rxr->rx_tpa_idx_map);
 	rxr->rx_tpa_idx_map = NULL;
 	if (rxr->rx_tpa) {
-		for (i = 0; i < bp->max_tpa; i++) {
+		for (i = 0; i < bp->max_tpa_roundup_size; i++) {
 			kfree(rxr->rx_tpa[i].agg_arr);
 			rxr->rx_tpa[i].agg_arr = NULL;
 		}
@@ -3807,13 +3809,14 @@ static int bnxt_alloc_one_tpa_info(struct bnxt *bp,
 	struct rx_agg_cmp *agg;
 	int i;
 
-	rxr->rx_tpa = kzalloc_objs(struct bnxt_tpa_info, bp->max_tpa);
+	rxr->rx_tpa = kzalloc_objs(struct bnxt_tpa_info,
+				   bp->max_tpa_roundup_size);
 	if (!rxr->rx_tpa)
 		return -ENOMEM;
 
 	if (!(bp->flags & BNXT_FLAG_CHIP_P5_PLUS))
 		return 0;
-	for (i = 0; i < bp->max_tpa; i++) {
+	for (i = 0; i < bp->max_tpa_roundup_size; i++) {
 		agg = kzalloc_objs(*agg, MAX_SKB_FRAGS);
 		if (!agg)
 			return -ENOMEM;
@@ -3832,6 +3835,9 @@ static int bnxt_alloc_tpa_info(struct bnxt *bp)
 
 	bp->max_tpa = MAX_TPA;
 	if (bp->flags & BNXT_FLAG_CHIP_P5_PLUS) {
+		/* TPA is not supported at all, so there is nothing to
+		 * allocate.
+		 */
 		if (!bp->max_tpa_v2)
 			return 0;
 		bp->max_tpa = min_t(u16, bp->max_tpa_v2, MAX_TPA_P5);
@@ -3839,6 +3845,7 @@ static int bnxt_alloc_tpa_info(struct bnxt *bp)
 		if (bp->max_tpa <= 32 && BNXT_CHIP_P5(bp) && !BNXT_NPAR(bp))
 			bp->max_tpa = MAX_TPA_P5;
 	}
+	bp->max_tpa_roundup_size = roundup_pow_of_two(bp->max_tpa);
 
 	for (i = 0; i < bp->rx_nr_rings; i++) {
 		struct bnxt_rx_ring_info *rxr = &bp->rx_ring[i];
@@ -4551,7 +4558,7 @@ static int bnxt_alloc_one_tpa_info_data(struct bnxt *bp,
 	u8 *data;
 	int i;
 
-	for (i = 0; i < bp->max_tpa; i++) {
+	for (i = 0; i < bp->max_tpa_roundup_size; i++) {
 		data = __bnxt_alloc_rx_frag(bp, &mapping, rxr,
 					    GFP_KERNEL);
 		if (!data)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index ab894f8addef..de46b42d7c98 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -789,7 +789,6 @@ struct nqe_cn {
 
 #define MAX_TPA		64
 #define MAX_TPA_P5	256
-#define MAX_TPA_P5_MASK	(MAX_TPA_P5 - 1)
 #define MAX_TPA_SEGS_P5	0x3f
 
 #if (BNXT_PAGE_SHIFT == 16)
@@ -2380,6 +2379,7 @@ struct bnxt {
 
 	u16			max_tpa_v2;
 	u16			max_tpa;
+	u16			max_tpa_roundup_size;
 	u32			rx_buf_size;
 	u32			rx_buf_use_size;	/* useable size */
 	u16			rx_offset;
-- 
2.53.0-Meta

Re: [net,v5,4/6] bnxt_en: Handle buffer allocation failure in bnxt_rx_ring_reset()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 1

- [Medium] Commit message misdescribes the failure mechanism it is
  fixing.
- [Low] The new error path in `bnxt_rx_ring_reset()`
  (drivers/net/ethernet/broadcom/bnxt/bnxt.c:14606-14616) loses the…

--

bnxt_en: Handle buffer allocation failure in bnxt_rx_ring_reset()

bnxt_rx_ring_reset() used to ignore the return value of
bnxt_alloc_one_rx_ring().  The patch checks it and, on failure, logs a
warning and falls back to a global reset instead of re-enabling TPA with a
partially populated rxr->rx_tpa[] array.

Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260828190900.1767611-1-joe%40dama.to

A couple of questions below, one about the commit message and one about the
new error path.

>     bnxt_alloc_one_rx_ring() can fail in bnxt_alloc_one_tpa_info_data(), which
>     returns -ENOMEM on the first failed allocation and leaves the remaining
>     rxr->rx_tpa[] entries zeroed.
>
>     This means that when the agg_id from hardware is mapped to a SW index in
>     rxr->rx_tpa[], an uninitialized slot can be chosen which would hand a
>     zero DMA address to the device.

[Severity: Medium]
Is the "entries zeroed" / "zero DMA address" description accurate for the
reset path?

bnxt_rx_ring_reset() calls bnxt_free_one_rx_ring_skbs() ->
bnxt_free_one_tpa_info_data() first, and that only clears ->data:

    for (i = 0; i < bp->max_tpa; i++) {
            struct bnxt_tpa_info *tpa_info = &rxr->rx_tpa[i];
            u8 *data = tpa_info->data;

            if (!data)
                    continue;

            tpa_info->data = NULL;
            page_pool_free_va(rxr->head_pool, data, false);
    }

->data_ptr and ->mapping are left pointing at the buffer that was just
returned to the head pool.  bnxt_alloc_one_tpa_info_data() then bails out
at the first failure:

    for (i = 0; i < bp->max_tpa; i++) {
            data = __bnxt_alloc_rx_frag(bp, &mapping, rxr, GFP_KERNEL);
            if (!data)
                    return -ENOMEM;
            ...
    }

so entries [i, bp->max_tpa) keep the stale mapping from before the reset
rather than becoming zero.  In the unfixed code bnxt_tpa_start() would then
program that stale address into the RX BD:

    mapping = tpa_info->mapping;
    prod_rx_buf->mapping = mapping;
    prod_bd = &rxr->rx_desc_ring[RX_RING(bp, prod)][RX_IDX(prod)];
    prod_bd->rx_bd_haddr = cpu_to_le64(mapping);

Would it be more precise to say the device would DMA into a recycled
page-pool buffer (and prod_rx_buf->data would be NULL), instead of a zero
DMA address?  Since this is tagged for stable, the description is what
backporters will read.

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 3755a30f8d403..a8e5fdfcdf591 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -14604,7 +14604,14 @@ static void bnxt_rx_ring_reset(struct bnxt *bp)
>  		rxr->rx_sw_agg_prod = 0;
>  		rxr->rx_next_cons = 0;
>  		rxr->bnapi->in_reset = false;
> -		bnxt_alloc_one_rx_ring(bp, i);
> +		rc = bnxt_alloc_one_rx_ring(bp, i);
> +		if (rc) {
> +			netdev_warn(bp->dev, "RX ring reset failed to allocate buffers, rc = %d, falling back to global reset\n",
> +				    rc);
> +			bnxt_reset_task(bp, true);
> +			bnxt_rtnl_unlock_sp(bp);
> +			return;
> +		}
>  		cpr = &rxr->bnapi->cp_ring;
>  		cpr->sw_stats->rx.rx_resets++;

[Severity: Low]
Does this new path lose the rx_resets accounting for the reset it just did?

rxr->bnapi->in_reset = false is done just above the allocation, and the new
error path returns before reaching cpr->sw_stats->rx.rx_resets++.

The fallback bnxt_reset_task(bp, true) -> bnxt_close_nic() ->
bnxt_disable_napi() is the only other place that bumps this counter, and it
is gated on in_reset:

    if (bnapi->in_reset)
            cpr->sw_stats->rx.rx_resets++;

which is already false for this ring, so the event is counted nowhere.

Contrast with the sibling failure path in the same function: the
bnxt_hwrm_rx_ring_reset() failure does a break with in_reset still set, so
its global reset does get counted by bnxt_disable_napi().  That leaves two
paths that both do an RX ring reset plus a global reset reporting different
rx_resets values via ethtool -S.

cpr->sw_stats is only freed by bnxt_free_ring_stats(), which is not reached
for an irq_re_init == false close, so the missing increment stays visible in
rx_resets and rx_total_resets.

Would incrementing rx_resets before the early return, or only clearing
in_reset after a successful bnxt_alloc_one_rx_ring(), be preferable here?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260902015652.2421609-1-joe%40dama.to

Re: [PATCH net v5 0/6] Fix a variety of TPA bugs

[Paolo Abeni]

On 9/2/26 3:56 AM, Joe Damato wrote:
> I am sending this series as an extension to my v4 [1] which was just 1 patch.
> 
> Note that patch 5 of this series can now cause the device to fail closed if
> memory is tight; bnxt_init_nic propagates an error that was previously
> swallowed and fails closed instead of succeeding in a degraded state. If the
> maintainers want the device to come up with a partially populated rx_tpa[],
> then patch 5 can be dropped and this series can still be applied
> and will otherwise work as intended.
> 
> This series addresses a variety of bugs orbiting the TPA code in the bnxt
> driver that Sashiko (or Clashiko or whatever) pointed out and the series ends
> with the patch from the v4 [1].
> 
> A lot of the noise generated by the AIs while reviewing my v4 are unrelated
> bugs with different fixes tags that, IMHO, distract a bit from the crash at
> boot that is currently occurring with Thor2 hardware on recent kernels.
> 
> That said, I've tried to wrangle this series together which I hope will solve
> most of the important bugs the AIs are feeling something about.
> 
> I do not know what other rabbit holes the AIs will find when I submit this
> series, but if there is some reasonable stop-gap that we can get applied to
> fix the crashes on Thor2 (while I iterate on the rest of the bugs at the
> pleasure of the AIs) that would be excellent.
> 
> I boot tested this on a Thor1 and a Thor2 machine and there were no crashes at
> boot.
Waiting for Michael's ack here. Note that I think that the AI report on
patch 5 is not a blocker.

/P

Re: [PATCH net v5 0/6] Fix a variety of TPA bugs

[Joe Damato]

On Thu, Sep 03, 2026 at 11:37:34AM +0200, Paolo Abeni wrote:
> On 9/2/26 3:56 AM, Joe Damato wrote:

[...]

> > I boot tested this on a Thor1 and a Thor2 machine and there were no crashes at
> > boot.
> Waiting for Michael's ack here. Note that I think that the AI report on
> patch 5 is not a blocker.

Aright. If there's no reply within 24hr of posting do you want me to spin a v6
that:
  - fixes the commit message sashiko doesnt like for patch 4
  - fixes the resets stat it mentions in patch 4
  - add the return value check for bnxt_init_ring_grps in patch 5

or just leave the series as is ?

Re: [PATCH net v5 0/6] Fix a variety of TPA bugs

[Paolo Abeni]

On 9/3/26 5:19 PM, Joe Damato wrote:
> On Thu, Sep 03, 2026 at 11:37:34AM +0200, Paolo Abeni wrote:
>> On 9/2/26 3:56 AM, Joe Damato wrote:
> 
> [...]
> 
>>> I boot tested this on a Thor1 and a Thor2 machine and there were no crashes at
>>> boot.
>> Waiting for Michael's ack here. Note that I think that the AI report on
>> patch 5 is not a blocker.
> 
> Aright. If there's no reply within 24hr of posting do you want me to spin a v6
> that:
>   - fixes the commit message sashiko doesnt like for patch 4
>   - fixes the resets stat it mentions in patch 4
>   - add the return value check for bnxt_init_ring_grps in patch 5
> 
> or just leave the series as is ?
No other revision required IMHO just for the above. The 2nd and 3rd
points could be eventually follow-ups.

/P