[PATCH net 0/4] Revert "tun/tap & vhost-net: apply qdisc backpressure on full ptr_ring to reduce TX drops"

[PATCH net 0/4] Revert "tun/tap & vhost-net: apply qdisc backpressure on full ptr_ring to reduce TX drops"

[Simon Schippers]

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 causes a
significant throughput drop in an IPv6 multicast testcase. The regression
can be pinpointed to multiple iperf3 TCP threads sending: for 8 threads
the throughput dropped from 13.5 Gbit/s to 9.13 Gbit/s.

Therefore this series reverts the qdisc backpressure work.

Making the backpressure opt-in via a new IFF_BACKPRESSURE flag was
proposed in [1], but a new IFF_* flag needs more review scrutiny than is
available at the moment, so a revert was requested instead. The opt-in
will be resubmitted for net-next later.

[1] Link: https://lore.kernel.org/netdev/20260709095511.168235-1-simon.schippers@tu-dortmund.de/

Reported-by: Brett Sheffield <brett@librecast.net>
Closes: https://lore.kernel.org/netdev/akVnoOYQOrt8k-Gu@karahi.librecast.net/

Simon Schippers (4):
  Revert "tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is
    present"
  Revert "ptr_ring: move free-space check into separate helper"
  Revert "vhost-net: wake queue of tun/tap after ptr_ring consume"
  Revert "tun/tap: add ptr_ring consume helper with netdev queue wakeup"

 drivers/net/tun.c        | 109 +++------------------------------------
 drivers/vhost/net.c      |  21 +++-----
 include/linux/if_tun.h   |   3 --
 include/linux/ptr_ring.h |  20 +------
 4 files changed, 14 insertions(+), 139 deletions(-)


base-commit: b14361aca6350ff7907b0e9903c7b94dc7d5d4a0
-- 
2.43.0

[PATCH net 1/4] Revert "tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present"

[Simon Schippers]

This reverts commit 1d6e569b7d0c0b2736636749e4be0a27f3cefcb3.

The commit stops the netdev queue when the ptr_ring is full instead of
dropping the packet. My own tests showed no relevant regression, but on
Brett Sheffield's librecast testbed an IPv6 multicast testcase got
slower. With 8 iperf3 TCP threads sending, the throughput dropped from
13.5 Gbit/s to 9.13 Gbit/s.

Reported-by: Brett Sheffield <brett@librecast.net>
Closes: https://lore.kernel.org/netdev/akVnoOYQOrt8k-Gu@karahi.librecast.net/
Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
 drivers/net/tun.c | 25 ++-----------------------
 1 file changed, 2 insertions(+), 23 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index ffbe6f13fb1f..ec5573f545af 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1018,7 +1018,6 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct netdev_queue *queue;
 	struct tun_file *tfile;
 	int len = skb->len;
-	int ret;
 
 	rcu_read_lock();
 	tfile = rcu_dereference(tun->tfiles[txq]);
@@ -1073,33 +1072,13 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	nf_reset_ct(skb);
 
-	queue = netdev_get_tx_queue(dev, txq);
-
-	spin_lock(&tfile->tx_ring.producer_lock);
-	ret = __ptr_ring_produce(&tfile->tx_ring, skb);
-	if (!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() */
-		smp_mb__after_atomic();
-		if (!__ptr_ring_check_produce(&tfile->tx_ring))
-			netif_tx_wake_queue(queue);
-	}
-	spin_unlock(&tfile->tx_ring.producer_lock);
-
-	if (ret) {
-		/* This should be a rare case if a qdisc is present, but
-		 * can happen due to lltx.
-		 * Since skb_tx_timestamp(), skb_orphan(),
-		 * run_ebpf_filter() and pskb_trim() could have tinkered
-		 * with the SKB, returning NETDEV_TX_BUSY is unsafe and
-		 * we must drop instead.
-		 */
+	if (ptr_ring_produce(&tfile->tx_ring, skb)) {
 		drop_reason = SKB_DROP_REASON_FULL_RING;
 		goto drop;
 	}
 
 	/* dev->lltx requires to do our own update of trans_start */
+	queue = netdev_get_tx_queue(dev, txq);
 	txq_trans_cond_update(queue);
 
 	/* Notify and wake up reader process */
-- 
2.43.0

Re: [PATCH net 1/4] Revert "tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present"

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete Revert / Orphaned Memory Barrier
--

--- Patch [1]: [PATCH net 1/4] Revert "tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present" ---
commit 18e68b71511b6b4155def5921c5f149c1b6e74e9
Author: Simon Schippers <simon.schippers@tu-dortmund.de>

Revert "tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present"

This commit reverts a previous change that stopped the netdev queue when the
ptr_ring is full. It goes back to dropping packets instead, resolving a reported
performance regression in a multicast testcase.

Link: https://lore.kernel.org/netdev/akVnoOYQOrt8k-Gu@karahi.librecast.net/

> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index ffbe6f13fb1fa..ec5573f545af6 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
[ ... ]
> @@ -1073,33 +1072,13 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>  
>  	nf_reset_ct(skb);
>  
> -	queue = netdev_get_tx_queue(dev, txq);
> -
> -	spin_lock(&tfile->tx_ring.producer_lock);
> -	ret = __ptr_ring_produce(&tfile->tx_ring, skb);
> -	if (!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() */
> -		smp_mb__after_atomic();

[Severity: Medium]
Does this revert leave an orphaned memory barrier in __tun_wake_queue()?

Since the producer-side queue stopping logic and smp_mb__after_atomic() were
removed here in tun_net_xmit(), the consumer-side logic in __tun_wake_queue()
now executes an unconditional full memory barrier on every consumed packet:

drivers/net/tun.c:__tun_wake_queue() {
    ...
    /* Paired with smp_mb__after_atomic() in tun_net_xmit() */
    smp_mb();
    if (netif_tx_queue_stopped(txq)) {
    ...
}

Should the corresponding wakeup logic and smp_mb() in __tun_wake_queue() also
be removed to complete the revert and avoid unnecessary overhead?

> -		if (!__ptr_ring_check_produce(&tfile->tx_ring))
> -			netif_tx_wake_queue(queue);
> -	}
> -	spin_unlock(&tfile->tx_ring.producer_lock);
> -
> -	if (ret) {
> -		/* This should be a rare case if a qdisc is present, but
> -		 * can happen due to lltx.
> -		 * Since skb_tx_timestamp(), skb_orphan(),
> -		 * run_ebpf_filter() and pskb_trim() could have tinkered
> -		 * with the SKB, returning NETDEV_TX_BUSY is unsafe and
> -		 * we must drop instead.
> -		 */
> +	if (ptr_ring_produce(&tfile->tx_ring, skb)) {
>  		drop_reason = SKB_DROP_REASON_FULL_RING;
>  		goto drop;
>  	}

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

[PATCH net 2/4] Revert "ptr_ring: move free-space check into separate helper"

[Simon Schippers]

This reverts commit fba362c17d9d9211fc51f272156bb84fc23bdf98.

__ptr_ring_check_produce() has no users left after reverting
commit 1d6e569b7d0c ("tun/tap & vhost-net: avoid ptr_ring tail-drop
when a qdisc is present").

Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
 include/linux/ptr_ring.h | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
index c95e891903f0..d2c3629bbe45 100644
--- a/include/linux/ptr_ring.h
+++ b/include/linux/ptr_ring.h
@@ -96,20 +96,6 @@ static inline bool ptr_ring_full_bh(struct ptr_ring *r)
 	return ret;
 }
 
-/* Note: callers invoking this in a loop must use a compiler barrier,
- * for example cpu_relax(). Callers must hold producer_lock.
- */
-static inline int __ptr_ring_check_produce(struct ptr_ring *r)
-{
-	if (unlikely(!r->size))
-		return -EINVAL;
-
-	if (data_race(r->queue[r->producer]))
-		return -ENOSPC;
-
-	return 0;
-}
-
 /* Note: callers invoking this in a loop must use a compiler barrier,
  * for example cpu_relax(). Callers must hold producer_lock.
  * Callers are responsible for making sure pointer that is being queued
@@ -117,10 +103,8 @@ static inline int __ptr_ring_check_produce(struct ptr_ring *r)
  */
 static inline int __ptr_ring_produce(struct ptr_ring *r, void *ptr)
 {
-	int p = __ptr_ring_check_produce(r);
-
-	if (p)
-		return p;
+	if (unlikely(!r->size) || data_race(r->queue[r->producer]))
+		return -ENOSPC;
 
 	/* Make sure the pointer we are storing points to a valid data. */
 	/* Pairs with the dependency ordering in __ptr_ring_consume. */
-- 
2.43.0

[PATCH net 3/4] Revert "vhost-net: wake queue of tun/tap after ptr_ring consume"

[Simon Schippers]

This reverts commit baf808fe4fcd35767ab732b4ab2ea80dabfd97a6.

There is no netdev queue left to wake after reverting
commit 1d6e569b7d0c ("tun/tap & vhost-net: avoid ptr_ring tail-drop
when a qdisc is present").

Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
 drivers/net/tun.c      | 23 -----------------------
 drivers/vhost/net.c    | 21 ++++++---------------
 include/linux/if_tun.h |  3 ---
 3 files changed, 6 insertions(+), 41 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index ec5573f545af..39abc3078097 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -3787,29 +3787,6 @@ struct ptr_ring *tun_get_tx_ring(struct file *file)
 }
 EXPORT_SYMBOL_GPL(tun_get_tx_ring);
 
-/* Callers must hold ring.consumer_lock */
-void tun_wake_queue(struct file *file, int consumed)
-{
-	struct tun_file *tfile;
-	struct tun_struct *tun;
-
-	if (file->f_op != &tun_fops)
-		return;
-
-	tfile = file->private_data;
-	if (!tfile)
-		return;
-
-	rcu_read_lock();
-
-	tun = rcu_dereference(tfile->tun);
-	if (tun)
-		__tun_wake_queue(tun, tfile, consumed);
-
-	rcu_read_unlock();
-}
-EXPORT_SYMBOL_GPL(tun_wake_queue);
-
 module_init(tun_init);
 module_exit(tun_cleanup);
 MODULE_DESCRIPTION(DRV_DESCRIPTION);
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 3e72b9c6af0c..6949b704166d 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -176,21 +176,13 @@ static void *vhost_net_buf_consume(struct vhost_net_buf *rxq)
 	return ret;
 }
 
-static int vhost_net_buf_produce(struct sock *sk,
-				 struct vhost_net_virtqueue *nvq)
+static int vhost_net_buf_produce(struct vhost_net_virtqueue *nvq)
 {
-	struct file *file = sk->sk_socket->file;
 	struct vhost_net_buf *rxq = &nvq->rxq;
 
 	rxq->head = 0;
-	spin_lock(&nvq->rx_ring->consumer_lock);
-	rxq->tail = __ptr_ring_consume_batched(nvq->rx_ring, rxq->queue,
-					       VHOST_NET_BATCH);
-
-	if (rxq->tail)
-		tun_wake_queue(file, rxq->tail);
-
-	spin_unlock(&nvq->rx_ring->consumer_lock);
+	rxq->tail = ptr_ring_consume_batched(nvq->rx_ring, rxq->queue,
+					      VHOST_NET_BATCH);
 	return rxq->tail;
 }
 
@@ -217,15 +209,14 @@ static int vhost_net_buf_peek_len(void *ptr)
 	return __skb_array_len_with_tag(ptr);
 }
 
-static int vhost_net_buf_peek(struct sock *sk,
-			      struct vhost_net_virtqueue *nvq)
+static int vhost_net_buf_peek(struct vhost_net_virtqueue *nvq)
 {
 	struct vhost_net_buf *rxq = &nvq->rxq;
 
 	if (!vhost_net_buf_is_empty(rxq))
 		goto out;
 
-	if (!vhost_net_buf_produce(sk, nvq))
+	if (!vhost_net_buf_produce(nvq))
 		return 0;
 
 out:
@@ -1013,7 +1004,7 @@ static int peek_head_len(struct vhost_net_virtqueue *rvq, struct sock *sk)
 	unsigned long flags;
 
 	if (rvq->rx_ring)
-		return vhost_net_buf_peek(sk, rvq);
+		return vhost_net_buf_peek(rvq);
 
 	spin_lock_irqsave(&sk->sk_receive_queue.lock, flags);
 	head = skb_peek(&sk->sk_receive_queue);
diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
index 5f3e206c7a73..80166eb62f41 100644
--- a/include/linux/if_tun.h
+++ b/include/linux/if_tun.h
@@ -22,7 +22,6 @@ struct tun_msg_ctl {
 #if defined(CONFIG_TUN) || defined(CONFIG_TUN_MODULE)
 struct socket *tun_get_socket(struct file *);
 struct ptr_ring *tun_get_tx_ring(struct file *file);
-void tun_wake_queue(struct file *file, int consumed);
 
 static inline bool tun_is_xdp_frame(void *ptr)
 {
@@ -56,8 +55,6 @@ static inline struct ptr_ring *tun_get_tx_ring(struct file *f)
 	return ERR_PTR(-EINVAL);
 }
 
-static inline void tun_wake_queue(struct file *f, int consumed) {}
-
 static inline bool tun_is_xdp_frame(void *ptr)
 {
 	return false;
-- 
2.43.0

[PATCH net 4/4] Revert "tun/tap: add ptr_ring consume helper with netdev queue wakeup"

[Simon Schippers]

This reverts commit d4c22d70d7253dd727c71484c58d504f6c630343.

There is no netdev queue left to wake after reverting
commit 1d6e569b7d0c ("tun/tap & vhost-net: avoid ptr_ring tail-drop
when a qdisc is present").

Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
---
 drivers/net/tun.c | 61 ++++-------------------------------------------
 1 file changed, 4 insertions(+), 57 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 39abc3078097..fed9dfdfcc3b 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -145,8 +145,6 @@ struct tun_file {
 	struct list_head next;
 	struct tun_struct *detached;
 	struct ptr_ring tx_ring;
-	/* Protected by tx_ring.consumer_lock */
-	int cons_cnt;
 	struct xdp_rxq_info xdp_rxq;
 };
 
@@ -590,13 +588,8 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
 		rcu_assign_pointer(tun->tfiles[index],
 				   tun->tfiles[tun->numqueues - 1]);
 		ntfile = rtnl_dereference(tun->tfiles[index]);
-		spin_lock(&ntfile->tx_ring.consumer_lock);
 		ntfile->queue_index = index;
 		ntfile->xdp_rxq.queue_index = index;
-		ntfile->cons_cnt = 0;
-		if (__ptr_ring_empty(&ntfile->tx_ring))
-			netif_wake_subqueue(tun->dev, index);
-		spin_unlock(&ntfile->tx_ring.consumer_lock);
 		rcu_assign_pointer(tun->tfiles[tun->numqueues - 1],
 				   NULL);
 
@@ -737,9 +730,6 @@ 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;
 
@@ -2126,46 +2116,13 @@ static ssize_t tun_put_user(struct tun_struct *tun,
 	return total;
 }
 
-/* Callers must hold ring.consumer_lock */
-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);
-
-	/* Paired with smp_mb__after_atomic() in tun_net_xmit() */
-	smp_mb();
-	if (netif_tx_queue_stopped(txq)) {
-		tfile->cons_cnt += consumed;
-		if (tfile->cons_cnt >= tfile->tx_ring.size / 2 ||
-		    __ptr_ring_empty(&tfile->tx_ring)) {
-			netif_tx_wake_queue(txq);
-			tfile->cons_cnt = 0;
-		}
-	}
-}
-
-static void *tun_ring_consume(struct tun_struct *tun, struct tun_file *tfile)
-{
-	void *ptr;
-
-	spin_lock(&tfile->tx_ring.consumer_lock);
-	ptr = __ptr_ring_consume(&tfile->tx_ring);
-	if (ptr)
-		__tun_wake_queue(tun, tfile, 1);
-
-	spin_unlock(&tfile->tx_ring.consumer_lock);
-	return ptr;
-}
-
-static void *tun_ring_recv(struct tun_struct *tun, struct tun_file *tfile,
-			   int noblock, int *err)
+static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err)
 {
 	DECLARE_WAITQUEUE(wait, current);
 	void *ptr = NULL;
 	int error = 0;
 
-	ptr = tun_ring_consume(tun, tfile);
+	ptr = ptr_ring_consume(&tfile->tx_ring);
 	if (ptr)
 		goto out;
 	if (noblock) {
@@ -2177,7 +2134,7 @@ static void *tun_ring_recv(struct tun_struct *tun, struct tun_file *tfile,
 
 	while (1) {
 		set_current_state(TASK_INTERRUPTIBLE);
-		ptr = tun_ring_consume(tun, tfile);
+		ptr = ptr_ring_consume(&tfile->tx_ring);
 		if (ptr)
 			break;
 		if (signal_pending(current)) {
@@ -2214,7 +2171,7 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
 
 	if (!ptr) {
 		/* Read frames from ring */
-		ptr = tun_ring_recv(tun, tfile, noblock, &err);
+		ptr = tun_ring_recv(tfile, noblock, &err);
 		if (!ptr)
 			return err;
 	}
@@ -3669,16 +3626,6 @@ 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);
-		}
-	}
-
 	kfree(rings);
 	return ret;
 }
-- 
2.43.0

Re: [PATCH net 0/4] Revert "tun/tap & vhost-net: apply qdisc backpressure on full ptr_ring to reduce TX drops"

[Michael S. Tsirkin]

On Tue, Jul 28, 2026 at 11:22:36AM +0200, Simon Schippers wrote:
> 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 causes a
> significant throughput drop in an IPv6 multicast testcase. The regression
> can be pinpointed to multiple iperf3 TCP threads sending: for 8 threads
> the throughput dropped from 13.5 Gbit/s to 9.13 Gbit/s.
> 
> Therefore this series reverts the qdisc backpressure work.
> 
> Making the backpressure opt-in via a new IFF_BACKPRESSURE flag was
> proposed in [1], but a new IFF_* flag needs more review scrutiny than is
> available at the moment, so a revert was requested instead. The opt-in
> will be resubmitted for net-next later.


Acked-by: Michael S. Tsirkin <mst@redhat.com>


Thanks!

> [1] Link: https://lore.kernel.org/netdev/20260709095511.168235-1-simon.schippers@tu-dortmund.de/
> 
> Reported-by: Brett Sheffield <brett@librecast.net>
> Closes: https://lore.kernel.org/netdev/akVnoOYQOrt8k-Gu@karahi.librecast.net/
> 
> Simon Schippers (4):
>   Revert "tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is
>     present"
>   Revert "ptr_ring: move free-space check into separate helper"
>   Revert "vhost-net: wake queue of tun/tap after ptr_ring consume"
>   Revert "tun/tap: add ptr_ring consume helper with netdev queue wakeup"
> 
>  drivers/net/tun.c        | 109 +++------------------------------------
>  drivers/vhost/net.c      |  21 +++-----
>  include/linux/if_tun.h   |   3 --
>  include/linux/ptr_ring.h |  20 +------
>  4 files changed, 14 insertions(+), 139 deletions(-)
> 
> 
> base-commit: b14361aca6350ff7907b0e9903c7b94dc7d5d4a0
> -- 
> 2.43.0