[PATCH net 0/2] veth: fix txqs that are never woken after backpressure

[PATCH net 0/2] veth: fix txqs that are never woken after backpressure

[Simon Schippers]

While debugging the veth BQL series [1], I found two ways in which a veth
txq that veth_xmit() stopped on a full ptr_ring is never woken again.

Patch 1 lets veth_poll() wake txqs beyond real_num_tx_queues, as a txq
stopped before a TX channel shrink is otherwise left stopped.

Patch 2 wakes the peer txq when NAPI is deleted, as veth_xmit() can stop
it after the last poll.

The reproducers are below the --- of each patch. Patch 1 reproduces
without changes to the kernel, patch 2 needs a debug-only udelay() to
widen the race window.

Claude Opus 5.5 was used heavily for this series, to find and reproduce
both bugs and to write the patches.

[1] https://lore.kernel.org/netdev/20260612083530.1650245-1-hawk@kernel.org/

Simon Schippers (2):
  veth: wake a stopped peer txq beyond real_num_tx_queues
  veth: wake the peer txq when deleting NAPI

 drivers/net/veth.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)


base-commit: ead6fa9a0ab194f02a2f36d634b193e9c5f9a1c6
-- 
2.43.0

[PATCH net 1/2] veth: wake a stopped peer txq beyond real_num_tx_queues

[Simon Schippers]

Since commit dc82a33297fc ("veth: apply qdisc backpressure on full
ptr_ring to reduce TX drops"), only the peer's veth_poll() wakes a txq
that veth_xmit() stopped on a full ptr_ring. veth_poll() skips txqs at
or above the peer's real_num_tx_queues.

If the peer shrinks its TX channels while such a txq is stopped, nothing
wakes it. netif_set_real_num_tx_queues() keeps its state, so once the
channels are grown back, its first skb is requeued forever. With a single
root qdisc this blocks all transmission of the device.

Look the txq up below num_tx_queues instead. Waking a txq that is not in
use is harmless.

Reproduced in virtme-ng with a UDP flood from veth0 into veth1, which has
GRO, while switching veth0 between one and two TX channels: without this
patch veth0 stopped transmitting within 11 cycles in 3 of 3 runs, with
it veth0 kept transmitting through 30 cycles in 3 of 3 runs.

Fixes: dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring to reduce TX drops")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---

Notes:
    Reproducer (needs >= 4 CPUs, no debug hook needed). Run as root:
    
      ./repro-veth-chan-shrink-txq-stall.sh 30
    
    repro-veth-chan-shrink-txq-stall.sh:
    
      #!/bin/bash
      # SPDX-License-Identifier: GPL-2.0
      #
      # Reproducer: a veth txq stopped on a full ptr_ring stays stopped after the
      # device shrinks its TX channels and grows them back.
      #
      # veth0 (init netns, 2 txqs, pfifo qdisc, UDP flood) -> veth1 (netns rx, GRO
      # on, threaded NAPI). The ring fills, veth_xmit() returns NETDEV_TX_BUSY and
      # stops a veth0 txq until veth1's veth_poll() wakes it. veth0 is switched to
      # one TX channel and back to two. If txq 1 is stopped at the shrink,
      # veth_poll() skips it as it is beyond real_num_tx_queues. Once it is back in
      # use, its first skb is requeued forever and blocks the qdisc.
      #
      # Needs at least 4 CPUs. No debug hook needed.
      #
      # Usage: repro-veth-chan-shrink-txq-stall.sh [cycles]
      #
      # RULES (default 1000) nft rules in netns rx slow the consumer down.
      #
      # Exits 1 if veth0 got stuck, 0 if it survived all cycles, 2 if the ring never
      # filled.
    
      ITER=${1:-30}
      RULES=${RULES:-1000}
    
      cleanup() {
      	kill $(jobs -p) 2>/dev/null
      	wait 2>/dev/null
      	ip link del veth0 2>/dev/null
      	ip netns del rx 2>/dev/null
      }
      trap cleanup EXIT
    
      sent() { tc -s qdisc show dev veth0 | awk '/Sent/ { print $4; exit }'; }
      requeues() { tc -s qdisc show dev veth0 | sed -n 's/.*requeues \([0-9]*\).*/\1/p' | head -1; }
      rx() { ip netns exec rx "$@"; }
    
      ip netns add rx
      ip link add veth0 numtxqueues 2 numrxqueues 2 type veth \
      	peer name veth1 numtxqueues 2 numrxqueues 2 netns rx
      ip addr add 10.0.0.1/24 dev veth0
      rx ip addr add 10.0.0.2/24 dev veth1
      ip link set veth0 up
      rx ip link set veth1 up
      tc qdisc replace dev veth0 root pfifo limit 1000
      # veth only takes skbs to the ring without TSO, unless the peer has UDP GRO
      ethtool -K veth0 tso off
      rx ethtool -K veth1 gro on
      rx sh -c 'echo 1 > /sys/class/net/veth1/threaded'
      for p in $(pgrep napi/veth1); do taskset -pc 3 "$p" >/dev/null; done
    
      # Slow down the consumer so that the 256-entry rings fill up
      rx nft add table inet slow
      rx nft add chain inet slow in '{ type filter hook input priority 0; }'
      for i in $(seq 1 "$RULES"); do
      	echo "add rule inet slow in meta mark $i drop"
      done | rx nft -f -
      ping -c 1 -W 1 10.0.0.2 >/dev/null
    
      # One socket per flooder, so that they hash onto both txqs. Unlike pktgen they
      # keep sending while set_channels turns the carrier off.
      for f in 1 2 3 4; do
      	taskset -c $(((f - 1) % 3)) python3 -c '
      import socket
      s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
      s.setblocking(False)
      while True:
          try:
              s.sendto(b"x" * 18, ("10.0.0.2", 9))
          except OSError:
              pass' &
      done
      sleep 2
    
      echo "kernel $(uname -r), cycles $ITER"
      echo "after 2s flood: sent $(sent) pkts, requeues $(requeues)"
      if [ "$(requeues)" = 0 ]; then
      	echo "ring never filled, raise RULES"
      	exit 2
      fi
    
      for i in $(seq 1 "$ITER"); do
      	ethtool -L veth0 tx 1
      	ethtool -L veth0 tx 2
      	# give linkwatch time to bring the qdisc back after the carrier toggle
      	sleep 2
      	a=$(sent); sleep 1; b=$(sent)
      	if [ $((b - a)) -lt 1000 ]; then
      		echo "STUCK after $i TX shrink/grow cycles: sent $a -> $b in 1s"
      		sleep 3
      		echo "3s later: sent $b -> $(sent), $(ip -br link show veth0)"
      		exit 1
      	fi
      done
    
      echo "OK: veth0 kept sending through $ITER TX shrink/grow cycles," \
           "sent $(sent) pkts, requeues $(requeues)"
      exit 0

 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 a61842b2585b..62a60a033675 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -984,7 +984,7 @@ static int veth_poll(struct napi_struct *napi, int budget)
 
 	/* NAPI functions as RCU section */
 	peer_dev = rcu_dereference_check(priv->peer, rcu_read_lock_bh_held());
-	peer_txq = (peer_dev && queue_idx < peer_dev->real_num_tx_queues) ?
+	peer_txq = (peer_dev && queue_idx < peer_dev->num_tx_queues) ?
 		   netdev_get_tx_queue(peer_dev, queue_idx) : NULL;
 
 	xdp_set_return_frame_no_direct();
-- 
2.43.0

[PATCH net 2/2] veth: wake the peer txq when deleting NAPI

[Simon Schippers]

Since commit dc82a33297fc ("veth: apply qdisc backpressure on full
ptr_ring to reduce TX drops"), only the peer's veth_poll() wakes a txq
that veth_xmit() stopped on a full ptr_ring.

If veth_xmit() stops the txq after the last poll before napi_disable(),
its __veth_xdp_flush() can no longer schedule NAPI. Nothing wakes the
txq, also not after NAPI is added back, so the device stops transmitting
for good. This can happen whenever the peer deletes its NAPI: when it
goes down, turns GRO off, detaches XDP or reduces its RX channels.

Wake the peer txq in veth_napi_del_range() once NAPI is deleted, also
when the peer is down, as opening it does not restart its queues.

Reproduced in virtme-ng with a pktgen flood while toggling GRO on the
peer, with a udelay() before netif_tx_stop_queue() to widen the race:
the txq got stuck within four toggles without this patch, and not in 300
with it. Without the udelay() it was not hit in 1200 toggles.

Fixes: dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring to reduce TX drops")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---

Notes:
    Reproducer (needs >= 3 CPUs). The race only triggers reliably with the
    debug-only hook below, which widens the window before
    netif_tx_stop_queue(). Run as root:
    
      # stuck within a few toggles without this patch, never with it
      ./repro-veth-napi-del-txq-stall.sh 300 2000
    
    Debug hook (not for merging, indented so git am skips it):
    
      diff --git a/drivers/net/veth.c b/drivers/net/veth.c
      --- a/drivers/net/veth.c
      +++ b/drivers/net/veth.c
      @@ -29,6 +29,11 @@
       #include <linux/net_tstamp.h>
       #include <linux/skbuff_ref.h>
       #include <net/page_pool/helpers.h>
      +#include <linux/delay.h>
      +
      +/* REPRO ONLY, never to be committed */
      +static unsigned int repro_delay_us;
      +module_param(repro_delay_us, uint, 0644);
    
       #define DRV_NAME	"veth"
       #define DRV_VERSION	"1.0"
      @@ -402,6 +407,8 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
       		__skb_push(skb, ETH_HLEN);
       		/* qdisc requeues the skb on the txq it points to */
       		skb_set_queue_mapping(skb, rxq);
      +		if (READ_ONCE(repro_delay_us))
      +			udelay(READ_ONCE(repro_delay_us)); /* REPRO: widen window */
       		netif_tx_stop_queue(txq);
       		/* Makes sure NAPI peer consumer runs. Consumer is responsible
       		 * for starting txq again, until then ndo_start_xmit (this
    
    repro-veth-napi-del-txq-stall.sh:
    
      #!/bin/bash
      # SPDX-License-Identifier: GPL-2.0
      #
      # Reproducer: a veth txq stopped on a full ptr_ring stays stopped after the
      # peer's NAPI is deleted.
      #
      # veth0 (init netns, pfifo qdisc, pktgen queue_xmit flood) -> veth1 (netns rx,
      # GRO on, threaded NAPI). Needs at least 3 CPUs. The ring fills, veth_xmit()
      # returns NETDEV_TX_BUSY and stops veth0's txq until veth_poll() wakes it. GRO
      # on veth1 is toggled off and on, which deletes and re-adds its NAPI. If the
      # txq is stopped after the last poll's wake check, nothing wakes it and veth0
      # stops sending for good.
      #
      # Usage: repro-veth-napi-del-txq-stall.sh [iterations] [delay_us]
      #
      # delay_us > 0 writes veth.repro_delay_us, a debug-only hook (not part of any
      # patch) that udelay()s between the ring-full check and netif_tx_stop_queue()
      # to widen the race window.
      #
      # RULES (default 50) nft rules in netns rx slow the consumer down.
      #
      # Exits 1 if veth0 got stuck, 0 if it survived all iterations, 2 if the ring
      # never filled.
    
      ITER=${1:-300}
      DELAY=${2:-0}
      RULES=${RULES:-50}
      PG=/proc/net/pktgen
    
      cleanup() {
      	echo stop > $PG/pgctrl 2>/dev/null
      	wait 2>/dev/null
      	ip link del veth0 2>/dev/null
      	ip netns del rx 2>/dev/null
      }
      trap cleanup EXIT
    
      sent() { tc -s qdisc show dev veth0 | awk '/Sent/ { print $4; exit }'; }
      requeues() { tc -s qdisc show dev veth0 | sed -n 's/.*requeues \([0-9]*\).*/\1/p' | head -1; }
      rx() { ip netns exec rx "$@"; }
    
      # Threaded NAPI pinned away from kpktgend_0 (CPU 0), so the consumer can poll
      # while the producer is between the ring-full check and the txq stop
      gro_on() {
      	rx ethtool -K veth1 gro on
      	rx sh -c 'echo 1 > /sys/class/net/veth1/threaded'
      	taskset -pc 2 "$(pgrep napi/veth1)" >/dev/null
      }
    
      ip netns add rx
      ip link add veth0 type veth peer name veth1 netns rx
      ip addr add 10.0.0.1/24 dev veth0
      rx ip addr add 10.0.0.2/24 dev veth1
      ip link set veth0 up
      rx ip link set veth1 up
      tc qdisc replace dev veth0 root pfifo limit 1000
      # pktgen skbs have no socket, so veth only takes them to the ring without TSO
      ethtool -K veth0 tso off
      gro_on
    
      # Slow down the consumer so that the 256-entry ring fills up
      rx nft add table inet slow
      rx nft add chain inet slow in '{ type filter hook input priority 0; }'
      for i in $(seq 1 "$RULES"); do echo "add rule inet slow in meta mark $i drop"; done | rx nft -f -
    
      [ "$DELAY" -gt 0 ] && echo "$DELAY" > /sys/module/veth/parameters/repro_delay_us
    
      echo "rem_device_all" > $PG/kpktgend_0
      echo "add_device veth0" > $PG/kpktgend_0
      for c in "count 0" "pkt_size 60" "delay 0" "clone_skb 0" \
      	 "xmit_mode queue_xmit" "dst 10.0.0.2" "udp_dst_min 9" "udp_dst_max 9" \
      	 "dst_mac $(rx cat /sys/class/net/veth1/address)"; do
      	echo "$c" > $PG/veth0
      done
      echo start > $PG/pgctrl &
      sleep 1
    
      echo "kernel $(uname -r), iterations $ITER, repro_delay_us $DELAY"
      echo "after 1s flood: sent $(sent) pkts, requeues $(requeues)"
      if [ "$(requeues)" = 0 ]; then
      	echo "ring never filled, raise RULES"
      	exit 2
      fi
    
      for i in $(seq 1 "$ITER"); do
      	rx ethtool -K veth1 gro off
      	a=$(sent); sleep 0.3; b=$(sent)
      	if [ "$a" = "$b" ]; then
      		echo "STUCK after $i GRO toggles: veth0 qdisc sent nothing in 0.3s"
      		gro_on
      		sleep 1
      		echo "still stuck with GRO back on: sent $a -> $(sent)"
      		echo stop > $PG/pgctrl; wait
      		sleep 0.5
      		ping -c 3 -W 1 10.0.0.2 >/dev/null 2>&1 &&
      			echo "ping: ok" || echo "ping 10.0.0.2: fails"
      		tc -s qdisc show dev veth0
      		exit 1
      	fi
      	gro_on
      	sleep 0.05
      done
    
      echo "OK: veth0 kept sending through $ITER GRO toggles," \
           "sent $(sent) pkts, requeues $(requeues)"
      exit 0

 drivers/net/veth.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 62a60a033675..faa837acea7b 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -1087,6 +1087,7 @@ static int __veth_napi_enable(struct net_device *dev)
 static void veth_napi_del_range(struct net_device *dev, int start, int end)
 {
 	struct veth_priv *priv = netdev_priv(dev);
+	struct net_device *peer;
 	int i;
 
 	for (i = start; i < end; i++) {
@@ -1099,11 +1100,17 @@ static void veth_napi_del_range(struct net_device *dev, int start, int end)
 	}
 	synchronize_net();
 
+	peer = rtnl_dereference(priv->peer);
+
 	for (i = start; i < end; i++) {
 		struct veth_rq *rq = &priv->rq[i];
 
 		rq->rx_notify_masked = false;
 		ptr_ring_cleanup(&rq->xdp_ring, veth_ptr_free);
+
+		/* veth_poll() can no longer wake a txq stopped on a full ring */
+		if (peer && i < peer->num_tx_queues)
+			netif_wake_subqueue(peer, i);
 	}
 
 	for (i = start; i < end; i++) {
-- 
2.43.0

Re: [PATCH net 1/2] veth: wake a stopped peer txq beyond real_num_tx_queues

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] veth: Fixes trailer cites dc82a33297fc instead of 08f566e8f83b
--

commit 4511ff49371a3a9f93211f5b51b7a6404ecb1f22
Author: Simon Schippers <simon.schippers@tu-dortmund.de>

> veth: wake a stopped peer txq beyond real_num_tx_queues
>
> Since commit dc82a33297fc ("veth: apply qdisc backpressure on full
> ptr_ring to reduce TX drops"), only the peer's veth_poll() wakes a txq
> that veth_xmit() stopped on a full ptr_ring. veth_poll() skips txqs at
> or above the peer's real_num_tx_queues.
>
> If the peer shrinks its TX channels while such a txq is stopped, nothing
> wakes it. netif_set_real_num_tx_queues() keeps its state, so once the
> channels are grown back, its first skb is requeued forever. With a single
> root qdisc this blocks all transmission of the device.
>
> Look the txq up below num_tx_queues instead. Waking a txq that is not in
> use is harmless.
>
> Reproduced in virtme-ng with a UDP flood from veth0 into veth1, which has
> GRO, while switching veth0 between one and two TX channels: without this
> patch veth0 stopped transmitting within 11 cycles in 3 of 3 runs, with
> it veth0 kept transmitting through 30 cycles in 3 of 3 runs.
>
> Fixes: dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring to reduce TX drops")
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>

[Severity: Low]
Should the Fixes: tag reference 08f566e8f83b ("veth: fix OOB txq access
in veth_poll() with asymmetric queue counts") instead?

Commit dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring to
reduce TX drops") looked up peer_txq unconditionally via
netdev_get_tx_queue(peer_dev, queue_idx).  The queue_idx <
peer_dev->real_num_tx_queues check that skips txqs above
real_num_tx_queues in veth_poll() was introduced in 08f566e8f83b ("veth:
fix OOB txq access in veth_poll() with asymmetric queue counts").

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113104648.225983-1-simon.schippers@tu-dortmund.de?part=1