From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from atuin.qyliss.net (localhost [IPv6:::1]) by atuin.qyliss.net (Postfix) with ESMTP id 7B0031E2B7; Sat, 29 Nov 2025 14:28:51 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id 525571E328; Sat, 29 Nov 2025 14:28:48 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on atuin.qyliss.net X-Spam-Level: X-Spam-Status: No, score=-0.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DMARC_PASS,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE autolearn=unavailable autolearn_force=no version=4.0.1 Received: from mail.cyberchaos.dev (mail.cyberchaos.dev [195.39.247.168]) by atuin.qyliss.net (Postfix) with ESMTPS id 7AAE31E2A3 for ; Sat, 29 Nov 2025 14:28:46 +0000 (UTC) Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yuka.dev; s=mail; t=1764426523; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:autocrypt:autocrypt; bh=rE47IWhIO2it2K8DygHZlONHIAhU7leFK/FVsksLh8I=; b=lY1dUO+Bb56PDGcCtFOavRmIGTzMeUMrDA0A8wPnASJtqBy2FcWcOqjSLaqPJWKa8lPYnn mX7hagQON0szkjChWXfur2MeSz7k8qL/zAQVbf8i8iVuqhR+F0kglbTvpixczjtgAGFZVg ShIX0drVSwWzBMUERCl0mR3Gx8N8OJA= Date: Sat, 29 Nov 2025 15:28:42 +0100 MIME-Version: 1.0 Subject: Re: [PATCH v2 5/7] host: integrate router To: Alyssa Ross References: <20251128223038.97536-1-yureka@cyberchaos.dev> <20251128223038.97536-6-yureka@cyberchaos.dev> <87ldjp3q9o.fsf@alyssa.is> Content-Language: en-US From: Yureka Autocrypt: addr=yuka@yuka.dev; keydata= xjMEZ3vnnhYJKwYBBAHaRw8BAQdAn6RVMnaxLzmDDx+J3jSUGY7BqjyDhsWhdwKBSI6QpXfN Fll1cmVrYSA8eXVrYUB5dWthLmRldj7CjgQTFgoANhYhBPGINbLQ3ypM7JNhigKbtnC7kwpH BQJne+eeAhsDBAsJCAcEFQoJCAUWAgMBAAIeBQIXgAAKCRACm7Zwu5MKRx1qAP9ToLaOMd73 VVf1JdwoMc5G44OZfKNk/+ezt9Dl2oqZdQD/Xvgd0lytU3BZ4WnYeMNzo2xHeRxXmX+MfXhA D33tzQ/OOARne+eeEgorBgEEAZdVAQUBAQdAIs9uImfvgSCnJOcfvzshLuaSRJ/a0Vp/9rUA eBGZq10DAQgHwngEGBYKACAWIQTxiDWy0N8qTOyTYYoCm7Zwu5MKRwUCZ3vnngIbDAAKCRAC m7Zwu5MKRyW9AP0dBOuwgWso+QjBZUsbuEmGGUz2OWtszs2Yb7087RMerwEA3al6E7vqq0HC 7LiB3nisU+xqQojJ4n/fWCu70iEkjQw= In-Reply-To: <87ldjp3q9o.fsf@alyssa.is> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Message-ID-Hash: YNWY4N36AW36CT32NSRNMUYMUWFD3OTP X-Message-ID-Hash: YNWY4N36AW36CT32NSRNMUYMUWFD3OTP X-MailFrom: yuka@yuka.dev X-Mailman-Rule-Hits: member-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address CC: devel@spectrum-os.org X-Mailman-Version: 3.3.9 Precedence: list List-Id: Patches and low-level development discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 11/29/25 14:46, Alyssa Ross wrote: > Yureka Lilian 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 >> --- >> 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 >> + >> +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 >> +# SPDX-FileCopyrightText: 2025 Yureka Lilian >> + >> +# 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 >> +// SPDX-FileCopyrightText: 2025 Yureka Lilian >> >> -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 >> +// SPDX-FileCopyrightText: 2025 Yureka Lilian >> >> 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 { >> 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 { >> 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 >> >> -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.