net: batch per-connection wakeups in the virtio-net receiver - #1467
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a batched wakeup mechanism for virtio-net RX fast-path delivery, reducing per-packet net_channel::wake() calls by collecting distinct target channels during an RX drain pass and waking each channel once at the end of the pass (toggleable via OSV_NET_BATCH_WAKE).
Changes:
- Add
classifier::post_packet(m, batch)to record (deduplicated) target channels instead of waking per packet. - Introduce
net_channel_wake_batchto collect and flush distinct channel wakeups. - Update virtio-net receiver to optionally hold a single RCU read lock across a drain pass, accumulate wake targets, and flush them at pass end; add env-var-controlled enablement.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| include/osv/net_channel.hh | Adds net_channel_wake_batch and a batched classifier::post_packet() overload to deduplicate wakeups. |
| core/net_channel.cc | Implements the new batched classifier::post_packet(m, batch) path alongside the existing per-packet wake path. |
| drivers/virtio-net.hh | Declares net::batch_wakes_enabled() runtime toggle helper. |
| drivers/virtio-net.cc | Implements env-var toggle and updates RX drain to batch channel wakeups and flush once per pass. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // wake every distinct recorded channel once, then reset | ||
| void flush() { | ||
| for (unsigned i = 0; i < _n; i++) { | ||
| _inline[i]->wake(); | ||
| } | ||
| _n = 0; | ||
| if (_spill) { | ||
| for (auto* c : *_spill) { | ||
| c->wake(); | ||
| } | ||
| delete _spill; | ||
| _spill = nullptr; | ||
| } | ||
| } | ||
| bool empty() const { return _n == 0 && !_spill; } | ||
| ~net_channel_wake_batch() { delete _spill; } |
| bool fast_path = batch_wakes | ||
| ? _ifn->if_classifier.post_packet(m_head, wake_batch) | ||
| : _ifn->if_classifier.post_packet(m_head); | ||
| if (!fast_path) { | ||
| (*_ifn->if_input)(_ifn, m_head); |
|
Update after measuring this properly on a large multi-connection PostgreSQL workload (many-vCPU KVM guest, real tap+vhost NIC):
So I am not presenting this as a scaling fix. It is a mechanism cleanup that closes the in-tree |
The receive poll thread classified each inbound packet to its net_channel and woke that channel immediately, once per packet. Under a many-connection load that is one thread::wake() and, when the target runs on another CPU, one wakeup IPI per packet, which defeats the scheduler's per-target-CPU IPI coalescing (incoming_wakeups_mask) because the wakes are spread across the drain loop with other work between them. This closes the long-standing "FIXME: find a way to batch wakes" in core/net_channel.cc. Add classifier::post_packet(m, net_channel_wake_batch&): it classifies and pushes the packet like the existing post_packet() but records the touched net_channel in a small deduplicating batch instead of waking it. The receiver holds one osv::rcu_read_lock across the whole drain pass, collects the distinct channels, and flushes one wake per channel at the end of the pass. Because the flushed wakes run back to back with no intervening reschedule, wake_impl()'s per-destination-CPU IPI coalescing collapses them into at most one wakeup IPI per destination CPU per pass instead of one per packet. net_channel_wake_batch keeps a small inline array (spilling to the heap only on overflow) so the hot path allocates nothing, and it must be used and flushed under the same rcu_read_lock that guarded the post_packet() calls because the recorded net_channel pointers are rcu_dispose()d on connection teardown. The non-classified slow path runs the full BSD input stack, which may demand-fault a page and therefore requires a preemptable context, which is illegal under the rcu_read_lock the batch path holds across the drain. Defer those packets into a list and run them up the stack after the wake flush and rcu unlock, when the thread is preemptable again. OSV_NET_BATCH_WAKE=0 selects the original one-wake-per-packet path unchanged; the batched path is the default. Signed-off-by: Greg Burd <greg@burd.me>
fdcad6a to
aaa45d6
Compare
What
The virtio-net receiver drains many packets per RX pass but wakes their target connections one packet at a time:
classifier::post_packet()callsnet_channel::wake()for every packet (the long-standing// FIXME: find a way to batch wakes). Each wake runs a fullthread::wake_impl()and, when the woken connection last ran on a different CPU, sends a wakeup IPI. Under many concurrent connections this is one wake round-trip, often one cross-CPU IPI, per packet, on the single receiver thread's hot path.This adds a batched producer path.
classifier::post_packet(m, batch)classifies and pushes the packet exactly as before but records the target channel in a small deduplicatingnet_channel_wake_batchinstead of waking it. The receiver holds onercu_read_lockacross the whole drain pass, records every touched channel, then wakes each distinct channel once viabatch.flush()at the end of the pass. Because the flushed wakes run back-to-back with no intervening reschedule,thread::wake_impl()'s per-target-CPU IPI coalescing (incoming_wakeups_mask) collapses them into at most one wakeup IPI per destination CPU per pass instead of one per packet, and a connection that received several packets in the pass is woken only once.The single
rcu_read_lockover the drain keeps the recordednet_channelpointers valid until flushed (net_channelisrcu_dispose()d on teardown). The batch's inline capacity avoids heap traffic on the RX hot path and spills to a vector only past 16 distinct connections per pass.Selectable at runtime via
OSV_NET_BATCH_WAKE("0" disables, restoring the exact per-packet path) so it can be A/B measured on a single image; defaults on. The non-batched path is unchanged.Correctness / status
I want to be straight about the performance evidence: the cross-CPU IPI-coalescing benefit only appears at smp>1, and I have not yet been able to produce a clean smp>1 A/B throughput number. On this setup the guest wedges under concurrent RX load at smp>=2 independently of this change (it reproduces with
OSV_NET_BATCH_WAKE=0, i.e. the current per-packet path, on both a plain tap and a vhost NIC), so I could not isolate the batching delta at scale. That looks like a separate pre-existing RX-wakeup issue I am investigating on its own.So this PR is offered on its merits as a mechanism improvement: it removes a real per-packet wake/IPI cost on the receiver hot path, closes the in-tree FIXME, is off-by-a-flag reversible, and does not change the single-queue behavior. It is not making a measured throughput claim; the at-scale numbers depend on the separate wedge being resolved first.