From db2c610df81e34d497f49b68a0a1681c3f203bdb Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 27 Oct 2026 16:49:36 -0700 Subject: [PATCH 1/5] eth: bnxt: recycle agg buffers when Rx head buffer allocation fails bnxt_rx_pkt() tries to return the aggregation buffers of a packet to the ring when the copybreak allocation fails, but the same logic is missing on the rx_skb_func() failure path. The agg BDs consumed by the device are then never posted again. The agg ring is only refilled one-for-one as completions consume its buffers, so each such failure permanently shrinks it. Under sustained memory pressure the ring drains completely, and the queue stops receiving anything that does not fit into the head buffer, even once memory is available again. Only re-creating the rings (e.g. ifdown / ifup) recovers it. Commit 296d5b541639 ("bnxt_en: Fix aggregation buffer leak under OOM condition.") fixed the copybreak path only. Recently commit bd3beb5c8691 ("eth: bnxt: disable rx-copybreak by default") made the non-copybreak path the default, re-surfacing the issue. With XDP, the agg buffers are attached to the xdp_buff as frags by this point. Their agg ring slots have already been refilled. We need to keep the original head page (and therefore shinfo) around until the xdp_buff cleanup. Note the odd split of ownership here: rx_skb_func() only deals with the head buffer, the agg buffers stay with the caller in whatever state they are in, still on the ring without XDP, or frags of the xdp_buff with XDP. This complicates rx_skb_func() cleaning up all the buffers (head and frags), which would be more intuitive. Discovered in production, severe packet loss would persist after a transient OOM event. Tested on a BCM57504 by failing the head buffer allocations in bnxt_rx_skb() (bpftrace override() of should_failslab() and should_fail_alloc_page()) while receiving 1400B ICMPv6 echo requests. 341k packets were dropped on that queue, enough to drain its 8188 entry agg ring 40 times over, and the queue kept receiving large frames afterwards with no loss and no new HW drops. With a frags-capable XDP_PASS program at MTU 9000, receiving 8000B ICMPv6 echo requests while napi_build_skb() called from bnxt_rx_multi_page_skb() failed, 9.2k packets were dropped, the page pools' inflight count stayed flat and there were no KASAN or DEBUG_PAGEALLOC splats. Fixes: c0c050c58d84 ("bnxt_en: New Broadcom ethernet driver.") Fixes: 4c6c123c9af9 ("bnxt: adding bnxt_rx_agg_pages_xdp for aggregated xdp") Reviewed-by: Michael Chan Link: https://patch.msgid.link/20261027234940.1267225-2-kuba@kernel.org Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 33 ++++++++++++++--------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 0682060b048c..2e7276801fdd 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -1215,7 +1215,15 @@ static struct sk_buff *bnxt_rx_multi_page_skb(struct bnxt *bp, bp->rx_dir); skb = napi_build_skb(frag_start, rxr->rx_page_size); if (!skb) { - page_pool_recycle_direct(rxr->page_pool, page); + struct page *new_page; + + new_page = rxr->rx_buf_ring[RING_RX(bp, prod)].data; + /* The caller may still need the XDP frags from shinfo at the + * end of the head page. Release the new buffer instead and + * put the head back on the ring. + */ + page_pool_recycle_direct(rxr->page_pool, new_page); + bnxt_reuse_rx_data(rxr, cons, data); return NULL; } skb_mark_for_recycle(skb); @@ -2313,17 +2321,8 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr, skb = bnxt_copy_xdp(bnapi, &bnxt_xdp.xdp, len, dma_addr); bnxt_reuse_rx_data(rxr, cons, data); - if (!skb) { - if (agg_bufs) { - if (!xdp_active) - bnxt_reuse_rx_agg_bufs(cpr, cp_cons, 0, - agg_bufs, false); - else - bnxt_xdp_buff_frags_free(rxr, - &bnxt_xdp.xdp); - } - goto oom_next_rx; - } + if (!skb) + goto oom_next_rx_agg; } else { u32 payload; @@ -2334,7 +2333,7 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr, skb = bp->rx_skb_func(bp, rxr, cons, data, data_ptr, dma_addr, payload | len); if (!skb) - goto oom_next_rx; + goto oom_next_rx_agg; } if (agg_bufs) { @@ -2424,6 +2423,14 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr, return rc; +oom_next_rx_agg: + if (agg_bufs) { + if (!xdp_active) + bnxt_reuse_rx_agg_bufs(cpr, cp_cons, 0, agg_bufs, + false); + else + bnxt_xdp_buff_frags_free(rxr, &bnxt_xdp.xdp); + } oom_next_rx: cpr->sw_stats->rx.rx_oom_discards += 1; rc = -ENOMEM; From 09f10efee6f93863ea71489ce84563f4d51787f6 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 27 Oct 2026 16:49:37 -0700 Subject: [PATCH 2/5] eth: bnxt: resize Rx head buffers when rx-copybreak changes Since rx-copybreak became configurable, bnxt_set_ring_params() sizes the head buffers to fit at least rx_copybreak bytes, and the copybreak path in bnxt_copy_data() syncs rx_copybreak bytes of the head buffer. bnxt_set_tunable() however only updates bp->rx_copybreak, and nothing on the open path recalculates the ring parameters. After raising rx-copybreak the head buffers keep their old size until some other setting changes, and bnxt_copy_data() syncs past their end, into the next buffer in the page or past the end of the page: DMA-API: bnxt_en 0000:01:00.0: device driver tries to sync DMA memory it has not allocated [device address=0x00000002aa210c40] [size=1024 bytes] WARNING: kernel/dma/debug.c:1162 at check_sync+0x577/0x1eb0, CPU#22: swapper/22/0 Call Trace: debug_dma_sync_single_for_cpu+0x66/0x90 bnxt_copy_data+0x12f/0x290 bnxt_rx_pkt+0x24ea/0x50f0 __bnxt_poll_work+0x42c/0xc00 bnxt_poll_p5+0x579/0x17a0 __napi_poll+0x8b/0x2c0 net_rx_action+0x58f/0xbb0 Here the head buffers were still sized for the 640B HDS threshold, four 1kB buffers per page, while rx-copybreak was raised to 1024. Without coherent DMA or with swiotlb the oversized sync can corrupt a neighboring buffer owned by the device or by an skb. The device must be down for rx-copybreak to change, so recalculate the ring parameters right away, like bnxt_set_ringparam() does. Hit while testing patch 1, tested on BCM57504 - boot; set rx-copybreak to 1024 with the link down (CONFIG_DMA_API_DEBUG); ifup. Fixes: 152f4da05aee ("bnxt_en: add support for rx-copybreak ethtool command") Reviewed-by: Michael Chan Link: https://patch.msgid.link/20261027234940.1267225-3-kuba@kernel.org Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c index 622e89587e5d..ef4bea4a0f8c 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c @@ -4601,6 +4601,7 @@ static int bnxt_set_tunable(struct net_device *dev, if (netif_running(dev)) return -EBUSY; bp->rx_copybreak = rx_copybreak; + bnxt_set_ring_params(bp); } return 0; case ETHTOOL_PFC_PREVENTION_TOUT: From 1b2f02fb4ba92d01603c8261c94be8d348de65b0 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 27 Oct 2026 16:49:38 -0700 Subject: [PATCH 3/5] eth: bnxt: don't leak buffers when XDP agg buffer allocation fails If a replacement agg buffer can't be allocated for a packet received with XDP attached, bnxt_rx_pkt() drops the packet without returning its head buffer to the ring. The head buffer leaks, and since rx_prod still advances, the device gets a BD with a stale DMA address which points at a buffer the driver no longer owns. When that BD completes the driver finds no buffer in the slot. The agg buffers consumed before the failing one have already been replaced on the agg ring, they are referenced only by the frags of the xdp_buff, which nobody frees. bnxt_xdp_buff_frags_free() would skip them since the frags flag is set only on success, so set it upfront. Pointed out in LLM review of the first patch of the series. Tested on a BCM57504 at MTU 9000 with a frags-capable XDP_PASS program, receiving 8000B ICMPv6 echo requests (a head and two agg buffers each) while page_pool_alloc_netmems() called from bnxt_rx_agg_netmems_xdp() failed half of the time (bpftrace override()). 44.7k packets were dropped, the page pools' inflight count stayed flat and there were no KASAN or DEBUG_PAGEALLOC splats. Fixes: 4c6c123c9af9 ("bnxt: adding bnxt_rx_agg_pages_xdp for aggregated xdp") Reviewed-by: Michael Chan Link: https://patch.msgid.link/20261027234940.1267225-4-kuba@kernel.org Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 2e7276801fdd..4607c31eedf8 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -1427,16 +1427,19 @@ static u32 bnxt_rx_agg_netmems_xdp(struct bnxt *bp, struct skb_shared_info *shinfo = xdp_get_shared_info_from_buff(xdp); u32 total_frag_len = 0; - if (!xdp_buff_has_frags(xdp)) + if (!xdp_buff_has_frags(xdp)) { + xdp_buff_set_frags_flag(xdp); shinfo->nr_frags = 0; + } total_frag_len = __bnxt_rx_agg_netmems(bp, cpr, idx, agg_bufs, tpa, NULL, xdp); - if (total_frag_len) { - xdp_buff_set_frags_flag(xdp); - shinfo->nr_frags = agg_bufs; - shinfo->xdp_frags_size = total_frag_len; + if (!total_frag_len) { + bnxt_xdp_buff_frags_free(cpr->bnapi->rx_ring, xdp); + return 0; } + shinfo->nr_frags = agg_bufs; + shinfo->xdp_frags_size = total_frag_len; return total_frag_len; } @@ -2293,8 +2296,10 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr, cp_cons, agg_bufs, false); - if (!frag_len) + if (!frag_len) { + bnxt_reuse_rx_data(rxr, cons, data); goto oom_next_rx; + } } xdp_active = true; From c10f3420d59859df107838aa97b49357616d9b47 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 27 Oct 2026 16:49:39 -0700 Subject: [PATCH 4/5] eth: bnxt: free XDP frags when xdp_do_redirect() fails When xdp_do_redirect() fails bnxt_rx_xdp() recycles the head page but not the frags of a multi-buffer frame. Their agg ring slots have already been refilled, so the pages leak, and the page pool can't be destroyed until they are returned, which will never happen. Redirect to an AF_XDP socket in copy mode fails this way with -ENOMEM whenever the socket's fill ring runs out of buffers. A socket bound without XDP_USE_SG also fails with -ENOSPC for every frame that doesn't fit in a single chunk. All the other drop paths in this function free the frags. Pointed out in LLM review of the first patch of the series. Tested on a BCM57504 at MTU 9000 by redirecting 60k 8000B ICMPv6 echo requests (a head and two agg buffers each) to lo, which can't transmit XDP frames. The page pools' inflight count stayed flat rather than growing by two pages per frame. Fixes: a7559bc8c17c ("bnxt: support transmit and free of aggregation buffers") Reviewed-by: Michael Chan Link: https://patch.msgid.link/20261027234940.1267225-5-kuba@kernel.org Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c index 96845c8646e0..161e0dd62be6 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c @@ -307,6 +307,7 @@ bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons, if (xdp_do_redirect(bp->dev, xdp, xdp_prog)) { trace_xdp_exception(bp->dev, xdp_prog, act); + bnxt_xdp_buff_frags_free(rxr, xdp); page_pool_recycle_direct(rxr->page_pool, page); return true; } From 364b88b54fc260d22ee318bae862c2d7d8c31dc6 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 27 Oct 2026 16:49:40 -0700 Subject: [PATCH 5/5] eth: bnxt: hand XDP frags over to the copybreak skb With a multi-buffer XDP program attached, a packet only has agg buffers if it filled its head buffer, which in page mode holds most of a page. The program can still move the start of the packet forward with bpf_xdp_adjust_head() and leave at most rx_copybreak bytes in the head. bnxt_rx_pkt() then copies the head into a new skb, puts the head page back on the ring, and calls bnxt_xdp_build_skb() on the copy. bnxt_xdp_build_skb() only sets nr_frags and the skb's length accounting, because it expects an skb built around the head page, sharing its shinfo with the xdp_buff. The copy ends up with nr_frags set but no frag descriptors, and the stack later reads and releases whatever its frags array happens to contain. The real frags leak. Copy the frag descriptors to the new skb in bnxt_copy_xdp(), so that bnxt_xdp_build_skb() finds the frags where it expects them. The frags now belong to the skb, and the head page goes back on the ring as before. Pointed out in LLM review of the first patch of the series. Confirmed by reading the code, the unfixed kernel was not run with the reproducer below. Tested on a BCM57504 at MTU 9000 with rx-copybreak raised to 1024 and a frags-capable XDP program moving the start of multi-buffer frames 3000B forward with bpf_xdp_adjust_head(), receiving 8000B ICMPv6 echo requests (a head and two agg buffers each). All 60k frames took the copybreak path with frags attached, the sampled skbs reached GRO with len=4998 data_len=4492 nr_frags=2, and were freed by the stack with the page pools' inflight count staying flat, no KASAN or DEBUG_PAGEALLOC splats. Fixes: 1dc4c557bfed ("bnxt: adding bnxt_xdp_build_skb to build skb from multibuffer xdp_buff") Reviewed-by: Michael Chan Link: https://patch.msgid.link/20261027234940.1267225-6-kuba@kernel.org Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 4607c31eedf8..757c7266ce15 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -1511,6 +1511,14 @@ static struct sk_buff *bnxt_copy_xdp(struct bnxt_napi *bnapi, __skb_pull(skb, metasize); } + if (unlikely(xdp_buff_has_frags(xdp))) { + struct skb_shared_info *sinfo; + + sinfo = xdp_get_shared_info_from_buff(xdp); + memcpy(skb_shinfo(skb)->frags, sinfo->frags, + sinfo->nr_frags * sizeof(skb_frag_t)); + } + return skb; }