[PATCH net] net: bgmac: don't process RX with a zero NAPI budget

[PATCH net] net: bgmac: don't process RX with a zero NAPI budget

[Rosen Penev]

netpoll calls the NAPI poll with a budget of 0 and expects only TX
completions to be handled. bgmac_poll() passes the budget straight to
bgmac_dma_rx_read(), which checks it only after handling a descriptor,
so one received frame is always processed and bgmac_poll() returns 1.
That trips the "exceeded budget in poll" WARN_ONCE() in poll_one_napi()
when netconsole is used, and passes a frame up the stack from netpoll
context.

Return right after cleaning TX when the budget is 0. NAPI stays
scheduled, so the pending RX frames are picked up by the next regular
poll.

Fixes: dd4544f05469 ("bgmac: driver for GBit MAC core on BCMA bus")
Assisted-by: LLM
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/broadcom/bgmac.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 427a096e5ad4..b1cb973caee5 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1154,6 +1154,11 @@ static int bgmac_poll(struct napi_struct *napi, int weight)
 	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
 
 	bgmac_dma_tx_free(bgmac, &bgmac->tx_ring[0]);
+
+	/* A zero budget (netpoll) means only TX may be cleaned */
+	if (unlikely(!weight))
+		return 0;
+
 	handled += bgmac_dma_rx_read(bgmac, &bgmac->rx_ring[0], weight);
 
 	/* Poll again if more events arrived in the meantime */
-- 
2.56.0

Re: [PATCH net] net: bgmac: don't process RX with a zero NAPI budget

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] net: bgmac: don't process RX with a zero NAPI budget

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 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] In drivers/net/ethernet/broadcom/bgmac.c, bgmac_poll() still
  acknowledges every interrupt source, including BGMAC_IS_RX, with…

--

net: bgmac: don't process RX with a zero NAPI budget

When bgmac_poll() is called with a zero budget, this patch makes it
return right after TX completion cleanup. The goal is to stop netpoll
from processing an RX frame and tripping the "exceeded budget in poll"
WARN_ONCE() in poll_one_napi().

> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> index 3e9c57196a397..a545c0cc25977 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.c
> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> @@ -1154,6 +1154,11 @@ static int bgmac_poll(struct napi_struct *napi, int weight)
>  	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
>  
>  	bgmac_dma_tx_free(bgmac, &bgmac->tx_ring[0]);
> +
> +	/* A zero budget (netpoll) means only TX may be cleaned */
> +	if (unlikely(!weight))
> +		return 0;

[Severity: Medium]
Can this leave received frames stuck in the RX ring?

Before it reaches the new zero budget return, bgmac_poll() still clears
every interrupt status bit, including BGMAC_IS_RX, with a write-1-clear:

    bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);

The commit message says:

    NAPI stays scheduled, so the pending RX frames are picked up by the
    next regular poll.

Is that guaranteed?

poll_napi() calls poll_one_napi() for every NAPI on dev->napi_list
whose poll_owner it can take. poll_one_napi() then calls
napi->poll(napi, 0) without checking NAPI_STATE_SCHED.

bgmac_netdev_ops has no ndo_poll_controller, so bgmac_interrupt() is not
run first. As a result, bgmac_poll(napi, 0) can run while the NAPI is
idle and chip interrupts are on.

Suppose a frame arrives while netpoll is transmitting. One example is
__netpoll_send_skb() running with IRQs disabled while the TX queue is
stopped. The sequence would be:

netpoll_poll_dev()->poll_napi()->poll_one_napi()->bgmac_poll(napi, 0)
    bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);   /* clears BGMAC_IS_RX */
    bgmac_dma_tx_free(bgmac, &bgmac->tx_ring[0]);
    return 0;                                   /* NAPI not scheduled */

After that, bgmac_interrupt() finds nothing pending:

    u32 int_status = bgmac_read(bgmac, BGMAC_INT_STATUS);
    int_status &= bgmac->int_mask;

    if (!int_status)
        return IRQ_NONE;

bgmac_interrupt() is the only caller of napi_schedule() here. Frames
already in the RX ring would sit there until another frame raises
BGMAC_IS_RX. On a quiet link, for example after a single ARP or ping
reply, that wait has no upper bound.

The unconditional ack existed before this patch. However, one RX slot
used to be processed in this window, and now none are.

Would it work to ack only BGMAC_IS_TX0 when the budget is 0? That would
leave BGMAC_IS_RX set, so the interrupt handler still schedules NAPI.

The new comment says "only TX may be cleaned". The NAPI documentation
says a zero budget means processing "only skb Tx completions and no Rx".
This path still consumes RX interrupt state, which doesn't match either
statement.

> +
>  	handled += bgmac_dma_rx_read(bgmac, &bgmac->rx_ring[0], weight);
>  
>  	/* Poll again if more events arrived in the meantime */

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014001912.141532-1-rosenp%40gmail.com