Re: [PATCH net v2] tun/tap & vhost-net: make qdisc backpressure opt-in via IFF_BACKPRESSURE

[Simon Schippers <simon.schippers@tu-dortmund.de>]

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

On 7/6/26 19:10, Brett A C Sheffield wrote:
> On 2026-07-06 17:33, Simon Schippers wrote:
>> But even if we could perfectly fix the performance issues, maybe users
>> even users rely on the dropping behavior. From Brett [1]:
>>
>> "In our multicast use case data is sent by multiple threads to multiple
>> groups simultaneously, this just breaks things to the extent that a 
>> <2 second test times out after 5 minutes."
>>
>> We are *not* factor 5min * 60sec/min / 2s = 150 times slower than without
>> the patchset.
> 
> I didn't mean to suggest 150x slower. It would have been more correct if I'd
> simply said "the test normally takes <2s but fails to complete with the
> patchset". The 5min timeout was irrelevant detail.
> 
> The iperf3 tests give a much better picture of the performance impact.
> 
> I thought a simple TCP test with a familar tool might be easier than explaining
> the ways in which we're torturing multicast ;-)

Yes :D

> 
>> My theory is that the sender sends a fixed amount of data
>> of which most is dropped without backpressure, which is much faster then
>> the real processing, and so the test *relies* on the tail-dropping to
>> work.
>>
>> @Brett can you maybe support this theory?
> 
> The test synchronizes two blobs of data. The amount of data that needs syncing
> is fixed, but the amount sent will vary as it is encoded with RaptorQ.
> 
> The test sends on several multicast groups simultaneously. Each group is a
> stream of RaptorQ encoded symbols and the receiver listens on that group until
> is has enough symbols to decode. In practice, on a local tap interface, the
> packet loss is normally zero, so the amount of data sent is more or less fixed.

Hmm seems like my theory does not really hold up :D


Brett, can you try the two attached patches here with iperf3?
I think testing with 8 and 16 threads is enough, so where there is a
regression.

The two patches are about time when to wake:
Currently we wake after consuming half the internal ring buffer.
One of the attached patches wakes after 2 cachelines (128 of 1000
packets) and the other one just wakes once the ring buffer is empty.

This would really help :)


BTW: I will try to get hands on a Ryzen 9 9950X soon (I think we have a
     machine at our chair) and then try to reproduce.

Thank you!

[-- Attachment #2: 0001-tun-set-waking-threshold-to-tx_ring.batch.patch --]
[-- Type: text/x-patch, Size: 1212 bytes --]

From 87356bd512f039ea52f5f9901288d043f7ac32d9 Mon Sep 17 00:00:00 2001
From: Simon Schippers <simon.schippers@tu-dortmund.de>
Date: Tue, 7 Jul 2026 08:20:21 +0200
Subject: [PATCH] tun: set waking threshold to >= tx_ring.batch

Replace the waking threshold of >= ring.size/2 with >= tx_ring.batch
allowing an earlier call of netif_tx_wake_queue(). tx_ring.batch is
defined as r->batch = SMP_CACHE_BYTES * 2 / sizeof(*(r->queue)), meaning
a batch describes a two cachelines of the ring.

On a tap interface on a x86 system this means that instead of waking
after consuming 1000/2=500 skbs it now wakes after 64 * 2 = 128 skbs.
---
 drivers/net/tun.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index ffbe6f13fb1f..691432c042d6 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2158,7 +2158,7 @@ static void __tun_wake_queue(struct tun_struct *tun,
 	smp_mb();
 	if (netif_tx_queue_stopped(txq)) {
 		tfile->cons_cnt += consumed;
-		if (tfile->cons_cnt >= tfile->tx_ring.size / 2 ||
+		if (tfile->cons_cnt >= tfile->tx_ring.batch ||
 		    __ptr_ring_empty(&tfile->tx_ring)) {
 			netif_tx_wake_queue(txq);
 			tfile->cons_cnt = 0;
-- 
2.43.0


[-- Attachment #3: 0001-tun-tap-vhost-net-make-qdisc-backpressure-opt-in-via.patch --]
[-- Type: text/x-patch, Size: 8432 bytes --]

From dcea36e105bc2126e9c48d34f9542c22b9123f5b Mon Sep 17 00:00:00 2001
From: Simon Schippers <simon.schippers@tu-dortmund.de>
Date: Thu, 2 Jul 2026 18:21:56 +0200
Subject: [PATCH net v2] tun/tap & vhost-net: make qdisc backpressure opt-in
 via IFF_BACKPRESSURE

Commit 1d6e569b7d0c ("tun/tap & vhost-net: avoid ptr_ring tail-drop
when a qdisc is present") did not show a relevant performance regression
in my testing but on Brett Sheffield's librecast testbed it shows a
significant performance drop in a IPv6 multicast testcase. The regression
can be pinpointed when multiple iperf3 UDP threads are sending. For 8
threads the performance dropped from 13.5 Gbit/s to 9.13 Gbit/s. This is
the reason why this patch makes the qdisc backpressure behavior opt-in.

One option to accomplish the opt-in would be to set the default qdisc to
noqueue at init. However this may also break userspace as users might
have chosen a custom qdisc even though most of the qdiscs did nothing
for tun/tap in the past due to missing backpressure...

This is the reason why in this patch, the flag IFF_BACKPRESSURE is
introduced instead which is required to enable the backpressure logic.
This means the stopping logic in tun_net_xmit() and the waking logic in
__tun_wake_queue() are skipped if the flag is disabled.

To avoid a possible stall due to disabling IFF_BACKPRESSURE, the new
helper tun_force_wake_queue() is implemented. The helper safely wakes the
respective netdev queue and resets cons_cnt while the consumer_lock and
the producer_lock of the ring are held. The helper is run in tun_attach()
when a queue (re)attaches, in tun_set_iff() for attached tfiles, and
in tun_queue_resize().

The documentation in tuntap.rst is updated accordingly.

Fixes: 1d6e569b7d0c ("tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present")
Reported-by: Brett Sheffield <brett@librecast.net>
Closes: https://lore.kernel.org/netdev/akVnoOYQOrt8k-Gu@karahi.librecast.net/T/#u
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
V1 -> V2:
- Sashiko: Ensure detached queues are woken on re-attach by calling the
  new tun_force_wake_queue() helper from tun_attach(), and reuse it
  across the existing wake paths.
- Specify the failing test case in the commit message.
---
 Documentation/networking/tuntap.rst | 17 ++++++++++
 drivers/net/tun.c                   | 49 ++++++++++++++++++-----------
 include/uapi/linux/if_tun.h         |  1 +
 tools/include/uapi/linux/if_tun.h   |  1 +
 4 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/Documentation/networking/tuntap.rst b/Documentation/networking/tuntap.rst
index 4d7087f727be..599264825dd2 100644
--- a/Documentation/networking/tuntap.rst
+++ b/Documentation/networking/tuntap.rst
@@ -206,6 +206,23 @@ enable is true we enable it, otherwise we disable it::
       return ioctl(fd, TUNSETQUEUE, (void *)&ifr);
   }
 
+3.4 qdisc backpressure
+----------------------
+
+Starting with Linux 7.2, IFF_BACKPRESSURE can be set to enable qdisc
+backpressure. Without it, TX drops occur when the internal ring buffer is
+full. With it, the kernel stops the TX queue instead, letting the qdisc
+hold packets. Drops only occur as a rare race. This can benefit protocols
+like TCP that react to drops. Backpressure requires a qdisc to be
+attached and has no effect with noqueue.
+
+The TUN/TAP ring buffer size can be reduced alongside this flag to
+further shift buffering into the qdisc and reduce bufferbloat, but comes
+at possible performance cost.
+
+When running multiple network streams in parallel, the flag may reduce
+performance due to the extra overhead of the backpressure mechanism.
+
 Universal TUN/TAP device driver Frequently Asked Question
 =========================================================
 
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index ffbe6f13fb1f..62e5eab4e345 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -98,7 +98,8 @@ static void tun_default_link_ksettings(struct net_device *dev,
 #define TUN_FASYNC	IFF_ATTACH_QUEUE
 
 #define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
-		      IFF_MULTI_QUEUE | IFF_NAPI | IFF_NAPI_FRAGS)
+		      IFF_MULTI_QUEUE | IFF_NAPI | IFF_NAPI_FRAGS | \
+		      IFF_BACKPRESSURE)
 
 #define GOODCOPY_LEN 128
 
@@ -694,6 +695,20 @@ static void tun_detach_all(struct net_device *dev)
 		module_put(THIS_MODULE);
 }
 
+static void tun_force_wake_queue(struct tun_struct *tun,
+				 struct tun_file *tfile)
+{
+	/* Ensure that the producer can not stop the
+	 * queue concurrently by taking locks.
+	 */
+	spin_lock_bh(&tfile->tx_ring.consumer_lock);
+	spin_lock(&tfile->tx_ring.producer_lock);
+	netif_wake_subqueue(tun->dev, tfile->queue_index);
+	tfile->cons_cnt = 0;
+	spin_unlock(&tfile->tx_ring.producer_lock);
+	spin_unlock_bh(&tfile->tx_ring.consumer_lock);
+}
+
 static int tun_attach(struct tun_struct *tun, struct file *file,
 		      bool skip_filter, bool napi, bool napi_frags,
 		      bool publish_tun)
@@ -737,11 +752,9 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
 		goto out;
 	}
 
-	spin_lock(&tfile->tx_ring.consumer_lock);
-	tfile->cons_cnt = 0;
-	spin_unlock(&tfile->tx_ring.consumer_lock);
 	tfile->queue_index = tun->numqueues;
 	tfile->socket.sk->sk_shutdown &= ~RCV_SHUTDOWN;
+	tun_force_wake_queue(tun, tfile);
 
 	if (tfile->detached) {
 		/* Re-attach detached tfile, updating XDP queue_index */
@@ -1077,7 +1090,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	spin_lock(&tfile->tx_ring.producer_lock);
 	ret = __ptr_ring_produce(&tfile->tx_ring, skb);
-	if (!qdisc_txq_has_no_queue(queue) &&
+	if ((tun->flags & IFF_BACKPRESSURE) &&
+	    !qdisc_txq_has_no_queue(queue) &&
 	    __ptr_ring_check_produce(&tfile->tx_ring) == -ENOSPC) {
 		netif_tx_stop_queue(queue);
 		/* Paired with smp_mb() in __tun_wake_queue() */
@@ -2151,8 +2165,12 @@ static ssize_t tun_put_user(struct tun_struct *tun,
 static void __tun_wake_queue(struct tun_struct *tun,
 			     struct tun_file *tfile, int consumed)
 {
-	struct netdev_queue *txq = netdev_get_tx_queue(tun->dev,
-						tfile->queue_index);
+	struct netdev_queue *txq;
+
+	if (!(tun->flags & IFF_BACKPRESSURE))
+		return;
+
+	txq = netdev_get_tx_queue(tun->dev, tfile->queue_index);
 
 	/* Paired with smp_mb__after_atomic() in tun_net_xmit() */
 	smp_mb();
@@ -2764,7 +2782,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 	struct tun_struct *tun;
 	struct tun_file *tfile = file->private_data;
 	struct net_device *dev;
-	int err;
+	int err, i;
 
 	if (tfile->detached)
 		return -EINVAL;
@@ -2894,7 +2912,8 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 	 * xoff state.
 	 */
 	if (netif_running(tun->dev))
-		netif_tx_wake_all_queues(tun->dev);
+		for (i = 0; i < tun->numqueues; i++)
+			tun_force_wake_queue(tun, rtnl_dereference(tun->tfiles[i]));
 
 	strscpy(ifr->ifr_name, tun->dev->name);
 	return 0;
@@ -3690,15 +3709,9 @@ static int tun_queue_resize(struct tun_struct *tun)
 					  dev->tx_queue_len, GFP_KERNEL,
 					  tun_ptr_free);
 
-	if (!ret) {
-		for (i = 0; i < tun->numqueues; i++) {
-			tfile = rtnl_dereference(tun->tfiles[i]);
-			spin_lock(&tfile->tx_ring.consumer_lock);
-			netif_wake_subqueue(tun->dev, tfile->queue_index);
-			tfile->cons_cnt = 0;
-			spin_unlock(&tfile->tx_ring.consumer_lock);
-		}
-	}
+	if (!ret)
+		for (i = 0; i < tun->numqueues; i++)
+			tun_force_wake_queue(tun, rtnl_dereference(tun->tfiles[i]));
 
 	kfree(rings);
 	return ret;
diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
index 79d53c7a1ebd..73a77141315c 100644
--- a/include/uapi/linux/if_tun.h
+++ b/include/uapi/linux/if_tun.h
@@ -69,6 +69,7 @@
 #define IFF_NAPI_FRAGS	0x0020
 /* Used in TUNSETIFF to bring up tun/tap without carrier */
 #define IFF_NO_CARRIER	0x0040
+#define IFF_BACKPRESSURE	0x0080
 #define IFF_NO_PI	0x1000
 /* This flag has no real effect */
 #define IFF_ONE_QUEUE	0x2000
diff --git a/tools/include/uapi/linux/if_tun.h b/tools/include/uapi/linux/if_tun.h
index 2ec07de1d73b..97b670f5bc0a 100644
--- a/tools/include/uapi/linux/if_tun.h
+++ b/tools/include/uapi/linux/if_tun.h
@@ -67,6 +67,7 @@
 #define IFF_TAP		0x0002
 #define IFF_NAPI	0x0010
 #define IFF_NAPI_FRAGS	0x0020
+#define IFF_BACKPRESSURE	0x0080
 #define IFF_NO_PI	0x1000
 /* This flag has no real effect */
 #define IFF_ONE_QUEUE	0x2000
-- 
2.43.0