From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 7DFD11FF17A for ; Tue, 11 Nov 2025 15:02:11 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7DC1BC25C; Tue, 11 Nov 2025 15:01:08 +0100 (CET) From: Christoph Heiss To: pve-devel@lists.proxmox.com Date: Tue, 11 Nov 2025 14:59:56 +0100 Message-ID: <20251111140014.1443471-7-c.heiss@proxmox.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20251111140014.1443471-1-c.heiss@proxmox.com> References: <20251111140014.1443471-1-c.heiss@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1762869599222 X-SPAM-LEVEL: Spam detection results: 0 AWL -2.455 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_SOMETLD_ARE_BAD_TLD 5 .bar, .beauty, .buzz, .cam, .casa, .cfd, .club, .date, .guru, .link, .live, .monster, .online, .press, .pw, .quest, .rest, .sbs, .shop, .stream, .top, .trade, .wiki, .work, .xyz TLD abuse SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH installer v3 06/15] common: implement support for `network_interface_pin_map` config X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Adds all the pieces for installer frontends to wire up pinning support, i.e. deserializing from the runtime environment, doing verification and serializing it out to the low-level installer. Signed-off-by: Christoph Heiss --- Changes v2 -> v3: * rebased * switched fully-numeric check before first character digit check * simplified first character digit check * added more unit tests (thanks @Michael) Changes v1 -> v2: * improved validation for interface names, now according to our `pve-iface` json schema proxmox-auto-installer/src/utils.rs | 3 +- proxmox-installer-common/src/lib.rs | 5 + proxmox-installer-common/src/options.rs | 242 ++++++++++++++++++++++-- proxmox-installer-common/src/setup.rs | 51 ++++- proxmox-tui-installer/src/setup.rs | 3 +- 5 files changed, 279 insertions(+), 25 deletions(-) diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs index 2d68829..ce49baa 100644 --- a/proxmox-auto-installer/src/utils.rs +++ b/proxmox-auto-installer/src/utils.rs @@ -2,7 +2,7 @@ use anyhow::{Context, Result, bail}; use glob::Pattern; use log::info; use std::{ - collections::{BTreeMap, HashSet}, + collections::{BTreeMap, HashMap, HashSet}, process::Command, }; @@ -483,6 +483,7 @@ pub fn parse_answer( root_ssh_keys: answer.global.root_ssh_keys.clone(), mngmt_nic: network_settings.ifname, + network_interface_pin_map: HashMap::new(), hostname: network_settings .fqdn diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs index ea907a0..a85d5f8 100644 --- a/proxmox-installer-common/src/lib.rs +++ b/proxmox-installer-common/src/lib.rs @@ -10,6 +10,11 @@ pub mod http; #[cfg(feature = "cli")] pub mod cli; +pub mod net { + /// Maximum length of the (primary) name of a network interface + pub const MAX_IFNAME_LEN: usize = 15; // IFNAMSIZ - 1 to account for NUL byte +} + pub const RUNTIME_DIR: &str = "/run/proxmox-installer"; /// Default placeholder value for the administrator email address. diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs index fa91060..0d72f54 100644 --- a/proxmox-installer-common/src/options.rs +++ b/proxmox-installer-common/src/options.rs @@ -1,12 +1,14 @@ use anyhow::{Result, bail}; use regex::Regex; use serde::{Deserialize, Serialize}; +use std::collections::HashMap; use std::net::{IpAddr, Ipv4Addr}; use std::str::FromStr; use std::sync::OnceLock; use std::{cmp, fmt}; use crate::disk_checks::check_raid_min_disks; +use crate::net::MAX_IFNAME_LEN; use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo}; use crate::utils::{CidrAddress, Fqdn}; @@ -476,6 +478,75 @@ impl TimezoneOptions { } } +/// Options controlling the behaviour of the network interface pinning (by +/// creating appropriate systemd.link files) during the installation. +#[derive(Clone, Debug, Default, PartialEq, Deserialize)] +#[serde(rename_all = "kebab-case", deny_unknown_fields)] +pub struct NetworkInterfacePinningOptions { + /// Maps MAC address to custom name + #[serde(default)] + pub mapping: HashMap, +} + +impl NetworkInterfacePinningOptions { + /// Default prefix to prepend to the pinned interface ID as received from the low-level + /// installer. + pub const DEFAULT_PREFIX: &str = "nic"; + + /// Does some basic checks on the options. + /// + /// This includes checks for: + /// - empty interface names + /// - overlong interface names + /// - duplicate interface names + /// - only contains ASCII alphanumeric characters and underscore, as + /// enforced by our `pve-iface` json schema. + pub fn verify(&self) -> Result<()> { + let mut reverse_mapping = HashMap::::new(); + for (mac, name) in self.mapping.iter() { + if name.is_empty() { + bail!("interface name for '{mac}' cannot be empty"); + } + + if name.len() > MAX_IFNAME_LEN { + bail!( + "interface name '{name}' for '{mac}' cannot be longer than {} characters", + MAX_IFNAME_LEN + ); + } + + if name.chars().all(char::is_numeric) { + bail!( + "interface name '{name}' for '{mac}' is invalid: name must not be fully numeric" + ); + } + + // Mimicking the `pve-iface` schema verification + if name.starts_with(|c: char| c.is_ascii_digit()) { + bail!( + "interface name '{name}' for '{mac}' is invalid: name must not start with a number" + ); + } + + if !name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') { + bail!( + "interface name '{name}' for '{mac}' is invalid: name must only consist of alphanumeric characters and underscores" + ); + } + + if let Some(duplicate_mac) = reverse_mapping.get(name) + && mac != duplicate_mac + { + bail!("duplicate interface name mapping '{name}' for: {mac}, {duplicate_mac}"); + } + + reverse_mapping.insert(name.clone(), mac.clone()); + } + + Ok(()) + } +} + #[derive(Clone, Debug, PartialEq)] pub struct NetworkOptions { pub ifname: String, @@ -483,6 +554,7 @@ pub struct NetworkOptions { pub address: CidrAddress, pub gateway: IpAddr, pub dns_server: IpAddr, + pub pinning_opts: Option, } impl NetworkOptions { @@ -492,6 +564,7 @@ impl NetworkOptions { setup: &SetupInfo, network: &NetworkInfo, default_domain: Option<&str>, + pinning_opts: Option<&NetworkInterfacePinningOptions>, ) -> Self { // Sets up sensible defaults as much as possible, such that even in the // worse case nothing breaks down *completely*. @@ -507,28 +580,39 @@ impl NetworkOptions { address: CidrAddress::new(Ipv4Addr::new(192, 168, 100, 2), 24).unwrap(), gateway: Ipv4Addr::new(192, 168, 100, 1).into(), dns_server: Ipv4Addr::new(192, 168, 100, 1).into(), + pinning_opts: pinning_opts.cloned(), }; if let Some(ip) = network.dns.dns.first() { this.dns_server = *ip; } - if let Some(routes) = &network.routes { - if let Some(gw) = &routes.gateway4 - && let Some(iface) = network.interfaces.get(&gw.dev) - { - // we got some ipv4 connectivity, so use that + if let Some(routes) = &network.routes + && let Some(gw) = &routes.gateway4 + && let Some(iface) = network.interfaces.get(&gw.dev) + { + // we got some ipv4 connectivity, so use that + + if let Some(opts) = pinning_opts { + this.ifname.clone_from(&iface.to_pinned(opts).name); + } else { this.ifname.clone_from(&iface.name); - if let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv4()) { - this.gateway = gw.gateway; - this.address = addr.clone(); - } + } + + if let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv4()) { + this.gateway = gw.gateway; + this.address = addr.clone(); } else if let Some(gw) = &routes.gateway6 && let Some(iface) = network.interfaces.get(&gw.dev) && let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv6()) { // no ipv4, but ipv6 connectivity - this.ifname.clone_from(&iface.name); + if let Some(opts) = pinning_opts { + this.ifname.clone_from(&iface.to_pinned(opts).name); + } else { + this.ifname.clone_from(&iface.name); + } + this.gateway = gw.gateway; this.address = addr.clone(); } @@ -541,7 +625,20 @@ impl NetworkOptions { if this.ifname.is_empty() && let Some(iface) = network.interfaces.values().min_by_key(|v| v.index) { - this.ifname.clone_from(&iface.name); + if let Some(opts) = pinning_opts { + this.ifname.clone_from(&iface.to_pinned(opts).name); + } else { + this.ifname.clone_from(&iface.name); + } + } + + if let Some(ref mut opts) = this.pinning_opts { + // Ensure that all unique interfaces indeed have an entry in the map, + // as required by the low-level installer + for iface in network.interfaces.values() { + let pinned_name = iface.to_pinned(opts).name; + opts.mapping.entry(iface.mac.clone()).or_insert(pinned_name); + } } this @@ -668,6 +765,7 @@ mod tests { Interface { name: "eth0".to_owned(), index: 0, + pinned_id: "0".to_owned(), state: InterfaceState::Up, driver: "dummy".to_owned(), mac: "01:23:45:67:89:ab".to_owned(), @@ -699,49 +797,53 @@ mod tests { let (setup, mut info) = mock_setup_network(); pretty_assertions::assert_eq!( - NetworkOptions::defaults_from(&setup, &info, None), + NetworkOptions::defaults_from(&setup, &info, None, None), NetworkOptions { ifname: "eth0".to_owned(), fqdn: Fqdn::from("foo.bar.com").unwrap(), address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(), gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)), dns_server: Ipv4Addr::new(192, 168, 100, 1).into(), + pinning_opts: None, } ); info.hostname = None; pretty_assertions::assert_eq!( - NetworkOptions::defaults_from(&setup, &info, None), + NetworkOptions::defaults_from(&setup, &info, None, None), NetworkOptions { ifname: "eth0".to_owned(), fqdn: Fqdn::from("pve.bar.com").unwrap(), address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(), gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)), dns_server: Ipv4Addr::new(192, 168, 100, 1).into(), + pinning_opts: None, } ); info.dns.domain = None; pretty_assertions::assert_eq!( - NetworkOptions::defaults_from(&setup, &info, None), + NetworkOptions::defaults_from(&setup, &info, None, None), NetworkOptions { ifname: "eth0".to_owned(), fqdn: Fqdn::from("pve.example.invalid").unwrap(), address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(), gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)), dns_server: Ipv4Addr::new(192, 168, 100, 1).into(), + pinning_opts: None, } ); info.hostname = Some("foo".to_owned()); pretty_assertions::assert_eq!( - NetworkOptions::defaults_from(&setup, &info, None), + NetworkOptions::defaults_from(&setup, &info, None, None), NetworkOptions { ifname: "eth0".to_owned(), fqdn: Fqdn::from("foo.example.invalid").unwrap(), address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(), gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)), dns_server: Ipv4Addr::new(192, 168, 100, 1).into(), + pinning_opts: None, } ); } @@ -751,37 +853,40 @@ mod tests { let (setup, mut info) = mock_setup_network(); pretty_assertions::assert_eq!( - NetworkOptions::defaults_from(&setup, &info, None), + NetworkOptions::defaults_from(&setup, &info, None, None), NetworkOptions { ifname: "eth0".to_owned(), fqdn: Fqdn::from("foo.bar.com").unwrap(), address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(), gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)), dns_server: Ipv4Addr::new(192, 168, 100, 1).into(), + pinning_opts: None, } ); info.dns.domain = None; pretty_assertions::assert_eq!( - NetworkOptions::defaults_from(&setup, &info, Some("custom.local")), + NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None), NetworkOptions { ifname: "eth0".to_owned(), fqdn: Fqdn::from("foo.custom.local").unwrap(), address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(), gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)), dns_server: Ipv4Addr::new(192, 168, 100, 1).into(), + pinning_opts: None, } ); info.dns.domain = Some("some.domain.local".to_owned()); pretty_assertions::assert_eq!( - NetworkOptions::defaults_from(&setup, &info, Some("custom.local")), + NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None), NetworkOptions { ifname: "eth0".to_owned(), fqdn: Fqdn::from("foo.custom.local").unwrap(), address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(), gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)), dns_server: Ipv4Addr::new(192, 168, 100, 1).into(), + pinning_opts: None, } ); } @@ -794,6 +899,7 @@ mod tests { Interface { name: "eth0".to_owned(), index: 0, + pinned_id: "0".to_owned(), state: InterfaceState::Up, driver: "dummy".to_owned(), mac: "01:23:45:67:89:ab".to_owned(), @@ -814,14 +920,112 @@ mod tests { let setup = SetupInfo::mocked(); pretty_assertions::assert_eq!( - NetworkOptions::defaults_from(&setup, &info, None), + NetworkOptions::defaults_from(&setup, &info, None, None), NetworkOptions { ifname: "eth0".to_owned(), fqdn: Fqdn::from("pve.example.invalid").unwrap(), address: CidrAddress::new(Ipv4Addr::new(192, 168, 100, 2), 24).unwrap(), gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 100, 1)), dns_server: Ipv4Addr::new(192, 168, 100, 1).into(), + pinning_opts: None, } ); } + + #[test] + fn network_interface_pinning_options_fail_on_empty_name() { + let mut options = NetworkInterfacePinningOptions::default(); + options + .mapping + .insert("ab:cd:ef:12:34:56".to_owned(), String::new()); + + let res = options.verify(); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "interface name for 'ab:cd:ef:12:34:56' cannot be empty" + ) + } + + #[test] + fn network_interface_pinning_options_fail_on_overlong_name() { + let mut options = NetworkInterfacePinningOptions::default(); + options.mapping.insert( + "ab:cd:ef:12:34:56".to_owned(), + "waytoolonginterfacename".to_owned(), + ); + + let res = options.verify(); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "interface name 'waytoolonginterfacename' for 'ab:cd:ef:12:34:56' cannot be longer than 15 characters" + ) + } + + #[test] + fn network_interface_pinning_options_fail_on_duplicate_name() { + let mut options = NetworkInterfacePinningOptions::default(); + options + .mapping + .insert("ab:cd:ef:12:34:56".to_owned(), "nic0".to_owned()); + options + .mapping + .insert("12:34:56:ab:cd:ef".to_owned(), "nic0".to_owned()); + + let res = options.verify(); + assert!(res.is_err()); + let err = res.unwrap_err().to_string(); + + // [HashMap] does not guarantee iteration order, so just check for the substrings + // we expect to find + assert!(err.contains("duplicate interface name mapping 'nic0' for: ")); + assert!(err.contains("12:34:56:ab:cd:ef")); + assert!(err.contains("ab:cd:ef:12:34:56")); + } + + #[test] + fn network_interface_pinning_options_fail_on_invalid_characters() { + let mut options = NetworkInterfacePinningOptions::default(); + options + .mapping + .insert("ab:cd:ef:12:34:56".to_owned(), "nic-".to_owned()); + + let res = options.verify(); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "interface name 'nic-' for 'ab:cd:ef:12:34:56' is invalid: name must only consist of alphanumeric characters and underscores" + ) + } + + #[test] + fn network_interface_pinning_options_fail_on_name_starting_with_number() { + let mut options = NetworkInterfacePinningOptions::default(); + options + .mapping + .insert("ab:cd:ef:12:34:56".to_owned(), "0nic".to_owned()); + + let res = options.verify(); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "interface name '0nic' for 'ab:cd:ef:12:34:56' is invalid: name must not start with a number" + ) + } + + #[test] + fn network_interface_pinning_options_fail_on_fully_numeric_name() { + let mut options = NetworkInterfacePinningOptions::default(); + options + .mapping + .insert("ab:cd:ef:12:34:56".to_owned(), "12345".to_owned()); + + let res = options.verify(); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "interface name '12345' for 'ab:cd:ef:12:34:56' is invalid: name must not be fully numeric" + ) + } } diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs index 4873fff..3e99576 100644 --- a/proxmox-installer-common/src/setup.rs +++ b/proxmox-installer-common/src/setup.rs @@ -3,6 +3,7 @@ use std::{ collections::{BTreeMap, HashMap}, fmt, fs::File, + hash::Hash, io::{self, BufReader}, net::IpAddr, path::{Path, PathBuf}, @@ -13,8 +14,8 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; use crate::{ options::{ - BtrfsBootdiskOptions, BtrfsCompressOption, Disk, FsType, ZfsBootdiskOptions, - ZfsChecksumOption, ZfsCompressOption, + BtrfsBootdiskOptions, BtrfsCompressOption, Disk, FsType, NetworkInterfacePinningOptions, + ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption, }, utils::CidrAddress, }; @@ -443,8 +444,9 @@ pub struct Gateway { pub gateway: IpAddr, } -#[derive(Clone, Deserialize)] +#[derive(Clone, Deserialize, Serialize)] #[serde(rename_all = "UPPERCASE")] +/// The current interface state as reported by the kernel. pub enum InterfaceState { Up, Down, @@ -452,6 +454,8 @@ pub enum InterfaceState { Unknown, } +serde_plain::derive_display_from_serialize!(InterfaceState); + impl InterfaceState { // avoid display trait as this is not the string representation for a serializer pub fn render(&self) -> String { @@ -469,6 +473,9 @@ pub struct Interface { pub index: usize, + /// Sequential interface ID for pinning interface names. + pub pinned_id: String, + pub mac: String, pub state: InterfaceState, @@ -484,7 +491,22 @@ pub struct Interface { impl Interface { // avoid display trait as this is not the string representation for a serializer pub fn render(&self) -> String { - format!("{} {}", self.state.render(), self.name) + format!("{} {} ({})", self.state.render(), self.name, self.mac) + } + + pub fn to_pinned(&self, options: &NetworkInterfacePinningOptions) -> Self { + let mut this = self.clone(); + this.name = options + .mapping + .get(&this.mac) + .unwrap_or(&format!( + "{}{}", + NetworkInterfacePinningOptions::DEFAULT_PREFIX, + this.pinned_id + )) + .clone(); + + this } } @@ -577,6 +599,14 @@ pub struct InstallConfig { pub root_ssh_keys: Vec, pub mngmt_nic: String, + // Maps MAC addresses -> custom name. If set, enables pinning for all + // interfaces present. + #[serde( + default, + skip_serializing_if = "HashMap::is_empty", + deserialize_with = "deserialize_optional_map" + )] + pub network_interface_pin_map: HashMap, pub hostname: String, pub domain: String, @@ -610,3 +640,16 @@ pub enum LowLevelMessage { text: Option, }, } + +/// Deserializes an optional [HashMap]. +/// +/// If missing, it returns an empty map, otherwise the deserialized map. +fn deserialize_optional_map<'de, D, K, V>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, + K: Eq + Hash + Deserialize<'de>, + V: Deserialize<'de>, +{ + let map: Option> = Deserialize::deserialize(deserializer)?; + Ok(map.unwrap_or_default()) +} diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs index b90c7dc..d2ffb70 100644 --- a/proxmox-tui-installer/src/setup.rs +++ b/proxmox-tui-installer/src/setup.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use crate::options::InstallerOptions; use proxmox_installer_common::{ @@ -32,6 +32,7 @@ impl From for InstallConfig { root_ssh_keys: vec![], mngmt_nic: options.network.ifname, + network_interface_pin_map: HashMap::new(), // Safety: At this point, it is know that we have a valid FQDN, as // this is set by the TUI network panel, which only lets the user -- 2.51.0 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel