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

[Yureka <yuka@yuka.dev>]

On 11/29/25 14:46, Alyssa Ross wrote:
> 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.)

Ah, I completely missed the reason it was originally structured like 
this! Will do.

>
>> 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.

Is it by any chance even guaranteed that at the point when run-vmm for 
this VM runs, the vmm for the provider VM would already be up? That 
would simplify the process and make assign-driver-router-iface 
unnecessary because there would be one place where we can add the interface.

>
>> 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.