Repository navigation
test(minvmd): hold a marker's connection open past its handshake - #1846
Merged
norrietaylor merged 1 commit intoOct 3, 2026
Merged
Conversation
The host listener armed a 2s read timeout the moment it accepted a connection, and discarded the connection unrecorded when that timeout fired. gvisor-tap-vsock's TCP forwarder dials the backend *before* it creates the endpoint that answers the SYN, so the listener accepts, and starts that clock, before the sender has seen the SYN-ACK it must answer before it can push. A handshake slower than ~2s — one SYN-ACK retransmit on a loaded runner is enough — left the marker nowhere to land: the flow completed, and the wait meant to grant it 10s spent them against a socket already closed. Hold each connection for a budget longer than the longest wait, record every read as it arrives rather than once at the end, and read each connection on its own thread so one silent sender cannot delay another sender's marker. Report what the listener saw when a wait fails, so a next occurrence says whether the connection was never dialed, was dialed and stayed silent, or arrived after the wait gave up. Refs: #1845
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Contributor
|
Approved: no findings at Nothing to do. Clobber: none: |
jtnkminimal
approved these changes
Oct 3, 2026
norrietaylor
merged commit Oct 3, 2026
2587047
into
fix/ci-gvproxy-before-vm-harness
3 checks passed
yermakoffivan
pushed a commit
to yermakoffivan/minimal
that referenced
this pull request
Oct 3, 2026
…ominimal#1815) * ci: fetch the gvproxy switch before the VM harness step and export it Both VM lanes fetched the pinned gvproxy switch only after the "VM integration harnesses" step and never exported MINVMD_GVPROXY_BIN to it, so every harness that needs the host switch skipped and counted as a pass. Fetch it once ahead of the harness step, drop the later duplicate, and give the harness step MINVMD_GVPROXY_BIN plus the MINVMD_VM_LANE marker. * revert(ci): leave the frozen workflows untouched * test(minvmd): fetch the pinned gvproxy switch for VM harnesses under MINVMD_E2E * test(minvmd): give the vm_escape boxes a package source and the switch The vm_escape harness never ran a box: a shell exec launches the box's sandbox, which resolves the baseline packages against the project's package graph, and the harness uploaded a minimal.toml with no [upstream], so every exec failed with "no such package: base". It also booted with `minvmd boot --foreground`, which never stands up the gvproxy switch, so the guest daemon could not have fetched packages anyway. - Carry the pinned [upstream] (gominimal/pkgs at f4de33d0) in the boxes' minimal.toml, shared as common::PKGS_UPSTREAM. The guest daemon fetches it as node-plane traffic; under MINVMD_E2E=1 a failed fetch fails the test. - Boot through the supervisor path (`minvmd run --detach`, poll `status --json` until Running, `minvmd stop` on drop), with MINVMD_VM_OWN_IP and the helper's MINVMD_GVPROXY_BIN, and read the gate's lines from the detached supervisor's JSON log. - Gate a Pending ConfigureLoadout the way the CLI's no-prompt hook answers vars, failing on any patch or hook, and submit the verdict. - Return each exec's stderr and quote it when an exec exits nonzero, so a box that cannot launch names its cause. * fix(minvmd): reap the VMM child when a boot harness tears down boot_integration and minimald_session_integration SIGKILLed `minvmd boot --foreground` alone. Its `__krun-vmm` child was reparented to init and kept the VM running, holding the harness's inherited stdio open, which nextest reported as LEAK and which piled up on persistent runners. Spawn the boot as its own process group with stdin off the terminal, and tear down with common::kill_group, which kills the whole group before reaping the parent. * fix(minvmd): kill the boot harness's process group with killpg procps `kill -KILL -<pgid>` exits 0 on Linux without signalling the group, so the reap that followed blocked on a still-running `minvmd boot` and the KVM lane timed out in boot_integration. Signal the group with killpg(2) and kill the parent directly as well, so the reap cannot block on a group signal that missed. * fix(minvmd): harden vm_escape's minvmd helper and teardown - Export MINVMD_READY_TIMEOUT_SECS (150 s) to the supervisor: `--timeout` bounds only `run --detach`'s poll, and the READY wait otherwise defaults to 60 s, short of a cold boot on a lane that does not set it. - Drain the subcommand's stdout and stderr on reader threads while it runs, so a child that writes more than a pipe buffer cannot stall into the timeout. The Output shape and the timeout are unchanged. - Guest::drop never panics: `minvmd stop` runs through a non-panicking bounded helper, and when it errors or times out the VMM pid recorded from `status --json` is killed and the failure logged. * test(minvmd): hold a marker's connection open past its handshake (gominimal#1846) The host listener armed a 2s read timeout the moment it accepted a connection, and discarded the connection unrecorded when that timeout fired. gvisor-tap-vsock's TCP forwarder dials the backend *before* it creates the endpoint that answers the SYN, so the listener accepts, and starts that clock, before the sender has seen the SYN-ACK it must answer before it can push. A handshake slower than ~2s — one SYN-ACK retransmit on a loaded runner is enough — left the marker nowhere to land: the flow completed, and the wait meant to grant it 10s spent them against a socket already closed. Hold each connection for a budget longer than the longest wait, record every read as it arrives rather than once at the end, and read each connection on its own thread so one silent sender cannot delay another sender's marker. Report what the listener saw when a wait fails, so a next occurrence says whether the connection was never dialed, was dialed and stayed silent, or arrived after the wait gave up. Refs: gominimal#1845 * test(minvmd): print the gate's own account when a marker is lost (gominimal#1849) The listener's report now shows the spoofed flow's connection being accepted and then reading nothing, so the loss is after the handshake and not in the listener's timing budget. What it cannot show is which side lost the payload. Print the gate's lines for the flow alongside it. An egress-unregistered-source line and nothing else means the gate admitted the frames and the loss is past it, in gvproxy; an ingress-leg line means the relay came off on the switch side and whatever the guest had buffered went with it. Refs: gominimal#1845, #1847 * test(minvmd): hold a spoofed flow open until its marker is acknowledged (gominimal#1864) * test(minvmd): hold a spoofed flow open until its marker is acknowledged spoofed_flow wrote the ACK and the marker's push, returned, and dropped the gate socket: no wait for the switch's acknowledgement and no retransmit. A frame lost in the relay's or the switch's teardown of that connection lost the marker with nothing to say where (gominimal#1845). The flow now resends the push every 500 ms until the switch acknowledges every marker byte (SPOOF_ISN + 1 + marker length, compared in wrapping sequence space), answering ARP for its address meanwhile, and reports whether it was acknowledged. An acknowledged marker that never reaches the listener was lost past the switch's stack; an unacknowledged one was lost before it. * test(minvmd): record whether the switch acknowledged each admitted marker The admit verdict in the attempts record now carries the marker's acknowledgement, so a later red run shows whether the ACK path worked without a pre-fix timing baseline to infer it from.
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
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.
Fixes the flake in #1845.
The mechanism
HostListenerarmed a 2-second read timeout the moment it accepted a connection, and on that timeout dropped the connection without recording anything (got == 0, so nothing was pushed toseen).The listener's clock does not start where the test assumes. gvisor-tap-vsock's TCP forwarder dials the backend before it creates the endpoint that answers the SYN — pinned v0.8.9,
pkg/services/forwarder/tcp.go:34:So the order is: spoofer's SYN → gvproxy dials
127.0.0.1:<port>→ the listener accepts and arms its 2 s timer →CreateEndpoint→ SYN-ACK → spoofer's ACK and PSH. The listener is counting down through the handshake it is supposed to be waiting after.The spoofer's gratuitous ARP claim buys nothing against that clock: gvisor discards a confirmation for which it holds no entry (
neighbor_cache.go,handleConfirmationreturns early), so the stack must issue its own ARP request, anddefaultRetransmitTimeris a whole second. One SYN-ACK retransmit puts the PSH at ~1 s and it survives; two put it at ~3-4 s and the connection is already gone — whilespoofed_flowstill returnsOk(())well inside its own 10 sFLOW_DEADLINE. That is the observed signature exactly: handshake completed, marker never arrived, and 10 s ofwait_forspent against a socket that no longer existed.It is a wall-clock threshold crossing, which is why the same tree failed twice and passed once.
The change
CONNECTION_BUDGET(30 s), longer than the longestwait_for, instead of closing it on the first read timeout. A read timeout is not an end — the sender may still be completing the handshake this connection was dialed for.No production code changes.
Deliberately not done
spoofed_flow; worth doing only if the failure survives this.relay_frames'tokio::select!(crates/minvmd/src/net/egress_gate.rs:1199) has nobiased;, so when the ingress leg wins the race the pinned egress future is dropped un-drained and any frame still buffered from the guest is discarded — andtable.report_withdrawalsat:2235never runs, so the box's row outlives its shuttle connection. That is a real defect, and near-unreachable in production where a shuttle is not closed right after a data frame. It is out of scope here and should get its own issue rather than be bundled into a test fix.Base
Stacked on
fix/ci-gvproxy-before-vm-harness(#1 8000 815), because the test region only exists there — the file is 1358 lines onmainand 1628 with that change. Retarget tomainonce #1815 lands.Verification
cargo fmt,cargo checkandcargo clippyon-p minvmd --test vm_escape_integrationare clean. The test itself needs/dev/kvm, so the CI lane is the only place it runs; a flake at roughly two in three gives usable signal within a few runs.