diff --git a/docs/bridge-networking.md b/docs/bridge-networking.md index 0d16fd476..6bf3e6d5e 100644 --- a/docs/bridge-networking.md +++ b/docs/bridge-networking.md @@ -4,7 +4,7 @@ By default, dstack-vmm uses **user** networking (QEMU's built-in SLIRP stack, no ## When to use bridge networking -- High connection concurrency (passt becomes CPU-bound at ~25K+ concurrent connections) +- High connection concurrency (user-mode networking becomes CPU-bound at ~25K+ concurrent connections) - Workloads that need full L2 network access - Environments where VMs need to be directly reachable on the LAN @@ -21,11 +21,11 @@ bridge = "virbr0" ### Per-VM override Individual VMs can override the global networking mode via: -- **CLI**: `vmm-cli.py deploy --net bridge` or `--net passt` +- **CLI**: `vmm-cli.py deploy --net bridge`, `--net user`, or `--net macvtap` - **Web UI**: Networking dropdown in the deploy dialog - **API**: `networking: { mode: "bridge" }` in `VmConfiguration` -Only the mode is per-VM; the bridge interface name always comes from the global config. +The bridge interface name comes from the global config unless the node lists it in `cvm.allowed_bridges`. VMs may also override the vhost and queue settings — see [network-data-plane.md](network-data-plane.md). ## Host setup @@ -143,9 +143,15 @@ mode = "bridge" bridge = "dstack-br0" ``` -### QEMU bridge helper setup (required for both options) +### QEMU bridge helper setup (needed unless every bridge NIC goes through netd) The bridge helper allows QEMU to create and attach TAP devices without VMM needing root privileges. +It is used only on the single-queue bridge paths; a NIC that `netd` builds never touches it, so a +node that runs `netd` for all of its bridge VMs does not need it at all. + +The VMM probes `/usr/lib/qemu/qemu-bridge-helper`, `/usr/libexec/qemu-bridge-helper` and +`/usr/local/libexec/qemu-bridge-helper`. Set `cvm.qemu_bridge_helper` in `vmm.toml` for a path +outside that list. ```bash # Allow QEMU to use the bridge @@ -159,12 +165,13 @@ sudo chmod u+s /usr/lib/qemu/qemu-bridge-helper ## How it works -- VMM passes `-netdev bridge,id=net0,br=` to QEMU -- QEMU's bridge helper (setuid) creates a TAP device and attaches it to the bridge +- With more than one queue pair, or with libvirt filtering on, `netd` creates the TAP and the VMM passes `-netdev tap,id=net0,ifname=,...` — this is the usual case on a node running `netd` with multi-vCPU VMs, since queue pairs default to the VM's vCPU count. Without `netd`, a bridge NIC that took that default drops back to one queue pair and takes a helper path below +- Otherwise the VMM passes `-netdev tap,id=net0,br=,helper=,vhost=on`, or `-netdev bridge,id=net0,br=` when vhost is off or no helper is found +- QEMU's bridge helper (setuid) creates a TAP device and attaches it to the bridge on the two helper paths - Guest MAC address is derived from SHA256 of the VM ID, with an optional configurable prefix (stable across restarts for DHCP IP consistency) - The host DHCP server (dnsmasq) assigns an IP to the VM -- When QEMU exits, the TAP device is automatically destroyed -- VMM does not need root or `CAP_NET_ADMIN` +- On the two bridge-helper paths the TAP disappears when QEMU exits; a `netd`-created TAP is persistent and is deleted when the VMM tears the VM's networking down +- The VMM process itself needs neither root nor `CAP_NET_ADMIN` on any path; the `netd` path moves that privilege into a separate root service instead ### MAC address prefix @@ -194,13 +201,15 @@ The remaining bytes are derived from the VM ID hash. The prefix applies to all n ### Mixing networking modes -Bridge and passt VMs can coexist. Set the global default in `vmm.toml` and override per-VM as needed: +Bridge and user-mode VMs can coexist. Set the global default in `vmm.toml` and override per-VM as needed: ```bash -# Global default is bridge, but deploy this VM with passt -vmm-cli.py deploy --name my-vm --image dstack-0.5.6 --compose app.yaml --net passt +# Global default is bridge, but deploy this VM with user networking +vmm-cli.py deploy --name my-vm --image dstack-0.5.6 --compose app.yaml --net user ``` -### vhost-net and TDX +### vhost-net and multiqueue + +Bridge NICs can run on the host kernel's vhost-net data plane and expose several virtio-net queue pairs. Both are off by default and enabled per node or per VM — see [network-data-plane.md](network-data-plane.md) for the knobs, the enablement checklist, the mode support matrix, and how to pick a queue count. -vhost-net (kernel data plane offload for virtio-net) is **not enabled** for bridge mode. TDX encrypts guest memory, which prevents the host kernel from performing DMA-based packet offload. The default QEMU userspace virtio backend is used instead. +vhost-net works in a TDX guest: the virtio rings and buffers live in shared, unencrypted memory so that a host-side backend can reach them, which is the same mechanism `vhost-vsock-pci` has always relied on. diff --git a/docs/libvirt-network-filter.md b/docs/libvirt-network-filter.md index 942146f0f..6cc2724cf 100644 --- a/docs/libvirt-network-filter.md +++ b/docs/libvirt-network-filter.md @@ -14,8 +14,11 @@ host mechanism. The measurable acceptance criteria are: -- `network_filter = "none"` preserves the existing QEMU `-netdev bridge` - behavior and does not require `netd` or libvirt. +- `network_filter = "none"` installs no nwfilter binding. It still uses `netd` + for any NIC with more than one queue pair, and a `tap` netdev behind + `qemu-bridge-helper` whenever vhost is on; only a single-queue, non-vhost + bridge NIC keeps the historical `-netdev bridge` path with no `netd` or + libvirt dependency. - `network_filter = "libvirt"` creates the TAP and filter binding before QEMU is submitted to Supervisor, and uses QEMU `-netdev tap`. - A failed TAP or filter setup prevents QEMU from starting and rolls back all @@ -53,8 +56,11 @@ allowed_macvtap_parents = [] Macvtap is excluded from `allowed_network_modes` by default. Empty bridge and macvtap-parent allowlists prevent RPC callers from overriding the respective -node defaults. If macvtap is explicitly enabled, callers may select only a -parent in `allowed_macvtap_parents`; the macvtap forwarding mode always comes +node defaults. If macvtap is explicitly enabled, callers may select a +parent listed in `allowed_macvtap_parents`, the node's own configured parent, or +one this VM already holds — restating a value the node would have supplied +anyway grants nothing new. The same applies to `bridge_name` and +`allowed_bridges`. The macvtap forwarding mode always comes from `[cvm.networking].macvtap_mode` and cannot be selected through deployment RPCs. These allowlists authorize attachment targets; an nwfilter is not a substitute for that authorization. @@ -82,7 +88,8 @@ For libvirt mode, startup is: 2. Create the TAP for the configured QEMU UID and attach it to the bridge. 3. Create a libvirt nwfilter binding for the TAP. 4. Bring the TAP up and return success. -5. Start QEMU directly with `-netdev tap,script=no,downscript=no`. +5. Start QEMU directly with `-netdev tap,script=no,downscript=no`, carrying + `vhost=on|off` and, above one queue pair, `queues=N`. Teardown stops QEMU first, removes the binding, and deletes the TAP. Operations are serialized by `netd`. The design intentionally does not add ownership @@ -100,9 +107,9 @@ validated by libvirt. ## Deployment modes -Production should run one shared service. `netd` reads only the `[netd]` -section, so its root-owned configuration can be small and independent of every -VMM instance: +Production should run one shared service. `netd` reads the `[netd]` section, +plus `cvm.network_filter.mode` if the file has one, so its root-owned +configuration can be small and independent of every VMM instance: ```toml # /etc/dstack/netd.toml @@ -110,8 +117,33 @@ VMM instance: socket = "/run/dstack/netd.sock" socket_mode = 0o660 libvirt_uri = "qemu:///system" + +# Required here because this file has no [cvm] section for netd to read the +# node's policy from. +[netd.network_filter] +mode = "libvirt" +filter = "clean-traffic" +parameters = {} ``` +`[netd.network_filter]` is netd's own copy of the invariant, not a convenience. +netd is the privileged side of the socket, and anything that can reach the +socket can ask for an unfiltered TAP on a host bridge — a request a filtering +node has to refuse in the daemon rather than in its caller. When netd and the +VMM share one `vmm.toml`, leaving it unset derives it from +`[cvm.network_filter]` so the two cannot drift apart; a malformed section is a +startup error rather than a silent fallback to "filter nothing". + +The request says only *whether* to bind a filter, never which one. A caller that +named the filter could name `allow-arp`, which contains no drop rule at all, or +pin `clean-traffic` to the gateway's MAC and IP through its parameters, and +still satisfy a policy that asked for "some filter". + +A macvtap parent is refused when filtering is required and the parent is a host +bridge or is enslaved to one: nwfilter does not apply to macvtap, so that +request is the same unfiltered access to the same segment, spelled with a +different operation. + Production deployments can use systemd socket activation. The socket unit owns the filesystem mode and ownership; `netd.socket_mode` applies only to the standalone bind path. @@ -162,10 +194,37 @@ sudo dstack-vmm --config ./vmm.toml \ --netd-socket /run/dstack-dev/netd.sock ``` -User networking and bridge networking with `mode = "none"` never connect to -`netd`. Libvirt mode fails closed if `netd` is unavailable. - -Filtered TAP netdevs currently set `vhost=off`. This keeps the initial backend -on the directly bound TAP path and avoids adding `/dev/vhost-net` permissions -to the QEMU user. It is a deliberate security-first throughput tradeoff; a -future configurable vhost mode requires equivalent filter integration tests. +User networking never asks `netd` to build an interface; the VMM still opens a +short liveness-probe connection to the netd socket on every launch and when +describing a stopped VM. Libvirt mode fails closed if `netd` +is unavailable. Bridge networking with `mode = "none"` connects only when it +needs more than one queue pair, as described below. + +Filtered TAP netdevs follow the node's `vhost` and `queues` settings like any +other TAP-backed NIC (see [network-data-plane.md](network-data-plane.md)). The +nwfilter binding is installed on the host TAP interface, so packets traverse it +whether they were written by QEMU or by a vhost worker; filtering is unaffected +by the data plane choice. Enabling vhost does require the QEMU user to be able +to open `/dev/vhost-net`. + +`netd` also creates the TAP for unfiltered bridge NICs that ask for more than +one queue pair, because `qemu-bridge-helper` returns a single descriptor and +cannot create a `multi_queue` device. Those TAPs carry no nwfilter binding, so +a multiqueue bridge node needs `netd` even when `network_filter.mode = "none"`. + +An empty filter name is what selects that unfiltered TAP, so `mode = "libvirt"` +with an empty `filter` is rejected at config load rather than quietly producing +an unbound TAP. + +Removal carries the same distinction: the VMM tells `netd` whether the interface +it is asking about was created with a binding, from a record made when it was +built rather than from configuration that may have changed since. A binding it +was told about must be gone before `netd` returns; otherwise `netd` still asks +libvirt to clear one — an interface name is reused by the same VM, and a +leftover binding's rules would be inherited — but a `libvirtd` it cannot reach +is a warning rather than a failure. So a node with `virsh` installed and no +running `libvirtd` can create and destroy multiqueue TAPs. The flag defaults to +true on the wire, so an older VMM's removals still drop their bindings. + +`netd` requires the `virsh` binary to be present whatever the filter mode; it is +`libvirtd` that unfiltered work does not need. diff --git a/docs/macvtap-networking.md b/docs/macvtap-networking.md index 544408d31..362de4e87 100644 --- a/docs/macvtap-networking.md +++ b/docs/macvtap-networking.md @@ -19,6 +19,9 @@ Configure a NIC through node configuration or an authorized VMM RPC request: `parent` must name an existing host interface. `macvtap_mode` may be `private`, `bridge`, `vepa`, or `passthru`; an empty value selects `private`. +Macvtap NICs also honour the `vhost` and `queues` settings described in +[network-data-plane.md](network-data-plane.md); netd creates the interface with +matching hardware queues and the launcher opens `/dev/tapN` once per queue. The configured netd socket permissions apply in the same way as for libvirt-filtered bridge networking. @@ -49,8 +52,9 @@ and the same deterministic MAC address passed to QEMU. Netd then: 4. reads its kernel-assigned ifindex and waits for `/dev/tap`; and 5. returns that runtime device path to the VMM. -The per-VM launcher opens the character device, places it at the fd referenced -by QEMU's `-netdev tap,fd=...` argument, and then execs QEMU. This keeps device +The per-VM launcher opens the character device once per queue pair, places the +descriptors at the fds referenced by QEMU's `-netdev tap,fd=...` (or `fds=...`) +argument, and then execs QEMU. This keeps device paths out of persistent VM configuration, works with both Supervisor and systemd process managers, and does not pass network fds through `sudo`. diff --git a/docs/network-data-plane.md b/docs/network-data-plane.md new file mode 100644 index 000000000..57f0134a1 --- /dev/null +++ b/docs/network-data-plane.md @@ -0,0 +1,306 @@ +# virtio-net data plane tuning + +Every CVM NIC has two knobs that decide how many packets it can move: whether +the host kernel's vhost-net data plane is used, and how many virtio-net queue +pairs the device exposes. vhost is set per node and overridable per VM; queue +pairs have no node-wide setting at all, for the reason given under +Configuration. + +## Why it matters + +Without vhost-net, QEMU drains every received packet on its single main-loop +thread. That thread is the ceiling, and it does not grow with vCPUs: + +``` +maximum packets per second ≈ 1 core ÷ per-packet main-loop cost +``` + +The per-packet cost varies with traffic shape — a few microseconds for uniform +synthetic streams, tens of microseconds for bidirectional short-connection +traffic — so the ceiling is a property of the workload, not a fixed number. +What is fixed is the shape of the failure: throughput climbs normally until the +main loop saturates at 100% of one core, then packets are dropped at the TAP +before they ever reach the guest. Guest-side counters stay clean, which makes +the cliff easy to misdiagnose as a network problem. + +Guest-side outbound traffic uses the same thread, so a busy guest pays the +cost twice over. + +`vhost=on` moves that work into the host kernel. That returns a whole core, but +it relocates the ceiling rather than removing +it: packets now arrive faster than a single guest receive queue can drain, and +the drops reappear at a higher rate. More queue pairs is what removes them, +which is why enabling vhost also enables a queue count that follows the VM's +vCPU count — the two travel together. + +vhost is **off by default** and enabled per node (or per VM). Two things make +it an opt-in rather than a default: turning it on changes the virtio-net device +of every bridge/macvtap VM on its next boot, and it requires `/dev/vhost-net` +to be accessible to the account QEMU runs under, which the VMM cannot verify on +the operator's behalf — see [Enabling vhost on a node](#enabling-vhost-on-a-node). + +## Configuration + +```toml +[cvm] +# Ceiling for both the default and what a deployment may request. +max_net_queues = 16 + +[cvm.networking] +mode = "bridge" +bridge = "dstack-br0" +vhost = true +``` + +Queue pairs are not a node setting. With vhost on they default to the VM's vCPU +count, capped at 16, because the useful number follows the VM rather than the +host — the guest driver uses at most one queue pair per vCPU. A deployment +overrides that per VM, up to `max_net_queues`. + +Raising `max_net_queues` above 16 widens what a deployment may ask for without +moving the default's cap, so a larger VM never silently acquires a worse +default. Lowering it below 16 does lower the default too, because a node that +refuses a request for four queue pairs should not hand out sixteen by itself. +The hard ceiling from any source is 64. + +Without vhost the default is a single queue pair. The QEMU main loop drains +every queue on one thread, so extra queues buy little while still costing a +netd interface, more MSI-X vectors, and a changed guest device. An explicit +queue count is still honoured without vhost, since that combination is a +deliberate request rather than a default. The two defaults travelling together +also means a node that never sets `vhost` keeps building the device its VMs +have always had. + +A VM overrides either value at deploy time, and `UpdateVm` changes them +afterwards — the new values apply from the VM's next boot: + +```bash +vmm-cli.py deploy --name my-vm --image dstack-0.5.9 --compose app.yaml \ + --net bridge --net-queues 4 +vmm-cli.py deploy --name latency-vm --image dstack-0.5.9 --compose app.yaml \ + --net bridge --net-no-vhost + +# retune an existing VM +vmm-cli.py update --net-queues 8 --net-vhost + +# stop pinning either value and follow the node and the vCPU count again +vmm-cli.py update --net-queues auto +vmm-cli.py update --net-vhost-default + +# stop pinning the backend too, and follow whatever the node runs +vmm-cli.py update --net default +``` + +Every pin has an un-pin. A VM keeps reporting whatever it pinned, and the +deployment RPC accepts a VM's own held values back even after the node's +policy moves, so a read-modify-write update never strands a VM. + +The web UI exposes both per NIC in the deploy and update dialogs, alongside the +networking mode. Both fields are also on `NetworkingConfig` in the deployment +and update RPCs. A request that +sets only `vhost`/`queues` keeps the node's own networking mode, so tuning does +not force a caller to restate — or be allowed to choose — a backend. `queues` is +rejected above the node's `max_net_queues`; `vhost` is not otherwise restricted, +since it only affects the requesting VM. `GetMeta` reports +`networking.max_queues` so a client can present the real bound. + +The data plane settings are recorded only when a deployment asks for them. +Leave one out and it stays owned by the node, so changing `[cvm.networking]` +later — including setting `vhost = false` to roll the whole node back — still +reaches VMs deployed with some other networking override. + +Naming a backend is different: it pins that NIC's *backend*, resolved at +deployment. Its mode, and the bridge or macvtap parent that names it, are fixed +for the life of the VM, so a later edit to those fields in `[cvm.networking]` +does not move it to another segment. Nothing else is pinned — the MAC prefix, +the user-mode subnet and DHCP start, and the macvtap forwarding mode stay node +settings and are re-read at every launch, so editing them changes every VM's +next boot, including its MAC and therefore its DHCP lease. A request that only +tunes pins nothing at all, including the backend it inherited. + +`GetInfo` reports that configuration back, and both `vmm-cli.py update` and the +web UI read it, change one field, and resend the rest. Two things follow. A +request may name a bridge or macvtap parent the node itself configured even when +the allowlists are empty: leaving the field out already yields exactly that +value, so echoing it grants nothing policy was withholding. And an update may +restate whatever its own VM already pinned, so that moving the node's default +out from under a VM does not leave that VM's configuration unsendable. A NIC +that inherited its backend reports an empty mode, which is the same thing it was +deployed with. + +Neither field reaches the CVM's measurement. The measured VM configuration the +VMM controls covers the OS image, the vCPU and memory counts, several QEMU +layout flags, the *number* of NICs, and `mr_config_id`; the queue count and the +vhost state are not part of it, so retuning a NIC does not change app identity +or require an on-chain update. Adding or removing a NIC does: the NIC count +changes the guest's ACPI tables and therefore RTMR0. + +## Enabling vhost on a node + +Setting `vhost = true` in `[cvm.networking]` is a node-wide behaviour change: +every bridge or macvtap VM that has not pinned its own data plane gets a +different virtio-net device on its next boot — `vhost=on`, `mq=on` with +vCPU-scaled queue pairs, and the matching MSI-X vector count. The device is not +measured, so attestation and app identity are unaffected. Before flipping it: + +1. **Verify `/dev/vhost-net` is accessible to the account QEMU runs under.** + It is `root:kvm 0660` on Debian-family hosts, where adding the account to + the `kvm` group suffices, and `root:root 0600` on several others. If the + account lacks access, QEMU exits at launch and every affected VM stops + restarting. The VMM warns at startup when its own access fails, but it + cannot refuse on that basis — QEMU need not share its credentials. + +2. **Restart `netd` before or together with the VMM.** Multiqueue bridge NICs + are prepared by `netd`, and the VMM checks that `netd` echoes the queue + count it built. An older `netd` fails that check; the launch is rolled back + and fails with the reason in the VMM log, but the VM does not start until + `netd` is upgraded. + +3. **Roll back by setting `vhost = false`.** The node value reaches every VM + that did not pin `vhost` explicitly, from its next boot; a VM that pinned + `vhost = true` keeps it until updated. + +## What each mode supports + +| Mode | netdev | vhost | queues > 1 | +|---|---|---|---| +| `user` | `user,...` | no backend | not supported | +| `bridge` | `tap,ifname=` via netd, else `tap,br=,helper=`, else `bridge,br=` | yes | yes, through netd | +| `bridge` with libvirt filtering | `tap,ifname=` | yes | yes, through netd | +| `macvtap` | `tap,fd=` / `tap,fds=` | yes | yes | +| `custom` | operator's own string | operator's own string | no, not settable | + +QEMU's `bridge` netdev accepts neither `vhost=` nor `queues=`, so enabling +vhost switches bridge mode to a `tap` netdev driven by the same setuid +`qemu-bridge-helper`. The VMM still needs no network privileges. The helper has +no compiled-in default path for the `tap` netdev, so the VMM probes the known +distribution locations; set `cvm.qemu_bridge_helper` if yours is elsewhere. If +no helper is found the NIC falls back to the non-vhost `bridge` netdev with a +warning, because a node-wide setting must not stop a node from booting VMs +that never asked for it. + +The helper returns exactly one descriptor, which is why more than one queue +pair in bridge mode is created by `netd` instead: it adds a persistent +`multi_queue` TAP that QEMU then opens once per queue. `netd` requires the +`virsh` binary to be installed even when nothing is filtered, though it does +not require a reachable `libvirtd`. That applies whether or +not libvirt filtering is on, so a bridge node needs `netd` to get the default +queue count (see [libvirt-network-filter.md](libvirt-network-filter.md)). +Without it, bridge NICs fall back to a single queue pair with a warning rather +than failing to launch; a VM that asked for a queue count explicitly still +fails, so the caller learns their request was not met. `netd` is probed by +connecting, not by looking for its socket file, because a `netd` that died +leaves the socket behind. One-shot `dstack-vmm run` has no netd lifecycle at +all and behaves like a node without it. `netd` reports back the +queue count it created, and the VMM refuses to launch on a mismatch — a `netd` +deployed separately as a root service can be older than the VMM asking it for +multiqueue, and QEMU would otherwise reject the interface from inside the +per-VM launcher. + +For macvtap, the per-VM launcher opens the `/dev/tapN` character device once +per queue pair and hands QEMU the descriptors as `fds=`. `netd` creates the +interface with matching `numtxqueues`/`numrxqueues`. + +Custom mode owns its whole netdev string, including any `vhost=`/`queues=` +options, and its guest device stays single-queue: the VMM cannot edit that +string, so it has no way to make a multiqueue device line agree with it. For the +same reason `GetInfo` reports no vhost state and no queue count for a custom +NIC, rather than asserting the resolved defaults over a string it never read. A +hand-written multiqueue netdev will not pair with a multiqueue guest device +today. + +Naming a backend that cannot carry vhost or a queue count, and then asking for +one, is refused — the request is yours to correct. Inheriting such a backend is +not, because the node chose it and may choose another tomorrow; the request +reads as off, or as one queue pair, until then. + +## Choosing a queue count + +The default suits bandwidth-bound workloads. Latency-sensitive ones should ask +for fewer: more queues spread receive processing over more vCPUs, and under TDX +a cross-vCPU wakeup costs an IPI and a VM exit. Measured on one 8-vCPU TDX CVM, +changing only the guest's channel count: + +| Queue pairs | Short-connection throughput | +|---|---| +| 1 | 22.3k conn/s | +| 2 | ~20k conn/s | +| 4 | 15–21k conn/s | +| 8 | 6.2–7.7k conn/s | + +The same CVM with 8 queues moved 3.0 Mpps of 64-byte UDP with no loss, against +roughly 600k with one queue. The trade is real in both directions, so a VM +serving many short connections should set `--net-queues 1` and measure. + +A VM with fewer vCPUs than queues leaves the extra pairs idle — `ethtool -l +eth0` reports the smaller number. An explicit over-provision is not rejected at +deployment, because `vmm-cli.py resize` can raise the vCPU count later. + +Queue pairs also cost guest memory — each RX ring keeps 256 page-sized buffers +posted, about 1 MB per queue pair plus per-queue NAPI and socket state — but at +any realistic RAM/vCPU shape this is noise. Pushed to a shape no deployment +uses (1 GB of RAM with 16 vCPUs, so 16 queue pairs by default), sustained load +did produce atomic order-0 page-allocation failures in RX refill +(`try_fill_recv` in the guest log); 2 GB at the same shape ran clean. Since the +queue default follows the vCPU count and a VM with that many vCPUs carries far +more memory in practice, this needs no tuning — it is recorded here so the +symptom is searchable. The TDX bounce-buffer pool is not a constraint either: +the guest kernel sizes swiotlb at 6% of RAM clamped to [64 MB, 1 GB] with no +`swiotlb=` parameter, while peak demand is bounded by ring size at about 2 MB +per queue pair — a deliberately undersized 32 MB pool sustained full +multiqueue line rate with zero `swiotlb buffer is full` events. + +`vectors` is derived, never configured: `2N + 2`, one vector per queue +direction plus config and control. One queue pair emits no `mq=on` or +`vectors=` at all, leaving the guest device line byte for byte identical to the +one before this feature. The `-netdev` half does change wherever vhost is on, +since that is what selects the backend. + +## Requirements + +The account running QEMU must be able to open `/dev/vhost-net`, which is +`root:kvm 0660` on a stock host — add that account to the `kvm` group. The +`vhost_net` module autoloads on first open. + +`GetInfo` reports the data plane each interface actually got, so a bridge NIC +that fell back for want of a helper reads as `vhost: false` rather than +advertising something it is not using. For a VM that is not running there is no +interface to describe, so it reports what the next launch would build instead -- +the same calculation, against the node configuration and manifest as they stand +now, rather than the ones a finished boot ran under. + +If that account lacks access, QEMU exits at startup and the VM never boots — +there is no fallback to the userspace backend at this point, on any QEMU +version (verified on 8.2.2 and 10.2). What the per-VM launcher log shows +depends on the version: QEMU 8.2 prints `warning: tap: open vhost char device +failed: Permission denied` (once per queue) and then dies on `net/net.c:1185: +net_client_init1: Assertion 'nc' failed` — an upstream bug +([qemu#1486](https://gitlab.com/qemu-project/qemu/-/issues/1486)); later +versions exit cleanly with `Could not open '/dev/vhost-net'`. Grep for either. +The VMM does not refuse a launch over this: QEMU need not share the VMM's +credentials, so a refusal based on the VMM's own access would block deployments +the host can run. It warns instead — when the device node is missing outright, +and when the VMM's own open is denied, since QEMU usually does share its +account. + +QEMU does have a *runtime* fallback, at a different failure point: once the +netdev initialized with vhost, a later `vhost_net_start()` failure at guest +driver activation logs `unable to start vhost net: : falling back on +userspace virtio` and keeps the NIC working on the userspace data path. That +path is reachable only after `/dev/vhost-net` was opened successfully at +launch, so an access problem never lands there. If it does fire, it is the one +case where `GetInfo` can overstate the data plane — the interface reports the +vhost state the launch settled while the packets take the userspace path — and +that QEMU log line is the indicator. + +vhost-net works normally in a TDX guest: the virtio rings and buffers live in +shared, unencrypted memory precisely so a host-side backend can reach them. +This is the same mechanism behind `vhost-vsock-pci`, which dstack has always +used. + +On host kernels older than 6.4 the vhost worker is a free-standing kernel +thread: it is attached to the owner's cgroups, so `cpu.max` and cgroup +accounting do apply, but it is outside QEMU's thread group and so invisible to +`top -H` and to anything reading `/proc//task`. Since 6.4 it is a +`vhost_task` inside that thread group and shows up everywhere the VM's other +threads do. diff --git a/dstack/vmm/rpc/proto/vmm_rpc.proto b/dstack/vmm/rpc/proto/vmm_rpc.proto index c035e64d0..31fef51f4 100644 --- a/dstack/vmm/rpc/proto/vmm_rpc.proto +++ b/dstack/vmm/rpc/proto/vmm_rpc.proto @@ -39,19 +39,34 @@ message VmInfo { repeated GuestEvent events = 14; // Effective network interfaces resolved against node config. repeated NetworkInterfaceStatus interfaces = 15; + // Whether a QEMU process exists for this VM right now. The interfaces above + // are what it built only while this is true; otherwise they are what the next + // launch would build. + bool running = 16; } // Runtime status of a resolved VM network interface. message NetworkInterfaceStatus { - // Product-facing mode: "user", "bridge", or "custom". + // Product-facing mode: "user", "bridge", "macvtap", or "custom". string mode = 1; - // QEMU/network backend shape: "slirp", "tap_bridge", or "custom". + // QEMU/network backend shape: "slirp", "tap_bridge", "macvtap", or "custom". string backend = 2; string mac = 3; // Linux bridge name for tap_bridge backend. optional string bridge_name = 4; // QEMU netdev id, e.g. "net0". optional string netdev_id = 5; + // Effective vhost-net data plane state for this interface. Absent for custom + // mode, where the operator supplies the whole netdev string and the VMM does + // not parse it, so it knows of no data-plane state to report. + optional bool vhost = 6; + // Effective virtio-net queue pairs. Absent for custom mode, for the same + // reason as vhost. + optional uint32 queues = 7; + // Effective macvtap forwarding mode, for macvtap interfaces only. Node + // configuration decides it, so it is reported here with the rest of the + // resolved state rather than on the VM's own NetworkingConfig. + optional string macvtap_mode = 8; } // Structured log or lifecycle event emitted by the guest or runtime. @@ -132,9 +147,16 @@ message NetworkingConfig { string bridge_name = 2; // Parent host interface for macvtap mode. string parent = 3; - // Effective macvtap forwarding mode in responses. Deployment requests must - // leave this empty because the mode is controlled by node configuration. + // Deployment requests must leave this empty: the forwarding mode is node + // configuration, and a VM never pins one. The effective value is reported on + // NetworkInterfaceStatus.macvtap_mode instead. string macvtap_mode = 4; + // Move packet processing into the host kernel vhost-net data plane. Unset + // inherits the node default. User mode has no vhost backend and ignores it. + optional bool vhost = 5; + // virtio-net queue pairs. Unset inherits the node default. Bounded by the + // node's cvm.max_net_queues. + optional uint32 queues = 6; } // Requested GPU layout for a CVM. @@ -301,6 +323,13 @@ message NetworkingCapabilities { reserved "forward_service_enabled"; // Default bridge configured in vmm.toml [cvm.networking].bridge. string default_bridge = 4; + // Largest virtio-net queue pair count a deployment request may ask for. + uint32 max_queues = 5; + // Whether the node's own backend runs the vhost-net data plane. A NIC that + // pins neither vhost nor a queue count follows this, and without vhost the + // queue count does not scale with vCPUs -- so a client cannot describe what + // an empty queue field will do without it. + bool default_vhost = 6; } // Aggregated metadata exposed through GetMeta. diff --git a/dstack/vmm/src/app.rs b/dstack/vmm/src/app.rs index 836176d4b..e9205b07d 100644 --- a/dstack/vmm/src/app.rs +++ b/dstack/vmm/src/app.rs @@ -3,7 +3,10 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - config::{Config, NetworkFilterMode, Networking, NetworkingMode, ProcessAnnotation, Protocol}, + config::{ + Config, NetdInterface, Networking, NetworkingMode, NicNetworking, ProcessAnnotation, + Protocol, + }, logrotate, netd::{ self, InterfaceIdentity, PrepareBridgeRequest, PrepareMacvtapRequest, @@ -32,6 +35,7 @@ use rand::seq::SliceRandom; use serde::{Deserialize, Serialize}; use serde_json::json; use sha2::{Digest, Sha256}; +use std::cell::OnceCell; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet, VecDeque}; use std::net::IpAddr; use std::path::{Path, PathBuf}; @@ -42,9 +46,15 @@ use tracing::{debug, error, info, warn}; pub use image::{Image, ImageInfo}; pub(crate) use network::{ - resolve_networking, resolved_networks, validate_resolved_network, validate_resolved_networks, + clamp_queues_without_netd, filters_bridge_traffic, needs_netd_interface, netd_available, + netd_teardown, resolve_networking, resolved_networks, settle_vhost, validate_resolved_network, + validate_resolved_networks, }; pub use qemu::VmConfig; +// Exported so the RPC layer can assert that everything it reports is +// something it also accepts. +#[cfg(test)] +pub(crate) use vm_info::networking_to_proto; pub use workdir::VmWorkDir; mod host_share; @@ -126,7 +136,7 @@ pub struct Manifest { #[serde(default)] pub swtpm: bool, #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub networks: Vec, + pub networks: Vec, #[serde(default)] pub volumes: Vec, } @@ -355,7 +365,7 @@ impl App { let vm_id = manifest.id.clone(); let mut runtime_networks = vm_work_dir.runtime_networks(); if runtime_networks.is_empty() && cids_assigned.contains_key(&vm_id) { - runtime_networks = resolved_networks(&manifest, &self.config.cvm); + runtime_networks = self.inferred_runtime_networks(&manifest); if let Err(err) = vm_work_dir.set_runtime_networks(&runtime_networks) { warn!(id = %vm_id, "failed to persist inferred runtime networks: {err}"); } @@ -454,7 +464,7 @@ impl App { append_boot_separator(&path); } - let mut runtime_networks = resolved_networks(&vm_config.manifest, &self.config.cvm); + let mut runtime_networks = self.runtime_networks(&vm_config.manifest); let devices = self.try_allocate_gpus(&vm_config.manifest)?; let gpu_host_config = self.config.cvm.gpu.clone(); let devices_to_sanitize = devices.clone(); @@ -547,19 +557,16 @@ impl App { vm: &VmConfig, networks: &mut [Networking], ) -> Result<()> { - if self.config.cvm.network_filter.mode == NetworkFilterMode::None - && !networks - .iter() - .any(|network| network.mode == NetworkingMode::Macvtap) + if !networks + .iter() + .any(|network| needs_netd_interface(network, &self.config.cvm)) { return Ok(()); } let qemu_uid = Uid::effective().as_raw(); let mut prepared = Vec::new(); for (nic_index, network) in networks.iter_mut().enumerate() { - if network.mode == NetworkingMode::Bridge - && self.config.cvm.network_filter.mode == NetworkFilterMode::None - { + if !needs_netd_interface(network, &self.config.cvm) { continue; } let identity = InterfaceIdentity { @@ -572,21 +579,28 @@ impl App { &network.mac_prefix_bytes(), nic_index, ); - let request = match network.mode { + let queues = network.queue_pairs(); + let filtered = filters_bridge_traffic(network, &self.config.cvm); + let request = match network.nic.mode { NetworkingMode::Bridge => NetdRequest::PrepareBridge(PrepareBridgeRequest { identity: identity.clone(), - bridge: network.bridge.clone(), + bridge: network.nic.bridge.clone(), mac, qemu_uid, - filter: self.config.cvm.network_filter.filter.clone(), - parameters: self.config.cvm.network_filter.parameters.clone(), + // Which filter, and with what parameters, is netd's to + // decide from its own configuration. An unfiltered TAP is + // only asked for by multiqueue, where the node may not run + // libvirt at all. + filtered, + queues, }), NetworkingMode::Macvtap => NetdRequest::PrepareMacvtap(PrepareMacvtapRequest { identity: identity.clone(), - parent: network.parent.clone(), + parent: network.nic.parent.clone(), mac, qemu_uid, mode: network.macvtap_mode.clone(), + queues, }), NetworkingMode::User | NetworkingMode::Custom => continue, }; @@ -600,73 +614,224 @@ impl App { &self.config.netd.socket, &NetdRequest::Remove { identity: identity.clone(), + filtered, }, ) .await { warn!(%cleanup_error, "failed to roll back in-flight filtered network"); } - for identity in prepared.into_iter().rev() { - if let Err(cleanup_error) = netd::request( - &self.config.netd.socket, - &NetdRequest::Remove { identity }, - ) - .await - { - warn!(%cleanup_error, "failed to roll back prepared filtered network"); - } - } - return Err(error).context("failed to prepare libvirt-filtered networking"); + self.roll_back_prepared_networks(prepared).await; + // netd's own message is about a TAP, not about queues, so + // a caller who asked for multiqueue would not see their + // request named anywhere in the failure. + let unreachable = netd::is_unreachable(&error); + let error = Err(error).context("failed to prepare netd-managed networking"); + return if queues > 1 && unreachable { + error.with_context(|| { + format!( + "interface {nic_index} asked for {queues} queue pairs, which needs \ + a netd running on this host" + ) + }) + } else if queues > 1 { + error.with_context(|| { + format!("interface {nic_index} asked for {queues} queue pairs") + }) + } else { + error + }; } }; - if network.mode == NetworkingMode::Macvtap { - network.device = response - .device - .context("netd response omitted macvtap device")?; + prepared.push((identity.clone(), filtered)); + // netd built this one. Record it now, before anything else can + // fail, so teardown never has to re-derive it from a node + // configuration the operator may since have changed. + network.netd_interface = if filtered { + NetdInterface::Filtered + } else { + NetdInterface::Unfiltered + }; + // Everything below runs after netd already built a host interface, + // so a failure has to unwind the same way a failed Prepare does. + let accepted = (|| { + if network.nic.mode == NetworkingMode::Macvtap { + network.device = response + .device + .clone() + .context("netd response omitted macvtap device")?; + } + // QEMU refuses a TAP whose IFF_MULTI_QUEUE state disagrees with + // its own `queues=`, and reports it from inside the per-VM + // launcher. netd echoes what it built, so a netd too old to + // understand the request fails here, where the reason is + // legible. + if queues > 1 && response.queues != Some(queues) { + bail!( + "netd prepared interface {nic_index} with {} queue pairs instead of \ + {queues}; its version may predate multiqueue support", + response.queues.map_or_else( + || "an unreported number of".to_string(), + |q| q.to_string() + ) + ); + } + Ok(()) + })(); + if let Err(error) = accepted { + self.roll_back_prepared_networks(prepared).await; + return Err(error); } - prepared.push(identity); } Ok(()) } + /// The NICs a VM has now, or would get if it were started. + /// + /// While QEMU is up this is what the launch actually built. Once it is + /// down the snapshot describes a boot that is over: the node configuration + /// and the VM's own manifest can both have changed since, so reporting it + /// would answer a question about the past with the grammar of the present. + /// Predict instead, the same way the next launch will -- including the + /// drop to a single queue pair on a node with no netd. + /// + /// `netd_reachable` is shared across a request rather than probed here: + /// the probe is a blocking connect that netd's serialized accept loop has + /// to service, and one status query covers many VMs. + fn effective_networks( + &self, + info: &vm_info::VmInfo, + netd_reachable: &OnceCell, + ) -> Vec { + if info.running && !info.runtime_networks.is_empty() { + return info.runtime_networks.clone(); + } + let available = *netd_reachable.get_or_init(|| netd_available(&self.config.netd.socket)); + self.merge_networks(&info.manifest, available).0 + } + + /// Launch-time view of a VM's NICs: node defaults merged in, the + /// vCPU-scaled queue count made concrete, and multiqueue dropped when this + /// node has no netd to build the interface. + pub(crate) fn runtime_networks(&self, manifest: &Manifest) -> Vec { + let available = netd_available(&self.config.netd.socket); + let (networks, clamped, vhost_denied) = self.merge_networks(manifest, available); + if clamped > 0 { + warn!( + id = %manifest.id, + "netd is not available, so {clamped} bridge interface(s) fall back to a single \ + queue pair; run dstack-vmm netd to let queue pairs scale with vCPUs" + ); + } + if vhost_denied > 0 { + warn!( + id = %manifest.id, + "no qemu-bridge-helper found, so {vhost_denied} bridge interface(s) fall back to \ + the non-vhost bridge netdev; set cvm.qemu_bridge_helper to enable vhost" + ); + } + networks + } + + /// A running VM whose snapshot is missing, because a VMM that predates the + /// snapshot -- or predates it recording what netd built -- started it. + /// + /// Guessing is all that is left, so guess the way that VMM would have, and + /// then write the guess down. Leaving the marker unset would make every + /// later teardown re-derive it from node configuration that may by then + /// have moved, which is the failure this snapshot exists to prevent. + /// + /// The way *that* VMM would have, not this one: a build old enough to leave + /// no snapshot had no vhost and no multiqueue at all, so whatever this + /// node's defaults say now, the QEMU process actually running was given one + /// queue pair and no vhost. Asking `runtime_networks` would apply today's + /// defaults to a launch that predates them, and the guess is persisted, so + /// it would keep describing that VM wrongly for the life of its boot. + /// + /// `merge_networks` rather than `runtime_networks` for the same reason: the + /// latter probes netd and warns about a multiqueue fallback, which says + /// nothing about a VM that is already up. + fn inferred_runtime_networks(&self, manifest: &Manifest) -> Vec { + let mut networks = self.merge_networks(manifest, false).0; + for network in &mut networks { + network.nic.vhost = Some(false); + network.nic.queues = Some(1); + } + for network in &mut networks { + network.netd_interface = match netd_teardown(network, &self.config.cvm) { + Some(true) => NetdInterface::Filtered, + Some(false) => NetdInterface::Unfiltered, + None => NetdInterface::None, + }; + } + networks + } + + /// The merge itself, without the launch-time logging, plus how many NICs + /// lost multiqueue for want of netd. + fn merge_networks( + &self, + manifest: &Manifest, + netd_reachable: bool, + ) -> (Vec, usize, usize) { + let requested = if manifest.networks.is_empty() { + vec![self.config.cvm.networking.nic.clone()] + } else { + manifest.networks.clone() + }; + let mut resolved = resolved_networks(manifest, &self.config.cvm); + let clamped = + clamp_queues_without_netd(&requested, &mut resolved, &self.config.cvm, netd_reachable); + let vhost_denied = settle_vhost(&mut resolved, &self.config.cvm); + (resolved, clamped, vhost_denied) + } + + /// Removes interfaces netd already built for a launch that then failed. + async fn roll_back_prepared_networks(&self, prepared: Vec<(InterfaceIdentity, bool)>) { + for (identity, filtered) in prepared.into_iter().rev() { + if let Err(cleanup_error) = netd::request( + &self.config.netd.socket, + &NetdRequest::Remove { identity, filtered }, + ) + .await + { + warn!(%cleanup_error, "failed to roll back prepared network interface"); + } + } + } + pub(crate) async fn remove_filtered_networks( &self, vm_id: &str, networks: &[Networking], ) -> Result<()> { - if self.config.cvm.network_filter.mode == NetworkFilterMode::None - && !networks - .iter() - .any(|network| network.mode == NetworkingMode::Macvtap) + if networks + .iter() + .all(|network| netd_teardown(network, &self.config.cvm).is_none()) { return Ok(()); } let mut first_error = None; for (nic_index, network) in networks.iter().enumerate().rev() { - if network.mode == NetworkingMode::Bridge - && self.config.cvm.network_filter.mode == NetworkFilterMode::None - { - continue; - } - if !matches!( - network.mode, - NetworkingMode::Bridge | NetworkingMode::Macvtap - ) { + let Some(filtered) = netd_teardown(network, &self.config.cvm) else { continue; - } + }; let identity = InterfaceIdentity { instance_id: self.config.cvm.instance_id.clone(), vm_id: vm_id.to_string(), nic_index, }; - if let Err(error) = - netd::request(&self.config.netd.socket, &NetdRequest::Remove { identity }).await + if let Err(error) = netd::request( + &self.config.netd.socket, + &NetdRequest::Remove { identity, filtered }, + ) + .await { first_error.get_or_insert(error); } } if let Some(error) = first_error { - return Err(error).context("failed to remove libvirt-filtered networking"); + return Err(error).context("failed to remove netd-managed networking"); } Ok(()) } @@ -1059,7 +1224,7 @@ impl App { let already_running = cids_assigned.contains_key(&vm_id); let mut runtime_networks = vm_work_dir.runtime_networks(); if runtime_networks.is_empty() && already_running { - runtime_networks = resolved_networks(&manifest, &self.config.cvm); + runtime_networks = self.inferred_runtime_networks(&manifest); if let Err(err) = vm_work_dir.set_runtime_networks(&runtime_networks) { warn!(id = %vm_id, "failed to persist inferred runtime networks: {err}"); } @@ -1160,11 +1325,15 @@ impl App { }); let total = infos.len() as u32; + // One probe for the whole page, and none at all when every VM is + // running and has its own snapshot to report. + let netd_reachable = OnceCell::new(); let vms = paginate(infos, request.page, request.page_size) .map(|vm| { let work_dir = self.work_dir(&vm.config.manifest.id)?; let info = vm.merged_info(vms.get(&vm.config.manifest.id), &work_dir); - Ok(info.to_pb(&self.config.gateway, &self.config.cvm, request.brief)) + let networks = self.effective_networks(&info, &netd_reachable); + Ok(info.to_pb(&self.config.gateway, request.brief, &networks)) }) .collect::>>()?; Ok(StatusResponse { @@ -1188,14 +1357,19 @@ impl App { pub async fn vm_info(&self, id: &str) -> Result> { let proc_state = self.supervisor.info(id).await?; - let state = self.lock(); - let Some(vm_state) = state.get(id) else { - return Ok(None); + // Snapshot under the lock, then release it: describing the VM can + // probe netd, and that is a blocking connect the global state lock has + // no business being held across. + let info = { + let state = self.lock(); + let Some(vm_state) = state.get(id) else { + return Ok(None); + }; + vm_state.merged_info(proc_state.as_ref(), &self.work_dir(id)?) }; - let info = vm_state - .merged_info(proc_state.as_ref(), &self.work_dir(id)?) - .to_pb(&self.config.gateway, &self.config.cvm, false); - Ok(Some(info)) + let netd_reachable = OnceCell::new(); + let networks = self.effective_networks(&info, &netd_reachable); + Ok(Some(info.to_pb(&self.config.gateway, false, &networks))) } pub(crate) fn vm_event_report(&self, cid: u32, event: &str, body: String) -> Result<()> { @@ -1917,7 +2091,7 @@ mod tests { } use crate::config::{ - load_config_figment, CvmPlatform, Networking, NetworkingMode, TdxAttestationVariantConfig, + load_config_figment, CvmPlatform, NetworkingMode, TdxAttestationVariantConfig, }; use dstack_types::{ TdxImageMeasurement, TdxMrtdCandidates, TdxOsImageMeasurement, @@ -2230,17 +2404,10 @@ mod tests { )); let workdir = VmWorkDir::new(&temp); let mut manifest = test_manifest(1024); - manifest.networks = vec![Networking { + manifest.networks = vec![NicNetworking { mode: NetworkingMode::Bridge, bridge: "dstack-br0".to_string(), - parent: String::new(), - macvtap_mode: String::new(), - device: String::new(), - mac_prefix: String::new(), - net: String::new(), - dhcp_start: String::new(), - restrict: false, - netdev: String::new(), + ..NicNetworking::default() }]; workdir.put_manifest(&manifest)?; @@ -2495,17 +2662,10 @@ mod tests { fn vm_measurement_config_ignores_networking_changes() -> Result<()> { let config = test_tdx_config()?; let mut bridge_manifest = test_manifest(2048); - bridge_manifest.networks = vec![Networking { + bridge_manifest.networks = vec![NicNetworking { mode: NetworkingMode::Bridge, bridge: "dstack-br0".to_string(), - parent: String::new(), - macvtap_mode: String::new(), - device: String::new(), - mac_prefix: "02:aa:bb".to_string(), - net: String::new(), - dhcp_start: String::new(), - restrict: false, - netdev: String::new(), + ..NicNetworking::default() }]; let user_manifest = test_manifest(2048); let image = test_tdx_image(true); diff --git a/dstack/vmm/src/app/network.rs b/dstack/vmm/src/app/network.rs index 1b840154f..4017e89ff 100644 --- a/dstack/vmm/src/app/network.rs +++ b/dstack/vmm/src/app/network.rs @@ -10,57 +10,267 @@ use anyhow::{bail, Result}; use sha2::{Digest, Sha256}; use super::Manifest; -use crate::config::{CvmConfig, Networking, NetworkingMode}; +use crate::config::{ + CvmConfig, NetdInterface, NetworkFilterMode, Networking, NetworkingMode, NicNetworking, + MAX_NET_QUEUES, +}; -pub(crate) fn resolve_networking(networking: &Networking, cfg: &CvmConfig) -> Networking { +/// Node configuration merged with what one NIC pins. +/// +/// The node half is taken wholesale and the NIC half overrides it where it is +/// set. Nothing the node owns -- the macvtap forwarding mode, the MAC prefix, +/// the user-mode network parameters, a custom netdev string -- can be +/// overridden here any more, because a [`NicNetworking`] cannot carry one. +pub(crate) fn resolve_networking( + networking: &NicNetworking, + cfg: &CvmConfig, + vcpu: u32, +) -> Networking { let mut resolved = cfg.networking.clone(); - resolved.mode = networking.mode; - resolved.restrict = cfg.networking.restrict || networking.restrict; + // A deployment that only tuned the data plane never named a backend, so + // the node keeps deciding which one this NIC uses -- including after the + // operator changes it. + resolved.nic.inherit_mode = networking.inherit_mode; + resolved.nic.mode = if networking.inherit_mode { + cfg.networking.nic.mode + } else { + networking.mode + }; + // Runtime state, never inherited from configuration or from a previous + // launch. Interface preparation sets both for the NICs it builds, and a + // node configuration that names either is rejected at startup. + resolved.netd_interface = crate::config::NetdInterface::None; + resolved.device.clear(); if !networking.bridge.is_empty() { - resolved.bridge = networking.bridge.clone(); + resolved.nic.bridge = networking.bridge.clone(); } if !networking.parent.is_empty() { - resolved.parent = networking.parent.clone(); - } - if !networking.mac_prefix.is_empty() { - resolved.mac_prefix = networking.mac_prefix.clone(); - } - if !networking.net.is_empty() { - resolved.net = networking.net.clone(); + resolved.nic.parent = networking.parent.clone(); } - if !networking.dhcp_start.is_empty() { - resolved.dhcp_start = networking.dhcp_start.clone(); - } - if !networking.netdev.is_empty() { - resolved.netdev = networking.netdev.clone(); + if networking.vhost.is_some() { + resolved.nic.vhost = networking.vhost; } + // Make the vCPU-scaled default concrete here, so every later stage -- + // netd preparation, the QEMU arguments, and removal after a VMM restart -- + // reads one number instead of recomputing it from a vCPU count it may no + // longer have. + resolved.nic.queues = Some(match networking.queues { + // An explicit count is honoured whatever the data plane: multiqueue + // without vhost is a valid, if unusual, thing to ask for. + Some(queues) => queues, + // Without vhost the QEMU main loop drains every queue on one thread, + // so scaling up buys almost nothing while still costing a netd + // interface, extra vectors, and a changed device. Anyone turning vhost + // off is asking for the old data plane; give them the old shape too. + None if resolved.vhost_enabled() => { + Networking::default_queue_pairs(vcpu, cfg.max_net_queues) + } + None => 1, + }); resolved } pub(crate) fn resolved_networks(manifest: &Manifest, cfg: &CvmConfig) -> Vec { - if manifest.networks.is_empty() { - vec![cfg.networking.clone()] + let node_default = [cfg.networking.nic.clone()]; + let requested = if manifest.networks.is_empty() { + &node_default[..] } else { - manifest - .networks - .iter() - .map(|networking| resolve_networking(networking, cfg)) - .collect() + &manifest.networks[..] + }; + requested + .iter() + .map(|networking| resolve_networking(networking, cfg, manifest.vcpu)) + .collect() +} + +/// Whether netd must pre-create the host interface for this NIC. +/// +/// Macvtap always needs one. A bridge NIC needs one when libvirt filtering +/// binds an nwfilter to the TAP, and when multiqueue requires a persistent +/// `IFF_MULTI_QUEUE` device that `qemu-bridge-helper` cannot create. +pub(crate) fn needs_netd_interface(networking: &Networking, cfg: &CvmConfig) -> bool { + match networking.nic.mode { + NetworkingMode::Macvtap => true, + NetworkingMode::Bridge => { + cfg.network_filter.mode == NetworkFilterMode::Libvirt || networking.queue_pairs() > 1 + } + NetworkingMode::User | NetworkingMode::Custom => false, + } +} + +/// Whether this NIC's host interface carries a libvirt nwfilter binding. +/// Macvtap never does, and a bridge NIC only does when the node filters. +pub(crate) fn filters_bridge_traffic(networking: &Networking, cfg: &CvmConfig) -> bool { + networking.nic.mode == NetworkingMode::Bridge + && cfg.network_filter.mode == NetworkFilterMode::Libvirt +} + +/// Whether netd built this NIC's host interface, and if so whether it carries +/// an nwfilter binding. +/// +/// Interface preparation records this, because it is not derivable afterwards: +/// an operator can change `network_filter.mode` or `max_net_queues` while a VM +/// runs, and teardown has to undo what was built rather than what would be +/// built now. +pub(crate) fn netd_teardown(networking: &Networking, cfg: &CvmConfig) -> Option { + match networking.netd_interface { + NetdInterface::Filtered => Some(true), + NetdInterface::Unfiltered => Some(false), + // Either nothing was built, or this entry was persisted before + // preparation recorded the fact. Fall back to the derivation such an + // entry was created by; a Remove for an interface that does not exist + // is a no-op. + NetdInterface::None if needs_netd_interface(networking, cfg) => { + Some(filters_bridge_traffic(networking, cfg)) + } + NetdInterface::None => None, } } +/// Drops a NIC back to one queue pair when multiqueue would need a netd +/// interface this node cannot provide. +/// +/// Queue pairs are a default now, not something the operator asked for, so a +/// node that has never deployed netd must keep launching bridge VMs. An +/// explicit per-VM request is left alone: the caller asked for it, and failing +/// at prepare tells them why far better than silently halving their throughput. +/// Returns how many NICs it dropped, so a launch can say so and a status +/// query, which runs the same calculation to describe a stopped VM, stays +/// silent. +pub(crate) fn clamp_queues_without_netd( + requested: &[NicNetworking], + resolved: &mut [Networking], + cfg: &CvmConfig, + netd_available: bool, +) -> usize { + if netd_available { + return 0; + } + let mut clamped = 0; + for (networking, asked) in resolved.iter_mut().zip(requested) { + // Macvtap has nothing to fall back to: netd is the only thing that can + // create the device, so clamping one would describe a VM that cannot + // start either way. + if networking.nic.mode != NetworkingMode::Bridge + || asked.queues.is_some() + || !needs_netd_interface(networking, cfg) + // Filtering needs netd whatever the queue count, so dropping this + // NIC to one queue pair would not make it launchable. It would only + // describe it as something no launch can produce, and warn about a + // fallback that is not happening. + || filters_bridge_traffic(networking, cfg) + { + continue; + } + networking.nic.queues = Some(1); + clamped += 1; + } + clamped +} + +/// Locations distributions install `qemu-bridge-helper` in. The helper is +/// setuid root and attaches an unprivileged TAP to a whitelisted bridge, which +/// is how bridge mode avoids giving the VMM `CAP_NET_ADMIN`. +const BRIDGE_HELPER_CANDIDATES: [&str; 3] = [ + "/usr/lib/qemu/qemu-bridge-helper", + "/usr/libexec/qemu-bridge-helper", + "/usr/local/libexec/qemu-bridge-helper", +]; + +/// Absolute path of `qemu-bridge-helper`, which QEMU's `tap` netdev, unlike its +/// `bridge` netdev, has no compiled-in default for. +/// +/// A configured path is passed through unchecked: the operator is naming a +/// binary for QEMU to exec, and QEMU need not see this filesystem. +pub(crate) fn find_bridge_helper<'a>( + configured: &'a str, + candidates: &[&'a str], +) -> Option<&'a str> { + let configured = configured.trim(); + if !configured.is_empty() { + return Some(configured); + } + candidates + .iter() + .copied() + .find(|candidate| Path::new(candidate).exists()) +} + +pub(crate) fn bridge_helper(cfg: &CvmConfig) -> Option<&str> { + find_bridge_helper(&cfg.qemu_bridge_helper, &BRIDGE_HELPER_CANDIDATES) +} + +/// Whether this NIC will actually run on the vhost-net data plane. +/// +/// A bridge NIC that neither needs a netd interface nor can find +/// `qemu-bridge-helper` falls back to QEMU's `bridge` netdev, which has no +/// vhost support. Both the QEMU arguments and the reported status read this, +/// so a VM is never described as using a data plane it did not get. +pub(crate) fn effective_vhost(networking: &Networking, cfg: &CvmConfig) -> bool { + if !networking.vhost_enabled() { + return false; + } + networking.nic.mode != NetworkingMode::Bridge + || needs_netd_interface(networking, cfg) + || bridge_helper(cfg).is_some() +} + +/// Makes the effective data plane concrete on a launch-time NIC list, and +/// returns how many interfaces asked for vhost and did not get it. +/// +/// `vhost` on a freshly resolved entry is still a *request*: `None` means +/// inherit, and a bridge NIC that cannot reach `qemu-bridge-helper` runs on the +/// non-vhost netdev whatever it asked for. Settling it once, here, is what lets +/// the QEMU arguments and the reported status read the same value -- and keeps +/// them reading it after the operator moves the helper out from under a VM that +/// is already running. +pub(crate) fn settle_vhost(networks: &mut [Networking], cfg: &CvmConfig) -> usize { + let mut denied = 0; + for networking in networks.iter_mut() { + let effective = effective_vhost(networking, cfg); + if networking.vhost_enabled() && !effective { + denied += 1; + } + networking.nic.vhost = Some(effective); + } + denied +} + +/// Whether netd is reachable. A netd that died leaves its socket behind, so +/// existence alone would report a node as capable and fail every launch. +/// +/// A connect and nothing more, deliberately: netd serves connections serially, +/// so anything that waits for an answer reads a *busy* netd as a missing one +/// and silently drops the VM to a single queue pair. Accepting the connection +/// is the one signal that does not depend on what netd is doing right now. +pub(crate) fn netd_available(socket: &Path) -> bool { + std::os::unix::net::UnixStream::connect(socket).is_ok() +} + pub(crate) fn validate_resolved_network(networking: &Networking) -> Result<()> { - if networking.mode != NetworkingMode::Bridge { + // The vCPU-scaled default is bounded by construction; only an explicit + // request can exceed the hard cap. + if networking + .nic + .queues + .is_some_and(|queues| queues > MAX_NET_QUEUES) + { + bail!("networking queues must not exceed {MAX_NET_QUEUES}"); + } + if networking.nic.mode != NetworkingMode::Bridge { return Ok(()); } - if networking.bridge.is_empty() { + if networking.nic.bridge.is_empty() { bail!("bridge networking requested but no bridge is configured"); } if !Path::new("/sys/class/net") - .join(&networking.bridge) + .join(&networking.nic.bridge) .exists() { - bail!("bridge interface '{}' does not exist", networking.bridge); + bail!( + "bridge interface '{}' does not exist", + networking.nic.bridge + ); } Ok(()) } @@ -72,6 +282,53 @@ pub(crate) fn validate_resolved_networks(networks: &[Networking]) -> Result<()> Ok(()) } +/// Warns when a vhost NIC is about to launch on a host that has no +/// `/dev/vhost-net` at all. +/// +/// QEMU exits when `vhost=on` cannot open the device, and it does so from +/// inside the per-VM launcher where the reason is easy to miss. This puts the +/// remediation in the VMM log instead. +/// +/// Both checks are warnings, never refusals. QEMU is not necessarily this +/// process — an externally started supervisor can run it under another account +/// — so neither answers the question that decides the launch. They are the two +/// cheap statements that catch the two ways this actually goes wrong. +/// +/// The permission check matters because the node's mode is not uniform. It is +/// `root:kvm 0660` on Debian-family hosts, where a VMM in the `kvm` group is +/// fine, and `root:root 0600` on several others — where every bridge VM stops +/// restarting after an upgrade turns vhost on node-wide, with the only +/// explanation buried in a per-VM launcher's QEMU output. Existence alone says +/// nothing about that case, which is the likelier of the two. +pub(crate) fn warn_if_vhost_net_missing(networks: &[Networking]) { + const VHOST_NET: &str = "/dev/vhost-net"; + if !networks.iter().any(Networking::vhost_enabled) { + return; + } + // The node is a kmod static device node, so it is present even before + // vhost_net is loaded; QEMU's open autoloads the module. + if !Path::new(VHOST_NET).exists() { + tracing::warn!( + "{VHOST_NET} is missing; vhost networking will fail to start. load the vhost_net \ + module, or set vhost = false in [cvm.networking]" + ); + return; + } + if let Err(error) = std::fs::OpenOptions::new() + .read(true) + .write(true) + .open(VHOST_NET) + { + if error.kind() == std::io::ErrorKind::PermissionDenied { + tracing::warn!( + "{VHOST_NET} is not accessible to this process; if QEMU runs under the same \ + account, vhost networking will fail to start. add that account to the device's \ + group, or set vhost = false in [cvm.networking]" + ); + } + } +} + /// Derives a deterministic, locally administered unicast MAC address. /// /// Index zero preserves the legacy single-NIC derivation. Later interfaces @@ -98,7 +355,361 @@ pub(crate) fn mac_address_for_vm_index(vm_id: &str, prefix: &[u8], index: usize) #[cfg(test)] mod tests { - use super::mac_address_for_vm_index; + use super::{ + clamp_queues_without_netd, effective_vhost, mac_address_for_vm_index, needs_netd_interface, + netd_teardown, resolve_networking, resolved_networks, settle_vhost, + validate_resolved_networks, + }; + use crate::config::{Networking, NetworkingMode, NicNetworking}; + + fn macvtap_network() -> NicNetworking { + NicNetworking { + mode: NetworkingMode::Macvtap, + parent: "eth0".into(), + // Most tests here exercise the vhost data plane, which the + // shipped default leaves off; opt in the way a real node would. + vhost: Some(true), + ..NicNetworking::default() + } + } + + /// The same NIC as a resolved value, for the checks that run against what + /// a launch would see rather than against what a VM pins. + fn macvtap_resolved() -> Networking { + Networking { + nic: macvtap_network(), + ..Networking::default() + } + } + + fn node_config(mode: NetworkingMode) -> crate::config::CvmConfig { + use rocket::figment::providers::Format as _; + let config: crate::config::Config = rocket::figment::Figment::from( + rocket::figment::providers::Toml::string(crate::config::DEFAULT_CONFIG), + ) + .extract() + .unwrap(); + let mut cvm = config.cvm; + cvm.networking.nic.mode = mode; + cvm.networking.nic.bridge = "br0".into(); + cvm.networking.nic.parent = "eth0".into(); + // The shipped default leaves vhost off; the tests here are about the + // vhost data plane, so this node opts in the way a real one would. + cvm.networking.nic.vhost = Some(true); + cvm + } + + /// A node whose configuration never names `vhost` — the upgrade case, + /// where the toml predates the key entirely. + fn unconfigured_vhost_node(mode: NetworkingMode) -> crate::config::CvmConfig { + let mut cvm = node_config(mode); + cvm.networking.nic.vhost = None; + cvm + } + + fn manifest_with(vcpu: u32, networks: Vec) -> crate::app::Manifest { + let mut manifest: crate::app::Manifest = serde_json::from_value(serde_json::json!({ + "id": "vm-1", "name": "n", "app_id": "a", "vcpu": vcpu, "memory": 2048, + "disk_size": 10, "image": "i", "port_map": [], "created_at_ms": 0, + })) + .unwrap(); + manifest.networks = networks; + manifest + } + + #[test] + fn queue_pairs_default_to_the_vcpu_count_up_to_the_cap() { + let cvm = node_config(NetworkingMode::Bridge); + for (vcpu, want) in [(1, 1), (2, 2), (8, 8), (16, 16), (32, 16), (128, 16)] { + let resolved = resolved_networks(&manifest_with(vcpu, vec![]), &cvm); + assert_eq!( + resolved[0].queue_pairs(), + want, + "vcpu {vcpu} should give {want} queue pairs" + ); + } + } + + /// An upgraded node must keep building the device its VMs have always + /// had: userspace virtio, one queue pair. Both the toml that predates the + /// `vhost` key and the shipped default say so. + #[test] + fn a_node_that_never_asked_for_vhost_keeps_the_old_device_shape() { + let shipped = { + use rocket::figment::providers::Format as _; + let config: crate::config::Config = rocket::figment::Figment::from( + rocket::figment::providers::Toml::string(crate::config::DEFAULT_CONFIG), + ) + .extract() + .unwrap(); + config.cvm.networking.nic.vhost + }; + for vhost in [None, shipped] { + let mut cvm = unconfigured_vhost_node(NetworkingMode::Bridge); + cvm.networking.nic.vhost = vhost; + let resolved = resolved_networks(&manifest_with(16, vec![]), &cvm); + assert!(!resolved[0].vhost_enabled()); + assert_eq!(resolved[0].queue_pairs(), 1); + assert!(!needs_netd_interface(&resolved[0], &cvm)); + } + } + + #[test] + fn turning_vhost_off_also_turns_off_the_multiqueue_default() { + let mut cvm = node_config(NetworkingMode::Bridge); + cvm.networking.nic.vhost = Some(false); + let resolved = resolved_networks(&manifest_with(8, vec![]), &cvm); + assert!(!resolved[0].vhost_enabled()); + assert_eq!(resolved[0].queue_pairs(), 1); + assert!(!needs_netd_interface(&resolved[0], &cvm)); + + // Per-VM opt-out does the same thing. + let cvm = node_config(NetworkingMode::Bridge); + let mut asked = cvm.networking.nic.clone(); + asked.vhost = Some(false); + let resolved = resolved_networks(&manifest_with(8, vec![asked]), &cvm); + assert_eq!(resolved[0].queue_pairs(), 1); + + // But an explicit queue count is still honoured without vhost. + let mut asked = cvm.networking.nic.clone(); + asked.vhost = Some(false); + asked.queues = Some(4); + let resolved = resolved_networks(&manifest_with(8, vec![asked]), &cvm); + assert!(!resolved[0].vhost_enabled()); + assert_eq!(resolved[0].queue_pairs(), 4); + } + + #[test] + fn lowering_the_request_ceiling_also_lowers_the_default() { + let mut cvm = node_config(NetworkingMode::Bridge); + cvm.max_net_queues = 2; + let resolved = resolved_networks(&manifest_with(16, vec![]), &cvm); + assert_eq!(resolved[0].queue_pairs(), 2); + + // Raising it past the scaling cap widens requests, not the default. + cvm.max_net_queues = 32; + let resolved = resolved_networks(&manifest_with(24, vec![]), &cvm); + assert_eq!(resolved[0].queue_pairs(), 16); + } + + #[test] + fn status_never_claims_a_data_plane_the_nic_did_not_get() { + let mut cvm = node_config(NetworkingMode::Bridge); + // No helper on this filesystem and no netd interface needed, so the + // NIC falls back to QEMU's `bridge` netdev, which has no vhost. + cvm.qemu_bridge_helper = String::new(); + let mut single = cvm.networking.nic.clone(); + single.queues = Some(1); + let resolved = resolved_networks(&manifest_with(8, vec![single]), &cvm); + assert!(resolved[0].vhost_enabled()); + let fell_back = !effective_vhost(&resolved[0], &cvm); + assert_eq!(fell_back, super::bridge_helper(&cvm).is_none()); + + // A configured helper is taken at its word, so vhost is real. + cvm.qemu_bridge_helper = "/opt/qemu-bridge-helper".into(); + let resolved = resolved_networks(&manifest_with(8, vec![]), &cvm); + assert!(effective_vhost(&resolved[0], &cvm)); + + // Multiqueue goes through netd, which needs no helper at all. + let mut mq = cvm.networking.nic.clone(); + mq.queues = Some(4); + cvm.qemu_bridge_helper = String::new(); + let resolved = resolved_networks(&manifest_with(8, vec![mq]), &cvm); + assert!(needs_netd_interface(&resolved[0], &cvm)); + assert!(effective_vhost(&resolved[0], &cvm)); + } + + #[test] + fn an_explicit_queue_count_survives_resolution() { + let cvm = node_config(NetworkingMode::Bridge); + let mut asked = macvtap_network(); + asked.mode = NetworkingMode::Bridge; + asked.queues = Some(2); + let resolved = resolved_networks(&manifest_with(16, vec![asked]), &cvm); + assert_eq!(resolved[0].queue_pairs(), 2); + } + + #[test] + fn user_mode_stays_single_queue_whatever_the_vcpu_count() { + let cvm = node_config(NetworkingMode::User); + let resolved = resolved_networks(&manifest_with(32, vec![]), &cvm); + assert_eq!(resolved[0].queue_pairs(), 1); + } + + #[test] + fn without_netd_a_defaulted_bridge_drops_to_one_queue_but_a_request_does_not() { + let cvm = node_config(NetworkingMode::Bridge); + + // The default is ours to lower: a node that never deployed netd must + // keep launching bridge VMs. + let requested = vec![cvm.networking.nic.clone()]; + let mut resolved = resolved_networks(&manifest_with(8, vec![]), &cvm); + assert_eq!(resolved[0].queue_pairs(), 8); + clamp_queues_without_netd(&requested, &mut resolved, &cvm, false); + assert_eq!(resolved[0].queue_pairs(), 1); + assert!(!needs_netd_interface(&resolved[0], &cvm)); + + // An explicit request is left alone, so prepare fails where the caller + // can see why instead of silently halving their throughput. + let mut asked = cvm.networking.nic.clone(); + asked.queues = Some(4); + let requested = vec![asked.clone()]; + let mut resolved = resolved_networks(&manifest_with(8, vec![asked]), &cvm); + clamp_queues_without_netd(&requested, &mut resolved, &cvm, false); + assert_eq!(resolved[0].queue_pairs(), 4); + + // With netd present nothing is touched. + let requested = vec![cvm.networking.nic.clone()]; + let mut resolved = resolved_networks(&manifest_with(8, vec![]), &cvm); + clamp_queues_without_netd(&requested, &mut resolved, &cvm, true); + assert_eq!(resolved[0].queue_pairs(), 8); + } + + #[test] + fn validation_never_depends_on_this_process_reaching_vhost_net() { + // QEMU may run under different credentials, so a NIC that asks for + // vhost must validate on hosts where the VMM itself cannot open the + // device. Both of these hold whether or not /dev/vhost-net exists here. + let mut networking = macvtap_resolved(); + assert!(networking.vhost_enabled()); + validate_resolved_networks(&[networking.clone()]).unwrap(); + + networking.nic.queues = Some(4); + validate_resolved_networks(&[networking]).unwrap(); + } + + #[test] + fn queue_counts_above_the_hard_bound_are_rejected() { + let mut networking = macvtap_resolved(); + networking.nic.queues = Some(super::MAX_NET_QUEUES + 1); + let error = validate_resolved_networks(&[networking]).unwrap_err(); + assert!(error.to_string().contains("must not exceed")); + } + + /// The data plane a NIC actually gets is decided once, at launch, and + /// written into the runtime entry. Recomputing it later would let a report + /// about a running VM change under an operator's edit to node + /// configuration, describing a data plane QEMU is not using. + #[test] + fn settling_vhost_records_what_the_launch_decided() { + let mut cvm = node_config(NetworkingMode::Bridge); + cvm.qemu_bridge_helper = String::new(); + let mut single = cvm.networking.nic.clone(); + single.queues = Some(1); + let manifest = manifest_with(8, vec![single]); + + // Whether this host has a helper is not the test's business; that it + // gets written down, once, is. + let helper_missing = super::bridge_helper(&cvm).is_none(); + let mut networks = resolved_networks(&manifest, &cvm); + assert!(networks[0].vhost_enabled(), "the request starts out on"); + assert_eq!( + settle_vhost(&mut networks, &cvm), + usize::from(helper_missing) + ); + assert_eq!(networks[0].nic.vhost, Some(!helper_missing)); + // Settling an already-settled list reports nothing new, so a relaunch + // does not warn about a fallback that already happened. + assert_eq!(settle_vhost(&mut networks, &cvm), 0); + + // A configured helper is taken at its word, so the same NIC settles on. + cvm.qemu_bridge_helper = "/opt/qemu-bridge-helper".into(); + let mut with_helper = resolved_networks(&manifest, &cvm); + assert_eq!(settle_vhost(&mut with_helper, &cvm), 0); + assert_eq!(with_helper[0].nic.vhost, Some(true)); + + // The entry the first launch settled keeps its answer: nothing about a + // running VM is recomputed from the configuration as it stands now. + assert_eq!(networks[0].nic.vhost, Some(!helper_missing)); + } + + /// Dropping to a single queue pair is only worth doing when it makes the + /// NIC launchable. A filtered bridge needs netd whatever its queue count, + /// so clamping it would report a shape no launch can produce. + #[test] + fn a_filtered_bridge_is_not_clamped_because_it_cannot_help() { + use crate::config::NetworkFilterMode; + + let mut cvm = node_config(NetworkingMode::Bridge); + cvm.network_filter.mode = NetworkFilterMode::Libvirt; + let requested = vec![cvm.networking.nic.clone()]; + let mut resolved = resolved_networks(&manifest_with(8, vec![]), &cvm); + assert_eq!(resolved[0].queue_pairs(), 8); + assert_eq!( + clamp_queues_without_netd(&requested, &mut resolved, &cvm, false), + 0 + ); + assert_eq!(resolved[0].queue_pairs(), 8); + + // Unfiltered, the same NIC does drop, because then it can launch. + let cvm = node_config(NetworkingMode::Bridge); + let mut resolved = resolved_networks(&manifest_with(8, vec![]), &cvm); + assert_eq!( + clamp_queues_without_netd(&requested, &mut resolved, &cvm, false), + 1 + ); + assert_eq!(resolved[0].queue_pairs(), 1); + } + + /// Teardown has to undo what was built. Node configuration is mutable and + /// a VM outlives an edit to it, so re-deriving "did netd build this?" at + /// removal time orphans TAPs and leaks nwfilter bindings whose ebtables + /// rules the next VM at the same deterministic interface name inherits. + #[test] + fn teardown_follows_what_was_built_not_what_configuration_now_says() { + use crate::config::{NetdInterface, NetworkFilterMode}; + + let filtering = { + let mut cvm = node_config(NetworkingMode::Bridge); + cvm.network_filter.mode = NetworkFilterMode::Libvirt; + cvm + }; + let unfiltered = node_config(NetworkingMode::Bridge); + + let mut built_filtered = filtering.networking.clone(); + built_filtered.nic.queues = Some(1); + built_filtered.netd_interface = NetdInterface::Filtered; + // The operator turns filtering off while the VM runs. The binding is + // still there and still has to be deleted. + assert_eq!(netd_teardown(&built_filtered, &unfiltered), Some(true)); + + let mut built_unfiltered = unfiltered.networking.clone(); + built_unfiltered.nic.queues = Some(4); + built_unfiltered.netd_interface = NetdInterface::Unfiltered; + // The operator turns filtering on. There is no binding to delete, and + // asking libvirt for one would fail the removal. + assert_eq!(netd_teardown(&built_unfiltered, &filtering), Some(false)); + + // A NIC netd never touched stays untouched, whatever the node now says. + let mut untouched = unfiltered.networking.clone(); + untouched.nic.queues = Some(1); + assert_eq!(netd_teardown(&untouched, &unfiltered), None); + + // An entry persisted before preparation recorded the fact still gets + // torn down by the rule that created it. + let mut legacy = filtering.networking.clone(); + legacy.nic.queues = Some(1); + assert_eq!(legacy.netd_interface, NetdInterface::None); + assert_eq!(netd_teardown(&legacy, &filtering), Some(true)); + } + + /// Resolution produces launch input, never a claim about what exists. + #[test] + fn resolution_never_carries_a_stale_interface_record() { + use crate::config::NetdInterface; + + let cvm = node_config(NetworkingMode::Bridge); + // Single queue and no filtering, so nothing but a stale record could + // make teardown believe netd built something. + let mut previous = cvm.networking.clone(); + previous.nic.queues = Some(1); + previous.netd_interface = NetdInterface::Filtered; + assert_eq!(netd_teardown(&previous, &cvm), Some(true)); + + let resolved = resolve_networking(&previous.nic, &cvm, 4); + assert_eq!(resolved.netd_interface, NetdInterface::None); + assert_eq!(netd_teardown(&resolved, &cvm), None); + } #[test] fn primary_mac_keeps_legacy_derivation_and_later_nics_are_distinct() { diff --git a/dstack/vmm/src/app/qemu.rs b/dstack/vmm/src/app/qemu.rs index db2fd39d4..94a6348fe 100644 --- a/dstack/vmm/src/app/qemu.rs +++ b/dstack/vmm/src/app/qemu.rs @@ -9,14 +9,15 @@ use super::{ hugepage_numa_nodes, image::Image, mr_config::{snp_host_data, tdx_mr_config_id}, - network::{mac_address_for_vm_index, validate_resolved_networks}, + network::{ + bridge_helper, mac_address_for_vm_index, needs_netd_interface, validate_resolved_networks, + warn_if_vhost_net_missing, + }, pci_numa_node, round_up, GpuConfig, VmWorkDir, }; use crate::{ app::Manifest, - config::{ - CvmConfig, CvmPlatform, NetworkFilterMode, Networking, NetworkingMode, ProcessAnnotation, - }, + config::{CvmConfig, CvmPlatform, Networking, NetworkingMode, ProcessAnnotation}, netd::{tap_name, InterfaceIdentity}, vm_launcher::{ChildCommand, LaunchSpec, OpenFile}, }; @@ -167,6 +168,14 @@ fn create_hd( Ok(()) } +fn on_off(enabled: bool) -> &'static str { + if enabled { + "on" + } else { + "off" + } +} + fn virtio_pci_device(device: &str, snp: bool) -> String { if snp { format!("{device},disable-legacy=on,iommu_platform=true") @@ -179,6 +188,34 @@ struct PreparedVolume { source: String, } +/// First descriptor the per-VM launcher may hand to QEMU. Zero through two are +/// the standard streams. +const FIRST_INHERITED_FD: i32 = 3; + +/// Descriptors the launcher opens for each macvtap NIC, one per queue pair. +/// +/// Both the launcher's open list and the `-netdev` arguments derive from this +/// one layout, so they cannot disagree about which descriptor belongs to which +/// NIC. +fn macvtap_fd_layout(networks: &[Networking]) -> Vec> { + let mut next_fd = FIRST_INHERITED_FD; + networks + .iter() + .map(|network| { + if network.nic.mode != NetworkingMode::Macvtap { + return Vec::new(); + } + (0..network.queue_pairs()) + .map(|_| { + let fd = next_fd; + next_fd += 1; + fd + }) + .collect() + }) + .collect() +} + struct PreparedQemuLaunch { workdir: VmWorkDir, platform: CvmPlatform, @@ -209,6 +246,7 @@ impl PreparedQemuLaunch { let platform = cfg.resolved_platform(); let networks = networks.to_vec(); validate_resolved_networks(&networks)?; + warn_if_vhost_net_missing(&networks); let volumes = vm .manifest .volumes @@ -352,7 +390,7 @@ impl VmConfig { let has_macvtap = prepared .networks .iter() - .any(|network| network.mode == NetworkingMode::Macvtap); + .any(|network| network.nic.mode == NetworkingMode::Macvtap); let Some(socket) = prepared.swtpm_socket.as_deref() else { if has_macvtap { return self.wrap_launcher(&prepared, process, None, None); @@ -396,14 +434,17 @@ impl VmConfig { swtpm: Option, swtpm_socket: Option, ) -> Result> { + // Each queue pair is a separate open of the same macvtap character + // device; the kernel attaches one tap queue per open. let open_files = prepared .networks .iter() - .enumerate() - .filter(|(_, network)| network.mode == NetworkingMode::Macvtap) - .map(|(index, network)| OpenFile { - fd: (3 + index) as i32, - path: network.device.clone().into(), + .zip(macvtap_fd_layout(&prepared.networks)) + .flat_map(|(network, fds)| { + fds.into_iter().map(|fd| OpenFile { + fd, + path: network.device.clone().into(), + }) }) .collect(); let spec = LaunchSpec { @@ -589,11 +630,12 @@ impl QemuCommandBuilder<'_> { } fn configure_networking(&self, command: &mut Command) -> Result<()> { + let macvtap_fds = macvtap_fd_layout(&self.prepared.networks); let hostfwd_index = self .prepared .networks .iter() - .position(|networking| networking.mode == NetworkingMode::User); + .position(|networking| networking.nic.mode == NetworkingMode::User); for (index, networking) in self.prepared.networks.iter().enumerate() { let net_id = format!("net{index}"); let mac = mac_address_for_vm_index( @@ -601,12 +643,20 @@ impl QemuCommandBuilder<'_> { &networking.mac_prefix_bytes(), index, ); - let net_device = virtio_pci_device( - &format!("virtio-net-pci,netdev={net_id},mac={mac}"), - self.is_amd_sev_snp(), - ); - let netdev = match networking.mode { + let queues = networking.queue_pairs(); + let vhost = networking.vhost_enabled(); + let mut device = format!("virtio-net-pci,netdev={net_id},mac={mac}"); + if queues > 1 { + // One vector per queue direction, plus config and control. + device.push_str(&format!(",mq=on,vectors={}", 2 * queues + 2)); + } + let net_device = virtio_pci_device(&device, self.is_amd_sev_snp()); + let netdev = match networking.nic.mode { NetworkingMode::User => { + // The user-mode backend has neither, so both are ignored + // here. A caller who *named* this mode and then asked for + // vhost or more than one queue pair is refused by the RPC; + // one who inherited it is not, and lands here. let mut netdev = format!( "user,id={net_id},net={},dhcpstart={},restrict={}", networking.net, @@ -627,24 +677,46 @@ impl QemuCommandBuilder<'_> { netdev } NetworkingMode::Bridge => { - tracing::info!("bridge networking: mac={mac} bridge={}", networking.bridge); - match self.cfg.network_filter.mode { - NetworkFilterMode::None => { - format!("bridge,id={net_id},br={}", networking.bridge) - } - NetworkFilterMode::Libvirt => { - let tap = tap_name(&InterfaceIdentity { - instance_id: self.cfg.instance_id.clone(), - vm_id: self.vm.manifest.id.clone(), - nic_index: index, - }); - // Keep the filtered backend conservative: QEMU - // uses the TAP path on which libvirt installed the - // nwfilter binding instead of opening vhost-net. - format!( - "tap,id={net_id},ifname={tap},script=no,downscript=no,vhost=off" - ) + tracing::info!( + "bridge networking: mac={mac} bridge={} vhost={vhost} queues={queues}", + networking.nic.bridge + ); + if needs_netd_interface(networking, self.cfg) { + // netd owns this TAP: libvirt filtering binds an + // nwfilter to it, and multiqueue needs the persistent + // IFF_MULTI_QUEUE device the bridge helper cannot make. + let tap = tap_name(&InterfaceIdentity { + instance_id: self.cfg.instance_id.clone(), + vm_id: self.vm.manifest.id.clone(), + nic_index: index, + }); + let mut netdev = format!( + "tap,id={net_id},ifname={tap},script=no,downscript=no,vhost={}", + on_off(vhost) + ); + if queues > 1 { + netdev.push_str(&format!(",queues={queues}")); } + netdev + } else if let Some(helper) = vhost.then(|| bridge_helper(self.cfg)).flatten() { + // QEMU's `bridge` netdev has no vhost support, but the + // same setuid helper works behind a `tap` netdev, so + // the VMM still needs no network privileges. + format!( + "tap,id={net_id},br={},helper={helper},vhost=on", + networking.nic.bridge + ) + } else if vhost { + // vhost is a node-wide setting, so a node whose helper + // sits somewhere unusual must keep booting VMs rather + // than lose every bridge NIC to a path lookup. + tracing::warn!( + "{net_id}: no qemu-bridge-helper found, falling back to the \ + non-vhost bridge netdev. set cvm.qemu_bridge_helper to enable vhost" + ); + format!("bridge,id={net_id},br={}", networking.nic.bridge) + } else { + format!("bridge,id={net_id},br={}", networking.nic.bridge) } } NetworkingMode::Custom => { @@ -659,7 +731,23 @@ impl QemuCommandBuilder<'_> { if networking.device.is_empty() { bail!("macvtap interface {index} has not been prepared by netd"); } - format!("tap,id={net_id},fd={},vhost=off", 3 + index) + let fds = macvtap_fds + .get(index) + .filter(|fds| !fds.is_empty()) + .with_context(|| { + format!("macvtap interface {index} has no launcher descriptors") + })?; + let selector = if fds.len() == 1 { + format!("fd={}", fds[0]) + } else { + let fds = fds + .iter() + .map(|fd| fd.to_string()) + .collect::>() + .join(":"); + format!("fds={fds}") + }; + format!("tap,id={net_id},{selector},vhost={}", on_off(vhost)) } }; command.arg("-netdev").arg(netdev); @@ -1021,14 +1109,14 @@ mod tests { }; use super::{ - amd_sev_snp_memory_backend_arg, parse_amd_sev_snp_qmp_capabilities, virtio_pci_device, - PreparedQemuLaunch, PreparedVolume, QemuCommandBuilder, VmConfig, + amd_sev_snp_memory_backend_arg, macvtap_fd_layout, parse_amd_sev_snp_qmp_capabilities, + virtio_pci_device, PreparedQemuLaunch, PreparedVolume, QemuCommandBuilder, VmConfig, }; use crate::app::image::{Image, ImageInfo}; use crate::app::{needs_swtpm, GpuConfig, GpuSpec, Manifest, PortMapping, VmVolume, VmWorkDir}; use crate::config::{ - Config, CvmPlatform, NetworkFilterMode, Networking, NetworkingMode, Protocol, - DEFAULT_CONFIG, + Config, CvmPlatform, NetworkFilterMode, Networking, NetworkingMode, NicNetworking, + Protocol, DEFAULT_CONFIG, }; use crate::netd::{tap_name, InterfaceIdentity}; use dstack_types::{KeyProviderKind, TeeVariant}; @@ -1080,8 +1168,9 @@ mod tests { ); } - #[test] - fn qemu_command_builder_does_not_require_prepared_paths_to_exist() { + /// Minimal launch fixture. Nothing it points at has to exist on disk; every + /// test overrides the fields it asserts on. + fn test_launch_fixture() -> (Config, VmConfig, PreparedQemuLaunch) { let mut config: Config = Figment::from(Toml::string(DEFAULT_CONFIG)) .extract() .unwrap(); @@ -1151,7 +1240,7 @@ mod tests { workdir: PathBuf::from("/does-not-exist/vm-1"), gateway_enabled: false, }; - let mut prepared = PreparedQemuLaunch { + let prepared = PreparedQemuLaunch { workdir: VmWorkDir::new("/does-not-exist/vm-1"), platform: CvmPlatform::Tdx, networks: vec![config.cvm.networking.clone(), config.cvm.networking.clone()], @@ -1167,6 +1256,171 @@ mod tests { snp_host_data: None, snp_launch_params: None, }; + (config, vm, prepared) + } + + /// Builds the `-netdev`/`-device` pairs for one NIC layout. + fn net_args(config: &Config, networks: Vec) -> Vec { + let (_, vm, mut prepared) = test_launch_fixture(); + prepared.networks = networks; + let process = QemuCommandBuilder { + vm: &vm, + cfg: &config.cvm, + gpus: &GpuConfig::default(), + prepared: &prepared, + } + .build() + .unwrap(); + process + .args + .windows(2) + .filter(|args| args[0] == "-netdev" || args[0] == "-device") + .map(|args| args[1].clone()) + .collect() + } + + fn bridge_network(config: &Config) -> Networking { + let mut networking = config.cvm.networking.clone(); + networking.nic.mode = NetworkingMode::Bridge; + networking.nic.bridge = "br0".into(); + // The vhost tests below are about what an opted-in node builds; the + // shipped default leaves it off. + networking.nic.vhost = Some(true); + networking + } + + #[test] + fn bridge_vhost_uses_the_bridge_helper_behind_a_tap_netdev() { + // QEMU's `bridge` netdev has no vhost support at all, so enabling the + // kernel data plane has to switch netdev types while keeping the same + // unprivileged setuid helper. + let (mut config, ..) = test_launch_fixture(); + config.cvm.qemu_bridge_helper = "/usr/lib/qemu/qemu-bridge-helper".into(); + let args = net_args(&config, vec![bridge_network(&config)]); + assert!(args.contains( + &"tap,id=net0,br=br0,helper=/usr/lib/qemu/qemu-bridge-helper,vhost=on".to_string() + )); + // A single queue pair must keep the historical device line byte for byte. + assert!(args.iter().any( + |arg| arg.starts_with("virtio-net-pci,netdev=net0,mac=") && !arg.contains("mq=on") + )); + } + + #[test] + fn a_missing_bridge_helper_is_reported_rather_than_guessed() { + // Configured paths are trusted verbatim: QEMU execs them, and it need + // not share this filesystem. + assert_eq!( + crate::app::network::find_bridge_helper(" /opt/qemu-bridge-helper ", &[]), + Some("/opt/qemu-bridge-helper") + ); + assert_eq!( + crate::app::network::find_bridge_helper("", &["/nonexistent/a", "/nonexistent/b"]), + None + ); + } + + #[test] + fn disabling_vhost_restores_the_legacy_bridge_netdev() { + let (config, ..) = test_launch_fixture(); + let mut networking = bridge_network(&config); + networking.nic.vhost = Some(false); + let args = net_args(&config, vec![networking]); + assert!(args.contains(&"bridge,id=net0,br=br0".to_string())); + } + + #[test] + fn multiqueue_bridge_uses_the_netd_tap_and_derives_vectors() { + let (mut config, ..) = test_launch_fixture(); + config.cvm.instance_id = "vmm-a".into(); + let mut networking = bridge_network(&config); + networking.nic.queues = Some(4); + let args = net_args(&config, vec![networking]); + let tap = tap_name(&InterfaceIdentity { + instance_id: "vmm-a".into(), + vm_id: "vm-1".into(), + nic_index: 0, + }); + assert!(args.contains(&format!( + "tap,id=net0,ifname={tap},script=no,downscript=no,vhost=on,queues=4" + ))); + // vectors = 2 per queue pair, plus config and control. + assert!(args.iter().any(|arg| arg.contains("mq=on,vectors=10"))); + } + + #[test] + fn macvtap_queues_take_one_inherited_descriptor_each() { + let (config, ..) = test_launch_fixture(); + let mut first = config.cvm.networking.clone(); + first.nic.mode = NetworkingMode::Macvtap; + first.nic.parent = "eth0".into(); + first.nic.vhost = Some(true); + first.device = "/dev/tap7".into(); + first.nic.queues = Some(2); + let mut second = first.clone(); + second.device = "/dev/tap9".into(); + second.nic.queues = Some(3); + + let networks = vec![first, second]; + let args = net_args(&config, networks.clone()); + assert!(args.contains(&"tap,id=net0,fds=3:4,vhost=on".to_string())); + assert!(args.contains(&"tap,id=net1,fds=5:6:7,vhost=on".to_string())); + + // The launcher must open exactly those descriptors, in that order. + let layout = macvtap_fd_layout(&networks); + assert_eq!(layout, vec![vec![3, 4], vec![5, 6, 7]]); + } + + #[test] + fn macvtap_keeps_a_single_fd_argument_for_one_queue() { + let (config, ..) = test_launch_fixture(); + let mut networking = config.cvm.networking.clone(); + networking.nic.mode = NetworkingMode::Macvtap; + networking.nic.parent = "eth0".into(); + networking.nic.vhost = Some(true); + networking.device = "/dev/tap7".into(); + let args = net_args(&config, vec![networking]); + assert!(args.contains(&"tap,id=net0,fd=3,vhost=on".to_string())); + } + + /// The operator owns a custom netdev string and the VMM cannot edit it, so + /// the generated device line must never claim more queues than that string + /// provides -- QEMU refuses the mismatch, from inside the per-VM launcher + /// where the reason is hard to see. + #[test] + fn custom_netdev_keeps_its_string_and_stays_single_queue() { + let (config, ..) = test_launch_fixture(); + let mut networking = config.cvm.networking.clone(); + networking.nic.mode = NetworkingMode::Custom; + networking.netdev = "tap,id=net0,ifname=custom0,vhost=on,queues=8".into(); + // Even a queue count that reached the entry some other way is ignored. + networking.nic.queues = Some(8); + let args = net_args(&config, vec![networking]); + assert!(args.contains(&"tap,id=net0,ifname=custom0,vhost=on,queues=8".to_string())); + assert!( + args.iter().all(|arg| !arg.contains("mq=on")), + "custom mode must not generate a multiqueue device line: {args:?}" + ); + } + + #[test] + fn user_mode_ignores_vhost_and_keeps_its_netdev() { + let (config, ..) = test_launch_fixture(); + let mut networking = config.cvm.networking.clone(); + networking.nic.mode = NetworkingMode::User; + networking.nic.vhost = Some(true); + let args = net_args(&config, vec![networking]); + assert!(args + .iter() + .any(|arg| arg.starts_with("user,id=net0,") && !arg.contains("vhost"))); + assert!(args.iter().any( + |arg| arg.starts_with("virtio-net-pci,netdev=net0,mac=") && !arg.contains("mq=on") + )); + } + + #[test] + fn qemu_command_builder_does_not_require_prepared_paths_to_exist() { + let (mut config, vm, mut prepared) = test_launch_fixture(); let process = QemuCommandBuilder { vm: &vm, @@ -1232,8 +1486,9 @@ mod tests { .any(|arg| arg.contains("virtio-net-pci,netdev=net1"))); for network in &mut prepared.networks { - network.mode = NetworkingMode::Bridge; - network.bridge = "br0".into(); + network.nic.mode = NetworkingMode::Bridge; + network.nic.bridge = "br0".into(); + network.nic.vhost = Some(false); } let process = QemuCommandBuilder { vm: &vm, @@ -1266,6 +1521,10 @@ mod tests { assert!(process.args.iter().any(|arg| { arg == &format!("tap,id=net0,ifname={expected_tap},script=no,downscript=no,vhost=off") })); + assert!(process + .args + .iter() + .all(|arg| !arg.contains("mq=on") && !arg.contains("vectors="))); prepared.swtpm_socket = Some(PathBuf::from("/does-not-exist/vm-1/swtpm/swtpm.sock")); let process = QemuCommandBuilder { @@ -1294,16 +1553,12 @@ mod tests { prepared.swtpm_socket = None; prepared.networks = vec![Networking { - mode: NetworkingMode::Custom, - bridge: String::new(), - parent: String::new(), - macvtap_mode: String::new(), - device: String::new(), - mac_prefix: String::new(), - net: String::new(), - dhcp_start: String::new(), - restrict: false, + nic: NicNetworking { + mode: NetworkingMode::Custom, + ..NicNetworking::default() + }, netdev: "tap,id=wrong".into(), + ..Networking::default() }]; let error = QemuCommandBuilder { vm: &vm, diff --git a/dstack/vmm/src/app/vm_info.rs b/dstack/vmm/src/app/vm_info.rs index eab200244..19d72118a 100644 --- a/dstack/vmm/src/app/vm_info.rs +++ b/dstack/vmm/src/app/vm_info.rs @@ -11,16 +11,16 @@ use dstack_vmm_rpc as pb; use fs_err as fs; use supervisor_client::supervisor::ProcessInfo; -use super::{ - network::{mac_address_for_vm_index, resolved_networks}, - Manifest, VmState, VmWorkDir, -}; -use crate::config::{CvmConfig, GatewayConfig, Networking, NetworkingMode}; +use super::{network::mac_address_for_vm_index, Manifest, VmState, VmWorkDir}; +use crate::config::{GatewayConfig, Networking, NetworkingMode, NicNetworking}; pub(crate) struct VmInfo { pub manifest: Manifest, pub workdir: PathBuf, pub status: &'static str, + /// Whether a QEMU process exists for this VM right now. The NICs it built + /// are real only while it does. + pub running: bool, pub uptime: String, pub exited_at: Option, pub instance_id: Option, @@ -51,16 +51,83 @@ fn networking_backend_name(mode: NetworkingMode) -> &'static str { } } -fn networking_to_proto(networking: &Networking) -> pb::NetworkingConfig { +/// The resolved NICs a launch built, or would build, as the RPC reports them. +fn interfaces_to_proto( + vm_id: &str, + effective_networks: &[Networking], +) -> Vec { + effective_networks + .iter() + .enumerate() + .map(|(index, networking)| { + let mac = mac_address_for_vm_index(vm_id, &networking.mac_prefix_bytes(), index); + pb::NetworkInterfaceStatus { + mode: networking_mode_name(networking.nic.mode).into(), + backend: networking_backend_name(networking.nic.mode).into(), + mac, + bridge_name: (networking.nic.mode == NetworkingMode::Bridge) + .then(|| networking.nic.bridge.clone()), + netdev_id: Some(format!("net{index}")), + // Custom mode hands the operator the whole netdev string and the + // VMM never parses it, so it has no data-plane state to report. + // Reporting the resolved fields anyway would assert "vhost: off, + // queues: 1" over a netdev the operator may have written with + // `vhost=on,queues=8`. + // + // Otherwise: settled at launch, so an entry carrying no decision + // was written before this VMM recorded one -- by a build that + // had no vhost at all, which is what it should read as. + // Recomputing here instead would let an edit to node + // configuration change what a running VM is said to use. + vhost: (networking.nic.mode != NetworkingMode::Custom) + .then(|| networking.nic.vhost.is_some() && networking.vhost_enabled()), + queues: (networking.nic.mode != NetworkingMode::Custom) + .then(|| networking.queue_pairs()), + // Node-decided, so it belongs with the rest of the resolved + // state. The VM's own record cannot carry one. + macvtap_mode: (networking.nic.mode == NetworkingMode::Macvtap) + .then(|| networking.macvtap_mode.clone()), + } + }) + .collect() +} + +pub(crate) fn networking_to_proto(networking: &NicNetworking) -> pb::NetworkingConfig { + // An entry that inherited its backend reports no mode, so it must report + // none of the fields that only make sense alongside one: a mode-less + // override carrying, say, a parent is something the deployment RPC + // rejects, which would strand the VM's tuning as uneditable. + let pins_backend = !networking.inherit_mode; pb::NetworkingConfig { - mode: networking_mode_name(networking.mode).into(), - bridge_name: if networking.mode == NetworkingMode::Bridge { + // An entry that only tuned the data plane named no backend, and the + // deployment RPC spells that as an empty mode. Reporting the node's + // current mode here would turn a read-modify-write into a request to + // pin it -- which policy may not even permit the caller to make. + mode: if networking.inherit_mode { + String::new() + } else { + networking_mode_name(networking.mode).into() + }, + bridge_name: if pins_backend && networking.mode == NetworkingMode::Bridge { networking.bridge.clone() } else { String::new() }, - parent: networking.parent.clone(), - macvtap_mode: networking.macvtap_mode.clone(), + // Scope the macvtap fields to macvtap, the way bridge_name is scoped to + // bridge. Reporting an inherited parent on a bridge NIC produced a + // configuration that could be read but not sent back: the deployment + // RPC rejects `parent` outside macvtap mode. + parent: if pins_backend && networking.mode == NetworkingMode::Macvtap { + networking.parent.clone() + } else { + String::new() + }, + // The forwarding mode is node-controlled, so a VM never pins one and + // the type it stores can no longer carry one. It stays on the wire + // because the deployment RPC still has to reject a caller that sets it. + macvtap_mode: String::new(), + vhost: networking.vhost, + queues: networking.queues, } } @@ -69,15 +136,20 @@ fn sanitize_optional>(value: Option) -> Option { } impl VmInfo { - pub fn effective_networks(&self, cvm: &CvmConfig) -> Vec { - if self.runtime_networks.is_empty() { - resolved_networks(&self.manifest, cvm) - } else { - self.runtime_networks.clone() - } - } - - pub fn to_pb(&self, gateway: &GatewayConfig, cvm: &CvmConfig, brief: bool) -> pb::VmInfo { + /// Takes no `CvmConfig` on purpose. Everything it reports about a VM's + /// data plane was decided when that VM launched and written into + /// `effective_networks`; consulting node configuration here is what let an + /// operator's edit change what a running VM was said to be using. + /// + /// `effective_networks` is passed in rather than derived for the same + /// reason, plus one more: a stopped VM's NICs are a prediction, and only + /// the caller can consult netd to make the prediction its launch would. + pub fn to_pb( + &self, + gateway: &GatewayConfig, + brief: bool, + effective_networks: &[Networking], + ) -> pb::VmInfo { let workdir = VmWorkDir::new(&self.workdir); let vm_config = workdir.manifest(); let custom_gateway_urls = vm_config @@ -91,30 +163,17 @@ impl VmInfo { .map(networking_to_proto) .collect::>(); let configured_networking = configured_networks.first().cloned(); - let interfaces = self - .effective_networks(cvm) - .iter() - .enumerate() - .map(|(index, networking)| { - let mac = mac_address_for_vm_index( - &self.manifest.id, - &networking.mac_prefix_bytes(), - index, - ); - pb::NetworkInterfaceStatus { - mode: networking_mode_name(networking.mode).into(), - backend: networking_backend_name(networking.mode).into(), - mac, - bridge_name: (networking.mode == NetworkingMode::Bridge) - .then(|| networking.bridge.clone()), - netdev_id: Some(format!("net{index}")), - } - }) - .collect(); + let interfaces = interfaces_to_proto(&self.manifest.id, effective_networks); pb::VmInfo { id: self.manifest.id.clone(), name: self.manifest.name.clone(), status: self.status.into(), + // The one predicate that says whether `interfaces` above is what a + // process built or what the next launch would build. Clients used to + // re-derive it from `status`, which answers a different question: + // a VM being removed with QEMU still up is not "running" by that + // string, yet its NICs are real. + running: self.running, uptime: self.uptime.clone(), boot_progress: self.boot_progress.clone(), boot_error: self.boot_error.clone(), @@ -261,6 +320,7 @@ impl VmState { workdir: workdir.path().to_path_buf(), instance_id, status, + running: is_running, uptime, exited_at: Some(exited_at), boot_progress: self.state.boot_progress.clone(), @@ -276,7 +336,68 @@ impl VmState { #[cfg(test)] mod tests { - use super::sanitize_optional; + use super::{interfaces_to_proto, networking_to_proto, sanitize_optional}; + use crate::config::{NetworkingMode, NicNetworking}; + + /// Custom mode hands the operator the whole netdev string and the VMM never + /// parses it, so it has no data-plane state to report. Reporting the + /// resolved defaults instead asserted "vhost off, one queue" over a netdev + /// the operator may well have written as `vhost=on,queues=8`. + #[test] + fn a_custom_netdev_reports_no_data_plane_rather_than_the_wrong_one() { + use crate::config::Networking; + + let custom = Networking { + nic: NicNetworking { + mode: NetworkingMode::Custom, + ..NicNetworking::default() + }, + netdev: "tap,id=net0,ifname=custom0,vhost=on,queues=8".into(), + ..Networking::default() + }; + let interfaces = interfaces_to_proto("vm-1", &[custom]); + assert_eq!(interfaces[0].backend, "custom"); + assert_eq!(interfaces[0].vhost, None); + assert_eq!(interfaces[0].queues, None); + + // Every other backend still answers the question. + let bridge = Networking { + nic: NicNetworking { + mode: NetworkingMode::Bridge, + vhost: Some(true), + queues: Some(4), + ..NicNetworking::default() + }, + ..Networking::default() + }; + let interfaces = interfaces_to_proto("vm-1", &[bridge]); + assert_eq!(interfaces[0].vhost, Some(true)); + assert_eq!(interfaces[0].queues, Some(4)); + } + + #[test] + fn a_reported_interface_can_be_sent_back_unchanged() { + // GetInfo output feeds UpdateVm, so anything it reports has to satisfy + // the deployment RPC's own validation. What a VM stores can no longer + // carry a node-owned field at all -- `parent` here belongs to bridge + // mode's own entry only because the type still allows both backends' + // identity fields, and reporting still scopes it to the owning mode. + let networking = NicNetworking { + mode: NetworkingMode::Bridge, + bridge: "br0".into(), + parent: "eth0".into(), + vhost: Some(false), + queues: Some(2), + ..NicNetworking::default() + }; + let proto = networking_to_proto(&networking); + assert_eq!(proto.mode, "bridge"); + assert_eq!(proto.bridge_name, "br0"); + assert!(proto.parent.is_empty()); + assert!(proto.macvtap_mode.is_empty()); + assert_eq!(proto.vhost, Some(false)); + assert_eq!(proto.queues, Some(2)); + } #[test] fn sanitize_optional_filters_empty_owned_values() { diff --git a/dstack/vmm/src/app/workdir.rs b/dstack/vmm/src/app/workdir.rs index 0b358229f..a9822c7ec 100644 --- a/dstack/vmm/src/app/workdir.rs +++ b/dstack/vmm/src/app/workdir.rs @@ -303,7 +303,7 @@ mod tests { let persisted = workdir.runtime_networks(); assert_eq!(persisted.len(), 1); - assert_eq!(persisted[0].parent, "br0"); + assert_eq!(persisted[0].nic.parent, "br0"); assert!(persisted[0].device.is_empty()); assert!(!fs::read_to_string(workdir.runtime_networks_path())?.contains("/dev/tap42")); fs::remove_dir_all(temp)?; diff --git a/dstack/vmm/src/config.rs b/dstack/vmm/src/config.rs index 2e82e67c6..d7479abc4 100644 --- a/dstack/vmm/src/config.rs +++ b/dstack/vmm/src/config.rs @@ -357,6 +357,10 @@ pub struct CvmConfig { pub qemu_pci_hole64_size: u64, /// QEMU hotplug_off pub qemu_hotplug_off: bool, + /// Path to `qemu-bridge-helper`, used to attach an unprivileged TAP to a + /// host bridge. Empty probes the known distribution locations. + #[serde(default)] + pub qemu_bridge_helper: String, /// TDX attestation/hash scheme policy. `legacy` keeps the existing /// digest.txt measurement path; `lite` opts into split measurement CBOR; @@ -384,6 +388,12 @@ pub struct CvmConfig { #[serde(default)] pub allowed_macvtap_parents: Vec, + /// Largest virtio-net queue pair count a deployment RPC caller may request. + /// There is no node-wide count for it to bind; lowering it below the + /// scaling cap does lower the vCPU-scaled default too. + #[serde(default = "default_max_net_queues")] + pub max_net_queues: u32, + /// Optional host-side filtering for bridge interfaces. This filter does /// not apply to macvtap interfaces. #[serde(default)] @@ -592,6 +602,13 @@ pub struct NetworkFilterConfig { pub parameters: BTreeMap, } +impl NetworkFilterConfig { + /// Whether every bridge TAP on this node must carry an nwfilter binding. + pub fn requires_binding(&self) -> bool { + self.mode == NetworkFilterMode::Libvirt + } +} + impl Default for NetworkFilterConfig { fn default() -> Self { Self { @@ -615,6 +632,23 @@ pub struct NetdConfig { pub socket_mode: u32, #[serde(default = "default_libvirt_uri")] pub libvirt_uri: String, + /// The bridge filtering policy netd enforces and applies: whether a binding + /// is required, which nwfilter it names, and with what parameters. + /// + /// netd holds this itself rather than taking it from each request. It is + /// the privileged side of the socket, and a caller that chose the filter + /// could name one that drops nothing -- `allow-arp` has no drop rule at all + /// -- or pin `clean-traffic` to the gateway's MAC and IP through its + /// parameters, and still satisfy a policy that only asked for "some + /// filter". + /// + /// Unset derives it from `cvm.network_filter` in the same file, which is + /// the whole answer whenever netd and the VMM share one `vmm.toml` -- the + /// normal deployment. Set it explicitly when netd runs with a config that + /// carries no `[cvm]` section, so the daemon holding the privilege is never + /// left inferring policy from a file that does not state it. + #[serde(default)] + pub network_filter: Option, } impl Default for NetdConfig { @@ -623,11 +657,21 @@ impl Default for NetdConfig { socket: default_netd_socket(), socket_mode: 0o660, libvirt_uri: default_libvirt_uri(), + network_filter: None, } } } impl NetdConfig { + /// Resolved policy. Unset means the config named no `[cvm]` section to + /// derive it from, and an unfiltered node is the historical shape. + pub fn filter_policy(&self) -> &NetworkFilterConfig { + static UNFILTERED: std::sync::OnceLock = std::sync::OnceLock::new(); + self.network_filter + .as_ref() + .unwrap_or_else(|| UNFILTERED.get_or_init(NetworkFilterConfig::default)) + } + pub fn validate(&self) -> Result<()> { anyhow::ensure!( self.socket_mode & !0o777 == 0, @@ -715,6 +759,29 @@ impl Config { } validate_networking(&self.cvm.networking)?; + // netd creates an unfiltered TAP when the filter name is empty, which + // is what unfiltered multiqueue bridges need. Libvirt mode must never + // reach that path: it would silently produce an unbound TAP where the + // operator asked for a filtered one. + anyhow::ensure!( + self.cvm.network_filter.mode != NetworkFilterMode::Libvirt + || !self.cvm.network_filter.filter.trim().is_empty(), + "cvm.network_filter.filter must not be empty when mode is libvirt" + ); + anyhow::ensure!( + (1..=MAX_NET_QUEUES).contains(&self.cvm.max_net_queues), + "cvm.max_net_queues must be between 1 and {MAX_NET_QUEUES}" + ); + // The helper path is interpolated into QEMU's `-netdev` option list, + // which QEMU splits on ',' and '='. A path carrying either would not be + // passed through, it would end the option and start a bogus one, and the + // launch failure names neither this setting nor the file. Volume sources + // are rejected for the same reason. + anyhow::ensure!( + !self.cvm.qemu_bridge_helper.contains([',', '=']), + "cvm.qemu_bridge_helper must not contain ',' or '=': {}", + self.cvm.qemu_bridge_helper + ); anyhow::ensure!( !self .cvm @@ -828,9 +895,28 @@ fn validate_networking(networking: &Networking) -> Result<()> { "cvm.networking.mac_prefix must contain 1 to 3 two-digit hexadecimal bytes" ); } - match networking.mode { + anyhow::ensure!( + networking.nic.queues.is_none(), + "cvm.networking.queues is not a node setting; queue pairs follow each VM's vCPU count, \ + bounded by cvm.max_net_queues, and a deployment overrides them per NIC" + ); + // Both describe a per-VM entry's relationship to this configuration, so + // neither means anything on the node default itself. + anyhow::ensure!( + !networking.nic.inherit_mode, + "cvm.networking.inherit_mode is per-deployment state and cannot be set on the node default" + ); + anyhow::ensure!( + networking.netd_interface.is_none(), + "cvm.networking.netd_interface is runtime state and cannot be set in configuration" + ); + anyhow::ensure!( + networking.device.is_empty(), + "cvm.networking.device is runtime state and cannot be set in configuration" + ); + match networking.nic.mode { NetworkingMode::Bridge => anyhow::ensure!( - !networking.bridge.trim().is_empty(), + !networking.nic.bridge.trim().is_empty(), "cvm.networking.bridge must not be empty in bridge mode" ), NetworkingMode::Custom => anyhow::ensure!( @@ -839,7 +925,7 @@ fn validate_networking(networking: &Networking) -> Result<()> { ), NetworkingMode::Macvtap => { anyhow::ensure!( - !networking.parent.trim().is_empty(), + !networking.nic.parent.trim().is_empty(), "cvm.networking.parent must not be empty in macvtap mode" ); anyhow::ensure!( @@ -850,6 +936,7 @@ fn validate_networking(networking: &Networking) -> Result<()> { "cvm.networking.macvtap_mode must be private, bridge, vepa, or passthru" ); } + // User mode has no identity fields of its own to check. NetworkingMode::User => {} } Ok(()) @@ -859,20 +946,49 @@ fn default_allowed_network_modes() -> Vec { vec![NetworkingMode::User, NetworkingMode::Bridge] } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)] +fn default_max_net_queues() -> u32 { + DEFAULT_MAX_NET_QUEUES +} + +/// Where the vCPU-scaled default stops growing. Each queue pair costs a host +/// vhost thread and two MSI-X vectors, and cross-vCPU wakeups are expensive +/// under TDX, so the benefit runs out well before a large VM's vCPU count. +/// Raising `cvm.max_net_queues` lets a deployment ask for more; it does not +/// move this, because a bigger VM should not silently get a worse default. +pub const DEFAULT_QUEUE_SCALING_CAP: u32 = 16; + +/// Default ceiling on what a deployment RPC caller may request. +pub const DEFAULT_MAX_NET_QUEUES: u32 = 16; + +/// Hard bound on queue pairs from any source, well below anything QEMU or the +/// guest driver would refuse. It exists so a malformed or hostile request +/// cannot ask the host kernel for an unbounded device, not because 64 is a +/// property of virtio-net. +pub const MAX_NET_QUEUES: u32 = 64; + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Deserialize, Serialize)] #[serde(rename_all = "lowercase")] pub enum NetworkingMode { + /// The backend that needs nothing from the host, so it is what a NIC that + /// names none falls back to. + #[default] User, Bridge, Custom, Macvtap, } -/// Flat networking configuration. The `mode` field selects which backend is -/// active; the remaining fields are only relevant for their respective mode -/// and carry serde defaults so they can be omitted in the config file. -#[derive(Debug, Clone, Deserialize, Serialize)] -pub struct Networking { +/// What a single NIC pins: the fields a deployment may name, a VM's manifest +/// stores, and `GetInfo` reports back. +/// +/// Separate from [`Networking`] because the node's `[cvm.networking]` is not a +/// NIC -- it is a NIC *plus* the backend settings only the node may set. When +/// the two were one type, resolution copied the whole node value into every +/// VM, so a bridge NIC's manifest entry carried whatever macvtap parent the +/// node happened to have configured, and every consumer that asked "what does +/// this VM pin?" had to know which fields to ignore. Several did not. +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize, Serialize)] +pub struct NicNetworking { pub mode: NetworkingMode, // ── Bridge fields ────────────────────────────────────────────── @@ -884,6 +1000,44 @@ pub struct Networking { /// Parent host interface for macvtap (e.g., "eth0"). #[serde(default)] pub parent: String, + + // ── Data plane tuning ────────────────────────────────────────── + /// Move packet processing from the QEMU main loop into the host kernel's + /// vhost-net data plane. `None` inherits the node default. Ignored by the + /// user-mode backend, which has no vhost support, and by custom mode, + /// which owns its whole netdev string. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub vhost: Option, + /// virtio-net queue pairs. `None` scales with the VM's vCPU count. Only a + /// deployment sets this; there is no node-wide value, because the useful + /// number depends on the VM rather than the host. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub queues: Option, + + // ── Ownership markers ────────────────────────────────────────── + /// Take `mode` from node configuration at every launch instead of from + /// this entry. + /// + /// A deployment that only tunes the data plane never named a backend, so + /// the node still owns which one this NIC uses. `mode` is not an `Option` + /// -- every consumer matches on it -- so the entry carries the node's + /// current mode and this flag says not to trust it across a node + /// configuration change. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub inherit_mode: bool, +} + +/// `[cvm.networking]`, and the resolved value a launch hands to QEMU: one NIC +/// plus the backend settings that belong to the node rather than to any VM. +/// +/// Resolution produces this same type because a launch needs both halves; what +/// it must never do is hand the node half back to a VM to store. +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize, Serialize)] +pub struct Networking { + #[serde(flatten)] + pub nic: NicNetworking, + + // ── Macvtap fields ──────────────────────────────────────────── /// macvtap forwarding mode. Empty selects "private". #[serde(default)] pub macvtap_mode: String, @@ -908,11 +1062,108 @@ pub struct Networking { // ── Custom fields ────────────────────────────────────────────── #[serde(default)] pub netdev: String, + + // ── Runtime markers ──────────────────────────────────────────── + /// What netd built for this NIC, recorded when it was built. + /// + /// Runtime state, like `device`: resolution always clears it. Teardown + /// reads this rather than re-deriving it from node configuration, because + /// an operator may change `network_filter.mode` or `max_net_queues` while + /// the VM runs, and what has to be removed is what was created. + #[serde(default, skip_serializing_if = "NetdInterface::is_none")] + pub netd_interface: NetdInterface, +} + +/// The host interface netd created for a NIC, if any. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Deserialize, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum NetdInterface { + /// netd was not involved: user mode, custom mode, or a bridge NIC that + /// QEMU's own bridge helper attaches. + #[default] + None, + /// netd created the interface and bound no libvirt nwfilter to it. + Unfiltered, + /// netd created the interface and bound a libvirt nwfilter to it, which + /// removal has to delete before the interface goes away. + Filtered, +} + +impl NetdInterface { + pub fn is_none(&self) -> bool { + matches!(self, NetdInterface::None) + } + + pub fn is_filtered(&self) -> bool { + matches!(self, NetdInterface::Filtered) + } } impl Networking { pub fn is_bridge(&self) -> bool { - self.mode == NetworkingMode::Bridge + self.nic.mode == NetworkingMode::Bridge + } + + /// Whether the vhost-net data plane applies to this interface. + /// + /// Defaults to disabled: an upgraded node keeps the exact device shape its + /// VMs booted with (one queue pair, userspace virtio) until the operator + /// opts in, because `vhost = true` requires `/dev/vhost-net` to be + /// accessible to the account QEMU runs under — a precondition the VMM + /// cannot verify on the operator's behalf. + pub fn vhost_enabled(&self) -> bool { + self.nic.vhost.unwrap_or(false) && self.supports_vhost() + } + + /// Whether the backend selected by `mode` can carry a vhost-net data plane + /// at all. Custom mode is excluded because the operator supplies the whole + /// netdev string, including any vhost options. + pub fn supports_vhost(&self) -> bool { + Self::mode_supports_vhost(self.nic.mode) + } + + /// The same question about a mode on its own, for a caller deciding + /// whether a request it has not built an entry for yet can be honoured. + pub fn mode_supports_vhost(mode: NetworkingMode) -> bool { + matches!(mode, NetworkingMode::Bridge | NetworkingMode::Macvtap) + } + + /// Whether the backend selected by `mode` can carry more than one queue + /// pair. + /// + /// Custom mode is excluded for the same reason as vhost: the operator + /// supplies the whole netdev string and the VMM cannot edit it, so a + /// multiqueue device line would have nothing to pair with. The RPC refuses + /// such a request, but a node that switches its default to custom must not + /// be able to produce one behind the RPC's back. + pub fn supports_multiqueue(&self) -> bool { + Self::mode_supports_multiqueue(self.nic.mode) + } + + /// The same question about a mode on its own, for a caller deciding + /// whether a request it has not built an entry for yet can be honoured. + pub fn mode_supports_multiqueue(mode: NetworkingMode) -> bool { + matches!(mode, NetworkingMode::Bridge | NetworkingMode::Macvtap) + } + + /// Effective virtio-net queue pair count of a resolved NIC, never below + /// one. Resolution makes the vCPU-scaled default concrete, so an entry that + /// still carries none is read conservatively as single-queue. + pub fn queue_pairs(&self) -> u32 { + if !self.supports_multiqueue() { + return 1; + } + self.nic.queues.unwrap_or(1).max(1) + } + + /// Queue pairs a VM with this many vCPUs gets when it asks for none. + /// + /// The guest driver uses at most one queue pair per vCPU, so the default + /// follows the vCPU count up to a fixed cap. A node that lowers + /// `max_net_queues` below that cap means it, so the default follows it + /// down; raising it above the cap only widens what a caller may request. + pub fn default_queue_pairs(vcpu: u32, max_net_queues: u32) -> u32 { + vcpu.clamp(1, DEFAULT_QUEUE_SCALING_CAP.min(max_net_queues).max(1)) } /// Parse the mac_prefix into bytes. Returns 0-3 bytes. @@ -1198,6 +1449,31 @@ mod tests { .expect("default VMM config should parse") } + /// The two ownership markers are additive on disk: manifests and runtime + /// network snapshots written before they existed still load, and an entry + /// that carries neither serializes exactly as it used to. + #[test] + fn ownership_markers_are_omitted_when_unset_and_default_when_absent() { + let mut networking: Networking = + serde_json::from_str(r#"{"mode":"bridge","bridge":"br0"}"#).unwrap(); + assert!(!networking.nic.inherit_mode); + assert_eq!(networking.netd_interface, NetdInterface::None); + + let json = serde_json::to_string(&networking).unwrap(); + assert!(!json.contains("inherit_mode"), "{json}"); + assert!(!json.contains("netd_interface"), "{json}"); + + networking.nic.inherit_mode = true; + networking.netd_interface = NetdInterface::Filtered; + let json = serde_json::to_string(&networking).unwrap(); + assert!(json.contains(r#""inherit_mode":true"#), "{json}"); + assert!(json.contains(r#""netd_interface":"filtered""#), "{json}"); + assert_eq!( + serde_json::from_str::(&json).unwrap(), + networking + ); + } + #[test] fn config_validation_accepts_defaults() { let config = default_config(); @@ -1234,6 +1510,42 @@ mod tests { .to_string() .contains("range start")); + // An empty filter tells netd to create an unfiltered TAP, so libvirt + // mode must never carry one. + let mut config = default_config(); + config.cvm.network_filter.mode = NetworkFilterMode::Libvirt; + config.cvm.network_filter.filter = String::new(); + assert!(config + .validate() + .unwrap_err() + .to_string() + .contains("network_filter.filter")); + + let mut config = default_config(); + config.cvm.max_net_queues = 0; + assert!(config + .validate() + .unwrap_err() + .to_string() + .contains("max_net_queues")); + + let mut config = default_config(); + config.cvm.max_net_queues = MAX_NET_QUEUES + 1; + assert!(config + .validate() + .unwrap_err() + .to_string() + .contains("max_net_queues")); + + // The node-wide value is gone; say so rather than ignoring it. + let mut config = default_config(); + config.cvm.networking.nic.queues = Some(4); + assert!(config + .validate() + .unwrap_err() + .to_string() + .contains("cvm.networking.queues is not a node setting")); + let mut config = default_config(); config.cvm.networking.mac_prefix = "02:not-hex".into(); assert!(config @@ -1270,8 +1582,8 @@ mod tests { .contains("supervisor.sock")); let mut config = default_config(); - config.cvm.networking.mode = NetworkingMode::Bridge; - config.cvm.networking.bridge.clear(); + config.cvm.networking.nic.mode = NetworkingMode::Bridge; + config.cvm.networking.nic.bridge.clear(); assert!(config .validate() .unwrap_err() @@ -1328,3 +1640,89 @@ mod tests { assert_eq!(CvmPlatform::resolve_from_cpuinfo(cpuinfo), CvmPlatform::Tdx); } } + +#[cfg(test)] +mod networking_shape_tests { + use super::{Config, Networking, NetworkingMode, NicNetworking, DEFAULT_CONFIG}; + + /// Splitting the type must not split the wire format. `[cvm.networking]` + /// is flattened, so a node config, a stored manifest and a runtime-networks + /// snapshot written by an earlier build all still parse, and what this + /// build writes is byte-for-byte what the old one did. + #[test] + fn the_split_types_keep_one_flat_serialized_shape() { + let legacy = serde_json::json!({ + "mode": "bridge", + "bridge": "br0", + "parent": "eth0", + "macvtap_mode": "private", + "device": "/dev/tap7", + "mac_prefix": "02:aa:bb", + "net": "10.0.2.0/24", + "dhcp_start": "10.0.2.15", + "restrict": true, + "netdev": "", + "vhost": false, + "queues": 4, + "inherit_mode": true, + "netd_interface": "filtered", + }); + + // A resolved value keeps every field, at the same names as before. + let resolved: Networking = serde_json::from_value(legacy.clone()).unwrap(); + assert_eq!(resolved.nic.mode, NetworkingMode::Bridge); + assert_eq!(resolved.nic.bridge, "br0"); + assert_eq!(resolved.nic.queues, Some(4)); + assert!(resolved.nic.inherit_mode); + assert_eq!(resolved.macvtap_mode, "private"); + assert_eq!(resolved.net, "10.0.2.0/24"); + assert!(resolved.restrict); + let round_tripped = serde_json::to_value(&resolved).unwrap(); + assert_eq!(round_tripped["mode"], "bridge"); + assert_eq!(round_tripped["macvtap_mode"], "private"); + assert_eq!(round_tripped["queues"], 4); + + // A manifest entry written by a build that stored the whole thing + // still loads; the node's half is simply dropped on the way in. + let pinned: NicNetworking = serde_json::from_value(legacy).unwrap(); + assert_eq!(pinned.bridge, "br0"); + assert_eq!(pinned.parent, "eth0"); + assert_eq!(pinned.vhost, Some(false)); + let stored = serde_json::to_value(&pinned).unwrap(); + assert_eq!(stored.as_object().unwrap().len(), 6); + assert!(stored.get("macvtap_mode").is_none()); + assert!(stored.get("net").is_none()); + } + + /// The path is interpolated into QEMU's `-netdev` option list, which QEMU + /// splits on ',' and '='. A path carrying either would end the option and + /// start a bogus one, and the launch failure names neither the setting nor + /// the file. + #[test] + fn a_bridge_helper_path_cannot_end_the_qemu_option_it_sits_in() { + use rocket::figment::providers::Format as _; + let mut config: Config = rocket::figment::Figment::from( + rocket::figment::providers::Toml::string(DEFAULT_CONFIG), + ) + .extract() + .unwrap(); + config.cvm.qemu_bridge_helper = "/opt/qemu,helper".into(); + let error = config.validate().unwrap_err(); + assert!(error.to_string().contains("qemu_bridge_helper"), "{error}"); + + config.cvm.qemu_bridge_helper = "/usr/libexec/qemu-bridge-helper".into(); + config.validate().unwrap(); + } + + /// The TOML section still deserializes through the flatten. + #[test] + fn the_node_section_still_parses_from_toml() { + use rocket::figment::providers::Format as _; + let config: Config = rocket::figment::Figment::from( + rocket::figment::providers::Toml::string(DEFAULT_CONFIG), + ) + .extract() + .unwrap(); + assert_eq!(config.cvm.networking.nic.mode, NetworkingMode::User); + } +} diff --git a/dstack/vmm/src/main.rs b/dstack/vmm/src/main.rs index 55099e81b..17b93d194 100644 --- a/dstack/vmm/src/main.rs +++ b/dstack/vmm/src/main.rs @@ -219,6 +219,22 @@ async fn main() -> Result<()> { if let Some(socket) = netd_args.socket.as_deref() { netd_config.socket = socket.into(); } + if netd_config.network_filter.is_none() { + // netd and the VMM normally share one vmm.toml, so the node has + // already stated whether its bridge traffic is filtered and with + // what. Reading it here keeps the two from drifting apart, which is + // what a second setting to keep in sync would invite. + // + // A malformed section is an error rather than a default: this is + // the daemon's security policy, and `[cvm.network_filter] mode = + // "Libvirt"` -- which the VMM itself refuses to start on -- must not + // quietly resolve to "filter nothing" here. + netd_config.network_filter = Some( + figment + .extract_inner("cvm.network_filter") + .context("failed to load [cvm.network_filter] for netd")?, + ); + } return netd::serve(netd_config).await; } diff --git a/dstack/vmm/src/main_service.rs b/dstack/vmm/src/main_service.rs index 931a373ae..6f9a0905d 100644 --- a/dstack/vmm/src/main_service.rs +++ b/dstack/vmm/src/main_service.rs @@ -28,7 +28,7 @@ use crate::app::{ needs_swtpm, resolve_networking, validate_resolved_network, validate_resolved_networks, App, AttachMode, GpuConfig, GpuSpec, Manifest, PortMapping, VmWorkDir, }; -use crate::config::{CvmConfig, Networking, NetworkingMode}; +use crate::config::{CvmConfig, Networking, NetworkingMode, NicNetworking}; fn hex_sha256(data: &str) -> String { use sha2::Digest; @@ -352,18 +352,37 @@ fn resolve_volume_source(base: &Path, source: &str) -> Result { fn networking_from_proto( proto: &rpc::NetworkingConfig, cvm_config: &CvmConfig, -) -> Result> { +) -> Result> { let bridge = proto.bridge_name.trim().to_string(); - let mode = match proto.mode.as_str() { - "bridge" => NetworkingMode::Bridge, - "user" => NetworkingMode::User, - "macvtap" => NetworkingMode::Macvtap, - "" if bridge.is_empty() => return Ok(None), - "" => bail!("networking mode is required when bridge is set"), + let parent = proto.parent.trim().to_string(); + // Checked before anything reads `queues`, because the "no override at all" + // arm below returns early: a request of `{queues: 0}` and nothing else + // would otherwise be answered with the vCPU-scaled default -- up to + // sixteen queue pairs -- for a caller who asked for none. + if proto.queues == Some(0) { + bail!("networking queues must be at least 1, or unset to follow the vCPU count"); + } + let tuned = proto.vhost.is_some() || proto.queues.is_some(); + // Naming a bridge or a macvtap parent is naming a backend, and a backend + // needs a mode to go with it. Without one the entry would freeze whichever + // mode the node happened to have, then report a mode-less override still + // carrying a field only one mode accepts -- something nothing can send + // back once the node moves on. + let names_backend = !bridge.is_empty() || !parent.is_empty(); + // A request that only tunes the data plane keeps the node's backend. Node + // policy governs which backend a caller may *choose*, so inheriting one + // must not be denied by it. + let (mode, chosen) = match proto.mode.as_str() { + "bridge" => (NetworkingMode::Bridge, true), + "user" => (NetworkingMode::User, true), + "macvtap" => (NetworkingMode::Macvtap, true), + "" if !names_backend && !tuned => return Ok(None), + "" if !names_backend => (cvm_config.networking.nic.mode, false), + "" => bail!("networking mode is required when a bridge or macvtap parent is set"), "custom" => bail!("custom networking mode is manifest-only"), other => bail!("unsupported networking mode '{other}'"), }; - if !cvm_config.allowed_network_modes.contains(&mode) { + if chosen && !cvm_config.allowed_network_modes.contains(&mode) { bail!( "networking mode '{}' is not allowed by node policy", proto.mode @@ -372,45 +391,161 @@ fn networking_from_proto( if mode != NetworkingMode::Bridge && !bridge.is_empty() { bail!("bridge_name is only valid for bridge networking mode"); } - if mode != NetworkingMode::Macvtap && !proto.parent.trim().is_empty() { + if mode != NetworkingMode::Macvtap && !parent.is_empty() { bail!("parent is only valid for macvtap networking mode"); } - if !proto.macvtap_mode.trim().is_empty() { + // `GetInfo` reports the resolved node defaults, and both vmm-cli and the + // web UI read that, change one field, and send the rest back. Naming a + // value the node would have supplied anyway therefore has to be accepted: + // leaving the field empty already yields exactly it, so echoing it grants + // nothing that policy was withholding. + // Node values are compared trimmed, the way the request's are: node + // configuration validation tolerates surrounding whitespace, and a value + // resolution would supply must not become unsendable over a space. + let node = &cvm_config.networking; + let macvtap_mode = proto.macvtap_mode.trim(); + if !macvtap_mode.is_empty() && macvtap_mode != node.macvtap_mode.trim() { bail!("macvtap_mode is node-controlled and cannot be set by deployment RPCs"); } - if !bridge.is_empty() && !cvm_config.allowed_bridges.contains(&bridge) { + if !bridge.is_empty() + && bridge != node.nic.bridge.trim() + && !cvm_config.allowed_bridges.contains(&bridge) + { bail!("bridge_name '{bridge}' is not allowed by node policy"); } - let parent = proto.parent.trim().to_string(); - if !parent.is_empty() && !cvm_config.allowed_macvtap_parents.contains(&parent) { + if !parent.is_empty() + && parent != node.nic.parent.trim() + && !cvm_config.allowed_macvtap_parents.contains(&parent) + { bail!("macvtap parent '{parent}' is not allowed by node policy"); } - Ok(Some(Networking { + // Same rule as the queue count below: a backend the caller chose and that + // has no vhost data plane is a request they can fix, so say so. An + // inherited one is not, and reads as off until the node moves. + if chosen && proto.vhost == Some(true) && !Networking::mode_supports_vhost(mode) { + bail!("{} networking has no vhost data plane", proto.mode); + } + // Queue pairs cost a host vhost thread and a pair of MSI-X vectors each, so + // the node caps what a deployment may ask for. + // + let queues = proto.queues; + if let Some(queues) = queues { + if queues > cvm_config.max_net_queues { + bail!( + "networking queues must not exceed {} on this node", + cvm_config.max_net_queues + ); + } + // Only a backend the caller *chose* is theirs to be wrong about. One + // they inherited can change under them -- that is the point of + // inheriting -- and refusing the request afterwards would strand the + // VM: GetInfo would keep reporting a queue count that nothing is + // allowed to send back. `queue_pairs()` already reads as one on both + // of these, so the request simply lies dormant until the node moves to + // a backend that can honour it. + if chosen && queues > 1 && !Networking::mode_supports_multiqueue(mode) { + bail!("{} networking does not support multiple queues", proto.mode); + } + } + // Every field the node owns -- the macvtap forwarding mode, the MAC prefix, + // the user-mode network parameters, a custom netdev string -- is absent by + // construction now rather than by remembering to write `String::new()` for + // each of them. + Ok(Some(NicNetworking { mode, + // The caller named no backend, so the node keeps owning which one this + // NIC uses. `mode` above is only the node's current choice. + inherit_mode: !chosen, bridge, parent, - // The forwarding mode is always inherited from node configuration. - macvtap_mode: String::new(), - device: String::new(), - mac_prefix: String::new(), - net: String::new(), - dhcp_start: String::new(), - restrict: false, - netdev: String::new(), + vhost: proto.vhost, + queues, })) } +/// Networking modes a client should offer. +/// +/// A mode the host could serve but node policy forbids is not one of them: +/// offering it puts a choice in the deploy dialog whose only outcome is "not +/// allowed by node policy", with nothing for the operator to do about it. A VM +/// can still be given a data plane override without naming any mode, which is +/// how a node whose own backend is not caller-selectable stays tunable. +fn advertised_modes(cvm_config: &CvmConfig, host_can_bridge: bool) -> Vec { + [ + (NetworkingMode::User, "user", true), + (NetworkingMode::Bridge, "bridge", host_can_bridge), + (NetworkingMode::Macvtap, "macvtap", true), + ] + .into_iter() + .filter(|(mode, _, host_supports)| { + *host_supports && cvm_config.allowed_network_modes.contains(mode) + }) + .map(|(_, name, _)| name.to_string()) + .collect() +} + +/// The node's policy, widened by what this VM's own NICs already pin. +/// +/// Deployment allowlists govern what a caller may *newly* select. A value one +/// of this VM's interfaces is already running on was selected when it was +/// still allowed, and it is reported back on every `GetInfo`; refusing it +/// would strand the VM rather than withhold anything. +fn held_networking_config(cvm_config: &CvmConfig, held: &[NicNetworking]) -> CvmConfig { + let mut widened = cvm_config.clone(); + for networking in held { + // Only a field the entry's own mode owns. Resolution starts from the + // node's whole `[cvm.networking]` value, so a bridge entry also carries + // whatever macvtap parent the node happened to have configured, and a + // macvtap entry carries its bridge -- baggage the VM never used. Reading + // those as "already running on it" would widen policy from a value + // nothing ever attached to, and let a VM move to a backend the node + // forbids. + if networking.mode == NetworkingMode::Bridge && !networking.bridge.is_empty() { + widened.allowed_bridges.push(networking.bridge.clone()); + } + if networking.mode == NetworkingMode::Macvtap && !networking.parent.is_empty() { + widened + .allowed_macvtap_parents + .push(networking.parent.clone()); + } + // Same rule for the queue cap. Lowering `max_net_queues` bounds what a + // deployment may newly ask for; it does not retroactively re-tune VMs + // that are already pinned above it, and those keep reporting their + // count on every `GetInfo`. Without this, lowering the cap makes every + // networking update on such a VM fail on a field the operator never + // typed -- including the `--net-queues auto` that would unpin it. + if let Some(queues) = networking.queues { + widened.max_net_queues = widened.max_net_queues.max(queues); + } + } + widened +} + +/// A NIC that overrides nothing: whatever the node's `[cvm.networking]` says, +/// now and after the operator changes it. +fn node_default_networking(cvm_config: &CvmConfig) -> NicNetworking { + NicNetworking { + mode: cvm_config.networking.nic.mode, + inherit_mode: true, + ..NicNetworking::default() + } +} + fn network_from_required_proto( proto: &rpc::NetworkingConfig, cvm_config: &CvmConfig, -) -> Result { - networking_from_proto(proto, cvm_config)?.context("networking mode is required") +) -> Result { + // An entry in a list that overrides nothing is not a missing mode: it is a + // NIC that follows the node entirely. Only the singular `networking` field + // can mean "no override at all", because there the absence is the message. + Ok(networking_from_proto(proto, cvm_config)? + .unwrap_or_else(|| node_default_networking(cvm_config))) } fn networks_from_proto( networks: &[rpc::NetworkingConfig], cvm_config: &CvmConfig, -) -> Result> { +) -> Result> { networks .iter() .map(|network| network_from_required_proto(network, cvm_config)) @@ -422,15 +557,68 @@ fn validate_default_network(cvm_config: &CvmConfig) -> Result<()> { } fn resolve_requested_networks( - networks: &[Networking], + requests: &[NicNetworking], cvm_config: &CvmConfig, -) -> Result> { - let resolved = networks + vcpu: u32, +) -> Result> { + let merged = requests .iter() - .map(|networking| resolve_networking(networking, cvm_config)) + .map(|request| resolve_networking(request, cvm_config, vcpu)) .collect::>(); - validate_resolved_networks(&resolved)?; - Ok(resolved) + // Validate the merged view, because that is what the launch sees, then + // record the narrower view the manifest keeps. + validate_resolved_networks(&merged)?; + Ok(manifest_networks(merged, requests)) +} + +/// What a deployment records against the VM, given the merged view its launch +/// would see. +/// +/// The backend a deployment *chose* is pinned here -- its mode, and the bridge +/// or macvtap parent that names it -- so a VM keeps the segment it was put on +/// for life. Everything else stays owned by the node and is re-read at every +/// launch: the MAC prefix, the user-mode subnet and DHCP start, the macvtap +/// forwarding mode, and the data plane an operator may want to roll back +/// node-wide. A VM's own record cannot carry any of those. +fn manifest_networks(merged: Vec, requests: &[NicNetworking]) -> Vec { + merged + .into_iter() + .zip(requests) + .map(|(entry, request)| { + if request.inherit_mode { + // The caller named no backend, so none of the node's matching + // identity fields are theirs to keep. + return request.clone(); + } + // Taking `entry.nic` rather than the whole resolved value is what + // keeps the node's half out of the VM's record; it used to take the + // whole thing and then clear the one node field anybody noticed. + // + // The two identity fields still share one type, and resolution + // fills both from the node, so scope them here -- once, where the + // record is written -- rather than leaving every later reader to + // remember which one its mode owns. Two already have to. + let mode = entry.nic.mode; + NicNetworking { + bridge: if mode == NetworkingMode::Bridge { + entry.nic.bridge + } else { + String::new() + }, + parent: if mode == NetworkingMode::Macvtap { + entry.nic.parent + } else { + String::new() + }, + // Data plane tuning is not identity: leave what the caller did + // not ask for unset. + vhost: request.vhost, + queues: request.queues, + mode, + inherit_mode: entry.nic.inherit_mode, + } + }) + .collect() } fn has_host_bridge_interface() -> bool { @@ -445,13 +633,13 @@ fn has_host_bridge_interface() -> bool { fn networks_from_vm_config( request: &VmConfiguration, cvm_config: &CvmConfig, -) -> Result> { +) -> Result> { if !request.networks.is_empty() { let networks = networks_from_proto(&request.networks, cvm_config)?; - resolve_requested_networks(&networks, cvm_config) + resolve_requested_networks(&networks, cvm_config, request.vcpu) } else if let Some(networking) = request.networking.as_ref() { match networking_from_proto(networking, cvm_config)? { - Some(networking) => resolve_requested_networks(&[networking], cvm_config), + Some(networking) => resolve_requested_networks(&[networking], cvm_config, request.vcpu), None => Ok(vec![]), } } else { @@ -747,8 +935,14 @@ impl VmmRpc for RpcHandler { validate_default_network(&self.app.config.cvm)?; vec![] } else { - let networks = networks_from_proto(&request.networks, &self.app.config.cvm)?; - resolve_requested_networks(&networks, &self.app.config.cvm)? + // A bridge or parent this VM already holds is not a new grant. + // `GetInfo` keeps reporting it, and read-modify-write keeps + // sending it back, so refusing it once the node changes its own + // default would make the VM's configuration unsendable -- with + // no flag anywhere to clear a field the caller never typed. + let cvm = held_networking_config(&self.app.config.cvm, &manifest.networks); + let networks = networks_from_proto(&request.networks, &cvm)?; + resolve_requested_networks(&networks, &cvm, manifest.vcpu)? }; let is_running = self .app @@ -859,14 +1053,12 @@ impl VmmRpc for RpcHandler { } async fn get_meta(self) -> Result { - let mut supported_modes = vec!["user".to_string()]; let default_networking = &self.app.config.cvm.networking; let mut bridge_networking = default_networking.clone(); - bridge_networking.mode = NetworkingMode::Bridge; - if validate_resolved_network(&bridge_networking).is_ok() || has_host_bridge_interface() { - supported_modes.push("bridge".to_string()); - } - supported_modes.push("macvtap".to_string()); + bridge_networking.nic.mode = NetworkingMode::Bridge; + let host_can_bridge = + validate_resolved_network(&bridge_networking).is_ok() || has_host_bridge_interface(); + let supported_modes = advertised_modes(&self.app.config.cvm, host_can_bridge); Ok(GetMetaResponse { kms: Some(KmsSettings { url: self @@ -900,13 +1092,15 @@ impl VmmRpc for RpcHandler { }), networking: Some(rpc::NetworkingCapabilities { supported_modes, - default_mode: match default_networking.mode { + default_mode: match default_networking.nic.mode { NetworkingMode::User => "user".to_string(), NetworkingMode::Bridge => "bridge".to_string(), NetworkingMode::Custom => String::new(), NetworkingMode::Macvtap => "macvtap".to_string(), }, - default_bridge: default_networking.bridge.clone(), + default_bridge: default_networking.nic.bridge.clone(), + max_queues: self.app.config.cvm.max_net_queues, + default_vhost: default_networking.vhost_enabled(), }), }) } @@ -1282,7 +1476,9 @@ mod tests { assert_eq!(manifest.networks.len(), 1); assert_eq!(manifest.networks[0].mode, NetworkingMode::User); - assert!(!manifest.networks[0].net.is_empty()); + // The node's user-mode network parameters are not the VM's to store; + // the type it stores cannot carry them at all now. + assert!(manifest.networks[0].bridge.is_empty()); } #[test] @@ -1292,7 +1488,7 @@ mod tests { .allowed_network_modes .push(NetworkingMode::Macvtap); cvm_config.allowed_macvtap_parents.push("eth0".to_string()); - cvm_config.networking.parent = "node-default".to_string(); + cvm_config.networking.nic.parent = "node-default".to_string(); cvm_config.networking.macvtap_mode = "private".to_string(); let networks = networks_from_proto( &[rpc::NetworkingConfig { @@ -1306,11 +1502,789 @@ mod tests { assert_eq!(networks[0].mode, NetworkingMode::Macvtap); assert_eq!(networks[0].parent, "eth0"); - assert!(networks[0].macvtap_mode.is_empty()); - let resolved = resolve_requested_networks(&networks, &cvm_config).unwrap(); - assert_eq!(resolved[0].parent, "eth0"); - assert_eq!(resolved[0].macvtap_mode, "private"); + // The manifest keeps the parent, which is identity. The forwarding mode + // the node owns and supplies at every launch is not a field this type + // has. + let stored = resolve_requested_networks(&networks, &cvm_config, 4).unwrap(); + assert_eq!(stored[0].parent, "eth0"); + + let at_launch = resolve_networking(&stored[0], &cvm_config, 4); + assert_eq!(at_launch.nic.parent, "eth0"); + assert_eq!(at_launch.macvtap_mode, "private"); + + // Repointing the node's forwarding mode reaches the VM. + cvm_config.networking.macvtap_mode = "bridge".to_string(); + assert_eq!( + resolve_networking(&stored[0], &cvm_config, 4).macvtap_mode, + "bridge" + ); + } + + /// Node shapes a deployment can be reported against, each with the node + /// default fields that mode populates. + fn node_shapes() -> Vec<(&'static str, CvmConfig)> { + let mut bridge = test_cvm_config(); + bridge.networking.nic.mode = NetworkingMode::Bridge; + bridge.networking.nic.bridge = "br-node".into(); + + let mut macvtap = test_cvm_config(); + macvtap.networking.nic.mode = NetworkingMode::Macvtap; + macvtap.networking.nic.parent = "eth-node".into(); + macvtap.networking.macvtap_mode = "private".into(); + macvtap.allowed_network_modes.push(NetworkingMode::Macvtap); + + let user = test_cvm_config(); + vec![ + ("bridge node", bridge), + ("macvtap node", macvtap), + ("user node", user), + ] + } + + /// Every override a caller can express, including the tuning-only shape + /// that names no backend. + fn request_shapes(node: &CvmConfig) -> Vec<(String, rpc::NetworkingConfig)> { + let mode = networking_mode_name_for_test(node.networking.nic.mode); + // The user-mode backend has neither multiqueue nor vhost, and naming it + // and then asking for either is a refusal rather than a round-trip + // failure. Asking to turn vhost off is always legal. + let multiqueue = Networking::mode_supports_multiqueue(node.networking.nic.mode); + let vhost_on = Networking::mode_supports_vhost(node.networking.nic.mode).then_some(true); + // Naming a backend's identity field is only legal alongside a mode, so + // it varies with the named case. The node's own values are used because + // that is what GetInfo reports back. + let identity: &[(&str, &str, &str)] = &[ + ("", "", ""), + ("+ bridge", node.networking.nic.bridge.as_str(), ""), + ("+ parent", "", node.networking.nic.parent.as_str()), + ]; + let mut shapes = vec![]; + for (named, mode) in [("named", mode.to_string()), ("inherited", String::new())] { + for (tuning, vhost, queues) in [ + ("untuned", None, None), + ("vhost off", Some(false), None), + ("queues", None, multiqueue.then_some(2)), + ("both", vhost_on, multiqueue.then_some(2)), + ] { + for (label, bridge, parent) in identity { + // An inherited entry may not name a backend at all, which + // is a refusal covered by its own test. + let inherited = mode.is_empty(); + if inherited && !(bridge.is_empty() && parent.is_empty()) { + continue; + } + // Only the mode that owns a field may carry it. + let bridge_ok = node.networking.nic.mode == NetworkingMode::Bridge; + let parent_ok = node.networking.nic.mode == NetworkingMode::Macvtap; + if (!bridge.is_empty() && !bridge_ok) || (!parent.is_empty() && !parent_ok) { + continue; + } + shapes.push(( + format!("{named} + {tuning} {label}"), + rpc::NetworkingConfig { + mode: mode.clone(), + bridge_name: bridge.to_string(), + parent: parent.to_string(), + vhost, + queues, + ..Default::default() + }, + )); + } + } + } + shapes + } + + /// A backend's identity field without a mode would freeze whichever mode + /// the node had at deploy time, and then be reported alongside an empty + /// mode -- a combination the RPC itself rejects, which would leave the VM's + /// tuning permanently uneditable. + #[test] + fn an_inherited_entry_may_not_name_a_backend() { + let mut cvm = test_cvm_config(); + cvm.networking.nic.mode = NetworkingMode::Macvtap; + cvm.networking.nic.parent = "eth-node".into(); + cvm.allowed_macvtap_parents.push("eth1".into()); + cvm.allowed_network_modes.push(NetworkingMode::Macvtap); + + let err = networking_from_proto( + &rpc::NetworkingConfig { + parent: "eth1".into(), + queues: Some(2), + ..Default::default() + }, + &cvm, + ) + .unwrap_err(); + assert!(err.to_string().contains("networking mode is required")); + + // Naming the mode alongside it is fine. + networking_from_proto( + &rpc::NetworkingConfig { + mode: "macvtap".into(), + parent: "eth1".into(), + queues: Some(2), + ..Default::default() + }, + &cvm, + ) + .unwrap() + .expect("a named backend is an override"); + } + + fn networking_mode_name_for_test(mode: NetworkingMode) -> &'static str { + match mode { + NetworkingMode::Bridge => "bridge", + NetworkingMode::User => "user", + NetworkingMode::Macvtap => "macvtap", + NetworkingMode::Custom => "custom", + } + } + + /// `GetInfo` reports the configuration that `UpdateVm` and `UpgradeApp` + /// take back, and both vmm-cli and the web UI read it, change one field, + /// and resend the rest. So everything reportable has to be acceptable, and + /// accepting it has to land on the same VM. + /// + /// This asserts the property over every mode and tuning combination on + /// purpose. Asserting one shape is how an inherited `parent` on a bridge + /// NIC, and then `macvtap_mode` and `bridge_name`, each reached a release: + /// every one of them was a case the fixed example did not cover. + #[test] + fn everything_get_info_reports_is_accepted_back_unchanged() { + for (node_label, cvm) in node_shapes() { + for (shape_label, request) in request_shapes(&cvm) { + let case = format!("{node_label} / {shape_label}"); + let Some(requested) = networking_from_proto(&request, &cvm) + .unwrap_or_else(|error| panic!("{case}: deployment rejected: {error:#}")) + else { + // No override at all; nothing is recorded, nothing to report. + continue; + }; + // Skip the host-dependent bridge existence check: this is + // about what the RPC reports versus what it accepts. + let merged = resolve_networking(&requested, &cvm, 4); + let stored = manifest_networks(vec![merged.clone()], &[requested]); + + let reported = crate::app::networking_to_proto(&stored[0]); + let accepted = networking_from_proto(&reported, &cvm) + .unwrap_or_else(|error| { + panic!("{case}: GetInfo output was rejected on the way back: {error:#}") + }) + .unwrap_or_else(|| panic!("{case}: the override was lost in the round trip")); + + let restored = + manifest_networks(vec![resolve_networking(&accepted, &cvm, 4)], &[accepted]); + assert_eq!(stored, restored, "{case}: round trip changed the manifest"); + assert_eq!( + resolve_networking(&restored[0], &cvm, 4), + merged, + "{case}: round trip changed what the launch sees" + ); + } + } + } + + /// A VM must not become uneditable because its node moved somewhere its + /// tuning does not apply. The report and the deployment RPC have to agree + /// on every backend the node can be pointed at, not just the one it had + /// when the VM was deployed. + #[test] + fn an_inherited_override_still_round_trips_after_the_node_moves() { + let mut cvm = test_cvm_config(); + cvm.networking.nic.mode = NetworkingMode::Bridge; + cvm.networking.nic.bridge = "br-node".into(); + + let requested = networking_from_proto( + &rpc::NetworkingConfig { + queues: Some(4), + ..Default::default() + }, + &cvm, + ) + .unwrap() + .expect("tuning must produce an override"); + let stored = manifest_networks(vec![resolve_networking(&requested, &cvm, 8)], &[requested]); + + for mode in [ + NetworkingMode::User, + NetworkingMode::Custom, + NetworkingMode::Macvtap, + NetworkingMode::Bridge, + ] { + cvm.networking.nic.mode = mode; + cvm.networking.nic.parent = "eth-node".into(); + cvm.networking.netdev = "tap,id=net0,ifname=custom0".into(); + + let reported = crate::app::networking_to_proto(&stored[0]); + let accepted = networking_from_proto(&reported, &cvm) + .unwrap_or_else(|error| panic!("{mode:?}: report was rejected: {error:#}")) + .unwrap_or_else(|| panic!("{mode:?}: the override was lost")); + let restored = + manifest_networks(vec![resolve_networking(&accepted, &cvm, 8)], &[accepted]); + // An inherited entry still has to hold *some* mode -- every + // consumer matches on one -- and it is rewritten to whatever the + // node has now. Resolution ignores it, so compare what the launch + // sees rather than the field nothing reads. + assert!(restored[0].inherit_mode, "{mode:?}: pinned a backend"); + assert_eq!( + resolve_networking(&restored[0], &cvm, 8), + resolve_networking(&stored[0], &cvm, 8), + "{mode:?}: round trip changed what the launch sees" + ); + assert_eq!(restored[0].queues, Some(4), "{mode:?}: lost the request"); + } + } + + /// The counterpart: a node that changes its mind still reaches VMs that + /// never named a backend, and never reaches ones that did. + #[test] + fn a_node_backend_change_reaches_exactly_the_vms_that_inherited_it() { + let mut cvm = test_cvm_config(); + cvm.networking.nic.mode = NetworkingMode::Bridge; + cvm.networking.nic.bridge = "br-node".into(); + + let tuning_only = networking_from_proto( + &rpc::NetworkingConfig { + queues: Some(2), + ..Default::default() + }, + &cvm, + ) + .unwrap() + .expect("tuning must produce an override"); + let named = networking_from_proto( + &rpc::NetworkingConfig { + mode: "bridge".into(), + queues: Some(2), + ..Default::default() + }, + &cvm, + ) + .unwrap() + .expect("a named backend is an override"); + + let requests = vec![tuning_only, named]; + let merged = requests + .iter() + .map(|request| resolve_networking(request, &cvm, 4)) + .collect::>(); + let stored = manifest_networks(merged, &requests); + + // The operator repoints the node at a different backend. + cvm.networking.nic.mode = NetworkingMode::User; + assert_eq!( + resolve_networking(&stored[0], &cvm, 4).nic.mode, + NetworkingMode::User, + "a VM that never named a backend must follow the node" + ); + assert_eq!( + resolve_networking(&stored[1], &cvm, 4).nic.mode, + NetworkingMode::Bridge, + "a VM that named its backend keeps it for life" + ); + // User mode has no multiqueue backend, so the inherited NIC drops to + // one queue pair while it is there -- but the request is not lost. + assert_eq!(resolve_networking(&stored[0], &cvm, 4).queue_pairs(), 1); + assert_eq!(stored[0].queues, Some(2)); + cvm.networking.nic.mode = NetworkingMode::Bridge; + assert_eq!( + resolve_networking(&stored[0], &cvm, 4).queue_pairs(), + 2, + "tuning must survive an excursion through a backend that ignores it" + ); + } + + #[test] + fn queue_requests_are_bounded_by_node_policy() { + let mut cvm_config = test_cvm_config(); + cvm_config.max_net_queues = 4; + cvm_config.allowed_bridges.push("tenant-br0".to_string()); + let request = |queues: u32| { + [rpc::NetworkingConfig { + mode: "bridge".to_string(), + bridge_name: "tenant-br0".to_string(), + queues: Some(queues), + ..Default::default() + }] + }; + + let networks = networks_from_proto(&request(4), &cvm_config).unwrap(); + assert_eq!(networks[0].queues, Some(4)); + + let err = networks_from_proto(&request(5), &cvm_config).unwrap_err(); + assert!(err.to_string().contains("must not exceed 4")); + } + + /// A mode the deploy dialog offers has to be one the deployment RPC will + /// take. Offering one node policy forbids puts a choice in front of an + /// operator whose only outcome is "not allowed by node policy". + #[test] + fn advertised_modes_are_ones_the_rpc_would_accept() { + let mut cvm = test_cvm_config(); + cvm.allowed_network_modes = vec![NetworkingMode::User]; + for mode in ["bridge", "macvtap"] { + assert!( + networking_from_proto( + &rpc::NetworkingConfig { + mode: mode.to_string(), + ..Default::default() + }, + &cvm, + ) + .is_err(), + "{mode} should be refused by this policy" + ); + } + assert_eq!(advertised_modes(&cvm, true), vec!["user".to_string()]); + + cvm.allowed_network_modes = vec![NetworkingMode::User, NetworkingMode::Macvtap]; + assert_eq!( + advertised_modes(&cvm, true), + vec!["user".to_string(), "macvtap".to_string()] + ); + + // A mode policy allows but the host cannot serve is still not offered. + cvm.allowed_network_modes.push(NetworkingMode::Bridge); + assert!(!advertised_modes(&cvm, false).contains(&"bridge".to_string())); + assert!(advertised_modes(&cvm, true).contains(&"bridge".to_string())); + } + + /// A VM keeps its bridge for life, so the node dropping that bridge from + /// its own configuration must not make the VM's reported configuration + /// unsendable -- there is no flag anywhere to clear a field the caller + /// never typed. + /// Deployment allowlists govern what a caller may newly select. Widening + /// them from a VM's own holdings is what keeps read-modify-write working + /// across a node change -- but only from a field the entry's own mode + /// owns. Resolution used to copy the node's whole networking value into + /// every VM, so a bridge NIC carried whatever macvtap parent the node had + /// configured, and widening from that let the VM move to a parent policy + /// forbids. The split types make the node's other fields unreachable; the + /// two identity fields still share one type, so the scoping stays explicit. + #[test] + fn holdings_widen_policy_only_for_the_mode_that_owns_them() { + let mut cvm = test_cvm_config(); + cvm.networking.nic.mode = NetworkingMode::Bridge; + cvm.networking.nic.parent = "eth1".into(); + cvm.allowed_network_modes = vec![ + NetworkingMode::Bridge, + NetworkingMode::Macvtap, + NetworkingMode::User, + ]; + assert!(cvm.allowed_macvtap_parents.is_empty()); + + // What a bridge NIC deployed on this node used to end up holding. + let held = [NicNetworking { + mode: NetworkingMode::Bridge, + bridge: "br0".into(), + parent: "eth0".into(), + ..NicNetworking::default() + }]; + let widened = held_networking_config(&cvm, &held); + assert!(widened.allowed_bridges.contains(&"br0".to_string())); + assert!(!widened + .allowed_macvtap_parents + .contains(&"eth0".to_string())); + + let request = [rpc::NetworkingConfig { + mode: "macvtap".into(), + parent: "eth0".into(), + ..Default::default() + }]; + let err = networks_from_proto(&request, &widened).unwrap_err(); + assert!( + err.to_string().contains("not allowed by node policy"), + "{err}" + ); + } + + /// Lowering the node's queue cap bounds what a deployment may newly ask + /// for. It does not retune VMs already pinned above it, and those keep + /// reporting their count on every GetInfo -- so without this, lowering the + /// cap makes every networking update on such a VM fail on a field the + /// operator never typed, including the one that would unpin it. + #[test] + fn a_vm_may_restate_a_queue_count_the_node_has_since_capped() { + let mut cvm = test_cvm_config(); + cvm.networking.nic.mode = NetworkingMode::Bridge; + cvm.max_net_queues = 2; + + let held = [NicNetworking { + mode: NetworkingMode::Bridge, + queues: Some(8), + ..NicNetworking::default() + }]; + let request = [rpc::NetworkingConfig { + mode: "bridge".into(), + queues: Some(8), + vhost: Some(false), + ..Default::default() + }]; + + let err = networks_from_proto(&request, &cvm).unwrap_err(); + assert!(err.to_string().contains("must not exceed 2"), "{err}"); + + let widened = held_networking_config(&cvm, &held); + let networks = networks_from_proto(&request, &widened).unwrap(); + assert_eq!(networks[0].queues, Some(8)); + + // Only up to what it holds, though. + let more = [rpc::NetworkingConfig { + mode: "bridge".into(), + queues: Some(9), + ..Default::default() + }]; + let err = networks_from_proto(&more, &widened).unwrap_err(); + assert!(err.to_string().contains("must not exceed 8"), "{err}"); + } + + /// `optional uint32` tells an absent field from a typed zero, so reading + /// zero as "unset" would answer a request for no queues with the + /// vCPU-scaled default. Resize says the same about a zero vCPU count. + #[test] + fn an_explicit_zero_queue_count_is_an_error_not_an_absent_field() { + let cvm = test_cvm_config(); + let request = rpc::NetworkingConfig { + mode: "bridge".into(), + queues: Some(0), + ..Default::default() + }; + let err = networking_from_proto(&request, &cvm).unwrap_err(); + assert!(err.to_string().contains("must be at least 1"), "{err}"); + + // Including when it is the only thing the request says. That arm + // returns "no override at all" before anything reads the count, so a + // zero here used to be answered with up to sixteen queue pairs. + let bare = rpc::NetworkingConfig { + queues: Some(0), + ..Default::default() + }; + let err = networking_from_proto(&bare, &cvm).unwrap_err(); + assert!(err.to_string().contains("must be at least 1"), "{err}"); + } + + /// Resolution fills both identity fields from the node, so a bridge NIC's + /// resolved value carries whatever macvtap parent the node happens to have + /// configured. Storing that made every later reader responsible for knowing + /// which field its mode owns, and the one that widens deployment policy + /// from a VM's holdings got it wrong: repoint the node's parent and a + /// bridge VM could move itself to a parent policy forbids. + #[test] + fn a_vm_records_only_the_identity_field_its_own_mode_owns() { + let mut cvm = test_cvm_config(); + cvm.networking.nic.mode = NetworkingMode::Bridge; + cvm.networking.nic.bridge = "br-node".into(); + cvm.networking.nic.parent = "eth-node".into(); + + let networks = networks_from_proto( + &[rpc::NetworkingConfig { + mode: "bridge".into(), + ..Default::default() + }], + &cvm, + ) + .unwrap(); + // `manifest_networks` directly: what a VM records is the question, and + // `resolve_requested_networks` would first validate the merged view + // against this host's real interfaces. + let merged = networks + .iter() + .map(|request| resolve_networking(request, &cvm, 4)) + .collect::>(); + let stored = manifest_networks(merged, &networks); + assert_eq!(stored[0].bridge, "br-node"); + assert!(stored[0].parent.is_empty(), "{:?}", stored[0]); + + // And the launch still gets the node's parent, because that half was + // never the VM's to hold in the first place. + let at_launch = resolve_networking(&stored[0], &cvm, 4); + assert_eq!(at_launch.nic.parent, "eth-node"); + } + + #[test] + fn a_vm_may_restate_a_bridge_it_already_holds() { + let mut cvm = test_cvm_config(); + cvm.networking.nic.mode = NetworkingMode::Bridge; + cvm.networking.nic.bridge = "br-new".into(); + assert!(cvm.allowed_bridges.is_empty()); + + let held = [NicNetworking { + mode: NetworkingMode::Bridge, + bridge: "br-old".into(), + ..NicNetworking::default() + }]; + let request = [rpc::NetworkingConfig { + mode: "bridge".into(), + bridge_name: "br-old".into(), + queues: Some(2), + ..Default::default() + }]; + + // Without the VM's own holdings this is a bridge it may not select. + let err = networks_from_proto(&request, &cvm).unwrap_err(); + assert!(err.to_string().contains("not allowed by node policy")); + + let widened = held_networking_config(&cvm, &held); + let networks = networks_from_proto(&request, &widened).unwrap(); + assert_eq!(networks[0].bridge, "br-old"); + + // And it is still only this VM's own values that are permitted. + let other = [rpc::NetworkingConfig { + mode: "bridge".into(), + bridge_name: "br-someone-else".into(), + ..Default::default() + }]; + assert!(networks_from_proto(&other, &widened).is_err()); + } + + /// vhost follows the same rule as the queue count: refused for a backend + /// the caller chose and that has none, accepted and dormant for one they + /// inherited. Accepting it silently on a chosen backend would leave the + /// deploy dialog reporting `vhost: on` next to a NIC running without it. + #[test] + fn vhost_is_refused_only_for_a_backend_the_caller_chose() { + let cvm_config = test_cvm_config(); + let err = networking_from_proto( + &rpc::NetworkingConfig { + mode: "user".into(), + vhost: Some(true), + ..Default::default() + }, + &cvm_config, + ) + .unwrap_err(); + assert!(err.to_string().contains("no vhost data plane"), "{err:#}"); + + // Turning it off is a no-op that matches reality, so it is allowed. + networking_from_proto( + &rpc::NetworkingConfig { + mode: "user".into(), + vhost: Some(false), + ..Default::default() + }, + &cvm_config, + ) + .unwrap() + .expect("tuning must produce an override"); + + // Inherited from a user-mode node: accepted, dormant, and live again + // when the node moves to a backend that has one. + let mut cvm_config = test_cvm_config(); + assert_eq!(cvm_config.networking.nic.mode, NetworkingMode::User); + let requested = networking_from_proto( + &rpc::NetworkingConfig { + vhost: Some(true), + ..Default::default() + }, + &cvm_config, + ) + .unwrap() + .expect("tuning must produce an override"); + assert!(!resolve_networking(&requested, &cvm_config, 4).vhost_enabled()); + + cvm_config.networking.nic.mode = NetworkingMode::Bridge; + cvm_config.networking.nic.bridge = "br-node".into(); + assert!(resolve_networking(&requested, &cvm_config, 4).vhost_enabled()); + } + + /// A queue count is refused for a backend the caller chose and that cannot + /// honour it, because the caller can fix the request. It is accepted for + /// one they inherited, because they cannot: the node picked that backend + /// and may pick another tomorrow, and refusing would leave GetInfo + /// reporting a count nothing is allowed to send back. + #[test] + fn a_queue_count_is_refused_only_for_a_backend_the_caller_chose() { + let cvm_config = test_cvm_config(); + let err = networking_from_proto( + &rpc::NetworkingConfig { + mode: "user".into(), + queues: Some(4), + ..Default::default() + }, + &cvm_config, + ) + .unwrap_err(); + assert!(err.to_string().contains("does not support multiple queues")); + + // Inherited: accepted, and dormant until the node moves to a backend + // that can honour it. + for mode in [NetworkingMode::Custom, NetworkingMode::User] { + let mut cvm_config = test_cvm_config(); + cvm_config.networking.nic.mode = mode; + cvm_config.networking.netdev = "tap,id=net0,ifname=custom0".into(); + let requested = networking_from_proto( + &rpc::NetworkingConfig { + queues: Some(4), + ..Default::default() + }, + &cvm_config, + ) + .unwrap() + .expect("tuning must produce an override"); + assert_eq!(requested.queues, Some(4)); + assert_eq!( + resolve_networking(&requested, &cvm_config, 8).queue_pairs(), + 1, + "{mode:?} cannot carry multiqueue, whatever was asked for" + ); + + // And the request is still there when the node moves back. + cvm_config.networking.nic.mode = NetworkingMode::Bridge; + cvm_config.networking.nic.bridge = "br-node".into(); + assert_eq!( + resolve_networking(&requested, &cvm_config, 8).queue_pairs(), + 4 + ); + } + } + + #[test] + fn user_mode_rejects_multiqueue_but_a_single_queue_is_fine() { + let cvm_config = test_cvm_config(); + let request = |queues: u32| { + [rpc::NetworkingConfig { + mode: "user".to_string(), + queues: Some(queues), + ..Default::default() + }] + }; + + networks_from_proto(&request(1), &cvm_config).unwrap(); + let err = networks_from_proto(&request(2), &cvm_config).unwrap_err(); + assert!(err.to_string().contains("does not support multiple queues")); + } + + #[test] + fn tuning_alone_keeps_the_node_backend_without_tripping_mode_policy() { + let mut cvm_config = test_cvm_config(); + // A backend the node uses but does not let callers choose. + cvm_config.networking.nic.mode = NetworkingMode::Macvtap; + cvm_config.networking.nic.parent = "eth0".to_string(); + assert!(!cvm_config + .allowed_network_modes + .contains(&NetworkingMode::Macvtap)); + + let networking = networking_from_proto( + &rpc::NetworkingConfig { + vhost: Some(false), + ..Default::default() + }, + &cvm_config, + ) + .unwrap() + .expect("tuning must produce an override"); + assert_eq!(networking.mode, NetworkingMode::Macvtap); + assert_eq!(networking.vhost, Some(false)); + + // Naming that backend explicitly is still a choice, and still denied. + let err = networking_from_proto( + &rpc::NetworkingConfig { + mode: "macvtap".to_string(), + vhost: Some(false), + ..Default::default() + }, + &cvm_config, + ) + .unwrap_err(); + assert!(err.to_string().contains("not allowed by node policy")); + + // An untouched request still means "no override at all". + assert!( + networking_from_proto(&rpc::NetworkingConfig::default(), &cvm_config) + .unwrap() + .is_none() + ); + } + + #[test] + fn an_inherited_backend_is_never_pinned_into_the_manifest() { + // Tuning must not become a way to pin a backend the caller was never + // allowed to choose, nor to freeze one the node still owns. + let mut cvm_config = test_cvm_config(); + cvm_config.networking.nic.mode = NetworkingMode::Macvtap; + cvm_config.networking.nic.parent = "eth0".to_string(); + cvm_config.networking.macvtap_mode = "private".to_string(); + + let requested = networking_from_proto( + &rpc::NetworkingConfig { + queues: Some(2), + ..Default::default() + }, + &cvm_config, + ) + .unwrap() + .expect("tuning must produce an override"); + assert!(requested.inherit_mode); + + let persisted = resolve_requested_networks(&[requested], &cvm_config, 4).unwrap(); + assert_eq!(persisted[0].queues, Some(2)); + // Nothing the node owns was copied in. The forwarding mode, the MAC + // prefix and the user-mode network parameters are not fields this type + // has any more; the parent is, and a NIC that named no backend does not + // get to keep the node's. + assert!(persisted[0].parent.is_empty()); + assert!(persisted[0].bridge.is_empty()); + + // Repointing the node moves the VM with it. + cvm_config.networking.nic.parent = "eth1".to_string(); + let at_launch = resolve_networking(&persisted[0], &cvm_config, 4); + assert_eq!(at_launch.nic.parent, "eth1"); + assert_eq!(at_launch.queue_pairs(), 2); + } + + #[test] + fn deployment_pins_identity_but_not_data_plane_tuning() { + let mut cvm_config = test_cvm_config(); + cvm_config.networking.nic.vhost = Some(true); + cvm_config.networking.nic.queues = Some(2); + let networks = networks_from_proto( + &[rpc::NetworkingConfig { + mode: "user".to_string(), + vhost: Some(false), + ..Default::default() + }], + &cvm_config, + ) + .unwrap(); + + let resolved = resolve_requested_networks(&networks, &cvm_config, 4).unwrap(); + // The backend the caller chose is pinned. + assert_eq!(resolved[0].mode, NetworkingMode::User); + assert!(!resolved[0].inherit_mode); + // The explicit request is kept. + assert_eq!(resolved[0].vhost, Some(false)); + // What the caller never asked for stays unset, so the node still owns + // it: an operator disabling vhost node-wide must reach this VM too. + assert_eq!(resolved[0].queues, None); + } + + #[test] + fn a_node_wide_vhost_rollback_reaches_a_vm_deployed_with_an_override() { + // macvtap keeps this independent of which interfaces the test host has. + let mut cvm_config = test_cvm_config(); + cvm_config + .allowed_network_modes + .push(NetworkingMode::Macvtap); + cvm_config.allowed_macvtap_parents.push("eth0".to_string()); + cvm_config.networking.nic.parent = "eth0".to_string(); + let networks = networks_from_proto( + &[rpc::NetworkingConfig { + mode: "macvtap".to_string(), + parent: "eth0".to_string(), + ..Default::default() + }], + &cvm_config, + ) + .unwrap(); + let persisted = resolve_requested_networks(&networks, &cvm_config, 4).unwrap(); + assert!(persisted[0].vhost.is_none()); + + cvm_config.networking.nic.vhost = Some(false); + let at_launch = resolve_networking(&persisted[0], &cvm_config, 4); + assert!(!at_launch.vhost_enabled()); } #[test] @@ -1399,18 +2373,35 @@ mod tests { } #[test] - fn repeated_networks_rejects_empty_entries() { - let err = networks_from_proto( + /// An entry in a list that overrides nothing describes a NIC that follows + /// the node entirely -- which is a thing an operator can mean, and the + /// only way the web UI can leave a NIC's backend unpinned. Rejecting it + /// would make an inherited NIC uneditable the moment its tuning is cleared. + fn repeated_networks_accepts_an_entry_that_overrides_nothing() { + let mut cvm_config = test_cvm_config(); + cvm_config.networking.nic.mode = NetworkingMode::Bridge; + cvm_config.networking.nic.bridge = "br-node".into(); + + let networks = networks_from_proto( &[rpc::NetworkingConfig { mode: String::new(), bridge_name: String::new(), ..Default::default() }], - &test_cvm_config(), + &cvm_config, ) - .unwrap_err(); + .unwrap(); + assert_eq!(networks.len(), 1); + assert!(networks[0].inherit_mode); + assert_eq!(networks[0].queues, None); + assert!(networks[0].bridge.is_empty()); - assert!(err.to_string().contains("networking mode is required")); + // It follows the node, like a VM with no networks at all. + cvm_config.networking.nic.mode = NetworkingMode::User; + assert_eq!( + resolve_networking(&networks[0], &cvm_config, 4).nic.mode, + NetworkingMode::User + ); } #[test] diff --git a/dstack/vmm/src/netd.rs b/dstack/vmm/src/netd.rs index b7a946133..6a760d1eb 100644 --- a/dstack/vmm/src/netd.rs +++ b/dstack/vmm/src/netd.rs @@ -5,7 +5,6 @@ //! Small privileged broker for TAP creation and libvirt nwfilter bindings. use std::{ - collections::BTreeMap, fs::{File, OpenOptions, Permissions}, io::Write as _, os::{ @@ -29,11 +28,11 @@ use tokio::{ net::{UnixListener, UnixStream}, time::timeout, }; -use tracing::{info, warn}; +use tracing::{debug, info, warn}; use uuid::Uuid; use wait_timeout::ChildExt; -use crate::config::NetdConfig; +use crate::config::{NetdConfig, NetworkFilterConfig}; const MAX_MESSAGE_SIZE: u64 = 64 * 1024; const CONNECTION_TIMEOUT: Duration = Duration::from_secs(35); @@ -41,6 +40,9 @@ const COMMAND_TIMEOUT: Duration = Duration::from_secs(30); const IP_PATH: &str = "/usr/sbin/ip"; const VIRSH_PATH: &str = "/usr/bin/virsh"; const LOCK_PATH: &str = "/run/lock/dstack-netd.lock"; +/// Upper bound on TAP queue pairs netd will create. Mirrors the VMM's own cap +/// so a malformed request cannot ask the kernel for an unbounded device. +const MAX_QUEUES: u32 = 64; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct InterfaceIdentity { @@ -56,9 +58,17 @@ pub struct PrepareBridgeRequest { pub bridge: String, pub mac: String, pub qemu_uid: u32, - pub filter: String, - #[serde(default)] - pub parameters: BTreeMap, + /// Whether to bind an nwfilter to the TAP. *Which* filter, and with what + /// parameters, is netd's own configuration to decide -- a caller that named + /// them could name one that filters nothing, or pin the binding to the + /// gateway's MAC and IP, and still satisfy a node policy that only asked + /// for "some filter". An unfiltered TAP is what multiqueue bridge + /// networking needs on nodes that do not run libvirt. + pub filtered: bool, + /// virtio-net queue pairs. Zero or one creates a single-queue TAP. QEMU + /// rejects a device whose `IFF_MULTI_QUEUE` state differs from its own + /// `queues=` argument, so this must match the launch exactly. + pub queues: u32, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -70,6 +80,10 @@ pub struct PrepareMacvtapRequest { pub qemu_uid: u32, #[serde(default)] pub mode: String, + /// virtio-net queue pairs. The device is created with matching hardware + /// queues; QEMU then opens the character device once per queue. + #[serde(default)] + pub queues: u32, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -80,12 +94,17 @@ pub enum Request { Remove { #[serde(flatten)] identity: InterfaceIdentity, + /// Whether this interface was created with an nwfilter binding. + /// Macvtap TAPs never carry one, and removal detects them rather than + /// trusting this field. + filtered: bool, }, /// Verify a deterministic TAP and binding for operations and integration /// diagnostics. The VMM startup path uses Prepare rather than Check. Check { #[serde(flatten)] identity: InterfaceIdentity, + filtered: bool, }, } @@ -96,10 +115,33 @@ struct Response { tap: Option, #[serde(default, skip_serializing_if = "Option::is_none")] device: Option, + /// Queue pairs the interface was actually created with. Absent from a netd + /// that predates multiqueue, which is how the VMM tells the difference + /// between "one queue was requested" and "this netd ignored the request". + #[serde(default, skip_serializing_if = "Option::is_none")] + queues: Option, #[serde(default, skip_serializing_if = "Option::is_none")] error: Option, } +/// What netd built, echoed back so the caller can verify it matches the +/// request before handing the interface to QEMU. +struct Prepared { + tap: String, + device: Option, + queues: Option, +} + +impl Prepared { + fn tap(tap: String) -> Self { + Self { + tap, + device: None, + queues: None, + } + } +} + pub fn tap_name(identity: &InterfaceIdentity) -> String { let input = format!( "{}\0{}\0{}", @@ -119,6 +161,28 @@ pub fn instance_id(configured: &str, run_path: &Path) -> String { pub struct PreparedInterface { pub device: Option, + pub queues: Option, +} + +/// Marker carried in the error chain when the VMM could not reach netd at all. +/// +/// "netd refused this" and "netd is not there" call for different advice, and +/// the caller cannot tell them apart from the message alone -- a callback probe +/// afterwards would answer about a different moment. +#[derive(Debug)] +pub struct Unreachable; + +impl std::fmt::Display for Unreachable { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("netd is not reachable") + } +} + +impl std::error::Error for Unreachable {} + +/// Whether this error means netd was never reached. +pub fn is_unreachable(error: &anyhow::Error) -> bool { + error.chain().any(|cause| cause.is::()) } pub async fn request(socket: &Path, request: &Request) -> Result { @@ -131,6 +195,8 @@ pub async fn request(socket: &Path, request: &Request) -> Result MAX_MESSAGE_SIZE { @@ -154,11 +220,10 @@ pub async fn request(socket: &Path, request: &Request) -> Result Result { async fn serve_connection(config: &NetdConfig, stream: &mut UnixStream) -> Result<()> { // Access is authorized by the Unix socket's owner, group, and mode. Any // process that can connect is trusted with the complete netd protocol. - let response = match read_request(stream) - .await - .and_then(|request| handle_request(&config.libvirt_uri, request)) - { - Ok((tap, device)) => Response { + let outcome = match read_request(stream).await { + // A peer that connects and closes without sending is the VMM's + // reachability check: netd that died leaves its socket behind, so the + // VMM connects to tell the two apart. Answering that with a parse error + // and a warning would fill the log with reports of it working. + Ok(None) => { + debug!("netd liveness probe"); + return Ok(()); + } + Ok(Some(request)) => handle_request(config, request), + // A request that arrived but could not be understood still gets an + // answer. A VMM newer than this netd sends operations it does not + // know, and "unknown variant `prepare_foo`" is what tells the operator + // to upgrade; a closed connection tells them nothing. + Err(error) => Err(error), + }; + let response = match outcome { + Ok(prepared) => Response { ok: true, - tap: Some(tap), - device, + tap: Some(prepared.tap), + device: prepared.device, + queues: prepared.queues, error: None, }, Err(error) => { @@ -241,6 +320,7 @@ async fn serve_connection(config: &NetdConfig, stream: &mut UnixStream) -> Resul ok: false, tap: None, device: None, + queues: None, error: Some(format!("{error:#}")), } } @@ -251,77 +331,105 @@ async fn serve_connection(config: &NetdConfig, stream: &mut UnixStream) -> Resul Ok(()) } -async fn read_request(stream: &mut UnixStream) -> Result { +/// Reads one request, or `None` if the peer closed without sending anything. +async fn read_request(stream: &mut UnixStream) -> Result> { let mut message = Vec::new(); stream .take(MAX_MESSAGE_SIZE + 1) .read_to_end(&mut message) .await?; + if message.is_empty() { + return Ok(None); + } if message.len() as u64 > MAX_MESSAGE_SIZE { bail!("request exceeds {MAX_MESSAGE_SIZE} bytes"); } - serde_json::from_slice(&message).context("invalid netd request") + serde_json::from_slice(&message) + .map(Some) + .context("invalid netd request") } -fn handle_request(libvirt_uri: &str, request: Request) -> Result<(String, Option)> { +fn handle_request(config: &NetdConfig, request: Request) -> Result { + let libvirt_uri = config.libvirt_uri.as_str(); let _lock = OperationLock::acquire()?; match request { Request::PrepareBridge(request) => { - prepare_bridge(libvirt_uri, &request).map(|tap| (tap, None)) + prepare_bridge(libvirt_uri, &request, config.filter_policy()) } - Request::PrepareMacvtap(request) => prepare_macvtap( - libvirt_uri, - &request.identity, - &request.parent, - &request.mac, - request.qemu_uid, - &request.mode, - ) - .map(|(tap, device)| (tap, Some(device))), - Request::Remove { identity } => { + Request::PrepareMacvtap(request) => { + prepare_macvtap(libvirt_uri, &request, config.filter_policy()) + } + Request::Remove { identity, filtered } => { validate_identity(&identity)?; let tap = tap_name(&identity); - remove_interface(libvirt_uri, &tap)?; - Ok((tap, None)) + remove_interface(libvirt_uri, &tap, binding_cleanup(filtered))?; + Ok(Prepared::tap(tap)) } - Request::Check { identity } => { + Request::Check { identity, filtered } => { validate_identity(&identity)?; let tap = tap_name(&identity); if !Path::new("/sys/class/net").join(&tap).exists() { bail!("TAP {tap} does not exist"); } - if !is_macvtap(&tap) { + // An unfiltered TAP has no binding to dump; asking for one would + // report a healthy multiqueue interface as broken. + if filtered && !is_macvtap(&tap) { virsh(libvirt_uri, &["nwfilter-binding-dumpxml", &tap], None)?; } - Ok((tap, None)) + Ok(Prepared::tap(tap)) } } } fn prepare_macvtap( libvirt_uri: &str, - identity: &InterfaceIdentity, - parent: &str, - mac: &str, - qemu_uid: u32, - mode: &str, -) -> Result<(String, String)> { + request: &PrepareMacvtapRequest, + filter: &NetworkFilterConfig, +) -> Result { + let identity = &request.identity; + let parent = request.parent.as_str(); + let qemu_uid = request.qemu_uid; validate_identity(identity)?; validate_name("parent", parent, 15, "_.-")?; if !Path::new("/sys/class/net").join(parent).exists() { bail!("parent interface {parent} does not exist"); } - validate_mac(mac)?; - let mode = if mode.is_empty() { "private" } else { mode }; + // A macvtap parent may be a bridge, and libvirt nwfilter does not apply to + // macvtap. So on a node that requires every bridge TAP to be filtered, a + // macvtap request naming that same bridge is the identical unfiltered L2 + // access the policy exists to refuse, spelled with a different operation. + // An interface enslaved to a bridge reaches the same segment. + if filter.requires_binding() { + let sysfs = Path::new("/sys/class/net").join(parent); + if sysfs.join("bridge").exists() { + bail!("this netd requires filtering, so {parent} may not be a macvtap parent: it is a host bridge"); + } + if sysfs.join("master").exists() { + bail!("this netd requires filtering, so {parent} may not be a macvtap parent: it is enslaved to a bridge"); + } + } + validate_mac(&request.mac)?; + let mac = request.mac.as_str(); + let mode = if request.mode.is_empty() { + "private" + } else { + request.mode.as_str() + }; if !matches!(mode, "private" | "bridge" | "vepa" | "passthru") { bail!("invalid macvtap mode"); } + let queues = validate_queues(request.queues)?; let tap = tap_name(identity); - remove_interface(libvirt_uri, &tap)?; - ip(&[ - "link", "add", "link", parent, "name", &tap, "address", mac, "type", "macvtap", "mode", - mode, - ])?; + remove_interface(libvirt_uri, &tap, BindingCleanup::BestEffort)?; + let queue_count = queues.to_string(); + let mut add = vec!["link", "add", "link", parent, "name", &tap, "address", mac]; + if queues > 1 { + // macvtap defaults to a single hardware queue pair. Without this the + // extra tap queues exist but the lower device still serializes. + add.extend_from_slice(&["numtxqueues", &queue_count, "numrxqueues", &queue_count]); + } + add.extend_from_slice(&["type", "macvtap", "mode", mode]); + ip(&add)?; let result = (|| { let ifindex = std::fs::read_to_string(Path::new("/sys/class/net").join(&tap).join("ifindex")) @@ -346,11 +454,15 @@ fn prepare_macvtap( })(); match result { Ok(device) => { - info!(%tap, %parent, %mode, %device, "prepared macvtap"); - Ok((tap, device)) + info!(%tap, %parent, %mode, %device, %queues, "prepared macvtap"); + Ok(Prepared { + tap, + device: Some(device), + queues: Some(queues), + }) } Err(error) => { - let _ = remove_interface(libvirt_uri, &tap); + let _ = remove_interface(libvirt_uri, &tap, BindingCleanup::BestEffort); Err(error) } } @@ -384,41 +496,87 @@ impl Drop for OperationLock { } } -fn prepare_bridge(libvirt_uri: &str, request: &PrepareBridgeRequest) -> Result { - validate_prepare_bridge(request)?; +fn prepare_bridge( + libvirt_uri: &str, + request: &PrepareBridgeRequest, + filter: &NetworkFilterConfig, +) -> Result { + validate_prepare_bridge(request, filter)?; + let filtered = request.filtered; let tap = tap_name(&request.identity); // A failed VMM start may leave a deterministic resource behind. Replacing // it makes prepare idempotent without accepting a caller-selected TAP. - remove_interface(libvirt_uri, &tap)?; + remove_interface(libvirt_uri, &tap, binding_cleanup(filtered))?; let uid = request.qemu_uid.to_string(); - ip(&["tuntap", "add", "dev", &tap, "mode", "tap", "user", &uid])?; + let queues = validate_queues(request.queues)?; + let mut add = vec!["tuntap", "add", "dev", &tap, "mode", "tap"]; + if queues > 1 { + // QEMU refuses to attach when the device's IFF_MULTI_QUEUE state does + // not match its own `queues=` argument, in either direction. + add.push("multi_queue"); + } + add.extend_from_slice(&["user", &uid]); + ip(&add)?; let result = (|| { ip(&["link", "set", "dev", &tap, "master", &request.bridge])?; - let xml = binding_xml(request, &tap); - virsh( - libvirt_uri, - &["nwfilter-binding-create", "--validate", "/dev/stdin"], - Some(xml.as_bytes()), - )?; + if filtered { + let xml = binding_xml(request, &tap, filter); + virsh( + libvirt_uri, + &["nwfilter-binding-create", "--validate", "/dev/stdin"], + Some(xml.as_bytes()), + )?; + } ip(&["link", "set", "dev", &tap, "up"])?; Ok(()) })(); if let Err(error) = result { - let _ = remove_interface(libvirt_uri, &tap); + let _ = remove_interface(libvirt_uri, &tap, BindingCleanup::BestEffort); return Err(error); } - info!(%tap, bridge = %request.bridge, filter = %request.filter, "prepared filtered TAP"); - Ok(tap) + info!(%tap, bridge = %request.bridge, %filtered, %queues, "prepared TAP"); + Ok(Prepared { + tap, + device: None, + queues: Some(queues), + }) } -fn remove_interface(libvirt_uri: &str, tap: &str) -> Result<()> { +/// How hard removal must try to clear an nwfilter binding. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum BindingCleanup { + /// The binding must be gone before this returns, because the caller is + /// about to create one at the same interface name and libvirt refuses a + /// duplicate. + Required, + /// Delete a binding if libvirt can be reached, but do not fail the removal + /// when it cannot. Unfiltered TAPs live on nodes where `libvirtd` need not + /// be running at all, and a stale binding left by an earlier, filtered + /// interface at this name is still worth clearing when it is. + BestEffort, +} + +fn remove_interface(libvirt_uri: &str, tap: &str, cleanup: BindingCleanup) -> Result<()> { let macvtap = is_macvtap(tap); if Path::new("/sys/class/net").join(tap).exists() { let _ = ip(&["link", "set", "dev", tap, "down"]); } + // A macvtap interface never carries a binding. Anything else might: this + // name may have been a filtered bridge TAP before, and the binding + // outlives the interface. if !macvtap { - delete_binding(libvirt_uri, tap)?; + match cleanup { + BindingCleanup::Required => delete_binding(libvirt_uri, tap)?, + BindingCleanup::BestEffort => { + // netd refuses to start without virsh, so the binary is always + // here; libvirtd need not be running, and on a node that only + // wants macvtap or multiqueue it usually is not. + if let Err(error) = delete_binding(libvirt_uri, tap) { + warn!(%tap, "could not clear a possible nwfilter binding: {error:#}"); + } + } + } } if Path::new("/sys/class/net").join(tap).exists() { ip(&["link", "delete", "dev", tap])?; @@ -434,32 +592,36 @@ fn is_macvtap(interface: &str) -> bool { .exists() } +/// Deletes an interface's nwfilter binding, if it has one. +/// +/// Goes through the same `COMMAND_TIMEOUT`-bounded helper as every other virsh +/// call. netd's accept loop is strictly serialized, so an unbounded call here +/// would let one unreachable libvirt stall every other VM's prepare and remove. fn delete_binding(uri: &str, tap: &str) -> Result<()> { - let output = Command::new(VIRSH_PATH) - .args(["--connect", uri, "nwfilter-binding-delete", tap]) - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .output() - .context("failed to execute virsh")?; - if output.status.success() { - return Ok(()); - } - let error = String::from_utf8_lossy(&output.stderr); - if error.contains("Network filter binding not found") { - return Ok(()); + match virsh(uri, &["nwfilter-binding-delete", tap], None) { + Ok(()) => Ok(()), + // Removal is idempotent. Having no binding is the normal case for + // macvtap, for unfiltered multiqueue TAPs, and for any name being + // reused after an earlier removal already cleared it. + Err(error) + if error + .to_string() + .contains("Network filter binding not found") => + { + Ok(()) + } + Err(error) => Err(error).context(format!("virsh failed to delete binding {tap}")), } - bail!("virsh failed to delete binding {tap}: {}", error.trim()) } -fn binding_xml(request: &PrepareBridgeRequest, tap: &str) -> String { +fn binding_xml(request: &PrepareBridgeRequest, tap: &str, filter: &NetworkFilterConfig) -> String { let owner_uuid = stable_uuid(&request.identity); let owner_name = format!( "dstack:{}:{}:{}", request.identity.instance_id, request.identity.vm_id, request.identity.nic_index ); let mut parameters = String::new(); - for (name, value) in &request.parameters { + for (name, value) in &filter.parameters { parameters.push_str(&format!( "", xml_escape(name), @@ -474,7 +636,7 @@ fn binding_xml(request: &PrepareBridgeRequest, tap: &str) -> String { owner_uuid, xml_escape(tap), xml_escape(&request.mac), - xml_escape(&request.filter), + xml_escape(&filter.filter), parameters ) } @@ -494,9 +656,24 @@ fn stable_uuid(identity: &InterfaceIdentity) -> Uuid { Uuid::from_bytes(bytes) } -fn validate_prepare_bridge(request: &PrepareBridgeRequest) -> Result<()> { +fn validate_prepare_bridge( + request: &PrepareBridgeRequest, + filter: &NetworkFilterConfig, +) -> Result<()> { validate_identity(&request.identity)?; validate_name("bridge", &request.bridge, 15, "_.-")?; + // Unfiltered bridge TAPs exist for unfiltered multiqueue, and netd holds + // that policy itself rather than trusting the caller with it. netd is the + // privileged side of this socket; on a node configured to filter bridge + // traffic, "build me a TAP on br0 with no nwfilter binding" is precisely + // the request the boundary exists to refuse, and anything that can reach + // the socket can make it. + // + // Refused before the host is inspected: this is about the request, not + // about what happens to exist on this machine. + if filter.requires_binding() && !request.filtered { + bail!("this netd requires an nwfilter binding on every bridge TAP"); + } if !Path::new("/sys/class/net") .join(&request.bridge) .join("bridge") @@ -505,16 +682,6 @@ fn validate_prepare_bridge(request: &PrepareBridgeRequest) -> Result<()> { bail!("{} is not a host bridge", request.bridge); } validate_mac(&request.mac)?; - validate_name("filter", &request.filter, 128, "_.:-")?; - if request.parameters.len() > 64 { - bail!("too many nwfilter parameters"); - } - for (name, value) in &request.parameters { - validate_name("parameter name", name, 64, "_")?; - if value.len() > 512 || value.contains('\0') { - bail!("invalid nwfilter parameter value"); - } - } Ok(()) } @@ -533,9 +700,34 @@ fn validate_identity(identity: &InterfaceIdentity) -> Result<()> { Ok(()) } +/// A caller that knows a binding is there needs it gone; one that does not +/// still clears whatever it finds, without failing when libvirt is absent. +fn binding_cleanup(filtered: bool) -> BindingCleanup { + if filtered { + BindingCleanup::Required + } else { + BindingCleanup::BestEffort + } +} + +/// Normalizes a requested queue pair count. Zero means the caller did not ask +/// for multiqueue, which is the same device shape as one queue pair. +fn validate_queues(queues: u32) -> Result { + if queues > MAX_QUEUES { + bail!("queues must not exceed {MAX_QUEUES}"); + } + Ok(queues.max(1)) +} + fn validate_name(label: &str, value: &str, max: usize, punctuation: &str) -> Result<()> { if value.is_empty() || value.len() > max + // `.` and `..` pass the charset check below, and every name validated + // here is then joined onto a sysfs path to ask whether the interface + // exists. `/sys/class/net/..` exists, so the question would be answered + // about a directory rather than about an interface. + || value == "." + || value == ".." || !value .chars() .all(|ch| ch.is_ascii_alphanumeric() || punctuation.contains(ch)) @@ -648,6 +840,7 @@ fn prepare_socket_path(socket: &Path) -> Result<()> { #[cfg(test)] mod tests { use super::*; + use std::collections::BTreeMap; fn identity(instance: &str, vm: &str, nic_index: usize) -> InterfaceIdentity { InterfaceIdentity { @@ -673,10 +866,15 @@ mod tests { bridge: "br0".into(), mac: "02:00:00:00:00:01".into(), qemu_uid: 1000, + filtered: true, + queues: 0, + }; + let filter = NetworkFilterConfig { + mode: crate::config::NetworkFilterMode::Libvirt, filter: "clean-traffic".into(), parameters: BTreeMap::from([("IP".into(), "10.0.0.2<&".into())]), }; - let xml = binding_xml(&request, "dt123"); + let xml = binding_xml(&request, "dt123", &filter); assert!(xml.contains("instance<&")); assert!(xml.contains("10.0.0.2<&")); assert!(!xml.contains("instance<&")); @@ -685,6 +883,10 @@ mod tests { #[test] fn validation_rejects_injected_host_names() { assert!(validate_name("bridge", "br0;id", 15, "_.-").is_err()); + // `/sys/class/net/..` exists, so an existence check on this name would + // answer about a directory rather than about an interface. + assert!(validate_name("parent", "..", 15, "_.-").is_err()); + assert!(validate_name("parent", ".", 15, "_.-").is_err()); assert!(validate_name("filter", "../../filter", 128, "_.:-").is_err()); assert!(validate_mac("ff:ff:ff:ff:ff:ff").is_err()); } @@ -693,6 +895,7 @@ mod tests { fn remove_protocol_keeps_identity_fields_flat() { let request = Request::Remove { identity: identity("instance", "vm", 2), + filtered: true, }; let value = serde_json::to_value(request).unwrap(); assert_eq!(value["operation"], "remove"); @@ -709,8 +912,8 @@ mod tests { bridge: "br0".into(), mac: "02:00:00:00:00:01".into(), qemu_uid: 1000, - filter: "clean-traffic".into(), - parameters: BTreeMap::new(), + filtered: true, + queues: 0, }); let value = serde_json::to_value(request).unwrap(); assert_eq!(value["operation"], "prepare_bridge"); @@ -719,6 +922,90 @@ mod tests { assert!(value.get("identity").is_none()); } + /// `filtered` says which of two shapes was built, and both are reachable + /// on any node this build can produce. There is no released peer that omits + /// it -- netd does not exist before v0.6 -- so it is required rather than + /// defaulted, and a request that leaves it out is a bug, not an old client. + #[test] + fn removal_states_which_shape_it_is_undoing() { + let error = serde_json::from_value::(serde_json::json!({ + "operation": "remove", + "instance_id": "instance", + "vm_id": "vm", + "nic_index": 0, + })) + .unwrap_err(); + assert!(error.to_string().contains("filtered"), "{error}"); + + for filtered in [true, false] { + let decoded: Request = serde_json::from_value(serde_json::json!({ + "operation": "remove", + "instance_id": "instance", + "vm_id": "vm", + "nic_index": 0, + "filtered": filtered, + })) + .unwrap(); + let Request::Remove { + filtered: decoded, .. + } = decoded + else { + panic!("wrong variant"); + }; + assert_eq!(decoded, filtered); + } + } + + /// A binding outlives the interface it was bound to, and TAP names are a + /// deterministic hash of the VM identity, so the same name comes back. + /// Removing an interface therefore clears whatever binding is there, and + /// only insists when the caller is about to create a replacement. + #[test] + fn binding_cleanup_insists_only_when_a_replacement_follows() { + assert_eq!(binding_cleanup(true), BindingCleanup::Required); + assert_eq!(binding_cleanup(false), BindingCleanup::BestEffort); + } + + #[test] + fn queue_counts_normalize_to_at_least_one_and_stay_bounded() { + assert_eq!(validate_queues(0).unwrap(), 1); + assert_eq!(validate_queues(1).unwrap(), 1); + assert_eq!(validate_queues(MAX_QUEUES).unwrap(), MAX_QUEUES); + assert!(validate_queues(MAX_QUEUES + 1).is_err()); + } + + #[test] + fn queue_count_travels_with_the_prepare_request() { + let request = Request::PrepareBridge(PrepareBridgeRequest { + identity: identity("instance", "vm", 0), + bridge: "br0".into(), + mac: "02:00:00:00:00:01".into(), + qemu_uid: 1000, + filtered: false, + queues: 4, + }); + let value = serde_json::to_value(request).unwrap(); + assert_eq!(value["queues"], 4); + assert_eq!(value["filtered"], false); + + // QEMU refuses a device whose IFF_MULTI_QUEUE state disagrees with its + // own `queues=`, so a request that leaves the count to netd's + // imagination is one netd must not answer. + let error = serde_json::from_value::(serde_json::json!({ + "operation": "prepare_bridge", + "instance_id": "instance", + "vm_id": "vm", + "nic_index": 0, + "bridge": "br0", + "mac": "02:00:00:00:00:01", + "qemu_uid": 1000, + "filtered": true, + })) + .unwrap_err(); + assert!(error.to_string().contains("queues"), "{error}"); + assert_eq!(validate_queues(0).unwrap(), 1); + } + #[test] fn macvtap_prepare_has_a_dedicated_operation() { let request = Request::PrepareMacvtap(PrepareMacvtapRequest { @@ -727,6 +1014,7 @@ mod tests { mac: "02:00:00:00:00:01".into(), qemu_uid: 1000, mode: "private".into(), + queues: 0, }); let value = serde_json::to_value(request).unwrap(); assert_eq!(value["operation"], "prepare_macvtap"); @@ -742,8 +1030,8 @@ mod tests { bridge: "br0".into(), mac: "02:00:00:00:00:01".into(), qemu_uid: 1000, - filter: "clean-traffic".into(), - parameters: BTreeMap::new(), + filtered: true, + queues: 0, }); let value = serde_json::to_value(request).unwrap(); assert_eq!(value["operation"], "prepare_bridge"); @@ -752,8 +1040,14 @@ mod tests { assert!(value.get("identity").is_none()); } + /// Connecting and closing without sending is how the VMM checks that netd + /// is alive, because a netd that died leaves its socket behind. It has to + /// be handled promptly, and quietly: the VMM does it once per status query + /// that mentions a stopped VM, and netd's accept loop is serialized, so + /// treating a probe as a failed request would both fill the log and put + /// noise in front of real work. #[tokio::test] - async fn disconnected_client_is_confined_to_one_connection() { + async fn a_connection_that_sends_nothing_is_a_liveness_probe() { let (mut server, client) = UnixStream::pair().unwrap(); drop(client); let result = timeout( @@ -762,10 +1056,143 @@ mod tests { ) .await; assert!(result.is_ok(), "disconnected peer blocked the handler"); - // Either the EOF is reported while reading or the response write sees - // EPIPE. In both cases serve() logs this per-connection error and keeps - // accepting clients. - assert!(result.unwrap().is_err()); + assert!(result.unwrap().is_ok(), "a probe is not a failed request"); + } + + /// Only an empty connection is a probe. A peer that does send something, + /// and sends nonsense, is still a request -- and still gets an answer it + /// can read, which is how a VMM newer than its netd learns to say so. + #[tokio::test] + async fn a_request_that_cannot_be_understood_still_gets_an_answer() { + let (mut server, client) = UnixStream::pair().unwrap(); + drop(client); + assert!(read_request(&mut server).await.unwrap().is_none()); + + let (mut server, mut client) = UnixStream::pair().unwrap(); + // An operation only a newer VMM knows about. + client + .write_all(br#"{"operation":"prepare_something_new"}"#) + .await + .unwrap(); + client.shutdown().await.unwrap(); + serve_connection(&NetdConfig::default(), &mut server) + .await + .unwrap(); + + let mut reply = Vec::new(); + client.read_to_end(&mut reply).await.unwrap(); + let reply: serde_json::Value = serde_json::from_slice(&reply).unwrap(); + assert_eq!(reply["ok"], false); + assert!( + reply["error"].as_str().unwrap().contains("unknown variant"), + "{reply}" + ); + } + + /// netd is the privileged side of this socket. "Build me a TAP on br0 with + /// no nwfilter binding" is the request the boundary exists to refuse on a + /// filtering node, and before this the daemon simply did what it was told, + /// leaving the invariant with the unprivileged caller. + #[test] + fn a_filtering_node_refuses_an_unfiltered_bridge_tap() { + let request = PrepareBridgeRequest { + identity: InterfaceIdentity { + instance_id: "i".into(), + vm_id: "v".into(), + nic_index: 0, + }, + // A name no host has, so the check after this one is the one that + // fails when this one does not. + bridge: "dstack-nobr0".into(), + mac: "02:00:00:00:00:01".into(), + qemu_uid: 1000, + filtered: false, + queues: 4, + }; + let filtering = NetworkFilterConfig { + mode: crate::config::NetworkFilterMode::Libvirt, + ..NetworkFilterConfig::default() + }; + let error = validate_prepare_bridge(&request, &filtering).unwrap_err(); + assert!( + error.to_string().contains("requires an nwfilter binding"), + "{error}" + ); + + // An unfiltered node still builds them; that is what multiqueue needs. + // It gets as far as asking the host about the bridge, which is the + // next check and not this one's business. + let error = validate_prepare_bridge(&request, &NetworkFilterConfig::default()).unwrap_err(); + assert!( + error.to_string().contains("is not a host bridge"), + "{error}" + ); + } + + /// nwfilter does not apply to macvtap, and a macvtap parent may be the very + /// bridge the policy protects. Refusing an unfiltered bridge TAP while + /// handing out a macvtap on the same segment would leave the policy + /// enforced only against the spelling that happens to be checked. + #[test] + fn a_filtering_node_refuses_a_macvtap_parent_that_is_a_bridge() { + let filtering = NetworkFilterConfig { + mode: crate::config::NetworkFilterMode::Libvirt, + ..NetworkFilterConfig::default() + }; + let bridges: Vec = std::fs::read_dir("/sys/class/net") + .into_iter() + .flatten() + .flatten() + .filter(|entry| entry.path().join("bridge").exists()) + .filter_map(|entry| entry.file_name().into_string().ok()) + .collect(); + let Some(bridge) = bridges.first() else { + // Nothing to assert against on a host with no bridge; the unit + // below still pins the enslaved case's sysfs predicate. + return; + }; + let request = PrepareMacvtapRequest { + identity: identity("i", "v", 0), + parent: bridge.clone(), + mac: "02:00:00:00:00:01".into(), + qemu_uid: 1000, + mode: "bridge".into(), + queues: 4, + }; + let error = match prepare_macvtap("test:///default", &request, &filtering) { + Err(error) => error, + Ok(_) => panic!("a filtering node must not build a macvtap on a host bridge"), + }; + assert!(error.to_string().contains("is a host bridge"), "{error}"); + } + + /// The request says whether to bind a filter, never which one. `allow-arp` + /// contains no drop rule at all, and `clean-traffic` pinned to the + /// gateway's MAC and IP through its parameters filters nothing useful + /// either -- both would satisfy a policy that only asked for "some filter". + #[test] + fn the_bound_filter_comes_from_netds_own_configuration() { + let request = PrepareBridgeRequest { + identity: identity("i", "v", 0), + bridge: "br0".into(), + mac: "02:00:00:00:00:01".into(), + qemu_uid: 1000, + filtered: true, + queues: 1, + }; + // Nothing on the wire can name a filter: the field does not exist. + let wire = serde_json::to_value(Request::PrepareBridge(request.clone())).unwrap(); + assert!(wire.get("filter").is_none(), "{wire}"); + assert!(wire.get("parameters").is_none(), "{wire}"); + + let policy = NetworkFilterConfig { + mode: crate::config::NetworkFilterMode::Libvirt, + filter: "clean-traffic".into(), + parameters: BTreeMap::from([("IP".into(), "10.0.0.2".into())]), + }; + let xml = binding_xml(&request, "dt123", &policy); + assert!(xml.contains("filter='clean-traffic'"), "{xml}"); + assert!(xml.contains("value='10.0.0.2'"), "{xml}"); } #[test] diff --git a/dstack/vmm/src/one_shot.rs b/dstack/vmm/src/one_shot.rs index dd7a23d79..d7500b008 100644 --- a/dstack/vmm/src/one_shot.rs +++ b/dstack/vmm/src/one_shot.rs @@ -3,10 +3,11 @@ // SPDX-License-Identifier: Apache-2.0 use crate::app::{ - make_sys_config, resolved_networks, simulator_config_for_manifest, sync_tee_simulator_config, - Image, VmConfig, VmWorkDir, + clamp_queues_without_netd, make_sys_config, needs_netd_interface, resolved_networks, + settle_vhost, simulator_config_for_manifest, sync_tee_simulator_config, Image, VmConfig, + VmWorkDir, }; -use crate::config::{Config, NetworkFilterMode, NetworkingMode}; +use crate::config::Config; use crate::main_service; use anyhow::{Context, Result}; use fs_err as fs; @@ -279,18 +280,45 @@ Compose file content (first 200 chars): gateway_enabled: app_compose.gateway_enabled(), }; + // One-shot has no netd lifecycle, so a bridge NIC that only wanted the + // vCPU-scaled default drops to a single queue here exactly as it would on a + // server without netd. Anything still needing an interface was asked for + // explicitly, and is refused rather than silently downgraded. + let requested = if manifest.networks.is_empty() { + vec![config.cvm.networking.nic.clone()] + } else { + manifest.networks.clone() + }; + let mut runtime_networks = resolved_networks(&manifest, &config.cvm); + let clamped = clamp_queues_without_netd(&requested, &mut runtime_networks, &config.cvm, false); + // The server settles vhost after clamping, because clamping changes whether + // a NIC needs netd and that changes which netdev it gets. Skipping it here + // left `vhost_enabled()` reading as a request rather than a decision, so + // the launch warned about a `/dev/vhost-net` the netdev it then built does + // not open. + let vhost_denied = settle_vhost(&mut runtime_networks, &config.cvm); + if vhost_denied > 0 { + tracing::warn!( + "no qemu-bridge-helper found, so {vhost_denied} bridge interface(s) fall back to the \ + non-vhost bridge netdev; set cvm.qemu_bridge_helper to enable vhost" + ); + } + if clamped > 0 { + tracing::warn!( + "one-shot execution has no netd, so {clamped} bridge interface(s) fall back to a \ + single queue pair; run the VMM server to let queue pairs scale with vCPUs" + ); + } if !dry_run - && config.cvm.network_filter.mode == NetworkFilterMode::Libvirt - && resolved_networks(&manifest, &config.cvm) + && runtime_networks .iter() - .any(|network| network.mode == NetworkingMode::Bridge) + .any(|network| needs_netd_interface(network, &config.cvm)) { anyhow::bail!( - "one-shot execution does not manage libvirt-filtered TAP lifecycle; run the VMM server directly or use --dry-run" + "one-shot execution does not manage netd interface lifecycle; run the VMM server directly or use --dry-run" ); } - let runtime_networks = resolved_networks(&manifest, &config.cvm); let process_configs = vm_builder_config .config_qemu(&workdir_path, &config.cvm, &gpus, &runtime_networks) .context("Failed to build QEMU configuration")?; diff --git a/dstack/vmm/src/vmm-cli.py b/dstack/vmm/src/vmm-cli.py index 1a533434f..20e515520 100755 --- a/dstack/vmm/src/vmm-cli.py +++ b/dstack/vmm/src/vmm-cli.py @@ -919,8 +919,18 @@ def create_vm(self, args) -> None: params["kms_urls"] = args.kms_url if args.gateway_url: params["gateway_urls"] = args.gateway_url - if args.net: - params["networking"] = {"mode": args.net} + # "auto" is what a fresh deployment already does, so it only means + # something to `update`, where it clears a pinned count. + net_queues = None if args.net_queues == "auto" else args.net_queues + if args.net or args.net_vhost is not None or net_queues: + networking = {} + if args.net: + networking["mode"] = args.net + if args.net_vhost is not None: + networking["vhost"] = args.net_vhost + if net_queues: + networking["queues"] = net_queues + params["networking"] = networking app_id = args.app_id or self.calc_app_id(compose_content) print(f"App ID: {app_id}") @@ -1030,6 +1040,10 @@ def update_vm( no_gpus: bool = False, kms_urls: Optional[List[str]] = None, no_tee: Optional[bool] = None, + net: Optional[str] = None, + net_vhost: Optional[bool] = None, + net_vhost_inherit: bool = False, + net_queues: Optional[Union[int, str]] = None, ) -> None: """Update multiple aspects of a VM in one command.""" # Validate: --env-file requires --kms-url @@ -1153,6 +1167,75 @@ def update_vm( app_compose, indent=4, ensure_ascii=False ) + if net or net_vhost is not None or net_vhost_inherit or net_queues: + # The RPC replaces the whole NIC list, so merge into what the VM + # already has rather than silently dropping its other interfaces or + # un-pinning a bridge it was deployed with. + if vm_info_response is None: + vm_info_response = self.rpc_call("GetInfo", {"id": vm_id}) + if not vm_info_response.get("found", False): + raise Exception(f"VM with ID {vm_id} not found") + configuration = vm_info_response["info"].get("configuration") or {} + current = configuration.get("networks") or [] + if not current and configuration.get("networking"): + current = [configuration["networking"]] + if len(current) > 1: + raise Exception( + "this VM has multiple network interfaces; edit them through the " + "web UI or the UpgradeApp API rather than these flags" + ) + # Only the fields the deployment RPC accepts back travel with the + # update. macvtap_mode is node-controlled and can never be changed, + # so resending it can only fail if the node changed meanwhile. An + # empty mode is meaningful: it says the VM never named a backend + # and still follows the node's. + source = current[0] if current else {} + networking = { + key: source[key] + for key in ("mode", "bridge_name", "parent", "vhost", "queues") + if source.get(key) not in (None, "") + } + if net == "default": + # The only way back to "whatever backend the node runs". Without + # it a VM that named a mode once is pinned to it for life, since + # the merge above carries the reported mode forward on every + # later update. The data plane keeps whatever it was told. + networking.pop("mode", None) + networking.pop("bridge_name", None) + networking.pop("parent", None) + elif net: + networking["mode"] = net + # A field belongs to the mode that owns it. Carrying a bridge into + # a macvtap request, or a parent into a bridge one, asks the server + # about a field the caller never typed and has no flag to clear. + mode = networking.get("mode", "") + if mode and mode != "bridge": + networking.pop("bridge_name", None) + if mode and mode != "macvtap": + networking.pop("parent", None) + # Same rule for the data plane. User networking has neither a vhost + # backend nor multiple queues, so carrying an inherited pin into it + # is rejected for a flag the operator never typed -- and the two + # flags that would clear it are the ones they have not found yet. + # An explicitly typed value still earns the error: that one is + # theirs to be wrong about. + if mode == "user": + if net_vhost is None: + networking.pop("vhost", None) + if not net_queues: + networking.pop("queues", None) + if net_vhost is not None: + networking["vhost"] = net_vhost + elif net_vhost_inherit: + networking.pop("vhost", None) + if net_queues == "auto": + networking.pop("queues", None) + elif net_queues: + networking["queues"] = net_queues + upgrade_params["update_networking"] = True + upgrade_params["networks"] = [networking] + updates.append(f"networking ({networking})") + if user_config: upgrade_params["user_config"] = user_config updates.append("user config") @@ -1247,6 +1330,38 @@ def show_info(self, vm_id: str, json_output: bool = False) -> None: if info.get("shutdown_progress"): print(f"Shutdown: {info['shutdown_progress']}") + interfaces = info.get("interfaces") or [] + if interfaces: + print("\nNetwork Interfaces:") + for iface in interfaces: + parts = [ + f"{iface.get('netdev_id') or '-':<6}", + f"{iface.get('mode') or '-'}/{iface.get('backend') or '-'}", + iface.get("mac") or "-", + ] + if iface.get("bridge_name"): + parts.append(f"bridge={iface['bridge_name']}") + if iface.get("macvtap_mode"): + parts.append(f"macvtap_mode={iface['macvtap_mode']}") + # Absent, not false: custom mode carries an operator-written + # netdev string the VMM never parses, so it reports no data + # plane rather than asserting the resolved default over one that + # may well say vhost=on,queues=8. + vhost = iface.get("vhost") + parts.append( + "vhost=" + ("-" if vhost is None else ("on" if vhost else "off")) + ) + parts.append(f"queues={iface.get('queues') or '-'}") + print(" " + " ".join(parts)) + # A stopped VM has no interfaces to describe, so these are what its + # next launch would build -- which can differ from its last one. + # + # The server's own predicate, not the status string: a VM being + # removed with QEMU still up reports the interfaces that process + # built, and no status value says so. + if not info.get("running", False): + print(" (not running; shown as its next launch would build them)") + events = info.get("events", []) if events: print("\nRecent Events:") @@ -1536,6 +1651,26 @@ def save_whitelist(whitelist: List[str]) -> None: json.dump({"trusted_signers": whitelist}, f, indent=2) +def queue_count(value: str) -> Union[int, str]: + """Parse a queue pair count the node could act on, or "auto" to stop pinning one. + + Zero would otherwise reach the wire as "unset" and be answered with the + default, and a negative one as a decoding error naming a column offset -- + neither of which tells the caller what they asked for was impossible. + """ + if value == "auto": + return "auto" + try: + count = int(value) + except ValueError: + raise argparse.ArgumentTypeError(f"'{value}' is not a whole number or 'auto'") + if count < 1: + raise argparse.ArgumentTypeError( + f"queue pairs must be at least 1, or 'auto' to follow the vCPU count; got {count}" + ) + return count + + def main(): """Parse arguments and dispatch to the appropriate command handler.""" parser = argparse.ArgumentParser(description="dstack-vmm CLI - Manage VMs") @@ -1831,9 +1966,31 @@ def _patched_format_help(): ) deploy_parser.add_argument( "--net", - choices=["bridge", "user"], + choices=["bridge", "user", "macvtap"], help="Networking mode (default: use global config)", ) + net_vhost = deploy_parser.add_mutually_exclusive_group() + net_vhost.add_argument( + "--net-vhost", + dest="net_vhost", + action="store_true", + default=None, + help="Use the host kernel vhost-net data plane (default: use global config)", + ) + net_vhost.add_argument( + "--net-no-vhost", + dest="net_vhost", + action="store_false", + help="Keep packet processing in the QEMU main loop", + ) + deploy_parser.add_argument( + "--net-queues", + type=queue_count, + metavar="N", + help="virtio-net queue pairs, bounded by the node's max_net_queues. " + "Without --net, the node's own networking mode is kept " + "(default: use global config)", + ) # Images command lsimage_parser = subparsers.add_parser("lsimage", help="List available images") @@ -1934,6 +2091,42 @@ def _patched_format_help(): "--env-file", help="File with environment variables to encrypt" ) update_parser.add_argument("--user-config", help="Path to user config file") + update_parser.add_argument( + "--net", + choices=["bridge", "user", "macvtap", "default"], + help=( + "Networking mode (applies from the next boot). 'default' stops " + "pinning a mode and follows the node's, the way --net-queues auto " + "and --net-vhost-default stop pinning the data plane" + ), + ) + update_net_vhost = update_parser.add_mutually_exclusive_group() + update_net_vhost.add_argument( + "--net-vhost", + dest="net_vhost", + action="store_true", + default=None, + help="Use the host kernel vhost-net data plane", + ) + update_net_vhost.add_argument( + "--net-no-vhost", + dest="net_vhost", + action="store_false", + help="Keep packet processing in the QEMU main loop", + ) + update_net_vhost.add_argument( + "--net-vhost-default", + dest="net_vhost_inherit", + action="store_true", + help="Stop pinning vhost and follow the node default again", + ) + update_parser.add_argument( + "--net-queues", + type=queue_count, + metavar="N", + help="virtio-net queue pairs, bounded by the node's max_net_queues. " + "Use 'auto' to stop pinning a count and follow the vCPU count again", + ) # Port mapping options (mutually exclusive with --no-ports) port_group = update_parser.add_mutually_exclusive_group() port_group.add_argument( @@ -2077,6 +2270,10 @@ def _patched_format_help(): no_gpus=args.no_gpus if hasattr(args, "no_gpus") else False, kms_urls=args.kms_url, no_tee=args.no_tee, + net=args.net, + net_vhost=args.net_vhost, + net_vhost_inherit=getattr(args, "net_vhost_inherit", False), + net_queues=args.net_queues, ) elif args.command == "kms": if not args.kms_action: diff --git a/dstack/vmm/ui/src/components/CreateVmDialog.ts b/dstack/vmm/ui/src/components/CreateVmDialog.ts index 82653a62d..27adcaecd 100644 --- a/dstack/vmm/ui/src/components/CreateVmDialog.ts +++ b/dstack/vmm/ui/src/components/CreateVmDialog.ts @@ -22,7 +22,44 @@ const CreateVmDialogComponent = { portMappingEnabled: { type: Boolean, required: true }, networkingModes: { type: Array, required: true }, defaultBridge: { type: String, default: '' }, + maxNetQueues: { type: Number, default: 0 }, defaultNetworkingLabel: { type: String, required: true }, + defaultModeTunable: { type: Boolean, default: false }, + defaultVhostOn: { type: Boolean, default: false }, + }, + methods: { + // Whether this NIC will end up on the vhost data plane. An unset select + // means it follows the node, and a node with vhost off gives one queue pair + // however many vCPUs the VM has -- so the answer is not readable from this + // row alone. + vhostOn(network: { vhost?: string }) { + if (network.vhost === 'on') { + return true; + } + if (network.vhost === 'off') { + return false; + } + return (this as any).defaultVhostOn; + }, + // What an empty queues field actually resolves to. It is the vCPU count + // only when vhost is on: with vhost off the backend has no multiqueue data + // plane and the NIC gets exactly one queue pair. + queuesHint(network: { vhost?: string }) { + if (!this.vhostOn(network)) { + return 'virtio-net queue pairs. Empty means one queue pair, because vhost is off.'; + } + const cap = (this as any).maxNetQueues + ? `, capped at ${(this as any).maxNetQueues} on this node` + : ''; + return `virtio-net queue pairs. Empty follows the VM's vCPU count${cap}.`; + }, + queuesPlaceholder(network: { vhost?: string }) { + if (!this.vhostOn(network)) { + return 'queues: auto (1, vhost off)'; + } + const cap = (this as any).maxNetQueues ? ` (max ${(this as any).maxNetQueues})` : ''; + return `queues: auto${cap}`; + }, }, emits: ['close', 'submit', 'load-compose'], template: /* html */ ` @@ -161,6 +198,7 @@ const CreateVmDialogComponent = {
{{ defaultNetworkingLabel }}
+ + + + + {{ defaultBridge ? 'Leave empty to use the VMM default bridge from vmm.toml: ' + defaultBridge + '.' : 'No default bridge is configured in vmm.toml; enter a bridge interface name.' }} Guest IP is assigned by host DHCP on that bridge and reported after boot. + + Leave empty to use the macvtap parent from vmm.toml. The forwarding mode stays node-controlled. + + + {{ defaultNetworkingLabel }} has no vhost-net or multiqueue data plane, so these two settings are recorded + but stay dormant until this node's default backend can carry them. +
- + diff --git a/dstack/vmm/ui/src/components/UpdateVmDialog.ts b/dstack/vmm/ui/src/components/UpdateVmDialog.ts index 5dc9568b1..ec2bfbfa6 100644 --- a/dstack/vmm/ui/src/components/UpdateVmDialog.ts +++ b/dstack/vmm/ui/src/components/UpdateVmDialog.ts @@ -21,10 +21,47 @@ const UpdateVmDialogComponent = { portMappingEnabled: { type: Boolean, required: true }, networkingModes: { type: Array, required: true }, defaultBridge: { type: String, default: '' }, + maxNetQueues: { type: Number, default: 0 }, defaultNetworkingLabel: { type: String, required: true }, + defaultModeTunable: { type: Boolean, default: false }, + defaultVhostOn: { type: Boolean, default: false }, kmsEnabled: { type: Boolean, required: true }, composeHashPreview: { type: String, required: true }, }, + methods: { + // Whether this NIC will end up on the vhost data plane. An unset select + // means it follows the node, and a node with vhost off gives one queue pair + // however many vCPUs the VM has -- so the answer is not readable from this + // row alone. + vhostOn(network: { vhost?: string }) { + if (network.vhost === 'on') { + return true; + } + if (network.vhost === 'off') { + return false; + } + return (this as any).defaultVhostOn; + }, + // What an empty queues field actually resolves to. It is the vCPU count + // only when vhost is on: with vhost off the backend has no multiqueue data + // plane and the NIC gets exactly one queue pair. + queuesHint(network: { vhost?: string }) { + if (!this.vhostOn(network)) { + return 'virtio-net queue pairs. Empty means one queue pair, because vhost is off.'; + } + const cap = (this as any).maxNetQueues + ? `, capped at ${(this as any).maxNetQueues} on this node` + : ''; + return `virtio-net queue pairs. Empty follows the VM's vCPU count${cap}.`; + }, + queuesPlaceholder(network: { vhost?: string }) { + if (!this.vhostOn(network)) { + return 'queues: auto (1, vhost off)'; + } + const cap = (this as any).maxNetQueues ? ` (max ${(this as any).maxNetQueues})` : ''; + return `queues: auto${cap}`; + }, + }, emits: ['close', 'submit', 'load-compose'], template: /* html */ `
@@ -150,24 +187,67 @@ const UpdateVmDialogComponent = {
{{ defaultNetworkingLabel }}
+ + + + + + {{ defaultBridge ? 'Leave empty to use the VMM default bridge from vmm.toml: ' + defaultBridge + '.' : 'No default bridge is configured in vmm.toml; enter a bridge interface name.' }} Guest IP is assigned by host DHCP on that bridge and reported after boot. + + Leave empty to use the macvtap parent from vmm.toml. The forwarding mode stays node-controlled. + + + {{ defaultNetworkingLabel }} has no vhost-net or multiqueue data plane, so these two settings are recorded + but stay dormant until this node's default backend can carry them. +
- +
diff --git a/dstack/vmm/ui/src/composables/useVmManager.ts b/dstack/vmm/ui/src/composables/useVmManager.ts index 1abb5ab75..70e9db9df 100644 --- a/dstack/vmm/ui/src/composables/useVmManager.ts +++ b/dstack/vmm/ui/src/composables/useVmManager.ts @@ -92,6 +92,7 @@ type VmListItem = { shutdown_progress?: string; image_version?: string; interfaces?: VmmTypes.INetworkInterfaceStatus[]; + running?: boolean; configuration?: VmConfiguration; appCompose?: AppCompose; }; @@ -111,6 +112,15 @@ type PortFormEntry = { type NetworkFormEntry = { mode: string; bridge_name?: string; + /** Pinned at deployment for macvtap NICs; carried through edits unchanged. */ + parent?: string; + /** '' inherits the node default, otherwise 'on' or 'off'. */ + vhost?: string; + /** + * '' lets the queue count follow the vCPU count. A `number` once the operator + * types into the input: `v-model` casts for ``. + */ + queues?: string | number; }; type VmFormState = { @@ -351,6 +361,18 @@ fi return Array.from(new Set(fallback)); }); const defaultBridge = computed(() => config.value.networking?.default_bridge || ''); + const maxNetQueues = computed(() => config.value.networking?.max_queues || 0); + const defaultVhostOn = computed(() => !!config.value.networking?.default_vhost); + // Whether the node's own default backend can carry vhost-net and multiqueue. + // The RPC accepts the two tuning fields on a mode-less entry regardless -- + // deliberately, so a NIC that inherits its backend stays tunable across a node + // change -- but on a node whose default is user or custom they lie dormant, + // and an operator who sets them deserves to be told that rather than discover + // it in the interfaces panel afterwards. + const defaultModeTunable = computed(() => { + const mode = config.value.networking?.default_mode || ''; + return mode === 'bridge' || mode === 'macvtap'; + }); const defaultNetworkingLabel = computed(() => { const mode = config.value.networking?.default_mode || ''; if (mode === 'bridge') { @@ -450,16 +472,67 @@ fi return configured.map((network) => ({ mode: network.mode || '', bridge_name: network.bridge_name || '', + parent: network.parent || '', + vhost: network.vhost === null || network.vhost === undefined ? '' : (network.vhost ? 'on' : 'off'), + queues: network.queues ? String(network.queues) : '', })); }; + // Queue pairs, whatever shape the model is in. + // + // Not a string: Vue's `v-model` casts for ``, so this + // field is a `string` while it holds a value loaded from `GetInfo` and a + // `number` the moment the operator types into it. Assuming either one is how + // this threw a `TypeError` out of every deploy that set a queue count. + // + // The number input already refuses everything but a numeric literal, and the + // cast turns `2.7` into `2.7` rather than into `parseInt`'s `2`, so what is + // left to check is that the value is a whole number at least one. The node's + // cap is deliberately *not* checked here: the server widens it by whatever a + // VM already holds, so a client-side copy would refuse an update the server + // accepts and leave that VM's networking uneditable. + const parseQueueCount = (raw: unknown, label: string): number | undefined => { + if (raw === null || raw === undefined || raw === '') { + return undefined; + } + const queues = typeof raw === 'number' ? raw : Number(String(raw).trim()); + if (!Number.isInteger(queues) || queues < 1) { + throw new Error(`${label}: queue pairs must be a whole number of at least 1, or empty to follow the vCPU count; got '${raw}'`); + } + return queues; + }; + + // Nothing is filtered out. A mode-less entry is the "keep the node's backend, + // change only the data plane" override the RPC accepts, and is what a VM + // deployed that way reports back; dropping it would delete a NIC and renumber + // the ones after it, which changes their MAC addresses. + // A row added here starts with no mode, which the RPC reads as "keep the + // node's backend" rather than as a missing field, so the only way to reach an + // entry it refuses is to empty a loaded one's bridge or parent, and that + // earns an error rather than silence. const normalizeNetworks = (networks: NetworkFormEntry[] = []): VmmTypes.INetworkingConfig[] => networks - .map((network) => ({ - mode: (network.mode || '').trim(), - bridge_name: network.mode === 'bridge' ? (network.bridge_name || '').trim() : '', - })) - .filter((network) => network.mode.length > 0); + .map((network, index) => { + // Leave vhost and queues unset unless the operator picked something, so + // the node keeps owning them and can still change them later. + const entry: VmmTypes.INetworkingConfig = { + mode: (network.mode || '').trim(), + bridge_name: network.mode === 'bridge' ? (network.bridge_name || '').trim() : '', + parent: network.mode === 'macvtap' ? (network.parent || '').trim() : '', + }; + // The tuning controls are hidden for user mode, so sending values the + // operator cannot see would fail the deploy with nothing to fix. + if (network.mode !== 'user') { + if (network.vhost === 'on' || network.vhost === 'off') { + entry.vhost = network.vhost === 'on'; + } + const queues = parseQueueCount(network.queues, `network ${index + 1}`); + if (queues !== undefined) { + entry.queues = queues; + } + } + return entry; + }); function networkModeLabel(mode?: string | null) { if (!mode) { @@ -1014,6 +1087,11 @@ type CreateVmPayloadSource = { function showDeployDialog() { showCreateDialog.value = true; vmForm.value.encryptedEnvs = []; + // A cancelled deploy and "Clone config" both leave their networking behind + // in the shared form. Carrying it into the next deploy would silently pin + // that VM's backend, bridge, macvtap parent and data plane to another VM's + // -- invisibly, since the operator never opened the Networking section. + vmForm.value.networks = []; vmForm.value.app_id = null; vmForm.value.swapValue = 0; vmForm.value.swapUnit = 'GB'; @@ -1833,7 +1911,10 @@ type CreateVmPayloadSource = { config, networkingModes, defaultBridge, + maxNetQueues, defaultNetworkingLabel, + defaultModeTunable, + defaultVhostOn, composeHashPreview, updateComposeHashPreview, showDeployDialog, diff --git a/dstack/vmm/ui/src/styles/main.css b/dstack/vmm/ui/src/styles/main.css index 7a7b0ee72..113109a42 100644 --- a/dstack/vmm/ui/src/styles/main.css +++ b/dstack/vmm/ui/src/styles/main.css @@ -620,7 +620,7 @@ h1, h2, h3, h4, h5, h6 { .runtime-network-item { display: grid; - grid-template-columns: 1.1fr 0.6fr 0.8fr 1.4fr; + grid-template-columns: 1.1fr 0.6fr 0.8fr 1.4fr 0.7fr 0.7fr; gap: 12px; overflow-wrap: anywhere; } @@ -1420,13 +1420,35 @@ h1, h2, h3, h4, h5, h6 { font-size: 14px; } +/* Fixed track widths summed to a floor wider than the dialog, which put a + horizontal scrollbar under the Networking section on any window narrower than + about 930px. The tracks shrink and the tuning pair wraps instead. */ .network-config-row { display: grid; - grid-template-columns: 160px minmax(180px, 1fr) 100px; + grid-template-columns: minmax(120px, 160px) minmax(140px, 1fr) minmax(200px, auto) auto; gap: 12px; align-items: center; } +.network-config-tuning { + display: flex; + flex-wrap: wrap; + gap: 8px; + min-width: 0; +} + +.network-config-tuning select { + flex: 1 1 140px; + min-width: 110px; + max-width: 160px; +} + +.network-config-tuning input { + flex: 1 1 150px; + min-width: 90px; + max-width: 190px; +} + .network-config-placeholder { min-height: 1px; } diff --git a/dstack/vmm/ui/src/templates/app.html b/dstack/vmm/ui/src/templates/app.html index 139155032..965f7bcc2 100644 --- a/dstack/vmm/ui/src/templates/app.html +++ b/dstack/vmm/ui/src/templates/app.html @@ -80,7 +80,10 @@

dstack-vmm

:port-mapping-enabled="config.portMappingEnabled" :networking-modes="networkingModes" :default-bridge="defaultBridge" + :max-net-queues="maxNetQueues" :default-networking-label="defaultNetworkingLabel" + :default-mode-tunable="defaultModeTunable" + :default-vhost-on="defaultVhostOn" @close="showCreateDialog = false" @submit="createVm" @load-compose="loadComposeFile" @@ -95,7 +98,10 @@

dstack-vmm

:port-mapping-enabled="config.portMappingEnabled" :networking-modes="networkingModes" :default-bridge="defaultBridge" + :max-net-queues="maxNetQueues" :default-networking-label="defaultNetworkingLabel" + :default-mode-tunable="defaultModeTunable" + :default-vhost-on="defaultVhostOn" :kms-enabled="kmsEnabled(updateDialog.vm || {})" :compose-hash-preview="updateComposeHashPreview" @close="updateDialog.show = false" @@ -345,11 +351,16 @@

Port Mappings

VMM Network Interfaces

+ + Not running; shown as the next launch would build them. +
{{ networkModeLabel(iface.mode) }} / {{ iface.backend || '-' }} {{ iface.netdev_id || '-' }} - {{ iface.bridge_name || '-' }} + {{ iface.bridge_name || iface.macvtap_mode || '-' }} {{ iface.mac || '-' }} + vhost: {{ iface.vhost === null || iface.vhost === undefined ? '-' : (iface.vhost ? 'on' : 'off') }} + queues: {{ iface.queues || '-' }}
diff --git a/dstack/vmm/vmm.toml b/dstack/vmm/vmm.toml index 26f221866..3a83d5016 100644 --- a/dstack/vmm/vmm.toml +++ b/dstack/vmm/vmm.toml @@ -54,6 +54,16 @@ allowed_network_modes = ["user", "bridge"] # Empty allowlists mean callers can only use the node networking defaults. allowed_bridges = [] allowed_macvtap_parents = [] +# Largest virtio-net queue pair count a deployment request may ask for. With +# vhost on, queue pairs otherwise default to the VM's vCPU count, capped at +# 16 (without vhost the default is a single queue pair); raising this +# above 16 widens what a caller may request without moving that default, and +# lowering it below 16 lowers the default too. Bridge mode needs netd for +# anything above 1, because qemu-bridge-helper cannot create a multiqueue TAP. +# Without netd an unfiltered bridge NIC that took the default drops to one +# queue; one that asked for a count keeps it and fails to launch instead, so +# the caller learns their request was not met. +max_net_queues = 16 use_mrconfigid = true # QEMU flags @@ -62,6 +72,10 @@ use_mrconfigid = true #qemu_version = "" qemu_pci_hole64_size = 0 qemu_hotplug_off = false +# Path to qemu-bridge-helper, needed by vhost bridge networking because QEMU's +# `tap` netdev, unlike its `bridge` netdev, has no compiled-in default. Empty +# probes the known distribution locations. +#qemu_bridge_helper = "/usr/lib/qemu/qemu-bridge-helper" # TDX attestation/hash scheme policy: # - "legacy": digest.txt + legacy verifier # - "lite": digest.txt + measurement.tdx.cbor + no-QEMU verifier @@ -111,6 +125,16 @@ product_name = "dstack" [cvm.networking] mode = "user" +# Kernel vhost-net data plane. Off by default: enabling it changes the +# virtio-net device of every bridge/macvtap VM on its next boot (vhost plus +# vCPU-scaled queue pairs) and requires /dev/vhost-net to be accessible to +# the account QEMU runs under — verify that first, or QEMU exits at launch. +# With it off, QEMU drains every packet on its single main loop thread, so a +# CVM cannot exceed one core's worth of packet processing no matter how many +# vCPUs it has. The user-mode backend has no vhost support and ignores this. +# Individual VMs may override it. +vhost = false + # for mode = "user" net = "10.0.2.0/24" dhcp_start = "10.0.2.10" @@ -120,21 +144,36 @@ restrict = false # bridge = "virbr0" # Optional filtering for bridge interfaces only. It does not apply to macvtap. -# "none" preserves the existing QEMU bridge-helper behavior and has no -# netd/libvirt dependency. +# "none" installs no nwfilter binding. It does not by itself remove the netd +# dependency: netd also builds the multiqueue TAP that qemu-bridge-helper +# cannot create, so a bridge node without netd is limited to one queue pair. [cvm.network_filter] mode = "none" filter = "clean-traffic" parameters = {} -# Shared privileged networking service. Only used when network_filter.mode is -# "libvirt". Socket filesystem permissions authorize clients. +# Shared privileged networking service. Used for macvtap NICs, for libvirt +# filtering, and for multiqueue bridge NICs. Socket filesystem permissions +# authorize clients. [netd] socket = "/run/dstack/netd.sock" # Applied when netd creates the socket itself. A systemd socket unit controls # its own SocketMode instead. socket_mode = 0o660 libvirt_uri = "qemu:///system" +# The bridge filtering policy netd enforces and applies: whether a binding is +# required, which nwfilter it names, and with what parameters. netd holds this +# itself rather than taking it from each request, because a caller that chose +# the filter could name one that drops nothing and still satisfy a policy that +# only asked for "some filter". Left unset it follows [cvm.network_filter] in +# this same file, which is the whole answer when netd and the VMM share one +# vmm.toml. Set it explicitly when netd runs with a config that has no [cvm] +# section, so the daemon holding the privilege never infers policy from a file +# that does not state it. +#[netd.network_filter] +#mode = "libvirt" +#filter = "clean-traffic" +#parameters = {} [cvm.port_mapping] enabled = false