Skip to content

Commit 7e1e688

Browse files
author
Kevin Wang
committed
feat(netd): report what netd implements and what it still holds
netd creates persistent TAPs, so an interface outlives the QEMU that used it. A VMM that died between creating one and recording it leaves an interface nothing can name again: `check` and `remove` both answer about an identity, and the identity is exactly what was lost. Nothing could enumerate them, so the leak was invisible until someone went looking at `ip link`. Add `list` and `remove_tap` to close that, and run them from the VMM at startup next to the cleanup it already does for orphaned supervisor processes. Listing is scoped to an instance namespace: one host runs several VMMs against one netd, and a caller reconciling its own VMs must not mistake another's interfaces for orphans. netd records the namespace in the TAP's `ifalias` rather than in a state file, so a crash leaves nothing to repair, and records only the namespace because IFALIAS_MAX is 255 bytes while two 128-byte identifiers are not -- a caller derives every TAP name it owns anyway. Reconciliation over-approximates which TAPs are still wanted, since keeping a leak is the cheaper mistake than deleting a live interface out from under a running QEMU. `remove_tap` is the one operation that names an interface instead of an identity, because the identity is what the caller no longer has. The name must be one `tap_name` could have produced, so it reaches nothing netd did not create. Add `hello` alongside it. Support has so far been detected by a field's own absence from a response -- `queues` is documented that way -- which works once per field, only for fields that are echoed back, and says nothing until something has already failed. netd ships as its own package on its own release cadence and a host may run an implementation that is not this one, so version skew is the normal case; asking once at startup beats inferring repeatedly. A netd that predates the operation rejects it, and that rejection is the answer rather than an error. Add `workdir` to both prepare operations. It is untrusted and never read for a decision, and exists so an operator reading netd's log can get from an opaque TAP name back to the VM that asked for it.
1 parent 1aa1d35 commit 7e1e688

3 files changed

Lines changed: 532 additions & 19 deletions

File tree

docs/libvirt-network-filter.md

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,55 @@ Deploy VMM instances that do not share this trust boundary with dedicated netd
101101
sockets and distinct filesystem permissions.
102102

103103
`netd` invokes fixed absolute `ip` and `virsh` executables with separate
104-
arguments. It never accepts a command, executable path, TAP name, or raw XML
105-
from a client. Filter XML is generated internally with XML escaping and is
106-
validated by libvirt.
104+
arguments. It never accepts a command, executable path, or raw XML from a
105+
client. Filter XML is generated internally with XML escaping and is validated by
106+
libvirt.
107+
108+
### Asking what a netd implements
109+
110+
`netd` ships as its own package on its own release cadence, and a host may run
111+
an implementation that is not this one, so a VMM newer than its `netd` is the
112+
normal case rather than the exception. `hello` answers with a protocol version
113+
and a list of feature names, which the VMM asks for once at startup instead of
114+
inferring support one failed operation at a time. A `netd` that predates the
115+
operation rejects it, and that rejection *is* the answer: it claims nothing.
116+
117+
### Finding interfaces whose VM is gone
118+
119+
A `netd`-created TAP outlives the QEMU that used it. A VMM that died between
120+
creating one and recording it leaves an interface nothing can name again —
121+
`check` and `remove` both answer about an identity, and the identity is exactly
122+
what was lost. Two operations close that:
123+
124+
- `list` enumerates the interfaces `netd` holds for one instance namespace.
125+
- `remove_tap` deletes one by name. The name must be one `tap_name` could have
126+
produced, so it reaches nothing `netd` did not create, and any binding is
127+
cleared best-effort because the caller cannot say whether there was one.
128+
129+
The VMM runs this at startup, alongside the cleanup it already does for
130+
supervisor processes: anything `netd` holds that maps to no VM it knows is
131+
removed. It over-approximates which TAPs are still wanted, since keeping a leak
132+
is the cheaper mistake.
133+
134+
Listing is scoped to an instance namespace because one host runs several VMMs
135+
against one `netd`, and a caller reconciling its own VMs must not mistake
136+
another's interfaces for orphans. `netd` records the namespace in the TAP's
137+
`ifalias` rather than in a state file, so a crash leaves nothing to repair. The
138+
identity itself is not recorded: `IFALIAS_MAX` is 255 bytes and two 128-byte
139+
identifiers do not fit, and a caller can already derive every TAP name it owns.
140+
The namespace is the only part it cannot.
141+
142+
An interface created by an older `netd` carries no stamp and is therefore listed
143+
by nobody. It is left in place rather than attributed to whoever asks, which
144+
keeps reconciliation from deleting an interface it cannot prove is an orphan.
145+
146+
### Fields that only travel
147+
148+
`workdir` on both prepare operations names the VM's directory on the host. It is
149+
untrusted and never read for a decision — any process that can reach the socket
150+
can assert anything — and exists so that an operator reading `netd`'s log can get
151+
from an opaque TAP name back to the VM that asked for it without going through
152+
the VMM.
107153

108154
## Deployment modes
109155

dstack/vmm/src/app.rs

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,12 @@ pub struct App {
314314

315315
const GUEST_AGENT_RPC_TIMEOUT: Duration = Duration::from_secs(30);
316316

317+
/// How many NIC indices per VM startup reconciliation treats as possibly still
318+
/// in use. Deliberately larger than any VM the VMM builds: naming a TAP that no
319+
/// longer belongs to anything keeps a leak, while failing to name a live one
320+
/// would delete it.
321+
const RECONCILE_MAX_NICS: usize = 16;
322+
317323
impl App {
318324
pub(crate) fn lock(&self) -> MutexGuard<'_, AppState> {
319325
self.state.lock().or_panic("mutex poisoned")
@@ -564,6 +570,13 @@ impl App {
564570
return Ok(());
565571
}
566572
let qemu_uid = Uid::effective().as_raw();
573+
// Only ever read back out of a log line. netd is told where the VM
574+
// lives so an operator holding an opaque TAP name can get to the VM
575+
// without going through the VMM first.
576+
let workdir = self
577+
.work_dir(&vm.manifest.id)
578+
.map(|dir| dir.path().display().to_string())
579+
.unwrap_or_default();
567580
let mut prepared = Vec::new();
568581
for (nic_index, network) in networks.iter_mut().enumerate() {
569582
if !needs_netd_interface(network, &self.config.cvm) {
@@ -593,6 +606,7 @@ impl App {
593606
// libvirt at all.
594607
filtered,
595608
queues,
609+
workdir: workdir.clone(),
596610
}),
597611
NetworkingMode::Macvtap => NetdRequest::PrepareMacvtap(PrepareMacvtapRequest {
598612
identity: identity.clone(),
@@ -601,6 +615,7 @@ impl App {
601615
qemu_uid,
602616
mode: network.macvtap_mode.clone(),
603617
queues,
618+
workdir: workdir.clone(),
604619
}),
605620
NetworkingMode::User | NetworkingMode::Custom => continue,
606621
};
@@ -1067,9 +1082,75 @@ impl App {
10671082
}
10681083
}
10691084

1085+
self.reconcile_netd_interfaces(&loaded_vm_ids).await;
1086+
10701087
Ok(())
10711088
}
10721089

1090+
/// Deletes host interfaces netd still holds for VMs this VMM no longer has.
1091+
///
1092+
/// The same cleanup the loop above does for supervisor processes, for the
1093+
/// other resource a crash can strand. A netd-created TAP outlives the QEMU
1094+
/// that used it, so a VMM that died between creating one and recording it
1095+
/// leaves an interface nothing can find again: `Check` answers about an
1096+
/// identity, and the identity is exactly what was lost.
1097+
///
1098+
/// Failure is never fatal. Most nodes run no netd at all, and a leaked TAP
1099+
/// costs a name and an ifindex -- not a reason to refuse to start.
1100+
async fn reconcile_netd_interfaces(&self, loaded_vm_ids: &HashSet<String>) {
1101+
let socket = &self.config.netd.socket;
1102+
let instance_id = &self.config.cvm.instance_id;
1103+
// Unreachable is the ordinary case: most nodes run no netd at all.
1104+
let Ok(capabilities) = netd::capabilities(socket).await else {
1105+
debug!("no netd on this node, so nothing to reconcile");
1106+
return;
1107+
};
1108+
info!(
1109+
version = capabilities.version,
1110+
features = ?capabilities.features,
1111+
"netd is available"
1112+
);
1113+
if !capabilities.has("list") {
1114+
// Asking anyway would log a failure on every start of a node whose
1115+
// netd is simply older than this VMM.
1116+
info!("netd cannot list its interfaces, so orphans are left in place");
1117+
return;
1118+
}
1119+
let held = match netd::list(socket, instance_id).await {
1120+
Ok(held) => held,
1121+
Err(error) => {
1122+
warn!(%error, "failed to list netd interfaces");
1123+
return;
1124+
}
1125+
};
1126+
if held.is_empty() {
1127+
return;
1128+
}
1129+
// Over-approximate on purpose. A manifest edited while the VMM was down
1130+
// can leave a TAP at a NIC index the VM no longer has, and treating
1131+
// that as expected only means a leak survives -- the opposite mistake
1132+
// would delete a live interface out from under a running QEMU.
1133+
let mut expected = HashSet::new();
1134+
for vm_id in loaded_vm_ids {
1135+
for nic_index in 0..RECONCILE_MAX_NICS {
1136+
expected.insert(netd::tap_name(&InterfaceIdentity {
1137+
instance_id: instance_id.clone(),
1138+
vm_id: vm_id.clone(),
1139+
nic_index,
1140+
}));
1141+
}
1142+
}
1143+
for interface in held {
1144+
if expected.contains(&interface.tap) {
1145+
continue;
1146+
}
1147+
info!(tap = %interface.tap, kind = %interface.kind, "removing orphaned netd interface");
1148+
if let Err(error) = netd::remove_tap(socket, &interface.tap).await {
1149+
warn!(tap = %interface.tap, %error, "failed to remove orphaned netd interface");
1150+
}
1151+
}
1152+
}
1153+
10731154
/// Reload VMs directory and sync with memory state while preserving statistics
10741155
pub async fn reload_vms_sync(&self) -> Result<ReloadVmsResponse> {
10751156
let vm_path = self.vm_dir();

0 commit comments

Comments
 (0)