Skip to content

consomme: deliver TCP window payload without an extra copy#3871

Open
benhillis wants to merge 3 commits into
microsoft:mainfrom
benhillis:benhill/consomme-zerocopy-window
Open

consomme: deliver TCP window payload without an extra copy#3871
benhillis wants to merge 3 commits into
microsoft:mainfrom
benhillis:benhill/consomme-zerocopy-window

Conversation

@benhillis

Copy link
Copy Markdown
Member

On the guest-download path, consomme linearized every TCP segment into a scratch buffer, copying the payload out of the TCP window ring even though the client then copies it again into guest memory.

This adds a discontiguous receive path (BufferAccess::write_packet_segments + Client::recv_segments) so the payload stays in the ring: send_packet emits only headers and passes the header plus the ring slices, recomputing the TCP checksum across the segments. netvsp and virtio_net write each segment directly into guest memory; other backends fall back to a linearizing default.

Draft: pending Windows/IOCP + single-stream burette validation.

Copilot AI review requested due to automatic review settings July 2, 2026 23:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces copying on the consomme guest-download (RX-to-guest) path by introducing a discontiguous packet delivery API. TCP payload bytes can remain in the TCP window ring while the stack sends a header segment plus ring slices, recomputing the TCP checksum across segments; virtio-net and netvsp then write each segment directly into guest memory.

Changes:

  • Add segmented packet APIs: Client::recv_segments (consomme) and BufferAccess::write_packet_segments (net_backend) with copy-based defaults.
  • Update TCP transmit path to avoid linearizing the TCP window payload, and recompute the TCP checksum across discontiguous segments.
  • Implement zero-copy segmented writes for netvsp and virtio-net RX buffer backends, plus add a unit test for segmented checksum correctness.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
vm/devices/virtio/virtio_net/src/buffers.rs Implements write_packet_segments to write discontiguous packet bytes directly into virtio RX guest buffers.
vm/devices/net/netvsp/src/buffers.rs Implements write_packet_segments to write discontiguous packet bytes directly into netvsp RX guest buffers.
vm/devices/net/net_consomme/src/lib.rs Switches receive path to use segmented writes into the backend buffer pool and accounts for segment-total length.
vm/devices/net/net_consomme/consomme/src/tcp/tests.rs Adds a unit test validating checksum computation across discontiguous segments and odd split points.
vm/devices/net/net_consomme/consomme/src/tcp/ring.rs Removes View::copy_to_slice now that payload copying is no longer required in the TCP send path.
vm/devices/net/net_consomme/consomme/src/tcp.rs Sends TCP packets as header + ring slices (zero-copy payload) and recomputes TCP checksum across segments.
vm/devices/net/net_consomme/consomme/src/lib.rs Extends the Client trait with recv_segments (default linearizing implementation) to support discontiguous packet delivery.
vm/devices/net/net_backend/src/lib.rs Extends BufferAccess with write_packet_segments (default linearizing implementation) to support discontiguous packet writes.

@benhillis

Copy link
Copy Markdown
Member Author

Local Linux/KVM benchmark (corroborating, draft)

A/B on the guest-download path (iperf3 -R, vmbus/consomme), swapping only the openvmm binary between main and this branch, interleaved back-to-back to cancel machine drift.

Single stream: no benefit, marginally negative (candidate lower in 6/6 rounds, ~-1.3%). Expected: at ~5 Gbps single-stream, throughput is bounded by single-core scheduling and the TCP window, not copy cost, and at ~1440B/segment the removed payload memcpy is roughly offset by the added discontiguous-checksum work.

Multi-stream (-R -P 16, CPU-bound RX): clear win. Once the consomme worker core saturates, eliminating the per-byte copy raises the ceiling.

Aggregate tcp_rx Baseline (main) This branch Delta
mean of 5 interleaved rounds 6.18 Gbps 6.76 Gbps +9.4%

Candidate higher in 5/5 clean rounds (+2.8% to +17.8%; paired +0.58 +/- 0.33 Gbps). One round excluded for an unrelated pipette node-disconnect.

Net: this pays off exactly where intended, high-bandwidth multi-connection RX. Keeping the PR as draft pending Windows/IOCP confirmation.

@benhillis benhillis marked this pull request as ready for review July 7, 2026 22:28
@benhillis benhillis requested a review from a team as a code owner July 7, 2026 22:28
@benhillis benhillis requested a review from Copilot July 7, 2026 23:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread vm/devices/net/net_consomme/consomme/src/tcp/tests.rs
Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs Outdated
Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs Outdated
Copilot AI review requested due to automatic review settings July 8, 2026 14:58
@benhillis benhillis force-pushed the benhill/consomme-zerocopy-window branch from ef6a404 to 5350aec Compare July 8, 2026 14:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs
Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs Outdated
Comment thread vm/devices/net/net_consomme/consomme/src/tcp/tests.rs Outdated
Comment thread vm/devices/net/net_consomme/consomme/src/tcp/tests.rs Outdated
Copilot AI review requested due to automatic review settings July 8, 2026 16:10
@benhillis benhillis force-pushed the benhill/consomme-zerocopy-window branch from 5350aec to 9b6dfd6 Compare July 8, 2026 16:10
@benhillis benhillis force-pushed the benhill/consomme-zerocopy-window branch from 9b6dfd6 to e29c3d0 Compare July 8, 2026 16:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs Outdated
Comment thread vm/devices/net/net_consomme/consomme/src/tcp/tests.rs Outdated
Copilot AI review requested due to automatic review settings July 8, 2026 16:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs Outdated
Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs Outdated
Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs Outdated
Copilot AI review requested due to automatic review settings July 8, 2026 20:23
@benhillis benhillis force-pushed the benhill/consomme-zerocopy-window branch from e29c3d0 to f8de9c8 Compare July 8, 2026 20:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs
Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs
Comment thread vm/devices/net/net_backend/src/lib.rs
Comment thread vm/devices/net/net_consomme/consomme/src/lib.rs
Copilot AI review requested due to automatic review settings July 8, 2026 23:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs Outdated
Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs Outdated
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

Ben Hillis and others added 2 commits July 9, 2026 07:53
The guest-download path linearized every TCP segment into a scratch
buffer before handing it to the client, copying the payload out of the
TCP window ring even though the client immediately copies it again into
guest memory.

Add a discontiguous receive path so the payload can stay in the ring:

- net_backend: BufferAccess::write_packet_segments delivers a packet from
  ordered, discontiguous segments. The default linearizes; netvsp and
  virtio_net override it to write each segment directly into guest memory.
- consomme: Client::recv_segments mirrors recv for discontiguous data
  (default linearizes). send_packet now emits only the headers into the
  scratch buffer and passes the header plus the (up to two) ring slices,
  recomputing the TCP checksum over those segments.

The checksum is built from smoltcp's RFC 1071 primitives (pseudo_header,
data, combine). Since smoltcp's checksum module is crate-private these
are vendored locally (smoltcp is 0BSD) pending smoltcp-rs/smoltcp#1172,
which exposes them; once released the local copy becomes a `use` of
smoltcp::wire::checksum. Because the TCP header is a multiple of 4 bytes,
only the second ring slice can be misaligned, so its partial checksum is
byte-swapped when the first slice has odd length.

Correctness of the segmented checksum is covered by a unit test that
compares it against smoltcp across address families, payload lengths, and
every payload split point.
Use wrapping_add in the one's-complement accumulators, build the IPv6
pseudo-header with the full 32-bit upper-layer length, and fast-path the
single-segment case in the default recv_segments/write_packet_segments
implementations to avoid an allocation.

Co-authored-by: Copilot <[email protected]>
@benhillis

benhillis commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

@jstarks smoltcp merged my PR to expose the checksum module publicly (smoltcp-rs/smoltcp#1172). Once a new package is released, we can drop the local version.

Copilot AI review requested due to automatic review settings July 9, 2026 15:18
@benhillis benhillis force-pushed the benhill/consomme-zerocopy-window branch from 9dd8526 to 205d350 Compare July 9, 2026 15:18
Treat a zero-length payload the same as no payload so pure ACK/FIN sends
take the contiguous fast path, and omit the trailing empty ring slice when
the payload view does not wrap.

Co-authored-by: Copilot <[email protected]>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread vm/devices/virtio/virtio_net/src/buffers.rs
Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs
Copilot AI review requested due to automatic review settings July 9, 2026 15:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread vm/devices/net/net_consomme/consomme/src/tcp.rs
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants