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

[Alyssa Ross <hi@alyssa.is>]

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

Yureka <yuka@yuka.dev> writes:

> On 11/29/25 14:46, Alyssa Ross wrote:
>> Yureka Lilian <yureka@cyberchaos.dev> writes:
>>
>>> 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.

No, but you could s6-svwait in there, in the existing background block.

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