Re: [REGRESSION][BISECTED] tun/tap & vhost-net: multi-threaded network performance

[Simon Schippers]

On 7/3/26 00:55, Michael S. Tsirkin wrote:
> Maybe it's the supposedly rare case? Does this change anything
> for you?

In the "rare case" we would have packet drops, visible by iperf3 TCP
retransmissions. But these packet drops don't occur as I stated before.

Also, we are still not allowed to return NETDEV_TX_BUSY here, because
run_ebpf_filter() and pskb_trim() could have tinkered with the SKB.

> 
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index bfa49fa9e3a1..bacd89460078 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]);
> @@ -1064,19 +1063,24 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>  		goto drop;
>  	}
>  
> -	skb_tx_timestamp(skb);
> -
> -	/* Orphan the skb - required as we might hang on to it
> -	 * for indefinite time.
> -	 */
> -	skb_orphan(skb);
> -
> -	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 (__ptr_ring_check_produce(&tfile->tx_ring)) {
> +		spin_unlock(&tfile->tx_ring.producer_lock);
> +		netif_tx_stop_queue(queue);
> +		smp_mb__after_atomic();
> +		if (!__ptr_ring_check_produce(&tfile->tx_ring))
> +			netif_tx_wake_queue(queue);
> +		rcu_read_unlock();
> +		return NETDEV_TX_BUSY;
> +	}
> +
> +	skb_tx_timestamp(skb);
> +	skb_orphan(skb);
> +	nf_reset_ct(skb);
> +
> +	__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);
> @@ -1087,18 +1091,6 @@ 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.
> -		 * 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.
> -		 */
> -		drop_reason = SKB_DROP_REASON_FULL_RING;
> -		goto drop;
> -	}
> -
>  	/* dev->lltx requires to do our own update of trans_start */
>  	txq_trans_cond_update(queue);
>  
>