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 993D41FF16F for ; Tue, 14 Oct 2025 15:23:43 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 757FA43F3; Tue, 14 Oct 2025 15:22:57 +0200 (CEST) From: Christoph Heiss To: pve-devel@lists.proxmox.com Date: Tue, 14 Oct 2025 15:21:51 +0200 Message-ID: <20251014132207.1171073-7-c.heiss@proxmox.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20251014132207.1171073-1-c.heiss@proxmox.com> References: <20251014132207.1171073-1-c.heiss@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1760448095933 X-SPAM-LEVEL: Spam detection results: 0 AWL -2.460 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 06/14] 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 --- proxmox-auto-installer/src/utils.rs | 3 +- proxmox-installer-common/src/lib.rs | 5 + proxmox-installer-common/src/options.rs | 158 ++++++++++++++++++++++-- proxmox-installer-common/src/setup.rs | 51 +++++++- proxmox-tui-installer/src/setup.rs | 3 +- 5 files changed, 203 insertions(+), 17 deletions(-) diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs index 7d42f2c..eb666d1 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, }; @@ -485,6 +485,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 59e8560..60ea227 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,54 @@ 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"; + + /// Do some basic checks on the options. + /// + /// This checks for: + /// - empty interface names + /// - overlong interface names + /// - duplicate interface names + 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 mapping for '{mac}' cannot be empty"); + } + + if name.len() > MAX_IFNAME_LEN { + bail!( + "interface name mapping '{name}' for '{mac}' cannot be longer than {} characters", + MAX_IFNAME_LEN + ); + } + + if let Some(duplicate_mac) = reverse_mapping.get(name) { + if 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 +533,7 @@ pub struct NetworkOptions { pub address: CidrAddress, pub gateway: IpAddr, pub dns_server: IpAddr, + pub pinning_opts: Option, } impl NetworkOptions { @@ -492,6 +543,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,6 +559,7 @@ 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() { @@ -517,7 +570,11 @@ impl NetworkOptions { let mut filled = false; if let Some(gw) = &routes.gateway4 { if let Some(iface) = network.interfaces.get(&gw.dev) { - 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(addr) = iface.addresses.iter().find(|addr| addr.is_ipv4()) { this.gateway = gw.gateway; this.address = addr.clone(); @@ -529,7 +586,11 @@ impl NetworkOptions { if let Some(gw) = &routes.gateway6 { if let Some(iface) = network.interfaces.get(&gw.dev) { if let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv6()) { - 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(); } @@ -544,7 +605,20 @@ impl NetworkOptions { // earlier in that case, so use the first one enumerated. if this.ifname.is_empty() { if 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); } } @@ -672,6 +746,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(), @@ -703,49 +778,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, } ); } @@ -755,37 +834,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, } ); } @@ -798,6 +880,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(), @@ -818,14 +901,67 @@ 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 mapping 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 mapping '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")); + } } 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