Re: [PATCH net] net: lan966x: don't free XDP buffers in fdma tx clear with budget 0

[Simon Horman]

On Mon, Nov 02, 2026 at 09:53:39AM +0100, Daniel Machon wrote:

...

> > 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.
> 
> Seems right, however, AFAICT, this is only an issue if the panic hits while the
> ring is completely full of finished XDP frames. Freeing those frames from
> netpoll is what napi.rst forbids: with a budget of 0 the driver cannot call any
> XDP or page pool APIs, and doing so is the splat this patch fixes.
> 
> The only other way I see is to give XDP its own FDMA injection channel, so XDP
> frames can never take the slots netpoll needs. lan966x has free injection
> channels for that, but it is a rework for net-next, not a fix for net.

Yes, agreed.

Reviewed-by: Simon Horman <horms@xxxxxxxxxx>