On 11/25/25 17:54, Michael S. Tsirkin wrote:
> On Thu, Nov 20, 2025 at 04:29:08PM +0100, Simon Schippers wrote:
>> Implement new ring buffer produce and consume functions for tun and tap
>> drivers that provide lockless producer-consumer synchronization and
>> netdev queue management to prevent ptr_ring tail drop and permanent
>> starvation.
>>
>> - tun_ring_produce(): Produces packets to the ptr_ring with proper memory
>> barriers and proactively stops the netdev queue when the ring is about
>> to become full.
>>
>> - __tun_ring_consume() / __tap_ring_consume(): Internal consume functions
>> that check if the netdev queue was stopped due to a full ring, and wake
>> it when space becomes available. Uses memory barriers to ensure proper
>> ordering between producer and consumer.
>>
>> - tun_ring_consume() / tap_ring_consume(): Wrapper functions that acquire
>> the consumer lock before calling the internal consume functions.
>>
>> Key features:
>> - Proactive queue stopping using __ptr_ring_full_next() to stop the queue
>> before it becomes completely full.
>> - Not stopping the queue when the ptr_ring is full already, because if
>> the consumer empties all entries in the meantime, stopping the queue
>> would cause permanent starvation.
>
> what is permanent starvation? this comment seems to answer this
> question:
>
>
> /* Do not stop the netdev queue if the ptr_ring is full already.
> * The consumer could empty out the ptr_ring in the meantime
> * without noticing the stopped netdev queue, resulting in a
> * stopped netdev queue and an empty ptr_ring. In this case the
> * netdev queue would stay stopped forever.
> */
>
>
> why having a single entry in
> the ring we never use helpful to address this?
>
>
>
>
> In fact, all your patch does to solve it, is check
> netif_tx_queue_stopped on every consumed packet.
>
>
> I already proposed:
>
> static inline int __ptr_ring_peek_producer(struct ptr_ring *r)
> {
> if (unlikely(!r->size) || r->queue[r->producer])
> return -ENOSPC;
> return 0;
> }
>
> And with that, why isn't avoiding the race as simple as
> just rechecking after stopping the queue?
I think you are right and that is quite similar to what veth [1] does.
However, there are two differences:
- Your approach avoids returning NETDEV_TX_BUSY by already stopping
when the ring becomes full (and not when the ring is full already)
- ...and the recheck of the producer wakes on !full instead of empty.
I like both aspects better than the veth implementation.
Just one thing: like the veth implementation, we probably need a
smp_mb__after_atomic() after netif_tx_stop_queue() as they also discussed
in their v6 [2].
On the consumer side, I would then just do:
__ptr_ring_consume();
if (unlikely(__ptr_ring_consume_created_space()))
netif_tx_wake_queue(txq);
Right?
And for the batched consume method, I would just call this in a loop.
Thank you!
[1] Link:
https://lore.kernel.org/netdev/174559288731.827981.8748257839971869213.stgit@firesoul/T/#m2582fcc48901e2e845b20b89e0e7196951484e5f
[2] Link:
https://lore.kernel.org/all/174549933665.608169.392044991754158047.stgit@firesoul/T/#m63f2deb86ffbd9ff3a27e1232077a3775606c14d
>
> __ptr_ring_produce();
> if (__ptr_ring_peek_producer())
> netif_tx_stop_queue
smp_mb__after_atomic(); // Right here
> if (!__ptr_ring_peek_producer())
> netif_tx_wake_queue(txq);
>
>
>
>
>
>
>
