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? 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 >>> >> >
