Linux Documentation
 help / color / mirror / Atom feed
From: Brett Sheffield <brett@librecast.net>
To: Simon Schippers <simon.schippers@tu-dortmund.de>
Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
	Jason Wang <jasowangio@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	netdev@vger.kernel.org, Simon Horman <horms@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	Tim Gebauer <tim.gebauer@tu-dortmund.de>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net v3] tun/tap & vhost-net: make qdisc backpressure opt-in via IFF_BACKPRESSURE
Date: Thu, 9 Jul 2026 18:06:09 +0200	[thread overview]
Message-ID: <ak_G8auk2xI8PtlM@karahi.librecast.net> (raw)
In-Reply-To: <20260709095511.168235-1-simon.schippers@tu-dortmund.de>

On 2026-07-09 11:55, 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 shows a
> significant performance drop in a IPv6 multicast testcase. The regression
> can be pinpointed when multiple iperf3 TCP 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. Setting
> IFF_BACKPRESSURE makes an attached qdisc effective by stopping the queue
> instead of tail-dropping when the internal ring is full.
> 
> 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>

Tested v3 patch applied to 7.2.0-rc2.  OK.

Tested-by: Brett A C Sheffield <bacs@librecast.net>

> ---
> V2 -> V3:
> - As suggested by MST: Clarify in tuntap.rst and the UAPI header what
>   enabling IFF_BACKPRESSURE opts into: an attached qdisc becomes effective
>   instead of the driver tail-dropping when the internal ring is full.
> - Avoid lines over 75 characters.
> - Update comment in tun_net_xmit() to include IFF_BACKPRESSURE.
> - Brett: Update in commit message that the referenced tests were TCP.
> 
> 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.
> 
> V1: https://lore.kernel.org/netdev/20260704112058.95421-1-simon.schippers@tu-dortmund.de/T/#u
> V2: https://lore.kernel.org/netdev/20260706094242.115992-1-simon.schippers@tu-dortmund.de/T/#u
> 
>  Documentation/networking/tuntap.rst | 22 +++++++++++++
>  drivers/net/tun.c                   | 51 ++++++++++++++++++++---------
>  include/uapi/linux/if_tun.h         |  4 +++
>  tools/include/uapi/linux/if_tun.h   |  1 +
>  4 files changed, 62 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/networking/tuntap.rst b/Documentation/networking/tuntap.rst
> index 4d7087f727be..5921a924c2ae 100644
> --- a/Documentation/networking/tuntap.rst
> +++ b/Documentation/networking/tuntap.rst
> @@ -206,6 +206,28 @@ 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, so any attached qdisc is effectively bypassed and applications
> +only learn about congestion through those drops.
> +
> +With it, the kernel stops instead, letting the qdisc hold and schedule
> +packets, so its AQM, shaping and fairness actually apply. This helps
> +protocols like TCP, which cut throughput in reaction to packet drops.
> +With IFF_BACKPRESSURE, drops then only occur as a rare race. Backpressure
> +requires a qdisc to be attached and has no effect with noqueue.
> +
> +The txqueuelen 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 through a single
> +TUN/TAP queue, 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..5941e8f302ea 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() */
> @@ -1088,8 +1102,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>  	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.
> +		/* This should be a rare case if IFF_BACKPRESSURE is enabled and
> +		 * 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
> @@ -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;
> @@ -2893,8 +2911,12 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>  	/* Make sure persistent devices do not get stuck in
>  	 * xoff state.
>  	 */
> -	if (netif_running(tun->dev))
> -		netif_tx_wake_all_queues(tun->dev);
> +	if (netif_running(tun->dev)) {
> +		for (i = 0; i < tun->numqueues; i++) {
> +			tfile = rtnl_dereference(tun->tfiles[i]);
> +			tun_force_wake_queue(tun, tfile);
> +		}
> +	}
>  
>  	strscpy(ifr->ifr_name, tun->dev->name);
>  	return 0;
> @@ -3693,10 +3715,7 @@ static int tun_queue_resize(struct tun_struct *tun)
>  	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);
> +			tun_force_wake_queue(tun, tfile);
>  		}
>  	}
>  
> diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> index 79d53c7a1ebd..a0ddc50a7534 100644
> --- a/include/uapi/linux/if_tun.h
> +++ b/include/uapi/linux/if_tun.h
> @@ -69,6 +69,10 @@
>  #define IFF_NAPI_FRAGS	0x0020
>  /* Used in TUNSETIFF to bring up tun/tap without carrier */
>  #define IFF_NO_CARRIER	0x0040
> +/* Stop the queue instead of dropping when the internal ring is full, so an
> + * attached qdisc applies backpressure instead of being bypassed.
> + */
> +#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
> 

-- 
Brett Sheffield (he/him)
Librecast - Decentralising the Internet with Multicast
https://librecast.net/
https://blog.brettsheffield.com/

      reply	other threads:[~2026-07-09 16:06 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09  9:55 [PATCH net v3] tun/tap & vhost-net: make qdisc backpressure opt-in via IFF_BACKPRESSURE Simon Schippers
2026-07-09 16:06 ` Brett Sheffield [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ak_G8auk2xI8PtlM@karahi.librecast.net \
    --to=brett@librecast.net \
    --cc=andrew+netdev@lunn.ch \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jasowangio@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=simon.schippers@tu-dortmund.de \
    --cc=skhan@linuxfoundation.org \
    --cc=tim.gebauer@tu-dortmund.de \
    --cc=willemdebruijn.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox