consomme: perf improvements#3555
Conversation
There was a problem hiding this comment.
Pull request overview
This PR targets performance improvements in the net_consomme networking backend by reducing per-packet overhead in the virtio queue TX path and optimizing the TCP fast path (ACK behavior, socket settings) while adding richer diagnostics via inspectable counters/histograms.
Changes:
- Reuse a TX scratch buffer in the queue to avoid per-packet heap allocations, and add a stored waker to wake the poll loop when RX buffers become available.
- Update the TCP implementation to reduce ACK traffic (defer pure ACKs to poll cycles), disable Nagle (
TCP_NODELAY), and refine window-scaling behavior to activate only after the handshake completes. - Add aggregate and per-connection TCP statistics for inspect/diagnostics.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
vm/devices/net/net_consomme/src/lib.rs |
Adds TX scratch-buffer reuse and queue waker handling to reduce allocations and improve wakeups when RX buffers reappear. |
vm/devices/net/net_consomme/consomme/src/tcp.rs |
TCP-path perf/behavior changes (ACK coalescing, window-scaling activation timing, TCP_NODELAY) plus new inspect stats; includes a new “re-arm waker” read when TX buffer fills. |
| // on outbound data if any becomes available. Without this, | ||
| // every guest packet would trigger a zero-payload ACK back, | ||
| // doubling packet rate and creating an ACK storm. | ||
| e.get_mut().inner.send_next(&mut sender, AckPolicy::Defer); |
There was a problem hiding this comment.
Oh. Here's where we do the defer thing. This seems to add a bunch of complexity--does this actually meaninguflly improve perf?
There was a problem hiding this comment.
It does meaningfully reduce the number of pure ACK packets. Just a simple scenario of connecting via RDP and opening edge (20s or so of usage) gave me a 25% reduction (~500 packets). How this translates to perf is less clear.
|
This PR modifies files containing For more on why we check whole files, instead of just diffs, check out the Rustonomicon |
No description provided.