The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <hawk@kernel.org>
To: "Jonas Köppeler" <j.koeppeler@tu-berlin.de>,
	"Andrew Lunn" <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Stanislav Fomichev" <sdf@fomichev.me>,
	"Toshiaki Makita" <toshiaki.makita1@gmail.com>,
	"Simon Schippers" <simon.schippers@tu-dortmund.de>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	bpf@vger.kernel.org, kernel-team <kernel-team@cloudflare.com>
Subject: Re: [PATCH net] veth: fix queue index used to wake the peer txq in veth_poll
Date: Thu, 6 Aug 2026 15:45:05 +0200	[thread overview]
Message-ID: <17f81247-ebe2-42ce-ac34-02f4779ccaef@kernel.org> (raw)
In-Reply-To: <20260806-veth-fix-poll-queue-idx-v1-1-c5357fb7573d@tu-berlin.de>



On 06/08/2026 13.43, Jonas Köppeler wrote:
> veth_poll() derives the index of the peer TX queue to wake from
> rq->xdp_rxq.queue_index. That field is only initialized by
> xdp_rxq_info_reg() in veth_enable_xdp_range(), which runs only when an
> XDP program is attached. On the plain GRO/NAPI path
> (veth_napi_enable_range()) xdp_rxq_info_reg() is never called, so
> queue_index stays 0 for every queue, as priv->rq is zero-allocated. The
> same happens once an XDP program is detached while GRO stays enabled,
> since xdp_rxq_info_unreg() clears the struct again.
> 
> So in a multi-queue setup with GRO enabled and no XDP program attached,
> every NAPI instance looks at the peer's TX queue 0. If veth_xmit() stops
> peer TX queue 1 because the ptr_ring is full (NETDEV_TX_BUSY), nothing
> ever wakes it again: the poller draining queue 1 wakes queue 0 instead.
> veth implements no ndo_tx_timeout, so the netdev watchdog does not kick
> in either, and the queue stays stopped indefinitely.
> 
> Derive the index from the position of the rq within priv->rq instead,
> which is correct regardless of whether XDP was ever enabled.
> 
> Scripts to reproduce the stall are available at
> https://github.com/netoptimizer/veth-backpressure-performance-testing

I've modified both the reproducer[12] and selftests[13] to detect this.
Previously reproducer only had a 50% chance to catch this, and we didn't 
notice, code have been changed to catch this 100% of the time.  The 
selftests script never hit the bug as it always used one queue, this 
have been changed to always use two queues, but force/steer traffic to 
always hit queue index 1 via XPS config (as we want to see the queue 
build up and ptr_ring HoL queuing effects in the latency measurements).


- [12] 
https://github.com/netoptimizer/veth-backpressure-performance-testing/pull/12
- [13] 
https://github.com/netoptimizer/veth-backpressure-performance-testing/pull/13

> Fixes: dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring to reduce TX drops")
> Signed-off-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
> Tested-by: Jesper Dangaard Brouer <hawk@kernel.org>

Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>

> ---
>   drivers/net/veth.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 00e34afd858e..e927ef3f47b3 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -961,7 +961,7 @@ static int veth_poll(struct napi_struct *napi, int budget)
>   	struct veth_rq *rq =
>   		container_of(napi, struct veth_rq, xdp_napi);
>   	struct veth_priv *priv = netdev_priv(rq->dev);
> -	int queue_idx = rq->xdp_rxq.queue_index;
> +	int queue_idx = rq - priv->rq;

It took me a while to convince myself that this is correct via checking
the data-structures and how they get allocated.

This is a stable fix so it is good to keep this change as small as possible.

I do feel like we should add a queue_idx to struct veth_rq, and init
this in function veth_alloc_queues(). This could be a followup to net-
next IMHO.


>   	struct netdev_queue *peer_txq;
>   	struct veth_stats stats = {};
>   	struct net_device *peer_dev;


      reply	other threads:[~2026-08-06 13:45 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-06 11:43 [PATCH net] veth: fix queue index used to wake the peer txq in veth_poll Jonas Köppeler
2026-08-06 13:45 ` Jesper Dangaard Brouer [this message]

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=17f81247-ebe2-42ce-ac34-02f4779ccaef@kernel.org \
    --to=hawk@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=j.koeppeler@tu-berlin.de \
    --cc=john.fastabend@gmail.com \
    --cc=kernel-team@cloudflare.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@fomichev.me \
    --cc=simon.schippers@tu-dortmund.de \
    --cc=toshiaki.makita1@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox