From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 56E361FF16F for ; Tue, 14 Oct 2025 15:23:54 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5AB184581; Tue, 14 Oct 2025 15:23:17 +0200 (CEST) From: Christoph Heiss To: pve-devel@lists.proxmox.com Date: Tue, 14 Oct 2025 15:21:58 +0200 Message-ID: <20251014132207.1171073-14-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: 1760448096136 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.042 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 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 13/14] tui: add support for pinning network interface names 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 an additional checkbox and option button in the network panel, the latter triggering a dialog for setting custom names per network interface present on the system. Pinning is enabled by default. Each pinned network interface name defaults to `nicN`, where N is the pinned ID from the low-level installer. Signed-off-by: Christoph Heiss --- proxmox-installer-common/src/setup.rs | 10 +- proxmox-tui-installer/src/main.rs | 13 +- proxmox-tui-installer/src/setup.rs | 6 +- proxmox-tui-installer/src/views/mod.rs | 5 + proxmox-tui-installer/src/views/network.rs | 357 +++++++++++++++++++-- 5 files changed, 353 insertions(+), 38 deletions(-) diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs index 1a584ba..c93ee30 100644 --- a/proxmox-installer-common/src/setup.rs +++ b/proxmox-installer-common/src/setup.rs @@ -436,7 +436,7 @@ pub struct Gateway { pub gateway: IpAddr, } -#[derive(Clone, Deserialize, Serialize)] +#[derive(Clone, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)] #[serde(rename_all = "UPPERCASE")] /// The current interface state as reported by the kernel. pub enum InterfaceState { @@ -483,7 +483,13 @@ 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, self.mac) + format!( + "{} {} ({}, {})", + self.state.render(), + self.name, + self.mac, + self.driver + ) } pub fn to_pinned(&self, options: &NetworkInterfacePinningOptions) -> Self { diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs index fce9fc2..cd590b8 100644 --- a/proxmox-tui-installer/src/main.rs +++ b/proxmox-tui-installer/src/main.rs @@ -18,7 +18,10 @@ use options::{InstallerOptions, PasswordOptions}; use proxmox_installer_common::{ ROOT_PASSWORD_MIN_LENGTH, - options::{BootdiskOptions, NetworkOptions, TimezoneOptions, email_validate}, + options::{ + BootdiskOptions, NetworkInterfacePinningOptions, NetworkOptions, TimezoneOptions, + email_validate, + }, setup::{LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo, installer_setup}, }; mod setup; @@ -167,7 +170,13 @@ fn main() { bootdisk: BootdiskOptions::defaults_from(&runtime_info.disks[0]), timezone: TimezoneOptions::defaults_from(&runtime_info, &locales), password: Default::default(), - network: NetworkOptions::defaults_from(&setup_info, &runtime_info.network, None), + network: NetworkOptions::defaults_from( + &setup_info, + &runtime_info.network, + None, + // We enable network interface pinning by default in the TUI + Some(&NetworkInterfacePinningOptions::default()), + ), autoreboot: true, }, setup_info, diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs index d2ffb70..3ab1869 100644 --- a/proxmox-tui-installer/src/setup.rs +++ b/proxmox-tui-installer/src/setup.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, HashMap}; +use std::collections::BTreeMap; use crate::options::InstallerOptions; use proxmox_installer_common::{ @@ -8,6 +8,8 @@ use proxmox_installer_common::{ impl From for InstallConfig { fn from(options: InstallerOptions) -> Self { + let pinning_opts = options.network.pinning_opts.as_ref(); + let mut config = Self { autoreboot: options.autoreboot as usize, @@ -32,7 +34,7 @@ impl From for InstallConfig { root_ssh_keys: vec![], mngmt_nic: options.network.ifname, - network_interface_pin_map: HashMap::new(), + network_interface_pin_map: pinning_opts.map(|o| o.mapping.clone()).unwrap_or_default(), // 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 diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs index 5723fed..5784681 100644 --- a/proxmox-tui-installer/src/views/mod.rs +++ b/proxmox-tui-installer/src/views/mod.rs @@ -436,6 +436,11 @@ impl FormView { self.add_to_column(1, view); } + pub fn add_child_with_custom_label(&mut self, label: &str, view: impl View) { + self.add_to_column(0, TextView::new(label).no_wrap()); + self.add_to_column(1, view); + } + pub fn add_child_with_data(&mut self, label: &str, view: impl View, data: UDT) { self.add_to_column(0, TextView::new(format!("{label}: ")).no_wrap()); self.add_to_column(1, view); diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs index 960c25e..4388c7d 100644 --- a/proxmox-tui-installer/src/views/network.rs +++ b/proxmox-tui-installer/src/views/network.rs @@ -1,40 +1,82 @@ -use std::net::IpAddr; - use cursive::{ - view::ViewWrapper, - views::{EditView, SelectView}, + Cursive, View, + view::{Nameable, Resizable, ViewWrapper}, + views::{ + Button, Checkbox, Dialog, DummyView, EditView, LinearLayout, NamedView, ResizedView, + ScrollView, SelectView, TextView, + }, +}; +use std::{ + collections::HashMap, + net::IpAddr, + sync::{Arc, Mutex}, }; use super::{CidrAddressEditView, FormView}; use proxmox_installer_common::{ - options::NetworkOptions, - setup::NetworkInfo, + net::MAX_IFNAME_LEN, + options::{NetworkInterfacePinningOptions, NetworkOptions}, + setup::{Interface, NetworkInfo}, utils::{CidrAddress, Fqdn}, }; +struct NetworkViewOptions { + selected_mac: String, + pinning_enabled: bool, + // For UI purposes, we want to always save the mapping, to save the state + // between toggling the checkbox + pinning_options: NetworkInterfacePinningOptions, +} + +/// Convenience wrapper when needing to take a (interior-mutable) reference to +/// `NetworkViewOptions`. +type NetworkViewOptionsRef = Arc>; + +/// View for configuring anything related to network setup. pub struct NetworkOptionsView { - view: FormView, + view: LinearLayout, + options: NetworkViewOptionsRef, } impl NetworkOptionsView { + const PINNING_OPTIONS_BUTTON_NAME: &str = "network-pinning-options-button"; + const MGMT_IFNAME_SELECTVIEW_NAME: &str = "network-management-ifname-selectview"; + pub fn new(options: &NetworkOptions, network_info: &NetworkInfo) -> Self { - let ifaces = network_info.interfaces.values(); - let ifnames = ifaces - .clone() - .map(|iface| (iface.render(), iface.name.clone())); - let mut ifaces_selection = SelectView::new().popup().with_all(ifnames.clone()); + let mut ifaces = network_info + .interfaces + .values() + .collect::>(); - // sort first to always have stable view - ifaces_selection.sort(); - let selected = ifaces_selection - .iter() - .position(|(_label, iface)| *iface == options.ifname) - .unwrap_or(ifaces.len() - 1); + // First, sort interfaces by their link state and then name + ifaces.sort_unstable_by_key(|x| (&x.state, &x.name)); - ifaces_selection.set_selection(selected); + let selected_mac = network_info + .interfaces + .get(&options.ifname) + .map(|iface| iface.mac.clone()) + .unwrap_or_else(|| { + ifaces + .first() + .expect("at least one network interface") + .mac + .clone() + }); + + let options_ref = Arc::new(Mutex::new(NetworkViewOptions { + selected_mac, + pinning_enabled: options.pinning_opts.is_some(), + pinning_options: options.pinning_opts.clone().unwrap_or_default(), + })); + + let iface_selection = + Self::build_mgmt_ifname_selectview(ifaces.clone(), options_ref.clone()); let form = FormView::<()>::new() - .child("Management interface", ifaces_selection) + .child( + "Management interface", + iface_selection.with_name(Self::MGMT_IFNAME_SELECTVIEW_NAME), + ) .child( "Hostname (FQDN)", EditView::new().content(options.fqdn.to_string()), @@ -52,44 +94,106 @@ impl NetworkOptionsView { EditView::new().content(options.dns_server.to_string()), ); - Self { view } + let pinning_checkbox = LinearLayout::horizontal() + .child(Checkbox::new().checked().on_change({ + let ifaces = ifaces + .iter() + .map(|iface| (*iface).clone()) + .collect::>(); + let options_ref = options_ref.clone(); + move |siv, enable_pinning| { + siv.call_on_name(Self::PINNING_OPTIONS_BUTTON_NAME, { + let options_ref = options_ref.clone(); + move |view: &mut Button| { + view.set_enabled(enable_pinning); + + options_ref.lock().expect("unpoisoned lock").pinning_enabled = + enable_pinning; + } + }); + + Self::refresh_ifname_selectview(siv, &ifaces, options_ref.clone()); + } + })) + .child(TextView::new(" Pin network interface names").no_wrap()) + .child(DummyView.full_width()) + .child( + Button::new("Pinning options", { + let options_ref = options_ref.clone(); + let network_info = network_info.clone(); + move |siv| { + let mut view = + Self::custom_name_mapping_view(&network_info, options_ref.clone()); + + // Pre-compute the child's layout, since it might depend on the size. Without this, + // the view will be empty until focused. + // The screen size never changes in our case, so this is completely OK. + view.layout(siv.screen_size()); + + siv.add_layer(view); + } + }) + .with_name(Self::PINNING_OPTIONS_BUTTON_NAME), + ); + + let view = LinearLayout::vertical() + .child(form) + .child(DummyView.full_width()) + .child(pinning_checkbox); + + Self { + view, + options: options_ref, + } } pub fn get_values(&mut self) -> Result { - let ifname = self + let form = self .view - .get_value::(0) + .get_child(0) + .and_then(|v| v.downcast_ref::()) + .ok_or("failed to retrieve network options form")?; + + let iface = form + .get_value::>, _>(0) .ok_or("failed to retrieve management interface name")?; - let fqdn = self - .view + let fqdn = form .get_value::(1) .ok_or("failed to retrieve host FQDN")? .parse::() .map_err(|err| format!("hostname does not look valid:\n\n{err}"))?; - let address = self - .view + let address = form .get_value::(2) .ok_or("failed to retrieve host address".to_string()) .and_then(|(ip_addr, mask)| { CidrAddress::new(ip_addr, mask).map_err(|err| err.to_string()) })?; - let gateway = self - .view + let gateway = form .get_value::(3) .ok_or("failed to retrieve gateway address")? .parse::() .map_err(|err| err.to_string())?; - let dns_server = self - .view + let dns_server = form .get_value::(4) .ok_or("failed to retrieve DNS server address")? .parse::() .map_err(|err| err.to_string())?; + let pinning_opts = self + .options + .lock() + .map(|opt| opt.pinning_enabled.then_some(opt.pinning_options.clone())) + .map_err(|err| err.to_string())?; + + let ifname = match &pinning_opts { + Some(opts) => iface.to_pinned(opts).name, + None => iface.name, + }; + if address.addr().is_ipv4() != gateway.is_ipv4() { Err("host and gateway IP address version must not differ".to_owned()) } else if address.addr().is_ipv4() != dns_server.is_ipv4() { @@ -103,11 +207,200 @@ impl NetworkOptionsView { address, gateway, dns_server, + pinning_opts, }) } } + + fn custom_name_mapping_view( + network_info: &NetworkInfo, + options_ref: NetworkViewOptionsRef, + ) -> impl View { + const DIALOG_NAME: &str = "network-interface-name-pinning-dialog"; + + let mut interfaces = network_info + .interfaces + .values() + .collect::>(); + + interfaces.sort_by(|a, b| (&a.state, &a.name).cmp(&(&b.state, &b.name))); + + Dialog::around(InterfacePinningOptionsView::new( + &interfaces, + options_ref.clone(), + )) + .title("Interface Name Pinning Options") + .button("Ok", { + let interfaces = interfaces + .iter() + .map(|v| (*v).clone()) + .collect::>(); + move |siv| { + let options = siv + .call_on_name(DIALOG_NAME, |view: &mut Dialog| { + view.get_content_mut() + .downcast_mut::() + .map(InterfacePinningOptionsView::get_values) + }) + .flatten(); + + let options = match options { + Some(Ok(options)) => options, + Some(Err(err)) => { + siv.add_layer(Dialog::info(err)); + return; + } + None => { + siv.add_layer(Dialog::info( + "Failed to retrieve network interface name pinning options view", + )); + return; + } + }; + + siv.pop_layer(); + options_ref.lock().expect("unpoisoned lock").pinning_options = options; + + Self::refresh_ifname_selectview(siv, &interfaces, options_ref.clone()); + } + }) + .with_name(DIALOG_NAME) + .max_size((80, 40)) + } + + fn refresh_ifname_selectview( + siv: &mut Cursive, + ifaces: &[Interface], + options_ref: NetworkViewOptionsRef, + ) { + siv.call_on_name( + Self::MGMT_IFNAME_SELECTVIEW_NAME, + |view: &mut SelectView| { + *view = Self::build_mgmt_ifname_selectview(ifaces.iter().collect(), options_ref); + }, + ); + } + + fn build_mgmt_ifname_selectview( + ifaces: Vec<&Interface>, + options_ref: NetworkViewOptionsRef, + ) -> SelectView { + let options = options_ref.lock().expect("unpoisoned lock"); + + // Map all interfaces to a list of (human-readable interface name, [Interface]) pairs + let ifnames = ifaces + .iter() + .map(|iface| { + let iface = if options.pinning_enabled { + &iface.to_pinned(&options.pinning_options) + } else { + iface + }; + + (iface.render(), iface.clone()) + }) + .collect::>(); + + let mut view = SelectView::new() + .popup() + .with_all(ifnames.clone()) + .on_submit({ + let options_ref = options_ref.clone(); + move |_, iface| { + options_ref.lock().expect("unpoisoned lock").selected_mac = iface.mac.clone(); + } + }); + + // Finally, (try to) select the current one + let selected = view + .iter() + .position(|(_label, iface)| iface.mac == options.selected_mac) + .unwrap_or(0); // we sort UP interfaces first, so select the first UP interface + // + view.set_selection(selected); + + view + } } impl ViewWrapper for NetworkOptionsView { - cursive::wrap_impl!(self.view: FormView); + cursive::wrap_impl!(self.view: LinearLayout); +} + +struct InterfacePinningOptionsView { + view: ScrollView>>, +} + +impl InterfacePinningOptionsView { + const FORM_NAME: &str = "network-interface-name-pinning-form"; + + fn new(interfaces: &[&Interface], options_ref: NetworkViewOptionsRef) -> Self { + let options = options_ref.lock().expect("unpoisoned lock"); + + let mut form = FormView::::new(); + + for iface in interfaces { + let label = format!( + "{} ({}, {}, {})", + iface.mac, iface.name, iface.driver, iface.state + ); + + let view = LinearLayout::horizontal() + .child(DummyView.full_width()) // right align below form elements + .child( + EditView::new() + .content(iface.to_pinned(&options.pinning_options).name) + .max_content_width(MAX_IFNAME_LEN) + .fixed_width(MAX_IFNAME_LEN), + ); + + form.add_child_with_data(&label, view, iface.mac.clone()); + + if !iface.addresses.is_empty() { + for chunk in iface.addresses.chunks(2) { + let addrs = chunk + .iter() + .map(|v| v.to_string()) + .collect::>() + .join(", "); + + form.add_child_with_custom_label(&format!(" {addrs}\n"), DummyView); + } + } + } + + Self { + view: ScrollView::new(form.with_name(Self::FORM_NAME)), + } + } + + fn get_values(&mut self) -> Result { + let form = self.view.get_inner_mut().get_mut(); + + let mut mapping = HashMap::new(); + + for i in 0..form.len() { + let (inner, mac) = match form.get_child_with_data::(i) { + Some(formdata) => formdata, + None => continue, + }; + + let name = inner + .get_child(1) + .and_then(|v| v.downcast_ref::>()) + .map(|v| v.get_inner().get_content()) + .ok_or_else(|| format!("failed to retrieve pinning ID for interface {}", mac))?; + + mapping.insert(mac.clone(), (*name).clone()); + } + + let opts = NetworkInterfacePinningOptions { mapping }; + opts.verify().map_err(|err| err.to_string())?; + + Ok(opts) + } +} + +impl ViewWrapper for InterfacePinningOptionsView { + cursive::wrap_impl!(self.view: ScrollView>>); } -- 2.51.0 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel