From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id C92821FF0B4 for ; Tue, 08 Sep 2026 12:19:35 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 2C3CB2153C; Tue, 08 Sep 2026 12:19:23 +0200 (CEST) From: Christoph Heiss To: pve-devel@lists.proxmox.com Subject: [PATCH installer 15/15] tui: network: add bond setup options to advanced network dialog Date: Tue, 8 Sep 2026 12:16:29 +0200 Message-ID: <20260908101647.1057780-16-c.heiss@proxmox.com> X-Mailer: git-send-email 2.55.0 In-Reply-To: <20260908101647.1057780-1-c.heiss@proxmox.com> References: <20260908101647.1057780-1-c.heiss@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1788862688886 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.338 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: NIX4453ZYFLDGM7YBMVIAHUBT4EIUKNC X-Message-ID-Hash: NIX4453ZYFLDGM7YBMVIAHUBT4EIUKNC X-MailFrom: c.heiss@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Partially fixes #2164 [0]. Adds a second panel to the advanced network options dialog, allowing to enable bonding and setting all the required options for it; such as members, mode, hash policy and primary bond member, as required. [0] https://bugzilla.proxmox.com/show_bug.cgi?id=2164 Signed-off-by: Christoph Heiss --- proxmox-tui-installer/Cargo.toml | 1 + proxmox-tui-installer/src/main.rs | 7 +- proxmox-tui-installer/src/options.rs | 45 ++- proxmox-tui-installer/src/setup.rs | 5 + proxmox-tui-installer/src/views/network.rs | 415 ++++++++++++++++++--- 5 files changed, 424 insertions(+), 49 deletions(-) diff --git a/proxmox-tui-installer/Cargo.toml b/proxmox-tui-installer/Cargo.toml index 56395a4..cb950b3 100644 --- a/proxmox-tui-installer/Cargo.toml +++ b/proxmox-tui-installer/Cargo.toml @@ -9,6 +9,7 @@ homepage = "https://www.proxmox.com" [dependencies] proxmox-installer-common.workspace = true +proxmox-network-api.workspace = true proxmox-network-types.workspace = true proxmox-installer-types.workspace = true anyhow.workspace = true diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs index 61a06f7..6e7c691 100644 --- a/proxmox-tui-installer/src/main.rs +++ b/proxmox-tui-installer/src/main.rs @@ -177,6 +177,7 @@ fn main() { None, // We enable network interface pinning by default in the TUI Some(&NetworkInterfacePinningOptions::default()), + None, ), autoreboot: true, }, @@ -578,7 +579,11 @@ fn summary_dialog(siv: &mut Cursive) -> InstallerView { ("name".to_owned(), "Option".to_owned()), ("value".to_owned(), "Selected value".to_owned()), ]) - .items(state.options.to_summary(&state.locales)), + .items( + state + .options + .to_summary(&state.runtime_info, &state.locales), + ), )) .child( LinearLayout::horizontal() diff --git a/proxmox-tui-installer/src/options.rs b/proxmox-tui-installer/src/options.rs index 2c156e8..8b650f1 100644 --- a/proxmox-tui-installer/src/options.rs +++ b/proxmox-tui-installer/src/options.rs @@ -2,9 +2,9 @@ use crate::SummaryOption; use proxmox_installer_common::{ options::{BootdiskOptions, NetworkOptions, TimezoneOptions}, - setup::LocaleInfo, + setup::{LocaleInfo, NetworkInfo, RuntimeInfo}, }; -use proxmox_installer_types::EMAIL_DEFAULT_PLACEHOLDER; +use proxmox_installer_types::{EMAIL_DEFAULT_PLACEHOLDER, answer::NetworkBondOptions}; #[derive(Clone)] pub struct PasswordOptions { @@ -31,13 +31,27 @@ pub struct InstallerOptions { } impl InstallerOptions { - pub fn to_summary(&self, locales: &LocaleInfo) -> Vec { + pub fn to_summary( + &self, + runtime_info: &RuntimeInfo, + locales: &LocaleInfo, + ) -> Vec { let kb_layout = locales .kmap .get(&self.timezone.kb_layout) .map(|l| &l.name) .unwrap_or(&self.timezone.kb_layout); + let mngmt_interface = if let Some(opts) = &self.network.bond_opts { + let members = self + .resolve_bond_member_names(opts, &runtime_info.network) + .join(" | "); + + format!("bond0 (mode: {}, members: {members})", opts.mode) + } else { + self.network.ifname.to_owned() + }; + vec![ SummaryOption::new("Bootdisk filesystem", self.bootdisk.fstype.to_string()), SummaryOption::new( @@ -52,11 +66,34 @@ impl InstallerOptions { SummaryOption::new("Timezone", &self.timezone.timezone), SummaryOption::new("Keyboard layout", kb_layout), SummaryOption::new("Administrator email", &self.password.email), - SummaryOption::new("Management interface", &self.network.ifname), + SummaryOption::new("Management interface", &mngmt_interface), SummaryOption::new("Hostname", self.network.fqdn.to_string()), SummaryOption::new("Host IP (CIDR)", self.network.address.to_string()), SummaryOption::new("Gateway", self.network.gateway.to_string()), SummaryOption::new("DNS", self.network.dns_server.to_string()), ] } + + fn resolve_bond_member_names( + &self, + bond: &NetworkBondOptions, + network: &NetworkInfo, + ) -> Vec { + let pin_opts = self.network.pinning_opts.as_ref(); + + bond.members + .iter() + .map(|mac| { + // Resolve either to the pinned name, if enabled, otherwise to the kernel-assigned one. + pin_opts + .and_then(|o| o.mapping.get(mac).cloned()) + .or_else(|| { + network.interfaces.iter().find_map(|(_, iface)| { + (iface.mac == *mac).then_some(iface.name.to_owned()) + }) + }) + .unwrap_or("".to_owned()) + }) + .collect() + } } diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs index ae4b717..fb9a305 100644 --- a/proxmox-tui-installer/src/setup.rs +++ b/proxmox-tui-installer/src/setup.rs @@ -5,6 +5,7 @@ use proxmox_installer_common::{ options::AdvancedBootdiskOptions, setup::{InstallConfig, InstallFirstBootSetup, InstallRootPassword}, }; +use proxmox_installer_types::answer::NetworkBondOptions; impl From for InstallConfig { fn from(options: InstallerOptions) -> Self { @@ -35,6 +36,10 @@ impl From for InstallConfig { subscription_key: None, mngmt_nic: options.network.ifname, + mngmt_bond: options + .network + .bond_opts + .unwrap_or_else(NetworkBondOptions::disabled), network_interface_pin_map: pinning_opts.map(|o| o.mapping.clone()).unwrap_or_default(), hostname: options.network.fqdn.host().to_owned(), diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs index f040881..f1b677d 100644 --- a/proxmox-tui-installer/src/views/network.rs +++ b/proxmox-tui-installer/src/views/network.rs @@ -18,16 +18,24 @@ use proxmox_installer_common::{ options::{NetworkInterfacePinningOptions, NetworkOptions}, setup::{Interface, NetworkInfo}, }; +use proxmox_installer_types::answer::NetworkBondOptions; +use proxmox_network_api::{BondXmitHashPolicy, LinuxBondMode}; use proxmox_network_types::{MacAddress, fqdn::Fqdn, ip_address::Cidr}; -use super::{CidrAddressEditView, FormView}; +use super::{CheckboxGroup, CidrAddressEditView, FormView}; struct NetworkViewOptions { pinning_enabled: bool, + bond_enabled: bool, interfaces: Vec, + /// Last selected interface (by its MAC address) in the [SelectView]. + /// Keeps track of the selected interface across enabling/disabling bonding, which replaces the + /// view. + last_selected_iface: MacAddress, // For UI purposes, we want to always save the state for the below options, even if not enabled // by the user - to preserve the state between toggling the checkbox pinning_options: NetworkInterfacePinningOptions, + bond_options: NetworkBondOptions, } /// Convenience wrapper when needing to take a (interior-mutable) reference to @@ -46,6 +54,7 @@ impl NetworkOptionsView { const ADVANCED_OPTIONS_BUTTON_NAME: &str = "network-pinning-options-button"; const INTERFACE_PINNING_VIEW_NAME: &str = "network-name-pinning-view"; + const BOND_OPTIONS_VIEW_NAME: &str = "network-bond-options-view"; pub fn new(options: &NetworkOptions, network_info: &NetworkInfo) -> Self { let mut interfaces = network_info @@ -57,13 +66,30 @@ impl NetworkOptionsView { // First, sort interfaces by their link state and then name interfaces.sort_unstable_by(|a, b| (&a.state, &a.name).cmp(&(&b.state, &b.name))); + let selected_mac = network_info + .interfaces + .get(&options.ifname) + .map(|iface| iface.mac) + .unwrap_or_else(|| { + interfaces + .first() + .expect("at least one network interface") + .mac + }); + let options_ref = Arc::new(Mutex::new(NetworkViewOptions { pinning_enabled: options.pinning_opts.is_some(), + bond_enabled: options.bond_opts.is_some(), interfaces, + last_selected_iface: selected_mac, pinning_options: options.pinning_opts.clone().unwrap_or_default(), + bond_options: options + .bond_opts + .clone() + .unwrap_or_else(NetworkBondOptions::disabled), })); - let iface_selection = Self::build_mgmt_ifname_selectview(options_ref.clone(), None); + let iface_selection = Self::build_mgmt_ifname_selectview(options_ref.clone()); let form = FormView::<()>::new() .child( @@ -148,18 +174,26 @@ impl NetworkOptionsView { .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 options = self.options.lock().map_err(|err| err.to_string())?; - let ifname = if let Some(opts) = &pinning_opts - && let Some(pinned) = iface.to_pinned(opts) - { - pinned.name + let pinning_opts = options + .pinning_enabled + .then_some(options.pinning_options.clone()); + + let bond_opts = options.bond_enabled.then_some(options.bond_options.clone()); + + let ifname = if options.bond_enabled { + "bond0".to_owned() } else { - iface.name + let iface = form + .get_value::>, _>(0) + .ok_or("failed to retrieve management interface name")?; + + pinning_opts + .as_ref() + .and_then(|opts| iface.to_pinned(opts)) + .unwrap_or(iface) + .name }; if address.address().is_ipv4() != gateway.is_ipv4() { @@ -176,42 +210,51 @@ impl NetworkOptionsView { gateway, dns_server, pinning_opts, + bond_opts, }) } } fn advanced_options_view(options_ref: NetworkViewOptionsRef) -> impl View { let inner = ScrollView::new( - LinearLayout::vertical().child(Panel::new( - InterfacePinningOptionsView::new(options_ref.clone()) - .with_name(Self::INTERFACE_PINNING_VIEW_NAME), - )), + LinearLayout::vertical() + .child(Panel::new( + InterfacePinningOptionsView::new(options_ref.clone()) + .with_name(Self::INTERFACE_PINNING_VIEW_NAME), + )) + .child(Panel::new( + BondOptionsView::new(options_ref.clone()) + .with_name(Self::BOND_OPTIONS_VIEW_NAME), + )), ); Dialog::around(inner) .title("Advanced Network Options") .button("Ok", { move |siv| { - match Self::get_view_values(siv) { - Ok(pin_opts) => { + match Self::get_view_values(siv, options_ref.clone()) { + Ok((pin_opts, bond_opts)) => { siv.pop_layer(); let options = &mut options_ref.lock().expect("unpoisoned lock"); options.pinning_options = pin_opts; + options.bond_options = bond_opts; } Err(err) => { - siv.pop_layer(); siv.add_layer(Dialog::info(err.to_string())); return; } } - Self::refresh_ifname_selectview(siv, options_ref.clone()); + Self::refresh_ifname_view(siv, options_ref.clone()); } }) .max_size((80, 40)) } - fn get_view_values(siv: &mut Cursive) -> Result { + fn get_view_values( + siv: &mut Cursive, + options_ref: NetworkViewOptionsRef, + ) -> Result<(NetworkInterfacePinningOptions, NetworkBondOptions)> { let pin_opts = siv .call_on_name( Self::INTERFACE_PINNING_VIEW_NAME, @@ -221,23 +264,42 @@ impl NetworkOptionsView { bail!("failed to retrieve network interface name pinning options") })?; - Ok(pin_opts) + let bond_opts = siv + .call_on_name(Self::BOND_OPTIONS_VIEW_NAME, BondOptionsView::get_values) + .unwrap_or_else(|| bail!("failed to retrieve management network bond options"))?; + + if options_ref + .lock() + .map(|opts| opts.bond_enabled) + .unwrap_or(false) + { + bond_opts.verify()?; + } + + Ok((pin_opts, bond_opts)) } - fn refresh_ifname_selectview(siv: &mut Cursive, options_ref: NetworkViewOptionsRef) { - siv.call_on_name( - Self::MGMT_IFNAME_SELECTVIEW_NAME, - |view: &mut SelectView| { - let selected_mac = view.selection().map(|iface| iface.mac); - *view = Self::build_mgmt_ifname_selectview(options_ref, selected_mac); - }, - ); + fn refresh_ifname_view(siv: &mut Cursive, options_ref: NetworkViewOptionsRef) { + siv.call_on_name(Self::FORM_VIEW_NAME, |form: &mut FormView<()>| { + let options = options_ref.lock().expect("unpoisoned lock"); + + if options.bond_enabled { + form.replace_child( + 0, + TextView::new(format!( + "bond0 (mode: {}, {} members)", + options.bond_options.mode, + options.bond_options.members.len() + )), + ); + } else { + drop(options); + form.replace_child(0, Self::build_mgmt_ifname_selectview(options_ref.clone())); + } + }); } - fn build_mgmt_ifname_selectview( - options_ref: NetworkViewOptionsRef, - selected_mac: Option, - ) -> SelectView { + fn build_mgmt_ifname_selectview(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 @@ -255,20 +317,21 @@ impl NetworkOptionsView { }) .collect::>(); - let mut view = SelectView::new().popup().with_all(ifnames.clone()); - - let selected_mac = selected_mac.unwrap_or_else(|| { - options - .interfaces - .first() - .expect("at least one network interface") - .mac - }); + let mut view = SelectView::new() + .popup() + .with_all(ifnames.clone()) + .on_submit({ + let options_ref = options_ref.clone(); + move |_, iface| { + let mut options = options_ref.lock().expect("unpoisoned lock"); + options.last_selected_iface = iface.mac; + } + }); // Finally, (try to) select the current one let selected = view .iter() - .position(|(_label, iface)| iface.mac == selected_mac) + .position(|(_label, iface)| iface.mac == options.last_selected_iface) .unwrap_or(0); // we sort UP interfaces first, so select the first UP interface // view.set_selection(selected); @@ -313,6 +376,7 @@ impl InterfacePinningOptionsView { enabled; Self::toggle_all(siv, enabled); + BondOptionsView::refresh_ifnames(siv, options_ref.clone()); } }) .with_name(Self::ENABLED_CHECKBOX_NAME), @@ -414,3 +478,266 @@ impl InterfacePinningOptionsView { impl ViewWrapper for InterfacePinningOptionsView { cursive::wrap_impl!(self.view: LinearLayout); } + +struct BondOptionsView { + view: LinearLayout, + options_ref: NetworkViewOptionsRef, +} + +impl BondOptionsView { + const FORM_NAME: &str = "network-bond-form-view"; + const MEMBERS_VIEW_NAME: &str = "network-bond-members-checkboxes-view"; + + fn new(options_ref: NetworkViewOptionsRef) -> Self { + let options = &options_ref.lock().expect("unpoisoned lock"); + + let mut view = LinearLayout::vertical() + .child( + LinearLayout::horizontal() + .child( + Checkbox::new() + .with_checked(options.bond_enabled) + .on_change({ + let options_ref = options_ref.clone(); + move |siv, enabled| { + options_ref.lock().expect("unpoisoned lock").bond_enabled = + enabled; + + Self::toggle_all(siv, enabled); + } + }), + ) + .child(TextView::new(" Enable bonding for management interface").no_wrap()), + ) + .child(DummyView::new()) + .child( + FormView::<()>::new() + .child( + "Bond Mode", + SelectView::new() + .popup() + .with_all( + NetworkBondOptions::MODES + .iter() + .map(|m| (m.to_string(), *m)), + ) + .selected( + NetworkBondOptions::MODES + .iter() + .position(|m| *m == options.bond_options.mode) + .unwrap_or_default(), + ) + .on_submit(Self::toggle_mode_dependent_options) + .with_enabled(options.bond_enabled), + ) + .child( + "Hash Policy", + SelectView::new() + .popup() + .with_all( + NetworkBondOptions::HASH_POLICIES + .iter() + .map(|m| (m.to_string(), *m)), + ) + .selected( + NetworkBondOptions::HASH_POLICIES + .iter() + .position(|m| *m == options.bond_options.hash_policy) + .unwrap_or_default(), + ) + .with_enabled( + options.bond_enabled + && Self::hash_policy_view_enabled(options.bond_options.mode), + ), + ) + .child( + "Primary interface", + EditView::new() + .content({ + options + .bond_options + .primary_interface + .and_then(|mac| { + options.interfaces.iter().find(|iface| iface.mac == mac) + }) + .and_then(|iface| iface.to_pinned(&options.pinning_options)) + .map(|iface| iface.name) + .unwrap_or_default() + }) + .with_enabled( + options.bond_enabled + && Self::primary_interface_view_enable( + options.bond_options.mode, + ), + ), + ) + .with_name(Self::FORM_NAME), + ) + .child(DummyView::new()); + + let mut checkboxes = CheckboxGroup::::new(); + for iface in options + .interfaces + .iter() + .filter(|iface| iface.pinned_id.is_some()) + { + let label = Self::format_interface_label(options, iface); + let is_member = options.bond_options.members.contains(&iface.mac); + + checkboxes.add(&label, is_member, iface.mac); + } + checkboxes.set_enabled(options.bond_enabled); + view.add_child(checkboxes.with_name(Self::MEMBERS_VIEW_NAME)); + + Self { + view, + options_ref: options_ref.clone(), + } + } + + fn hash_policy_view_enabled(mode: LinuxBondMode) -> bool { + matches!(mode, LinuxBondMode::Ieee802_3ad | LinuxBondMode::BalanceXor) + } + + fn primary_interface_view_enable(mode: LinuxBondMode) -> bool { + mode == LinuxBondMode::ActiveBackup + } + + fn toggle_all(siv: &mut Cursive, enabled: bool) { + siv.call_on_name(Self::FORM_NAME, |form: &mut FormView| { + if let Some(v) = form.get_child_mut::>(0) { + v.set_enabled(enabled); + } + if let Some(v) = form.get_child_mut::>(1) { + v.set_enabled(enabled); + } + if let Some(v) = form.get_child_mut::(2) { + v.set_enabled(enabled); + } + }); + + siv.call_on_name( + Self::MEMBERS_VIEW_NAME, + |group: &mut CheckboxGroup| { + group.set_enabled(enabled); + }, + ); + } + + fn toggle_mode_dependent_options(siv: &mut Cursive, selected: &LinuxBondMode) { + siv.call_on_name(Self::FORM_NAME, |form: &mut FormView| { + if let Some(v) = form.get_child_mut::>(1) { + v.set_enabled(Self::hash_policy_view_enabled(*selected)); + } + if let Some(v) = form.get_child_mut::(2) { + v.set_enabled(Self::primary_interface_view_enable(*selected)); + } + }); + } + + fn refresh_ifnames(siv: &mut Cursive, options_ref: NetworkViewOptionsRef) { + siv.call_on_name( + Self::MEMBERS_VIEW_NAME, + |group: &mut CheckboxGroup| { + let options = options_ref.lock().expect("unpoisoned lock"); + group.update_labels(&|_, label, mac| { + if let Some(iface) = options.interfaces.iter().find(|iface| iface.mac == *mac) { + label.set_content(Self::format_interface_label(&options, iface)); + } + }); + }, + ); + } + + fn get_values(&mut self) -> Result { + let form = self + .view + .get_child_mut(2) + .and_then(|v| v.downcast_mut::>>()) + .ok_or(anyhow!("failed to retrieve bond options form"))? + .get_mut(); + + let mode = form + .get_value::, _>(0) + .ok_or(anyhow!("failed to retrieve bond mode"))?; + + let hash_policy = form + .get_value::, _>(1) + .ok_or(anyhow!("failed to retrieve bond hash policy"))?; + + let primary_interface = form + .get_value::(2) + // translate the interface name back to the mac address, which is used as identifier + .ok_or(anyhow!("failed to retrieve bond primary interface")) + .and_then(|name| { + if name.is_empty() { + Ok(None) + } else { + Ok(Some(self.get_mac_address_for_interface_name(&name)?)) + } + })?; + + let members = self + .view + .get_child_mut(4) + .and_then(|v| v.downcast_mut::>>()) + .ok_or(anyhow!("failed to retrieve bond member interface group"))? + .get_mut() + .entries() + .filter_map(|(checked, mac)| checked.then_some(*mac)) + .collect(); + + Ok(NetworkBondOptions { + members, + mode, + hash_policy, + primary_interface, + }) + } + + fn format_interface_label(options: &NetworkViewOptions, iface: &Interface) -> String { + let name = if options.pinning_enabled { + iface + .to_pinned(&options.pinning_options) + .map(|p| p.name) + .unwrap_or_else(|| iface.name.clone()) + } else { + iface.name.clone() + }; + + let mut label = format!("{name} ({}, {}, {})", iface.mac, iface.driver, iface.state); + + if !iface.addresses.is_empty() { + label += "\n "; + label += iface + .addresses + .iter() + .fold(String::new(), |acc, addr| format!("{acc}, {addr}")) + .trim_start_matches(", "); + } + + label + } + + fn get_mac_address_for_interface_name(&self, ifname: &str) -> Result { + let options = &self.options_ref.lock().expect("unpoisoned lock"); + + if options.pinning_enabled { + options + .pinning_options + .mapping + .iter() + .find_map(|(mac, name)| (*name == ifname).then_some(*mac)) + } else { + options + .interfaces + .iter() + .find_map(|iface| (iface.name == ifname).then_some(iface.mac)) + } + .ok_or_else(|| anyhow!("interface '{ifname}' not found")) + } +} + +impl ViewWrapper for BondOptionsView { + cursive::wrap_impl!(self.view: LinearLayout); +} -- 2.55.0