feat(vmm): run virtio-net on vhost-net with configurable queue pairs - #1145
Open
kvinwang wants to merge 5 commits into
Open
feat(vmm): run virtio-net on vhost-net with configurable queue pairs#1145kvinwang wants to merge 5 commits into
kvinwang wants to merge 5 commits into
Conversation
kvinwang
force-pushed
the
feat/vmm-vhost-multiqueue
branch
8 times, most recently
from
August 26, 2026 12:17
a350589 to
0d3ad45
Compare
Collaborator
|
Reviewed against 1. 2. Queue-echo mismatch leaks the prepared interface; upgrade order is undocumented. 3. Existing nodes change their QEMU command line on upgrade with no config change. 4. Context that may be useful in the docs:
Separately, |
Collaborator
|
Reference notes on how clouds pick a queue count — background for later tuning. dstack operators set 16 is a tuning convention, not a spec limit
RHEL's "up to 16" is host-thread budget (one How clouds encode it on the instance typeThey publish default per NIC, max per NIC, and often an instance-wide quota. The SKU fills the hypervisor knob. AWS ENA — per SKU three columns. General-purpose default plateaus at 8; network-optimized default goes to 16/32. Formula:
GCP — formula from machine type + NIC driver, override at create.
Aliyun ECS — closest to a SKU column. Bind-to-type applies the default; API exposes
OpenStack Nova —
Azure — SKU capability is Accelerated Networking (SR-IOV), not virtio queue count. Different dataplane. Practices that show up across those sourcesLined up with the numbers already in this PR (short connections 22k→6k conn/s from 1q to 8q; 64B UDP ~600k→3.0 Mpps). TDX makes the left-hand side worse than plain KVM: a cross-vCPU wakeup is an IPI plus a VM exit.
|
kvinwang
force-pushed
the
feat/vmm-vhost-multiqueue
branch
4 times, most recently
from
August 27, 2026 02:16
36bcb8d to
57f5328
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
kvinwang
force-pushed
the
feat/vmm-vhost-multiqueue
branch
3 times, most recently
from
August 27, 2026 04:13
4756d45 to
a9e20a2
Compare
Collaborator
Author
|
Thanks for the review — every point landed. Status, in your order: 1. 2. Prepared-interface leak on the queue-echo mismatch — fixed; both post-prepare exits now route through the same rollback as a failed prepare. 3. Upgrade path — resolved more conservatively than an upgrade note: the default is now 4. Runtime fallback footnote — verified on hardware (QEMU 8.2.2, the version dstack resolves on the test node, plus 10.2): an unopenable swiotlb footnote — tested adversarially (dstack-0.6.0 guest, 16 vCPU × 16 queue pairs, offloads off, ~177 Gbit/s plus concurrent direct block I/O): not a risk at any realistic shape. The guest kernel clamps the pool to a 64 MB floor regardless of RAM — the small-RAM tail where the concern would live doesn't exist — and demand is ring-bounded at ~2 MB per queue pair; a deliberately undersized 32 MB pool produced zero On your provider survey: with vhost now opt-in, no VM acquires the |
kvinwang
force-pushed
the
feat/vmm-vhost-multiqueue
branch
from
August 27, 2026 04:20
a9e20a2 to
1aa1d35
Compare
This was referenced Aug 28, 2026
* feat(netd): carry host port mappings and the VM workdir to netd `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. * feat(vmm): give port mappings a NIC, and carry them to netd 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. * fixup! feat(vmm): give port mappings a NIC, and carry them to netd * fix(vmm): ask netd what it can do instead of guessing from failures `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. * feat(netd): record who an interface belongs to, on the interface 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. * feat(vmm): collect host interfaces no VM claims 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. * feat(vmm): report which ports a VM actually publishes 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. * test(netd): put the ownership record next to the kernel, and document it 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. * fix(vmm): claim a VM's interfaces from its directory, not just from what 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. * fix(vmm): refuse an instance ID no interface could be recorded as belonging 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. * docs(vmm): say what cvm.instance_id now decides 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. * fix(cli): carry the new port mapping field through the client crates Additive on the wire and in the API: a client states a request, and `published` is the server's answer to it. * fix(netd): stop a busy netd from reading as an absent one 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. * fix(netd): stop the connection timeout from throwing away work it cannot 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. * docs(vmm): note that an orphaned nwfilter binding is always collected * fix(netd): make the binding listing work, and decide collections where 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. * fix(vmm): do not cache an answer that may have been a blip `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. * chore(netd): drop a test helper the collection reshape left unused * fix(vmm): decide whether a VM is running under the lock that keeps it 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. * fix(vmm): stop the removal lock from starving everything that waits on 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. * fix(vmm): refuse a stop or an update of a VM being removed, before waiting 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. * fix(vmm): put the CLI help back on the subcommand it describes `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. * fix(vmm): keep the removal mark where the removal can be seen `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. * fix(vmm): hold an update to one VM against the removal of that VM `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. * fix(vmm): stop a netd the VMM cannot open from reading as one that is 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. * fix(vmm/ui): carry the NIC a port mapping was pinned to `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. * docs: stop recommending the setuid helper this PR stopped using `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. * refactor(vmm): stop tracking whether the host publishes a port `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. * refactor(vmm): say that only QEMU publishes a host port, and stop pretending 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". * refactor(vmm): one notion of a failed sweep, not two and a bit `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. * refactor(vmm): retry a removal that could not release, instead of collecting 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. * docs(vmm): correct three comments the collection removal left behind * docs(netd): correct two comments the ingress removal left behind * docs(vmm): correct two more comments the ingress removal left behind --------- Co-authored-by: Kevin Wang <fremontkevin@icloud.com>
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.
Problem
A CVM's virtio-net data plane was limited by QEMU's single main-loop thread. At higher packet rates the host TAP dropped traffic before it reached the guest, so guest counters stayed clean while throughput hit one core's ceiling.
Multi-NIC port mappings also had no explicit NIC: QEMU attached every
hostfwd=entry to the first user-mode NIC, mappings could silently land on a management NIC, and mappings with no user-mode backend were silently stranded.Finally, bridge interfaces had two owners. Depending on filtering and queue count, either
qemu-bridge-helperor netd created the TAP. That made vhost capability, multiqueue preparation, teardown, and recovery depend on which path a node happened to take.Data plane
The node and each VM can select the vhost-net data plane, and each VM can select a queue-pair count:
Vhost is off in the shipped configuration for upgrade compatibility. With vhost enabled, queues default to the VM's vCPU count capped at 16 and
cvm.max_net_queues; without vhost they default to one. An explicit queue count is preserved with either vhost setting. The hard queue limit is 64.For
N > 1, QEMU receives matching backend and device settings:Macvtap opens
/dev/tapNonce per queue pair and passesfds=a:b:.... User networking has no vhost or multiqueue backend. Custom mode owns its complete netdev string and reports no inferred data-plane state.Neither vhost nor queue count is measured, so retuning a NIC does not change application identity.
One owner for host interfaces
Every bridge and macvtap NIC is now prepared by netd, including unfiltered single-queue bridge NICs.
qemu-bridge-helperis no longer used, and/etc/qemu/bridge.confno longer needs anallowentry.This is an operational compatibility change: netd must be upgraded and running before the VMM on every host that runs bridge or macvtap VMs. User and custom networking do not require it.
The VMM sends netd the required queue count and verifies that netd reports the same count. A mismatch, including an old netd that omits the field, rolls back preparation and fails the launch with a clear error.
Port mappings
PortMapping.nic_indexand the CLI@<nic>suffix select the NIC that receives a mapping:An unpinned mapping uses the first user-mode NIC, preserving existing behavior. QEMU user networking's
hostfwd=is the only publishing mechanism in this repository: bridge, macvtap, and custom NICs cannot carry a port mapping. Explicit invalid pins are refused; a legacy unpinned mapping with no user NIC is named in the launch log rather than silently disappearing.Interface lifecycle
Before every launch and during stop/removal, the VMM asks netd to sweep all interfaces for
(instance_id, vm_id). netd derives all possible NIC names, so cleanup does not depend on a runtime record that may be missing after a crash or stale after a topology change.Each interface records its owner in the kernel interface alias. This supports host inspection and manual recovery:
A stop remains successful if netd is unavailable. A removal that cannot release interfaces keeps the VM directory and
.removingmarker, remains inaccessible in the running VMM, and is retried during the next VMM startup. The marker must be persisted before asynchronous removal begins.API and UI
NetworkingConfig: optionalvhostandqueuesNetworkInterfaceStatus: effectivevhost,queues, and macvtap modeNetworkingCapabilities: nodedefault_vhostandmax_queuesPortMapping: optionalnic_indexRunning VMs report the data plane captured at launch. Stopped VMs report what the next launch would resolve from their manifest and current node configuration. A running VM started by an older VMM with no runtime snapshot is conservatively reported as vhost off with one queue.
Verification
cargo test -p dstack-vmm: 196 passed, 1 root/network-namespace integration test ignored by the normal suitecargo clippy -p dstack-vmm --all-targets -- -D warningsdocs/network-data-plane.mdfor the measurements and operational guidance.