lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4db234bd-ebd7-4325-9157-e74eccb58616@tu-dortmund.de>
Date: Wed, 26 Nov 2025 10:23:50 +0100
From: Simon Schippers <simon.schippers@...dortmund.de>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: willemdebruijn.kernel@...il.com, jasowang@...hat.com,
        andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, eperezma@...hat.com,
        jon@...anix.com, tim.gebauer@...dortmund.de, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        virtualization@...ts.linux.dev
Subject: [PATCH net-next v6 3/8] tun/tap: add synchronized ring
 produce/consume with queue management

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);
> 
> 
> 
> 
> 
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ