Re: [PATCH net-next v7 3/9] tun/tap: add ptr_ring consume helper with netdev queue wakeup

[Simon Schippers <simon.schippers@tu-dortmund.de>]

On 2/15/26 11:38, Michael S. Tsirkin wrote:
> On Sat, Feb 14, 2026 at 08:51:53PM +0100, Simon Schippers wrote:
>> On 2/14/26 19:18, Michael S. Tsirkin wrote:
>>> On Sat, Feb 14, 2026 at 06:13:14PM +0100, Simon Schippers wrote:
>>>
>>> ...
>>>
>>>> Patched: Waking on __ptr_ring_produce_created_space() is too early. The
>>>>          stop/wake cycle occurs too frequently which slows down
>>>>          performance as can be seen for TAP.
>>>>
>>>> Wake on empty variant: Waking on __ptr_ring_empty() is (slightly) too
>>>>                        late. The consumer starves because the producer
>>>>                        first has to produce packets again. This slows
>>>>                        down performance aswell as can be seen for TAP
>>>> 		       and TAP+vhost-net (both down ~30-40Kpps).
>>>>
>>>> I think something inbetween should be used.
>>>> The wake should be done as late as possible to have as few
>>>> NET_TX_SOFTIRQs as possible but early enough that there are still
>>>> consumable packets remaining to not starve the consumer.
>>>>
>>>> However, I can not think of a proper way to implement this right now.
>>>>
>>>> Thanks!
>>>
>>> What is the difficulty?
>>
>> There is no way to tell how many entries are currently in the ring.
>>
>>>
>>> Your patches check __ptr_ring_consume_created_space(..., 1).
>>
>> Yes, and this returns if either 0 space or a batch size space was
>> created.
>> (In the current implementation it would be false or true, but as
>> discussed earlier this can be changed.)
>>
>>>
>>> How about __ptr_ring_consume_created_space(..., 8) then? 16?
>>>
>>
>> This would return how much space the last 8/16 consume operations
>> created. But in tap_ring_consume() we only consume a single entry.
>>
>> Maybe we could avoid __ptr_ring_consume_created_space with this:
>> 1. Wait for the queue to stop with netif_tx_queue_stopped()
>> 2. Then count the numbers of consumes we did after the queue stopped
>> 3. Wake the queue if count >= threshold with threshold >= ring->batch
>>
>> I would say that such a threshold could be something like ring->size/2.
> 
> 
> To add to what i wrote, size/2 means:
> leave half a ring for consumer, half a ring for producer.
> 
> If one of the two is more bursty, we might want a different
> balance. Offhand, the kernel is less bursty and userspace is
> more bursty.
> 
> So it's an interesting question but size/2 is a good start.
> 

I implemented this (I can post the implementation if you want)
and I got:
- 1216Kpps for TAP --> worse performance than stock (1293 Kpps) and
  also worse performance than wake on empty (1248 Kpps)
- 1408Kpps for TAP+vhost-net --> pretty much same performance as
  stock (1411 Kpps)

I also tried 7/8 for producer, 1/8 for consumer the results did not
really get better: 
- 1227Kpps for TAP --> worse performance than stock (1293 Kpps) and
  also worse performance than wake on empty (1248 Kpps); better
  performance than 1/2
- 1350Kpps for TAP+vhost-net --> worse performance than everything


So my theory of using something inbetween did not hold up here.
Judging from my benchmarking the best solution would be to use:
- Wake on empty for TAP --> 1248Kpps (1293 Kpps stock, 3% worse)
- Wake on __ptr_ring_consume_created_space() for TAP+vhost-net
  --> 1410Kpps (1411 Kpps stock, 0% worse)

This would also keep the implementation simple.