revert(minimald): restore the 8 MiB guest vsock receive window - #922
Merged
Conversation
Reverts commit b0f07a2 (PR #916), which reverted 4823653 (PR #885). #885 was reverted before the libkrun fix had been tested in production. That test has now run and the fix is not sufficient on its own: the shipped 6d239bc build carries the fill loop and still fails, because the loop exits on EAGAIN with a partly filled descriptor whenever the writer is slower than libkrun's reader. The window is the one measure with an unambiguous result behind it -- 0 of 6 failures at 8 MiB against 6 of 6 on the same machine at the 256 KiB default. It raises the skb-depth ceiling from 455 queued packets to 14,563, which is the bound every observed reset has hit. This is headroom, not immunity: it does not change the ratio between the two ceilings, it keeps a normal upload out of the fragmented tail where the reset happens. It belongs alongside a libkrun fix rather than instead of one. Note `SO_VM_SOCKETS_BUFFER_MAX_SIZE` must be raised before `SO_VM_SOCKETS_BUFFER_SIZE`: vsock_update_buffer_size() clamps the latter to the former and both default to 256 KiB, so the obvious ordering silently changes nothing. Refs: #869
📝 WalkthroughWalkthroughLinux AF_VSOCK startup now requests an 8 MiB receive window, reads back the effective size, and logs whether the kernel applied, clamped, or rejected the configuration. ChangesAF_VSOCK receive window
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
norrietaylor
marked this pull request as ready for review
July 22, 2026 21:02
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/minimald/src/main.rs`:
- Around line 739-760: Gate the vsock listener branch in async_main, including
VsockListener, VSOCK_RX_WINDOW_BYTES, and set_vsock_rx_window usage, behind
#[cfg(target_os = "linux")] or a Linux-only helper so non-Linux builds do not
type-check it. Add the std::mem::size_of import required by set_vsock_rx_window.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 38dcbfa6-ccb7-4e92-983c-87ae3f0cc607
📒 Files selected for processing (1)
crates/minimald/src/main.rs
Comment on lines
+739
to
+760
|
|
||
| // Widen the receive window before anything connects: `__vsock_create` | ||
| // copies `buffer_size` from the listening socket onto every socket it | ||
| // accepts, so this one call covers every session. Best effort — a | ||
| // daemon with the default window is the old, buggy behaviour, not a | ||
| // reason to refuse to boot. | ||
| match set_vsock_rx_window(&listener, VSOCK_RX_WINDOW_BYTES) { | ||
| Ok(effective) if effective >= VSOCK_RX_WINDOW_BYTES => { | ||
| tracing::debug!(bytes = effective, "raised the vsock receive window"); | ||
| } | ||
| Ok(effective) => tracing::warn!( | ||
| requested = VSOCK_RX_WINDOW_BYTES, | ||
| effective, | ||
| "vsock receive window clamped below the requested size; large uploads may still \ | ||
| fail with ENOBUFS" | ||
| ), | ||
| Err(e) => tracing::warn!( | ||
| error = %e, | ||
| "could not raise the vsock receive window; large uploads may fail with ENOBUFS" | ||
| ), | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Check for cfg(target_os = "linux") near the vsock branch/import in main.rs
rg -n -B5 -A5 'VsockListener::bind|use tokio_vsock' crates/minimald/src/main.rs
# 2. Check for an existing size_of import in the file
rg -n 'use std::mem::(size_of|\*)|size_of::<u64>' crates/minimald/src/main.rs
# 3. Check Cargo.toml for minimald for target-specific deps/features
fd Cargo.toml crates/minimald --exec cat {}
# 4. Check declared Rust version/edition (rust-toolchain, workspace Cargo.toml)
fd rust-toolchain -H
fd Cargo.toml --max-depth 1 --exec grep -n 'edition\|rust-version' {}Repository: gominimal/minimal
Length of output: 4695
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- crates/minimald/src/main.rs: top imports and linux-gated items ---'
sed -n '1,120p' crates/minimald/src/main.rs
echo
echo '--- crates/minimald/src/main.rs: vsock branch around call site ---'
sed -n '720,770p' crates/minimald/src/main.rs
echo
echo '--- crates/minimald/src/main.rs: helper definition ---'
sed -n '790,870p' crates/minimald/src/main.rs
echo
echo '--- rust-toolchain.toml ---'
cat rust-toolchain.toml
echo
echo '--- workspace Cargo.toml edition/rust-version ---'
grep -n '^\s*\(edition\|rust-version\)\s*=' Cargo.toml || trueRepository: gominimal/minimal
Length of output: 11210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- size_of imports/usages in crates/minimald/src/main.rs ---'
rg -n 'size_of|mem::size_of|use .*size_of' crates/minimald/src/main.rs
echo
echo '--- cfg gates around the async entry/body ---'
rg -n -B3 -A8 '#\[cfg\(target_os = "linux"\)\]|async fn|fn async_main|match .*Command::Run|VsockListener::bind' crates/minimald/src/main.rsRepository: gominimal/minimal
Length of output: 5423
Gate the Linux-only vsock path and fix the missing import
async_mainstill type-checks this branch on non-Linux targets, but it references Linux-onlyVsockListener,VSOCK_RX_WINDOW_BYTES, andset_vsock_rx_window; wrap the whole vsock branch in#[cfg(target_os = "linux")]or move it behind a Linux-only helper.set_vsock_rx_windowalso callssize_of::<u64>()without importingstd::mem::size_of, so it will not compile as written.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/minimald/src/main.rs` around lines 739 - 760, Gate the vsock listener
branch in async_main, including VsockListener, VSOCK_RX_WINDOW_BYTES, and
set_vsock_rx_window usage, behind #[cfg(target_os = "linux")] or a Linux-only
helper so non-Linux builds do not type-check it. Add the std::mem::size_of
import required by set_vsock_rx_window.
twitchyliquid64
approved these changes
Jul 22, 2026
2 tasks
This was referenced Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #916, which reverted #885. Restores the 8 MiB guest vsock receive window.
Why it should go back
#885 was reverted before the libkrun fix had been tested in production. That test has now run, and the libkrun fix is not sufficient on its own.
unstable(6d239bc) carries the fill loop from #884 and still fails. Tom hit it, and it reproduces here on a stack verified identical to his — his on-disklibkrun.1.dylibis byte-identical to the publishedversions/6d239bc1/libkrun-macos-arm64.dylib(sha2564d1056da…), and disassembly confirms the fill loop is present in it.The fill loop stops us fragmenting the stream when libkrun outruns the writer inside one wakeup. It does nothing when the writer is slower across wakeups: each notification finds a few hundred bytes, the loop drains them, hits EAGAIN, and emits a packet a fraction of the descriptor's size. An instrumented guest kernel attributed every reset to the skb-depth branch,
rx_qlenexactly 455, packets of 100–452 bytes.What the window buys
0 of 6 failures at 8 MiB against 6 of 6 on the same machine at the 256 KiB default.
buf_allocbounds queue depth atbuf_alloc / SKB_TRUESIZE(0). At 256 KiB that is 455 packets — the exactrx_qlenin every reject record. At 8 MiB it is 14,563.This is headroom, not immunity. It does not change the ratio between the byte ceiling and the skb ceiling; it keeps a normal upload out of the fragmented tail where the reset happens. It belongs alongside a fix for the fragmentation, not instead of one.
Implementation note worth keeping
SO_VM_SOCKETS_BUFFER_MAX_SIZEmust be raised beforeSO_VM_SOCKETS_BUFFER_SIZE.vsock_update_buffer_size()clamps the latter to the former and both default to 256 KiB, so the obvious ordering appears to succeed and changes nothing. The restored code does them in the right order and reads back the applied value.Related work in flight
write()on the unix socket and UDS does no coalescing. Buffering there removes the cause rather than compensating for it in the VMM.Draft until we settle which of the three lands first and in what combination.
Refs: #869
Note
Restore 8 MiB guest vsock receive window in
minimaldReintroduces the AF_VSOCK receive window configuration that was previously removed. On Linux, after binding the
VsockListener, the daemon calls the newset_vsock_rx_windowhelper to setSO_VM_SOCKETS_BUFFER_MAX_SIZEandSO_VM_SOCKETS_BUFFER_SIZEto 8 MiB, then reads back the effective size and logs whether the request succeeded, was partially applied, or failed.Macroscope summarized cdaf024.
Summary by CodeRabbit