From: Simon Schippers <simon.schippers@tu-dortmund.de>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: willemdebruijn.kernel@gmail.com, jasowang@redhat.com,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, eperezma@redhat.com,
jon@nutanix.com, tim.gebauer@tu-dortmund.de,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
kvm@vger.kernel.org, virtualization@lists.linux.dev
Subject: [PATCH net-next v6 3/8] tun/tap: add synchronized ring produce/consume with queue management
Date: Wed, 26 Nov 2025 10:23:50 +0100 [thread overview]
Message-ID: <4db234bd-ebd7-4325-9157-e74eccb58616@tu-dortmund.de> (raw)
In-Reply-To: <20251125100655-mutt-send-email-mst@kernel.org>
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);
>
>
>
>
>
>
>
next prev parent reply other threads:[~2025-11-26 9:24 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-20 15:29 [PATCH net-next v6 0/8] tun/tap & vhost-net: netdev queue flow control to avoid ptr_ring tail drop Simon Schippers
2025-11-20 15:29 ` [PATCH net-next v6 1/8] ptr_ring: add __ptr_ring_full_next() to predict imminent fullness Simon Schippers
2025-11-25 14:54 ` Michael S. Tsirkin
2025-11-20 15:29 ` [PATCH net-next v6 2/8] ptr_ring: add helper to check if consume created space Simon Schippers
2025-11-25 15:01 ` Michael S. Tsirkin
2025-11-25 16:12 ` Simon Schippers
2025-11-25 17:18 ` Michael S. Tsirkin
2025-11-30 18:16 ` Willem de Bruijn
2025-11-20 15:29 ` [PATCH net-next v6 3/8] tun/tap: add synchronized ring produce/consume with queue management Simon Schippers
2025-11-25 16:54 ` Michael S. Tsirkin
2025-11-26 9:23 ` Simon Schippers [this message]
2025-11-26 15:25 ` Michael S. Tsirkin
2025-11-26 16:04 ` Simon Schippers
2025-11-26 18:16 ` Michael S. Tsirkin
2025-11-20 15:29 ` [PATCH net-next v6 4/8] tun/tap: add batched ring consume function Simon Schippers
2025-11-20 15:29 ` [PATCH net-next v6 5/8] tun/tap: add uncomsume function for returning entries to ring Simon Schippers
2025-11-20 15:29 ` [PATCH net-next v6 6/8] tun/tap: add helper functions to check file type Simon Schippers
2025-11-20 15:29 ` [PATCH net-next v6 7/8] tun/tap & vhost-net: use {tun|tap}_ring_{consume|produce} to avoid tail drops Simon Schippers
2025-11-20 15:29 ` [PATCH net-next v6 7/8] tun/tap/vhost: " Simon Schippers
2025-11-20 15:29 ` [PATCH net-next v6 8/8] tun/tap: drop get ring exports Simon Schippers
2025-11-21 6:19 ` [PATCH net-next v6 0/8] tun/tap & vhost-net: netdev queue flow control to avoid ptr_ring tail drop Jason Wang
2025-11-21 9:22 ` Simon Schippers
2025-11-24 1:04 ` Jason Wang
2025-11-24 9:19 ` Simon Schippers
2025-11-25 1:34 ` Jason Wang
2025-11-25 14:04 ` Simon Schippers
2025-11-26 6:19 ` Jason Wang
2025-11-26 7:15 ` Michael S. Tsirkin
2025-11-26 9:24 ` Simon Schippers
2025-11-21 9:18 ` [syzbot ci] " syzbot ci
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4db234bd-ebd7-4325-9157-e74eccb58616@tu-dortmund.de \
--to=simon.schippers@tu-dortmund.de \
--cc=andrew+netdev@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=eperezma@redhat.com \
--cc=jasowang@redhat.com \
--cc=jon@nutanix.com \
--cc=kuba@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=tim.gebauer@tu-dortmund.de \
--cc=virtualization@lists.linux.dev \
--cc=willemdebruijn.kernel@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®