docs(spec-minvmd-networking-gvproxy): minvmd networking — gvproxy userspace net for VM sessions - #407
Conversation
…ce networking Spec for tracking issue #404. Adds gvproxy (containers/gvisor-tap-vsock) as the default outbound network transport for minvmd-managed VMs, with TSI remaining selectable via MINVMD_NETMODE=tsi. Three demoable units: - Unit 1: gvproxy child spawn/lifecycle/reap (net.rs, run.rs, state.rs) - Unit 2: krun_set_passt_fd FFI + VmConfig virtio-net wiring - Unit 3: network allowlist enforcement hook (no-op stub, wired call site) Resolves the ~62-concurrent-connection TSI cap that blocks cache-fetch parallelism (informed by #204). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a comprehensive architectural specification for integrating Changesminvmd gvproxy Networking Specification
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related issues
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
sdd-validate · Spec boundaryBoundary resolved: Spec — PR adds Gate 1 — Acceptance criteria testable ✅ PassAll proof artifacts are stated as observable pass/fail conditions:
No aspirational or untestable criteria found. Gate 2 — No implementation leakage
|
| Location | Leakage |
|---|---|
03-spec-minvmd-networking-gvproxy.md:132–141 (R1.1) |
Prescribes enum NetworkMode { GvProxy, Tsi }, fn resolve_net_mode() -> NetworkMode, and fn gvproxy_bin() -> std::path::PathBuf by name and signature. |
03-spec-minvmd-networking-gvproxy.md:142–149 (R1.2) |
Prescribes fn spawn_gvproxy(net_fd: RawFd) -> Result<Child> verbatim. |
03-spec-minvmd-networking-gvproxy.md:219–227 (R2.1) |
Includes the literal Rust FFI declaration pub fn krun_set_passt_fd(ctx_id: u32, fd: i32) -> i32; and prescribes the // SAFETY: comment structure. |
03-spec-minvmd-networking-gvproxy.md:228–235 (R2.2) |
Includes the verbatim safe-wrapper signature pub fn set_passt_fd(&mut self, fd: std::os::unix::io::RawFd) -> Result<(), VmError>. |
03-spec-minvmd-networking-gvproxy.md:236–246 (R2.3) |
Prescribes VmConfig.net_mode: NetworkMode field name, the VmConfig::new parameter addition, and the changed apply signature fn apply(&self, ctx: &mut Context, net_fd: Option<RawFd>). |
03-spec-minvmd-networking-gvproxy.md:294–321 (R3.1) |
Provides the exact Rust source for NetworkPolicy (including variant names and Vec<String> inner type) and the full body of check_network_policy. |
The behavioral intent behind each requirement is sound; the prescription of implementation names is what crosses the line. An implementer could satisfy every stated behavior with a different module layout, different type names, or a different safe-wrapper signature. This is a Warning, not a Blocker — the spec is implementable as written and the leakage is consistent with an intent-driven design rather than accidental over-specification.
Gate 3 — Assumptions explicit ✅ Pass
Key assumptions are surfaced:
- libkrun v1.18+ required for
krun_set_passt_fd— flagged explicitly in Open Question 5 (03-spec-minvmd-networking-gvproxy.md:450–453). - gvproxy exits on EOF from its socketpair FD — stated in Design Considerations (
03-spec-minvmd-networking-gvproxy.md:406–409). FD_CLOEXECinheritance semantics acrossfork+exec— stated at03-spec-minvmd-networking-gvproxy.md:381–385.- gvproxy's
--fd <n>interface and immediate readiness — stated at03-spec-minvmd-networking-gvproxy.md:468–471.
Minor: the Unit 1 proof artifact references serial_test for env-var isolation (03-spec-minvmd-networking-gvproxy.md:183) without noting whether the crate is already a dev-dependency. Not a finding — a standard Rust test-isolation crate is a reasonable assumption.
Gate 4 — Proof artifacts present and behavioral ✅ Pass
Each demoable unit carries at least one proof artifact. None would pass against an empty PR:
- Unit 1 (2 artifacts): targets
net.rs(new file) and an e2e integration that would fail without the fallback wiring. - Unit 2 (2 artifacts): targets new
VmConfig.net_modefield andset_passt_fd— both absent today. - Unit 3 (1 artifact): targets
net::check_network_policy, which does not yet exist.
Summary
| Gate | Result |
|---|---|
| 1 · Acceptance criteria testable | ✅ Pass |
| 2 · No implementation leakage | |
| 3 · Assumptions explicit | ✅ Pass |
| 4 · Proof artifacts present and behavioral | ✅ Pass |
No Blockers. The spec boundary passes clean. The single Warning (Gate 2) is advisory — the spec is detailed enough that the implementation leakage may be intentional to give the executing agent precise direction, but it narrows the implementer's latitude beyond what the behavioral goals require.
Generated by sdd-validate for issue #407 · sonnet46 1.6M · ◷
Spec for tracking issue #404.
What this spec covers
Adds
gvproxy(containers/gvisor-tap-vsock) as the default outbound network transport forminvmd-managed VMs on both macOS (Hypervisor.framework) and Linux (KVM), with TSI remaining selectable as a fallback viaMINVMD_NETMODE=tsi.The TSI transport in
vm.rscaps at ~62 concurrent connections — incompatible with the 200-concurrent cache-fetch target (informed by #204).gvproxyreplaces TSI via libkrun'skrun_set_passt_fdAPI (virtio-net backed by an external passt-protocol process), eliminating the cap.Demoable Units
src/net.rs(new),cmd/run.rs,state.rskrun_set_passt_fdFFI +VmConfigvirtio-net wiringkrun/raw.rs,krun/ctx.rs,vm.rs,cmd/vmm_child.rssrc/net.rs,cmd/run.rsKey design decisions in the spec
socketpair(AF_UNIX, SOCK_STREAM), passes one end to gvproxy (--fd <n>) and the other FD number viaMINVMD_NET_FDto the VMM child — avoids listen-bind racesadd_vsock_port2(the minimald control socket bridge) is orthogonal to the outbound networking mode and unchanged in all modeskrun_set_passt_fdis platform-agnostic;VmConfig.apply()is un-gated from macOSOpen questions (flagged for ADRs)
Next step
Merging this spec PR advances tracking issue #404 from the spec phase into triage — a write-access author comments
/triageon #404 to start the architecture phase.Closes #406
Summary by CodeRabbit