Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support
@ 2026-06-12  8:35 hawk
  2026-06-12  8:35 ` [PATCH net-next v7 1/5] net: add dev->bql flag to allow BQL sysfs for IFF_NO_QUEUE devices hawk
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: hawk @ 2026-06-12  8:35 UTC (permalink / raw)
  To: netdev
  Cc: kernel-team, simon.schippers, Jesper Dangaard Brouer,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Chris Arges, Mike Freemon,
	Toke Høiland-Jørgensen, Jonas Köppeler,
	Breno Leitao, Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Stanislav Fomichev, bpf

From: Jesper Dangaard Brouer <hawk@kernel.org>

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

Problem: veth's 256-entry ptr_ring acts as a "dark buffer" invisible
to the qdisc's AQM.  Under load the ring fills, adding up to 256
packets of unmanaged latency before the qdisc sees congestion.

Solution: BQL stops the queue before the ring fills, pushing excess
packets into the qdisc where sojourn-based AQM can drop them.
Time-based completion coalescing (ethtool tx-usecs, default 100 us)
lets DQL converge on a limit that bounds actual queuing delay rather
than oscillating at limit=2 with per-packet completion.

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

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

BQL reduces ping RTT by ~17x for both qdiscs.  Consumer throughput
unchanged.

Why BQL over configurable ring size (ethtool -G): BQL auto-tunes per
workload via DQL; a static ring size requires per-setup manual tuning
and a too-small ring drops XDP packets in batches of 16-64.

Selftests: https://github.com/netoptimizer/veth-backpressure-performance-testing

Background:
  Mike Freemon reported the veth dark buffer problem internally at
  Cloudflare and showed that recompiling with ptr_ring size 30 made
  fq_codel work dramatically better -- motivating a dynamic BQL
  solution.  Chris Arges wrote the reproducer.  Jonas Koeppeler and
  Simon Schippers provided extensive testing and code review.

  During BQL development we also fixed an unrelated 12-year-old CoDel
  bug (stale first_above_time in empty flows), see
  commit 815980fe6dbb ("net_sched: codel: fix stale state for empty flows in fq_codel").
  BQL remains valuable independently.

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

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

Simon Schippers (1):
  veth: time-based BQL completion coalescing via ethtool tx-usecs

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Chris Arges <carges@cloudflare.com>
Cc: Mike Freemon <mfreemon@cloudflare.com>
Cc: Toke Høiland-Jørgensen <toke@toke.dk>
Cc: Jonas Köppeler <j.koeppeler@tu-berlin.de>
Cc: Breno Leitao <leitao@debian.org>
Cc: Simon Schippers <simon.schippers@tu-dortmund.de>
Cc: kernel-team@cloudflare.com

---
Changes since V6:
  - Patch 2 (teardown): rework veth_napi_del_range() to drain the ptr_ring
    and balance the peer txq's BQL/DQL by completing the outstanding charges
    via netdev_tx_completed_queue(), instead of netdev_tx_reset_queue().
    dql_reset() races with a concurrent producer, whereas completion is the
    normal single-completer path and is safe once NAPI is gone
    (synchronize_net()) and the producer has stopped charging BQL (it
    observes rq->napi == NULL).  Fixes races reported by sashiko.  The
    peer txq is still woken to clear any leaked DRV_XOFF.
  - Patch 2: document the lltx/DQL locking model at the BQL charge site --
    veth is lltx so the stack skips HARD_TX_LOCK; the ring producer_lock is
    the single-producer lock for dql_queued() (1:1 with the peer txq) and
    the peer NAPI in veth_xdp_rcv() is the single completer.
  - Patch 3 (watchdog): add a VETH_WATCHDOG_TIMEOUT_MS #define for the
    tx_timeout value instead of an inline magic number, and expand the
    math behind the value (64-packet NAPI budget * 250 ms/pkt = 16 s)
    in a comment (Paolo).
  - Patches 2+5: add Co-developed-by for Jonas Koeppeler.
  - Patch 5 (coalescing): flush when n_bql exceeds dql.limit to handle
    BQL starvation.  Removes the empty-ring STACK_XOFF check (and its
    smp_rmb) -- the dql.limit comparison handles it more directly.
  - Patch 5: at teardown, also complete the coalesced bql_state.n_bql
    pending from the last NAPI poll, on top of the patch 2 ring drain.
  - Patch 5: mirror tx_coal_usecs onto the peer in veth_set_coalesce() so
    a veth pair always coalesces symmetrically; the completion path reads
    its own device's value.
  - Patch 5: use WRITE_ONCE/READ_ONCE for tx_coal_usecs (Paolo).
  - Patch 5: init bql_state before napi_enable() to avoid race (sashiko).
  - Patch 5: tx-usecs=0 handled as normal path, no special case.
  - Patch 5: n_bql type changed to uint; explicit if/++ instead of
    implicit bool-to-int addition.
  - Patch 5: call veth_bql_maybe_complete() on every iteration for
    accurate completion intervals with mixed XDP/non-XDP packets.
  - Patch 5: reject tx-usecs above half the tx_timeout watchdog in
    veth_set_coalesce() (return -ERANGE) -- the coalescing window delays
    BQL completions, so an over-large value could trip a false watchdog;
    half the watchdog leaves a generous margin.

Prior versions:
  V6: https://lore.kernel.org/all/20260527135418.1166665-1-hawk@kernel.org/
  V5: https://lore.kernel.org/all/20260505132159.241305-1-hawk@kernel.org/
  V4: https://lore.kernel.org/all/20260501071633.644353-1-hawk@kernel.org/
  V3: https://lore.kernel.org/all/20260429172036.1028526-1-hawk@kernel.org/
  V2: https://lore.kernel.org/all/20260413094442.1376022-1-hawk@kernel.org/
  V1: https://lore.kernel.org/all/20260324174719.1224337-1-hawk@kernel.org/

Changes since V5:
  - Drop patch 1 (OOB txq fix) -- applied to net tree by Paolo as
    08f566e8f83b, already in net-next.
  - New patch 5: time-based BQL completion coalescing via ethtool
    tx-usecs (Simon Schippers).  Resolves the throughput regression
    Paolo flagged on V5: per-packet completion forced DQL to limit=2,
    causing cache-line bouncing between producer/consumer CPUs.
    Coalescing batches completions on a configurable time threshold
    (default 100 us), letting DQL discover a higher useful limit.
  - Patch 2: use __ptr_ring_check_produce() instead of open-coded
    ring-full check (Simon Schippers nit).

Changes since V4:
  - New patch 1: fix OOB txq access in veth_poll() when veth peers have
    asymmetric RX/TX queue counts.  XDP redirect can deliver frames to
    an RX queue index that exceeds the peer's TX queue count, causing
    an out-of-bounds netdev_get_tx_queue() access.  Found by sashiko-bot.
  - Patch 3 (veth BQL): wake stopped peer txqs in veth_napi_del_range()
    to clear DRV_XOFF after NAPI teardown.  A concurrent veth_xmit()
    can set DRV_XOFF between rcu_assign_pointer(napi, NULL) and
    synchronize_net(); with NAPI gone, no veth_poll() clears it.
    Guarded by netif_running() to skip during device close.

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

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

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

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

-- 
2.43.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next v7 1/5] net: add dev->bql flag to allow BQL sysfs for IFF_NO_QUEUE devices
  2026-06-12  8:35 [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support hawk
@ 2026-06-12  8:35 ` hawk
  2026-06-12  8:35 ` [PATCH net-next v7 2/5] veth: implement Byte Queue Limits (BQL) for latency reduction hawk
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: hawk @ 2026-06-12  8:35 UTC (permalink / raw)
  To: netdev
  Cc: kernel-team, simon.schippers, Jesper Dangaard Brouer,
	Jonas Köppeler, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
	Shuah Khan, Kuniyuki Iwashima, Stanislav Fomichev,
	Christian Brauner, Frederic Weisbecker, Yajun Deng, linux-doc,
	linux-kernel

From: Jesper Dangaard Brouer <hawk@kernel.org>

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

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

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

Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
Tested-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
---
 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(-)

diff --git a/Documentation/networking/net_cachelines/net_device.rst b/Documentation/networking/net_cachelines/net_device.rst
index eb2e6851c6f6..a65d48b6ecc1 100644
--- a/Documentation/networking/net_cachelines/net_device.rst
+++ b/Documentation/networking/net_cachelines/net_device.rst
@@ -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
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7f4f0837c09f..f699fded20b4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -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;
 
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 0e71c9ed41e8..3cb470b0f17d 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -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)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH net-next v7 2/5] veth: implement Byte Queue Limits (BQL) for latency reduction
  2026-06-12  8:35 [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support hawk
  2026-06-12  8:35 ` [PATCH net-next v7 1/5] net: add dev->bql flag to allow BQL sysfs for IFF_NO_QUEUE devices hawk
@ 2026-06-12  8:35 ` hawk
  2026-06-12  8:35 ` [PATCH net-next v7 3/5] veth: add tx_timeout watchdog as BQL safety net hawk
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: hawk @ 2026-06-12  8:35 UTC (permalink / raw)
  To: netdev
  Cc: kernel-team, simon.schippers, Jesper Dangaard Brouer,
	Jonas Köppeler, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
	John Fastabend, Stanislav Fomichev, linux-kernel, bpf

From: Jesper Dangaard Brouer <hawk@kernel.org>

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 <j.koeppeler@tu-berlin.de>
Signed-off-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
---
 drivers/net/veth.c | 131 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 120 insertions(+), 11 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 0cfb19b760dd..a3505627f49e 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -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;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH net-next v7 3/5] veth: add tx_timeout watchdog as BQL safety net
  2026-06-12  8:35 [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support hawk
  2026-06-12  8:35 ` [PATCH net-next v7 1/5] net: add dev->bql flag to allow BQL sysfs for IFF_NO_QUEUE devices hawk
  2026-06-12  8:35 ` [PATCH net-next v7 2/5] veth: implement Byte Queue Limits (BQL) for latency reduction hawk
@ 2026-06-12  8:35 ` hawk
  2026-06-12  8:35 ` [PATCH net-next v7 4/5] net: sched: add timeout count to NETDEV WATCHDOG message hawk
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: hawk @ 2026-06-12  8:35 UTC (permalink / raw)
  To: netdev
  Cc: kernel-team, simon.schippers, Jesper Dangaard Brouer,
	Jonas Köppeler, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel

From: Jesper Dangaard Brouer <hawk@kernel.org>

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 <hawk@kernel.org>
Tested-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
---
 drivers/net/veth.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index a3505627f49e..2473f730734b 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -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;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH net-next v7 4/5] net: sched: add timeout count to NETDEV WATCHDOG message
  2026-06-12  8:35 [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support hawk
                   ` (2 preceding siblings ...)
  2026-06-12  8:35 ` [PATCH net-next v7 3/5] veth: add tx_timeout watchdog as BQL safety net hawk
@ 2026-06-12  8:35 ` hawk
  2026-06-12  8:35 ` [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs hawk
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: hawk @ 2026-06-12  8:35 UTC (permalink / raw)
  To: netdev
  Cc: kernel-team, simon.schippers, Jesper Dangaard Brouer,
	Jakub Kicinski, Jonas Köppeler, Jamal Hadi Salim, Jiri Pirko,
	David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
	linux-kernel

From: Jesper Dangaard Brouer <hawk@kernel.org>

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.

Suggested-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/all/20251107175445.58eba452@kernel.org/
Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
Tested-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
---
 net/sched/sch_generic.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 237ee1cd0136..eb6066d1ed90 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -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);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs
  2026-06-12  8:35 [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support hawk
                   ` (3 preceding siblings ...)
  2026-06-12  8:35 ` [PATCH net-next v7 4/5] net: sched: add timeout count to NETDEV WATCHDOG message hawk
@ 2026-06-12  8:35 ` hawk
  2026-06-13 14:14   ` Simon Schippers
  2026-06-12 14:10 ` [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support Simon Schippers
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: hawk @ 2026-06-12  8:35 UTC (permalink / raw)
  To: netdev
  Cc: kernel-team, simon.schippers, Jesper Dangaard Brouer,
	Jonas Köppeler, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
	John Fastabend, Stanislav Fomichev, linux-kernel, bpf

From: Simon Schippers <simon.schippers@tu-dortmund.de>

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 <hawk@kernel.org>
Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
Co-developed-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
Signed-off-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
 drivers/net/veth.c | 123 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 117 insertions(+), 6 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 2473f730734b..c62d87a8402c 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -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;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support
  2026-06-12  8:35 [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support hawk
                   ` (4 preceding siblings ...)
  2026-06-12  8:35 ` [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs hawk
@ 2026-06-12 14:10 ` Simon Schippers
  2026-06-12 17:21   ` Jonas Köppeler
  2026-06-16  1:53 ` Jakub Kicinski
  2026-08-10 13:25 ` Simon Schippers
  7 siblings, 1 reply; 18+ messages in thread
From: Simon Schippers @ 2026-06-12 14:10 UTC (permalink / raw)
  To: hawk, netdev
  Cc: kernel-team, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Chris Arges, Mike Freemon,
	Toke Høiland-Jørgensen, Jonas Köppeler,
	Breno Leitao, Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Stanislav Fomichev, bpf

On 6/12/26 10:35, hawk@kernel.org wrote:
> From: Jesper Dangaard Brouer <hawk@kernel.org>
> 
> 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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support
  2026-06-12 14:10 ` [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support Simon Schippers
@ 2026-06-12 17:21   ` Jonas Köppeler
  2026-06-13 13:57     ` Simon Schippers
  0 siblings, 1 reply; 18+ messages in thread
From: Jonas Köppeler @ 2026-06-12 17:21 UTC (permalink / raw)
  To: Simon Schippers, hawk, netdev
  Cc: kernel-team, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Chris Arges, Mike Freemon,
	Toke Høiland-Jørgensen, Breno Leitao,
	Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Stanislav Fomichev, bpf

On 6/12/26 16:10, Simon Schippers wrote:
> On 6/12/26 10:35, hawk@kernel.org wrote:
>> From: Jesper Dangaard Brouer <hawk@kernel.org>
>>
>> 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?
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.

> 
> 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
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support
  2026-06-12 17:21   ` Jonas Köppeler
@ 2026-06-13 13:57     ` Simon Schippers
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Schippers @ 2026-06-13 13:57 UTC (permalink / raw)
  To: Jonas Köppeler, hawk, netdev
  Cc: kernel-team, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Chris Arges, Mike Freemon,
	Toke Høiland-Jørgensen, Breno Leitao,
	Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Stanislav Fomichev, bpf

On 6/12/26 19:21, Jonas Köppeler wrote:
> On 6/12/26 16:10, Simon Schippers wrote:
>> On 6/12/26 10:35, hawk@kernel.org wrote:
>>> From: Jesper Dangaard Brouer <hawk@kernel.org>
>>>
>>> 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?
> 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.

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

>>
>> 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
>>
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs
  2026-06-12  8:35 ` [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs hawk
@ 2026-06-13 14:14   ` Simon Schippers
  2026-06-30 14:00     ` Jonas Köppeler
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Schippers @ 2026-06-13 14:14 UTC (permalink / raw)
  To: hawk, netdev
  Cc: kernel-team, Jonas Köppeler, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
	Daniel Borkmann, John Fastabend, Stanislav Fomichev, linux-kernel,
	bpf

On 6/12/26 10:35, hawk@kernel.org wrote:
> From: Simon Schippers <simon.schippers@tu-dortmund.de>
> 
> 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 <hawk@kernel.org>
> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
> Co-developed-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
> Signed-off-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
> ---
>  drivers/net/veth.c | 123 ++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 117 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 2473f730734b..c62d87a8402c 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -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) {

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

> +		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++;

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/c8650d3a-e488-4279-b28f-549d766c23a1@tu-dortmund.de/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support
  2026-06-12  8:35 [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support hawk
                   ` (5 preceding siblings ...)
  2026-06-12 14:10 ` [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support Simon Schippers
@ 2026-06-16  1:53 ` Jakub Kicinski
  2026-08-10 13:25 ` Simon Schippers
  7 siblings, 0 replies; 18+ messages in thread
From: Jakub Kicinski @ 2026-06-16  1:53 UTC (permalink / raw)
  To: hawk
  Cc: netdev, kernel-team, simon.schippers, David S. Miller,
	Eric Dumazet, Paolo Abeni, Simon Horman, Chris Arges,
	Mike Freemon, Toke Høiland-Jørgensen,
	Jonas Köppeler, Breno Leitao, Alexei Starovoitov,
	Daniel Borkmann, John Fastabend, Stanislav Fomichev, bpf

On Fri, 12 Jun 2026 10:35:23 +0200 hawk@kernel.org 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...
-- 
pw-bot: defer

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs
  2026-06-13 14:14   ` Simon Schippers
@ 2026-06-30 14:00     ` Jonas Köppeler
  2026-06-30 19:07       ` Simon Schippers
  0 siblings, 1 reply; 18+ messages in thread
From: Jonas Köppeler @ 2026-06-30 14:00 UTC (permalink / raw)
  To: Simon Schippers, hawk, netdev
  Cc: kernel-team, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
	John Fastabend, Stanislav Fomichev, linux-kernel, bpf

On 6/13/26 4:14 PM, Simon Schippers wrote:
> On 6/12/26 10:35, hawk@kernel.org wrote:
>> From: Simon Schippers <simon.schippers@tu-dortmund.de>
>>
>> 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 <hawk@kernel.org>
>> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
>> Co-developed-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
>> Signed-off-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
>> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
>> ---
>>   drivers/net/veth.c | 123 ++++++++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 117 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
>> index 2473f730734b..c62d87a8402c 100644
>> --- a/drivers/net/veth.c
>> +++ b/drivers/net/veth.c
>> @@ -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;
>> +
>> +	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.

> 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
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
> 
>> +		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++;
> 
> 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/c8650d3a-e488-4279-b28f-549d766c23a1@tu-dortmund.de/


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs
  2026-06-30 14:00     ` Jonas Köppeler
@ 2026-06-30 19:07       ` Simon Schippers
  2026-07-09 10:03         ` Jonas Köppeler
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Schippers @ 2026-06-30 19:07 UTC (permalink / raw)
  To: Jonas Köppeler, hawk, netdev
  Cc: kernel-team, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
	John Fastabend, Stanislav Fomichev, linux-kernel, bpf

On 6/30/26 16:00, Jonas Köppeler wrote:
> On 6/13/26 4:14 PM, Simon Schippers wrote:
>> On 6/12/26 10:35, hawk@kernel.org wrote:
>>> From: Simon Schippers <simon.schippers@tu-dortmund.de>
>>>
>>> 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 <hawk@kernel.org>
>>> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
>>> Co-developed-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
>>> Signed-off-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
>>> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
>>> ---
>>>   drivers/net/veth.c | 123 ++++++++++++++++++++++++++++++++++++++++++---
>>>   1 file changed, 117 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
>>> index 2473f730734b..c62d87a8402c 100644
>>> --- a/drivers/net/veth.c
>>> +++ b/drivers/net/veth.c
>>> @@ -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;
>>> +
>>> +    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.

We would put #ifdef CONFIG_BQL around that logic aswell.

> 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.
 */ 

> 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.

> 
> 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.

> 
> 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.
> 
>> 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
> 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

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

Thanks,
Simon

>>
>>> +        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++;
>>
>> 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/c8650d3a-e488-4279-b28f-549d766c23a1@tu-dortmund.de/
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs
  2026-06-30 19:07       ` Simon Schippers
@ 2026-07-09 10:03         ` Jonas Köppeler
  0 siblings, 0 replies; 18+ messages in thread
From: Jonas Köppeler @ 2026-07-09 10:03 UTC (permalink / raw)
  To: Simon Schippers, hawk, netdev
  Cc: kernel-team, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann,
	John Fastabend, Stanislav Fomichev, linux-kernel, bpf

On 6/30/26 21:07, Simon Schippers wrote:
> On 6/30/26 16:00, Jonas Köppeler wrote:
>> On 6/13/26 4:14 PM, Simon Schippers wrote:
>>> On 6/12/26 10:35, hawk@kernel.org wrote:
>>>> From: Simon Schippers <simon.schippers@tu-dortmund.de>
>>>>
>>>> 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 <hawk@kernel.org>
>>>> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
>>>> Co-developed-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
>>>> Signed-off-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
>>>> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
>>>> ---
>>>>    drivers/net/veth.c | 123 ++++++++++++++++++++++++++++++++++++++++++---
>>>>    1 file changed, 117 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
>>>> index 2473f730734b..c62d87a8402c 100644
>>>> --- a/drivers/net/veth.c
>>>> +++ b/drivers/net/veth.c
>>>> @@ -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;
>>>> +
>>>> +    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.
> 
> We would put #ifdef CONFIG_BQL around that logic aswell.
> 
>> 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.
>   */
> 
>> 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.
> 
>>
>> 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.
> 
>>
>> 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.
>>
>>> 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
>> 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
> 
> 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?= <j.koeppeler@tu-berlin.de>
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.

Signed-off-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
---
  drivers/net/veth.c | 53 ++++++++++++++++++++++++++++++----------------
  1 file changed, 35 insertions(+), 18 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index c62d87a8402c..2963f190988f 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -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;
-- 
2.53.0

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

From: =?UTF-8?q?Jonas=20K=C3=B6ppeler?= <j.koeppeler@tu-berlin.de>
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 <j.koeppeler@tu-berlin.de>
---
  drivers/net/veth.c | 173 +++++++++++++++++++++++++++++----------------
  1 file changed, 111 insertions(+), 62 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 2963f190988f..5c3b7820c55c 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -73,7 +73,7 @@ struct veth_rq_stats {

  struct veth_bql_state {
  	u64	time;	/* sched_clock() when current coalescing window started */
-	uint	n_bql;	/* BQL completions batched in the current window */
+	unsigned int	n_bql;	/* BQL completions batched in the current window */
  };

  struct veth_rq {
@@ -83,7 +83,9 @@ struct veth_rq {
  	struct bpf_prog __rcu	*xdp_prog;
  	struct xdp_mem_info	xdp_mem;
  	struct veth_rq_stats	stats;
+#ifdef CONFIG_BQL
  	struct veth_bql_state	bql_state;
+#endif
  	bool			rx_notify_masked;
  	struct ptr_ring		xdp_ring;
  	struct xdp_rxq_info	xdp_rxq;
@@ -300,6 +302,12 @@ static int veth_set_coalesce(struct net_device *dev,
  	struct veth_priv *priv = netdev_priv(dev);
  	struct net_device *peer;

+	/* tx-usecs only batches BQL completions; without BQL it is inert. */
+	if (!IS_ENABLED(CONFIG_BQL)) {
+		NL_SET_ERR_MSG_MOD(extack, "tx-usecs requires CONFIG_BQL");
+		return -EOPNOTSUPP;
+	}
+
  	/* 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
@@ -351,11 +359,6 @@ 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);
@@ -384,25 +387,6 @@ static void veth_ptr_free(void *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)
  {
  	/* Write ptr_ring before reading rx_notify_masked */
@@ -1029,6 +1013,31 @@ static struct sk_buff *veth_xdp_rcv_skb(struct 
veth_rq *rq,
  	return NULL;
  }

+#ifdef CONFIG_BQL
+static bool veth_ptr_is_bql(void *ptr)
+{
+	return (unsigned long)ptr & VETH_BQL_FLAG;
+}
+
+/* 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_bql_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_bql_complete(struct veth_bql_state *state,
  			      struct netdev_queue *peer_txq,
  			      u64 now)
@@ -1039,18 +1048,18 @@ static void veth_bql_complete(struct 
veth_bql_state *state,
  	state->n_bql = 0;
  }

-static void veth_bql_maybe_complete(struct veth_bql_state *state,
-				    struct netdev_queue *peer_txq,
-				    u64 bql_flush_ns)
+static void veth_bql_account(struct veth_rq *rq,
+			     struct netdev_queue *peer_txq,
+			     void *ptr, u64 bql_flush_ns)
  {
+	struct veth_bql_state *state = &rq->bql_state;
  	u64 now;

-	/* There is no reason to complete with 0 and
-	 * peer_txq could go away.
-	 */
-	if (!state->n_bql || !peer_txq)
+	if (!peer_txq || !veth_ptr_is_bql(ptr))
  		return;

+	state->n_bql++;
+
  	/* Release the batched completions once the coalescing window has
  	 * elapsed.
  	 */
@@ -1059,24 +1068,81 @@ static void veth_bql_maybe_complete(struct 
veth_bql_state *state,
  		veth_bql_complete(state, peer_txq, now);
  }

+/* Per-poll setup: clamp the window timestamp and return the length of the
+ * coalescing window in ns.
+ */
+static u64 veth_bql_poll_prepare(struct veth_rq *rq)
+{
+	struct veth_priv *priv = netdev_priv(rq->dev);
+	struct veth_bql_state *state = &rq->bql_state;
+
+	/* 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());
+
+	/* Mirrored to both peers; paired with WRITE_ONCE() in 
veth_set_coalesce */
+	return (u64)READ_ONCE(priv->tx_coal_usecs) * NSEC_PER_USEC;
+}
+
+static void veth_bql_flush_starved(struct veth_rq *rq,
+				   struct netdev_queue *peer_txq)
+{
+	struct veth_bql_state *state = &rq->bql_state;
+
+	if (!peer_txq)
+		return;
+
+	/* 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 (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());
+	}
+}
+
+static void veth_bql_state_init(struct veth_rq *rq)
+{
+	rq->bql_state.time = sched_clock();
+	rq->bql_state.n_bql = 0;
+}
+
+static unsigned int veth_bql_drain_and_reset(struct veth_rq *rq)
+{
+	unsigned int n_bql = veth_bql_ring_drain(&rq->xdp_ring) + 
rq->bql_state.n_bql;
+
+	rq->bql_state.n_bql = 0;
+	rq->bql_state.time = 0;
+	return n_bql;
+}
+#else
+static inline void veth_bql_account(struct veth_rq *rq,
+				    struct netdev_queue *peer_txq,
+				    void *ptr, u64 bql_flush_ns) {}
+static inline u64 veth_bql_poll_prepare(struct veth_rq *rq) { return 0; }
+static inline void veth_bql_flush_starved(struct veth_rq *rq,
+					  struct netdev_queue *peer_txq) {}
+static inline void veth_bql_state_init(struct veth_rq *rq) {}
+static inline unsigned int veth_bql_drain_and_reset(struct veth_rq *rq) 
{ return 0; }
+#endif
+
  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());
+	bql_flush_ns = veth_bql_poll_prepare(rq);

  	for (i = 0; i < budget; i++) {
  		void *ptr = __ptr_ring_consume(&rq->xdp_ring);
@@ -1103,12 +1169,10 @@ static int veth_xdp_rcv(struct veth_rq *rq, int 
budget,
  			/* ndo_start_xmit */
  			struct sk_buff *skb = veth_ptr_to_skb(ptr);

-			if (veth_ptr_is_bql(ptr))
-				state->n_bql++;
  			/* 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);
+			veth_bql_account(rq, peer_txq, ptr, bql_flush_ns);

  			stats->xdp_bytes += skb->len;
  			skb = veth_xdp_rcv_skb(rq, skb, bq, stats);
@@ -1125,19 +1189,7 @@ static int veth_xdp_rcv(struct veth_rq *rq, int 
budget,
  	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());
-	}
+	veth_bql_flush_starved(rq, peer_txq);

  	u64_stats_update_begin(&rq->stats.syncp);
  	rq->stats.vs.xdp_redirect += stats->xdp_redirect;
@@ -1241,8 +1293,7 @@ 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;
+		veth_bql_state_init(rq);

  		napi_enable(&rq->xdp_napi);
  		rcu_assign_pointer(priv->rq[i].napi, &priv->rq[i].xdp_napi);
@@ -1298,10 +1349,8 @@ static void veth_napi_del_range(struct net_device 
*dev, int start, int end)
  		 * (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) + rq->bql_state.n_bql;
+		n_bql = veth_bql_drain_and_reset(rq);
  		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;
-- 
2.53.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)
>>>>    {
>>>> +    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++;
>>>
>>> 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/c8650d3a-e488-4279-b28f-549d766c23a1@tu-dortmund.de/
>>


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support
  2026-06-12  8:35 [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support hawk
                   ` (6 preceding siblings ...)
  2026-06-16  1:53 ` Jakub Kicinski
@ 2026-08-10 13:25 ` Simon Schippers
  2026-09-15 14:30   ` Simon Schippers
  7 siblings, 1 reply; 18+ messages in thread
From: Simon Schippers @ 2026-08-10 13:25 UTC (permalink / raw)
  To: hawk, netdev, Jonas Köppeler
  Cc: kernel-team, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Chris Arges, Mike Freemon,
	Toke Høiland-Jørgensen, Breno Leitao,
	Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Stanislav Fomichev, bpf

[-- Attachment #1: Type: text/plain, Size: 1626 bytes --]

On 6/12/26 10:35, hawk@kernel.org wrote:
> From: Jesper Dangaard Brouer <hawk@kernel.org>
> 
> 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

[-- Attachment #2: 0001-net-dql-Add-completion-coalescing-for-software-inter.patch --]
[-- Type: text/x-patch, Size: 10835 bytes --]

From 1a6d2f0a15c23bb4ebb44139193c2649de1142ad Mon Sep 17 00:00:00 2001
From: Simon Schippers <simon.schippers@tu-dortmund.de>
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.

Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
 include/linux/dynamic_queue_limits.h | 53 ++++++++++++++++++++---
 lib/dynamic_queue_limits.c           | 63 +++++++++++++++++++++++++---
 2 files changed, 104 insertions(+), 12 deletions(-)

diff --git a/include/linux/dynamic_queue_limits.h b/include/linux/dynamic_queue_limits.h
index 808b1a5102e7..cb7338209246 100644
--- a/include/linux/dynamic_queue_limits.h
+++ b/include/linux/dynamic_queue_limits.h
@@ -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);
diff --git a/lib/dynamic_queue_limits.c b/lib/dynamic_queue_limits.c
index f97a752e900a..af4ae1d11c81 100644
--- a/lib/dynamic_queue_limits.c
+++ b/lib/dynamic_queue_limits.c
@@ -10,6 +10,7 @@
 #include <linux/dynamic_queue_limits.h>
 #include <linux/compiler.h>
 #include <linux/export.h>
+#include <linux/sched/clock.h>
 #include <trace/events/napi.h>
 
 #define POSDIFF(A, B) ((int)((A) - (B)) > 0 ? (A) - (B) : 0)
@@ -80,20 +81,65 @@ static void dql_check_stall(struct dql *dql, unsigned short stall_thrs)
 }
 
 /* Records completed count and recalculates the queue limit */
-void dql_completed(struct dql *dql, unsigned int count)
+bool dql_completed(struct dql *dql, unsigned int count)
 {
 	unsigned int inprogress, prev_inprogress, limit;
 	unsigned int ovlimit, completed, num_queued;
+	unsigned int max_limit, min_limit;
 	unsigned short stall_thrs;
 	bool all_prev_completed;
+	u16 coal_usecs;
+	bool coal;
+
+	/* Coalescing is active while a window is configured, or while a batch
+	 * is still outstanding and needs draining. Read coal_usecs once so that
+	 * the two uses below cannot disagree if it is changed concurrently.
+	 */
+	coal_usecs = READ_ONCE(dql->coal_usecs);
+	coal = coal_usecs || dql->coal_pending;
+
+	/* Without coalescing there is nothing to do unless something completed.
+	 * A coalescing queue may be called with @count == 0 to flush a batch
+	 * whose window has elapsed.
+	 */
+	if (!coal && !count)
+		return false;
+
+	if (coal) {
+		u32 now;
+
+		dql->coal_pending += count;
+		if (!dql->coal_pending)
+			return false;
+
+		/* Read the clock only once there is something to hold back, so
+		 * that a flush call on a queue with nothing pending is free.
+		 */
+		now = local_clock() >> DQL_COAL_NS_TO_USECS_SHIFT;
+
+		/* Hold coal_pending back until the window has elapsed.
+		 * coal_pending above the limit means the queue is starved: it
+		 * cannot make progress until those objects are accounted for,
+		 * so flush regardless of the window.
+		 */
+		if (now - dql->coal_last_flush < coal_usecs &&
+		    dql->coal_pending <= dql->limit)
+			return false;
+
+		count = dql->coal_pending;
+		dql->coal_pending = 0;
+		dql->coal_last_flush = now;
+	}
 
 	num_queued = READ_ONCE(dql->num_queued);
-	/* Read stall_thrs in advance since it belongs to the same (first)
-	 * cache line as ->num_queued. This way, dql_check_stall() does not
-	 * need to touch the first cache line again later, reducing the window
-	 * of possible false sharing.
+	/* Read stall_thrs, max_limit and min_limit in advance since they belong
+	 * to the same (first) cache line as ->num_queued. This way, neither
+	 * dql_check_stall() nor the clamp() below need to touch the first cache
+	 * line again later, reducing the window of possible false sharing.
 	 */
 	stall_thrs = READ_ONCE(dql->stall_thrs);
+	max_limit = READ_ONCE(dql->max_limit);
+	min_limit = READ_ONCE(dql->min_limit);
 
 	/* Can't complete more than what's in queue */
 	BUG_ON(count > num_queued - dql->num_completed);
@@ -170,7 +216,7 @@ void dql_completed(struct dql *dql, unsigned int count)
 	}
 
 	/* Enforce bounds on limit */
-	limit = clamp(limit, dql->min_limit, dql->max_limit);
+	limit = clamp(limit, min_limit, max_limit);
 
 	if (limit != dql->limit) {
 		dql->limit = limit;
@@ -184,6 +230,8 @@ void dql_completed(struct dql *dql, unsigned int count)
 	dql->prev_num_queued = num_queued;
 
 	dql_check_stall(dql, stall_thrs);
+
+	return true;
 }
 EXPORT_SYMBOL(dql_completed);
 
@@ -199,6 +247,8 @@ void dql_reset(struct dql *dql)
 	dql->prev_ovlimit = 0;
 	dql->lowest_slack = UINT_MAX;
 	dql->slack_start_time = jiffies;
+	dql->coal_pending = 0;
+	dql->coal_last_flush = local_clock() >> DQL_COAL_NS_TO_USECS_SHIFT;
 
 	dql->last_reap = jiffies;
 	dql->history_head = jiffies / BITS_PER_LONG;
@@ -212,6 +262,7 @@ void dql_init(struct dql *dql, unsigned int hold_time)
 	dql->min_limit = 0;
 	dql->slack_hold_time = hold_time;
 	dql->stall_thrs = 0;
+	dql->coal_usecs = 0;
 	dql_reset(dql);
 }
 EXPORT_SYMBOL(dql_init);

base-commit: 001b5d347d8ba39b2dccaefcc57967b18caec8fe
-- 
2.43.0


[-- Attachment #3: 0002-net-bql-Provide-support-for-dql-coalescing-for-softw.patch --]
[-- Type: text/x-patch, Size: 8101 bytes --]

From ad97ecb1ea25a16bd61549e95dce1a86b4efcd7e Mon Sep 17 00:00:00 2001
From: Simon Schippers <simon.schippers@tu-dortmund.de>
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.

Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
 .../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(-)

diff --git a/Documentation/ABI/testing/sysfs-class-net-queues b/Documentation/ABI/testing/sysfs-class-net-queues
index 84aa25e0d14d..4a0413d353a9 100644
--- a/Documentation/ABI/testing/sysfs-class-net-queues
+++ b/Documentation/ABI/testing/sysfs-class-net-queues
@@ -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:	netdev@vger.kernel.org
+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
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index db9dce7f0aa6..36a8d4362d3e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3975,28 +3975,41 @@ static inline bool __netdev_sent_queue(struct net_device *dev,
  *
  *	Must be called at most once per TX completion round (and not per
  *	individual packet), so that BQL can adjust its limits appropriately.
+ *	A queue with DQL completion coalescing enabled is exempt from this,
+ *	see netif_set_bql_coalesce_usecs(): coalescing does the batching, so
+ *	such a queue may report per packet.
+ *
+ *	Returns: true if the completion was applied and the limit recalculated,
+ *	false if nothing completed or the completion was only batched by
+ *	coalescing. A true return also means the memory barrier below was
+ *	issued, which netdev_txq_completed_mb() depends on.
  */
-static inline void netdev_tx_completed_queue(struct netdev_queue *dev_queue,
+static inline bool netdev_tx_completed_queue(struct netdev_queue *dev_queue,
 					     unsigned int pkts, unsigned int bytes)
 {
 #ifdef CONFIG_BQL
-	if (unlikely(!bytes))
-		return;
-
-	dql_completed(&dev_queue->dql, bytes);
+	/* There is nothing to wake if nothing completed, or if the completion
+	 * was only batched by coalescing and the limit has not moved.
+	 */
+	if (!dql_completed(&dev_queue->dql, bytes))
+		return false;
 
 	/*
 	 * Without the memory barrier there is a small possibility that
 	 * netdev_tx_sent_queue will miss the update and cause the queue to
 	 * be stopped forever
 	 */
-	smp_mb(); /* NOTE: netdev_txq_completed_mb() assumes this exists */
+	smp_mb();
 
 	if (unlikely(dql_avail(&dev_queue->dql) < 0))
-		return;
+		return true;
 
 	if (test_and_clear_bit(__QUEUE_STATE_STACK_XOFF, &dev_queue->state))
 		netif_schedule_queue(dev_queue);
+
+	return true;
+#else
+	return false;
 #endif
 }
 
@@ -4024,6 +4037,52 @@ static inline void netdev_tx_reset_queue(struct netdev_queue *q)
 #endif
 }
 
+/**
+ * netif_set_bql_coalesce_usecs - set the BQL completion coalescing window
+ * @dev: network device
+ * @usecs: window in microseconds, 0 disables coalescing
+ *
+ * Configure DQL completion coalescing on all TX queues of @dev, so that
+ * netdev_tx_completed_queue() batches completions instead of recalculating the
+ * limit on every call. Intended for software interfaces, which have no hardware
+ * completion interrupt and therefore complete per packet.
+ *
+ * Safe to call on a live device to change the window. @usecs must not exceed
+ * DQL_COAL_MAX_USECS.
+ */
+static inline void netif_set_bql_coalesce_usecs(struct net_device *dev,
+						unsigned int usecs)
+{
+#ifdef CONFIG_BQL
+	unsigned int i;
+
+	for (i = 0; i < dev->num_tx_queues; i++)
+		dql_set_coal_usecs(&netdev_get_tx_queue(dev, i)->dql, usecs);
+#endif
+}
+
+/**
+ * netif_get_bql_coalesce_usecs - get the BQL completion coalescing window
+ * @dev: network device
+ *
+ * The window is per queue and can also be set through
+ * byte_queue_limits/coal_usecs, so this reports TX queue 0 only. Meant for
+ * drivers that set the same window on every queue with
+ * netif_set_bql_coalesce_usecs().
+ *
+ * Returns: the window in microseconds, 0 if coalescing is disabled or BQL is
+ * not built in.
+ */
+static inline unsigned int
+netif_get_bql_coalesce_usecs(const struct net_device *dev)
+{
+#ifdef CONFIG_BQL
+	return dql_get_coal_usecs(&netdev_get_tx_queue(dev, 0)->dql);
+#else
+	return 0;
+#endif
+}
+
 /**
  * netdev_tx_reset_subqueue - reset the BQL stats and state of a netdev queue
  * @dev: network device
diff --git a/include/net/netdev_queues.h b/include/net/netdev_queues.h
index 70c9fe9e83cc..c17d0a49c6db 100644
--- a/include/net/netdev_queues.h
+++ b/include/net/netdev_queues.h
@@ -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();
 }
 
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 25546deacec8..286abb88dad8 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -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
 };
 
-- 
2.43.0


[-- Attachment #4: 0003-net-add-dev-bql-flag-to-allow-BQL-sysfs-for-IFF_NO_Q.patch --]
[-- Type: text/x-patch, Size: 3681 bytes --]

From be9913fa1bd36bec9bf4f38daf33ab51923656ac Mon Sep 17 00:00:00 2001
From: Jesper Dangaard Brouer <hawk@kernel.org>
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

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

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

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

Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
Tested-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
 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(-)

diff --git a/Documentation/networking/net_cachelines/net_device.rst b/Documentation/networking/net_cachelines/net_device.rst
index 512f6d6fa3d8..52709375ee21 100644
--- a/Documentation/networking/net_cachelines/net_device.rst
+++ b/Documentation/networking/net_cachelines/net_device.rst
@@ -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
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 36a8d4362d3e..cdd25832357e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -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;
 
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 286abb88dad8..180065ad7ed9 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -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)
-- 
2.43.0


[-- Attachment #5: 0004-veth-implement-Byte-Queue-Limits-BQL-for-latency-red.patch --]
[-- Type: text/x-patch, Size: 13397 bytes --]

From 64b7a0a2b41aa3f4d0a639dc86819a1470706118 Mon Sep 17 00:00:00 2001
From: Jesper Dangaard Brouer <hawk@kernel.org>
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 <hawk@kernel.org>
Co-developed-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
Signed-off-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
Co-developed-by: Simon Schippers <simon.schippers@tu-dortmund.de>
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
 drivers/net/veth.c | 170 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 157 insertions(+), 13 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 42e4f246c91d..51ef8474f447 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -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)
 
+/* Default DQL completion coalescing window (us), tunable with ethtool -C */
+#define VETH_BQL_COAL_USECS	100
+
 #define VETH_XDP_TX_BULK_SIZE	16
 #define VETH_XDP_BATCH		16
 
@@ -262,7 +266,51 @@ 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)
+{
+	/* Read back from DQL rather than a shadow copy, so that a window set
+	 * through byte_queue_limits/coal_usecs is reported here too.
+	 */
+	ec->tx_coalesce_usecs = netif_get_bql_coalesce_usecs(dev);
+
+	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;
+
+	if (!IS_ENABLED(CONFIG_BQL)) {
+		NL_SET_ERR_MSG_MOD(extack, "BQL is not enabled in this kernel");
+		return -EOPNOTSUPP;
+	}
+
+	if (ec->tx_coalesce_usecs > DQL_COAL_MAX_USECS) {
+		NL_SET_ERR_MSG_MOD(extack, "tx-usecs too large");
+		return -EINVAL;
+	}
+
+	netif_set_bql_coalesce_usecs(dev, ec->tx_coalesce_usecs);
+
+	/* Mirror onto the peer to keep the pair symmetric: both directions
+	 * coalesce with the same tx-usecs. Called under RTNL.
+	 */
+	peer = rtnl_dereference(priv->peer);
+	if (peer)
+		netif_set_bql_coalesce_usecs(peer, 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,
@@ -272,6 +320,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 */
@@ -281,6 +331,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);
@@ -296,7 +361,21 @@ static void veth_ptr_free(void *ptr)
 	if (veth_is_xdp_frame(ptr))
 		xdp_return_frame(veth_ptr_to_xdp(ptr));
 	else
-		kfree_skb(ptr);
+		kfree_skb(veth_ptr_to_skb(ptr));
+}
+
+static 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)
@@ -310,19 +389,36 @@ 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 */
+	}
+
+	if (do_bql)
+		netdev_tx_sent_queue(txq, 1); /* one unit per packet */
+
+	/* Its smp_wmb() orders the BQL charge above ahead of the entry, so
+	 * the peer NAPI cannot see the skb without its charge. Pairs with
+	 * the smp_rmb() in veth_xdp_rcv().
+	 */
+	__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);
 }
 
@@ -349,10 +445,11 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
 	struct veth_rq *rq = NULL;
-	struct netdev_queue *txq;
+	struct netdev_queue *txq = NULL;
 	struct net_device *rcv;
 	int length = skb->len;
 	bool use_napi = false;
+	bool do_bql = false;
 	int ret, rxq;
 
 	rcu_read_lock();
@@ -377,7 +474,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);
+		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)
@@ -389,9 +491,7 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 		/* If a qdisc is attached to our virtual device, returning
 		 * NETDEV_TX_BUSY is allowed.
 		 */
-		txq = netdev_get_tx_queue(dev, rxq);
-
-		if (qdisc_txq_has_no_queue(txq)) {
+		if (!txq || qdisc_txq_has_no_queue(txq)) {
 			dev_kfree_skb_any(skb);
 			goto drop;
 		}
@@ -901,11 +1001,18 @@ 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];
 
+	/* Flush completions batched during an earlier poll whose coalescing
+	 * window has elapsed in the meantime.
+	 */
+	if (peer_txq)
+		netdev_tx_completed_queue(peer_txq, 0, 0);
+
 	for (i = 0; i < budget; i++) {
 		void *ptr = __ptr_ring_consume(&rq->xdp_ring);
 
@@ -929,9 +1036,21 @@ 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) {
+				/* Pairs with the smp_wmb() in
+				 * __ptr_ring_produce(). The charge is not
+				 * reached through ptr, so nothing else
+				 * orders dql_completed()'s read of
+				 * num_queued against the consume above.
+				 */
+				smp_rmb();
+				netdev_tx_completed_queue(peer_txq, 1, 1);
+			}
+
 			skb = veth_xdp_rcv_skb(rq, skb, bq, stats);
 			if (skb) {
 				if (skb_shared(skb) || skb_unclone(skb, GFP_ATOMIC))
@@ -977,7 +1096,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();
@@ -1075,6 +1194,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++) {
@@ -1086,11 +1206,31 @@ static void veth_napi_del_range(struct net_device *dev, int start, int end)
 	}
 	synchronize_net();
 
+	peer = rtnl_dereference(priv->peer);
+
 	for (i = start; i < end; i++) {
 		struct veth_rq *rq = &priv->rq[i];
+		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);
+
+		if (n_bql)
+			netdev_tx_completed_queue(txq, n_bql, n_bql);
+
+		if (netif_running(peer))
+			netif_tx_wake_queue(txq);
 	}
 
 	for (i = start; i < end; i++) {
@@ -1744,6 +1884,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;
@@ -1919,6 +2060,9 @@ static int veth_newlink(struct net_device *dev,
 	veth_set_xdp_features(dev);
 	veth_set_xdp_features(peer);
 
+	netif_set_bql_coalesce_usecs(dev, VETH_BQL_COAL_USECS);
+	netif_set_bql_coalesce_usecs(peer, VETH_BQL_COAL_USECS);
+
 	return 0;
 
 err_peer_queues:
-- 
2.43.0


[-- Attachment #6: 0005-net-sched-add-timeout-count-to-NETDEV-WATCHDOG-messa.patch --]
[-- Type: text/x-patch, Size: 2405 bytes --]

From ab29d87b9b357e34b935376e1fe81786672e7310 Mon Sep 17 00:00:00 2001
From: Jesper Dangaard Brouer <hawk@kernel.org>
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

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.

Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
Tested-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
 net/sched/sch_generic.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index ef2b4bf51564..4cbefeab726a 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -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);
-- 
2.43.0


[-- Attachment #7: 0006-veth-add-tx_timeout-watchdog-as-BQL-safety-net.patch --]
[-- Type: text/x-patch, Size: 3359 bytes --]

From f75b304361388b050425d5b3796e5583dccb0d96 Mon Sep 17 00:00:00 2001
From: Jesper Dangaard Brouer <hawk@kernel.org>
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.

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 <hawk@kernel.org>
Tested-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
 drivers/net/veth.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 51ef8474f447..2310d994da51 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -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 */
@@ -1522,6 +1529,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);
@@ -1860,6 +1883,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 = {
@@ -1899,6 +1923,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;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support
  2026-08-10 13:25 ` Simon Schippers
@ 2026-09-15 14:30   ` Simon Schippers
  2026-09-16  6:45     ` Jesper Dangaard Brouer
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Schippers @ 2026-09-15 14:30 UTC (permalink / raw)
  To: hawk, netdev, Jonas Köppeler
  Cc: kernel-team, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Chris Arges, Mike Freemon,
	Toke Høiland-Jørgensen, Breno Leitao,
	Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Stanislav Fomichev, bpf

On 8/10/26 15:25, Simon Schippers wrote:
> On 6/12/26 10:35, hawk@kernel.org wrote:
>> From: Jesper Dangaard Brouer <hawk@kernel.org>
>>
>> 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

Hi! Any thoughts on this? Do you want to continue this series?

Thanks!


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support
  2026-09-15 14:30   ` Simon Schippers
@ 2026-09-16  6:45     ` Jesper Dangaard Brouer
  2026-09-16  8:14       ` Simon Schippers
  0 siblings, 1 reply; 18+ messages in thread
From: Jesper Dangaard Brouer @ 2026-09-16  6:45 UTC (permalink / raw)
  To: Simon Schippers, netdev, Jonas Köppeler
  Cc: kernel-team, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Chris Arges, Mike Freemon,
	Toke Høiland-Jørgensen, Breno Leitao,
	Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Stanislav Fomichev, bpf, kernel-team



On 9/15/26 16:30, Simon Schippers wrote:
> On 8/10/26 15:25, Simon Schippers wrote:
>> On 6/12/26 10:35, hawk@kernel.org wrote:
>>> From: Jesper Dangaard Brouer <hawk@kernel.org>
>>>
>>> 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
> 
> Hi! Any thoughts on this? Do you want to continue this series?

Appreciate getting poked :-)

I will not have time to work on this until after October 5th.

If you Simon have time, feel free to submit a V8 patchset with the
barrier fix mentioned above.  I should have cycles to review and ACK
(except between 26 sep to Oct 4).

We are still interested in getting this merged. Notice the bug fix from
Jonas 60db47f02bfa ("veth: fix queue index used to wake the peer txq in
veth_poll").  We are running XDP on our veth production interfaces, so
we didn't notice this.  Still, we are currently waiting for this fix to
get fully rolled out, before proceeding with the BQL variant.

--Jesper



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support
  2026-09-16  6:45     ` Jesper Dangaard Brouer
@ 2026-09-16  8:14       ` Simon Schippers
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Schippers @ 2026-09-16  8:14 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, netdev, Jonas Köppeler
  Cc: kernel-team, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Chris Arges, Mike Freemon,
	Toke Høiland-Jørgensen, Breno Leitao,
	Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Stanislav Fomichev, bpf

On 9/16/26 08:45, Jesper Dangaard Brouer wrote:
> 
> 
> On 9/15/26 16:30, Simon Schippers wrote:
>> On 8/10/26 15:25, Simon Schippers wrote:
>>> On 6/12/26 10:35, hawk@kernel.org wrote:
>>>> From: Jesper Dangaard Brouer <hawk@kernel.org>
>>>>
>>>> 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
>>
>> Hi! Any thoughts on this? Do you want to continue this series?
> 
> Appreciate getting poked :-)
> 
> I will not have time to work on this until after October 5th.

Noted, no problem.

> 
> If you Simon have time, feel free to submit a V8 patchset with the
> barrier fix mentioned above.  I should have cycles to review and ACK
> (except between 26 sep to Oct 4).

I would then also move the coalescing logic to
dynamic_queue_limits.{h,c}, ok?

I am convinced that it is the right decision, because it:
- saves us the new struct veth_bql_state, everything fits nicely into
  the dql struct in the existing cachelines
- allows other software interfaces like tun/tap to use the same logic,
  which is my goal :-) others could be e.g. wireguard and ifb
- allows to change coal_usecs per queue

> 
> We are still interested in getting this merged. Notice the bug fix from
> Jonas 60db47f02bfa ("veth: fix queue index used to wake the peer txq in
> veth_poll").  We are running XDP on our veth production interfaces, so
> we didn't notice this.  Still, we are currently waiting for this fix to
> get fully rolled out, before proceeding with the BQL variant.

Yes, I saw that. Probably did not happen to me because I was always
using single queue..

Thanks.


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2026-09-16  8:14 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-12  8:35 [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support hawk
2026-06-12  8:35 ` [PATCH net-next v7 1/5] net: add dev->bql flag to allow BQL sysfs for IFF_NO_QUEUE devices hawk
2026-06-12  8:35 ` [PATCH net-next v7 2/5] veth: implement Byte Queue Limits (BQL) for latency reduction hawk
2026-06-12  8:35 ` [PATCH net-next v7 3/5] veth: add tx_timeout watchdog as BQL safety net hawk
2026-06-12  8:35 ` [PATCH net-next v7 4/5] net: sched: add timeout count to NETDEV WATCHDOG message hawk
2026-06-12  8:35 ` [PATCH net-next v7 5/5] veth: time-based BQL completion coalescing via ethtool tx-usecs hawk
2026-06-13 14:14   ` Simon Schippers
2026-06-30 14:00     ` Jonas Köppeler
2026-06-30 19:07       ` Simon Schippers
2026-07-09 10:03         ` Jonas Köppeler
2026-06-12 14:10 ` [PATCH net-next v7 0/5] veth: add Byte Queue Limits (BQL) support Simon Schippers
2026-06-12 17:21   ` Jonas Köppeler
2026-06-13 13:57     ` Simon Schippers
2026-06-16  1:53 ` Jakub Kicinski
2026-08-10 13:25 ` Simon Schippers
2026-09-15 14:30   ` Simon Schippers
2026-09-16  6:45     ` Jesper Dangaard Brouer
2026-09-16  8:14       ` Simon Schippers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox