Skip to contentSkip to conversation

veth: add Byte Queue Limits (BQL) support

PATCH Superseded [email protected] 2 participants 7 messages May 1 – May 5

Subsystems (5)

v4 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v4 cover veth: add Byte Queue Limits (BQL) support

From: Jesper Dangaard Brouer <[email protected]>

This series adds BQL (Byte Queue Limits) to the veth driver, reducing latency by dynamically limiting in-flight packets in the ptr_ring and moving buffering into the qdisc where AQM algorithms can act on it.

Problem:
veth's 256-entry ptr_ring acts as a "dark buffer" -- packets queued there are invisible to the qdisc's AQM. Under load, the ring fills completely (DRV_XOFF backpressure), adding up to 256 packets of unmanaged latency before the qdisc even sees congestion.

Solution:
BQL (STACK_XOFF) dynamically limits in-flight packets, stopping the queue before the ring fills. This keeps the ring shallow and pushes excess packets into the qdisc, where sojourn-based AQM can measure and drop them.

Test setup: veth pair, UDP flood, 13000 iptables rules in consumer namespace (slows NAPI-64 cycle to ~6-7ms), ping measures RTT under load.

BQL off BQL on

  fq_codel:  RTT ~22ms, 4% loss         RTT ~1.3ms, 0% loss
  sfq:       RTT ~24ms, 0% loss         RTT ~1.5ms, 0% loss

BQL reduces ping RTT by ~17x for both qdiscs. Consumer throughput is unchanged (~10K pps) -- BQL adds no overhead.

CoDel bug discovered during BQL development:
Our original motivation for BQL was fq_codel ping loss observed under load (4-26% depending on NAPI cycle time). Investigating this led us to discover a bug in the CoDel implementation: codel_dequeue() does not reset vars->first_above_time when a flow goes empty, contrary to the reference algorithm. This causes stale CoDel state to persist across empty periods in fq_codel's per-flow queues, penalizing sparse flows like ICMP ping. A fix for this has been applied to the net tree 815980fe6dbb ("net_sched: codel: fix stale state for empty flows in fq_codel")

BQL remains valuable independently: it reduces RTT by ~17x by moving buffering from the dark ptr_ring into the qdisc. Additionally, BQL clears STACK_XOFF per-SKB as each packet completes, rather than batch-waking after 64 packets (DRV_XOFF). This keeps sojourn times below fq_codel's target, preventing CoDel from entering dropping state on non-congested flows in the first place.

Key design decisions:
- Charge-under-lock in veth_xdp_rx(): The BQL charge must precede the ptr_ring produce, because the NAPI consumer can run on another CPU and complete the SKB immediately after it becomes visible. To avoid a pre-charge/undo pattern, the charge is done under the ptr_ring producer_lock after confirming the ring is not full. BQL is only charged when produce is guaranteed to succeed, keeping num_queued monotonically increasing. HARD_TX_LOCK already serializes dql_queued() (veth requires a qdisc for BQL); the ptr_ring lock additionally would allow noqueue to work correctly.

- Per-SKB BQL tracking via pointer tag: A VETH_BQL_FLAG bit in the ptr_ring pointer records whether each SKB was BQL-charged. This is necessary because the qdisc can be replaced live (noqueue->sfq or vice versa) while SKBs are in-flight -- the completion side must know the charge state that was decided at enqueue time.

- IFF_NO_QUEUE + BQL coexistence: A new dev->bql flag enables BQL sysfs exposure for IFF_NO_QUEUE devices that opt in to DQL accounting, without changing IFF_NO_QUEUE semantics.

Background and acknowledgments:
Mike Freemon reported the veth dark buffer problem internally at Cloudflare and showed that recompiling the kernel with a ptr_ring size of 30 (down from 256) made fq_codel work dramatically better. This was the primary motivation for a proper BQL solution that achieves the same effect dynamically without a kernel rebuild.

Chris Arges wrote a reproducer for the dark buffer latency problem:
https://github.com/netoptimizer/veth-backpressure-performance-testing
This is where we first observed ping packets being dropped under fq_codel, which became our secondary motivation for BQL. In production we switched to SFQ on veth devices as a workaround.

Jonas Koeppeler provided extensive testing and code review. Together we discovered that the fq_codel ping loss was actually a 12-year-old CoDel bug (stale first_above_time in empty flows), not caused by the dark buffer itself.

Patch overview:
1. net: add dev->bql flag to allow BQL sysfs for IFF_NO_QUEUE devices
2. veth: implement Byte Queue Limits (BQL) for latency reduction
3. veth: add tx_timeout watchdog as BQL safety net
4. net: sched: add timeout count to NETDEV WATCHDOG message

Jesper Dangaard Brouer (4):
net: add dev->bql flag to allow BQL sysfs for IFF_NO_QUEUE devices
veth: implement Byte Queue Limits (BQL) for latency reduction
veth: add tx_timeout watchdog as BQL safety net
net: sched: add timeout count to NETDEV WATCHDOG message

 .../networking/net_cachelines/net_device.rst  |  1 +
 drivers/net/veth.c                            | 94 ++++++++++++++++---
 include/linux/netdevice.h                     |  2 +
 net/core/net-sysfs.c                          |  8 +-
 net/sched/sch_generic.c                       |  8 +-
 5 files changed, 97 insertions(+), 16 deletions(-)

V3: https://lore.kernel.org/all/[email protected]/

Changes since V3:
- Drop selftest patch (patch 5 from V3) per maintainer request.
- Rebase on latest net-next.

V2: https://lore.kernel.org/all/[email protected]/

Changes since V2:
- Patch 2 (veth BQL): fix syzbot WARNING in veth_napi_del_range():
clamp BQL reset loop to peer's real_num_tx_queues. The loop was iterating dev->real_num_rx_queues but indexing peer's txq[], which goes out of bounds when the peer has fewer TX queues (e.g. veth enslaved to a bond with XDP attached).

V1: https://lore.kernel.org/all/[email protected]/

Changes since V1:
- Patch 1 (dev->bql flag): add kdoc entry for @bql in struct net_device.
- Patch 2 (veth BQL): charge fixed VETH_BQL_UNIT (1) per packet instead of skb->len. veth has no link speed; the ptr_ring is packet-indexed. Byte-based charging lets small packets sneak many entries into the ring.
Testing: min-size packet flood causes 3.7x ping RTT degradation with skb->len vs no change with fixed-unit charging.
- Patch 3 (tx_timeout watchdog): fix race with peer NAPI: replace netdev_tx_reset_queue() with clear_bit(STACK_XOFF) + netif_tx_wake_queue() to avoid dql_reset() racing with concurrent dql_completed().
- Cover letter: update CoDel fix reference to merged commit in net tree.

Cc "David S. Miller" <[email protected]>
Cc Eric Dumazet <[email protected]>
Cc Jakub Kicinski <[email protected]>
Cc Paolo Abeni <[email protected]>
Cc Simon Horman <[email protected]>
Cc Chris Arges <[email protected]>
Cc Mike Freemon <[email protected]>
Cc Toke Høiland-Jørgensen <[email protected]>
Cc Jonas Köppeler <[email protected]>
Cc Breno Leitao <[email protected]>
Signature · 1 line
2.43.0
#1 lore raw

PATCH v4 1/4 net: add dev->bql flag to allow BQL sysfs for IFF_NO_QUEUE devices

From: Jesper Dangaard Brouer <[email protected]>

Virtual devices with IFF_NO_QUEUE or lltx are excluded from BQL sysfs by netdev_uses_bql(), since they traditionally lack real hardware queues. However, some virtual devices like veth implement a real ptr_ring FIFO with NAPI processing and benefit from BQL to limit in-flight bytes and reduce latency.

Add a per-device 'bql' bitfield boolean in the priv_flags_slow section of struct net_device. When set, it overrides the IFF_NO_QUEUE/lltx exclusion and exposes BQL sysfs entries (/sys/class/net/<dev>/queues/ tx-<n>/byte_queue_limits/). The flag is still gated on CONFIG_BQL.

This allows drivers that use BQL despite being IFF_NO_QUEUE to opt in to sysfs visibility for monitoring and debugging.

---
 Documentation/networking/net_cachelines/net_device.rst | 1 +
 include/linux/netdevice.h                              | 2 ++
 net/core/net-sysfs.c                                   | 8 +++++++-
 3 files changed, 10 insertions(+), 1 deletion(-)
Documentation/networking/net_cachelines/net_device.rst +1 -0
index 1c19bb7705df..b775d3235a2d 100644
@@ -170,6 +170,7 @@ unsigned_long:1                     see_all_hwtstamp_requests
 unsigned_long:1                     change_proto_down
 unsigned_long:1                     netns_immutable
 unsigned_long:1                     fcoe_mtu
+unsigned_long:1                     bql                                                                 netdev_uses_bql(net-sysfs.c)
 struct list_head                    net_notifier_list
 struct macsec_ops*                  macsec_ops
 struct udp_tunnel_nic_info*         udp_tunnel_nic_info
include/linux/netdevice.h +2 -0
index 0e1e581efc5a..405bdf9172ca 100644
@@ -2065,6 +2065,7 @@ enum netdev_reg_state {
  *	@change_proto_down: device supports setting carrier via IFLA_PROTO_DOWN
  *	@netns_immutable: interface can't change network namespaces
  *	@fcoe_mtu:	device supports maximum FCoE MTU, 2158 bytes
+ *	@bql:		device uses BQL (DQL sysfs) despite having IFF_NO_QUEUE
  *
  *	@net_notifier_list:	List of per-net netdev notifier block
  *				that follow this device when it is moved
@@ -2479,6 +2480,7 @@ struct net_device {
 	unsigned long		change_proto_down:1;
 	unsigned long		netns_immutable:1;
 	unsigned long		fcoe_mtu:1;
+	unsigned long		bql:1;
 
 	struct list_head	net_notifier_list;
 
net/core/net-sysfs.c +7 -1
index 3318b5666e43..82833e5dae03 100644
@@ -1945,10 +1945,16 @@ static const struct kobj_type netdev_queue_ktype = {
 
 static bool netdev_uses_bql(const struct net_device *dev)
 {
+	if (!IS_ENABLED(CONFIG_BQL))
+		return false;
+
+	if (dev->bql)
+		return true;
+
 	if (dev->lltx || (dev->priv_flags & IFF_NO_QUEUE))
 		return false;
 
-	return IS_ENABLED(CONFIG_BQL);
+	return true;
 }
 
 static int netdev_queue_add_kobject(struct net_device *dev, int index)
Signature · 1 line
2.43.0
#2 lore raw

PATCH v4 2/4 veth: implement Byte Queue Limits (BQL) for latency reduction

In reply to [email protected]

From: Jesper Dangaard Brouer <[email protected]>

Commit dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring to reduce TX drops") gave qdiscs control over veth by returning NETDEV_TX_BUSY when the ptr_ring is full (DRV_XOFF). That commit noted a known limitation: the 256-entry ptr_ring sits in front of the qdisc as a dark buffer, adding base latency because the qdisc has no visibility into how many bytes are already queued there.

Add BQL support so the qdisc gets feedback and can begin shaping traffic before the ring fills. In testing with fq_codel, BQL reduces ping RTT under UDP load from ~6.61ms to ~0.36ms (18x).

Charge a fixed VETH_BQL_UNIT (1) per packet rather than skb->len, so the DQL limit tracks packets-in-flight. Unlike a physical NIC, veth has no link speed -- the ptr_ring drains at CPU speed and is packet-indexed, not byte-indexed, so bytes are not the natural unit. With byte-based charging, small packets sneak many more entries into the ring before STACK_XOFF fires, deepening the dark buffer under mixed-size workloads. Testing with a concurrent min-size packet flood shows 3.7x ping RTT degradation with skb->len charging versus no change with fixed-unit charging.

Charge BQL inside veth_xdp_rx() under the ptr_ring producer_lock, after confirming the ring is not full. The charge must precede the produce because the NAPI consumer can run on another CPU and complete the SKB the instant it becomes visible in the ring. Doing both under the same lock avoids a pre-charge/undo pattern -- BQL is only charged when produce is guaranteed to succeed.

BQL is enabled only when a real qdisc is attached (guarded by !qdisc_txq_has_no_queue), as HARD_TX_LOCK provides serialization for TXQ modification like dql_queued(). For lltx devices, like veth, this HARD_TX_LOCK serialization isn't provided. The ptr_ring producer_lock provides additional serialization that would allow BQL to work correctly even with noqueue, though that combination is not currently enabled, as the netstack will drop and warn.

Track per-SKB BQL state via a VETH_BQL_FLAG pointer tag in the ptr_ring entry. This is necessary because the qdisc can be replaced live while SKBs are in-flight -- each SKB must carry the charge decision made at enqueue time rather than re-checking the peer's qdisc at completion.

Complete per-SKB in veth_xdp_rcv() rather than in bulk, so STACK_XOFF clears promptly when producer and consumer run on different CPUs.

BQL introduces a second independent queue-stop mechanism (STACK_XOFF) alongside the existing DRV_XOFF (ring full). Both must be clear for the queue to transmit. Reset BQL state in veth_napi_del_range() after synchronize_net() to avoid racing with in-flight veth_poll() calls. Clamp the reset loop to the peer's real_num_tx_queues, since the peer may have fewer TX queues than the local device has RX queues (e.g. when veth is enslaved to a bond with XDP attached).

---
 drivers/net/veth.c | 76 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 65 insertions(+), 11 deletions(-)
drivers/net/veth.c +65 -11
index e35df717e65e..3de25ba34a90 100644
@@ -34,9 +34,13 @@
 #define DRV_VERSION	"1.0"
 
 #define VETH_XDP_FLAG		BIT(0)
+#define VETH_BQL_FLAG		BIT(1)
 #define VETH_RING_SIZE		256
 #define VETH_XDP_HEADROOM	(XDP_PACKET_HEADROOM + NET_IP_ALIGN)
 
+/* Fixed BQL charge: DQL limit tracks packets-in-flight, not bytes */
+#define VETH_BQL_UNIT		1
+
 #define VETH_XDP_TX_BULK_SIZE	16
 #define VETH_XDP_BATCH		16
 
@@ -280,6 +284,21 @@ static bool veth_is_xdp_frame(void *ptr)
 	return (unsigned long)ptr & VETH_XDP_FLAG;
 }
 
+static bool veth_ptr_is_bql(void *ptr)
+{
+	return (unsigned long)ptr & VETH_BQL_FLAG;
+}
+
+static struct sk_buff *veth_ptr_to_skb(void *ptr)
+{
+	return (void *)((unsigned long)ptr & ~VETH_BQL_FLAG);
+}
+
+static void *veth_skb_to_ptr(struct sk_buff *skb, bool bql)
+{
+	return bql ? (void *)((unsigned long)skb | VETH_BQL_FLAG) : skb;
+}
+
 static struct xdp_frame *veth_ptr_to_xdp(void *ptr)
 {
 	return (void *)((unsigned long)ptr & ~VETH_XDP_FLAG);
@@ -295,7 +314,7 @@ static void veth_ptr_free(void *ptr)
 	if (veth_is_xdp_frame(ptr))
 		xdp_return_frame(veth_ptr_to_xdp(ptr));
 	else
-		kfree_skb(ptr);
+		kfree_skb(veth_ptr_to_skb(ptr));
 }
 
 static void __veth_xdp_flush(struct veth_rq *rq)
@@ -309,19 +328,33 @@ static void __veth_xdp_flush(struct veth_rq *rq)
 	}
 }
 
-static int veth_xdp_rx(struct veth_rq *rq, struct sk_buff *skb)
+static int veth_xdp_rx(struct veth_rq *rq, struct sk_buff *skb, bool do_bql,
+		       struct netdev_queue *txq)
 {
-	if (unlikely(ptr_ring_produce(&rq->xdp_ring, skb)))
+	struct ptr_ring *ring = &rq->xdp_ring;
+
+	spin_lock(&ring->producer_lock);
+	if (unlikely(!ring->size) || __ptr_ring_full(ring)) {
+		spin_unlock(&ring->producer_lock);
 		return NETDEV_TX_BUSY; /* signal qdisc layer */
+	}
+
+	/* BQL charge before produce; consumer cannot see entry yet */
+	if (do_bql)
+		netdev_tx_sent_queue(txq, VETH_BQL_UNIT);
+
+	__ptr_ring_produce(ring, veth_skb_to_ptr(skb, do_bql));
+	spin_unlock(&ring->producer_lock);
 
 	return NET_RX_SUCCESS; /* same as NETDEV_TX_OK */
 }
 
 static int veth_forward_skb(struct net_device *dev, struct sk_buff *skb,
-			    struct veth_rq *rq, bool xdp)
+			    struct veth_rq *rq, bool xdp, bool do_bql,
+			    struct netdev_queue *txq)
 {
 	return __dev_forward_skb(dev, skb) ?: xdp ?
-		veth_xdp_rx(rq, skb) :
+		veth_xdp_rx(rq, skb, do_bql, txq) :
 		__netif_rx(skb);
 }
 
@@ -352,6 +385,7 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct net_device *rcv;
 	int length = skb->len;
 	bool use_napi = false;
+	bool do_bql = false;
 	int ret, rxq;
 
 	rcu_read_lock();
@@ -375,8 +409,11 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	skb_tx_timestamp(skb);
+	txq = netdev_get_tx_queue(dev, rxq);
 
-	ret = veth_forward_skb(rcv, skb, rq, use_napi);
+	/* BQL charge happens inside veth_xdp_rx() under producer_lock */
+	do_bql = use_napi && !qdisc_txq_has_no_queue(txq);
+	ret = veth_forward_skb(rcv, skb, rq, use_napi, do_bql, txq);
 	switch (ret) {
 	case NET_RX_SUCCESS: /* same as NETDEV_TX_OK */
 		if (!use_napi)
@@ -388,8 +425,6 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 		/* If a qdisc is attached to our virtual device, returning
 		 * NETDEV_TX_BUSY is allowed.
 		 */
-		txq = netdev_get_tx_queue(dev, rxq);
-
 		if (qdisc_txq_has_no_queue(txq)) {
 			dev_kfree_skb_any(skb);
 			goto drop;
@@ -412,6 +447,7 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 		net_crit_ratelimited("%s(%s): Invalid return code(%d)",
 				     __func__, dev->name, ret);
 	}
+
 	rcu_read_unlock();
 
 	return ret;
@@ -900,7 +936,8 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
 
 static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 			struct veth_xdp_tx_bq *bq,
-			struct veth_stats *stats)
+			struct veth_stats *stats,
+			struct netdev_queue *peer_txq)
 {
 	int i, done = 0, n_xdpf = 0;
 	void *xdpf[VETH_XDP_BATCH];
@@ -928,9 +965,13 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 			}
 		} else {
 			/* ndo_start_xmit */
-			struct sk_buff *skb = ptr;
+			bool bql_charged = veth_ptr_is_bql(ptr);
+			struct sk_buff *skb = veth_ptr_to_skb(ptr);
 
 			stats->xdp_bytes += skb->len;
+			if (peer_txq && bql_charged)
+				netdev_tx_completed_queue(peer_txq, 1, VETH_BQL_UNIT);
+
 			skb = veth_xdp_rcv_skb(rq, skb, bq, stats);
 			if (skb) {
 				if (skb_shared(skb) || skb_unclone(skb, GFP_ATOMIC))
@@ -975,7 +1016,7 @@ static int veth_poll(struct napi_struct *napi, int budget)
 	peer_txq = peer_dev ? netdev_get_tx_queue(peer_dev, queue_idx) : NULL;
 
 	xdp_set_return_frame_no_direct();
-	done = veth_xdp_rcv(rq, budget, &bq, &stats);
+	done = veth_xdp_rcv(rq, budget, &bq, &stats, peer_txq);
 
 	if (stats.xdp_redirect > 0)
 		xdp_do_flush();
@@ -1073,6 +1114,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++) {
@@ -1091,6 +1133,17 @@ static void veth_napi_del_range(struct net_device *dev, int start, int end)
 		ptr_ring_cleanup(&rq->xdp_ring, veth_ptr_free);
 	}
 
+	/* Reset BQL on peer's txqs: remaining ring items were freed above
+	 * without BQL completion, so DQL state must be reset.
+	 */
+	peer = rtnl_dereference(priv->peer);
+	if (peer) {
+		int peer_end = min(end, (int)peer->real_num_tx_queues);
+
+		for (i = start; i < peer_end; i++)
+			netdev_tx_reset_queue(netdev_get_tx_queue(peer, i));
+	}
+
 	for (i = start; i < end; i++) {
 		page_pool_destroy(priv->rq[i].page_pool);
 		priv->rq[i].page_pool = NULL;
@@ -1740,6 +1793,7 @@ static void veth_setup(struct net_device *dev)
 	dev->priv_flags |= IFF_PHONY_HEADROOM;
 	dev->priv_flags |= IFF_DISABLE_NETPOLL;
 	dev->lltx = true;
+	dev->bql = true;
 
 	dev->netdev_ops = &veth_netdev_ops;
 	dev->xdp_metadata_ops = &veth_xdp_metadata_ops;
Signature · 1 line
2.43.0
#3 lore raw

Re: [PATCH net-next v4 2/4] veth: implement Byte Queue Limits (BQL) for latency reduction

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] Out-of-bounds array access on peer_dev->_tx in veth_poll() when local RX queues exceed peer TX…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Out-of-bounds array access on peer_dev->_tx in veth_poll() when local RX queues exceed peer TX queues.
- [High] Disabling XDP permanently stops peer TX queues if they were backpressured (DRV_XOFF leak). --

commit 9bc3ed022846dda6dd5cc131e798f949f63f491f
Author: Jesper Dangaard Brouer <[email protected]>

veth: implement Byte Queue Limits (BQL) for latency reduction

This commit adds byte queue limits support to the veth driver to provide qdiscs with feedback and reduce latency. It charges a fixed unit per packet rather than byte-based charging.

Quoted 4 lines
drivers/net/veth.c +0 -0
index e35df717e65e2..3de25ba34a906 100644

[ ... ]

drivers/net/veth.c static int veth_poll(struct napi_struct *napi, int budget) · line 1019

Quoted 6 lines
drivers/net/veth.c +1 -1
@@ -975,7 +1016,7 @@ static int veth_poll(struct napi_struct *napi, int budget)
 	peer_txq = peer_dev ? netdev_get_tx_queue(peer_dev, queue_idx) : NULL;
 
 	xdp_set_return_frame_no_direct();
-	done = veth_xdp_rcv(rq, budget, &bq, &stats);
+	done = veth_xdp_rcv(rq, budget, &bq, &stats, peer_txq);

Could this lead to an out-of-bounds array access on the peer device's transmit queues?

If a veth pair is configured asymmetrically, the local device might have more receive queues than the peer has transmit queues.

If packets are redirected from another interface into a higher-indexed receive queue, the poll function will run with a queue_idx greater than or equal to peer_dev->real_num_tx_queues.

Since netdev_get_tx_queue() performs a direct array access without bounds checking, peer_txq would point out-of-bounds. This out-of-bounds pointer is then passed into the new veth_xdp_rcv() code paths and accessed by netif_tx_queue_stopped().

Should queue_idx be checked against peer_dev->real_num_tx_queues before fetching the transmit queue?

[ ... ]

Quoted 4 lines
drivers/net/veth.c +0 -0
@@ -1091,6 +1133,17 @@ static void veth_napi_del_range(struct net_device *dev, int start, int end)
 		ptr_ring_cleanup(&rq->xdp_ring, veth_ptr_free);
 	}
 

drivers/net/veth.c static void veth_napi_del_range(struct net_device *dev, int start, int end) · line 1145

Quoted 10 lines
drivers/net/veth.c +10 -0
+	/* Reset BQL on peer's txqs: remaining ring items were freed above
+	 * without BQL completion, so DQL state must be reset.
+	 */
+	peer = rtnl_dereference(priv->peer);
+	if (peer) {
+		int peer_end = min(end, (int)peer->real_num_tx_queues);
+
+		for (i = start; i < peer_end; i++)
+			netdev_tx_reset_queue(netdev_get_tx_queue(peer, i));
+	}

Does this cleanup leave peer transmit queues permanently stopped if they were backpressured when xdp was disabled?

When the ptr_ring is full, veth_xmit() stops the transmit queue using netif_tx_stop_queue(), which sets the __QUEUE_STATE_DRV_XOFF flag.

While netdev_tx_reset_queue() successfully clears the newly added __QUEUE_STATE_STACK_XOFF flag, it does not clear __QUEUE_STATE_DRV_XOFF. If a queue was backpressured exactly when xdp is disabled, it appears it would remain locked forever.

Would it be appropriate to wake the queues using netif_tx_wake_queue() or explicitly clear the __QUEUE_STATE_DRV_XOFF flag during this cleanup?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#4 lore raw

Re: [PATCH net-next v4 2/4] veth: implement Byte Queue Limits (BQL) for latency reduction

Quoted 38 lines · On 02/05/2026 09.18, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Out-of-bounds array access on peer_dev->_tx in veth_poll() when local RX queues exceed peer TX queues.
- [High] Disabling XDP permanently stops peer TX queues if they were backpressured (DRV_XOFF leak). --

commit 9bc3ed022846dda6dd5cc131e798f949f63f491f
Author: Jesper Dangaard Brouer <[email protected]>

veth: implement Byte Queue Limits (BQL) for latency reduction

This commit adds byte queue limits support to the veth driver to provide qdiscs with feedback and reduce latency. It charges a fixed unit per packet rather than byte-based charging.

Quoted 4 lines
drivers/net/veth.c +0 -0
index e35df717e65e2..3de25ba34a906 100644

[ ... ]

drivers/net/veth.c static int veth_poll(struct napi_struct *napi, int budget) · line 1019

Quoted 6 lines
drivers/net/veth.c +1 -1
@@ -975,7 +1016,7 @@ static int veth_poll(struct napi_struct *napi, int budget)
  	peer_txq = peer_dev ? netdev_get_tx_queue(peer_dev, queue_idx) : NULL;
 
  	xdp_set_return_frame_no_direct();
-	done = veth_xdp_rcv(rq, budget, &bq, &stats);
+	done = veth_xdp_rcv(rq, budget, &bq, &stats, peer_txq);

Could this lead to an out-of-bounds array access on the peer device's transmit queues?

If a veth pair is configured asymmetrically, the local device might have more receive queues than the peer has transmit queues.

If packets are redirected from another interface into a higher-indexed receive queue, the poll function will run with a queue_idx greater than or equal to peer_dev->real_num_tx_queues.

Quoted 7 lines

Since netdev_get_tx_queue() performs a direct array access without bounds checking, peer_txq would point out-of-bounds. This out-of-bounds pointer is then passed into the new veth_xdp_rcv() code paths and accessed by netif_tx_queue_stopped().

Should queue_idx be checked against peer_dev->real_num_tx_queues before fetching the transmit queue?

Yes, for the XDP redirect code path, which enqueues frames into the peer's ptr_ring using smp_processor_id() % peer->real_num_rx_queues as the ring index. Then this is actually possible.

It is not a bug introduced in this patchset, but given we access peer_txq a bit earlier, then a fix for this is needed as part of this patchset. I have prepared a patch that fixes this.

drivers/net/veth.c +0 -0
index e35df717e65e..0cfb19b760dd 100644
@@ -972,7 +972,8 @@ 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 ? netdev_get_tx_queue(peer_dev, queue_idx) : NULL;
+ peer_txq = (peer_dev && queue_idx < peer_dev->real_num_tx_queues) ?
+ netdev_get_tx_queue(peer_dev, queue_idx) : NULL;

Quoted 23 lines

[ ... ]

Quoted 4 lines
drivers/net/veth.c +0 -0
@@ -1091,6 +1133,17 @@ static void veth_napi_del_range(struct net_device *dev, int start, int end)
  		ptr_ring_cleanup(&rq->xdp_ring, veth_ptr_free);
  	}
 

drivers/net/veth.c static void veth_napi_del_range(struct net_device *dev, int start, int end) · line 1145

Quoted 10 lines
drivers/net/veth.c +10 -0
+	/* Reset BQL on peer's txqs: remaining ring items were freed above
+	 * without BQL completion, so DQL state must be reset.
+	 */
+	peer = rtnl_dereference(priv->peer);
+	if (peer) {
+		int peer_end = min(end, (int)peer->real_num_tx_queues);
+
+		for (i = start; i < peer_end; i++)
+			netdev_tx_reset_queue(netdev_get_tx_queue(peer, i));
+	}

Does this cleanup leave peer transmit queues permanently stopped if they were backpressured when xdp was disabled?

When the ptr_ring is full, veth_xmit() stops the transmit queue using netif_tx_stop_queue(), which sets the __QUEUE_STATE_DRV_XOFF flag.

Quoted 7 lines

While netdev_tx_reset_queue() successfully clears the newly added __QUEUE_STATE_STACK_XOFF flag, it does not clear __QUEUE_STATE_DRV_XOFF. If a queue was backpressured exactly when xdp is disabled, it appears it would remain locked forever.

Would it be appropriate to wake the queues using netif_tx_wake_queue() or explicitly clear the __QUEUE_STATE_DRV_XOFF flag during this cleanup?

The watchdog timer in this patchset will call netif_tx_wake_queue(). So, we would recover from this unlikely case. I guess there is no harm in adding a netif_tx_wake_queue() call here (but we should likely guard it with netif_running(dev)).

--Jesper

#5 lore raw

PATCH v4 3/4 veth: add tx_timeout watchdog as BQL safety net

In reply to [email protected]

From: Jesper Dangaard Brouer <[email protected]>

With the introduction of BQL (Byte Queue Limits) for veth, there are now two independent mechanisms that can stop a transmit queue:

- DRV_XOFF: set by netif_tx_stop_queue() when the ptr_ring is full
- STACK_XOFF: set by BQL when the byte-in-flight limit is reached

If either mechanism stalls without a corresponding wake/completion, the queue stops permanently. Enable the net device watchdog timer and implement ndo_tx_timeout as a failsafe recovery.

The timeout handler resets BQL state (clearing STACK_XOFF) and wakes the queue (clearing DRV_XOFF), covering both stop mechanisms. The watchdog fires after 16 seconds, which accommodates worst-case NAPI processing (budget=64 packets x 250ms per-packet consumer delay) without false positives under normal backpressure.

---
 drivers/net/veth.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
drivers/net/veth.c +18 -0
index 3de25ba34a90..9d7b085c9548 100644
@@ -1431,6 +1431,22 @@ static int veth_set_channels(struct net_device *dev,
 	goto out;
 }
 
+static void veth_tx_timeout(struct net_device *dev, unsigned int txqueue)
+{
+	struct netdev_queue *txq = netdev_get_tx_queue(dev, txqueue);
+
+	netdev_err(dev,
+		   "veth backpressure(0x%lX) stalled(n:%ld) TXQ(%u) re-enable\n",
+		   txq->state, atomic_long_read(&txq->trans_timeout), txqueue);
+
+	/* Cannot call netdev_tx_reset_queue(): dql_reset() races with
+	 * peer NAPI calling dql_completed() concurrently.
+	 * Just clear the stop bits; the qdisc will re-stop if still stuck.
+	 */
+	clear_bit(__QUEUE_STATE_STACK_XOFF, &txq->state);
+	netif_tx_wake_queue(txq);
+}
+
 static int veth_open(struct net_device *dev)
 {
 	struct veth_priv *priv = netdev_priv(dev);
@@ -1769,6 +1785,7 @@ static const struct net_device_ops veth_netdev_ops = {
 	.ndo_bpf		= veth_xdp,
 	.ndo_xdp_xmit		= veth_ndo_xdp_xmit,
 	.ndo_get_peer_dev	= veth_peer_dev,
+	.ndo_tx_timeout		= veth_tx_timeout,
 };
 
 static const struct xdp_metadata_ops veth_xdp_metadata_ops = {
@@ -1808,6 +1825,7 @@ static void veth_setup(struct net_device *dev)
 	dev->priv_destructor = veth_dev_free;
 	dev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
 	dev->max_mtu = ETH_MAX_MTU;
+	dev->watchdog_timeo = msecs_to_jiffies(16000);
 
 	dev->hw_features = VETH_FEATURES;
 	dev->hw_enc_features = VETH_FEATURES;
Signature · 1 line
2.43.0
#6 lore raw

PATCH v4 4/4 net: sched: add timeout count to NETDEV WATCHDOG message

In reply to [email protected]

From: Jesper Dangaard Brouer <[email protected]>

Add the per-queue timeout counter (trans_timeout) to the core NETDEV WATCHDOG log message. This makes it easy to determine how frequently a particular queue is stalling from a single log line, without having to search through and correlate spaced-out log entries.

Useful for production monitoring where timeouts are spaced by the watchdog interval, making frequency hard to judge.

---
 net/sched/sch_generic.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
net/sched/sch_generic.c +4 -4
index a93321db8fd7..3e2e2e887a86 100644
@@ -533,13 +533,12 @@ static void dev_watchdog(struct timer_list *t)
 		    netif_running(dev) &&
 		    netif_carrier_ok(dev)) {
 			unsigned int timedout_ms = 0;
+			struct netdev_queue *txq;
 			unsigned int i;
 			unsigned long trans_start;
 			unsigned long oldest_start = jiffies;
 
 			for (i = 0; i < dev->num_tx_queues; i++) {
-				struct netdev_queue *txq;
-
 				txq = netdev_get_tx_queue(dev, i);
 				if (!netif_xmit_stopped(txq))
 					continue;
@@ -561,9 +560,10 @@ static void dev_watchdog(struct timer_list *t)
 
 			if (unlikely(timedout_ms)) {
 				trace_net_dev_xmit_timeout(dev, i);
-				netdev_crit(dev, "NETDEV WATCHDOG: CPU: %d: transmit queue %u timed out %u ms\n",
+				netdev_crit(dev, "NETDEV WATCHDOG: CPU: %d: transmit queue %u timed out %u ms (n:%ld)\n",
 					    raw_smp_processor_id(),
-					    i, timedout_ms);
+					    i, timedout_ms,
+					    atomic_long_read(&txq->trans_timeout));
 				netif_freeze_queues(dev);
 				dev->netdev_ops->ndo_tx_timeout(dev, i);
 				netif_unfreeze_queues(dev);
Signature · 1 line
2.43.0