feat(vmm): give port mappings a NIC, and say which NIC publishes them - #1154
Merged
Conversation
`port_map` is implemented as QEMU `hostfwd=` entries on a user-mode netdev. A bridge NIC has none, so `configure_networking` finds no interface to hang them on and emits none -- with no warning, no validation, and `GetInfo` still reporting the ports as though they worked. A VM moved from user mode to a bridge silently loses every published port. The VMM cannot fix that itself: it runs without CAP_NET_ADMIN by design, and the userspace forwarder that used to cover this was removed in e2e607f because proxying on the host hands back the per-packet cost that leaving user mode was meant to escape. netd is privileged and is the only component that sees every VMM instance on a host, so it is also the only one that can arbitrate a host port between them. So state the requirement rather than assume it is met. `ingress` on `prepare_bridge` carries protocol, host address, host port and guest port. The host address is not decoration -- an admin port bound to loopback and a published one differ only there. Every field is caller-named, which is what `bridge`, `mac` and `queues` already get and the opposite of `filtered`. The line is whether netd can check what it is handed: an nwfilter name cannot be checked for whether it filters anything, while a host port is a closed space a node policy can be stated over. Naming is not deciding; which ports may be handed out stays netd's own configuration, exactly as `allowed_bridges` governs the bridge a caller names. The netd here builds interfaces and does not forward ports. It says so by leaving `ingress` out of its response, which is the reading `queues` already has: absent separates "this netd does not do that" from "nothing was asked for". The VMM warns on that rather than failing, because a VM deployed before this has been running with its ports dropped and refusing to launch it now would turn a silent misconfiguration into an outage on upgrade. Also add `workdir` to both prepare operations. Untrusted and never read for a decision, it is there so an operator reading netd's log can get from an opaque TAP name back to the VM that asked for it. No new operations, and nothing removed.
kvinwang
force-pushed
the
feat/netd-ingress
branch
from
August 28, 2026 14:24
7e1e688 to
104b526
Compare
kvinwang
changed the title
Three things that are one thing: a port mapping had no way to say which
NIC it uses, the NIC it silently got was often the wrong one, and on a
bridge it got nothing at all.
`port_map` is implemented as QEMU `hostfwd=` entries, and those need a
user-mode netdev:
let hostfwd_index = self.prepared.networks.iter()
.position(|n| n.mode == NetworkingMode::User);
Multi-NIC (#756) made that a choice, and it has been made silently ever
since. A bridge NIC for external traffic beside a user-mode NIC for
management -- the topology multi-NIC was added for -- puts every
published port on the *management* NIC: the traffic reaches the guest,
but over slirp, bypassing whatever the bridge NIC's nwfilter was there to
enforce and hiding the client's address behind the slirp gateway. A
second user-mode NIC can never publish anything, because `position`
returns the first. And with no user-mode NIC at all there is no
`hostfwd_index`, so every mapping is dropped with no warning while
`GetInfo` keeps reporting the ports as though they worked.
So say it. `PortMapping.nic_index` names the NIC a mapping enters
through, `@<nic>` on the CLI, unset resolving to the first user-mode NIC
and failing that the first bridge NIC. Existing VMs keep their behaviour
exactly wherever a user-mode NIC exists; where none does, ports now have
somewhere to go instead of nowhere.
One mapping resolves to exactly one NIC, and that NIC's backend decides
the mechanism: `hostfwd=` for user mode, netd for a bridge. Both sides
read the same resolution, so no host port can be claimed twice.
The VMM cannot forward a bridge NIC's ports itself -- it runs without
CAP_NET_ADMIN by design, and the userspace forwarder that used to cover
this was removed in e2e607f because proxying on the host hands back the
per-packet cost that leaving user mode was meant to escape. netd is
privileged and is the only component that sees every VMM instance on a
host, so it is also the only one that can arbitrate a host port between
them. `ingress` on `prepare_bridge` carries the requirement there.
Every field of it is caller-named, which is what `bridge`, `mac` and
`queues` already get and the opposite of `filtered`. The line is whether
netd can check what it is handed: an nwfilter name cannot be checked for
whether it filters anything, while a host port is a closed space a node
policy can be stated over. Naming is not deciding.
The netd here builds interfaces and does not forward ports. It says so by
leaving `ingress` out of its response -- the reading `queues` already has,
where absent separates "this netd does not do that" from "nothing was
asked for". The VMM warns rather than failing, because a VM deployed
before this has been running with its ports dropped and refusing to
launch it now would turn a silent misconfiguration into an outage.
Also add `remove_all`. Teardown by identity only reaches the NIC indices
its caller still has a record of, and that record is written after the
interface exists: a VMM killed in between leaves a TAP nothing on disk
points at, a lost record reads as "nothing to remove", and a manifest
that lost a NIC leaves an index the list no longer reaches. netd derives
every name a VM could occupy instead, bounded by what an identity may
say. The VMM sweeps before preparing a launch as well as on stop, so a
launch is self-healing regardless of what the record says, and an
unreachable netd no longer fails a stop.
And `workdir` on both prepare operations: untrusted, never read for a
decision, there so an operator reading netd's log can get from an opaque
TAP name back to the VM that asked for it.
kvinwang
changed the title
`remove_all` is an operation, and an operation a netd does not have answers the same way one that failed does: `ok: false` with a message. The VMM read that as a failed sweep, so against any netd older than this branch a stop returned an error *and* left every interface behind -- strictly worse than the per-NIC removal it replaced. `removed` was shaped to carry the other half of the signal and then discarded with `unwrap_or_default`, which turns "this netd does not sweep" into "this VM had nothing to remove": the exact conflation `queues` and `ingress` are shaped to avoid. So ask. `hello` answers a capability set, is served before the operation lock so a probe never waits behind a collection, and is bounded well below the request timeout because every teardown asks it. A netd that predates it answers an error, which is still an answer: reached, and old. The three states a caller has to act on -- absent, old, capable -- are distinguishable here and nowhere else. Teardown is then unconditional and non-fatal, which is one decision made twice. It was gated on `needs_netd_interface`, which reads the VM's *current* backend: a VM whose NIC was a bridge when its TAP was built and is user-mode now skipped the release for interfaces that exist, and neither its stop nor its next launch would ever reach them again. And it returned an error a stop propagated, so a netd outage became a fleet that could not be stopped. Nothing is lost by not failing: a launch releases before it prepares, so the next one is self-healing. `is_unreachable` never returned true. The marker is attached with `context`, which makes it the context *of* a chain link rather than a link, so the walk over `chain()` could not see it -- every "an unreachable netd is not a failure" branch in this crate was dead. `downcast_ref` sees it. And a fake netd to talk to. The VMM's side of this protocol had no test at all, because every path looked like it needed a privileged daemon; it needs something that answers on a socket.
Deriving a name answers "where is this VM's interface". It cannot answer "whose is this interface", and that is the question a leak is made of: a VM whose directory was deleted, a VMM instance that was decommissioned, an interface built before an upgrade. A digest is not reversible, so nothing on the host could attribute one -- and with several VMM instances sharing a netd, nothing could even tell whose it was to collect. So write it down where it cannot drift: `ip link set dev <tap> alias`. The kernel holds it for exactly the interface's lifetime. A file under /run would be a second thing to keep in step with the first, and a record that got out of step -- written after the interface, lost with the directory -- is the failure being fixed, not the fix. The record is a hint, never an authority. Anything that can reach this socket can assert an identity, and the interface name is a digest of one, so a record is believed only when re-deriving the name from it reproduces the name it is written on. Forged, truncated, ambiguous and absent all fail that one check and land in the same bucket, which is the bucket a collection has to treat conservatively anyway. `list` then enumerates: the reserved `dt` name space, narrowed by the kernel's own answer about what kind of device it is, so netd will not offer up something it did not create. Orphaned nwfilter bindings are listed too -- a binding outlives the interface it was bound to, so an inventory that only looked at interfaces would miss the one piece of state that survives them. Absent rather than empty on a netd that cannot enumerate: "I hold nothing" and "I cannot say" are not the same answer. `dstack-vmm netd list` reads it. An operator could not previously see what netd held at all. One new refusal: an identity whose record would exceed the kernel's 255-byte alias, or whose instance ID contains the separator. Both would produce an interface nothing can attribute, which is the state this exists to stop creating. Real identities are a `path-` digest and a VM ID: about sixty bytes.
Per-VM teardown reaches only what its caller can still name, and a leak is made of exactly the interfaces nothing names any more: a VM removed while the VMM was down, a workdir deleted by hand, a teardown that raced a netd outage and whose VM no longer exists to retry it. No amount of care at the per-VM call sites reaches those, because the call site is gone. `gc` compares what netd holds against the VMs a VMM instance has. The live set is every VM it has, running or not: a VMM restarts under VMs that keep running, and collecting by what is running would delete their networking out from under them. It runs at startup after the VMs are loaded and before the API is served -- the set is a snapshot, and a VM created between taking it and acting on it would be in netd's listing and not in the set -- and then on an interval, for what accumulates while the VMM is up. Three rules, and the second is why interfaces carry a record at all: - recorded as another instance's: never touched. Two VMM instances share one netd, and without the record a collection cannot tell that instance's running VM from garbage. - recorded as ours, for a VM we no longer have: collected. - no record that checks out: kept by default, and reported. It is not nobody's -- it is an interface from before ownership was recorded, or from another netd. `collect_unattributed` opts in where the operator knows nothing else creates interfaces in netd's name space. The upgrade is safe by construction: a collection derives the names its own live VMs would occupy and keeps those even when told to collect unattributed, so a fleet running from before this existed survives the first pass, and each interface gains a record the next time its VM launches. Both passes are bounded. Under the operation lock and inside a serialized accept loop, an unbounded pass is not slow, it is an outage -- one hung `virsh` per interface holds every other VM's prepare behind it while the caller that asked has long since timed out. A deadline stops the pass and reports what it did; libvirt is asked once per pass rather than once per interface, because asking again after it has failed is how a bounded pass becomes an unbounded one. `dstack-vmm netd remove-vm` for a VM whose VMM will never ask again.
A port mapping is a request. Which NIC carries it decides who answers it: QEMU's `hostfwd` on a user-mode NIC, always; the node's netd on a bridge NIC, which -- like the netd in this repository -- may not forward host ports at all. `GetInfo` reported the request either way, so a VM could list published ports that nothing on the host forwarded any traffic to. So keep the answer. A prepare's `ingress` response is recorded on the NIC beside `netd_interface` and `device`, and `PortMapping.published` reports it: absent for a VM that is not running or a VMM that predates the field, true only where something actually publishes the port. Held to it per mapping, not per interface -- a netd may refuse one port out of a set, and the mapping that lost is the one worth naming. And refuse at deployment what a launch can only warn about. Two ways to have nowhere to go: a mapping that resolves to no NIC at all, and one that resolves to a bridge NIC on a node whose netd does not forward. Refusing costs nothing there, because nothing is running on the answer yet; refusing at launch would turn a VM that has been running with its ports dropped into an outage on upgrade. An update refuses only when it moved the ports or the networking, so a VM deployed before the node could answer for its ports stays editable in every other respect. The ownership contract that makes this collectable is now written down: a host port a netd publishes is owned by the interface and released with it, and there is deliberately no operation that releases one separately. A reservation outliving its interface could never be attributed to a VM again, because the interface is the only thing that carries a record.
Everything else here reasons about strings. This creates a real TAP, reads the alias back off it, finds it by enumeration, and checks that the guards refuse what they are meant to: a device with one of netd's names that netd did not create, and a record that does not re-derive the name it is written on. It refuses to run in the host's network namespace. Unsharing one from inside the test is not enough -- `/sys/class/net` keeps showing the old namespace until sysfs is remounted, which is most of what `ip netns exec` does -- so it asks to be put in one instead, and says so in the doc comment. That also means it can never touch a real node's interfaces, including when it fails. The docs gain what an operator needs to act on any of this: how to read an interface's owner, how a collection decides, why an interface with no ownership record is kept rather than deleted, and why upgrading needs no migration step.
…hat loaded A VM whose manifest is corrupt or whose image is missing fails to load and is only logged -- but its QEMU may well still be running. Collecting by what loaded would delete a running VM's networking over a file the VMM could not parse. A directory is enough of a claim, and what has been removed for real leaves none behind. An unreadable VM directory is not an empty one either: read as empty it would offer every interface on the host up for collection, so it is an answer the reconciliation declines to act on at all.
…onging to At startup rather than at the first launch. The VMM derives one that is always valid; an operator who configured their own learns here rather than from the first VM that fails to get a NIC.
It is what netd records on every host interface this VMM asks for, and what a collection uses to tell this instance's interfaces from another's. Two VMMs sharing one value on one host would each collect the other's running VMs.
Additive on the wire and in the API: a client states a request, and `published` is the server's answer to it.
The accept loop served one connection at a time and `handle_request` blocks, so answering `hello` "before the lock" bought nothing: the connection was not *accepted* until whatever netd was doing finished. A collection may run for twenty seconds and a single `virsh` for thirty, while the capability probe gives up after five -- so any netd doing real work looked absent, and an absent netd is one whose teardown the VMM skips and whose deployments it refuses with "this netd does not forward host ports". Both wrong, and both introduced by the probe this branch added. One task per connection, with the blocking half on `spawn_blocking`. Serialization still holds, and now holds where it is actually stated: the operation lock is an flock, which contends between two open descriptions in one process exactly as it does between processes. What the single-connection loop added on top was head-of-line blocking and nothing else. The client side stops asking a question it does not need to ask. A release asks netd to release, rather than asking whether it may: the operation cannot be misread as absence, and only a refusal -- netd answering -- is worth a round trip, which is also exactly when the fallback matters. That removes the probe from every stop, and removes the case where the netd released one commit before this one, which does implement `remove_all`, was told it could not and given eight per-NIC removals instead. Reconciliation reads the same way. The one probe left in front of an action is the one that decides whether to act on the *absence* of an ownership record, which means nothing unless netd writes one -- so `attribution` is finally read where its documentation already claimed it was. Also: an orphaned nwfilter binding is now collected by default. It is the one unattributable thing that is unambiguously dead -- a record can only live on an interface, so it can never gain one, and nothing is using a binding with nothing to bind to. Left to the conservative default it was the single leak in this design that nothing could ever collect, since the VM it belonged to is gone and not even an operator could name it. And three narrower ones: a directory entry that cannot be read no longer silently drops a VM from the set that claims interfaces (the same care the `read_dir` error already got); a binding that answers on a different address than it was asked for is no longer reported as publishing the port, since an admin port on loopback and a published one differ only there; and two live VMMs configured with one `cvm.instance_id` now refuse to start rather than each collecting the other's running VMs.
…not cancel It wrapped the whole exchange, which reads as a bound on the request and is not one: `handle_request` is synchronous and shells out, so the timeout could not cancel it. All it did was drop the connection at thirty-five seconds while the work went on running to completion -- and then log "netd connection timed out" for a request netd in fact finished. A caller has its own deadline and has gone by then; what this cost was netd's own account of what it did. Now the timeouts bound the socket reads and writes, which is what the comment said they were for. Observed against a netd wedged in a twenty-five second helper: `hello` answers in 0.00s, a queued `list` answers at 49s rather than being cut off at 35s, and the log says what happened.
…e the lock is Two findings from review, and the second reshapes the design. `virsh nwfilter-binding-list` accepts no options at all -- `--name` is not one of them, and asking for it fails the whole call. So `existing_bindings` has always returned `None`, which was survivable until this branch made a sweep read that as "libvirt is down, skip the bindings": every stop, every pre-launch release and every removal then deleted the TAP and left the binding, permanently for a VM that was removed. Verified against virsh 10.0.0 on a node holding seven real bindings. It parses the table now, narrowed to netd's own name space so a header, a rule line or a moved column cannot produce a name; and a listing that could not be produced no longer decides whether deletions are attempted. The collection moves out of netd. A collection decided inside netd is decided against a set of live VMs that was true when the caller *sent* it -- netd runs the request when it wins the operation lock, which may be much later -- so a VM created in between is absent from the set and present on the host, and its TAP is deleted while QEMU is starting on it. That was safe at startup, where the API is not yet served, and the interval pass ran the same code with the API served. netd cannot close it: the lock that closes it is the VMM's per-VM launch lock, and netd has no way to take one. So the VMM asks `list` and decides for itself, per VM, under exactly that lock, re-reading whether it claims the VM while holding it. A launch and a collection of the same VM can no longer both believe they are alone. It also deletes `Gc`, `gc_plan`, `live_interface_names`, `collect_garbage`, `UnattributedPolicy`, `Collection`, dry runs, and the `collect_unattributed` knob: the collection is `list` plus the whole-VM sweep a stop already uses. What is left over is what nothing can attribute, and nothing decides about it -- correct, and previously a dead end, since `netd list` shows no VM to name in `remove-vm`. `netd remove-interface <name>` is the operator's way to say what a machine cannot work out. Also: a VM directory that is not there is no longer read as "no VMs". "Never ran" and "the volume is not mounted yet" are the same error, and only one of them means there is nothing to collect.
`Unreachable` and `Legacy` are both produced by transient failures, and holding either for half a minute turns one blip into a deployment refused because 'this netd does not forward host ports' -- which may not be true. Only a real capability answer is worth reusing; the other two are re-asked on paths that were already making a round trip.
… still An audit of the stop-time removal found the removal itself sound and the *decisions* around it not. Two callers read "is it running" outside the per-VM launch lock and acted on the answer inside it, which is precisely the race the lock exists to close. `update_vm` with a networking change: the read says not-running, a launch then takes the lock, prepares TAPs and deploys QEMU, and the update takes the lock afterwards and releases the interfaces of a VM that is now running. QEMU keeps running with a dead NIC, the supervisor still reports it healthy, and nothing logs a thing. `finish_remove_vm`: it stopped the process and waited for exit entirely outside the lock. A launch that passed the `removing` check before the marker was set is already inside the lock and has not deployed yet, so the wait sees nothing running and returns at once; the launch then starts QEMU, and removal takes the lock and deletes its interfaces, its workdir and its CID. The VM runs on, invisible, until the next orphan sweep at startup. Both now take the lock first and ask afterwards. A third, smaller: an automatic restart reads the started flag off disk and only then queues a launch, which waits for the lock a concurrent stop is holding. Without a re-read under that lock, the launch resurrects a VM the operator was told was stopped. An explicit start sets the flag itself and has nothing to re-read, so only the automatic path re-checks. Two things the same audit found in the removal proper. A sweep truncated by netd's deadline reported `incomplete`, which the client dropped -- so a host with interfaces left over logged "released 3 interfaces" and nothing else; it now says so. And `Request::Remove` no longer fails when libvirt cannot delete a binding: that strictness was written for prepare, where a leftover binding blocks the creation about to happen, and at removal it only leaves the interface itself up on the bridge instead of a binding libvirt hands back on its next listing. It also now agrees with the whole-VM sweep, which was always best effort. The comment on `stop_vm` claimed reconciliation would collect what a failed release left behind. It will not: a stopped VM is still one this instance claims, so a VM that is never started or removed again keeps its interfaces. Say that instead.
…n it Holding the per-VM launch lock across removal closed a race and opened two queues behind a wait its own comment measures in hours. A start of a VM being removed asked `removing` only after taking the lock, so it now waited out the whole removal to be told no. It asks before as well -- not instead: the marker can be set while it waits, so the answer under the lock is still the authoritative one. Reconciliation was worse. It takes the lock per VM, in sequence, so one VM stuck in removal stalled the collection of every other VM on the host -- and because the hourly task awaits it, the interval never fires again. The case is real rather than theoretical: an orphaned supervisor process cleaned up by `reload_vms` has no VM in memory and may have no directory, so it is exactly the kind of VM a collection considers dead while its removal is still polling for exit. It takes the lock without waiting now, and skips what it cannot get. That is not a compromise: a held lock means a launch, a stop or a removal of that VM is in flight, and every one of those manages the VM's interfaces itself, so waiting would be waiting for the thing that makes the work unnecessary.
…iting on it Both took the launch lock that removal holds until the VM has exited, and neither asked whether the VM was being removed at all -- so a stop or an update issued during a removal hung for as long as the removal took, to do work the removal was already doing. Neither has an internal caller; both are RPCs, and an RPC that hangs for hours is worse than one that says why.
`netd remove-interface` was inserted between `remove-vm`'s doc comment and its variant, so clap printed "Delete every interface netd holds for one VM" as the help for removing a single interface, and nothing at all for removing a VM's. Three claims that stopped being true, while their code changed underneath them: `Request::Remove`'s `filtered` field is still sent and still required on decode, but the handler no longer derives strictness from it; removal's wait is a SIGKILL teardown, not the "2+ hours" a graceful stop used to take; and what a truncated sweep leaves behind is collected by reconciliation when the caller was the removal, since there is no next launch to do it.
`refuse_if_removing` read a flag on the VM's entry, which the orphan cleanup never sets: `spawn_finish_remove` marks nothing when the ID has no entry, and that is exactly the case it exists for. So `finish_remove_vm` held the launch lock across a whole teardown while every operation on that ID was told it was free to proceed -- and `StartVm` then waited the teardown out with no error and no log, only to fail at the end on a VM no longer in memory, after it had already written started to disk. The mark now lives in the state, not in the entry, so an ID with no entry can carry one. It is cleared by a guard rather than by the last step of the happy path: `finish_remove_vm` has two `?`s after the wait, and a mark left behind by either is not a stale flag but a VM no operation can reach again, including the removal that would retry. `claimable_vm_ids` hands back the set it built instead of collecting it into a vector for the caller to linear-scan once per interface.
`update_vm` asked whether the VM was being removed only inside the branch that changes networking, and dropped the launch lock at the end of that branch. What follows is the part that writes: `put_manifest` creates the directory it writes into. An update that resumed after a removal deleted that directory recreated it holding nothing but a manifest -- invisible to `list_vms`, failing to load at every start, answering "VM not found" to a second removal, and claiming that VM's netd interfaces against collection for as long as the VMM runs. The interfaces a netd outage kept the removal from releasing then had nothing left that could reach them. The refusal moves to the top, before the compose file is written, and the lock is taken there and held to the end, with the refusal repeated under it. `validate_port_mapping_nics` is gated the way the publishability check beside it already is. It ran on every update, against the node's current default, so changing `cvm.networking.nic.mode` made every later update of a VM that pinned a mapping fail over a field the request never touched -- the unmanageable- rather-than-fixed outcome the comment three lines below it rules out.
… not there Every `connect` failure carried the `Unreachable` marker, and both callers that treat absence as "nothing to do on this host" -- the interface release and the periodic collection -- skip their work at `debug!` when they see it. The default configuration reaches that state: netd runs as root and chmods its socket to `0660`, while the VMM is meant to run unprivileged, so a VMM whose user is not in root's group gets `EACCES` on every call. Interfaces then accumulate with nothing said at the default filter, while `create_vm` tells the operator to go and run a netd that is already running. Only the two errnos that mean nothing is listening are read as absence now. `hello` is answered before the blocking pool rather than inside it. The pool is finite and its tasks cannot be cancelled, so a node whose `virsh` calls are all timing out fills it, and a `hello` queued behind them times out too -- putting back the busy-netd-reads-as-absent this daemon exists to avoid. A sweep that deleted an interface but could not clear its nwfilter binding reported success, while the orphaned-binding half of the same loop treated the same failure as an error. It reports the pass as incomplete, which is what the stop it came from already knows how to say. `owner_of` strips what sysfs appended rather than trimming the whole alias: an identity with trailing whitespace re-derived a name that is not the one it is on, leaving the interface permanently unattributable -- never collected, and removable only by hand.
`normalizePorts` built the pin and then rebuilt the object without it in a
trailing `map`, so the web UI could not pin a mapping at all -- and an edit
made in the UI silently unpinned a mapping that had been pinned from the CLI,
which the composable's own comment says cannot happen. TypeScript did not catch
it because the field is optional.
Three more, all in the same feature:
- A cleared NIC box pinned NIC 0. `v-model.number` hands back the raw string
when it does not parse, and `Number('')` is 0.
- The update dialog never passed `nic-count`, so its NIC column was always
hidden. A VM shrunk to one NIC could not have a stale pin cleared.
- The update always sent `update_ports`, so the server-side "only when this
request moved one of them" gate did not cover the UI at all: on a node whose
netd does not forward host ports, a VM with any port mapping could not be
edited from the UI in any respect. It is sent only when the mappings differ
from what the dialog opened with.
`setup-bridge.sh check` still failed the node when `qemu-bridge-helper` was not setuid root and `/etc/qemu/bridge.conf` had no `allow` line, three pages after the doc it is recommended by says neither is needed any more. Following both left a setuid binary nothing uses and a standing grant for any local user to attach a TAP to the bridge. The checks and the setup steps go; the teardown still removes the `allow` line, now saying why. Four statements corrected to match the code: - `dstack-vmm netd -c vmm.toml` does not parse. `--config` is not a global argument, so it has to come before the subcommand, as every other doc has it. - A VM on user networking does contact the netd socket: the release runs on every launch and every stop, before the decision about whether anything needs building. Nothing about the VM depends on the answer, which is the part worth saying. - The `instance_id` comment described two VMMs collecting each other's running VMs as a live hazard. A VMM that finds another live instance on its value refuses to start. - The CLI guide's port mapping section never mentioned `@<nic>`, which is in `--help` and in the onboarding doc.
`netd` in this repository builds interfaces; it does not forward host ports. `prepare_bridge` validates the `ingress` field of a request and then never reads it, and `capabilities()` said `ingress: false` unconditionally. Every mechanism built on top of that answer therefore described a forwarding netd that does not exist -- and each one had to be right about a host state the VMM does not control and cannot re-check, which is a bug surface bought with nothing. Gone, and with them 700 lines: - `PortMapping.published` in the RPC, `published_at`, `IngressBinding` and its `answers`, `Networking.ingress`, and the echo-and-compare in the launch path. No shipped client ever surfaced the field. - `refuse_unpublishable_ports`. It made `create_vm` with a port mapping fail outright on every bridge node running this repository's netd -- including `dstackup install`, which creates the KMS VM with one -- and refused on a netd that was merely restarting. What is left is the part that is a fact about the VM rather than about the host: a pin to a NIC that is macvtap or does not exist, or a VM with no user-mode and no bridge NIC at all. That check is local, synchronous, and now covers the unpinned case too, so `create_vm` and `update_vm` ask exactly one question about port mappings. - The `hello` handshake: `Capabilities`, `Reachability`, `probe`, the 30-second answer cache, `supports`, `records_ownership`, `forwards_ingress`. It existed to tell "netd does not have this operation" from "the operation failed", to decide between a sweep and a fallback. With the fallback gone there is nothing to decide: the release asks netd to sweep, and a refusal is a warning. What it holds is reclaimed by the VM's next launch or by reconciliation. - `release_recorded_interfaces` and `LEGACY_TEARDOWN_SPAN`, the eight-round-trip teardown for a netd too old to sweep. netd ships in this binary; the fix for one that cannot sweep is to restart it. `release_vm_interfaces` no longer takes the recorded networks -- it sweeps by identity, which is what made the record unnecessary -- so three call sites stop reading `runtime_networks` to hand it something it ignored, and `finish_remove_vm` loses one of the two `?`s that could strand a removal.
…tending Three structures modelled a host that forwards bridge-NIC ports. Nothing on this host does. `PrepareBridgeRequest.ingress` and `IngressRequest` carried a per-NIC port list to netd, which read it exactly zero times. It was a declaration of intent to a daemon with no mechanism to honour it -- the request half of the `published` chain the previous commit removed from the response. With it gone, `mode_carries_ingress` can say what is true: QEMU's `hostfwd=`, and nothing else. That makes `default_ingress_nic` one line rather than a fallback to a bridge NIC that would have accepted a mapping and dropped it -- and a bridge pin is now refused at deployment, where the caller is there to be told, rather than resolving to a NIC with no path into the guest. The refusal is narrower in the other direction: an *unpinned* mapping is never refused, because a bridge-only VM with a port map deploys today and must keep deploying. What it strands, the launch names. `NetdInterface` and `netd_teardown` were a closed loop. At the base of this branch the record decided which per-NIC `Remove` teardown sent; this PR replaced that with `remove_all`, which derives names and needs no record. What was left wrote the field, persisted it, and read it only through `netd_teardown`, whose one production caller assigned the result straight back into the field it had just read. `is_filtered` had no callers at all. The 6-line comment justifying it still described the behaviour this PR deleted. `Request::Remove.filtered` was mandatory on decode and explicitly ignored by the handler, kept for "a netd that predates that reasoning". `origin/next` has no such field on `Remove`, so the compatibility was with a stacked branch rather than with anything released -- and serde ignores an unknown flattened key, so an in-flight old netd decodes a request without it either way. Also repairs two comments a `published: None` cleanup truncated mid-sentence, and three places -- a doc, a vmm.toml comment and a qemu.rs comment -- claiming netd "arbitrates host ports between VMM instances".
`sweep_vm_interfaces` reported failure three ways. A binding it could not delete was an error in the branch for an orphaned one and a `warn!` plus an `incomplete` bit in the branch for a live one -- the same failure, read two ways, and the silent reading was the one that mattered. Both halves now fold into the same `first_error`, which folds the two branches into one loop and retires `remove_interface_in_pass`. That leaves `incomplete` with one producer: `COLLECTION_DEADLINE`, which cannot fire. The pass is 256 `stat()` calls; libvirt is asked once for a listing and at most once more for a delete, because the first failure latches it off. To spend twenty seconds it would need ~250 interfaces present for a single VM, against a `validate_identity` cap of 256 NICs and a real VM's one to four. The bit was threaded through `Outcome::Swept`, `Response`, `netd::Sweep`, a `warn!` and a `println!`, and no branch anywhere read it. `remove_all` returns a count. `InterfaceRecord.bound` goes the same way: written by `list_interfaces`, read only by the `FILTERED` column of `netd list`, where `kind` already says `binding` for the one case it distinguishes -- and it reported "no" both for an interface with no binding and for one libvirt could not be asked about. `create_vm` validated its port mapping NICs twice, on the same mappings against the same modes, because `create_manifest_from_vm_config` had already done it. Deleting the second call lets the UI drop `originalPorts` and the `JSON.stringify` deep-compare it needed to decide whether to send `update_ports`: a read-modify-write client does send the ports, so saying so is honest, and what the server checks on that path is now a local question about the VM's own pins. Also inlines `binding_cleanup` into its only caller, and corrects three comments the capability-probe removal left describing things the code no longer does.
…lecting after it The leak this PR set out to close is a VM removed while its interfaces could not be released: the release is deliberately non-fatal, so the removal went on to delete the workdir, and with it the only thing on the host that could still name what netd was holding. The answer was a whole-host reconciliation -- `netd list`, a claimable-VM set, a per-VM re-check under the launch lock, and an hourly task -- deciding from the outside which interfaces no VM claims. The VM already knows. `release_vm_interfaces` now says whether the sweep landed, and `finish_remove_vm` deletes the workdir only when it did. If netd refused, or was not there to ask, the directory and its `.removing` marker stay and the next VMM start resumes the removal, which `reload_vms` already knows how to do. `remove_all` is idempotent, so the retry costs one round trip. A VM that never asked netd for an interface is unaffected: the removal reads its recorded networks first, so an absent netd is not a reason to keep the directory of a VM netd never held anything for. Retrying at the next start is also strictly better than the timer it replaces, which waited up to an hour and skipped any VM whose launch lock was held. Gone with it: `reconcile_netd_interfaces`, `claimable_vm_ids`, `claims_vm`, `try_launch_lock`, `netd_reconcile_task`, `netd.reconcile_interval_secs`, and the startup pass -- about 500 lines including tests and docs. An existing `vmm.toml` that still sets the interval keeps loading; nothing denies unknown fields. Also gone is the duplicate-`instance_id` startup refusal and the `instance_id` it added to the discovery record. It caught one configuration and silently passed four -- a peer under a different uid (the discovery directory is per-user while the netd socket is host-wide), a peer registered by an older VMM, two VMMs starting at once, and any later edit -- while its error message taught operators that the collision was checked. `vmm.toml` says plainly that two instances must not share the value, and the derived default cannot collide. Kept: the ownership record. Teardown never needed it -- a sweep derives the names it deletes -- but an operator does, and on a host running several VMM instances it is the only thing that tells one instance's interfaces from another's. `netd list`, `remove-vm` and `remove-interface` are now the whole recovery story for what no VMM will retry: a workdir deleted by hand, or an interface recorded under an instance ID nothing uses any more.
kvinwang
changed the title
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.
Stacked on #1145 — that PR reshaped these structs (
filtered: bool,queues), so this lands on top of it.Three things that are one thing: a port mapping had no way to say which NIC it uses, the NIC it silently got was often the wrong one, and on a bridge it got nothing at all.
The bug
port_mapis implemented as QEMUhostfwd=entries, and those need a user-mode netdev:Multi-NIC (#756) turned that into a choice, and it has been made silently ever since:
positionreturns the first.GetInfokeeps reporting the ports as though they worked. A VM moved from user mode to a bridge silently loses every published port.I hit the third one planning a production gateway's move off slirp: five mappings, including the WireGuard endpoint 100+ CVMs depend on, would have evaporated on first boot with nothing in the log to say so.
nic_indexPortMapping.nic_indexnames the NIC a mapping enters through;@<nic>on the CLI:Unset resolves to the first user-mode NIC, where
hostfwd=has always gone. Existing VMs keep their behaviour exactly wherever a user-mode NIC exists.What publishes a host port
QEMU's
hostfwd=, and nothing else on this host.An earlier revision of this PR carried the port list to netd on
prepare_bridge, so that a netd which forwards could honour it. Nothing does: the netd in this repository builds interfaces, and the field was validated and then read exactly zero times. Every mechanism built on top of that answer — apublishedflag in the RPC, an echo-and-compare in the launch path, a capability handshake to tell "this netd cannot" from "this failed" — described a host that does not exist, and each had to be right about state the VMM does not control and cannot re-check. All of it is gone, along with acreate_vmrefusal that failed every port-mapped deployment on a bridge node,dstackup install's KMS VM included.What is left is the part that is a fact about the VM rather than about the host: a pin to a NIC that cannot carry a port is refused at deployment, where the caller is there to be told, and an unpinned mapping on a bridge-only VM still deploys — that shape exists today and must keep working — with the launch naming what it strands.
remove_allTeardown by identity only reaches the NIC indices its caller still has a record of — and that record is written after the interface exists. So:
The first one is not hypothetical: on a production host running a third-party netd, a failed prepare from two days earlier was still sitting in its state, half-built, with nothing able to find it.
remove_allnames a VM instead of an interface and derives every name that VM could occupy — bounded by whatvalidate_identitylets an identity say, so it is 256stats that usually miss. No state, no record. The VMM sweeps before preparing a launch as well as on stop, which makes a launch self-healing regardless of what the record says.Two follow-on fixes fall out: an unreachable netd no longer fails a
stop_vm(a VM's teardown should not depend on a daemon being up, andfinish_removealready only warned), and teardown no longer depends on the runtime record being accurate.Whose is this interface
Deriving a name answers "where is this VM's interface". It cannot answer "whose is this interface", which is the question a leak is made of.
netd now writes an ownership record onto every interface it creates — in the kernel's interface alias, so it has exactly the resource's lifetime rather than a file's. It is never trusted as authority: a record is believed only when re-deriving the interface name from it reproduces the name it is written on, so ambiguity, truncation and forgery all land in the same bucket as no record at all.
netd listenumerates them, andnetd remove-interfacedeletes what nothing can attribute — an interface built before netd recorded ownership, or by another netd, whose VM is gone.When a release does not land
The leak this PR set out to close is a VM removed while its interfaces could not be released: the release is deliberately non-fatal, so the removal went on to delete the workdir, and with it the only thing on the host that could still name what netd was holding.
The VM already knows.
release_vm_interfacessays whether the sweep landed, and the workdir — with its.removingmarker — is deleted only when it did. Otherwise the next VMM start resumes the removal, whichreload_vmsalready knows how to do, andremove_allis idempotent, so the retry costs one round trip. That replaced a whole-host reconciliation task, which waited up to an hour and skipped any VM whose launch lock was held.vmm_idcvm.instance_idnamed two different things one word apart: the otherinstance_ididentifies a CVM, is covered by attestation, and appears in the same RPC surface. Sitting under[cvm], the field that must differ between two VMMs on a host read as the field that differs between VMs.It is now the top-level
vmm_id, next tonode_name, carrying the constraintnode_namedoes not: unique per host, stable across restarts, no:.[cvm] instance_idstill loads with a deprecation warning, and the netd protocol keepsinstance_idas a serde alias. The discovery record carriesvmm_idtoo, so a VMM ID fromnetd listleads back to a VMM throughvmm ls.workdirUntrusted, never read for a decision, present so an operator reading netd's log can get from an opaque TAP name back to the VM that asked for it.
Compatibility
nic_indexandworkdirare optional and default to today's behaviour.PortMappinggains proto tag 5;NetworkInterfaceStatusis untouched.[cvm] instance_idand the netd protocol'sinstance_idboth still work. The interface alias record is positional, so no interface on a running host needs rebuilding.The one behaviour change is deliberate: a VM with ports and no NIC that can carry them used to drop them silently and now says so.
Testing
cargo test -p dstack-vmm— 196 pass.cargo clippy --all-targetsclean. CLI parsing exercised directly for the@<nic>suffix and its rejections, andvmm ls/vmm switchagainst a synthetic discovery record.New cases cover the unpinned resolution order (including that it still lands where
hostfwdalways put it), pinning and out-of-range pinning, the one-mapping-one-NIC property, the request shape forworkdir, the sweep's shape and bound, the ownership record against the kernel, and that the pre-renameinstance_idspelling still decodes on every path that carries it.Not covered by unit tests: the sysfs enumeration in the sweep.