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.
