mirror of
https://github.com/torvalds/linux.git
synced 2026-09-19 17:18:02 +02:00
Merge branch 'revert-tun-tap-vhost-net-apply-qdisc-backpressure-on-full-ptr_ring-to-reduce-tx-drops'
Simon Schippers says:
====================
Revert "tun/tap & vhost-net: apply qdisc backpressure on full ptr_ring to reduce TX drops"
Commit 1d6e569b7d ("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/
====================
Link: https://patch.msgid.link/20260728092240.250257-1-simon.schippers@tu-dortmund.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
commit
7e107ab85c
|
|
@ -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;
|
||||
|
||||
|
|
@ -1018,7 +1008,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 +1062,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 */
|
||||
|
|
@ -2147,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) {
|
||||
|
|
@ -2198,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)) {
|
||||
|
|
@ -2235,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;
|
||||
}
|
||||
|
|
@ -3690,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;
|
||||
}
|
||||
|
|
@ -3808,29 +3734,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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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. */
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user