On Mon, Feb 2, 2026 at 4:19 AM Simon Schippers <[email protected]> wrote: > > On 1/30/26 02:51, Jason Wang wrote: > > On Thu, Jan 29, 2026 at 5:25 PM Simon Schippers > > <[email protected]> wrote: > >> > >> On 1/29/26 02:14, Jason Wang wrote: > >>> On Wed, Jan 28, 2026 at 3:54 PM Simon Schippers > >>> <[email protected]> wrote: > >>>> > >>>> On 1/28/26 08:03, Jason Wang wrote: > >>>>> On Wed, Jan 28, 2026 at 12:48 AM Simon Schippers > >>>>> <[email protected]> wrote: > >>>>>> > >>>>>> On 1/23/26 10:54, Simon Schippers wrote: > >>>>>>> On 1/23/26 04:05, Jason Wang wrote: > >>>>>>>> On Thu, Jan 22, 2026 at 1:35 PM Jason Wang <[email protected]> > >>>>>>>> wrote: > >>>>>>>>> > >>>>>>>>> On Wed, Jan 21, 2026 at 5:33 PM Simon Schippers > >>>>>>>>> <[email protected]> wrote: > >>>>>>>>>> > >>>>>>>>>> On 1/9/26 07:02, Jason Wang wrote: > >>>>>>>>>>> On Thu, Jan 8, 2026 at 3:41 PM Simon Schippers > >>>>>>>>>>> <[email protected]> wrote: > >>>>>>>>>>>> > >>>>>>>>>>>> On 1/8/26 04:38, Jason Wang wrote: > >>>>>>>>>>>>> On Thu, Jan 8, 2026 at 5:06 AM Simon Schippers > >>>>>>>>>>>>> <[email protected]> wrote: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Introduce {tun,tap}_ring_consume() helpers that wrap > >>>>>>>>>>>>>> __ptr_ring_consume() > >>>>>>>>>>>>>> and wake the corresponding netdev subqueue when consuming an > >>>>>>>>>>>>>> entry frees > >>>>>>>>>>>>>> space in the underlying ptr_ring. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Stopping of the netdev queue when the ptr_ring is full will be > >>>>>>>>>>>>>> introduced > >>>>>>>>>>>>>> in an upcoming commit. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Co-developed-by: Tim Gebauer <[email protected]> > >>>>>>>>>>>>>> Signed-off-by: Tim Gebauer <[email protected]> > >>>>>>>>>>>>>> Signed-off-by: Simon Schippers <[email protected]> > >>>>>>>>>>>>>> --- > >>>>>>>>>>>>>> drivers/net/tap.c | 23 ++++++++++++++++++++++- > >>>>>>>>>>>>>> drivers/net/tun.c | 25 +++++++++++++++++++++++-- > >>>>>>>>>>>>>> 2 files changed, 45 insertions(+), 3 deletions(-) > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> diff --git a/drivers/net/tap.c b/drivers/net/tap.c > >>>>>>>>>>>>>> index 1197f245e873..2442cf7ac385 100644 > >>>>>>>>>>>>>> --- a/drivers/net/tap.c > >>>>>>>>>>>>>> +++ b/drivers/net/tap.c > >>>>>>>>>>>>>> @@ -753,6 +753,27 @@ static ssize_t tap_put_user(struct > >>>>>>>>>>>>>> tap_queue *q, > >>>>>>>>>>>>>> return ret ? ret : total; > >>>>>>>>>>>>>> } > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> +static void *tap_ring_consume(struct tap_queue *q) > >>>>>>>>>>>>>> +{ > >>>>>>>>>>>>>> + struct ptr_ring *ring = &q->ring; > >>>>>>>>>>>>>> + struct net_device *dev; > >>>>>>>>>>>>>> + void *ptr; > >>>>>>>>>>>>>> + > >>>>>>>>>>>>>> + spin_lock(&ring->consumer_lock); > >>>>>>>>>>>>>> + > >>>>>>>>>>>>>> + ptr = __ptr_ring_consume(ring); > >>>>>>>>>>>>>> + if (unlikely(ptr && > >>>>>>>>>>>>>> __ptr_ring_consume_created_space(ring, 1))) { > >>>>>>>>>>>>>> + rcu_read_lock(); > >>>>>>>>>>>>>> + dev = rcu_dereference(q->tap)->dev; > >>>>>>>>>>>>>> + netif_wake_subqueue(dev, q->queue_index); > >>>>>>>>>>>>>> + rcu_read_unlock(); > >>>>>>>>>>>>>> + } > >>>>>>>>>>>>>> + > >>>>>>>>>>>>>> + spin_unlock(&ring->consumer_lock); > >>>>>>>>>>>>>> + > >>>>>>>>>>>>>> + return ptr; > >>>>>>>>>>>>>> +} > >>>>>>>>>>>>>> + > >>>>>>>>>>>>>> static ssize_t tap_do_read(struct tap_queue *q, > >>>>>>>>>>>>>> struct iov_iter *to, > >>>>>>>>>>>>>> int noblock, struct sk_buff *skb) > >>>>>>>>>>>>>> @@ -774,7 +795,7 @@ static ssize_t tap_do_read(struct > >>>>>>>>>>>>>> tap_queue *q, > >>>>>>>>>>>>>> TASK_INTERRUPTIBLE); > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> /* Read frames from the queue */ > >>>>>>>>>>>>>> - skb = ptr_ring_consume(&q->ring); > >>>>>>>>>>>>>> + skb = tap_ring_consume(q); > >>>>>>>>>>>>>> if (skb) > >>>>>>>>>>>>>> break; > >>>>>>>>>>>>>> if (noblock) { > >>>>>>>>>>>>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c > >>>>>>>>>>>>>> index 8192740357a0..7148f9a844a4 100644 > >>>>>>>>>>>>>> --- a/drivers/net/tun.c > >>>>>>>>>>>>>> +++ b/drivers/net/tun.c > >>>>>>>>>>>>>> @@ -2113,13 +2113,34 @@ static ssize_t tun_put_user(struct > >>>>>>>>>>>>>> tun_struct *tun, > >>>>>>>>>>>>>> return total; > >>>>>>>>>>>>>> } > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> +static void *tun_ring_consume(struct tun_file *tfile) > >>>>>>>>>>>>>> +{ > >>>>>>>>>>>>>> + struct ptr_ring *ring = &tfile->tx_ring; > >>>>>>>>>>>>>> + struct net_device *dev; > >>>>>>>>>>>>>> + void *ptr; > >>>>>>>>>>>>>> + > >>>>>>>>>>>>>> + spin_lock(&ring->consumer_lock); > >>>>>>>>>>>>>> + > >>>>>>>>>>>>>> + ptr = __ptr_ring_consume(ring); > >>>>>>>>>>>>>> + if (unlikely(ptr && > >>>>>>>>>>>>>> __ptr_ring_consume_created_space(ring, 1))) { > >>>>>>>>>>>>> > >>>>>>>>>>>>> I guess it's the "bug" I mentioned in the previous patch that > >>>>>>>>>>>>> leads to > >>>>>>>>>>>>> the check of __ptr_ring_consume_created_space() here. If it's > >>>>>>>>>>>>> true, > >>>>>>>>>>>>> another call to tweak the current API. > >>>>>>>>>>>>> > >>>>>>>>>>>>>> + rcu_read_lock(); > >>>>>>>>>>>>>> + dev = rcu_dereference(tfile->tun)->dev; > >>>>>>>>>>>>>> + netif_wake_subqueue(dev, tfile->queue_index); > >>>>>>>>>>>>> > >>>>>>>>>>>>> This would cause the producer TX_SOFTIRQ to run on the same cpu > >>>>>>>>>>>>> which > >>>>>>>>>>>>> I'm not sure is what we want. > >>>>>>>>>>>> > >>>>>>>>>>>> What else would you suggest calling to wake the queue? > >>>>>>>>>>> > >>>>>>>>>>> I don't have a good method in my mind, just want to point out its > >>>>>>>>>>> implications. > >>>>>>>>>> > >>>>>>>>>> I have to admit I'm a bit stuck at this point, particularly with > >>>>>>>>>> this > >>>>>>>>>> aspect. > >>>>>>>>>> > >>>>>>>>>> What is the correct way to pass the producer CPU ID to the > >>>>>>>>>> consumer? > >>>>>>>>>> Would it make sense to store smp_processor_id() in the tfile inside > >>>>>>>>>> tun_net_xmit(), or should it instead be stored in the skb (similar > >>>>>>>>>> to the > >>>>>>>>>> XDP bit)? In the latter case, my concern is that this information > >>>>>>>>>> may > >>>>>>>>>> already be significantly outdated by the time it is used. > >>>>>>>>>> > >>>>>>>>>> Based on that, my idea would be for the consumer to wake the > >>>>>>>>>> producer by > >>>>>>>>>> invoking a new function (e.g., tun_wake_queue()) on the producer > >>>>>>>>>> CPU via > >>>>>>>>>> smp_call_function_single(). > >>>>>>>>>> Is this a reasonable approach? > >>>>>>>>> > >>>>>>>>> I'm not sure but it would introduce costs like IPI. > >>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> More generally, would triggering TX_SOFTIRQ on the consumer CPU be > >>>>>>>>>> considered a deal-breaker for the patch set? > >>>>>>>>> > >>>>>>>>> It depends on whether or not it has effects on the performance. > >>>>>>>>> Especially when vhost is pinned. > >>>>>>>> > >>>>>>>> I meant we can benchmark to see the impact. For example, pin vhost to > >>>>>>>> a specific CPU and the try to see the impact of the TX_SOFTIRQ. > >>>>>>>> > >>>>>>>> Thanks > >>>>>>>> > >>>>>>> > >>>>>>> I ran benchmarks with vhost pinned to CPU 0 using taskset -p -c 0 ... > >>>>>>> for both the stock and patched versions. The benchmarks were run with > >>>>>>> the full patch series applied, since testing only patches 1-3 would > >>>>>>> not > >>>>>>> be meaningful - the queue is never stopped in that case, so no > >>>>>>> TX_SOFTIRQ is triggered. > >>>>>>> > >>>>>>> Compared to the non-pinned CPU benchmarks in the cover letter, > >>>>>>> performance is lower for pktgen with a single thread but higher with > >>>>>>> four threads. The results show no regression for the patched version, > >>>>>>> with even slight performance improvements observed: > >>>>>>> > >>>>>>> +-------------------------+-----------+----------------+ > >>>>>>> | pktgen benchmarks to | Stock | Patched with | > >>>>>>> | Debian VM, i5 6300HQ, | | fq_codel qdisc | > >>>>>>> | 100M packets | | | > >>>>>>> | vhost pinned to core 0 | | | > >>>>>>> +-----------+-------------+-----------+----------------+ > >>>>>>> | TAP | Transmitted | 452 Kpps | 454 Kpps | > >>>>>>> | + +-------------+-----------+----------------+ > >>>>>>> | vhost-net | Lost | 1154 Kpps | 0 | > >>>>>>> +-----------+-------------+-----------+----------------+ > >>>>>>> > >>>>>>> +-------------------------+-----------+----------------+ > >>>>>>> | pktgen benchmarks to | Stock | Patched with | > >>>>>>> | Debian VM, i5 6300HQ, | | fq_codel qdisc | > >>>>>>> | 100M packets | | | > >>>>>>> | vhost pinned to core 0 | | | > >>>>>>> | *4 threads* | | | > >>>>>>> +-----------+-------------+-----------+----------------+ > >>>>>>> | TAP | Transmitted | 71 Kpps | 79 Kpps | > >>>>>>> | + +-------------+-----------+----------------+ > >>>>>>> | vhost-net | Lost | 1527 Kpps | 0 | > >>>>>>> +-----------+-------------+-----------+----------------+ > >>>>> > >>>>> The PPS seems to be low. I'd suggest using testpmd (rxonly) mode in > >>>>> the guest or an xdp program that did XDP_DROP in the guest. > >>>> > >>>> I forgot to mention that these PPS values are per thread. > >>>> So overall we have 71 Kpps * 4 = 284 Kpps and 79 Kpps * 4 = 326 Kpps, > >>>> respectively. For packet loss, that comes out to 1154 Kpps * 4 = > >>>> 4616 Kpps and 0, respectively. > >>>> > >>>> Sorry about that! > >>>> > >>>> The pktgen benchmarks with a single thread look fine, right? > >>> > >>> Still looks very low. E.g I just have a run of pktgen (using > >>> pktgen_sample03_burst_single_flow.sh) without a XDP_DROP in the guest, > >>> I can get 1Mpps. > >> > >> Keep in mind that I am using an older CPU (i5-6300HQ). For the > >> single-threaded tests I always used pktgen_sample01_simple.sh, and for > >> the multi-threaded tests I always used pktgen_sample02_multiqueue.sh. > >> > >> Using pktgen_sample03_burst_single_flow.sh as you did fails for me (even > >> though the same parameters work fine for sample01 and sample02): > >> > >> samples/pktgen/pktgen_sample03_burst_single_flow.sh -i tap0 -m > >> 52:54:00:12:34:56 -d 10.0.0.2 -n 100000000 > >> /samples/pktgen/functions.sh: line 79: echo: write error: Operation not > >> supported > >> ERROR: Write error(1) occurred > >> cmd: "burst 32 > /proc/net/pktgen/tap0@0" > >> > >> ...and I do not know what I am doing wrong, even after looking at > >> Documentation/networking/pktgen.rst. Every burst size except 1 fails. > >> Any clues? > > > > Please use -b 0, and I'm Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz. > > I tried using "-b 0", and while it worked, there was no noticeable > performance improvement. > > > > > Another thing I can think of is to disable > > > > 1) mitigations in both guest and host > > 2) any kernel debug features in both host and guest > > I also rebuilt the kernel with everything disabled under > "Kernel hacking", but that didn’t make any difference either. > > Because of this, I ran "pktgen_sample01_simple.sh" and > "pktgen_sample02_multiqueue.sh" on my AMD Ryzen 5 5600X system. The > results were about 374 Kpps with TAP and 1192 Kpps with TAP+vhost_net, > with very similar performance between the stock and patched kernels. > > Personally, I think the low performance is to blame on the hardware.
Let's double confirm this by: 1) make sure pktgen is using 100% CPU 2) Perf doesn't show anything strange for pktgen thread Thanks > > Thanks! > > > > > Thanks > > > >> > >> Thanks! > >> > >>> > >>>> > >>>> I'll still look into using an XDP program that does XDP_DROP in the > >>>> guest. > >>>> > >>>> Thanks! > >>> > >>> Thanks > >>> > >>>> > >>>>> > >>>>>>> > >>>>>>> +------------------------+-------------+----------------+ > >>>>>>> | iperf3 TCP benchmarks | Stock | Patched with | > >>>>>>> | to Debian VM 120s | | fq_codel qdisc | > >>>>>>> | vhost pinned to core 0 | | | > >>>>>>> +------------------------+-------------+----------------+ > >>>>>>> | TAP | 22.0 Gbit/s | 22.0 Gbit/s | > >>>>>>> | + | | | > >>>>>>> | vhost-net | | | > >>>>>>> +------------------------+-------------+----------------+ > >>>>>>> > >>>>>>> +---------------------------+-------------+----------------+ > >>>>>>> | iperf3 TCP benchmarks | Stock | Patched with | > >>>>>>> | to Debian VM 120s | | fq_codel qdisc | > >>>>>>> | vhost pinned to core 0 | | | > >>>>>>> | *4 iperf3 client threads* | | | > >>>>>>> +---------------------------+-------------+----------------+ > >>>>>>> | TAP | 21.4 Gbit/s | 21.5 Gbit/s | > >>>>>>> | + | | | > >>>>>>> | vhost-net | | | > >>>>>>> +---------------------------+-------------+----------------+ > >>>>>> > >>>>>> What are your thoughts on this? > >>>>>> > >>>>>> Thanks! > >>>>>> > >>>>>> > >>>>> > >>>>> Thanks > >>>>> > >>>> > >>> > >> > > >
