* Re: [PATCH RFC net-next v4 06/14] xsk: support dynamic xmit.more control for batch xmit
[not found] ` <20260415093539.4A4BFC19424@smtp.kernel.org>
@ 2026-04-16 3:43 ` Jason Xing
2026-04-16 4:50 ` Dmitry Torokhov
0 siblings, 1 reply; 3+ messages in thread
From: Jason Xing @ 2026-04-16 3:43 UTC (permalink / raw)
To: sashiko; +Cc: bpf
On Wed, Apr 15, 2026 at 5:35 PM <sashiko-bot@kernel.org> wrote:
>
> Sashiko AI review found 1 potential issue(s):
> - [High] Stalled TX queue due to missing hardware doorbell ring on early loop exit
> --
>
> commit 6d7c8f2ad056eb4cb2eec8d360e632f9ef9044f5
> Author: Jason Xing <kernelxing@tencent.com>
>
> xsk: support dynamic xmit.more control for batch xmit
>
> This commit updates xsk batch transmission to only set xmit.more to false for
> the last skb in the send queue, aiming to reduce unnecessary IRQs and improve
> batching performance.
>
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4873,6 +4873,7 @@ int xsk_direct_xmit_batch(struct xdp_sock *xs, struct net_device *dev)
> > struct sk_buff_head *send_queue = &xs->batch.send_queue;
> > int ret = NETDEV_TX_BUSY;
> > struct sk_buff *skb;
> > + bool more = true;
> >
> > local_bh_disable();
> > HARD_TX_LOCK(dev, txq, smp_processor_id());
>
> [ ... ]
>
> > @@ -4892,8 +4893,12 @@ int xsk_direct_xmit_batch(struct xdp_sock *xs, struct net_device *dev)
> > __skb_queue_head(send_queue, skb);
> > break;
> > }
> > +
> > + if (!skb_peek(send_queue))
> > + more = false;
> > +
> > skb_set_queue_mapping(skb, queue_id);
> > - ret = netdev_start_xmit(skb, dev, txq, false);
> > + ret = netdev_start_xmit(skb, dev, txq, more);
> > if (ret != NETDEV_TX_OK) {
> > if (ret == NETDEV_TX_BUSY)
> > __skb_queue_head(send_queue, skb);
> > break;
> > }
> > }
>
> If an error occurs and the loop breaks early, does this leave previously
> transmitted packets stalled in the driver's TX ring?
>
> Since more is passed as true for all packets prior to the failure, the
> network driver defers ringing the hardware TX doorbell.
>
> If the code breaks out of the loop due to validate_xmit_skb_list() failing,
> netif_xmit_frozen_or_drv_stopped(), or netdev_start_xmit() returning
> NETDEV_TX_BUSY, the loop terminates without ever passing more = false to the
> driver.
>
> Could this omission prevent the hardware from being notified to transmit
> the successfully processed packets that preceded the error?
The quite similar code snippet was added in dev_hard_start_xmit()
introduced by the following commit in 2014:
commit 95f6b3dda2a4a052f7dabe9998e4ffac491b7bc2
Author: David S. Miller <davem@davemloft.net>
Date: Fri Aug 29 21:57:30 2014 -0700
net: Have xmit_list() signal more==true when appropriate.
Signed-off-by: David S. Miller <davem@davemloft.net>
Look at the doorbell of drivers, like ixgbe:
if (netif_xmit_stopped(txring_txq(tx_ring)) || !netdev_xmit_more()) {
writel(i, tx_ring->tail);
}
It has already taken care of it.
If we really want to cover all the possible drivers in the above two
cases, maybe we need a separate patchset to add a common interface of
devices to actively kick the hardware. But now I choose to defer it
unless someone insists on this.
Thanks,
Jason
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260415082654.21026-1-kerneljasonxing@gmail.com?part=6
^ permalink raw reply [flat|nested] 3+ messages in thread