veth: add Byte Queue Limits (BQL) support

---
 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 eb2e6851c6f6..a65d48b6ecc1 100644
@@ -169,6 +169,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 7f4f0837c09f..f699fded20b4 100644
@@ -2079,6 +2079,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

@@ -2495,6 +2496,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 0e71c9ed41e8..3cb470b0f17d 100644
@@ -1939,10 +1939,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

[email protected]

author

2026-06-12 08:35 UTC #2 lore raw

PATCH v7 2/5 veth: implement Byte Queue Limits (BQL) for latency reduction

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. At teardown, veth_napi_del_range() drains the leftover ring entries after synchronize_net() -- once NAPI is gone and the producer has stopped charging BQL (it observes rq->napi == NULL). Rather than netdev_tx_reset_queue(), which calls dql_reset() and races with a concurrent producer, balance the DQL accounting by completing the outstanding charges via netdev_tx_completed_queue(). The peer txq is still woken to clear any DRV_XOFF a late veth_xmit() may have set. Clamp the loop to the peer's num_tx_queues, since the peer may have fewer TX queues than the local device has RX queues (e.g. veth enslaved to a bond with XDP attached).

Co-developed-by Jonas Köppeler <[email protected]>

Signed-off-by Jesper Dangaard Brouer <[email protected]>

---
 drivers/net/veth.c | 131 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 120 insertions(+), 11 deletions(-)

drivers/net/veth.c +120 ~~-11~~

index 0cfb19b760dd..a3505627f49e 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,26 @@ 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));
+}
+
+/* Drain frames left in the ptr_ring at teardown, freeing each one and
+ * returning the number of BQL-charged SKBs.  The caller completes these
+ * via netdev_tx_completed_queue() to balance the DQL accounting, avoiding
+ * the racy netdev_tx_reset_queue()/dql_reset().
+ */
+static unsigned int veth_ptr_ring_drain(struct ptr_ring *ring)
+{
+	unsigned int n_bql = 0;
+	void *ptr;
+
+	while ((ptr = ptr_ring_consume(ring))) {
+		if (veth_ptr_is_bql(ptr))
+			n_bql++;
+		veth_ptr_free(ptr);
+	}
+
+	return n_bql;
 }
 
 static void __veth_xdp_flush(struct veth_rq *rq)

@@ -309,19 +347,39 @@ 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(__ptr_ring_check_produce(ring))) {
+		spin_unlock(&ring->producer_lock);
 		return NETDEV_TX_BUSY; /* signal qdisc layer */
+	}
+
+	/* Charge BQL before produce; the consumer cannot see the entry yet.
+	 * veth is lltx, so the stack skips HARD_TX_LOCK and txq->_xmit_lock
+	 * does not serialise txq->dql here.  This producer_lock is the single
+	 * producer lock for dql_queued() (1:1 with this rq's peer txq), and
+	 * the peer NAPI in veth_xdp_rcv() is the single completer -- the
+	 * two-context model that dql_queued()/dql_completed() require.
+	 */
+	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);
 }

@@ -347,11 +405,12 @@ static bool veth_skb_is_eligible_for_gro(const struct net_device *dev,
 static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
+	struct netdev_queue *txq = NULL;
 	struct veth_rq *rq = NULL;
-	struct netdev_queue *txq;
 	struct net_device *rcv;
 	int length = skb->len;
 	bool use_napi = false;
+	bool do_bql = false;
 	int ret, rxq;
 
 	rcu_read_lock();

@@ -375,8 +434,12 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	skb_tx_timestamp(skb);
-
-	ret = veth_forward_skb(rcv, skb, rq, use_napi);
+	if (rxq < dev->real_num_tx_queues) {
+		txq = netdev_get_tx_queue(dev, rxq);
+		/* 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)

@@ -412,6 +475,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 +964,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 +993,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))

@@ -976,7 +1045,7 @@ static int veth_poll(struct napi_struct *napi, int budget)
 		   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();

@@ -1074,6 +1143,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++) {

@@ -1085,11 +1155,49 @@ static void veth_napi_del_range(struct net_device *dev, int start, int end)
 	}
 	synchronize_net();
 
+	/* This rq's frames were BQL-charged on the peer's txq[i]. */
+	peer = rtnl_dereference(priv->peer);
+
 	for (i = start; i < end; i++) {
 		struct veth_rq *rq = &priv->rq[i];
+		struct netdev_queue *txq;
+		unsigned int n_bql;
 
 		rq->rx_notify_masked = false;
+
+		/* Drain leftover ring frames, counting BQL-charged SKBs that
+		 * were charged via netdev_tx_sent_queue() but never consumed.
+		 */
+		n_bql = veth_ptr_ring_drain(&rq->xdp_ring);
 		ptr_ring_cleanup(&rq->xdp_ring, veth_ptr_free);
+
+		if (!peer || i >= peer->num_tx_queues)
+			continue;
+
+		txq = netdev_get_tx_queue(peer, i);
+
+		/* Balance the peer txq's DQL accounting by completing the
+		 * outstanding charges instead of netdev_tx_reset_queue():
+		 * dql_reset() races with a concurrent producer, while
+		 * netdev_tx_completed_queue() is the normal single-completer
+		 * path and is safe here -- NAPI is gone (synchronize_net()
+		 * above) and the producer stopped charging BQL once it
+		 * observed rq->napi == NULL.  Completing every charge drives
+		 * DQL inflight to 0 and clears STACK_XOFF.
+		 */
+		if (n_bql)
+			netdev_tx_completed_queue(txq, n_bql,
+						  n_bql * VETH_BQL_UNIT);
+
+		/* DRV_XOFF is independent of BQL/STACK_XOFF: a concurrent
+		 * veth_xmit() may have set it between rcu_assign_pointer(napi,
+		 * NULL) and synchronize_net(); with NAPI gone nothing else
+		 * clears it.  The completion above only clears STACK_XOFF, so
+		 * still wake the txq to clear DRV_XOFF -- but only when the
+		 * device is still up.
+		 */
+		if (netif_running(dev))
+			netif_tx_wake_queue(txq);
 	}
 
 	for (i = start; i < end; i++) {

@@ -1741,6 +1849,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

[email protected]

AI review

2026-06-13 08:36 UTC #3 lore raw

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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The attempt to clear DRV_XOFF during teardown incorrectly checks netif_running(dev), leaving the peer's TX queue stalled for 16 seconds on interface restart. --

commit 9a6517a905c3dcf9d565963659494eab75fbf1f5
Author: Jesper Dangaard Brouer <[email protected]>
Subject: veth: implement Byte Queue Limits (BQL) for latency reduction

This commit implements Byte Queue Limits (BQL) in the veth driver to give qdiscs visibility into the ptr_ring fill level and reduce base latency. It charges a fixed unit per packet rather than byte lengths, and introduces logic to drain leftover ring entries and balance DQL accounting at teardown.

Quoted 4 lines

drivers/net/veth.c +0 -0
index 0cfb19b760dd5..a3505627f49e4 100644

[ ... ]

Quoted 37 lines

drivers/net/veth.c +28 -0

@@ -1085,11 +1155,49 @@ static void veth_napi_del_range(struct net_device *dev, int start, int end)
 	}
 	synchronize_net();
 
+	/* This rq's frames were BQL-charged on the peer's txq[i]. */
+	peer = rtnl_dereference(priv->peer);
+
 	for (i = start; i < end; i++) {
 		struct veth_rq *rq = &priv->rq[i];
+		struct netdev_queue *txq;
+		unsigned int n_bql;
 
 		rq->rx_notify_masked = false;
+
+		/* Drain leftover ring frames, counting BQL-charged SKBs that
+		 * were charged via netdev_tx_sent_queue() but never consumed.
+		 */
+		n_bql = veth_ptr_ring_drain(&rq->xdp_ring);
 		ptr_ring_cleanup(&rq->xdp_ring, veth_ptr_free);
+
+		if (!peer || i >= peer->num_tx_queues)
+			continue;
+
+		txq = netdev_get_tx_queue(peer, i);
+
+		/* Balance the peer txq's DQL accounting by completing the
+		 * outstanding charges instead of netdev_tx_reset_queue():
+		 * dql_reset() races with a concurrent producer, while
+		 * netdev_tx_completed_queue() is the normal single-completer
+		 * path and is safe here -- NAPI is gone (synchronize_net()
+		 * above) and the producer stopped charging BQL once it
+		 * observed rq->napi == NULL.  Completing every charge drives
+		 * DQL inflight to 0 and clears STACK_XOFF.
+		 */
+		if (n_bql)
+			netdev_tx_completed_queue(txq, n_bql,
+						  n_bql * VETH_BQL_UNIT);

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

Quoted 10 lines

drivers/net/veth.c +10 -0

+
+		/* DRV_XOFF is independent of BQL/STACK_XOFF: a concurrent
+		 * veth_xmit() may have set it between rcu_assign_pointer(napi,
+		 * NULL) and synchronize_net(); with NAPI gone nothing else
+		 * clears it.  The completion above only clears STACK_XOFF, so
+		 * still wake the txq to clear DRV_XOFF -- but only when the
+		 * device is still up.
+		 */
+		if (netif_running(dev))
+			netif_tx_wake_queue(txq);

[Severity: High]
This isn't a bug introduced by this patch, but the attempt to clear DRV_XOFF during teardown relies on netif_running(dev) which will always be false during dev_close().

Will this leave the peer's TX queue stalled?

When the local RX ring fills up, veth_xmit() stops the peer's TX queue by setting the __QUEUE_STATE_DRV_XOFF bit. During device close, the driver core in __dev_close_many() clears the __LINK_STATE_START bit before the ndo_stop callback is reached. Because of this, netif_running(dev) will always evaluate to false when veth_napi_del_range() is called.

Consequently, the netif_tx_wake_queue() call is bypassed and the peer's transmit queue remains frozen. When the local device is brought back up, nothing clears the DRV_XOFF bit on the peer, leaving the transmit path completely stalled until the TX watchdog timer expires 16 seconds later.

Could this check be adjusted or removed so the peer's queue can be properly woken and cleared of DRV_XOFF during teardown?

Quoted 3 lines

drivers/net/veth.c +0 -0

 	}
 
 	for (i = start; i < end; i++) {

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

[email protected]

author

2026-06-12 08:35 UTC #4 lore raw

PATCH v7 3/5 veth: add tx_timeout watchdog as BQL safety net

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.

Signed-off-by Jesper Dangaard Brouer <[email protected]>

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

drivers/net/veth.c +25 -0

index a3505627f49e..2473f730734b 100644
@@ -44,6 +44,13 @@
 #define VETH_XDP_TX_BULK_SIZE	16
 #define VETH_XDP_BATCH		16
 
+/* tx_timeout watchdog timeout. DRV_XOFF is only cleared at the end of a NAPI
+ * veth_poll() (netif_tx_wake_queue()), so the timeout must outlast a full
+ * worst-case poll: a 64-packet budget with a pessimistic 250 ms/pkt consumer
+ * delay => 64 * 250 ms = 16 s.
+ */
+#define VETH_WATCHDOG_TIMEOUT_MS	(64 * 250)
+
 struct veth_stats {
 	u64	rx_drops;
 	/* xdp */

@@ -1487,6 +1494,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);

@@ -1825,6 +1848,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 = {

@@ -1864,6 +1888,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(VETH_WATCHDOG_TIMEOUT_MS);
 
 	dev->hw_features = VETH_FEATURES;
 	dev->hw_enc_features = VETH_FEATURES;

Signature · 1 line

2.43.0

[email protected]

author

2026-06-12 08:35 UTC #5 lore raw

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

Link https://lore.kernel.org/all/[email protected]/ open here

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.

Signed-off-by Jesper Dangaard Brouer <[email protected]>

---
 net/sched/sch_generic.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

net/sched/sch_generic.c +4 -2

index 237ee1cd0136..eb6066d1ed90 100644
@@ -533,6 +533,7 @@ static void dev_watchdog(struct timer_list *t)
 		    netif_running(dev) &&
 		    netif_carrier_ok(dev)) {
 			unsigned int timedout_ms = 0;
+			unsigned long trans_timeout = 0;
 			unsigned int i;
 			unsigned long trans_start;
 			unsigned long oldest_start = jiffies;

@@ -553,6 +554,7 @@ static void dev_watchdog(struct timer_list *t)
 				if (time_after(jiffies, trans_start + dev->watchdog_timeo)) {
 					timedout_ms = jiffies_to_msecs(jiffies - trans_start);
 					atomic_long_inc(&txq->trans_timeout);
+					trans_timeout = atomic_long_read(&txq->trans_timeout);
 					break;
 				}
 				if (time_after(oldest_start, trans_start))

@@ -561,9 +563,9 @@ 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, trans_timeout);
 				netif_freeze_queues(dev);
 				dev->netdev_ops->ndo_tx_timeout(dev, i);
 				netif_unfreeze_queues(dev);

Signature · 1 line

2.43.0

[email protected]

author

2026-06-12 08:35 UTC #6 lore raw

PATCH v7 5/5 veth: time-based BQL completion coalescing via ethtool tx-usecs

From: Simon Schippers <[email protected]>

Per-packet BQL completion forces DQL to converge on limit=2, causing excessive NAPI scheduling overhead and qdisc requeues.

Accumulate BQL completions and flush them when a configurable time threshold (tx-usecs) is exceeded, letting DQL discover a limit that bounds actual queuing delay to the configured interval. Coalescing state persists across NAPI polls in struct veth_rq so completions can accumulate beyond a single budget=64 cycle.

The flush condition is:

state->time + bql_flush_ns <= current_time || state->n_bql > dql.limit

Flushing when n_bql exceeds dql.limit handles BQL starvation.

The comparison is strictly greater-than because netdev_tx_sent_queue() always lets the producer exceed the limit by one before it stops, so n_bql == dql.limit is a normal in-flight state. dql.limit lives in the same cacheline as the completion path, so the check is cheap.

Add ethtool tx-usecs support for runtime tuning. Default is 100 us; setting tx-usecs to 0 disables coalescing and falls back to per-packet completion.

ethtool -C <veth-dev> tx-usecs 500 # 500us coalescing
ethtool -C <veth-dev> tx-usecs 0 # per-packet (no coalescing)

Co-developed-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Jesper Dangaard Brouer <[email protected]>

Co-developed-by Jonas Köppeler <[email protected]>

---
 drivers/net/veth.c | 123 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 117 insertions(+), 6 deletions(-)

drivers/net/veth.c +117 -6

index 2473f730734b..c62d87a8402c 100644
@@ -28,6 +28,7 @@
 #include <linux/bpf_trace.h>
 #include <linux/net_tstamp.h>
 #include <linux/skbuff_ref.h>
+#include <linux/sched/clock.h>
 #include <net/page_pool/helpers.h>
 
 #define DRV_NAME	"veth"

@@ -50,6 +51,7 @@
  * delay => 64 * 250 ms = 16 s.
  */
 #define VETH_WATCHDOG_TIMEOUT_MS	(64 * 250)
+#define VETH_BQL_COAL_TX_USECS	100 /* default tx-usecs for BQL batching */
 
 struct veth_stats {
 	u64	rx_drops;

@@ -69,6 +71,11 @@ struct veth_rq_stats {
 	struct u64_stats_sync	syncp;
 };
 
+struct veth_bql_state {
+	u64	time;	/* sched_clock() when current coalescing window started */
+	uint	n_bql;	/* BQL completions batched in the current window */
+};
+
 struct veth_rq {
 	struct napi_struct	xdp_napi;
 	struct napi_struct __rcu *napi; /* points to xdp_napi when the latter is initialized */

@@ -76,6 +83,7 @@ struct veth_rq {
 	struct bpf_prog __rcu	*xdp_prog;
 	struct xdp_mem_info	xdp_mem;
 	struct veth_rq_stats	stats;
+	struct veth_bql_state	bql_state;
 	bool			rx_notify_masked;
 	struct ptr_ring		xdp_ring;
 	struct xdp_rxq_info	xdp_rxq;

@@ -88,6 +96,7 @@ struct veth_priv {
 	struct bpf_prog		*_xdp_prog;
 	struct veth_rq		*rq;
 	unsigned int		requested_headroom;
+	unsigned int		tx_coal_usecs;	/* BQL completion coalescing */
 };
 
 struct veth_xdp_tx_bq {

@@ -272,7 +281,56 @@ static void veth_get_channels(struct net_device *dev,
 static int veth_set_channels(struct net_device *dev,
 			     struct ethtool_channels *ch);
 
+static int veth_get_coalesce(struct net_device *dev,
+			     struct ethtool_coalesce *ec,
+			     struct kernel_ethtool_coalesce *kernel_coal,
+			     struct netlink_ext_ack *extack)
+{
+	struct veth_priv *priv = netdev_priv(dev);
+
+	ec->tx_coalesce_usecs = priv->tx_coal_usecs;
+	return 0;
+}
+
+static int veth_set_coalesce(struct net_device *dev,
+			     struct ethtool_coalesce *ec,
+			     struct kernel_ethtool_coalesce *kernel_coal,
+			     struct netlink_ext_ack *extack)
+{
+	struct veth_priv *priv = netdev_priv(dev);
+	struct net_device *peer;
+
+	/* The coalescing window delays BQL completions, so keep tx-usecs well
+	 * below the tx_timeout watchdog; otherwise a large value could stall a
+	 * stopped queue long enough to trip a false watchdog timeout. Cap at
+	 * half the watchdog to leave a generous safety margin. tx-usecs is
+	 * microseconds, the watchdog is milliseconds.
+	 */
+	if (ec->tx_coalesce_usecs > VETH_WATCHDOG_TIMEOUT_MS / 2 * USEC_PER_MSEC) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "tx-usecs must stay below half the tx_timeout watchdog");
+		return -ERANGE;
+	}
+
+	/* Paired with READ_ONCE in veth_xdp_rcv(). */
+	WRITE_ONCE(priv->tx_coal_usecs, ec->tx_coalesce_usecs);
+
+	/* veth_xdp_rcv() reads each device's own value, so mirror it onto
+	 * the peer to keep the pair symmetric: both directions coalesce
+	 * with the same tx-usecs. Called under RTNL, rtnl_dereference() is safe.
+	 */
+	peer = rtnl_dereference(priv->peer);
+	if (peer) {
+		struct veth_priv *peer_priv = netdev_priv(peer);
+
+		WRITE_ONCE(peer_priv->tx_coal_usecs, ec->tx_coalesce_usecs);
+	}
+
+	return 0;
+}
+
 static const struct ethtool_ops veth_ethtool_ops = {
+	.supported_coalesce_params = ETHTOOL_COALESCE_TX_USECS,
 	.get_drvinfo		= veth_get_drvinfo,
 	.get_link		= ethtool_op_get_link,
 	.get_strings		= veth_get_strings,

@@ -282,6 +340,8 @@ static const struct ethtool_ops veth_ethtool_ops = {
 	.get_ts_info		= ethtool_op_get_ts_info,
 	.get_channels		= veth_get_channels,
 	.set_channels		= veth_set_channels,
+	.get_coalesce		= veth_get_coalesce,
+	.set_coalesce		= veth_set_coalesce,
 };
 
 /* general routines */

@@ -969,13 +1029,54 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
 	return NULL;
 }
 
+static void veth_bql_maybe_complete(struct veth_bql_state *state,
+				    struct netdev_queue *peer_txq,
+				    u64 bql_flush_ns)
+{
+	u64 current_time;
+
+	/* There is no reason to complete with 0 and
+	 * peer_txq could go away.
+	 */
+	if (!state->n_bql || !peer_txq)
+		return;
+
+	current_time = sched_clock();
+
+	/* We complete if:
+	 * 1. We reach bql_flush_ns.
+	 * 2. We potentially have BQL starvation.
+	 */
+	if (state->time + bql_flush_ns <= current_time ||
+	    state->n_bql > peer_txq->dql.limit) {
+		netdev_tx_completed_queue(peer_txq, state->n_bql,
+					  state->n_bql * VETH_BQL_UNIT);
+		state->time = current_time;
+		state->n_bql = 0;
+	}
+}
+
 static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 			struct veth_xdp_tx_bq *bq,
 			struct veth_stats *stats,
 			struct netdev_queue *peer_txq)
 {
+	struct veth_priv *priv = netdev_priv(rq->dev);
+	struct veth_bql_state *state = &rq->bql_state;
 	int i, done = 0, n_xdpf = 0;
 	void *xdpf[VETH_XDP_BATCH];
+	u64 bql_flush_ns;
+
+	/* Mirrored to both peers; paired with WRITE_ONCE() in veth_set_coalesce */
+	bql_flush_ns = (u64)READ_ONCE(priv->tx_coal_usecs) * 1000;
+
+	/* Clamp stored timestamp in case we migrated to a CPU with a behind
+	 * sched_clock(); tries to reduce late BQL flushes.
+	 */
+	state->time = min(state->time, sched_clock());
+
+	/* Flush completions that timed out since the previous NAPI poll. */
+	veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
 
 	for (i = 0; i < budget; i++) {
 		void *ptr = __ptr_ring_consume(&rq->xdp_ring);

@@ -1000,12 +1101,11 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 			}
 		} else {
 			/* ndo_start_xmit */
-			bool bql_charged = veth_ptr_is_bql(ptr);
 			struct sk_buff *skb = veth_ptr_to_skb(ptr);
 
+			if (veth_ptr_is_bql(ptr))
+				state->n_bql++;
 			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) {

@@ -1015,6 +1115,7 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 					napi_gro_receive(&rq->xdp_napi, skb);
 			}
 		}
+		veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
 		done++;
 	}

@@ -1123,6 +1224,9 @@ static int __veth_napi_enable_range(struct net_device *dev, int start, int end)
 	for (i = start; i < end; i++) {
 		struct veth_rq *rq = &priv->rq[i];
 
+		rq->bql_state.time = sched_clock();
+		rq->bql_state.n_bql = 0;
+
 		napi_enable(&rq->xdp_napi);
 		rcu_assign_pointer(priv->rq[i].napi, &priv->rq[i].xdp_napi);
 	}

@@ -1172,11 +1276,15 @@ static void veth_napi_del_range(struct net_device *dev, int start, int end)
 
 		rq->rx_notify_masked = false;
 
-		/* Drain leftover ring frames, counting BQL-charged SKBs that
-		 * were charged via netdev_tx_sent_queue() but never consumed.
+		/* Drain leftover ring frames, counting BQL-charged SKBs, and
+		 * add the completions still pending in the coalescing window
+		 * (consumed by NAPI but not yet flushed).  Both were charged
+		 * via netdev_tx_sent_queue() and are still outstanding.
 		 */
-		n_bql = veth_ptr_ring_drain(&rq->xdp_ring);
+		n_bql = veth_ptr_ring_drain(&rq->xdp_ring) + rq->bql_state.n_bql;
 		ptr_ring_cleanup(&rq->xdp_ring, veth_ptr_free);
+		rq->bql_state.n_bql = 0;
+		rq->bql_state.time = 0;
 
 		if (!peer || i >= peer->num_tx_queues)
 			continue;

@@ -1865,6 +1973,8 @@ static const struct xdp_metadata_ops veth_xdp_metadata_ops = {
 
 static void veth_setup(struct net_device *dev)
 {
+	struct veth_priv *priv = netdev_priv(dev);
+
 	ether_setup(dev);
 
 	dev->priv_flags &= ~IFF_TX_SKB_SHARING;

@@ -1889,6 +1999,7 @@ static void veth_setup(struct net_device *dev)
 	dev->pcpu_stat_type = NETDEV_PCPU_STAT_TSTATS;
 	dev->max_mtu = ETH_MAX_MTU;
 	dev->watchdog_timeo = msecs_to_jiffies(VETH_WATCHDOG_TIMEOUT_MS);
+	priv->tx_coal_usecs = VETH_BQL_COAL_TX_USECS;
 
 	dev->hw_features = VETH_FEATURES;
 	dev->hw_enc_features = VETH_FEATURES;

Signature · 1 line

2.43.0

[email protected]

AI review

2026-06-13 08:36 UTC #7 lore raw

Re: [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] BQL completions are stranded when NAPI goes to sleep if the timer has not expired, artificially…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] BQL completions are stranded when NAPI goes to sleep if the timer has not expired, artificially destroying DQL performance estimates.
- [Low] Direct access to peer_txq->dql.limit breaks compilation when CONFIG_BQL is disabled.
--

commit 7aba7f3c0951cf87cb2ef78e981e1f66a316a71d
Author: Simon Schippers <[email protected]>

veth: time-based BQL completion coalescing via ethtool tx-usecs

This patch accumulates BQL completions and flushes them when a configurable time threshold (tx-usecs) is exceeded. This lets DQL discover a limit that bounds actual queuing delay to the configured interval.

Quoted 4 lines

drivers/net/veth.c +0 -0
index 2473f730734b7..c62d87a8402cb 100644

[ ... ]

Quoted 14 lines

drivers/net/veth.c +10 -0

@@ -969,13 +1029,54 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
 	return NULL;
 }
 
+static void veth_bql_maybe_complete(struct veth_bql_state *state,
+				    struct netdev_queue *peer_txq,
+				    u64 bql_flush_ns)
+{
+	u64 current_time;
+
+	/* There is no reason to complete with 0 and
+	 * peer_txq could go away.
+	 */
+	if (!state->n_bql || !peer_txq)

drivers/net/veth.c static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq, · line 1051

Quoted 10 lines

drivers/net/veth.c +10 -0

+		return;
+
+	current_time = sched_clock();
+
+	/* We complete if:
+	 * 1. We reach bql_flush_ns.
+	 * 2. We potentially have BQL starvation.
+	 */
+	if (state->time + bql_flush_ns <= current_time ||
+	    state->n_bql > peer_txq->dql.limit) {

[Severity: Low]
Does this direct access to peer_txq->dql.limit cause compilation errors when CONFIG_BQL is disabled? The dql member of struct netdev_queue is conditionally compiled, so accessing it directly might break the build.

Quoted 11 lines

drivers/net/veth.c +7 -0

+		netdev_tx_completed_queue(peer_txq, state->n_bql,
+					  state->n_bql * VETH_BQL_UNIT);
+		state->time = current_time;
+		state->n_bql = 0;
+	}
+}
+
 static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 			struct veth_xdp_tx_bq *bq,
 			struct veth_stats *stats,
 			struct netdev_queue *peer_txq)

[ ... ]

Quoted 16 lines

drivers/net/veth.c +2 -3

@@ -1000,12 +1101,11 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 			}
 		} else {
 			/* ndo_start_xmit */
-			bool bql_charged = veth_ptr_is_bql(ptr);
 			struct sk_buff *skb = veth_ptr_to_skb(ptr);
 
+			if (veth_ptr_is_bql(ptr))
+				state->n_bql++;
 			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) {

drivers/net/veth.c static int veth_xdp_rcv(struct veth_rq *rq, int budget, · line 1120

Quoted 6 lines

drivers/net/veth.c +1 -0

@@ -1015,6 +1115,7 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 					napi_gro_receive(&rq->xdp_napi, skb);
 			}
 		}
+		veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
 		done++;
 	}

[Severity: High]
If veth_xdp_rcv() finishes and returns a done count less than the budget, NAPI will go to sleep in veth_poll(). Do we need to unconditionally flush any stranded BQL completions in veth_poll() before sleeping?

If completions are left in rq->bql_state indefinitely across NAPI idle periods, it might present an artificially massive delay to DQL. This could cause DQL to mistakenly conclude the hardware is extremely slow and aggressively shrink dql.limit to its minimum, crippling throughput on subsequent bursts.

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5

Simon Schippers

2026-06-13 14:14 UTC #8 lore raw

Re: [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs

Quoted 166 lines · On 6/12/26 10:35, [email protected] wrote:

From: Simon Schippers <[email protected]>

Per-packet BQL completion forces DQL to converge on limit=2, causing excessive NAPI scheduling overhead and qdisc requeues.

The flush condition is:

state->time + bql_flush_ns <= current_time || state->n_bql > dql.limit

Flushing when n_bql exceeds dql.limit handles BQL starvation.

Add ethtool tx-usecs support for runtime tuning. Default is 100 us; setting tx-usecs to 0 disables coalescing and falls back to per-packet completion.

ethtool -C <veth-dev> tx-usecs 500 # 500us coalescing
ethtool -C <veth-dev> tx-usecs 0 # per-packet (no coalescing)

Co-developed-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Jesper Dangaard Brouer <[email protected]>

Co-developed-by Jonas Köppeler <[email protected]>

[1] https://lore.kernel.org/netdev/[email protected]/

---
 drivers/net/veth.c | 123 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 117 insertions(+), 6 deletions(-)

drivers/net/veth.c +70 -0

index 2473f730734b..c62d87a8402c 100644
@@ -28,6 +28,7 @@
 #include <linux/bpf_trace.h>
 #include <linux/net_tstamp.h>
 #include <linux/skbuff_ref.h>
+#include <linux/sched/clock.h>
 #include <net/page_pool/helpers.h>
 
 #define DRV_NAME	"veth"

@@ -50,6 +51,7 @@
  * delay => 64 * 250 ms = 16 s.
  */
 #define VETH_WATCHDOG_TIMEOUT_MS	(64 * 250)
+#define VETH_BQL_COAL_TX_USECS	100 /* default tx-usecs for BQL batching */
 
 struct veth_stats {
 	u64	rx_drops;

@@ -69,6 +71,11 @@ struct veth_rq_stats {
 	struct u64_stats_sync	syncp;
 };
 
+struct veth_bql_state {
+	u64	time;	/* sched_clock() when current coalescing window started */
+	uint	n_bql;	/* BQL completions batched in the current window */
+};
+
 struct veth_rq {
 	struct napi_struct	xdp_napi;
 	struct napi_struct __rcu *napi; /* points to xdp_napi when the latter is initialized */

@@ -76,6 +83,7 @@ struct veth_rq {
 	struct bpf_prog __rcu	*xdp_prog;
 	struct xdp_mem_info	xdp_mem;
 	struct veth_rq_stats	stats;
+	struct veth_bql_state	bql_state;
 	bool			rx_notify_masked;
 	struct ptr_ring		xdp_ring;
 	struct xdp_rxq_info	xdp_rxq;

@@ -88,6 +96,7 @@ struct veth_priv {
 	struct bpf_prog		*_xdp_prog;
 	struct veth_rq		*rq;
 	unsigned int		requested_headroom;
+	unsigned int		tx_coal_usecs;	/* BQL completion coalescing */
 };
 
 struct veth_xdp_tx_bq {

@@ -272,7 +281,56 @@ static void veth_get_channels(struct net_device *dev,
 static int veth_set_channels(struct net_device *dev,
 			     struct ethtool_channels *ch);
 
+static int veth_get_coalesce(struct net_device *dev,
+			     struct ethtool_coalesce *ec,
+			     struct kernel_ethtool_coalesce *kernel_coal,
+			     struct netlink_ext_ack *extack)
+{
+	struct veth_priv *priv = netdev_priv(dev);
+
+	ec->tx_coalesce_usecs = priv->tx_coal_usecs;
+	return 0;
+}
+
+static int veth_set_coalesce(struct net_device *dev,
+			     struct ethtool_coalesce *ec,
+			     struct kernel_ethtool_coalesce *kernel_coal,
+			     struct netlink_ext_ack *extack)
+{
+	struct veth_priv *priv = netdev_priv(dev);
+	struct net_device *peer;
+
+	/* The coalescing window delays BQL completions, so keep tx-usecs well
+	 * below the tx_timeout watchdog; otherwise a large value could stall a
+	 * stopped queue long enough to trip a false watchdog timeout. Cap at
+	 * half the watchdog to leave a generous safety margin. tx-usecs is
+	 * microseconds, the watchdog is milliseconds.
+	 */
+	if (ec->tx_coalesce_usecs > VETH_WATCHDOG_TIMEOUT_MS / 2 * USEC_PER_MSEC) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "tx-usecs must stay below half the tx_timeout watchdog");
+		return -ERANGE;
+	}
+
+	/* Paired with READ_ONCE in veth_xdp_rcv(). */
+	WRITE_ONCE(priv->tx_coal_usecs, ec->tx_coalesce_usecs);
+
+	/* veth_xdp_rcv() reads each device's own value, so mirror it onto
+	 * the peer to keep the pair symmetric: both directions coalesce
+	 * with the same tx-usecs. Called under RTNL, rtnl_dereference() is safe.
+	 */
+	peer = rtnl_dereference(priv->peer);
+	if (peer) {
+		struct veth_priv *peer_priv = netdev_priv(peer);
+
+		WRITE_ONCE(peer_priv->tx_coal_usecs, ec->tx_coalesce_usecs);
+	}
+
+	return 0;
+}
+
 static const struct ethtool_ops veth_ethtool_ops = {
+	.supported_coalesce_params = ETHTOOL_COALESCE_TX_USECS,
 	.get_drvinfo		= veth_get_drvinfo,
 	.get_link		= ethtool_op_get_link,
 	.get_strings		= veth_get_strings,

@@ -282,6 +340,8 @@ static const struct ethtool_ops veth_ethtool_ops = {
 	.get_ts_info		= ethtool_op_get_ts_info,
 	.get_channels		= veth_get_channels,
 	.set_channels		= veth_set_channels,
+	.get_coalesce		= veth_get_coalesce,
+	.set_coalesce		= veth_set_coalesce,
 };
 
 /* general routines */

@@ -969,13 +1029,54 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
 	return NULL;
 }
 
+static void veth_bql_maybe_complete(struct veth_bql_state *state,
+				    struct netdev_queue *peer_txq,
+				    u64 bql_flush_ns)
+{
+	u64 current_time;
+
+	/* There is no reason to complete with 0 and
+	 * peer_txq could go away.
+	 */
+	if (!state->n_bql || !peer_txq)

drivers/net/veth.c static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq, · line 1051

Quoted 10 lines

drivers/net/veth.c +10 -0

+		return;
+
+	current_time = sched_clock();
+
+	/* We complete if:
+	 * 1. We reach bql_flush_ns.
+	 * 2. We potentially have BQL starvation.
+	 */
+	if (state->time + bql_flush_ns <= current_time ||
+	    state->n_bql > peer_txq->dql.limit) {

Both Sashiko-Nipa and Sashiko-Gemini are right, this is missing a #ifdef CONFIG_BQL. Not sure what is the best way to add them. And for the struct we could maybe do:

#ifdef CONFIG_BQL
struct veth_bql_state {
u64 time; /* sched_clock() when current coalescing window started */
uint n_bql; /* BQL completions batched in the current window */
};
#else
struct veth_bql_state {};
#endif

Quoted 47 lines

drivers/net/veth.c +23 -3

+		netdev_tx_completed_queue(peer_txq, state->n_bql,
+					  state->n_bql * VETH_BQL_UNIT);
+		state->time = current_time;
+		state->n_bql = 0;
+	}
+}
+
 static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 			struct veth_xdp_tx_bq *bq,
 			struct veth_stats *stats,
 			struct netdev_queue *peer_txq)
 {
+	struct veth_priv *priv = netdev_priv(rq->dev);
+	struct veth_bql_state *state = &rq->bql_state;
 	int i, done = 0, n_xdpf = 0;
 	void *xdpf[VETH_XDP_BATCH];
+	u64 bql_flush_ns;
+
+	/* Mirrored to both peers; paired with WRITE_ONCE() in veth_set_coalesce */
+	bql_flush_ns = (u64)READ_ONCE(priv->tx_coal_usecs) * 1000;
+
+	/* Clamp stored timestamp in case we migrated to a CPU with a behind
+	 * sched_clock(); tries to reduce late BQL flushes.
+	 */
+	state->time = min(state->time, sched_clock());
+
+	/* Flush completions that timed out since the previous NAPI poll. */
+	veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
 
 	for (i = 0; i < budget; i++) {
 		void *ptr = __ptr_ring_consume(&rq->xdp_ring);

@@ -1000,12 +1101,11 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 			}
 		} else {
 			/* ndo_start_xmit */
-			bool bql_charged = veth_ptr_is_bql(ptr);
 			struct sk_buff *skb = veth_ptr_to_skb(ptr);
 
+			if (veth_ptr_is_bql(ptr))
+				state->n_bql++;
 			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) {

drivers/net/veth.c static int veth_xdp_rcv(struct veth_rq *rq, int budget, · line 1119

Quoted 5 lines

drivers/net/veth.c +1 -0

@@ -1015,6 +1115,7 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 					napi_gro_receive(&rq->xdp_napi, skb);
 			}
 		}
+		veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
 		done++;

Sashiko-Nipa reports:

"If veth_xdp_rcv() finishes and returns a done count less than the budget, NAPI will go to sleep in veth_poll(). Do we need to unconditionally flush any stranded BQL completions in veth_poll() before sleeping? If completions are left in rq->bql_state indefinitely across NAPI idle periods, it might present an artificially massive delay to DQL. This could cause DQL to mistakenly conclude the hardware is extremely slow and aggressively shrink dql.limit to its minimum, crippling throughput on subsequent bursts."

Again the issue that I found to be non-problematic in [1] and can be seen by an BQL inflight > 0 when for example pktgen suddenly stops.

If we would "unconditionally flush any stranded BQL completions in veth_poll() before sleeping" we would *not* accumulate BQL completions across NAPI polls but we want to do that.

Do you agree?

Jonas Köppeler

2026-06-30 14:03 UTC #9 lore raw

Re: [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs

Quoted 169 lines · Simon Schippers

Quoted 168 lines · On 6/12/26 10:35, [email protected] wrote:

From: Simon Schippers <[email protected]>

Per-packet BQL completion forces DQL to converge on limit=2, causing excessive NAPI scheduling overhead and qdisc requeues.

The flush condition is:

state->time + bql_flush_ns <= current_time || state->n_bql > dql.limit

Flushing when n_bql exceeds dql.limit handles BQL starvation.

Add ethtool tx-usecs support for runtime tuning. Default is 100 us; setting tx-usecs to 0 disables coalescing and falls back to per-packet completion.

ethtool -C <veth-dev> tx-usecs 500 # 500us coalescing
ethtool -C <veth-dev> tx-usecs 0 # per-packet (no coalescing)

Co-developed-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Jesper Dangaard Brouer <[email protected]>

Co-developed-by Jonas Köppeler <[email protected]>

---
  drivers/net/veth.c | 123 ++++++++++++++++++++++++++++++++++++++++++---
  1 file changed, 117 insertions(+), 6 deletions(-)

drivers/net/veth.c +72 -0

index 2473f730734b..c62d87a8402c 100644
@@ -28,6 +28,7 @@
  #include <linux/bpf_trace.h>
  #include <linux/net_tstamp.h>
  #include <linux/skbuff_ref.h>
+#include <linux/sched/clock.h>
  #include <net/page_pool/helpers.h>
 
  #define DRV_NAME	"veth"

@@ -50,6 +51,7 @@
   * delay => 64 * 250 ms = 16 s.
   */
  #define VETH_WATCHDOG_TIMEOUT_MS	(64 * 250)
+#define VETH_BQL_COAL_TX_USECS	100 /* default tx-usecs for BQL batching*/
 
  struct veth_stats {
  	u64	rx_drops;

@@ -69,6 +71,11 @@ struct veth_rq_stats {
  	struct u64_stats_sync	syncp;
  };
 
+struct veth_bql_state {
+	u64	time;	/* sched_clock() when current coalescing window started */
+	uint	n_bql;	/* BQL completions batched in the current window */
+};
+
  struct veth_rq {
  	struct napi_struct	xdp_napi;
  	struct napi_struct __rcu *napi; /* points to xdp_napi when the latteris initialized */

@@ -76,6 +83,7 @@ struct veth_rq {
  	struct bpf_prog __rcu	*xdp_prog;
  	struct xdp_mem_info	xdp_mem;
  	struct veth_rq_stats	stats;
+	struct veth_bql_state	bql_state;
  	bool			rx_notify_masked;
  	struct ptr_ring		xdp_ring;
  	struct xdp_rxq_info	xdp_rxq;

@@ -88,6 +96,7 @@ struct veth_priv {
  	struct bpf_prog		*_xdp_prog;
  	struct veth_rq		*rq;
  	unsigned int		requested_headroom;
+	unsigned int		tx_coal_usecs;	/* BQL completion coalescing */
  };
 
  struct veth_xdp_tx_bq {

@@ -272,7 +281,56 @@ static void veth_get_channels(struct net_device *dev,
  static int veth_set_channels(struct net_device *dev,
  			     struct ethtool_channels *ch);
 
+static int veth_get_coalesce(struct net_device *dev,
+			     struct ethtool_coalesce *ec,
+			     struct kernel_ethtool_coalesce *kernel_coal,
+			     struct netlink_ext_ack *extack)
+{
+	struct veth_priv *priv = netdev_priv(dev);
+
+	ec->tx_coalesce_usecs = priv->tx_coal_usecs;
+	return 0;
+}
+
+static int veth_set_coalesce(struct net_device *dev,
+			     struct ethtool_coalesce *ec,
+			     struct kernel_ethtool_coalesce *kernel_coal,
+			     struct netlink_ext_ack *extack)
+{
+	struct veth_priv *priv = netdev_priv(dev);
+	struct net_device *peer;
+
+	/* The coalescing window delays BQL completions, so keep tx-usecs well
+	 * below the tx_timeout watchdog; otherwise a large value could stall a
+	 * stopped queue long enough to trip a false watchdog timeout. Cap at
+	 * half the watchdog to leave a generous safety margin. tx-usecs is
+	 * microseconds, the watchdog is milliseconds.
+	 */
+	if (ec->tx_coalesce_usecs > VETH_WATCHDOG_TIMEOUT_MS / 2 * USEC_PER_MSEC) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "tx-usecs must stay below half the tx_timeout watchdog");
+		return -ERANGE;
+	}
+
+	/* Paired with READ_ONCE in veth_xdp_rcv(). */
+	WRITE_ONCE(priv->tx_coal_usecs, ec->tx_coalesce_usecs);
+
+	/* veth_xdp_rcv() reads each device's own value, so mirror it onto
+	 * the peer to keep the pair symmetric: both directions coalesce
+	 * with the same tx-usecs. Called under RTNL, rtnl_dereference() is safe.
+	 */
+	peer = rtnl_dereference(priv->peer);
+	if (peer) {
+		struct veth_priv *peer_priv = netdev_priv(peer);
+
+		WRITE_ONCE(peer_priv->tx_coal_usecs, ec->tx_coalesce_usecs);
+	}
+
+	return 0;
+}
+
  static const struct ethtool_ops veth_ethtool_ops = {
+	.supported_coalesce_params = ETHTOOL_COALESCE_TX_USECS,
  	.get_drvinfo		= veth_get_drvinfo,
  	.get_link		= ethtool_op_get_link,
  	.get_strings		= veth_get_strings,

@@ -282,6 +340,8 @@ static const struct ethtool_ops veth_ethtool_ops ={
  	.get_ts_info		= ethtool_op_get_ts_info,
  	.get_channels		= veth_get_channels,
  	.set_channels		= veth_set_channels,
+	.get_coalesce		= veth_get_coalesce,
+	.set_coalesce		= veth_set_coalesce,
  };
 
  /* general routines */

@@ -969,13 +1029,54 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
  	return NULL;
  }
 
+static void veth_bql_maybe_complete(struct veth_bql_state *state,
+				    struct netdev_queue *peer_txq,
+				    u64 bql_flush_ns)
+{
+	u64 current_time;
+
+	/* There is no reason to complete with 0 and
+	 * peer_txq could go away.
+	 */
+	if (!state->n_bql || !peer_txq)
+		return;
+

Quoted 9 lines

drivers/net/veth.c static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq, · line 1051
Quoted 8 lines
drivers/net/veth.c +8 -0
+	current_time = sched_clock();
+
+	/* We complete if:
+	 * 1. We reach bql_flush_ns.
+	 * 2. We potentially have BQL starvation.
+	 */
+	if (state->time + bql_flush_ns <= current_time ||
+	    state->n_bql > peer_txq->dql.limit) {

Indeed, this does not compile when CONFIG_BQL is not set. I think we should just bring back the 'queue is empty + queue is stopped' check from v6 back at the end of the poll and remove the n_bql > dql.limit check. It also feels not obvious why this is handling the starvation case. This only works, because the producer has went overlimit previously and was stopped. So more than 'limit' packets have been enqueued to the ring, and they are eventually drained when this check is true. By removing this we can also avoid accessing dql internal members, but if you don't think that's a problem we can leave as is.

Further, this is only works if VETH_BQL_UNIT stays 1, otherwise it will never fire. Anyway, still its necessary to check for CONFIG_BQL. But we could solve this by adding VETH_BQL_UNIT to n_bql instead of 1. This is also safe from any overflows, since limit is bound to limit_max, inflight is always less than limit + 1*VETH_BQL_UNIT and n_bql <= inflight.

In a version of bringing back the 'queue-empty' check and keeping most of the current logic (so a mixture of v6 and v7) resulted in the same performance on an x86_64 architecture.

Quoted 4 lines

Both Sashiko-Nipa and Sashiko-Gemini are right, this is missing a #ifdef CONFIG_BQL. Not sure what is the best way to add them. And for the struct we could maybe do:

Quoted 8 lines

#ifdef CONFIG_BQL
struct veth_bql_state {
u64 time; /* sched_clock() when current coalescing window started */
uint n_bql; /* BQL completions batched in the current window */
};
#else
struct veth_bql_state {};
#endif

Regarding the configs: we can just do something along those lines. struct veth_rq {
...

#ifdef CONFIG_BQL
	struct veth_bql_state		dql;

#endif
...
}

and we put the rest of the code that accesses or performs an action regarding bql in some functions and do it like in netdev_* functions with

Function-Signature()
{
#ifdef CONFIG_BQL
// Code
#endif
}

Wdyt?
- Jonas

Quoted 74 lines

Quoted 47 lines
drivers/net/veth.c +23 -3
+		netdev_tx_completed_queue(peer_txq, state->n_bql,
+					  state->n_bql * VETH_BQL_UNIT);
+		state->time = current_time;
+		state->n_bql = 0;
+	}
+}
+
  static int veth_xdp_rcv(struct veth_rq *rq, int budget,
  			struct veth_xdp_tx_bq *bq,
  			struct veth_stats *stats,
  			struct netdev_queue *peer_txq)
  {
+	struct veth_priv *priv = netdev_priv(rq->dev);
+	struct veth_bql_state *state = &rq->bql_state;
  	int i, done = 0, n_xdpf = 0;
  	void *xdpf[VETH_XDP_BATCH];
+	u64 bql_flush_ns;
+
+	/* Mirrored to both peers; paired with WRITE_ONCE() in veth_set_coalesce */
+	bql_flush_ns = (u64)READ_ONCE(priv->tx_coal_usecs) * 1000;
+
+	/* Clamp stored timestamp in case we migrated to a CPU with a behind
+	 * sched_clock(); tries to reduce late BQL flushes.
+	 */
+	state->time = min(state->time, sched_clock());
+
+	/* Flush completions that timed out since the previous NAPI poll. */
+	veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);>>
  	for (i = 0; i < budget; i++) {
  		void *ptr = __ptr_ring_consume(&rq->xdp_ring);
@@ -1000,12 +1101,11 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
  			}
  		} else {
  			/* ndo_start_xmit */
-			bool bql_charged = veth_ptr_is_bql(ptr);
  			struct sk_buff *skb = veth_ptr_to_skb(ptr);
 
+			if (veth_ptr_is_bql(ptr))
+				state->n_bql++;
  			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) {
drivers/net/veth.c static int veth_xdp_rcv(struct veth_rq *rq, int budget, · line 1119
Quoted 5 lines
drivers/net/veth.c +1 -0
@@ -1015,6 +1115,7 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
  					napi_gro_receive(&rq->xdp_napi, skb);
  			}
  		}
+		veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
  		done++;
Sashiko-Nipa reports:
"If veth_xdp_rcv() finishes and returns a done count less than the budget, NAPI will go to sleep in veth_poll(). Do we need to unconditionally flush any stranded BQL completions in veth_poll() before sleeping? If completions are left in rq->bql_state indefinitely across NAPI idle periods, it might present an artificially massive delay to DQL. This could cause DQL to mistakenly conclude the hardware is extremely slow and aggressively shrink dql.limit to its minimum, crippling throughput on subsequent bursts."
Again the issue that I found to be non-problematic in [1] and can be seen by an BQL inflight > 0 when for example pktgen suddenly stops.
If we would "unconditionally flush any stranded BQL completions in veth_poll() before sleeping" we would *not* accumulate BQL completions across NAPI polls but we want to do that.
Do you agree?
[1] https://lore.kernel.org/netdev/[email protected]/

Simon Schippers

2026-06-30 19:07 UTC #10 lore raw

Re: [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs

Quoted 172 lines · Jonas Köppeler

Quoted 162 lines · Simon Schippers

Quoted 161 lines · On 6/12/26 10:35, [email protected] wrote:

From: Simon Schippers <[email protected]>

Per-packet BQL completion forces DQL to converge on limit=2, causing excessive NAPI scheduling overhead and qdisc requeues.

The flush condition is:

state->time + bql_flush_ns <= current_time || state->n_bql > dql.limit

Flushing when n_bql exceeds dql.limit handles BQL starvation.

Add ethtool tx-usecs support for runtime tuning. Default is 100 us; setting tx-usecs to 0 disables coalescing and falls back to per-packet completion.

ethtool -C <veth-dev> tx-usecs 500 # 500us coalescing
ethtool -C <veth-dev> tx-usecs 0 # per-packet (no coalescing)

Co-developed-by Jesper Dangaard Brouer <[email protected]>

Signed-off-by Jesper Dangaard Brouer <[email protected]>

Co-developed-by Jonas Köppeler <[email protected]>

[1] https://lore.kernel.org/netdev/[email protected]/

---
  drivers/net/veth.c | 123 ++++++++++++++++++++++++++++++++++++++++++---
  1 file changed, 117 insertions(+), 6 deletions(-)

drivers/net/veth.c +69 -0

index 2473f730734b..c62d87a8402c 100644
@@ -28,6 +28,7 @@
  #include <linux/bpf_trace.h>
  #include <linux/net_tstamp.h>
  #include <linux/skbuff_ref.h>
+#include <linux/sched/clock.h>
  #include <net/page_pool/helpers.h>
    #define DRV_NAME    "veth"

@@ -50,6 +51,7 @@
   * delay => 64 * 250 ms = 16 s.
   */
  #define VETH_WATCHDOG_TIMEOUT_MS    (64 * 250)
+#define VETH_BQL_COAL_TX_USECS    100 /* default tx-usecs for BQL batching*/
    struct veth_stats {
      u64    rx_drops;

@@ -69,6 +71,11 @@ struct veth_rq_stats {
      struct u64_stats_sync    syncp;
  };
  +struct veth_bql_state {
+    u64    time;    /* sched_clock() when current coalescing window started */
+    uint    n_bql;    /* BQL completions batched in the current window */
+};
+
  struct veth_rq {
      struct napi_struct    xdp_napi;
      struct napi_struct __rcu *napi; /* points to xdp_napi when the latteris initialized */

@@ -76,6 +83,7 @@ struct veth_rq {
      struct bpf_prog __rcu    *xdp_prog;
      struct xdp_mem_info    xdp_mem;
      struct veth_rq_stats    stats;
+    struct veth_bql_state    bql_state;
      bool            rx_notify_masked;
      struct ptr_ring        xdp_ring;
      struct xdp_rxq_info    xdp_rxq;

@@ -88,6 +96,7 @@ struct veth_priv {
      struct bpf_prog        *_xdp_prog;
      struct veth_rq        *rq;
      unsigned int        requested_headroom;
+    unsigned int        tx_coal_usecs;    /* BQL completion coalescing */
  };
    struct veth_xdp_tx_bq {

@@ -272,7 +281,56 @@ static void veth_get_channels(struct net_device *dev,
  static int veth_set_channels(struct net_device *dev,
                   struct ethtool_channels *ch);
  +static int veth_get_coalesce(struct net_device *dev,
+                 struct ethtool_coalesce *ec,
+                 struct kernel_ethtool_coalesce *kernel_coal,
+                 struct netlink_ext_ack *extack)
+{
+    struct veth_priv *priv = netdev_priv(dev);
+
+    ec->tx_coalesce_usecs = priv->tx_coal_usecs;
+    return 0;
+}
+
+static int veth_set_coalesce(struct net_device *dev,
+                 struct ethtool_coalesce *ec,
+                 struct kernel_ethtool_coalesce *kernel_coal,
+                 struct netlink_ext_ack *extack)
+{
+    struct veth_priv *priv = netdev_priv(dev);
+    struct net_device *peer;
+
+    /* The coalescing window delays BQL completions, so keep tx-usecs well
+     * below the tx_timeout watchdog; otherwise a large value could stall a
+     * stopped queue long enough to trip a false watchdog timeout. Cap at
+     * half the watchdog to leave a generous safety margin. tx-usecs is
+     * microseconds, the watchdog is milliseconds.
+     */
+    if (ec->tx_coalesce_usecs > VETH_WATCHDOG_TIMEOUT_MS / 2 * USEC_PER_MSEC) {
+        NL_SET_ERR_MSG_MOD(extack,
+                   "tx-usecs must stay below half the tx_timeout watchdog");
+        return -ERANGE;
+    }
+
+    /* Paired with READ_ONCE in veth_xdp_rcv(). */
+    WRITE_ONCE(priv->tx_coal_usecs, ec->tx_coalesce_usecs);
+
+    /* veth_xdp_rcv() reads each device's own value, so mirror it onto
+     * the peer to keep the pair symmetric: both directions coalesce
+     * with the same tx-usecs. Called under RTNL, rtnl_dereference() is safe.
+     */
+    peer = rtnl_dereference(priv->peer);
+    if (peer) {
+        struct veth_priv *peer_priv = netdev_priv(peer);
+
+        WRITE_ONCE(peer_priv->tx_coal_usecs, ec->tx_coalesce_usecs);
+    }
+
+    return 0;
+}
+
  static const struct ethtool_ops veth_ethtool_ops = {
+    .supported_coalesce_params = ETHTOOL_COALESCE_TX_USECS,
      .get_drvinfo        = veth_get_drvinfo,
      .get_link        = ethtool_op_get_link,
      .get_strings        = veth_get_strings,

@@ -282,6 +340,8 @@ static const struct ethtool_ops veth_ethtool_ops ={
      .get_ts_info        = ethtool_op_get_ts_info,
      .get_channels        = veth_get_channels,
      .set_channels        = veth_set_channels,
+    .get_coalesce        = veth_get_coalesce,
+    .set_coalesce        = veth_set_coalesce,
  };
    /* general routines */

@@ -969,13 +1029,54 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
      return NULL;
  }
  +static void veth_bql_maybe_complete(struct veth_bql_state *state,
+                    struct netdev_queue *peer_txq,
+                    u64 bql_flush_ns)
+{
+    u64 current_time;
+
+    /* There is no reason to complete with 0 and
+     * peer_txq could go away.
+     */
+    if (!state->n_bql || !peer_txq)
+        return;
+

Quoted 9 lines

drivers/net/veth.c static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq, · line 1051

Quoted 8 lines

drivers/net/veth.c +8 -0

+    current_time = sched_clock();
+
+    /* We complete if:
+     * 1. We reach bql_flush_ns.
+     * 2. We potentially have BQL starvation.
+     */
+    if (state->time + bql_flush_ns <= current_time ||
+        state->n_bql > peer_txq->dql.limit) {

Quoted 1 line

Indeed, this does not compile when CONFIG_BQL is not set. I think we should just bring back the 'queue is empty + queue is stopped' check from v6 back at the end of the poll and remove the n_bql > dql.limit check.

We would put #ifdef CONFIG_BQL around that logic aswell.

Quoted 1 line

It also feels not obvious why this is handling the starvation case. This only works, because the producer has went overlimit previously and was stopped. So more than 'limit' packets have been enqueued to the ring, and they are eventually drained when this check is true.

I think it just needs some comment tweaking:

/* We complete if:
* 1. We reach bql_flush_ns.
* 2. We have BQL starvation. This means that the queue was over-limit
* in the last interval, and there is no more data in the queue,
* which is equivalent to we consumed more than limit items.
*/

Quoted 1 line

By removing this we can also avoid accessing dql internal members, but if you don't think that's a problem we can leave as is.

I agree accessing dql internal variables is not perfect.

That is why I have locally implemented DQL for software interfaces in a generic way inside dynamic_queue_limits.{h,c}.
I was able to squeeze the time and n_bql variables into the completion cacheline of the dql struct by moving around variables.
The logic applies inside dql_completed() if enabled.
With this we just have to call netdev_completed_queue(). Also it allows for per-queue tweaking of tx_usecs via sysfs. Works well for me, can share it if we want to use it.

Quoted 2 lines

Further, this is only works if VETH_BQL_UNIT stays 1, otherwise it will never fire. Anyway, still its necessary to check for CONFIG_BQL. But we could solve this by adding VETH_BQL_UNIT to n_bql instead of 1. This is also safe from any overflows, since limit is bound to limit_max, inflight is always less than limit + 1*VETH_BQL_UNIT and n_bql <= inflight.

You are right.

But I think there is no reason for VETH_BQL_UNIT anyway. There should be no difference in the BQL algorithm, I personally would replace VETH_BQL_UNIT with a hard-coded 1.

Quoted 33 lines

In a version of bringing back the 'queue-empty' check and keeping most of the current logic (so a mixture of v6 and v7) resulted in the same performance on an x86_64 architecture.
Quoted 4 lines
Both Sashiko-Nipa and Sashiko-Gemini are right, this is missing a #ifdef CONFIG_BQL. Not sure what is the best way to add them. And for the struct we could maybe do:
Quoted 8 lines
#ifdef CONFIG_BQL
struct veth_bql_state {
u64 time; /* sched_clock() when current coalescing window started */
uint n_bql; /* BQL completions batched in the current window */
};
#else
struct veth_bql_state {};
#endif
Regarding the configs: we can just do something along those lines. struct veth_rq {
...
#ifdef CONFIG_BQL
    struct veth_bql_state        dql;
#endif
...
}
and we put the rest of the code that accesses or performs an action regarding bql in some functions and do it like in netdev_* functions with
Function-Signature()
{
#ifdef CONFIG_BQL
// Code
#endif
}

Quoted 2 lines

Wdyt?
- Jonas

Yes, we have to. Unless we put it into dynamic_queue_limits.{h,c} of course :^)

Thanks,
Simon

Quoted 73 lines

Quoted 72 lines

Quoted 45 lines

drivers/net/veth.c +22 -3

+        netdev_tx_completed_queue(peer_txq, state->n_bql,
+                      state->n_bql * VETH_BQL_UNIT);
+        state->time = current_time;
+        state->n_bql = 0;
+    }
+}
+
  static int veth_xdp_rcv(struct veth_rq *rq, int budget,
              struct veth_xdp_tx_bq *bq,
              struct veth_stats *stats,
              struct netdev_queue *peer_txq)
  {
+    struct veth_priv *priv = netdev_priv(rq->dev);
+    struct veth_bql_state *state = &rq->bql_state;
      int i, done = 0, n_xdpf = 0;
      void *xdpf[VETH_XDP_BATCH];
+    u64 bql_flush_ns;
+
+    /* Mirrored to both peers; paired with WRITE_ONCE() in veth_set_coalesce */
+    bql_flush_ns = (u64)READ_ONCE(priv->tx_coal_usecs) * 1000;
+
+    /* Clamp stored timestamp in case we migrated to a CPU with a behind
+     * sched_clock(); tries to reduce late BQL flushes.
+     */
+    state->time = min(state->time, sched_clock());
+
+    /* Flush completions that timed out since the previous NAPI poll. */
+    veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);>>
      for (i = 0; i < budget; i++) {
          void *ptr = __ptr_ring_consume(&rq->xdp_ring);

@@ -1000,12 +1101,11 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
              }
          } else {
              /* ndo_start_xmit */
-            bool bql_charged = veth_ptr_is_bql(ptr);
              struct sk_buff *skb = veth_ptr_to_skb(ptr);
  +            if (veth_ptr_is_bql(ptr))
+                state->n_bql++;
              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) {

drivers/net/veth.c static int veth_xdp_rcv(struct veth_rq *rq, int budget, · line 1119

Quoted 5 lines

drivers/net/veth.c +1 -0

@@ -1015,6 +1115,7 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
                      napi_gro_receive(&rq->xdp_napi, skb);
              }
          }
+        veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
          done++;

Sashiko-Nipa reports:

Again the issue that I found to be non-problematic in [1] and can be seen by an BQL inflight > 0 when for example pktgen suddenly stops.

If we would "unconditionally flush any stranded BQL completions in veth_poll() before sleeping" we would *not* accumulate BQL completions across NAPI polls but we want to do that.

Do you agree?

Jonas Köppeler

2026-07-09 10:04 UTC #11 lore raw

Re: [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs

Quoted 246 lines · Simon Schippers

Quoted 172 lines · Jonas Köppeler
Quoted 162 lines · Simon Schippers
Quoted 161 lines · On 6/12/26 10:35, [email protected] wrote:
From: Simon Schippers <[email protected]>
Per-packet BQL completion forces DQL to converge on limit=2, causing excessive NAPI scheduling overhead and qdisc requeues.
Accumulate BQL completions and flush them when a configurable time threshold (tx-usecs) is exceeded, letting DQL discover a limit that bounds actual queuing delay to the configured interval. Coalescing state persists across NAPI polls in struct veth_rq so completions can accumulate beyond a single budget=64 cycle.
The flush condition is:
state->time + bql_flush_ns <= current_time || state->n_bql > dql.limit
Flushing when n_bql exceeds dql.limit handles BQL starvation.
The comparison is strictly greater-than because netdev_tx_sent_queue() always lets the producer exceed the limit by one before it stops, so n_bql == dql.limit is a normal in-flight state. dql.limit lives in the same cacheline as the completion path, so the check is cheap.
Add ethtool tx-usecs support for runtime tuning. Default is 100 us; setting tx-usecs to 0 disables coalescing and falls back to per-packet completion.
ethtool -C <veth-dev> tx-usecs 500 # 500us coalescing
ethtool -C <veth-dev> tx-usecs 0 # per-packet (no coalescing)
Co-developed-by Jesper Dangaard Brouer <[email protected]>
Signed-off-by Jesper Dangaard Brouer <[email protected]>
Co-developed-by Jonas Köppeler <[email protected]>
Signed-off-by Jonas Köppeler <[email protected]>
Signed-off-by Simon Schippers <[email protected]>
---
   drivers/net/veth.c | 123 ++++++++++++++++++++++++++++++++++++++++++---
   1 file changed, 117 insertions(+), 6 deletions(-)
drivers/net/veth.c +69 -0
index 2473f730734b..c62d87a8402c 100644
@@ -28,6 +28,7 @@
   #include <linux/bpf_trace.h>
   #include <linux/net_tstamp.h>
   #include <linux/skbuff_ref.h>
+#include <linux/sched/clock.h>
   #include <net/page_pool/helpers.h>
     #define DRV_NAME    "veth"
@@ -50,6 +51,7 @@
    * delay => 64 * 250 ms = 16 s.
    */
   #define VETH_WATCHDOG_TIMEOUT_MS    (64 * 250)
+#define VETH_BQL_COAL_TX_USECS    100 /* default tx-usecs for BQL batching*/
     struct veth_stats {
       u64    rx_drops;
@@ -69,6 +71,11 @@ struct veth_rq_stats {
       struct u64_stats_sync    syncp;
   };
   +struct veth_bql_state {
+    u64    time;    /* sched_clock() when current coalescing window started */
+    uint    n_bql;    /* BQL completions batched in the current window */
+};
+
   struct veth_rq {
       struct napi_struct    xdp_napi;
       struct napi_struct __rcu *napi; /* points to xdp_napi when the latteris initialized */
@@ -76,6 +83,7 @@ struct veth_rq {
       struct bpf_prog __rcu    *xdp_prog;
       struct xdp_mem_info    xdp_mem;
       struct veth_rq_stats    stats;
+    struct veth_bql_state    bql_state;
       bool            rx_notify_masked;
       struct ptr_ring        xdp_ring;
       struct xdp_rxq_info    xdp_rxq;
@@ -88,6 +96,7 @@ struct veth_priv {
       struct bpf_prog        *_xdp_prog;
       struct veth_rq        *rq;
       unsigned int        requested_headroom;
+    unsigned int        tx_coal_usecs;    /* BQL completion coalescing */
   };
     struct veth_xdp_tx_bq {
@@ -272,7 +281,56 @@ static void veth_get_channels(struct net_device *dev,
   static int veth_set_channels(struct net_device *dev,
                    struct ethtool_channels *ch);
   +static int veth_get_coalesce(struct net_device *dev,
+                 struct ethtool_coalesce *ec,
+                 struct kernel_ethtool_coalesce *kernel_coal,
+                 struct netlink_ext_ack *extack)
+{
+    struct veth_priv *priv = netdev_priv(dev);
+
+    ec->tx_coalesce_usecs = priv->tx_coal_usecs;
+    return 0;
+}
+
+static int veth_set_coalesce(struct net_device *dev,
+                 struct ethtool_coalesce *ec,
+                 struct kernel_ethtool_coalesce *kernel_coal,
+                 struct netlink_ext_ack *extack)
+{
+    struct veth_priv *priv = netdev_priv(dev);
+    struct net_device *peer;
+
+    /* The coalescing window delays BQL completions, so keep tx-usecs well
+     * below the tx_timeout watchdog; otherwise a large value could stall a
+     * stopped queue long enough to trip a false watchdog timeout. Cap at
+     * half the watchdog to leave a generous safety margin. tx-usecs is
+     * microseconds, the watchdog is milliseconds.
+     */
+    if (ec->tx_coalesce_usecs > VETH_WATCHDOG_TIMEOUT_MS / 2 * USEC_PER_MSEC) {
+        NL_SET_ERR_MSG_MOD(extack,
+                   "tx-usecs must stay below half the tx_timeout watchdog");
+        return -ERANGE;
+    }
+
+    /* Paired with READ_ONCE in veth_xdp_rcv(). */
+    WRITE_ONCE(priv->tx_coal_usecs, ec->tx_coalesce_usecs);
+
+    /* veth_xdp_rcv() reads each device's own value, so mirror it onto
+     * the peer to keep the pair symmetric: both directions coalesce
+     * with the same tx-usecs. Called under RTNL, rtnl_dereference() is safe.
+     */
+    peer = rtnl_dereference(priv->peer);
+    if (peer) {
+        struct veth_priv *peer_priv = netdev_priv(peer);
+
+        WRITE_ONCE(peer_priv->tx_coal_usecs, ec->tx_coalesce_usecs);
+    }
+
+    return 0;
+}
+
   static const struct ethtool_ops veth_ethtool_ops = {
+    .supported_coalesce_params = ETHTOOL_COALESCE_TX_USECS,
       .get_drvinfo        = veth_get_drvinfo,
       .get_link        = ethtool_op_get_link,
       .get_strings        = veth_get_strings,
@@ -282,6 +340,8 @@ static const struct ethtool_ops veth_ethtool_ops ={
       .get_ts_info        = ethtool_op_get_ts_info,
       .get_channels        = veth_get_channels,
       .set_channels        = veth_set_channels,
+    .get_coalesce        = veth_get_coalesce,
+    .set_coalesce        = veth_set_coalesce,
   };
     /* general routines */
@@ -969,13 +1029,54 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
       return NULL;
   }
   +static void veth_bql_maybe_complete(struct veth_bql_state *state,
+                    struct netdev_queue *peer_txq,
+                    u64 bql_flush_ns)
+{
+    u64 current_time;
+
+    /* There is no reason to complete with 0 and
+     * peer_txq could go away.
+     */
+    if (!state->n_bql || !peer_txq)
+        return;
+
Quoted 9 lines
drivers/net/veth.c static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq, · line 1050
Quoted 8 lines
drivers/net/veth.c +8 -0
+    current_time = sched_clock();
+
+    /* We complete if:
+     * 1. We reach bql_flush_ns.
+     * 2. We potentially have BQL starvation.
+     */
+    if (state->time + bql_flush_ns <= current_time ||
+        state->n_bql > peer_txq->dql.limit) {
Quoted 1 line
Indeed, this does not compile when CONFIG_BQL is not set. I think we should just bring back the 'queue is empty + queue is stopped' check from v6 back at the end of the poll and remove the n_bql > dql.limit check.
We would put #ifdef CONFIG_BQL around that logic aswell.
Quoted 1 line
It also feels not obvious why this is handling the starvation case. This only works, because the producer has went overlimit previously and was stopped. So more than 'limit' packets have been enqueued to the ring, and they are eventually drained when this check is true.
I think it just needs some comment tweaking:
/* We complete if:
* 1. We reach bql_flush_ns.
* 2. We have BQL starvation. This means that the queue was over-limit
* in the last interval, and there is no more data in the queue,
* which is equivalent to we consumed more than limit items.
*/
Quoted 1 line
By removing this we can also avoid accessing dql internal members, but if you don't think that's a problem we can leave as is.
I agree accessing dql internal variables is not perfect.
That is why I have locally implemented DQL for software interfaces in a generic way inside dynamic_queue_limits.{h,c}.
I was able to squeeze the time and n_bql variables into the completion cacheline of the dql struct by moving around variables.
The logic applies inside dql_completed() if enabled.
With this we just have to call netdev_completed_queue(). Also it allows for per-queue tweaking of tx_usecs via sysfs. Works well for me, can share it if we want to use it.
Quoted 2 lines
Further, this is only works if VETH_BQL_UNIT stays 1, otherwise it will never fire. Anyway, still its necessary to check for CONFIG_BQL. But we could solve this by adding VETH_BQL_UNIT to n_bql instead of 1. This is also safe from any overflows, since limit is bound to limit_max, inflight is always less than limit + 1*VETH_BQL_UNIT and n_bql <= inflight.
You are right.
But I think there is no reason for VETH_BQL_UNIT anyway. There should be no difference in the BQL algorithm, I personally would replace VETH_BQL_UNIT with a hard-coded 1.
Quoted 33 lines
In a version of bringing back the 'queue-empty' check and keeping most of the current logic (so a mixture of v6 and v7) resulted in the same performance on an x86_64 architecture.
Quoted 4 lines
Both Sashiko-Nipa and Sashiko-Gemini are right, this is missing a #ifdef CONFIG_BQL. Not sure what is the best way to add them. And for the struct we could maybe do:
Quoted 8 lines
#ifdef CONFIG_BQL
struct veth_bql_state {
u64 time; /* sched_clock() when current coalescing window started */
uint n_bql; /* BQL completions batched in the current window */
};
#else
struct veth_bql_state {};
#endif
Regarding the configs: we can just do something along those lines. struct veth_rq {
...
#ifdef CONFIG_BQL
     struct veth_bql_state        dql;
#endif
...
}
and we put the rest of the code that accesses or performs an action regarding bql in some functions and do it like in netdev_* functions with
Function-Signature()
{
#ifdef CONFIG_BQL
// Code
#endif
}
Quoted 2 lines
Wdyt?
- Jonas

Quoted 6 lines

Yes, we have to. Unless we put it into dynamic_queue_limits.{h,c} of course :^)
Thanks,
Simon
I did implement the CONFIG_BQL guard, and reordered the completion call,

dropping the pre-loop completion call and moved the in-loop completion call in front of the packet processing. I think we can drop one of the completion calls, since the there is only a difference of one packet more or less that is completed.

Two patches on top of 5/5, inline below as RFC (not for application):

1) veth: simplify BQL completion condition
2) veth: Add CONFIG_BQL guards

Patch 1 is the "bring back the queue-empty check" idea: it drops the state->n_bql > dql.limit test and splits the flush into (a) the time-based completion, kept per packet, and (b) an explicit post-loop "ring drained + peer stopped" wake. So veth no longer touches dql.limit. Basically the same we had in v6.

However, if we like we can just replace it in the eth_bql_flush_starved with the barrier-free alternative:

	if (peer_txq &&
	    (u64)state->n_bql * VETH_BQL_UNIT > peer_txq->dql.limit)
		veth_bql_complete(...);

Once the producer is stopped, num_queued/num_completed/limit are frozen, so as the ring drains n_bql rises to inflight and this becomes true exactly when the ring empties under backpressure — same event, no barrier.

So there are three options how to handle this case:

a) barrier + STACK_XOFF (patch 1 as posted).

b) n_bql * VETH_BQL_UNIT > dql.limit.

c) Simon's generic DQL-for-software-interfaces in

     dynamic_queue_limits.{h,c}

I do not have a strong opinion. For a and b I could not measure any performance difference.

Patch 2 does the CONFIG_BQL wrapping the way I sketched (BQL-only helpers with no-op stubs so veth_xdp_rcv()/teardown stay ifdef-free) and rejects ethtool -C tx-usecs with -EOPNOTSUPP when BQL is compiled out.

Performance of !CONFIG_BQL+v7+patch-2 and net-next/main is the same.

Full diffs below.

- Jonas

---8<--- patch 1 ---8<---

From: =?UTF-8?q?Jonas=20K=C3=B6ppeler?= <[email protected]>
Date: Sun, 28 Jun 2026 10:26:15 +0200
Subject: [PATCH] veth: simplify BQL completion condition
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The previous patch flushed batched BQL completions when either the coalescing window elapsed or the batch grew past the DQL limit (state->n_bql > peer_txq->dql.limit). The latter test reaches into DQL internals and is unit-fragile: it compares the raw count n_bql against dql.limit, dropping the VETH_BQL_UNIT factor of the charge (n_bql * VETH_BQL_UNIT), so it only holds because that unit is 1.

Replace that test. The time-based completion stays per packet in veth_xdp_rcv(), issued before veth_xdp_rcv_skb() so the producer wake overlaps with the first skb processing. The wake-a-stalled-producer case becomes an explicit post-loop block: once the ring has drained, if the peer TX queue is stopped by BQL backpressure (STACK_XOFF), release the batched completions to unblock it. DRV_XOFF is left to the existing wake in veth_poll().

Reading STACK_XOFF after the drain needs an smp_rmb(): the producer sets
STACK_XOFF before publishing into the ring, so a consumer on another CPU
that observed the packet must order its ring read ahead of the state
read, or it may read a stale, un-stopped state and drop the wakeup. Pairs with the set_bit()/smp_wmb() on the producer side.

[1] https://lore.kernel.org/netdev/[email protected]/

---
 drivers/net/veth.c | 53 ++++++++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 18 deletions(-)

drivers/net/veth.c +35 ~~-18~~

index c62d87a8402c..2963f190988f 100644
@@ -1029,11 +1029,21 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
 	return NULL;
 }
 
+static void veth_bql_complete(struct veth_bql_state *state,
+			      struct netdev_queue *peer_txq,
+			      u64 now)
+{
+	netdev_tx_completed_queue(peer_txq, state->n_bql,
+				  state->n_bql * VETH_BQL_UNIT);
+	state->time = now;
+	state->n_bql = 0;
+}
+
 static void veth_bql_maybe_complete(struct veth_bql_state *state,
 				    struct netdev_queue *peer_txq,
 				    u64 bql_flush_ns)
 {
-	u64 current_time;
+	u64 now;
 
 	/* There is no reason to complete with 0 and
 	 * peer_txq could go away.

@@ -1041,19 +1051,12 @@ static void veth_bql_maybe_complete(struct veth_bql_state *state,
 	if (!state->n_bql || !peer_txq)
 		return;
 
-	current_time = sched_clock();
-
-	/* We complete if:
-	 * 1. We reach bql_flush_ns.
-	 * 2. We potentially have BQL starvation.
+	/* Release the batched completions once the coalescing window has
+	 * elapsed.
 	 */
-	if (state->time + bql_flush_ns <= current_time ||
-	    state->n_bql > peer_txq->dql.limit) {
-		netdev_tx_completed_queue(peer_txq, state->n_bql,
-					  state->n_bql * VETH_BQL_UNIT);
-		state->time = current_time;
-		state->n_bql = 0;
-	}
+	now = sched_clock();
+	if (state->time + bql_flush_ns <= now)
+		veth_bql_complete(state, peer_txq, now);
 }
 
 static int veth_xdp_rcv(struct veth_rq *rq, int budget,

@@ -1075,9 +1078,6 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 	 */
 	state->time = min(state->time, sched_clock());
 
-	/* Flush completions that timed out since the previous NAPI poll. */
-	veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
-
 	for (i = 0; i < budget; i++) {
 		void *ptr = __ptr_ring_consume(&rq->xdp_ring);

@@ -1105,8 +1105,12 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 
 			if (veth_ptr_is_bql(ptr))
 				state->n_bql++;
-			stats->xdp_bytes += skb->len;
+			/* Complete before processing so the producer wakes
+			 * sooner; ring-empty case handled after the loop.
+			 */
+			veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
 
+			stats->xdp_bytes += skb->len;
 			skb = veth_xdp_rcv_skb(rq, skb, bq, stats);
 			if (skb) {
 				if (skb_shared(skb) || skb_unclone(skb, GFP_ATOMIC))

@@ -1115,13 +1119,26 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
 					napi_gro_receive(&rq->xdp_napi, skb);
 			}
 		}
-		veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
 		done++;
 	}
 
 	if (n_xdpf)
 		veth_xdp_rcv_bulk_skb(rq, xdpf, n_xdpf, bq, stats);
 
+	/* If the ring drained and the peer TX queue is stalled by BQL
+	 * backpressure (STACK_XOFF), release the batched completions now to
+	 * unblock the producer. DRV_XOFF is handled by the wake in veth_poll().
+	 */
+	if (peer_txq && state->n_bql && __ptr_ring_empty(&rq->xdp_ring)) {
+		/* The consume above observed the producer's publish; order it
+		 * before reading STACK_XOFF. Pairs with the smp_wmb() and XOFF
+		 * set_bit() on the producer side.
+		 */
+		smp_rmb();
+		if (test_bit(__QUEUE_STATE_STACK_XOFF, &peer_txq->state))
+			veth_bql_complete(state, peer_txq, sched_clock());
+	}
+
 	u64_stats_update_begin(&rq->stats.syncp);
 	rq->stats.vs.xdp_redirect += stats->xdp_redirect;
 	rq->stats.vs.xdp_bytes += stats->xdp_bytes;

Signature · 34 lines

2.53.0

---8<--- patch 2 ---8<---

From: =?UTF-8?q?Jonas=20K=C3=B6ppeler?= <[email protected]>
Date: Thu, 2 Jul 2026 10:13:02 +0000
Subject: [PATCH] veth: Add CONFIG_BQL guards
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Wrap the BQL-only code under CONFIG_BQL and expose it to the driver
through a small set of helpers with no-op stubs for the !CONFIG_BQL
case, so veth_xdp_rcv() and the NAPI teardown path stay free of ifdefs:

  veth_bql_poll_prepare()   per-poll setup: clamps the coalescing window
                            timestamp and returns the interval length, so
                            !CONFIG_BQL builds never read tx_coal_usecs
  veth_bql_account()        counts a consumed BQL-tagged skb and releases
                            the batch once the interval window has elapsed
  veth_bql_flush_starved()  releases the batch early when the ring has
                            drained and the peer txq is stopped by BQL
                            backpressure (STACK_XOFF)
  veth_bql_state_init()
  veth_bql_drain_and_reset() per-queue state setup and teardown

Since tx-usecs only batches BQL completions, reject ethtool -C tx-usecs
with -EOPNOTSUPP when BQL is compiled out instead of silently storing an
inert value.

Signed-off-by: Jonas Köppeler <[email protected]>
---
 drivers/net/veth.c | 173 +++++++++++++++++++++++++++++----------------
 1 file changed, 111 insertions(+), 62 deletions(-)

drivers/net/veth.c +111 ~~-62~~

264 lines not on this page · raw message

Signature · 1 line

2.53.0

Quoted 73 lines

Quoted 72 lines

Quoted 45 lines

drivers/net/veth.c +22 -3

+        netdev_tx_completed_queue(peer_txq, state->n_bql,
+                      state->n_bql * VETH_BQL_UNIT);
+        state->time = current_time;
+        state->n_bql = 0;
+    }
+}
+
   static int veth_xdp_rcv(struct veth_rq *rq, int budget,
               struct veth_xdp_tx_bq *bq,
               struct veth_stats *stats,
               struct netdev_queue *peer_txq)
   {
+    struct veth_priv *priv = netdev_priv(rq->dev);
+    struct veth_bql_state *state = &rq->bql_state;
       int i, done = 0, n_xdpf = 0;
       void *xdpf[VETH_XDP_BATCH];
+    u64 bql_flush_ns;
+
+    /* Mirrored to both peers; paired with WRITE_ONCE() in veth_set_coalesce */
+    bql_flush_ns = (u64)READ_ONCE(priv->tx_coal_usecs) * 1000;
+
+    /* Clamp stored timestamp in case we migrated to a CPU with a behind
+     * sched_clock(); tries to reduce late BQL flushes.
+     */
+    state->time = min(state->time, sched_clock());
+
+    /* Flush completions that timed out since the previous NAPI poll. */
+    veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);>>
       for (i = 0; i < budget; i++) {
           void *ptr = __ptr_ring_consume(&rq->xdp_ring);

@@ -1000,12 +1101,11 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
               }
           } else {
               /* ndo_start_xmit */
-            bool bql_charged = veth_ptr_is_bql(ptr);
               struct sk_buff *skb = veth_ptr_to_skb(ptr);
   +            if (veth_ptr_is_bql(ptr))
+                state->n_bql++;
               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) {

drivers/net/veth.c static int veth_xdp_rcv(struct veth_rq *rq, int budget, · line 1119

Quoted 5 lines

drivers/net/veth.c +1 -0

@@ -1015,6 +1115,7 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
                       napi_gro_receive(&rq->xdp_napi, skb);
               }
           }
+        veth_bql_maybe_complete(state, peer_txq, bql_flush_ns);
           done++;

Sashiko-Nipa reports:

Again the issue that I found to be non-problematic in [1] and can be seen by an BQL inflight > 0 when for example pktgen suddenly stops.

If we would "unconditionally flush any stranded BQL completions in veth_poll() before sleeping" we would *not* accumulate BQL completions across NAPI polls but we want to do that.

Do you agree?

Simon Schippers

2026-06-12 14:11 UTC #12 lore raw

[1] https://github.com/simoschip2000/veth-backpressure-performance-testing/blob/v7/results/tx-usecs/text_sweep.txt

Quoted 5 lines · On 6/12/26 10:35, [email protected] wrote:

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.

LGTM, thanks for the detailed changelog :)

Maybe we should stop searching for the perfect tx-usecs value. 100us is probably fine for most hardware to not have a performance regression. And lowering it does not really improve the RTT anyways. Do you agree?

Nevertheless, I will compile and run the benchmarks again.

I will go on vacation from 15th to 24th of June, so I will not be able to contribute code or run benchmarks then.

Thanks,
Simon

Jonas Köppeler

2026-06-12 17:21 UTC #13 lore raw

Quoted 7 lines · Simon Schippers

Quoted 5 lines · On 6/12/26 10:35, [email protected] wrote:
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.

Quoted 6 lines

LGTM, thanks for the detailed changelog :)
Maybe we should stop searching for the perfect tx-usecs value. 100us is probably fine for most hardware to not have a performance regression. And lowering it does not really improve the RTT anyways. Do you agree?

I agree, I already thought that it just might be a very lucky case when using 50us where something accidentally aligns nicely. Interestingly, I could also reproduce that 50us was consistently a little better compared to 100us on an Intel CPU. Maybe if I get the time, I'll have another look at it, but in general I think 50us or 100us does not really matter.

Quoted 9 lines

Nevertheless, I will compile and run the benchmarks again.
I will go on vacation from 15th to 24th of June, so I will not be able to contribute code or run benchmarks then.
Thanks,
Simon

Simon Schippers

2026-06-13 13:58 UTC #14 lore raw

Quoted 8 lines · Jonas Köppeler

Quoted 7 lines · Simon Schippers
Quoted 5 lines · On 6/12/26 10:35, [email protected] wrote:
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.

Quoted 8 lines

Quoted 6 lines
LGTM, thanks for the detailed changelog :)
Maybe we should stop searching for the perfect tx-usecs value. 100us is probably fine for most hardware to not have a performance regression. And lowering it does not really improve the RTT anyways. Do you agree?
I agree, I already thought that it just might be a very lucky case when using 50us where something accidentally aligns nicely. Interestingly, I could also reproduce that 50us was consistently a little better compared to 100us on an Intel CPU. Maybe if I get the time, I'll have another look at it, but in general I think 50us or 100us does not really matter.

Interesting.

I ran the benchmarks again, the results are at [1].
I tested values between 0-9us, 10-90us, 100, 500, 1000, 5000 and 10000us.

TLDR: Throughput is fine for everything > 0us. RTT only improves
slightly for < 100us. So 100us is fine.

Quoted 10 lines

Quoted 9 lines
Nevertheless, I will compile and run the benchmarks again.
I will go on vacation from 15th to 24th of June, so I will not be able to contribute code or run benchmarks then.
Thanks,
Simon

Jakub Kicinski

maintainer

2026-06-16 01:53 UTC #15 lore raw

Quoted 1 line · On Fri, 12 Jun 2026 10:35:23 +0200 [email protected] wrote:

Subject: [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support

I'm calling it a day and dropping all remaining net-next patches from pw since the merge window has started, sorry. You know the drill...

Signature · 1 line

pw-bot: defer

Simon Schippers

Tested-by Jonas Köppeler Tested-by Jonas Köppeler Tested-by Jonas Köppeler on the v7 series

2026-08-10 13:26 UTC #16 lore raw

Quoted 5 lines · On 6/12/26 10:35, [email protected] wrote:

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.

Hi :)

I worked on my implementation of DQL coalescing that lives in dynamic_queue_limits.{h,c} and wanted to share it so we can consider it for the next cycle. This is because in the next cycle I would like to add BQL support for tun/tap as well, in addition to veth. Is that fine for you?

I think it is in good shape. It uses the same logic as the v7, but every new field fits inside the existing dql struct, and drivers only need to call the usual netdev_tx_sent_queue() and netdev_tx_completed_queue() to use it. Patch 3, 5 and 6 are the same as before, only 1, 2 and 4 are new. Benchmarks looked fine for me.

There should be no regressions for other DQL/BQL users. I paid close attention not to break the dql cache lines or other logic. coal_usecs is now configurable per queue via sysfs and also via ethtool as usual.

While working on this, I found a missing barrier in v7:
There was no smp_rmb() pairing the smp_wmb() in __ptr_ring_produce() before dql_completed() reads dql->num_queued. This happens to be safe on x86, but on other platforms the read of dql->num_queued could be reordered before __ptr_ring_consume(), triggering a BUG_ON() in dql_completed(). Fixed by adding the missing smp_rmb() in veth_xdp_rcv() before completing.

Would love to hear your thoughts on the implementation!

Thanks,
Simon

From 1a6d2f0a15c23bb4ebb44139193c2649de1142ad Mon Sep 17 00:00:00 2001
From: Simon Schippers <[email protected]>
Date: Thu, 6 Aug 2026 16:17:35 +0200
Subject: [PATCH net-next v8 1/6] net: dql: Add completion coalescing for
software interfaces

Software interfaces like veth or tun have no hardware completion interrupt and consume packets one at a time. Calling netdev_tx_completed_queue() per packet makes DQL converge on a limit of two packets, which is a regression:
BQL sizes the limit from how much completes per call, so a caller that reports a single object every time forces such a limit. Add an optional coalescing window so such a queue can batch completions and report them periodically instead.

The window is set per queue with dql_set_coal_usecs(). While it is non-zero, dql_completed() accumulates the count in coal_pending and returns false instead of recalculating the limit, until the window has elapsed or the batch has grown beyond the limit. That condition is evaluated before any field of the first cache line is read, so a call that only batches does not touch that line at all.

A held-back batch cannot stall the queue. dql_queued() stops the queue once num_queued exceeds adj_limit, so a stopped queue has at least limit + 1 objects in flight. Draining it makes coal_pending exceed the limit and forces a flush on the call that completes the last one. A batch can therefore only defer a wake-up that is not needed yet.

Coalescing is active when coal_usecs or coal_pending is set, rather than being controlled by a separate enable flag, so no enable state can be toggled out from under a pending batch. The second term keeps an outstanding batch reachable after the window is set back to 0. Without it those objects would never be folded into num_completed and the queue would stall.

Timestamps come from local_clock(), which unlike raw sched_clock() is bounded across CPUs. They are kept in units of 1024 ns to avoid a division, so the window is ~2.4% longer than configured, and the u32 wraps every ~73 minutes, which unsigned subtraction handles. A CPU migration can move the timestamp backwards within local_clock()'s drift bound and flush one batch early. That is rare and only costs a little batching. The clock is read only once there is something pending, so a caller that flushes a queue with an empty batch does not pay for it.

Room for the three new fields comes from moving max_limit and min_limit into the enqueue cache line, which had eight bytes free, and reading them up front in dql_completed() next to num_queued and stall_thrs. The completion path already touched that line once, so the move adds no cache line touch. Both cache lines are now exactly full on x86_64 and sizeof(struct dql) is unchanged.

---
 include/linux/dynamic_queue_limits.h | 53 ++++++++++++++++++++---
 lib/dynamic_queue_limits.c           | 63 +++++++++++++++++++++++++---
 2 files changed, 104 insertions(+), 12 deletions(-)

include/linux/dynamic_queue_limits.h +47 -6

index 808b1a5102e7..cb7338209246 100644
@@ -10,7 +10,8 @@
  *
  *   1) Objects are queued up to some limit specified as number of objects.
  *   2) Periodically a completion process executes which retires consumed
- *      objects.
+ *      objects, or objects are coalesced, which suits software interfaces
+ *      completing one packet at a time.
  *   3) Starvation occurs when limit has been reached, all queued data has
  *      actually been consumed, but completion processing has not yet run
  *      so queuing new data is blocked.

@@ -39,6 +40,7 @@
 #ifdef __KERNEL__
 
 #include <linux/bitops.h>
+#include <linux/types.h>
 #include <asm/bug.h>
 
 #define DQL_HIST_LEN		4

@@ -53,6 +55,9 @@ struct dql {
 	/* Stall threshold (in jiffies), defined by user */
 	unsigned short	stall_thrs;
 
+	unsigned int	max_limit;		/* Max limit */
+	unsigned int	min_limit;		/* Minimum limit */
+
 	unsigned long	history_head;		/* top 58 bits of jiffies */
 	/* stall entries, a bit per entry */
 	unsigned long	history[DQL_HIST_LEN];

@@ -69,21 +74,38 @@ struct dql {
 	unsigned int	lowest_slack;		/* Lowest slack found */
 	unsigned long	slack_start_time;	/* Time slacks seen */
 
-	/* Configuration */
-	unsigned int	max_limit;		/* Max limit */
-	unsigned int	min_limit;		/* Minimum limit */
 	unsigned int	slack_hold_time;	/* Time to measure slack */
 
 	/* Longest stall detected, reported to user */
 	unsigned short	stall_max;
+
+	/* Completion coalescing window in us, 0 disables coalescing */
+	u16		coal_usecs;
+
 	unsigned long	last_reap;		/* Last reap (in jiffies) */
 	unsigned long	stall_cnt;		/* Number of stalls */
+
+	/* Objects completed but not yet accounted for, held back by
+	 * completion coalescing.
+	 */
+	u32		coal_pending;
+	/* Last flush, local_clock() >> DQL_COAL_NS_TO_USECS_SHIFT */
+	u32		coal_last_flush;
 };
 
 /* Set some static maximums */
 #define DQL_MAX_OBJECT (UINT_MAX / 16)
 #define DQL_MAX_LIMIT ((UINT_MAX / 2) - DQL_MAX_OBJECT)
 
+/* local_clock() is in ns.  Shifting by 10 (dividing by 1024) instead of
+ * dividing by NSEC_PER_USEC (1000) is close enough for a coalescing window and
+ * avoids a division, at the cost of a window ~2.4% longer than configured.
+ */
+#define DQL_COAL_NS_TO_USECS_SHIFT	10
+
+/* Maximum coalescing window, bounded by the width of ->coal_usecs */
+#define DQL_COAL_MAX_USECS		U16_MAX
+
 /* Populate the bitmap to be processed later in dql_check_stall() */
 static inline void dql_queue_stall(struct dql *dql)
 {

@@ -149,8 +171,27 @@ static inline int dql_avail(const struct dql *dql)
 	return READ_ONCE(dql->adj_limit) - READ_ONCE(dql->num_queued);
 }
 
-/* Record number of completed objects and recalculate the limit. */
-void dql_completed(struct dql *dql, unsigned int count);
+/* Set the completion coalescing window in us, 0 disables coalescing. */
+static inline void dql_set_coal_usecs(struct dql *dql, unsigned int usecs)
+{
+	if (WARN_ON_ONCE(usecs > DQL_COAL_MAX_USECS))
+		usecs = DQL_COAL_MAX_USECS;
+
+	WRITE_ONCE(dql->coal_usecs, usecs);
+}
+
+/* Return the completion coalescing window in us, 0 if disabled. */
+static inline unsigned int dql_get_coal_usecs(const struct dql *dql)
+{
+	return READ_ONCE(dql->coal_usecs);
+}
+
+/* Record number of completed objects and recalculate the limit.
+ *
+ * Returns true if the completion was applied and the limit recalculated, false
+ * if it was only batched by completion coalescing.
+ */
+bool dql_completed(struct dql *dql, unsigned int count);
 
 /* Reset dql state */
 void dql_reset(struct dql *dql);

lib/dynamic_queue_limits.c +57 -6

108 lines not on this page · raw message

base-commit: 001b5d347d8ba39b2dccaefcc57967b18caec8fe

Signature · 1 line

2.43.0

From ad97ecb1ea25a16bd61549e95dce1a86b4efcd7e Mon Sep 17 00:00:00 2001
From: Simon Schippers <[email protected]>
Date: Thu, 6 Aug 2026 16:18:52 +0200
Subject: [PATCH net-next v8 2/6] net: bql: Provide support for dql coalescing
for software interfaces

Make the DQL coalescing window from the previous commit usable by BQL.

netdev_tx_completed_queue() now returns false early if the completion was only batched by DQL coalescing, avoiding the memory barrier and the wake-up. netdev_txq_completed_mb() is adjusted accordingly and issues the smp_mb() itself if no completion was applied.

netif_set_bql_coalesce_usecs() sets the window on every TX queue of a device, both at setup and later on a live device, and netif_get_bql_coalesce_usecs() reads it back from queue 0 for drivers that want to report it. Both compile away to nothing when CONFIG_BQL is off, so a driver does not need its own guard. The window can also be read and written per queue as byte_queue_limits/coal_usecs, where values above DQL_COAL_MAX_USECS are rejected with -EINVAL.

---
 .../ABI/testing/sysfs-class-net-queues        | 15 ++++
 include/linux/netdevice.h                     | 73 +++++++++++++++++--
 include/net/netdev_queues.h                   |  4 +-
 net/core/net-sysfs.c                          | 29 ++++++++
 4 files changed, 111 insertions(+), 10 deletions(-)

Documentation/ABI/testing/sysfs-class-net-queues +15 -0

index 84aa25e0d14d..4a0413d353a9 100644
@@ -97,6 +97,21 @@ Description:
 		queued on this network device transmit queue. Default value is
 		0.
 
+What:		/sys/class/net/<iface>/queues/tx-<queue>/byte_queue_limits/coal_usecs
+Date:		August 2026
+KernelVersion:	7.4
+Contact:	[email protected]
+Description:
+		Completion coalescing window for this transmit queue, in
+		microseconds. While it is non-zero, completions reported to BQL
+		are accumulated and the queue limit is only recalculated once
+		the window has elapsed or the accumulated count would starve the
+		queue. This is meant for software interfaces without a hardware
+		completion interrupt, whose completion routine therefore runs
+		once per packet. Writing 0 disables coalescing and restores a
+		limit recalculation on every completion. Default value is 0,
+		except on devices whose driver sets a window of its own.
+
 What:		/sys/class/net/<iface>/queues/tx-<queue>/byte_queue_limits/stall_thrs
 Date:		Jan 2024
 KernelVersion:	6.9

include/linux/netdevice.h +66 -7

100 lines not on this page · raw message

include/net/netdev_queues.h +1 -3

index 70c9fe9e83cc..c17d0a49c6db 100644
@@ -279,9 +279,7 @@ static inline void
 netdev_txq_completed_mb(struct netdev_queue *dev_queue,
 			unsigned int pkts, unsigned int bytes)
 {
-	if (IS_ENABLED(CONFIG_BQL))
-		netdev_tx_completed_queue(dev_queue, pkts, bytes);
-	else if (bytes)
+	if (!netdev_tx_completed_queue(dev_queue, pkts, bytes) && bytes)
 		smp_mb();
 }

net/core/net-sysfs.c +29 -0

index 25546deacec8..286abb88dad8 100644
@@ -1667,6 +1667,34 @@ BQL_ATTR(limit, limit);
 BQL_ATTR(limit_max, max_limit);
 BQL_ATTR(limit_min, min_limit);
 
+static ssize_t bql_show_coal_usecs(struct kobject *kobj, struct attribute *attr,
+				   struct netdev_queue *queue, char *buf)
+{
+	return sysfs_emit(buf, "%u\n", dql_get_coal_usecs(&queue->dql));
+}
+
+static ssize_t bql_set_coal_usecs(struct kobject *kobj, struct attribute *attr,
+				  struct netdev_queue *queue, const char *buf,
+				  size_t len)
+{
+	unsigned int value;
+	int err;
+
+	err = kstrtouint(buf, 10, &value);
+	if (err < 0)
+		return err;
+
+	if (value > DQL_COAL_MAX_USECS)
+		return -EINVAL;
+
+	dql_set_coal_usecs(&queue->dql, value);
+
+	return len;
+}
+
+static struct netdev_queue_attribute bql_coal_usecs_attribute __ro_after_init
+	= __ATTR(coal_usecs, 0644, bql_show_coal_usecs, bql_set_coal_usecs);
+
 static struct attribute *dql_attrs[] __ro_after_init = {
 	&bql_limit_attribute.attr,
 	&bql_limit_max_attribute.attr,

@@ -1676,6 +1704,7 @@ static struct attribute *dql_attrs[] __ro_after_init = {
 	&bql_stall_thrs_attribute.attr,
 	&bql_stall_cnt_attribute.attr,
 	&bql_stall_max_attribute.attr,
+	&bql_coal_usecs_attribute.attr,
 	NULL
 };

Signature · 1 line

2.43.0

From be9913fa1bd36bec9bf4f38daf33ab51923656ac Mon Sep 17 00:00:00 2001
From: Jesper Dangaard Brouer <[email protected]>
Date: Thu, 6 Aug 2026 16:22:00 +0200
Subject: [PATCH net-next v8 3/6] net: add dev->bql flag to allow BQL sysfs for
IFF_NO_QUEUE devices
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

Signed-off-by Jesper Dangaard Brouer <[email protected]>

---
 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 512f6d6fa3d8..52709375ee21 100644
@@ -168,6 +168,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 36a8d4362d3e..cdd25832357e 100644
@@ -2093,6 +2093,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

@@ -2513,6 +2514,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 286abb88dad8..180065ad7ed9 100644
@@ -1968,10 +1968,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

From 64b7a0a2b41aa3f4d0a639dc86819a1470706118 Mon Sep 17 00:00:00 2001
From: Jesper Dangaard Brouer <[email protected]>
Date: Thu, 6 Aug 2026 16:28:28 +0200
Subject: [PATCH net-next v8 4/6] veth: implement Byte Queue Limits (BQL) for
latency reduction
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Commit dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring to reduce TX drops") let a qdisc see when veth's ptr_ring is full, but noted that the 256-entry ring still sits in front of the qdisc as a dark buffer:
the qdisc cannot shape until the ring overflows. Add BQL so it gets feedback earlier. With fq_codel under UDP load, ping RTT drops from ~6.61ms to ~0.36ms (18x).

Charge one fixed unit per packet instead of skb->len. veth has no link speed -- the ring drains at CPU speed and is packet-indexed. With byte-based charging, small packets get many more entries into the ring before the queue stops, deepening the dark buffer again: a concurrent min-size flood degrades ping RTT by 3.7x with skb->len and not at all with a fixed unit. As a result byte_queue_limits/limit, limit_max and inflight report packets for veth. Nothing in the ABI communicates the unit, so it is stated here.

Charge in veth_xdp_rx() under the ptr_ring producer_lock, once the ring is known not to be full. The charge must precede the produce, because the peer NAPI can complete the skb the moment it becomes visible. Holding the lock across both avoids a pre-charge/undo pattern. veth_xmit() therefore resolves the txq up front and reuses it on the NETDEV_TX_BUSY path. Charged skbs are tagged with VETH_BQL_FLAG in the ptr_ring entry, because the qdisc can be replaced while they are in flight and each skb has to carry the decision made at enqueue time.

Program order is not enough across CPUs. The smp_wmb() in __ptr_ring_produce() publishes the charge ahead of the entry, but dql_completed() reads num_queued, which is not reached through the entry and so is not dependency-ordered by the consume. veth_xdp_rcv() therefore pairs an smp_rmb() with it before completing.

Only enable BQL when a real qdisc is attached (!qdisc_txq_has_no_queue), since dql_queued() needs the serialization of HARD_TX_LOCK, which lltx devices like veth do not take.

veth has no completion interrupt, so veth_xdp_rcv() completes every skb it consumes. Against plain BQL that pays a full dql_completed() per packet and breaks the "at most once per TX completion round" contract, so enable DQL coalescing instead, default VETH_BQL_COAL_USECS (100 us) and tunable with ethtool -C tx-usecs for the direction that device transmits in. With CONFIG_BQL=n that knob reads back 0 and setting it returns -EOPNOTSUPP. Accumulating per NAPI poll in the driver is not equivalent, because a poll is not the periodic interval DQL asks for. Each poll therefore starts with a zero-count completion, so a batch held back at the end of the previous one is not left waiting on traffic that may never arrive.

BQL adds a second queue-stop mechanism (STACK_XOFF) next to the existing ring-full one, and both must be clear for the queue to transmit. At teardown veth_napi_del_range() drains the leftover ring entries after synchronize_net(), once NAPI is gone and the producer has stopped charging because it observes rq->napi == NULL, and balances the accounting by completing the outstanding charges rather than calling netdev_tx_reset_queue(), whose dql_reset() would race with a concurrent producer. The peer txq is still woken to clear any DRV_XOFF a late veth_xmit() may have set.

Signed-off-by Jesper Dangaard Brouer <[email protected]>

Co-developed-by Jonas Köppeler <[email protected]>

Co-developed-by Simon Schippers <[email protected]>

---
 drivers/net/veth.c | 170 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 157 insertions(+), 13 deletions(-)

drivers/net/veth.c +157 ~~-13~~

293 lines not on this page · raw message

Signature · 1 line

2.43.0

From ab29d87b9b357e34b935376e1fe81786672e7310 Mon Sep 17 00:00:00 2001
From: Jesper Dangaard Brouer <[email protected]>
Date: Thu, 6 Aug 2026 16:28:28 +0200
Subject: [PATCH net-next v8 5/6] net: sched: add timeout count to NETDEV
WATCHDOG message
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

Signed-off-by Jesper Dangaard Brouer <[email protected]>

---
 net/sched/sch_generic.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

net/sched/sch_generic.c +4 -2

index ef2b4bf51564..4cbefeab726a 100644
@@ -533,6 +533,7 @@ static void dev_watchdog(struct timer_list *t)
 		    netif_running(dev) &&
 		    netif_carrier_ok(dev)) {
 			unsigned int timedout_ms = 0;
+			unsigned long trans_timeout = 0;
 			unsigned int i;
 			unsigned long trans_start;
 			unsigned long oldest_start = jiffies;

@@ -553,6 +554,7 @@ static void dev_watchdog(struct timer_list *t)
 				if (time_after(jiffies, trans_start + dev->watchdog_timeo)) {
 					timedout_ms = jiffies_to_msecs(jiffies - trans_start);
 					atomic_long_inc(&txq->trans_timeout);
+					trans_timeout = atomic_long_read(&txq->trans_timeout);
 					break;
 				}
 				if (time_after(oldest_start, trans_start))

@@ -561,9 +563,9 @@ 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, trans_timeout);
 				netif_freeze_queues(dev);
 				dev->netdev_ops->ndo_tx_timeout(dev, i);
 				netif_unfreeze_queues(dev);

Signature · 1 line

2.43.0

From f75b304361388b050425d5b3796e5583dccb0d96 Mon Sep 17 00:00:00 2001
From: Jesper Dangaard Brouer <[email protected]>
Date: Thu, 6 Aug 2026 16:34:14 +0200
Subject: [PATCH net-next v8 6/6] veth: add tx_timeout watchdog as BQL safety
net
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.

Signed-off-by Jesper Dangaard Brouer <[email protected]>