Re: [PATCH net] veth: fix queue index used to wake the peer txq in veth_poll

Jesper Dangaard Brouer <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.bpf,org.kernel.vger.linux-kernel
Message-ID <[email protected]>

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 <[email protected]>
> Tested-by: Jesper Dangaard Brouer <[email protected]>

Acked-by: Jesper Dangaard Brouer <[email protected]>

> ---
>   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;
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.