Skip to content

VMM: networking mode management#756

Open
Leechael wants to merge 9 commits into
masterfrom
vmm-networking-rpc-iteration
Open

VMM: networking mode management#756
Leechael wants to merge 9 commits into
masterfrom
vmm-networking-rpc-iteration

Conversation

@Leechael

@Leechael Leechael commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@Leechael Leechael force-pushed the vmm-networking-rpc-iteration branch 2 times, most recently from 501848b to de1bbb1 Compare July 8, 2026 09:46
@Leechael Leechael marked this pull request as ready for review July 8, 2026 10:09
@kvinwang

kvinwang commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

A note on the guest_ips addition before we grow it further:

The whole guest_ip machinery only exists to serve VMM's built-in port-forward feature. And the way it learns the guest IP is pretty awkward — it has to talk to an external DHCP server out-of-band (dnsmasq --dhcp-script → dhcp-notify.sh → ReportDhcpLease RPC) just to find out what address the guest got. So the stored value is really "what dnsmasq handed out," not the guest's actual IP: it goes stale (the del/release path is a no-op), it's empty for user/slirp NICs, and the RPC that feeds it is unauthenticated. That's a lot of moving parts for what it delivers.

Context worth flagging: the built-in port forwarding — and guest_ip along with it — was already a candidate for removal; the intent was to deprecate and delete it rather than build on top of it.

Given that, I'd suggest not extending this into a per-MAC guest_ips map / guest-ips.json file plus the new interfaces[].ip surface. That doubles down on a subsystem we meant to drop: it adds a second on-disk file with the same staleness/trust caveats and grows the public API contract around it. If multi-NIC status reporting is genuinely needed, let's settle the fate of port-forward first — and if it stays, the DHCP-report path should get authentication + a release/cleanup path before we persist and expose it more widely.

Happy to help sketch the removal path if we decide to go that way.

Leechael added a commit that referenced this pull request Jul 9, 2026
@Leechael
revert(vmm): drop per-MAC guest_ips extension
36b18f3 
Per PR #756 review: the guest_ip machinery exists only to serve VMM's
built-in port-forward, a subsystem already slated for deprecation. It
learns the guest address out-of-band from an external DHCP server via an
unauthenticated ReportDhcpLease RPC, so the stored value is stale-prone
and empty for slirp NICs. Extending it into a per-MAC map doubled down on
that subsystem: a second on-disk file with the same staleness/trust
caveats plus a wider public API surface.

Roll back the extension rather than build on top of it:
- proto: remove NetworkInterfaceStatus.ip, renumber trailing fields
- app.rs: drop guest_ips map from state and DHCP-lease path; port
  forwarding uses the single guest_ip again
- qemu.rs: remove guest_ips field, guest-ips.json persistence
  (guest_ips_path/guest_ips/set_guest_ip_for_mac), and per-NIC ip
  population in VmInfo::interfaces
- ui: drop the per-interface IP column and its grid track

Multi-NIC status reporting can be revisited once the fate of port-forward
is settled; if it stays, the DHCP-report path needs authentication and a
release/cleanup path before persisting and exposing guest IPs more widely.
Leechael added a commit that referenced this pull request Jul 9, 2026
@Leechael
revert(vmm): drop per-MAC guest_ips extension
76b965a 
Per PR #756 review: the guest_ip machinery exists only to serve VMM's
built-in port-forward, a subsystem already slated for deprecation. It
learns the guest address out-of-band from an external DHCP server via an
unauthenticated ReportDhcpLease RPC, so the stored value is stale-prone
and empty for slirp NICs. Extending it into a per-MAC map doubled down on
that subsystem: a second on-disk file with the same staleness/trust
caveats plus a wider public API surface.

Roll back the extension rather than build on top of it:
- proto: remove NetworkInterfaceStatus.ip, renumber trailing fields
- app.rs: drop guest_ips map from state and DHCP-lease path; port
  forwarding uses the single guest_ip again
- qemu.rs: remove guest_ips field, guest-ips.json persistence
  (guest_ips_path/guest_ips/set_guest_ip_for_mac), per-NIC ip
  population in VmInfo::interfaces, and the now-dead VmInfo.guest_ip
  field that only fed that population
- ui: drop the per-interface IP column and its grid track

Multi-NIC status reporting can be revisited once the fate of port-forward
is settled; if it stays, the DHCP-report path needs authentication and a
release/cleanup path before persisting and exposing guest IPs more widely.
@Leechael Leechael force-pushed the vmm-networking-rpc-iteration branch from c447e16 to 76b965a Compare July 9, 2026 15:59
Leechael added 5 commits July 10, 2026 00:30
@Leechael
feat(vmm): multi-NIC networking with per-VM config and RPC support
821e873 
Single-NIC bridge/user networking could not express mixed topologies
(e.g. one bridge NIC for external traffic + one user-mode NIC for
isolated management). Extend the model to support multiple NICs per VM.

- Manifest field: networking (singular, Optional) -> networks (Vec)
  with backward-compatible deserialization of the legacy field
- Proto: add repeated NetworkingConfig in VmConfiguration/UpdateVmRequest,
  NetworkInterfaceStatus in VmInfo, and NetworkingCapabilities in
  GetMetaResponse
- QEMU: generate one -netdev/-device pair per entry with indexed net IDs
  and per-NIC deterministic MACs (SHA256 of vm_id:index)
- DHCP: match leases against all NICs, persist per-MAC guest IP map
- Port forwarding: select the first bridge NIC with forward_service_enabled
- Validation: reject bridge mode when the host interface does not exist,
  reject custom mode via RPC (manifest-only)
- Measurement: networking config excluded from VM measurement to allow
  topology changes without re-attestation
@Leechael
refactor(vmm): simplify networking code following upstream conventions
3e2a51b 
- Replace hand-written Manifest Deserialize impl with derive +
  Manifest::from_json() for legacy "networking" field migration
- Simplify resolve_networking to clone global config as base then
  selectively override non-empty per-VM fields
- Extract effective_networks() on VmState and VmInfo to eliminate
  four duplicated runtime_networks fallback blocks
- Remove redundant forward_service_enabled pre-assignment in
  resolve_requested_networks (now inherited from global config clone)
@Leechael
refactor(vmm): remove unused proto fields from NetworkInterfaceStatus
8868bfd 
Drop if_name, lowerdev, and pci_bdf — all three were always set to
None in the only construction site. Add them back when a backend
actually populates them.
@Leechael
review(vmm): align networking backend API
8c49e0c
@Leechael
feat(vmm-ui): add VM networking controls
080c6cc
Leechael added a commit that referenced this pull request Jul 9, 2026
@Leechael
revert(vmm): drop per-MAC guest_ips extension
584defe 
Per PR #756 review: the guest_ip machinery exists only to serve VMM's
built-in port-forward, a subsystem already slated for deprecation. It
learns the guest address out-of-band from an external DHCP server via an
unauthenticated ReportDhcpLease RPC, so the stored value is stale-prone
and empty for slirp NICs. Extending it into a per-MAC map doubled down on
that subsystem: a second on-disk file with the same staleness/trust
caveats plus a wider public API surface.

Roll back the extension rather than build on top of it:
- proto: remove NetworkInterfaceStatus.ip, renumber trailing fields
- app.rs: drop guest_ips map from state and DHCP-lease path; port
  forwarding uses the single guest_ip again
- qemu.rs: remove guest_ips field, guest-ips.json persistence
  (guest_ips_path/guest_ips/set_guest_ip_for_mac), per-NIC ip
  population in VmInfo::interfaces, and the now-dead VmInfo.guest_ip
  field that only fed that population
- ui: drop the per-interface IP column and its grid track

Multi-NIC status reporting can be revisited once the fate of port-forward
is settled; if it stays, the DHCP-report path needs authentication and a
release/cleanup path before persisting and exposing guest IPs more widely.
@Leechael Leechael force-pushed the vmm-networking-rpc-iteration branch from 76b965a to c728dcd Compare July 9, 2026 16:34
kvinwang and others added 4 commits July 10, 2026 07:40
@kvinwang @Leechael
feat(mr): measure virtio-net NIC count in RTMR0
d34fc6c 
Each virtio-net-pci device changes the guest ACPI/DSDT layout and thus
RTMR0. Before this change dstack-mr hardcoded exactly one NIC, so a CVM
booted with the new multi-network support (repeated `networks`) would
produce an RTMR0 the verifier could never reconstruct, and attestation
would fail closed. Dropping ACPI from the measurement instead would let
an untrusted host silently attach un-attested devices under an unchanged
quote, so the NIC count must be measured.

Mirror the existing `num_gpus` plumbing: add `num_nics` to
`dstack_types::VmConfig` and `dstack_mr::Machine`, emit one
`-netdev/-device virtio-net-pci` pair per NIC in the ACPI generator, and
thread the resolved interface count from the VMM through to the verifier.

Backward compatibility:
- `num_nics` defaults to 1 and is omitted from the serialized VmConfig
  when equal to 1, so existing configs, their measurement cache keys, and
  RTMR0 values are byte-for-byte unchanged (verified against the previous
  hardcoded layout).
- legacy configs without the field deserialize to 1.

Verified: RTMR0(num_nics=1) == RTMR0(previous hardcoded layout);
RTMR0(num_nics=2) differs.
@Leechael
revert(vmm): drop per-MAC guest_ips extension
89d5e89 
Per PR #756 review: the guest_ip machinery exists only to serve VMM's
built-in port-forward, a subsystem already slated for deprecation. It
learns the guest address out-of-band from an external DHCP server via an
unauthenticated ReportDhcpLease RPC, so the stored value is stale-prone
and empty for slirp NICs. Extending it into a per-MAC map doubled down on
that subsystem: a second on-disk file with the same staleness/trust
caveats plus a wider public API surface.

Roll back the extension rather than build on top of it:
- proto: remove NetworkInterfaceStatus.ip, renumber trailing fields
- app.rs: drop guest_ips map from state and DHCP-lease path; port
  forwarding uses the single guest_ip again
- qemu.rs: remove guest_ips field, guest-ips.json persistence
  (guest_ips_path/guest_ips/set_guest_ip_for_mac), per-NIC ip
  population in VmInfo::interfaces, and the now-dead VmInfo.guest_ip
  field that only fed that population
- ui: drop the per-interface IP column and its grid track

Multi-NIC status reporting can be revisited once the fate of port-forward
is settled; if it stays, the DHCP-report path needs authentication and a
release/cleanup path before persisting and exposing guest IPs more widely.
@Leechael
chore(vmm): adapt make_vm_config test callers to requirements param
d53aaa5 
Rebasing onto master pulled in the new
make_vm_config(.., requirements: Option<&Requirements>) parameter added
by the launch-token-requirement work (#763). The multi-NIC bridge/user
config-equality test still called the 5-arg form; pass None for the new
argument so the crate builds against the rebased signature.
@Leechael
fix(vmm): make runtime networking state consistent
5693786 
Reject configurations with multiple forwarding-enabled bridges so DHCP lease updates cannot redirect the VM-wide forwarding target between interfaces.\n\nPersist runtime network snapshots atomically before launching QEMU, publish the snapshot to in-memory state before DHCP can arrive, and clear both copies when process startup fails. Add regression coverage for forwarding validation and atomic snapshot replacement.
@Leechael Leechael force-pushed the vmm-networking-rpc-iteration branch from 37e88da to 5693786 Compare July 9, 2026 23:40
kvinwang
kvinwang approved these changes Jul 10, 2026
View reviewed changes

@kvinwang kvinwang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

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

Reviewers

@kvinwang kvinwang kvinwang approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Leechael @kvinwang