All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Daniel Machon <daniel.machon@microchip.com>
Cc: netdev-bot+sashiko@kernel.org, horatiu.vultur@microchip.com,
	UNGLinuxDriver@microchip.com, andrew+netdev@lunn.ch,
	davem@davemloft.net, edumazet@kernel.org, kuba@kernel.org,
	pabeni@redhat.com, ast@kernel.org, daniel@iogearbox.net,
	hawk@kernel.org, john.fastabend@gmail.com, sdf@fomichev.me,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	bpf@vger.kernel.org
Subject: Re: [PATCH net] net: lan966x: don't free XDP buffers in fdma tx clear with budget 0
Date: Mon, 2 Nov 2026 11:41:35 +0000	[thread overview]
Message-ID: <20261102114135.GZ257209@horms.kernel.org> (raw)
In-Reply-To: <20261102085339.ixdqujaly75yldp7@DEN-DL-M70577.microsemi.net>

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@kernel.org>


  reply	other threads:[~2026-11-02 11:41 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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
2026-11-02  8:53   ` Daniel Machon
2026-11-02 11:41     ` Simon Horman [this message]
2026-11-02 12:10 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261102114135.GZ257209@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel.machon@microchip.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=hawk@kernel.org \
    --cc=horatiu.vultur@microchip.com \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev-bot+sashiko@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@fomichev.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.