* Re: [PATCH net] net: lan966x: don't free XDP buffers in fdma tx clear with budget 0
2026-10-29 11:47 [PATCH net] net: lan966x: don't free XDP buffers in fdma tx clear with budget 0 Daniel Machon
@ 2026-10-30 21:32 ` netdev-bot+sashiko
0 siblings, 0 replies; 2+ messages in thread
From: netdev-bot+sashiko @ 2026-10-30 21:32 UTC (permalink / raw)
To: daniel.machon
Cc: horatiu.vultur, UNGLinuxDriver, andrew+netdev, davem, edumazet,
kuba, pabeni, ast, daniel, hawk, john.fastabend, sdf, netdev,
linux-kernel, bpf
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 (1) · Medium: 1 (1) · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] This patch adds a skip to lan966x_fdma_tx_clear_buf()
(drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c, line 359): `if…
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but
lan966x_fdma_reload()…
- [Medium] This problem wasn't introduced by this patch, but
lan966x_fdma_deinit()…
--
net: lan966x: don't free XDP buffers in fdma tx clear with budget 0
This patch makes lan966x_fdma_tx_clear_buf() skip completed XDP_TX and
ndo_xdp_xmit DCBs when netpoll calls it with a budget of 0. That way the
page pool and XDP return APIs are not called with hardirqs disabled. The
skipped buffers are left for the next NAPI poll, and skbs are still freed
via napi_consume_skb().
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> index 21668bbcc9063..1fbe910c5ec38 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> @@ -353,6 +353,12 @@ static void lan966x_fdma_tx_clear_buf(struct lan966x *lan966x, int weight)
> if (!fdma_db_is_done(db))
> continue;
>
> + /* XDP and page pool APIs can't be used with a budget of 0.
> + * Leave these buffers for the next NAPI poll.
> + */
> + if (!weight && !dcb_buf->use_skb)
> + continue;
[Severity: Medium]
Can this starve netconsole transmits on the port that owns the FDMA NAPI?
skbs and XDP frames get slots from the same ring through
lan966x_fdma_get_next_dcb(), which only checks the used bit:
lan966x_fdma_get_next_dcb() {
...
if (!dcb_buf->used &&
!fdma_is_last(&tx->fdma, &tx->fdma.dcbs[i]))
return i;
...
}
If heavy XDP_TX or ndo_xdp_xmit traffic fills every usable slot, a
netpoll skb ends up here in lan966x_fdma_xmit():
next_to_use = lan966x_fdma_get_next_dcb(tx);
if (next_to_use < 0) {
netif_stop_queue(dev);
return NETDEV_TX_BUSY;
}
__netpoll_send_skb() then polls between retries:
netpoll_poll_dev()->poll_one_napi()->napi->poll(napi, 0)
->lan966x_fdma_tx_clear_buf(lan966x, 0)
With the new check, every completed XDP DCB is skipped. No slot is freed,
clear stays false, and lan966x_fdma_wakeup_netdev() is never called.
Once the retries run out, the skb is put on npinfo->txq and handed to
delayed work, and later messages queue behind it. In normal operation a
later IRQ-driven NAPI poll frees the DCBs and output only comes late.
During a panic, with the other CPUs stopped and hardirqs off, would that
work ever run? If not, the netconsole output is lost.
Before this patch, the budget-0 poll freed those slots, so netpoll could
make progress.
[Severity: High]
This isn't a bug introduced by this patch, but can XDP_TX pages that TX
DCBs still hold end up in the wrong page_pool when lan966x_fdma_reload()
replaces the RX pool?
lan966x_fdma_reload() does:
napi_synchronize(&lan966x->napi);
napi_disable(&lan966x->napi);
...
err = lan966x_fdma_rx_alloc(&lan966x->rx);
...
page_pool_destroy(page_pool);
lan966x_fdma_wakeup_netdev(lan966x);
napi_enable(&lan966x->napi);
It never walks tx->dcbs_buf[]. Suppose an XDP_TX DCB posted by the last
poll completes in hardware after napi_disable(). The FDMA IRQ's
napi_schedule() then does nothing, and the DCB stays used.
The first poll after napi_enable() then runs this in
lan966x_fdma_tx_clear_buf():
page_pool_recycle_direct(rx->page_pool,
dcb_buf->data.page);
At that point rx->page_pool is the new pool. __page_pool_put_page() does
not check which pool owns the page. So the old pool's page, with its old
DMA mapping and order, goes into the new pool's cache.
If the MTU change raised page_order, could the new pool give this smaller
page to the RX ring? There db_size is PAGE_SIZE << new_order, so the
device could DMA a large frame past the end of the page.
Even if the order stays the same, the old pool's inflight count would
never reach zero, so its shutdown stalls and the pool is leaked. And
wouldn't the new pool's inflight count go negative when it is destroyed?
This can happen on these paths while XDP_TX traffic is flowing:
lan966x_port_change_mtu()->lan966x_fdma_change_mtu()
->__lan966x_fdma_reload()->lan966x_fdma_reload()
lan966x_xdp_setup()->lan966x_fdma_reload_page_pool()
->__lan966x_fdma_reload()->lan966x_fdma_reload()
This patch doesn't widen the window much. An IRQ-driven NAPI poll still
reclaims those DCBs if it runs before napi_disable().
[Severity: Medium]
This is a pre-existing issue, but does lan966x_fdma_deinit() leak the TX
DCBs that are still marked used?
lan966x_fdma_deinit() {
...
napi_synchronize(&lan966x->napi);
napi_disable(&lan966x->napi);
lan966x_fdma_rx_free_pages(&lan966x->rx);
fdma_free_coherent(lan966x->dma_dev, &lan966x->rx.fdma);
page_pool_destroy(lan966x->rx.page_pool);
lan966x_fdma_tx_free(&lan966x->tx);
}
lan966x_fdma_tx_free() only does:
kfree(tx->dcbs_buf);
fdma_free_coherent(lan966x->dma_dev, &tx->fdma);
Entries that are still used are never released:
- skb and xdp_ndo buffers stay DMA-mapped
- skbs are not consumed
- xdp_frames are not returned
- XDP_TX pages are not returned to the page pool
Those pages keep the destroyed pool inflight, so its shutdown stalls.
lan966x_remove() and the probe error path both call fdma_deinit() before
lan966x_cleanup_ports(). After napi_disable() no poll can reclaim these
entries.
This patch adds completed but deferred XDP buffers to what can still be
outstanding here. The window is narrow, because an IRQ-driven poll
normally frees them before napi_disable().
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261029-lan966x-fdma-tx-clear-budget-v1-1-38c52d7aab8c%40microchip.com
^ permalink raw reply [flat|nested] 2+ messages in thread