Re: [PATCH v2 5/7] host: integrate router

Alyssa Ross
2025-11-29T13:46:53Z

[-- Attachment #1: Type: text/plain, Size: 11759 bytes --]

Yureka Lilian <yureka@cyberchaos.dev> writes:

> This removes the old host bridge + taps glue, and instead connects the
> apps to their net provider's router instance.
>
> Signed-off-by: Yureka Lilian <yureka@cyberchaos.dev>
> ---
> host/rootfs/default.nix | 4 +-
> host/rootfs/file-list.mk | 3 +
> .../data/service/spectrum-router/down | 0
> .../template/data/service/spectrum-router/run | 13 ++++
> .../image/usr/bin/assign-driver-router-iface | 11 +++
> host/rootfs/image/usr/bin/run-vmm | 12 +--
> host/rootfs/image/usr/bin/vm-import | 13 ----
> pkgs/overlay.nix | 1 +
> tools/start-vmm/ch.rs | 38 ++--------
> tools/start-vmm/lib.rs | 76 +++++++++++++------
> tools/start-vmm/meson.build | 2 +-
> tools/start-vmm/net-util.c | 39 ----------
> tools/start-vmm/net-util.h | 6 --
> tools/start-vmm/net.c | 55 --------------
> tools/start-vmm/net.rs | 11 ---
> tools/start-vmm/tests/meson.build | 5 --
> .../start-vmm/tests/tap_open-name-too-long.c | 20 -----
> tools/start-vmm/tests/tap_open.c | 28 -------
> 18 files changed, 89 insertions(+), 248 deletions(-)
> create mode 100644 host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/spectrum-router/down
> create mode 100755 host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/spectrum-router/run
> create mode 100755 host/rootfs/image/usr/bin/assign-driver-router-iface
> delete mode 100644 tools/start-vmm/net-util.c
> delete mode 100644 tools/start-vmm/net-util.h
> delete mode 100644 tools/start-vmm/net.c
> delete mode 100644 tools/start-vmm/tests/tap_open-name-too-long.c
> delete mode 100644 tools/start-vmm/tests/tap_open.c
>
> diff --git a/host/rootfs/default.nix b/host/rootfs/default.nix
> index 4bbbe23..3b8557c 100644
> --- a/host/rootfs/default.nix
> +++ b/host/rootfs/default.nix
> @@ -8,7 +8,7 @@ import ../../lib/call-package.nix (
> }:
> pkgsMusl.callPackage (
>
> -{ spectrum-host-tools
> +{ spectrum-host-tools, spectrum-router
> , lib, stdenvNoCC, nixos, runCommand, writeClosure, erofs-utils, s6-rc
> , busybox, cloud-hypervisor, cosmic-files, crosvm, cryptsetup
> , dejavu_fonts, dbus, execline, foot, fuse3, iproute2, inotify-tools
> @@ -27,7 +27,7 @@ let
> cloud-hypervisor cosmic-files crosvm cryptsetup dbus execline
> fuse3 inotify-tools iproute2 jq kmod mdevd s6 s6-linux-init s6-rc
> socat spectrum-host-tools systemd util-linuxMinimal virtiofsd
> - xdg-desktop-portal-spectrum-host
> + xdg-desktop-portal-spectrum-host spectrum-router

Usually I try to keep these sorted, but I can always fix that sort of
thing up myself.

> diff --git a/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/spectrum-router/down b/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/spectrum-router/down
> new file mode 100644
> index 0000000..e69de29
> diff --git a/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/spectrum-router/run b/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/spectrum-router/run
> new file mode 100755
> index 0000000..fae9d9d
> --- /dev/null
> +++ b/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/spectrum-router/run
> @@ -0,0 +1,13 @@
> +#!/bin/execlineb -P
> +# SPDX-License-Identifier: EUPL-1.2+
> +# SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev>
> +
> +importas -i VM VM
> +
> +background {
> + assign-driver-router-iface ${VM}

You can just write $VM here since it's a whole word on its own.

> +}
> +
> +export RUST_LOG debug

This intentioally still here?

> +spectrum-router --app-listen-path ${VM}/router-app.sock --driver-listen-path ${VM}/router-driver.sock
> +
> diff --git a/host/rootfs/image/usr/bin/assign-driver-router-iface b/host/rootfs/image/usr/bin/assign-driver-router-iface
> new file mode 100755
> index 0000000..c555fb6
> --- /dev/null
> +++ b/host/rootfs/image/usr/bin/assign-driver-router-iface
> @@ -0,0 +1,11 @@
> +#!/bin/execlineb -S1
> +# SPDX-License-Identifier: EUPL-1.2+
> +# SPDX-FileCopyrightText: 2025 Alyssa Ross <hi@alyssa.is>
> +# SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev>
> +
> +# This script is to be called once it is known that this VM is a driver VM
> +# (net provider) AND the vmm endpoint is ready.
> +# It add the interface between the router and the driver VM.

add*s*

> diff --git a/tools/start-vmm/ch.rs b/tools/start-vmm/ch.rs
> index abe1742..56b18f4 100644
> --- a/tools/start-vmm/ch.rs
> +++ b/tools/start-vmm/ch.rs
> @@ -1,7 +1,7 @@
> // SPDX-License-Identifier: EUPL-1.2+
> // SPDX-FileCopyrightText: 2022-2024 Alyssa Ross <hi@alyssa.is>
> +// SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev>
>
> -use std::convert::TryFrom;
> use std::ffi::OsStr;
> use std::fs::File;
> use std::io::Write;
> @@ -10,7 +10,6 @@ use std::num::NonZeroI32;
> use std::os::unix::prelude::*;
> use std::path::Path;
> use std::process::{Command, Stdio};
> -use std::string::FromUtf8Error;
>
> use miniserde::{Serialize, json};
>
> @@ -46,7 +45,7 @@ pub struct GpuConfig {
>
> #[derive(Serialize)]
> pub struct NetConfig {
> - pub fd: RawFd,
> + pub vhost_user_sock: String,
> pub id: String,
> pub mac: MacAddress,
> }
> @@ -137,7 +136,10 @@ pub fn create_vm(vm_dir: &Path, ready_fd: File, mut config: VmConfig) -> Result<
>
> pub fn add_net(vm_dir: &Path, net: &NetConfig) -> Result<(), NonZeroI32> {
> let mut ch_remote = command(vm_dir, "add-net")
> - .arg(format!("fd={},id={},mac={}", net.fd, net.id, net.mac))
> + .arg(format!(
> + "vhost_user=on,socket={},id={},mac={}",
> + net.vhost_user_sock, net.id, net.mac
> + ))
> .stdout(Stdio::piped())
> .spawn()
> .or(Err(EPERM))?;

If we're not sending fds any more, I think we can just get rid of this,
and include network devices in the vm.create request. (vhost_user_sock
will need to be changed to vhost_socket to match the Cloud Hypervisor
API.)

> diff --git a/tools/start-vmm/lib.rs b/tools/start-vmm/lib.rs
> index 0422d85..246dd6d 100644
> --- a/tools/start-vmm/lib.rs
> +++ b/tools/start-vmm/lib.rs
> @@ -1,23 +1,24 @@
> // SPDX-License-Identifier: EUPL-1.2+
> // SPDX-FileCopyrightText: 2022-2024 Alyssa Ross <hi@alyssa.is>
> +// SPDX-FileCopyrightText: 2025 Yureka Lilian <yureka@cyberchaos.dev>
>
> mod ch;
> mod net;
> mod s6;
>
> use std::borrow::Cow;
> -use std::convert::TryInto;
> use std::env::args_os;
> use std::ffi::OsStr;
> use std::fs::File;
> -use std::io::{self, ErrorKind};
> +use std::hash::{Hash, Hasher};
> +use std::io::ErrorKind;
> use std::path::Path;
>
> use ch::{
> - ConsoleConfig, DiskConfig, FsConfig, GpuConfig, LandlockConfig, MemoryConfig, PayloadConfig,
> - VmConfig, VsockConfig,
> + ConsoleConfig, DiskConfig, FsConfig, GpuConfig, LandlockConfig, MemoryConfig, NetConfig,
> + PayloadConfig, VmConfig, VsockConfig,
> };
> -use net::net_setup;
> +use net::MacAddress;
>
> pub fn prog_name() -> String {
> args_os()
> @@ -40,8 +41,6 @@ pub fn vm_config(vm_dir: &Path) -> Result<VmConfig, String> {
> return Err(format!("VM name may not contain a colon: {vm_name:?}"));
> }
>
> - let name_bytes = vm_name.as_bytes();
> -
> let config_dir = vm_dir.join("config");
> let blk_dir = config_dir.join("blk");
> let kernel_path = config_dir.join("vmlinux");
> @@ -97,24 +96,51 @@ pub fn vm_config(vm_dir: &Path) -> Result<VmConfig, String> {
> shared: true,
> },
> net: match net_providers_dir.read_dir() {
> - Ok(_) => {
> - // SAFETY: we check the result.
> - let net = unsafe {
> - net_setup(
> - name_bytes.as_ptr().cast(),
> - name_bytes
> - .len()
> - .try_into()
> - .map_err(|e| format!("VM name too long: {e}"))?,
> - )
> - };
> - if net.fd == -1 {
> - let e = io::Error::last_os_error();
> - return Err(format!("setting up networking failed: {e}"));
> - }
> -
> - vec![net.try_into().unwrap()]
> - }
> + Ok(entries) => entries
> + .into_iter()
> + .map(|result| {
> + Ok(result
> + .map_err(|e| format!("examining directory entry: {e}"))?
> + .path())
> + })
> + .map(|result: Result<_, String>| {
> + let provider_name = result?.file_name().ok_or("unable to get net provider name".to_string())?.to_str().unwrap().to_string();
> +
> + if provider_name.contains(',') {
> + return Err(format!("illegal ',' character in net provider name {provider_name:?}"));
> + }
> +
> + let mut hasher = std::hash::DefaultHasher::new();
> + vm_name.hash(&mut hasher);
> + let id_hashed = hasher.finish();
> +
> + let mac = MacAddress::new([
> + 0x02, // IEEE 802c administratively assigned
> + 0x00, // Spectrum client
> + (id_hashed >> 24) as u8,
> + (id_hashed >> 16) as u8,
> + (id_hashed >> 8) as u8,
> + id_hashed as u8,
> + ]);
> +
> + let provider_id = std::fs::read_link(format!("/run/vm/by-name/{provider_name}")).map_err(|e| format!("unable to get net provider id: {e}"))?.file_name().ok_or("unable to get net provider id".to_string())?.to_str().unwrap().to_string();
> +
> + let svc_dir = format!("/run/service/vm-services/instance/{provider_id}/data/service/spectrum-router");
> + let svc_status = std::process::Command::new("s6-svc")
> + .args(["-U", &svc_dir])
> + .status()
> + .expect("setting up the upstream router via s6-svc failed");
> + if !svc_status.success() {
> + return Err(format!("setting up the upstream router via s6-svc failed with exit code {svc_status}"));
> + }

I'd prefer this was in run-vmm, since it's a bit surprising to stop in
the middle of constructing a Cloud Hypervisor API request to do service
management.

> diff --git a/tools/start-vmm/meson.build b/tools/start-vmm/meson.build
> index d07c5a0..aa9f6f3 100644
> --- a/tools/start-vmm/meson.build
> +++ b/tools/start-vmm/meson.build
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: EUPL-1.2+
> # SPDX-FileCopyrightText: 2022-2024 Alyssa Ross <hi@alyssa.is>
>
> -c_lib = static_library('start-vmm', 'net.c', 'net-util.c',
> +c_lib = static_library('start-vmm',
> c_args : '-D_GNU_SOURCE')

C_lib is now completely empty, so can be removed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]