[pve-devel] [PATCH installer 13/14] tui: add support for pinning network interface names

[Christoph Heiss <c.heiss@proxmox.com>]

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 <c.heiss@proxmox.com>
---
 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<InstallerOptions> 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<InstallerOptions> 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<UDT> FormView<UDT> {
         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<Mutex<NetworkViewOptions>>;
+
+/// 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::<Vec<&Interface>>();
 
-        // 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::<Vec<Interface>>();
+                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<NetworkOptions, String> {
-        let ifname = self
+        let form = self
             .view
-            .get_value::<SelectView, _>(0)
+            .get_child(0)
+            .and_then(|v| v.downcast_ref::<FormView>())
+            .ok_or("failed to retrieve network options form")?;
+
+        let iface = form
+            .get_value::<NamedView<SelectView<Interface>>, _>(0)
             .ok_or("failed to retrieve management interface name")?;
 
-        let fqdn = self
-            .view
+        let fqdn = form
             .get_value::<EditView, _>(1)
             .ok_or("failed to retrieve host FQDN")?
             .parse::<Fqdn>()
             .map_err(|err| format!("hostname does not look valid:\n\n{err}"))?;
 
-        let address = self
-            .view
+        let address = form
             .get_value::<CidrAddressEditView, _>(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::<EditView, _>(3)
             .ok_or("failed to retrieve gateway address")?
             .parse::<IpAddr>()
             .map_err(|err| err.to_string())?;
 
-        let dns_server = self
-            .view
+        let dns_server = form
             .get_value::<EditView, _>(4)
             .ok_or("failed to retrieve DNS server address")?
             .parse::<IpAddr>()
             .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::<Vec<&Interface>>();
+
+        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::<Vec<Interface>>();
+            move |siv| {
+                let options = siv
+                    .call_on_name(DIALOG_NAME, |view: &mut Dialog| {
+                        view.get_content_mut()
+                            .downcast_mut::<InterfacePinningOptionsView>()
+                            .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<Interface>| {
+                *view = Self::build_mgmt_ifname_selectview(ifaces.iter().collect(), options_ref);
+            },
+        );
+    }
+
+    fn build_mgmt_ifname_selectview(
+        ifaces: Vec<&Interface>,
+        options_ref: NetworkViewOptionsRef,
+    ) -> SelectView<Interface> {
+        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::<Vec<(String, Interface)>>();
+
+        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<NamedView<FormView<String>>>,
+}
+
+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::<String>::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::<Vec<String>>()
+                        .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<NetworkInterfacePinningOptions, String> {
+        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::<LinearLayout>(i) {
+                Some(formdata) => formdata,
+                None => continue,
+            };
+
+            let name = inner
+                .get_child(1)
+                .and_then(|v| v.downcast_ref::<ResizedView<EditView>>())
+                .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<NamedView<FormView<String>>>);
 }
-- 
2.51.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel