* [PATCH installer v5 01/23] install: run make tidy
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 02/23] tree-wide: use `MacAddress` type instead of string for MAC addresses Christoph Heiss
` (21 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* new patch
Proxmox/Install.pm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index 6298cff..7bb8e29 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -673,7 +673,8 @@ sub prepare_grub_efi_boot_esp {
# without a shim package installed (no signed shim exists for arm64 yet) grub-install
# deploys only the plain grub image, so use that as the removable-media default loader
$shim_src = $run_env->{arch} eq 'arm64' ? 'grubaa64.efi' : 'grubx64.efi';
- warn "no shim binary found, using '$shim_src' as removable-media default boot loader\n";
+ warn
+ "no shim binary found, using '$shim_src' as removable-media default boot loader\n";
}
syscmd(
"mv $targetdir/boot/efi/EFI/BOOT/$shim_src $targetdir/boot/efi/EFI/BOOT/$boot_dst")
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 02/23] tree-wide: use `MacAddress` type instead of string for MAC addresses
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 01/23] install: run make tidy Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 03/23] install: factor out ifupdown2 config generation into own subroutine Christoph Heiss
` (20 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Makes all Rust code dealing with MAC addresses a lot more typesafe. The
`MacAddress` type is also cheaper to construct than `String`, since the
former is just 6 bytes and avoids heap allocations.
MAC addresses are still passed to the low-level installer in lowercase,
as it compares them with the ones reported by iproute2, and the
post-installation webhook data keeps them in lowercase too. Error
messages now show them in uppercase.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* no changes
Cargo.toml | 2 +-
debian/control | 2 +-
proxmox-auto-installer/tests/parse-answer.rs | 2 +-
...ace_pinning_mixed_case_mac_addresses.json} | 0
...ace_pinning_mixed_case_mac_addresses.toml} | 4 +-
...rface_pinning_overlong_interface_name.json | 2 +-
proxmox-installer-common/src/options.rs | 66 +++++++++----------
proxmox-installer-common/src/setup.rs | 28 ++++++--
proxmox-post-hook/src/main.rs | 2 +-
proxmox-tui-installer/src/views/network.rs | 26 +++-----
10 files changed, 70 insertions(+), 64 deletions(-)
rename proxmox-auto-installer/tests/resources/parse_answer/{network_interface_pinning_uppercase_mac_address.json => network_interface_pinning_mixed_case_mac_addresses.json} (100%)
rename proxmox-auto-installer/tests/resources/parse_answer/{network_interface_pinning_uppercase_mac_address.toml => network_interface_pinning_mixed_case_mac_addresses.toml} (85%)
diff --git a/Cargo.toml b/Cargo.toml
index cda950c..747608e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -28,7 +28,7 @@ toml = "0.8"
proxmox-auto-installer.path = "./proxmox-auto-installer"
proxmox-installer-common.path = "./proxmox-installer-common"
proxmox-network-types = "1.1"
-proxmox-installer-types = { version = "0.2.1", features = ["legacy"] }
+proxmox-installer-types = { version = "0.3", features = ["legacy"] }
# Local path overrides
# NOTE: You must run `cargo update` after changing this for it to take effect!
diff --git a/debian/control b/debian/control
index eb99820..e860819 100644
--- a/debian/control
+++ b/debian/control
@@ -19,7 +19,7 @@ Build-Depends: cargo:native,
librust-native-tls-dev,
librust-pico-args-0.5-dev,
librust-pretty-assertions-1.4-dev,
- librust-proxmox-installer-types-0.2+legacy-dev (>= 0.2.1-~~),
+ librust-proxmox-installer-types-0.3+legacy-dev,
librust-proxmox-network-types-1-dev (>= 1.1-~~),
librust-proxmox-sys+crypt-dev,
librust-regex-1+default-dev (>= 1.7~~),
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 8b0bae7..ae5b87d 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -128,7 +128,7 @@ mod tests {
hashed_root_password,
minimal,
network_interface_pinning,
- network_interface_pinning_uppercase_mac_address,
+ network_interface_pinning_mixed_case_mac_addresses,
nic_matching,
no_network,
specific_nic,
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning_uppercase_mac_address.json b/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning_mixed_case_mac_addresses.json
similarity index 100%
rename from proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning_uppercase_mac_address.json
rename to proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning_mixed_case_mac_addresses.json
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning_uppercase_mac_address.toml b/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning_mixed_case_mac_addresses.toml
similarity index 85%
rename from proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning_uppercase_mac_address.toml
rename to proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning_mixed_case_mac_addresses.toml
index 6681fe9..deb1fd3 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning_uppercase_mac_address.toml
+++ b/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning_mixed_case_mac_addresses.toml
@@ -14,8 +14,8 @@ enabled = true
[network.interface-name-pinning.mapping]
"24:8A:07:1E:05:BC" = "lan0"
-"24:8A:07:1E:05:BD" = "lan1"
-"B4:2E:99:AC:AD:B4" = "mgmt"
+"24:8a:07:1e:05:bd" = "lan1"
+"B4:2e:99:ac:AD:b4" = "mgmt"
[disk-setup]
filesystem = "ext4"
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json
index f3c9169..952b34a 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json
@@ -1,3 +1,3 @@
{
- "error": "interface name 'waytoolonginterfacename' for 'ab:cd:ef:12:34:56' cannot be longer than 15 characters"
+ "error": "interface name 'waytoolonginterfacename' for 'AB:CD:EF:12:34:56' cannot be longer than 15 characters"
}
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index c7f0baf..fbbfe77 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -19,7 +19,7 @@ use proxmox_installer_types::{
ZfsChecksumOption, ZfsCompressOption, ZfsRaidLevel,
},
};
-use proxmox_network_types::{fqdn::Fqdn, ip_address::Cidr};
+use proxmox_network_types::{MacAddress, fqdn::Fqdn, ip_address::Cidr};
pub trait RaidLevel {
/// Returns the minimum number of disks needed for this RAID level.
@@ -315,7 +315,7 @@ impl TimezoneOptions {
pub struct NetworkInterfacePinningOptions {
/// Maps MAC address to custom name
#[serde(default)]
- pub mapping: HashMap<String, String>,
+ pub mapping: HashMap<MacAddress, String>,
}
impl NetworkInterfacePinningOptions {
@@ -341,7 +341,7 @@ impl NetworkInterfacePinningOptions {
.unwrap()
});
- let mut reverse_mapping = HashMap::<String, String>::new();
+ let mut reverse_mapping = HashMap::<String, MacAddress>::new();
for (mac, name) in self.mapping.iter() {
if name.len() < MIN_IFNAME_LEN {
bail!(
@@ -368,7 +368,7 @@ impl NetworkInterfacePinningOptions {
bail!("duplicate interface name mapping '{name}' for: {mac}, {duplicate_mac}");
}
- reverse_mapping.insert(name.clone(), mac.clone());
+ reverse_mapping.insert(name.clone(), *mac);
}
Ok(())
@@ -379,13 +379,7 @@ impl From<&NetworkInterfacePinningOptionsAnswer> for NetworkInterfacePinningOpti
fn from(answer: &NetworkInterfacePinningOptionsAnswer) -> Self {
if answer.enabled {
Self {
- // convert all MAC addresses to lowercase before further usage,
- // to enable easy comparison
- mapping: answer
- .mapping
- .iter()
- .map(|(k, v)| (k.to_lowercase(), v.clone()))
- .collect(),
+ mapping: answer.mapping.clone(),
}
} else {
Self::default()
@@ -480,7 +474,7 @@ impl NetworkOptions {
// required by the low-level installer
for iface in network.interfaces.values() {
if let Some(pinned) = iface.to_pinned(opts) {
- opts.mapping.entry(iface.mac.clone()).or_insert(pinned.name);
+ opts.mapping.entry(iface.mac).or_insert(pinned.name);
}
}
}
@@ -609,7 +603,7 @@ mod tests {
pinned_id: Some("0".to_owned()),
state: InterfaceState::Up,
driver: "dummy".to_owned(),
- mac: "01:23:45:67:89:ab".to_owned(),
+ mac: "01:23:45:67:89:ab".parse().unwrap(),
addresses: vec![Cidr::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap()],
},
);
@@ -699,7 +693,7 @@ mod tests {
pinned_id: Some("0".to_owned()),
state: InterfaceState::Up,
driver: "dummy".to_owned(),
- mac: "01:23:45:67:89:ab".to_owned(),
+ mac: "01:23:45:67:89:ab".parse().unwrap(),
addresses: vec![
Cidr::new(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 2), 64).unwrap(),
],
@@ -798,7 +792,7 @@ mod tests {
pinned_id: Some("0".to_owned()),
state: InterfaceState::Up,
driver: "dummy".to_owned(),
- mac: "01:23:45:67:89:ab".to_owned(),
+ mac: "01:23:45:67:89:ab".parse().unwrap(),
addresses: vec![],
},
);
@@ -836,13 +830,13 @@ mod tests {
let mut options = NetworkInterfacePinningOptions::default();
options
.mapping
- .insert("ab:cd:ef:12:34:56".to_owned(), String::new());
+ .insert("ab:cd:ef:12:34:56".parse().unwrap(), 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' must be at least 2 characters long"
+ "interface name for 'AB:CD:EF:12:34:56' must be at least 2 characters long"
)
}
@@ -851,13 +845,13 @@ mod tests {
let mut options = NetworkInterfacePinningOptions::default();
options
.mapping
- .insert("ab:cd:ef:12:34:56".to_owned(), "a".to_owned());
+ .insert("ab:cd:ef:12:34:56".parse().unwrap(), "a".to_owned());
let res = options.verify();
assert!(res.is_err());
assert_eq!(
res.unwrap_err().to_string(),
- "interface name for 'ab:cd:ef:12:34:56' must be at least 2 characters long"
+ "interface name for 'AB:CD:EF:12:34:56' must be at least 2 characters long"
)
}
@@ -865,7 +859,7 @@ mod tests {
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(),
+ "ab:cd:ef:12:34:56".parse().unwrap(),
"waytoolonginterfacename".to_owned(),
);
@@ -873,7 +867,7 @@ mod tests {
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"
+ "interface name 'waytoolonginterfacename' for 'AB:CD:EF:12:34:56' cannot be longer than 15 characters"
)
}
@@ -882,10 +876,10 @@ mod tests {
let mut options = NetworkInterfacePinningOptions::default();
options
.mapping
- .insert("ab:cd:ef:12:34:56".to_owned(), "nic0".to_owned());
+ .insert("ab:cd:ef:12:34:56".parse().unwrap(), "nic0".to_owned());
options
.mapping
- .insert("12:34:56:ab:cd:ef".to_owned(), "nic0".to_owned());
+ .insert("12:34:56:ab:cd:ef".parse().unwrap(), "nic0".to_owned());
let res = options.verify();
assert!(res.is_err());
@@ -894,8 +888,8 @@ mod tests {
// [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"));
+ assert!(err.contains("12:34:56:AB:CD:EF"));
+ assert!(err.contains("AB:CD:EF:12:34:56"));
}
#[test]
@@ -903,13 +897,13 @@ mod tests {
let mut options = NetworkInterfacePinningOptions::default();
options
.mapping
- .insert("ab:cd:ef:12:34:56".to_owned(), "nic-".to_owned());
+ .insert("ab:cd:ef:12:34:56".parse().unwrap(), "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 start with a letter and contain only ascii characters, digits and underscores"
+ "interface name 'nic-' for 'AB:CD:EF:12:34:56' is invalid: name must start with a letter and contain only ascii characters, digits and underscores"
)
}
@@ -918,24 +912,24 @@ mod tests {
let mut options = NetworkInterfacePinningOptions::default();
options
.mapping
- .insert("ab:cd:ef:12:34:56".to_owned(), "0nic".to_owned());
+ .insert("ab:cd:ef:12:34:56".parse().unwrap(), "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 start with a letter and contain only ascii characters, digits and underscores"
+ "interface name '0nic' for 'AB:CD:EF:12:34:56' is invalid: name must start with a letter and contain only ascii characters, digits and underscores"
);
options
.mapping
- .insert("ab:cd:ef:12:34:56".to_owned(), "_a".to_owned());
+ .insert("ab:cd:ef:12:34:56".parse().unwrap(), "_a".to_owned());
let res = options.verify();
assert!(res.is_err());
assert_eq!(
res.unwrap_err().to_string(),
- "interface name '_a' for 'ab:cd:ef:12:34:56' is invalid: name must start with a letter and contain only ascii characters, digits and underscores"
+ "interface name '_a' for 'AB:CD:EF:12:34:56' is invalid: name must start with a letter and contain only ascii characters, digits and underscores"
);
}
@@ -944,21 +938,21 @@ mod tests {
let mut options = NetworkInterfacePinningOptions::default();
options
.mapping
- .insert("ab:cd:ef:12:34:56".to_owned(), "Nic0".to_owned());
+ .insert("ab:cd:ef:12:34:56".parse().unwrap(), "Nic0".to_owned());
let res = options.verify();
assert!(res.is_ok());
options
.mapping
- .insert("ab:cd:ef:12:34:56".to_owned(), "nIc0".to_owned());
+ .insert("ab:cd:ef:12:34:56".parse().unwrap(), "nIc0".to_owned());
let res = options.verify();
assert!(res.is_ok());
options
.mapping
- .insert("ab:cd:ef:12:34:56".to_owned(), "nic0".to_owned());
+ .insert("ab:cd:ef:12:34:56".parse().unwrap(), "nic0".to_owned());
let res = options.verify();
assert!(res.is_ok());
@@ -969,13 +963,13 @@ mod tests {
let mut options = NetworkInterfacePinningOptions::default();
options
.mapping
- .insert("ab:cd:ef:12:34:56".to_owned(), "12345".to_owned());
+ .insert("ab:cd:ef:12:34:56".parse().unwrap(), "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 start with a letter and contain only ascii characters, digits and underscores"
+ "interface name '12345' for 'AB:CD:EF:12:34:56' is invalid: name must start with a letter and contain only ascii characters, digits and underscores"
)
}
}
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index b8af4e3..14882bc 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -1,4 +1,4 @@
-use serde::{Deserialize, Deserializer, Serialize, Serializer, de};
+use serde::{Deserialize, Deserializer, Serialize, Serializer, de, ser::SerializeMap};
use std::{
cmp,
collections::{BTreeMap, HashMap},
@@ -18,7 +18,7 @@ use proxmox_installer_types::{
BootType, IsoInfo, ProductConfig,
answer::{BtrfsCompressOption, FilesystemType, ZfsChecksumOption, ZfsCompressOption},
};
-use proxmox_network_types::Cidr;
+use proxmox_network_types::{Cidr, MacAddress};
/// Paths in the ISO environment containing installer data.
#[derive(Clone, Deserialize)]
@@ -395,7 +395,7 @@ pub struct Interface {
/// interface name cannot/should not be pinned due to being e.g. a non-physical link.
pub pinned_id: Option<String>,
- pub mac: String,
+ pub mac: MacAddress,
pub state: InterfaceState,
@@ -535,9 +535,10 @@ pub struct InstallConfig {
#[serde(
default,
skip_serializing_if = "HashMap::is_empty",
- deserialize_with = "deserialize_optional_map"
+ deserialize_with = "deserialize_optional_map",
+ serialize_with = "serialize_low_level_mac_address_map"
)]
- pub network_interface_pin_map: HashMap<String, String>,
+ pub network_interface_pin_map: HashMap<MacAddress, String>,
pub hostname: String,
pub domain: String,
@@ -583,3 +584,20 @@ where
let map: Option<HashMap<K, V>> = Deserialize::deserialize(deserializer)?;
Ok(map.unwrap_or_default())
}
+
+/// Serializes a [HashMap] using [MacAddress] as keys, normalizing MAC addresses into their
+/// lower-case format as the low-level installer expects it.
+fn serialize_low_level_mac_address_map<S, V>(
+ map: &HashMap<MacAddress, V>,
+ serializer: S,
+) -> Result<S::Ok, S::Error>
+where
+ S: Serializer,
+ V: Serialize,
+{
+ let mut serializer = serializer.serialize_map(Some(map.len()))?;
+ for (mac, v) in map {
+ serializer.serialize_entry(&mac.to_string().to_lowercase(), &v)?;
+ }
+ serializer.end()
+}
diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
index 579d325..7a0ae82 100644
--- a/proxmox-post-hook/src/main.rs
+++ b/proxmox-post-hook/src/main.rs
@@ -286,7 +286,7 @@ mod detail {
anyhow::Ok(NetworkInterfaceInfo {
name: ifname.clone(),
- mac: nic.mac.clone(),
+ mac: nic.mac,
// Use the actual IP address from the low-level install config, as the runtime info
// contains the original IP address from DHCP.
address: is_management.then_some(config.cidr),
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
index 12cef19..6401174 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -17,12 +17,12 @@ use proxmox_installer_common::{
options::{NetworkInterfacePinningOptions, NetworkOptions},
setup::{Interface, NetworkInfo},
};
-use proxmox_network_types::{fqdn::Fqdn, ip_address::Cidr};
+use proxmox_network_types::{MacAddress, fqdn::Fqdn, ip_address::Cidr};
use super::{CidrAddressEditView, FormView};
struct NetworkViewOptions {
- selected_mac: String,
+ selected_mac: MacAddress,
pinning_enabled: bool,
// For UI purposes, we want to always save the mapping, to save the state
// between toggling the checkbox
@@ -55,14 +55,8 @@ impl NetworkOptionsView {
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()
- });
+ .map(|iface| iface.mac)
+ .unwrap_or_else(|| ifaces.first().expect("at least one network interface").mac);
let options_ref = Arc::new(Mutex::new(NetworkViewOptions {
selected_mac,
@@ -309,7 +303,7 @@ impl NetworkOptionsView {
.on_submit({
let options_ref = options_ref.clone();
move |_, iface| {
- options_ref.lock().expect("unpoisoned lock").selected_mac = iface.mac.clone();
+ options_ref.lock().expect("unpoisoned lock").selected_mac = iface.mac;
}
});
@@ -330,7 +324,7 @@ impl ViewWrapper for NetworkOptionsView {
}
struct InterfacePinningOptionsView {
- view: ScrollView<NamedView<FormView<String>>>,
+ view: ScrollView<NamedView<FormView<MacAddress>>>,
}
impl InterfacePinningOptionsView {
@@ -344,7 +338,7 @@ impl InterfacePinningOptionsView {
// The low-level installer will skip them anyway.
let interfaces = interfaces.iter().filter(|iface| iface.pinned_id.is_some());
- let mut form = FormView::<String>::new();
+ let mut form = FormView::<MacAddress>::new();
for iface in interfaces {
let label = format!(
@@ -366,7 +360,7 @@ impl InterfacePinningOptionsView {
.fixed_width(MAX_IFNAME_LEN),
);
- form.add_child_with_data(&label, view, iface.mac.clone());
+ form.add_child_with_data(&label, view, iface.mac);
if !iface.addresses.is_empty() {
for chunk in iface.addresses.chunks(2) {
@@ -403,7 +397,7 @@ impl InterfacePinningOptionsView {
.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());
+ mapping.insert(*mac, (*name).clone());
}
let opts = NetworkInterfacePinningOptions { mapping };
@@ -414,5 +408,5 @@ impl InterfacePinningOptionsView {
}
impl ViewWrapper for InterfacePinningOptionsView {
- cursive::wrap_impl!(self.view: ScrollView<NamedView<FormView<String>>>);
+ cursive::wrap_impl!(self.view: ScrollView<NamedView<FormView<MacAddress>>>);
}
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 03/23] install: factor out ifupdown2 config generation into own subroutine
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 01/23] install: run make tidy Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 02/23] tree-wide: use `MacAddress` type instead of string for MAC addresses Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 04/23] install: do not rely on interface name map to be fully populated Christoph Heiss
` (19 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Makes it more readable and to add more logic in the future, as well as
being now unit-testable.
While at it, unify the `address` and `gateway` and keep it of the
branched paths for bridged/unbridged. Makes things simpler and the
final configuration more predictable in layout.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* only generate config in subroutine, keep the writing of
/etc/network/interfaces in the main logic
* fix missing translation of $ethdev to pinned name, by passing it
directly as a parameter
* add initial testsuite for ifupdown2 config generation (including
additional tests from @Thomas)
* simplify config generation a bit by moving address and gateway
setup into common path
Changes v1 -> v2:
* no changes
Proxmox/Install.pm | 134 ++++++++++----------
test/Makefile | 6 +-
test/generate-ifupdown2-config.pl | 197 ++++++++++++++++++++++++++++++
3 files changed, 270 insertions(+), 67 deletions(-)
create mode 100755 test/generate-ifupdown2-config.pl
diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index 7bb8e29..a98571f 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -798,6 +798,71 @@ my sub setup_proxmox_first_boot_service {
}
}
+sub generate_ifupdown2_config {
+ my $run_env = Proxmox::Install::RunEnv::get();
+ my $iso_env = Proxmox::Install::ISOEnv::get();
+
+ my $ip_version = Proxmox::Install::Config::get_ip_version();
+ my $ntype = $ip_version == 4 ? 'inet' : 'inet6';
+
+ my $interfaces = $run_env->{network}->{interfaces};
+ my $ethdev = Proxmox::Install::Config::get_mngmt_nic();
+ my $cidr = Proxmox::Install::Config::get_cidr();
+ my $gateway = Proxmox::Install::Config::get_gateway();
+
+ my $netif_pin_map = Proxmox::Install::Config::get_network_interface_pin_map();
+
+ # frontends may pass the original kernel name of an interface that gets pinned
+ if (defined($netif_pin_map) && defined($interfaces->{$ethdev})) {
+ $ethdev = $netif_pin_map->{ $interfaces->{$ethdev}->{mac} } // $ethdev;
+ }
+
+ my $ifaces = "auto lo\niface lo inet loopback\n\n";
+
+ # iproute2 omits the gateway and protocol keys for some routes; route devices are original
+ # kernel interface names, while $ethdev can be the pinned one
+ my $gateways6 = $run_env->{network}->{routes}->{gateways6} // [];
+ my $is_gateway6_from_ra = grep {
+ my $dev = $_->{dev} // '';
+ if (defined($netif_pin_map) && defined($interfaces->{$dev})) {
+ $dev = $netif_pin_map->{ $interfaces->{$dev}->{mac} } // $dev;
+ }
+ $dev eq $ethdev
+ && ($_->{gateway} // '') eq $gateway
+ && ($_->{protocol} // '') eq 'ra';
+ } $gateways6->@*;
+
+ if ($iso_env->{cfg}->{bridged_network}) {
+ $ifaces .= "iface $ethdev $ntype manual\n";
+
+ $ifaces .=
+ "\nauto vmbr0\niface vmbr0 $ntype static\n"
+ . "\tbridge-ports $ethdev\n"
+ . "\tbridge-stp off\n"
+ . "\tbridge-fd 0\n";
+ } else {
+ $ifaces .= "auto $ethdev\n" . #
+ "iface $ethdev $ntype static\n";
+ }
+
+ $ifaces .= "\taddress $cidr\n";
+ if (!$is_gateway6_from_ra) {
+ $ifaces .= "\tgateway $gateway\n";
+ }
+
+ foreach my $iface (sort keys $interfaces->%*) {
+ my $if = $interfaces->{$iface};
+ my $name = defined($netif_pin_map) ? $netif_pin_map->{ $if->{mac} } : $if->{name};
+
+ next if $name eq $ethdev;
+
+ $ifaces .= "\niface $name $ntype manual\n";
+ }
+
+ $ifaces .= "\n\nsource /etc/network/interfaces.d/*\n";
+ return $ifaces;
+}
+
sub extract_data {
my $iso_env = Proxmox::Install::ISOEnv::get();
my $run_env = Proxmox::Install::RunEnv::get();
@@ -1138,21 +1203,11 @@ sub extract_data {
syscmd("/bin/hostname $hostname") if !is_test_mode();
- # configure interfaces
-
- my $ifaces = "auto lo\niface lo inet loopback\n\n";
-
- my $ip_version = Proxmox::Install::Config::get_ip_version();
- my $ntype = $ip_version == 4 ? 'inet' : 'inet6';
-
- my $ethdev = Proxmox::Install::Config::get_mngmt_nic();
- my $cidr = Proxmox::Install::Config::get_cidr();
- my $gateway = Proxmox::Install::Config::get_gateway();
-
# configure pinned names for all network interfaces, if enabled
my $netif_pin_map = Proxmox::Install::Config::get_network_interface_pin_map();
if (defined($netif_pin_map)) {
+
mkdir "$targetdir/usr/local/lib/systemd", 0755;
mkdir "$targetdir/usr/local/lib/systemd/network", 0755;
@@ -1178,64 +1233,11 @@ sub extract_data {
"$targetdir/usr/local/lib/systemd/network/50-pmx-${pinned_name}.link",
$link_file_content,
);
-
- # ensure that we use the correct interface name when writing out
- # /etc/network/interfaces below - just in case the interface name
- # is pinned and get_mngmt_nic() returned the original interface name.
- # better be safe than sorry
- if ($ethdev eq $ifname) {
- $ethdev = $pinned_name;
- }
}
}
- my $interfaces = $run_env->{network}->{interfaces};
-
- # iproute2 omits the gateway and protocol keys for some routes; route devices are original
- # kernel interface names, while $ethdev can be the pinned one
- my $gateways6 = $run_env->{network}->{routes}->{gateways6} // [];
- my $is_gateway6_from_ra = grep {
- my $dev = $_->{dev} // '';
- if (defined($netif_pin_map) && defined($interfaces->{$dev})) {
- $dev = $netif_pin_map->{ $interfaces->{$dev}->{mac} } // $dev;
- }
- $dev eq $ethdev
- && ($_->{gateway} // '') eq $gateway
- && ($_->{protocol} // '') eq 'ra';
- } $gateways6->@*;
-
- if ($iso_env->{cfg}->{bridged_network}) {
- $ifaces .= "iface $ethdev $ntype manual\n";
-
- $ifaces .=
- "\nauto vmbr0\niface vmbr0 $ntype static\n"
- . "\taddress $cidr\n"
- . "\tgateway $gateway\n"
- . "\tbridge-ports $ethdev\n"
- . "\tbridge-stp off\n"
- . "\tbridge-fd 0\n";
- } else {
- $ifaces .= "auto $ethdev\n" . #
- "iface $ethdev $ntype static\n" . #
- "\taddress $cidr\n";
-
- if (!$is_gateway6_from_ra) {
- $ifaces .= "\tgateway $gateway\n";
- }
- }
-
- foreach my $iface (sort keys $interfaces->%*) {
- my $if = $interfaces->{$iface};
- my $name = defined($netif_pin_map) ? $netif_pin_map->{ $if->{mac} } : $if->{name};
-
- next if $name eq $ethdev;
-
- $ifaces .= "\niface $name $ntype manual\n";
- }
-
- $ifaces .= "\n\nsource /etc/network/interfaces.d/*\n";
-
- file_write_all("$targetdir/etc/network/interfaces", $ifaces);
+ # configure interfaces
+ file_write_all("$targetdir/etc/network/interfaces", generate_ifupdown2_config());
# configure dns
diff --git a/test/Makefile b/test/Makefile
index bfe7674..9727244 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -5,7 +5,7 @@ export PERLLIB=..
.PHONY: check
check: test-zfs-arc-max test-run-command test-parse-fqdn test-ui2-stdio \
test-zfs-get-pool-list test-parse-kernel-cmdline \
- test-validate-link-pin-map
+ test-validate-link-pin-map test-generate-ifupdown2-config
.PHONY: test-zfs-arc-max
test-zfs-arc-max:
@@ -34,3 +34,7 @@ test-parse-kernel-cmdline:
.PHONY: test-validate-link-pin-map
test-validate-link-pin-map:
./validate-link-pin-map.pl
+
+.PHONY: test-generate-ifupdown2-config
+test-generate-ifupdown2-config:
+ ./generate-ifupdown2-config.pl
diff --git a/test/generate-ifupdown2-config.pl b/test/generate-ifupdown2-config.pl
new file mode 100755
index 0000000..5c1794e
--- /dev/null
+++ b/test/generate-ifupdown2-config.pl
@@ -0,0 +1,197 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Test::More;
+use Test::MockModule qw(strict);
+
+use Proxmox::Install;
+use Proxmox::Install::Config;
+
+use Proxmox::Log;
+Proxmox::Log::init("/tmp/install.log");
+
+my $proxmox_install_runenv = Test::MockModule->new('Proxmox::Install::RunEnv');
+my $proxmox_install_isoenv = Test::MockModule->new('Proxmox::Install::ISOEnv');
+
+my $current_cfg;
+my sub set_config {
+ my (%cfg) = @_;
+
+ $current_cfg = \%cfg;
+ $proxmox_install_isoenv->redefine(
+ get => sub {
+ return {
+ product => $cfg{bridge} ? 'pve' : 'pbs',
+ cfg => {
+ product => $cfg{bridge} ? 'pve' : 'pbs',
+ bridged_network => $cfg{bridge},
+ },
+ };
+ },
+ );
+}
+
+my sub is_generated {
+ my ($expected, $msg) = @_;
+
+ is(
+ Proxmox::Install::generate_ifupdown2_config(), <<EOT,
+auto lo
+iface lo inet loopback
+
+$expected
+
+source /etc/network/interfaces.d/*
+EOT
+ "$msg (bridge:$current_cfg->{bridge})",
+ );
+}
+
+# global config
+$proxmox_install_runenv->redefine(
+ get => sub {
+ return {
+ network => {
+ interfaces => {
+ eno1 => {
+ mac => '12:34:56:ab:cd:ef',
+ index => 1,
+ name => 'eno1',
+ },
+ eno2 => {
+ mac => 'ab:cd:ef:12:34:56',
+ index => 2,
+ name => 'eno2',
+ },
+ },
+ },
+ };
+ },
+);
+set_config(bridge => 0);
+Proxmox::Install::Config::merge({
+ mngmt_nic => 'eno1',
+ cidr => '192.168.0.10/24',
+ gateway => '192.168.0.1',
+});
+
+###
+# There's essentially a matrix to test w.r.t the overall networking setup:
+#
+# +-------+--------+
+# | test# | bridge |
+# +=======+========+
+# | 1 | 0 |
+# | 2 | 1 |
+# +-------+--------+
+###
+
+# 1.
+set_config(bridge => 0);
+is_generated(
+ <<EOT
+auto eno1
+iface eno1 inet static
+ address 192.168.0.10/24
+ gateway 192.168.0.1
+
+iface eno2 inet manual
+EOT
+ , 'host network setup',
+);
+
+# 2.
+set_config(bridge => 1);
+is_generated(
+ <<EOT
+iface eno1 inet manual
+
+auto vmbr0
+iface vmbr0 inet static
+ bridge-ports eno1
+ bridge-stp off
+ bridge-fd 0
+ address 192.168.0.10/24
+ gateway 192.168.0.1
+
+iface eno2 inet manual
+EOT
+ , 'host network setup',
+);
+
+# interface name pinning test
+Proxmox::Install::Config::merge({
+ network_interface_pin_map => {
+ '12:34:56:ab:cd:ef' => 'nic0',
+ 'ab:cd:ef:12:34:56' => 'nic1',
+ },
+});
+is_generated(
+ <<EOT
+iface nic0 inet manual
+
+auto vmbr0
+iface vmbr0 inet static
+ bridge-ports nic0
+ bridge-stp off
+ bridge-fd 0
+ address 192.168.0.10/24
+ gateway 192.168.0.1
+
+iface nic1 inet manual
+EOT
+ , 'management bridge uses pinned interface name',
+);
+Proxmox::Install::Config::merge({
+ network_interface_pin_map => undef,
+});
+
+# A gateway learned via IPv6 router advertisements is not written out
+Proxmox::Install::Config::merge({
+ mngmt_nic => 'eno1',
+ cidr => '2001:db8::1/64',
+ gateway => 'fe80::1',
+});
+$proxmox_install_runenv->redefine(
+ get => sub {
+ return {
+ network => {
+ interfaces => {
+ eno1 => {
+ mac => '12:34:56:ab:cd:ef',
+ index => 1,
+ name => 'eno1',
+ },
+ eno2 => {
+ mac => 'ab:cd:ef:12:34:56',
+ index => 2,
+ name => 'eno2',
+ },
+ },
+ routes => {
+ gateways6 => [{ dev => 'eno1', gateway => 'fe80::1', protocol => 'ra' }],
+ },
+ },
+ };
+ },
+);
+
+is_generated(
+ <<EOT
+iface eno1 inet6 manual
+
+auto vmbr0
+iface vmbr0 inet6 static
+ bridge-ports eno1
+ bridge-stp off
+ bridge-fd 0
+ address 2001:db8::1/64
+
+iface eno2 inet6 manual
+EOT
+ , 'gateway learned from IPv6 router advertisements is not configured statically',
+);
+
+done_testing();
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 04/23] install: do not rely on interface name map to be fully populated
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (2 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 03/23] install: factor out ifupdown2 config generation into own subroutine Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 05/23] install: config: add option for setting up bond on management interface Christoph Heiss
` (18 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Interfaces without a pinned ID, i.e. virtual links, never get an entry
in the interface name pinning map. Should the live environment have
such an interface, the configuration would contain a stanza without an
interface name for it, so fall back to its kernel name instead.
While this hasn't been a problem yet AFAIK, better be safe than sorry.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* add test and expanded commit message, as proposed by Thomas
Changes v1 -> v2:
* no changes
Proxmox/Install.pm | 6 +++-
test/generate-ifupdown2-config.pl | 58 ++++++++++++++++++++++++++++---
2 files changed, 59 insertions(+), 5 deletions(-)
diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index a98571f..6c7af3d 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -852,7 +852,11 @@ sub generate_ifupdown2_config {
foreach my $iface (sort keys $interfaces->%*) {
my $if = $interfaces->{$iface};
- my $name = defined($netif_pin_map) ? $netif_pin_map->{ $if->{mac} } : $if->{name};
+ my $name =
+ defined($netif_pin_map)
+ && defined($netif_pin_map->{ $if->{mac} })
+ ? $netif_pin_map->{ $if->{mac} }
+ : $if->{name};
next if $name eq $ethdev;
diff --git a/test/generate-ifupdown2-config.pl b/test/generate-ifupdown2-config.pl
index 5c1794e..7f92e4f 100755
--- a/test/generate-ifupdown2-config.pl
+++ b/test/generate-ifupdown2-config.pl
@@ -49,6 +49,11 @@ EOT
);
}
+my $pin_map = {
+ '12:34:56:ab:cd:ef' => 'nic0',
+ 'ab:cd:ef:12:34:56' => 'nic1',
+};
+
# global config
$proxmox_install_runenv->redefine(
get => sub {
@@ -123,10 +128,8 @@ EOT
# interface name pinning test
Proxmox::Install::Config::merge({
- network_interface_pin_map => {
- '12:34:56:ab:cd:ef' => 'nic0',
- 'ab:cd:ef:12:34:56' => 'nic1',
- },
+ mngmt_nic => 'eno1',
+ network_interface_pin_map => $pin_map,
});
is_generated(
<<EOT
@@ -194,4 +197,51 @@ EOT
, 'gateway learned from IPv6 router advertisements is not configured statically',
);
+# Check that unpinnable interfaces (e.g. virtual ones) keep their kernel-assigned name
+Proxmox::Install::Config::merge({
+ network_interface_pin_map => $pin_map,
+ cidr => '192.168.0.10/24',
+ gateway => '192.168.0.1',
+});
+set_config(bridge => 0);
+$proxmox_install_runenv->redefine(
+ get => sub {
+ return {
+ network => {
+ interfaces => {
+ eno1 => {
+ mac => '12:34:56:ab:cd:ef',
+ index => 1,
+ name => 'eno1',
+ },
+ eno2 => {
+ mac => 'ab:cd:ef:12:34:56',
+ index => 2,
+ name => 'eno2',
+ },
+ dummy => {
+ mac => 'aa:bb:cc:dd:ee:ff',
+ index => 3,
+ name => 'dummy',
+ },
+ },
+ },
+ };
+ },
+);
+
+is_generated(
+ <<EOT
+auto nic0
+iface nic0 inet static
+ address 192.168.0.10/24
+ gateway 192.168.0.1
+
+iface dummy inet manual
+
+iface nic1 inet manual
+EOT
+ , 'unpinnable names keep their kernel name',
+);
+
done_testing();
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 05/23] install: config: add option for setting up bond on management interface
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (3 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 04/23] install: do not rely on interface name map to be fully populated Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 06/23] install: config: add option for setting up vlan " Christoph Heiss
` (17 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Prerequisite for the rest of the installer to support setting up a bond
as management interface during installations.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* add comments to new Proxmox::Sys::Net subroutines
* fix up line length on comments
* compare MAC addresses case-insensitively (@thanks Thomas)
* allow clearing the primary bond interface (@thanks Thomas)
Changes v1 -> v2:
* no changes
Proxmox/Install/Config.pm | 47 +++++++++++++++++++++++++++++++++++++++
Proxmox/Sys/Net.pm | 34 ++++++++++++++++++++++++++++
2 files changed, 81 insertions(+)
diff --git a/Proxmox/Install/Config.pm b/Proxmox/Install/Config.pm
index dba3692..967712a 100644
--- a/Proxmox/Install/Config.pm
+++ b/Proxmox/Install/Config.pm
@@ -103,6 +103,21 @@ my sub init_cfg {
# network related
mngmt_nic => undef,
+ # Options for creating a bond for the management interface with the given physical
+ # interfaces and mode.
+ mngmt_bond => {
+ # MAC addresses of member interfaces. If empty, bonding is disabled.
+ members => [],
+ # must be one of: 'balance-rr', 'active-backup', 'balance-xor', 'broadcast',
+ # '802.3ad', 'balance-tlb', 'balance-alb'
+ mode => 'active-backup', # recommended by our docs
+ # hash policy, only used with '802.3ad' and 'balance-xor' mode
+ # must be one of 'layer2', 'layer2+3', 'layer3+4'
+ hash_policy => 'layer2',
+ # MAC address of primary bond interface, only used with 'active-backup' mode, optional
+ primary_interface => undef,
+ },
+
# maps mac address -> custom name
# if set to a hash, enables interface name pinning for all interfaces
network_interface_pin_map => undef,
@@ -257,6 +272,38 @@ sub get_subscription_key { return get('subscription_key'); }
sub set_mngmt_nic { set_key('mngmt_nic', $_[0]); }
sub get_mngmt_nic { return get('mngmt_nic'); }
+sub set_mngmt_bond_opt {
+ my ($k, $v) = @_;
+ my $opts = get('mngmt_bond');
+
+ croak "unknown management bond option key '$k'" if !exists($opts->{$k});
+
+ die "unknown bond mode '$v'\n"
+ if $k eq 'mode' && !Proxmox::Sys::Net::is_valid_bond_mode($v);
+ die "unknown bond hash policy '$v'\n"
+ if $k eq 'hash_policy' && !Proxmox::Sys::Net::is_valid_bond_hash_policy($v);
+
+ # allow setting no interfaces (disabling bonding), or 2+ for enabling it
+ die "at least 2 member interfaces are required for bond\n"
+ if $k eq 'members' && scalar($v->@*) == 1;
+ die "primary interface '$v' is not a member of the bond\n"
+ if $k eq 'primary_interface'
+ && defined($v)
+ && !grep { lc($_) eq lc($v) } $opts->{members}->@*;
+
+ $opts->{$k} = $v;
+}
+
+sub get_mngmt_bond_opt {
+ my ($k) = @_;
+ my $opts = get('mngmt_bond');
+ return defined($k) ? $opts->{$k} : $opts;
+}
+
+sub get_mngmt_bond_enabled { # virtual config
+ return scalar(get_mngmt_bond_opt('members')->@*) >= 2;
+}
+
sub set_network_interface_pin_map { set_key('network_interface_pin_map', $_[0]); }
sub get_network_interface_pin_map { return get('network_interface_pin_map'); }
diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
index c43311d..630d96a 100644
--- a/Proxmox/Sys/Net.pm
+++ b/Proxmox/Sys/Net.pm
@@ -15,6 +15,8 @@ our @EXPORT_OK = qw(
parse_ip_mask
parse_fqdn
validate_link_pin_map
+ is_valid_bond_mode
+ is_valid_bond_hash_policy
MIN_IFNAME_LEN
MAX_IFNAME_LEN
DEFAULT_PIN_PREFIX
@@ -450,4 +452,36 @@ sub validate_link_pin_map : prototype($) {
}
}
+# Checks whether the given string is a valid network bond mode, as accepted by
+# ifupdown2.
+sub is_valid_bond_mode {
+ my ($mode) = @_;
+
+ my $valid = {
+ 'active-backup' => 1,
+ 'balance-alb' => 1,
+ 'balance-rr' => 1,
+ 'balance-tlb' => 1,
+ 'balance-xor' => 1,
+ 'broadcast' => 1,
+ '802.3ad' => 1,
+ };
+
+ return exists($valid->{$mode});
+}
+
+# Checks whether the given string is a valid network bond hash policy, as
+# accepted by ifupdown2.
+sub is_valid_bond_hash_policy {
+ my ($policy) = @_;
+
+ my $valid = {
+ layer2 => 1,
+ 'layer2+3' => 1,
+ 'layer3+4' => 1,
+ };
+
+ return exists($valid->{$policy});
+}
+
1;
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 06/23] install: config: add option for setting up vlan on management interface
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (4 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 05/23] install: config: add option for setting up bond on management interface Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 07/23] install: network: set up management bond if requested Christoph Heiss
` (16 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Prerequisite for the rest of the installer to support setting up a VLAN
interface for management during installation.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* new patch
Proxmox/Install/Config.pm | 32 ++++++++++++++++++++++++++++++++
Proxmox/Sys/Net.pm | 9 +++++++++
2 files changed, 41 insertions(+)
diff --git a/Proxmox/Install/Config.pm b/Proxmox/Install/Config.pm
index 967712a..3df66c2 100644
--- a/Proxmox/Install/Config.pm
+++ b/Proxmox/Install/Config.pm
@@ -118,6 +118,13 @@ my sub init_cfg {
primary_interface => undef,
},
+ # Options for setting up a VLAN on the management interface.
+ mngmt_vlan => {
+ enabled => 0,
+ # VLAN ID to configure.
+ vid => 0,
+ },
+
# maps mac address -> custom name
# if set to a hash, enables interface name pinning for all interfaces
network_interface_pin_map => undef,
@@ -304,6 +311,31 @@ sub get_mngmt_bond_enabled { # virtual config
return scalar(get_mngmt_bond_opt('members')->@*) >= 2;
}
+sub set_mngmt_vlan_opt {
+ my ($k, $v) = @_;
+ my $opts = get('mngmt_vlan');
+
+ croak "unknown management vlan option key '$k'\n" if !exists($opts->{$k});
+
+ if ($k eq 'vid') {
+ die "invalid VLAN tag '$v'\n"
+ if !Proxmox::Sys::Net::is_valid_vlan_id($v);
+ $opts->{$k} = int($v);
+ } else {
+ $opts->{$k} = $v;
+ }
+}
+
+sub get_mngmt_vlan_opt {
+ my ($k) = @_;
+ my $opts = get('mngmt_vlan');
+ return defined($k) ? $opts->{$k} : $opts;
+}
+
+sub get_mngmt_vlan_enabled {
+ return get_mngmt_vlan_opt('enabled') != 0;
+}
+
sub set_network_interface_pin_map { set_key('network_interface_pin_map', $_[0]); }
sub get_network_interface_pin_map { return get('network_interface_pin_map'); }
diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
index 630d96a..485c9e9 100644
--- a/Proxmox/Sys/Net.pm
+++ b/Proxmox/Sys/Net.pm
@@ -17,6 +17,7 @@ our @EXPORT_OK = qw(
validate_link_pin_map
is_valid_bond_mode
is_valid_bond_hash_policy
+ is_valid_vlan_id
MIN_IFNAME_LEN
MAX_IFNAME_LEN
DEFAULT_PIN_PREFIX
@@ -484,4 +485,12 @@ sub is_valid_bond_hash_policy {
return exists($valid->{$policy});
}
+# Returns whether the given ID is a valid VLAN ID.
+# 0 and 4095 are reserved, and 1 is the default PVID of VLAN-aware bridges on Linux.
+sub is_valid_vlan_id {
+ my ($vid) = @_;
+
+ return defined($vid) && $vid =~ m/^[0-9]+$/ && $vid >= 2 && $vid <= 4094;
+}
+
1;
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 07/23] install: network: set up management bond if requested
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (5 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 06/23] install: config: add option for setting up vlan " Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 08/23] install: network: set up vlan on management interface " Christoph Heiss
` (15 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Adds the necessary configuration to /etc/network/interfaces when bonding
is enabled in the installation config.
A bond named 'bond0' will be created, with the default bridge (as
before) 'vmbr0' on top of that. Without bridged networking, the address
is configured on the bond directly.
The bond options are written out in the same order and with the same
defaults the Proxmox VE network API uses, so that a later edit through
the web interface does not cause unrelated changes.
For the same reason, the members are sorted by name and marked auto.
On Proxmox VE, their order also matters on its own: ifupdown2 enslaves
them in the configured order and, as Proxmox VE disables the persistent
MAC address policy of systemd, the kernel assigns the MAC address of
the first one to the bond. If the first save through the web interface
reordered them, the bond would come up with a different MAC address on
the next boot, which networks filtering on MAC addresses may block.
Originally-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* use suggested patch from @Thomas, adapted tests
Changes v1 -> v2:
* no changes
Proxmox/Install.pm | 102 +++++++++++++--
test/generate-ifupdown2-config.pl | 207 +++++++++++++++++++++++++++---
2 files changed, 280 insertions(+), 29 deletions(-)
diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index 6c7af3d..9e167d8 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -819,35 +819,107 @@ sub generate_ifupdown2_config {
my $ifaces = "auto lo\niface lo inet loopback\n\n";
+ my $get_link_name = sub {
+ my ($mac) = @_;
+ $mac = lc($mac);
+
+ return $netif_pin_map->{$mac}
+ if defined($netif_pin_map) && defined($netif_pin_map->{$mac});
+
+ my ($if) = grep { $_->{mac} eq $mac } values $interfaces->%*;
+ die "no network interface with MAC address '$mac' found for the bond\n"
+ if !defined($if);
+ return $if->{name};
+ };
+
+ # the physical links carrying the management network
+ my @mngmt_links = ($ethdev);
+
+ my $bond_enabled = Proxmox::Install::Config::get_mngmt_bond_enabled();
+ my $bond_config = '';
+ if ($bond_enabled) {
+ my $bond_opts = Proxmox::Install::Config::get_mngmt_bond_opt();
+ my $mode = $bond_opts->{mode};
+
+ # When setting up bridged networking, the bond is attached to the main bridge as port.
+ # Otherwise, the bond represents the main management interface, i.e. with the address
+ # configured directly on it.
+
+ $ethdev = 'bond0';
+ my $addr_method = $iso_env->{cfg}->{bridged_network} ? 'manual' : 'static';
+
+ # translate all bond members MACs to their final interface name, either pinned or not
+ my @members;
+ for my $mac ($bond_opts->{members}->@*) {
+ $mac = lc($mac); # normalize into lowercase
+
+ if (defined($netif_pin_map) && defined($netif_pin_map->{$mac})) {
+ push @members, $netif_pin_map->{$mac};
+ } else {
+ my ($if) = grep { $_->{mac} eq $mac } values %$interfaces;
+ push @members, $if->{name};
+ }
+ }
+
+ # sorted as Proxmox VE saves them, where the first one enslaved sets the bond's MAC address
+ @mngmt_links = sort map { $get_link_name->($_) } $bond_opts->{members}->@*;
+
+ # same order and defaults as the Proxmox VE network API writes them
+ $bond_config = "\tbond-slaves " . join(' ', @mngmt_links) . "\n";
+ $bond_config .= "\tbond-miimon 100\n";
+ $bond_config .= "\tbond-mode $mode\n";
+ $bond_config .= "\tbond-xmit-hash-policy $bond_opts->{hash_policy}\n"
+ if $mode eq '802.3ad' || $mode eq 'balance-xor';
+ $bond_config .= "\tbond-primary " . $get_link_name->($bond_opts->{primary_interface}) . "\n"
+ if $mode eq 'active-backup' && defined($bond_opts->{primary_interface});
+ }
+
# iproute2 omits the gateway and protocol keys for some routes; route devices are original
- # kernel interface names, while $ethdev can be the pinned one
+ # kernel interface names, while the management links can be pinned ones
+ my %is_mngmt_link = map { $_ => 1 } @mngmt_links;
+
my $gateways6 = $run_env->{network}->{routes}->{gateways6} // [];
my $is_gateway6_from_ra = grep {
my $dev = $_->{dev} // '';
if (defined($netif_pin_map) && defined($interfaces->{$dev})) {
$dev = $netif_pin_map->{ $interfaces->{$dev}->{mac} } // $dev;
}
- $dev eq $ethdev
+ $is_mngmt_link{$dev}
&& ($_->{gateway} // '') eq $gateway
&& ($_->{protocol} // '') eq 'ra';
} $gateways6->@*;
if ($iso_env->{cfg}->{bridged_network}) {
- $ifaces .= "iface $ethdev $ntype manual\n";
+ if ($bond_enabled) {
+ $ifaces .= "auto $ethdev\n" #
+ . "iface $ethdev $ntype manual\n" #
+ . $bond_config;
+ } else {
+ $ifaces .= "iface $ethdev $ntype manual\n";
+ }
- $ifaces .=
- "\nauto vmbr0\niface vmbr0 $ntype static\n"
- . "\tbridge-ports $ethdev\n"
- . "\tbridge-stp off\n"
+ $ifaces .= "\nauto vmbr0\niface vmbr0 $ntype static\n";
+
+ $ifaces .= "\taddress $cidr\n";
+ if (!$is_gateway6_from_ra) {
+ $ifaces .= "\tgateway $gateway\n";
+ }
+
+ $ifaces .= "\tbridge-ports $ethdev\n" #
+ . "\tbridge-stp off\n" #
. "\tbridge-fd 0\n";
} else {
- $ifaces .= "auto $ethdev\n" . #
- "iface $ethdev $ntype static\n";
- }
+ # if bonding is enabled, this becomes the bond interface stanza
+ # otherwise, it's the physical interface stanza
+ $ifaces .= "auto $ethdev\n" #
+ . "iface $ethdev $ntype static\n";
- $ifaces .= "\taddress $cidr\n";
- if (!$is_gateway6_from_ra) {
- $ifaces .= "\tgateway $gateway\n";
+ $ifaces .= "\taddress $cidr\n";
+ if (!$is_gateway6_from_ra) {
+ $ifaces .= "\tgateway $gateway\n";
+ }
+
+ $ifaces .= $bond_config;
}
foreach my $iface (sort keys $interfaces->%*) {
@@ -860,7 +932,9 @@ sub generate_ifupdown2_config {
next if $name eq $ethdev;
- $ifaces .= "\niface $name $ntype manual\n";
+ # as Proxmox VE writes them, ifupdown2 then also keeps them up if removed from the bond
+ my $auto = $bond_enabled && $is_mngmt_link{$name} ? "auto $name\n" : '';
+ $ifaces .= "\n${auto}iface $name $ntype manual\n";
}
$ifaces .= "\n\nsource /etc/network/interfaces.d/*\n";
diff --git a/test/generate-ifupdown2-config.pl b/test/generate-ifupdown2-config.pl
index 7f92e4f..f40689c 100755
--- a/test/generate-ifupdown2-config.pl
+++ b/test/generate-ifupdown2-config.pl
@@ -31,6 +31,15 @@ my sub set_config {
};
},
);
+
+ Proxmox::Install::Config::merge({
+ mngmt_nic => $cfg{bond} ? 'bond0' : 'eno1',
+ mngmt_bond => {
+ members => $cfg{bond} ? ['12:34:56:ab:cd:ef', 'ab:cd:ef:12:34:56'] : [],
+ mode => 'active-backup',
+ hash_policy => 'layer2',
+ },
+ });
}
my sub is_generated {
@@ -45,7 +54,7 @@ $expected
source /etc/network/interfaces.d/*
EOT
- "$msg (bridge:$current_cfg->{bridge})",
+ "$msg (bridge:$current_cfg->{bridge} bond:$current_cfg->{bond})",
);
}
@@ -75,9 +84,8 @@ $proxmox_install_runenv->redefine(
};
},
);
-set_config(bridge => 0);
+set_config(bridge => 0, bond => 0);
Proxmox::Install::Config::merge({
- mngmt_nic => 'eno1',
cidr => '192.168.0.10/24',
gateway => '192.168.0.1',
});
@@ -85,16 +93,18 @@ Proxmox::Install::Config::merge({
###
# There's essentially a matrix to test w.r.t the overall networking setup:
#
-# +-------+--------+
-# | test# | bridge |
-# +=======+========+
-# | 1 | 0 |
-# | 2 | 1 |
-# +-------+--------+
+# +-------+--------+------+
+# | test# | bridge | bond |
+# +=======+========+======+
+# | 1 | 0 | 0 |
+# | 2 | 0 | 1 |
+# | 3 | 1 | 0 |
+# | 4 | 1 | 1 |
+# +-------+--------+------+
###
# 1.
-set_config(bridge => 0);
+set_config(bridge => 0, bond => 0);
is_generated(
<<EOT
auto eno1
@@ -108,19 +118,67 @@ EOT
);
# 2.
-set_config(bridge => 1);
+set_config(bridge => 0, bond => 1);
+is_generated(
+ <<EOT
+auto bond0
+iface bond0 inet static
+ address 192.168.0.10/24
+ gateway 192.168.0.1
+ bond-slaves eno1 eno2
+ bond-miimon 100
+ bond-mode active-backup
+
+auto eno1
+iface eno1 inet manual
+
+auto eno2
+iface eno2 inet manual
+EOT
+ , 'host network setup',
+);
+
+# 3.
+set_config(bridge => 1, bond => 0);
is_generated(
<<EOT
iface eno1 inet manual
auto vmbr0
iface vmbr0 inet static
+ address 192.168.0.10/24
+ gateway 192.168.0.1
bridge-ports eno1
bridge-stp off
bridge-fd 0
+
+iface eno2 inet manual
+EOT
+ , 'host network setup',
+);
+
+# 4.
+set_config(bridge => 1, bond => 1);
+is_generated(
+ <<EOT
+auto bond0
+iface bond0 inet manual
+ bond-slaves eno1 eno2
+ bond-miimon 100
+ bond-mode active-backup
+
+auto vmbr0
+iface vmbr0 inet static
address 192.168.0.10/24
gateway 192.168.0.1
+ bridge-ports bond0
+ bridge-stp off
+ bridge-fd 0
+auto eno1
+iface eno1 inet manual
+
+auto eno2
iface eno2 inet manual
EOT
, 'host network setup',
@@ -131,17 +189,18 @@ Proxmox::Install::Config::merge({
mngmt_nic => 'eno1',
network_interface_pin_map => $pin_map,
});
+set_config(bridge => 1, bond => 0);
is_generated(
<<EOT
iface nic0 inet manual
auto vmbr0
iface vmbr0 inet static
+ address 192.168.0.10/24
+ gateway 192.168.0.1
bridge-ports nic0
bridge-stp off
bridge-fd 0
- address 192.168.0.10/24
- gateway 192.168.0.1
iface nic1 inet manual
EOT
@@ -180,6 +239,7 @@ $proxmox_install_runenv->redefine(
};
},
);
+set_config(bridge => 1, bond => 0);
is_generated(
<<EOT
@@ -187,23 +247,49 @@ iface eno1 inet6 manual
auto vmbr0
iface vmbr0 inet6 static
+ address 2001:db8::1/64
bridge-ports eno1
bridge-stp off
bridge-fd 0
- address 2001:db8::1/64
iface eno2 inet6 manual
EOT
, 'gateway learned from IPv6 router advertisements is not configured statically',
);
+# A gateway learned via IPv6 router advertisements is not written out for a bond
+Proxmox::Install::Config::merge({
+ mngmt_nic => 'bond0',
+ cidr => '2001:db8::1/64',
+ gateway => 'fe80::1',
+});
+set_config(bridge => 0, bond => 1);
+
+is_generated(
+ <<EOT
+auto bond0
+iface bond0 inet6 static
+ address 2001:db8::1/64
+ bond-slaves eno1 eno2
+ bond-miimon 100
+ bond-mode active-backup
+
+auto eno1
+iface eno1 inet6 manual
+
+auto eno2
+iface eno2 inet6 manual
+EOT
+ , 'gateway learned from IPv6 router advertisements is not configured statically',
+);
+
# Check that unpinnable interfaces (e.g. virtual ones) keep their kernel-assigned name
Proxmox::Install::Config::merge({
network_interface_pin_map => $pin_map,
cidr => '192.168.0.10/24',
gateway => '192.168.0.1',
});
-set_config(bridge => 0);
+set_config(bridge => 0, bond => 0);
$proxmox_install_runenv->redefine(
get => sub {
return {
@@ -244,4 +330,95 @@ EOT
, 'unpinnable names keep their kernel name',
);
+set_config(bridge => 1, bond => 1);
+Proxmox::Install::Config::merge({
+ mngmt_nic => 'bond0',
+ mngmt_bond => {
+ members => ['12:34:56:ab:cd:ef', 'ab:cd:ef:12:34:56'],
+ mode => 'active-backup',
+ hash_policy => 'layer2',
+ primary_interface => 'ab:cd:ef:12:34:56',
+ },
+});
+is_generated(
+ <<EOT
+auto bond0
+iface bond0 inet manual
+ bond-slaves nic0 nic1
+ bond-miimon 100
+ bond-mode active-backup
+ bond-primary nic1
+
+auto vmbr0
+iface vmbr0 inet static
+ address 192.168.0.10/24
+ gateway 192.168.0.1
+ bridge-ports bond0
+ bridge-stp off
+ bridge-fd 0
+
+iface dummy inet manual
+
+auto nic0
+iface nic0 inet manual
+
+auto nic1
+iface nic1 inet manual
+EOT
+ , 'active-backup bond with interface pinning enabled and primary interface set',
+);
+
+Proxmox::Install::Config::merge({
+ network_interface_pin_map => undef,
+ mngmt_bond => {
+ members => ['12:34:56:ab:cd:ef', 'ab:cd:ef:12:34:56'],
+ mode => '802.3ad',
+ hash_policy => 'layer3+4',
+ # will be ignored on 802.3ad
+ primary_interface => 'ab:cd:ef:12:34:56',
+ },
+});
+is_generated(
+ <<EOT
+auto bond0
+iface bond0 inet manual
+ bond-slaves eno1 eno2
+ bond-miimon 100
+ bond-mode 802.3ad
+ bond-xmit-hash-policy layer3+4
+
+auto vmbr0
+iface vmbr0 inet static
+ address 192.168.0.10/24
+ gateway 192.168.0.1
+ bridge-ports bond0
+ bridge-stp off
+ bridge-fd 0
+
+iface dummy inet manual
+
+auto eno1
+iface eno1 inet manual
+
+auto eno2
+iface eno2 inet manual
+EOT
+ , 'LACP bond enabled, with hash policy and no primary interface',
+);
+
+Proxmox::Install::Config::merge({
+ mngmt_bond => {
+ members => ['12:34:56:ab:cd:ff', 'ab:cd:ef:12:34:56'],
+ mode => 'active-backup',
+ hash_policy => 'layer2',
+ primary_interface => undef,
+ },
+});
+eval { is_generated(); };
+is(
+ $@,
+ "no network interface with MAC address '12:34:56:ab:cd:ff' found for the bond\n",
+ 'unknown bond member is an error',
+);
+
done_testing();
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 08/23] install: network: set up vlan on management interface if requested
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (6 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 07/23] install: network: set up management bond if requested Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 09/23] gui: network: factor out name pinning into advanced options dialog Christoph Heiss
` (14 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Adds the necessary configuration to /etc/network/interfaces when a VLAN
ID was configured by the user in the installation config.
Depending on the rest of the network configuration (bridged, bonded), a
separated (dotted) VLAN interface will be created and integrated in the
rest of the configuration as needed.
This follows our recommended approach for setting up such a
network configuration.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Had a discussion with @Stefan about this, and this way of configuring
this was the best / most user-friendly we plotted out.
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* new patch
Proxmox/Install.pm | 45 +++++++--
test/generate-ifupdown2-config.pl | 159 ++++++++++++++++++++++++++----
2 files changed, 175 insertions(+), 29 deletions(-)
diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index 9e167d8..b39df6f 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -889,14 +889,40 @@ sub generate_ifupdown2_config {
&& ($_->{protocol} // '') eq 'ra';
} $gateways6->@*;
- if ($iso_env->{cfg}->{bridged_network}) {
- if ($bond_enabled) {
- $ifaces .= "auto $ethdev\n" #
- . "iface $ethdev $ntype manual\n" #
- . $bond_config;
- } else {
- $ifaces .= "iface $ethdev $ntype manual\n";
- }
+ my $vlan_enabled = Proxmox::Install::Config::get_mngmt_vlan_enabled();
+ my $vlan_id = Proxmox::Install::Config::get_mngmt_vlan_opt('vid');
+
+ # the live environment is not on the management VLAN, so its routes say nothing about it
+ $is_gateway6_from_ra = 0 if $vlan_enabled;
+
+ my $link_config =
+ $bond_enabled
+ ? "auto $ethdev\niface $ethdev $ntype manual\n$bond_config"
+ : "iface $ethdev $ntype manual\n";
+
+ my $get_vlan_config = sub {
+ my ($raw_device) = @_;
+
+ return
+ "\nauto $raw_device.$vlan_id\niface $raw_device.$vlan_id $ntype static\n"
+ . "\taddress $cidr\n"
+ . "\tgateway $gateway\n";
+ };
+
+ if ($iso_env->{cfg}->{bridged_network} && $vlan_enabled) {
+ $ifaces .= $link_config;
+
+ $ifaces .=
+ "\nauto vmbr0\niface vmbr0 $ntype manual\n"
+ . "\tbridge-ports $ethdev\n"
+ . "\tbridge-stp off\n"
+ . "\tbridge-fd 0\n"
+ . "\tbridge-vlan-aware yes\n"
+ . "\tbridge-vids 2-4094\n";
+
+ $ifaces .= $get_vlan_config->('vmbr0');
+ } elsif ($iso_env->{cfg}->{bridged_network}) {
+ $ifaces .= $link_config;
$ifaces .= "\nauto vmbr0\niface vmbr0 $ntype static\n";
@@ -908,6 +934,9 @@ sub generate_ifupdown2_config {
$ifaces .= "\tbridge-ports $ethdev\n" #
. "\tbridge-stp off\n" #
. "\tbridge-fd 0\n";
+ } elsif ($vlan_enabled) {
+ $ifaces .= $bond_enabled ? $link_config : "auto $ethdev\n$link_config";
+ $ifaces .= $get_vlan_config->($ethdev);
} else {
# if bonding is enabled, this becomes the bond interface stanza
# otherwise, it's the physical interface stanza
diff --git a/test/generate-ifupdown2-config.pl b/test/generate-ifupdown2-config.pl
index f40689c..9ac9b3d 100755
--- a/test/generate-ifupdown2-config.pl
+++ b/test/generate-ifupdown2-config.pl
@@ -34,6 +34,10 @@ my sub set_config {
Proxmox::Install::Config::merge({
mngmt_nic => $cfg{bond} ? 'bond0' : 'eno1',
+ mngmt_vlan => {
+ enabled => $cfg{vlan},
+ vid => $cfg{vlan} ? 5 : 0,
+ },
mngmt_bond => {
members => $cfg{bond} ? ['12:34:56:ab:cd:ef', 'ab:cd:ef:12:34:56'] : [],
mode => 'active-backup',
@@ -54,7 +58,7 @@ $expected
source /etc/network/interfaces.d/*
EOT
- "$msg (bridge:$current_cfg->{bridge} bond:$current_cfg->{bond})",
+ "$msg (bridge:$current_cfg->{bridge} bond:$current_cfg->{bond} vlan:$current_cfg->{vlan})",
);
}
@@ -84,7 +88,7 @@ $proxmox_install_runenv->redefine(
};
},
);
-set_config(bridge => 0, bond => 0);
+set_config(bridge => 0, bond => 0, vlan => 0);
Proxmox::Install::Config::merge({
cidr => '192.168.0.10/24',
gateway => '192.168.0.1',
@@ -93,18 +97,22 @@ Proxmox::Install::Config::merge({
###
# There's essentially a matrix to test w.r.t the overall networking setup:
#
-# +-------+--------+------+
-# | test# | bridge | bond |
-# +=======+========+======+
-# | 1 | 0 | 0 |
-# | 2 | 0 | 1 |
-# | 3 | 1 | 0 |
-# | 4 | 1 | 1 |
-# +-------+--------+------+
+# +-------+--------+------+------+
+# | test# | bridge | bond | vlan |
+# +=======+========+======+======+
+# | 1 | 0 | 0 | 0 |
+# | 2 | 0 | 0 | 1 |
+# | 3 | 0 | 1 | 0 |
+# | 4 | 0 | 1 | 1 |
+# | 5 | 1 | 0 | 0 |
+# | 6 | 1 | 0 | 1 |
+# | 7 | 1 | 1 | 0 |
+# | 8 | 1 | 1 | 1 |
+# +-------+--------+------+------+
###
# 1.
-set_config(bridge => 0, bond => 0);
+set_config(bridge => 0, bond => 0, vlan => 0);
is_generated(
<<EOT
auto eno1
@@ -118,7 +126,24 @@ EOT
);
# 2.
-set_config(bridge => 0, bond => 1);
+set_config(bridge => 0, bond => 0, vlan => 1);
+is_generated(
+ <<EOT
+auto eno1
+iface eno1 inet manual
+
+auto eno1.5
+iface eno1.5 inet static
+ address 192.168.0.10/24
+ gateway 192.168.0.1
+
+iface eno2 inet manual
+EOT
+ , 'host network setup',
+);
+
+# 3.
+set_config(bridge => 0, bond => 1, vlan => 0);
is_generated(
<<EOT
auto bond0
@@ -138,8 +163,32 @@ EOT
, 'host network setup',
);
-# 3.
-set_config(bridge => 1, bond => 0);
+# 4.
+set_config(bridge => 0, bond => 1, vlan => 1);
+is_generated(
+ <<EOT
+auto bond0
+iface bond0 inet manual
+ bond-slaves eno1 eno2
+ bond-miimon 100
+ bond-mode active-backup
+
+auto bond0.5
+iface bond0.5 inet static
+ address 192.168.0.10/24
+ gateway 192.168.0.1
+
+auto eno1
+iface eno1 inet manual
+
+auto eno2
+iface eno2 inet manual
+EOT
+ , 'host network setup',
+);
+
+# 5.
+set_config(bridge => 1, bond => 0, vlan => 0);
is_generated(
<<EOT
iface eno1 inet manual
@@ -157,8 +206,32 @@ EOT
, 'host network setup',
);
-# 4.
-set_config(bridge => 1, bond => 1);
+# 6.
+set_config(bridge => 1, bond => 0, vlan => 1);
+is_generated(
+ <<EOT
+iface eno1 inet manual
+
+auto vmbr0
+iface vmbr0 inet manual
+ bridge-ports eno1
+ bridge-stp off
+ bridge-fd 0
+ bridge-vlan-aware yes
+ bridge-vids 2-4094
+
+auto vmbr0.5
+iface vmbr0.5 inet static
+ address 192.168.0.10/24
+ gateway 192.168.0.1
+
+iface eno2 inet manual
+EOT
+ , 'host network setup',
+);
+
+# 7.
+set_config(bridge => 1, bond => 1, vlan => 0);
is_generated(
<<EOT
auto bond0
@@ -184,12 +257,44 @@ EOT
, 'host network setup',
);
+# 8.
+set_config(bridge => 1, bond => 1, vlan => 1);
+is_generated(
+ <<EOT
+auto bond0
+iface bond0 inet manual
+ bond-slaves eno1 eno2
+ bond-miimon 100
+ bond-mode active-backup
+
+auto vmbr0
+iface vmbr0 inet manual
+ bridge-ports bond0
+ bridge-stp off
+ bridge-fd 0
+ bridge-vlan-aware yes
+ bridge-vids 2-4094
+
+auto vmbr0.5
+iface vmbr0.5 inet static
+ address 192.168.0.10/24
+ gateway 192.168.0.1
+
+auto eno1
+iface eno1 inet manual
+
+auto eno2
+iface eno2 inet manual
+EOT
+ , 'host network setup',
+);
+
# interface name pinning test
Proxmox::Install::Config::merge({
mngmt_nic => 'eno1',
network_interface_pin_map => $pin_map,
});
-set_config(bridge => 1, bond => 0);
+set_config(bridge => 1, bond => 0, vlan => 0);
is_generated(
<<EOT
iface nic0 inet manual
@@ -239,7 +344,7 @@ $proxmox_install_runenv->redefine(
};
},
);
-set_config(bridge => 1, bond => 0);
+set_config(bridge => 1, bond => 0, vlan => 0);
is_generated(
<<EOT
@@ -263,7 +368,7 @@ Proxmox::Install::Config::merge({
cidr => '2001:db8::1/64',
gateway => 'fe80::1',
});
-set_config(bridge => 0, bond => 1);
+set_config(bridge => 0, bond => 1, vlan => 0);
is_generated(
<<EOT
@@ -289,7 +394,7 @@ Proxmox::Install::Config::merge({
cidr => '192.168.0.10/24',
gateway => '192.168.0.1',
});
-set_config(bridge => 0, bond => 0);
+set_config(bridge => 0, bond => 0, vlan => 0);
$proxmox_install_runenv->redefine(
get => sub {
return {
@@ -330,7 +435,7 @@ EOT
, 'unpinnable names keep their kernel name',
);
-set_config(bridge => 1, bond => 1);
+set_config(bridge => 1, bond => 1, vlan => 0);
Proxmox::Install::Config::merge({
mngmt_nic => 'bond0',
mngmt_bond => {
@@ -421,4 +526,16 @@ is(
'unknown bond member is an error',
);
+for my $id (0, 1, 4095, 'abc', '1.5') {
+ eval { Proxmox::Install::Config::set_mngmt_vlan_opt('vid', $id) };
+ like($@, qr/^invalid VLAN tag '\Q$id\E'/, "VLAN tag '$id' is rejected");
+}
+
+Proxmox::Install::Config::set_mngmt_vlan_opt('vid', '0005');
+is(
+ Proxmox::Install::Config::get_mngmt_vlan_opt('vid'),
+ 5,
+ 'VLAN tag with leading zeros is normalized',
+);
+
done_testing();
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 09/23] gui: network: factor out name pinning into advanced options dialog
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (7 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 08/23] install: network: set up vlan on management interface " Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 10/23] gui: network: add bond setup options to " Christoph Heiss
` (13 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
This dialog can now be easily extended with additional frames for other,
advanced network-related options.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* don't expand checkbox and grid, which resulted in unnecessary padding
and dialog size
Changes v2 -> v3:
* moved shuffling around of create_label_widget_grid() definition to
next commit (thanks Lukas S.!)
Changes v1 -> v2:
* no changes
proxinstall | 125 ++++++++++++++++++++++++++++++++++------------------
1 file changed, 81 insertions(+), 44 deletions(-)
diff --git a/proxinstall b/proxinstall
index 51efe79..9c1e5ac 100755
--- a/proxinstall
+++ b/proxinstall
@@ -349,34 +349,26 @@ my $create_basic_grid = sub {
return $grid;
};
-my sub create_network_interface_pin_view {
- my ($done_cb) = @_;
+my sub create_network_interface_pin_frame {
+ my ($on_enable_changed) = @_;
- my $dialog = Gtk3::Dialog->new();
- $dialog->set_title('Interface Name Pinning Options');
- $dialog->add_button('_OK', 'ok');
+ my $frame = Gtk3::Frame->new('Interface Name Pinning');
- my $content = $dialog->get_content_area();
+ my $vbox = Gtk3::Box->new('vertical', 5);
+ $frame->add($vbox);
+ $vbox->set_margin_start(10);
+ $vbox->set_margin_end(10);
+ $vbox->set_margin_top(5);
+ $vbox->set_margin_bottom(5);
- my $hbox = Gtk3::Box->new('horizontal', 0);
- $content->pack_start($hbox, 1, 1, 5);
+ my $enable_checkbox = Gtk3::CheckButton->new('Pin network interface names');
+ $enable_checkbox->set_active($gtk_state->{network_pinning_enabled});
+ $vbox->pack_start($enable_checkbox, 1, 0, 5);
my $grid = Gtk3::Grid->new();
$grid->set_column_spacing(10);
$grid->set_row_spacing(10);
-
- # make the list scrollable, in case there are lots of interfaces
- my $scrolled_window = Gtk3::ScrolledWindow->new();
- $scrolled_window->set_hexpand(1);
- $scrolled_window->set_propagate_natural_height(1);
-
- $scrolled_window->add($grid);
- $scrolled_window->set_policy('never', 'automatic');
- $scrolled_window->set_min_content_height(200);
- $scrolled_window->set_margin_start(10);
- $scrolled_window->set_margin_end(10);
-
- $hbox->pack_start($scrolled_window, 1, 0, 5);
+ $vbox->pack_start($grid, 1, 1, 5);
my $interfaces = Proxmox::Install::RunEnv::get()->{network}->{interfaces};
my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
@@ -412,24 +404,83 @@ my sub create_network_interface_pin_view {
$inputs->{ $iface->{mac} } = $input;
}
- $hbox->show_all();
+ my $set_sensitive = sub {
+ my ($active) = @_;
+ $grid->foreach(sub {
+ $_[0]->set_sensitive($active);
+ });
+ };
+
+ $set_sensitive->($gtk_state->{network_pinning_enabled});
+ $enable_checkbox->signal_connect(
+ toggled => sub {
+ my $active = !!$enable_checkbox->get_active();
+
+ $gtk_state->{network_pinning_enabled} = $active;
+ $set_sensitive->($active);
+
+ $on_enable_changed->($active);
+ },
+ );
+
+ my $on_done = sub {
+ if (!$enable_checkbox->get_active()) {
+ Proxmox::Install::Config::set_network_interface_pin_map();
+ return;
+ }
+
+ my %new_mapping = map { $_ => $inputs->{$_}->get_text() } keys %$inputs;
+
+ validate_link_pin_map(\%new_mapping);
+ Proxmox::Install::Config::set_network_interface_pin_map(\%new_mapping);
+ };
+
+ return ($frame, $on_done);
+}
+
+my sub create_network_advanced_options_view {
+ my ($done_cb) = @_;
+
+ my $dialog = Gtk3::Dialog->new();
+ $dialog->set_title('Management Network Options');
+ $dialog->add_button('_OK', 'ok');
+
+ my $content = $dialog->get_content_area();
+
+ my $scrolled_window = Gtk3::ScrolledWindow->new();
+ $scrolled_window->set_hexpand(1);
+ $scrolled_window->set_vexpand(1);
+ $scrolled_window->set_propagate_natural_height(1);
+ $scrolled_window->set_policy('never', 'automatic');
+ $scrolled_window->set_min_content_height(200);
+
+ my $hbox = Gtk3::Box->new('vertical', 0);
+
+ my ($pin_frame, $pin_on_done) = create_network_interface_pin_frame(sub {});
+
+ $hbox->pack_start($pin_frame, 0, 0, 10);
+
+ $scrolled_window->add($hbox);
+ $content->pack_start($scrolled_window, 1, 1, 10);
+
+ my $inputs = {};
$dialog->signal_connect(
response => sub {
- my %new_mapping = map { $_ => $inputs->{$_}->get_text() } keys %$inputs;
-
- eval { validate_link_pin_map(\%new_mapping); };
+ eval {
+ $pin_on_done->();
+ };
if ($@) {
Proxmox::UI::message($@, $dialog);
return;
}
- Proxmox::Install::Config::set_network_interface_pin_map(\%new_mapping);
$dialog->destroy();
$done_cb->();
},
);
+ $dialog->show_all();
$dialog->run();
}
@@ -522,21 +573,10 @@ sub create_ipconf_view {
$device_cb->set_active($active);
};
- my $name_pin_opts_button = Gtk3::Button->new('Options');
- $name_pin_opts_button->set_sensitive($gtk_state->{network_pinning_enabled});
- $name_pin_opts_button->signal_connect(
+ my $advanced_opts_button = Gtk3::Button->new('Advanced Options');
+ $advanced_opts_button->signal_connect(
clicked => sub {
- create_network_interface_pin_view($refresh_device_cb);
- },
- );
-
- my $name_pin_checkbox = Gtk3::CheckButton->new('Pin network interface names');
- $name_pin_checkbox->set_active($gtk_state->{network_pinning_enabled});
- $name_pin_checkbox->signal_connect(
- toggled => sub {
- $name_pin_opts_button->set_sensitive(!!$name_pin_checkbox->get_active());
- $gtk_state->{network_pinning_enabled} = !!$name_pin_checkbox->get_active();
- $refresh_device_cb->();
+ create_network_advanced_options_view($refresh_device_cb);
},
);
@@ -661,8 +701,7 @@ sub create_ipconf_view {
$grid->attach($dns_label, 0, 4, 1, 1);
$grid->attach($ipconf_entry_dns, 1, 4, 2, 1);
- $grid->attach($name_pin_checkbox, 1, 5, 1, 1);
- $grid->attach($name_pin_opts_button, 2, 5, 1, 1);
+ $grid->attach($advanced_opts_button, 1, 5, 2, 1);
$gtk_state->{inbox}->show_all;
set_next(
@@ -730,8 +769,6 @@ sub create_ipconf_view {
}
Proxmox::Install::Config::set_dns($dns_ip);
- $gtk_state->{network_pinning_enabled} = !!$name_pin_checkbox->get_active();
-
#print STDERR "TEST $ipaddress/$netmask $gateway_ip $dns_ip\n";
$step_number++;
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 10/23] gui: network: add bond setup options to advanced options dialog
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (8 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 09/23] gui: network: factor out name pinning into advanced options dialog Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 11/23] gui: network: add vlan setup options to advanced network dialog Christoph Heiss
` (12 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Partially fixes #2164 [0].
Adds a second frame 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 <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* don't expand checkbox and grid, which resulted in unnecessary padding
and dialog size
Changes v2 -> v3:
* incorporate changes from @Thomas;
* resolve the primary interface name to its MAC address also without
pinning
* clear primary interface if empty or not applicable to the mode
* keep the selected physical management interface instead of
overriding it with 'bond0', so that going back and disabling bonding
works,
* list the interfaces sorted
* only show the bond summary in place of the interface selection if
bonding is enabled
* use the current pinned names for the member labels, only from
physical links, and keep underscores, e.g. in driver names, in them
Changes v1 -> v2:
* fix passing an undefined index to GTK when having a bond setup and
switching between installer steps
proxinstall | 292 +++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 257 insertions(+), 35 deletions(-)
diff --git a/proxinstall b/proxinstall
index 9c1e5ac..a676018 100755
--- a/proxinstall
+++ b/proxinstall
@@ -85,7 +85,9 @@ my @steps = (
);
# GUI global variables
-my $gtk_state = {};
+my $gtk_state = {
+ network_bonding_enabled => 0,
+};
my $target_hds; # only for the summary view
@@ -349,6 +351,35 @@ my $create_basic_grid = sub {
return $grid;
};
+# takes an array ref of rows with [$label_text, $widget, $suffix_label] array refs as columns
+# $suffix_label is optional
+my $create_label_widget_grid = sub {
+ my ($labeled_widgets) = @_;
+
+ my $grid = &$create_basic_grid();
+
+ for (my $row = 0; $row < scalar($labeled_widgets->@*); $row++) {
+ my ($label_text, $widget, $suffix_label) = $labeled_widgets->[$row]->@*;
+
+ my $label = Gtk3::Label->new($label_text);
+ $label->set_visible(1);
+ $label->set_xalign(1.0);
+ $grid->attach($label, 0, $row, 1, 1);
+
+ $widget->set_visible(1);
+ $grid->attach($widget, 1, $row, 1, 1);
+
+ if ($suffix_label) {
+ my $suffix_label = Gtk3::Label->new($suffix_label);
+ $suffix_label->set_visible(1);
+ $suffix_label->set_xalign(1.0);
+ $grid->attach($suffix_label, 2, $row, 1, 1);
+ }
+ }
+
+ return $grid;
+};
+
my sub create_network_interface_pin_frame {
my ($on_enable_changed) = @_;
@@ -439,6 +470,177 @@ my sub create_network_interface_pin_frame {
return ($frame, $on_done);
}
+my sub create_network_bond_frame {
+ my $frame = Gtk3::Frame->new('Management Bond');
+
+ my $vbox = Gtk3::Box->new('vertical', 5);
+ $frame->add($vbox);
+ $vbox->set_margin_start(10);
+ $vbox->set_margin_end(10);
+ $vbox->set_margin_top(5);
+ $vbox->set_margin_bottom(5);
+
+ my $enable_checkbox = Gtk3::CheckButton->new('Enable bonding for management interface');
+ $enable_checkbox->set_active($gtk_state->{network_bonding_enabled});
+ $vbox->pack_start($enable_checkbox, 1, 0, 5);
+
+ my $bond_opts = Proxmox::Install::Config::get_mngmt_bond_opt();
+ my $interfaces = Proxmox::Install::RunEnv::get()->{network}->{interfaces};
+
+ # name the interface will have on the installed system
+ my $get_ifname = sub {
+ my ($iface) = @_;
+
+ return $iface->{name} if !$gtk_state->{network_pinning_enabled};
+
+ my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
+ return $mapping->{ $iface->{mac} } // $iface->{name};
+ };
+
+ my $mode_combo = Gtk3::ComboBoxText->new();
+ my @modes = qw(active-backup balance-alb balance-rr balance-tlb balance-xor broadcast 802.3ad);
+ for my $m (@modes) {
+ $mode_combo->append_text($m);
+ }
+ for my $i (0 .. $#modes) {
+ if ($modes[$i] eq $bond_opts->{mode}) {
+ $mode_combo->set_active($i);
+ last;
+ }
+ }
+
+ my $hash_policy_combo = Gtk3::ComboBoxText->new();
+ my @hash_policies = qw(layer2 layer2+3 layer3+4);
+ for my $p (@hash_policies) {
+ $hash_policy_combo->append_text($p);
+ }
+ for my $i (0 .. $#hash_policies) {
+ if ($hash_policies[$i] eq $bond_opts->{hash_policy}) {
+ $hash_policy_combo->set_active($i);
+ last;
+ }
+ }
+
+ my $primary_if_input = Gtk3::Entry->new();
+ $primary_if_input->set_tooltip_text('Name of the primary interface to set on the bond.');
+
+ if (defined(my $primary_mac = $bond_opts->{primary_interface})) {
+ my ($primary) = grep { lc($_->{mac}) eq lc($primary_mac) } values $interfaces->%*;
+ $primary_if_input->set_text($get_ifname->($primary)) if defined($primary);
+ }
+
+ my $on_mode_changed = sub {
+ my $mode = $mode_combo->get_active_text();
+ $hash_policy_combo->set_sensitive($mode eq '802.3ad' || $mode eq 'balance-xor');
+ $primary_if_input->set_sensitive($mode eq 'active-backup');
+ };
+ $mode_combo->signal_connect(changed => $on_mode_changed);
+
+ my $opts_grid = $create_label_widget_grid->([
+ ['Bond Mode', $mode_combo],
+ ['Hash Policy', $hash_policy_combo],
+ ['Primary Interface', $primary_if_input],
+ ]);
+
+ $vbox->pack_start($opts_grid, 0, 0, 5);
+
+ my $if_label = Gtk3::Label->new('Bond Interfaces:');
+ $if_label->set_xalign(0.0);
+ $if_label->set_margin_top(5);
+ $vbox->pack_start($if_label, 0, 0, 0);
+
+ my $checkboxes = {};
+ for my $ifname (sort keys $interfaces->%*) {
+ my $iface = $interfaces->{$ifname};
+ next if !defined($iface->{pinned_id});
+
+ # without a label, as one passed here would be parsed for mnemonics, eating underscores
+ my $checkbox = Gtk3::CheckButton->new();
+
+ my $is_member = grep { $_ eq $iface->{mac} } $bond_opts->{members}->@*;
+ $checkbox->set_active($is_member);
+
+ $vbox->pack_start($checkbox, 0, 0, 5);
+ $checkboxes->{ $iface->{mac} } = $checkbox;
+ }
+
+ my $set_sensitive = sub {
+ my ($active) = @_;
+
+ $opts_grid->foreach(sub {
+ $_[0]->set_sensitive($active);
+ });
+
+ # if activating all inputs, observe the rules applied by the mode change handler
+ $on_mode_changed->() if $active;
+
+ $if_label->set_sensitive($active);
+ for my $cb (values %$checkboxes) {
+ $cb->set_sensitive($active);
+ }
+ };
+ $set_sensitive->($gtk_state->{network_bonding_enabled});
+
+ $enable_checkbox->signal_connect(
+ toggled => sub {
+ my $active = !!$enable_checkbox->get_active();
+
+ $gtk_state->{network_bonding_enabled} = $active;
+ $set_sensitive->($active);
+ },
+ );
+
+ my $refresh_if_names = sub {
+ for my $iface (values $interfaces->%*) {
+ # virtual links can share the MAC address with a physical one
+ next if !defined($iface->{pinned_id}) || !defined($checkboxes->{ $iface->{mac} });
+
+ my $name = $get_ifname->($iface);
+ my $label = "$name ($iface->{mac}, $iface->{driver}, $iface->{state})";
+ $checkboxes->{ $iface->{mac} }->set_label($label);
+ }
+ };
+ $refresh_if_names->();
+
+ my $on_done = sub {
+ if (!$enable_checkbox->get_active()) {
+ Proxmox::Install::Config::set_mngmt_bond_opt('members', []);
+ return;
+ }
+
+ my @members = grep { $checkboxes->{$_}->get_active() } sort keys $checkboxes->%*;
+ die "at least 2 member interfaces are required for setting up bond\n"
+ if scalar(@members) < 2;
+
+ my $mode = $mode_combo->get_active_text();
+
+ # resolve the name to its MAC address, only applicable to the active-backup mode
+ my $primary_mac;
+ my $primary_name = $primary_if_input->get_text();
+ if ($mode eq 'active-backup' && length($primary_name) > 0) {
+ ($primary_mac) =
+ map { $_->{mac} }
+ grep { $get_ifname->($_) eq $primary_name } values $interfaces->%*;
+
+ die "unknown primary interface '$primary_name'\n" if !defined($primary_mac);
+ die "primary interface '$primary_name' is not a member of the bond\n"
+ if !grep { $_ eq $primary_mac } @members;
+ }
+
+ # unset the primary interface first, it is validated against the new members
+ Proxmox::Install::Config::set_mngmt_bond_opt('primary_interface', undef);
+ Proxmox::Install::Config::set_mngmt_bond_opt('members', \@members);
+ Proxmox::Install::Config::set_mngmt_bond_opt('mode', $mode);
+ Proxmox::Install::Config::set_mngmt_bond_opt(
+ 'hash_policy',
+ $hash_policy_combo->get_active_text(),
+ );
+ Proxmox::Install::Config::set_mngmt_bond_opt('primary_interface', $primary_mac);
+ };
+
+ return ($frame, $refresh_if_names, $on_done);
+}
+
my sub create_network_advanced_options_view {
my ($done_cb) = @_;
@@ -457,9 +659,11 @@ my sub create_network_advanced_options_view {
my $hbox = Gtk3::Box->new('vertical', 0);
- my ($pin_frame, $pin_on_done) = create_network_interface_pin_frame(sub {});
+ my ($bond_frame, $bond_refresh, $bond_on_done) = create_network_bond_frame();
+ my ($pin_frame, $pin_on_done) = create_network_interface_pin_frame($bond_refresh);
$hbox->pack_start($pin_frame, 0, 0, 10);
+ $hbox->pack_start($bond_frame, 0, 0, 10);
$scrolled_window->add($hbox);
$content->pack_start($scrolled_window, 1, 1, 10);
@@ -469,6 +673,7 @@ my sub create_network_advanced_options_view {
response => sub {
eval {
$pin_on_done->();
+ $bond_on_done->();
};
if ($@) {
Proxmox::UI::message($@, $dialog);
@@ -544,7 +749,29 @@ sub create_ipconf_view {
$device_cb->pack_start($cell, 0);
$device_cb->add_attribute($cell, 'text', 1);
+ my $device_bond_label = Gtk3::Label->new('bond0');
+ $device_bond_label->set_justify('center');
+ $device_bond_label->set_margin_top(5);
+ $device_bond_label->set_margin_bottom(5);
+
my $refresh_device_cb = sub {
+ # with bonding, show its summary in place of the interface selection
+ my $bond_active = Proxmox::Install::Config::get_mngmt_bond_enabled();
+ if ($bond_active) {
+ my $bond_opts = Proxmox::Install::Config::get_mngmt_bond_opt();
+
+ $device_bond_label->set_text(
+ "bond0 (mode: $bond_opts->{mode}, "
+ . scalar($bond_opts->{members}->@*)
+ . " members)",
+ );
+ }
+
+ $device_cb->set_visible(!$bond_active);
+ $device_cb->set_no_show_all($bond_active);
+ $device_bond_label->set_visible($bond_active);
+ $device_bond_label->set_no_show_all(!$bond_active);
+
# clear all entries and re-add them with their new names
my $active = $device_cb->get_active();
$device_model->clear();
@@ -682,6 +909,7 @@ sub create_ipconf_view {
$grid->attach($label, 0, 0, 1, 1);
$grid->attach($device_cb, 1, 0, 2, 1);
+ $grid->attach($device_bond_label, 1, 0, 2, 1);
my $fqdn = Proxmox::Install::Config::get_fqdn();
my $hostname = $run_env->{network}->{hostname} || $iso_env->{product};
@@ -805,11 +1033,34 @@ sub create_ack_view {
my $country = Proxmox::Install::Config::get_country();
my $mngmt_nic = Proxmox::Install::Config::get_mngmt_nic();
- my $iface = Proxmox::Install::RunEnv::get('network')->{interfaces}->{$mngmt_nic};
-
+ my $all_ifaces = Proxmox::Install::RunEnv::get('network')->{interfaces};
+ my $iface = $all_ifaces->{$mngmt_nic};
my $nic_mapping = Proxmox::Install::Config::get_network_interface_pin_map();
- my $interface =
- $gtk_state->{network_pinning_enabled} ? $nic_mapping->{ $iface->{mac} } : $iface->{name};
+
+ my $bond_opts = Proxmox::Install::Config::get_mngmt_bond_opt();
+
+ my $interface;
+ if (Proxmox::Install::Config::get_mngmt_bond_enabled()) {
+ my @member_names;
+
+ # translate all bond members MACs to their final interface name, either pinned or not
+ for my $mac ($bond_opts->{members}->@*) {
+ if ($gtk_state->{network_pinning_enabled}) {
+ push @member_names, $nic_mapping->{$mac};
+ } else {
+ my ($if) = grep { $_->{mac} eq $mac } values %$all_ifaces;
+ push @member_names, $if->{name};
+ }
+ }
+
+ my $members = join(' | ', @member_names);
+ $interface = "bond0 (mode: $bond_opts->{mode}, interfaces: $members)";
+ } else {
+ $interface =
+ $gtk_state->{network_pinning_enabled}
+ ? $nic_mapping->{ $iface->{mac} }
+ : $iface->{name};
+ }
my %config_values = (
__target_hd__ => join(' | ', $target_hds->@*),
@@ -1225,35 +1476,6 @@ my $target_hd_label;
my $hdoption_first_setup = 1;
-# takes an array ref of rows with [$label_text, $widget, $suffix_label] array refs as columns
-# $suffix_label is optional
-my $create_label_widget_grid = sub {
- my ($labeled_widgets) = @_;
-
- my $grid = &$create_basic_grid();
-
- for (my $row = 0; $row < scalar($labeled_widgets->@*); $row++) {
- my ($label_text, $widget, $suffix_label) = $labeled_widgets->[$row]->@*;
-
- my $label = Gtk3::Label->new($label_text);
- $label->set_visible(1);
- $label->set_xalign(1.0);
- $grid->attach($label, 0, $row, 1, 1);
-
- $widget->set_visible(1);
- $grid->attach($widget, 1, $row, 1, 1);
-
- if ($suffix_label) {
- my $suffix_label = Gtk3::Label->new($suffix_label);
- $suffix_label->set_visible(1);
- $suffix_label->set_xalign(1.0);
- $grid->attach($suffix_label, 2, $row, 1, 1);
- }
- }
-
- return $grid;
-};
-
# only relevant for raid with its multipl diskX to diskY mappings.
my $get_selected_hdsize = sub {
my $hdsize = shift;
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 11/23] gui: network: add vlan setup options to advanced network dialog
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (9 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 10/23] gui: network: add bond setup options to " Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 12/23] auto: pass through management network bond options to low-level installer Christoph Heiss
` (11 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Fixes #2164 [0].
Adds a third frame to the advanced network options dialog, allowing to
set a VLAN ID for the management interface.
[0] https://bugzilla.proxmox.com/show_bug.cgi?id=2164
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* don't expand checkbox and grid, which resulted in unnecessary padding
and dialog size
Changes v2 -> v3:
* new patch
proxinstall | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/proxinstall b/proxinstall
index a676018..2622871 100755
--- a/proxinstall
+++ b/proxinstall
@@ -87,6 +87,7 @@ my @steps = (
# GUI global variables
my $gtk_state = {
network_bonding_enabled => 0,
+ network_vlan_enabled => 0,
};
my $target_hds; # only for the summary view
@@ -641,6 +642,64 @@ my sub create_network_bond_frame {
return ($frame, $refresh_if_names, $on_done);
}
+my sub create_network_vlan_frame {
+ my $frame = Gtk3::Frame->new('Management VLAN');
+
+ my $vbox = Gtk3::Box->new('vertical', 5);
+ $frame->add($vbox);
+ $vbox->set_margin_start(10);
+ $vbox->set_margin_end(10);
+ $vbox->set_margin_top(5);
+ $vbox->set_margin_bottom(5);
+
+ my $enable_checkbox = Gtk3::CheckButton->new('Enable VLAN setup on management interface');
+ $enable_checkbox->set_active($gtk_state->{network_vlan_enabled});
+ $vbox->pack_start($enable_checkbox, 1, 0, 5);
+
+ my $vlan_opts = Proxmox::Install::Config::get_mngmt_vlan_opt();
+
+ my $initial_vid = $vlan_opts->{vid} > 0 ? $vlan_opts->{vid} : 1;
+ my $vid_adjustment = Gtk3::Adjustment->new($initial_vid, 2, 4095, 1, 1, 1);
+ my $vid_input = Gtk3::SpinButton->new($vid_adjustment, 1, 0);
+ $vid_input->set_tooltip_text('Must be between 2 and 4094 inclusive.');
+
+ my $opts_grid = $create_label_widget_grid->([
+ ['VLAN tag', $vid_input],
+ ]);
+
+ $vbox->pack_start($opts_grid, 0, 0, 5);
+
+ my $set_sensitive = sub {
+ my ($active) = @_;
+
+ $opts_grid->foreach(sub {
+ $_[0]->set_sensitive($active);
+ });
+ };
+ $set_sensitive->($gtk_state->{network_vlan_enabled});
+
+ $enable_checkbox->signal_connect(
+ toggled => sub {
+ my $active = !!$enable_checkbox->get_active();
+
+ $gtk_state->{network_vlan_enabled} = $active;
+ $set_sensitive->($active);
+ },
+ );
+
+ my $on_done = sub {
+ if (!$enable_checkbox->get_active()) {
+ Proxmox::Install::Config::set_mngmt_vlan_opt('enabled', 0);
+ return;
+ }
+
+ Proxmox::Install::Config::set_mngmt_vlan_opt('vid', $vid_input->get_value_as_int());
+ Proxmox::Install::Config::set_mngmt_vlan_opt('enabled', 1);
+ };
+
+ return ($frame, $on_done);
+}
+
my sub create_network_advanced_options_view {
my ($done_cb) = @_;
@@ -661,9 +720,11 @@ my sub create_network_advanced_options_view {
my ($bond_frame, $bond_refresh, $bond_on_done) = create_network_bond_frame();
my ($pin_frame, $pin_on_done) = create_network_interface_pin_frame($bond_refresh);
+ my ($vlan_frame, $vlan_on_done) = create_network_vlan_frame();
$hbox->pack_start($pin_frame, 0, 0, 10);
$hbox->pack_start($bond_frame, 0, 0, 10);
+ $hbox->pack_start($vlan_frame, 0, 0, 10);
$scrolled_window->add($hbox);
$content->pack_start($scrolled_window, 1, 1, 10);
@@ -674,6 +735,7 @@ my sub create_network_advanced_options_view {
eval {
$pin_on_done->();
$bond_on_done->();
+ $vlan_on_done->();
};
if ($@) {
Proxmox::UI::message($@, $dialog);
@@ -1062,6 +1124,11 @@ sub create_ack_view {
: $iface->{name};
}
+ if (Proxmox::Install::Config::get_mngmt_vlan_enabled()) {
+ my $vid = Proxmox::Install::Config::get_mngmt_vlan_opt('vid');
+ $interface .= ", VLAN $vid";
+ }
+
my %config_values = (
__target_hd__ => join(' | ', $target_hds->@*),
__target_fs__ => Proxmox::Install::Config::get_filesys(),
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 12/23] auto: pass through management network bond options to low-level installer
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (10 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 11/23] gui: network: add vlan setup options to advanced network dialog Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 13/23] auto: pass through management network vlan " Christoph Heiss
` (10 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Partially fixes #2164 [0].
The auto-installer gains a new section under [network] in its answer
file for setting up a management bond:
[network.bond]
members = ["ab:cd:ef:12:34:56", "ab:cd:ef:12:34:57"]
mode = "active-backup"
hash-policy = "layer2+3"
primary-interface = "ab:cd:ef:12:34:57"
This then just passes through the given bond options in the answer file
to the low-level installation configuration, verifying them beforehand
and printing a status message that bonding is enabled.
[0] https://bugzilla.proxmox.com/show_bug.cgi?id=2164
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* rebased on applied `proxmox-installer-types` changes
Changes v3 -> v4:
* skip serializing disabled bond options to low-level config
* adapt to updated proxmox-installer-types, add support for
`members-filter`
Changes v2 -> v3:
* fixed typo in subject
* fixed patch section in Cargo.toml
Changes v1 -> v2:
* no changes
Cargo.toml | 2 +
debian/control | 1 +
proxmox-auto-installer/Cargo.toml | 1 +
proxmox-auto-installer/src/utils.rs | 99 ++++++++++++++++---
proxmox-auto-installer/tests/parse-answer.rs | 7 +-
.../resources/parse_answer/network_bond.json | 25 +++++
.../resources/parse_answer/network_bond.toml | 20 ++++
.../network_bond_with_filter.json | 24 +++++
.../network_bond_with_filter.toml | 22 +++++
.../network_bond_invalid_hash_policy.json | 3 +
.../network_bond_invalid_hash_policy.toml | 20 ++++
.../network_bond_invalid_mode.json | 3 +
.../network_bond_invalid_mode.toml | 18 ++++
.../network_bond_primary_not_member.json | 3 +
.../network_bond_primary_not_member.toml | 19 ++++
.../network_bond_single_interface.json | 3 +
.../network_bond_single_interface.toml | 18 ++++
proxmox-installer-common/Cargo.toml | 1 +
proxmox-installer-common/src/options.rs | 38 ++++---
proxmox-installer-common/src/setup.rs | 45 ++++++++-
20 files changed, 344 insertions(+), 28 deletions(-)
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_bond.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_bond.toml
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_bond_with_filter.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_bond_with_filter.toml
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_hash_policy.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_hash_policy.toml
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_mode.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_mode.toml
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_primary_not_member.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_primary_not_member.toml
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_single_interface.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_single_interface.toml
diff --git a/Cargo.toml b/Cargo.toml
index 747608e..75b5628 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -27,11 +27,13 @@ serde_plain = "1.0"
toml = "0.8"
proxmox-auto-installer.path = "./proxmox-auto-installer"
proxmox-installer-common.path = "./proxmox-installer-common"
+proxmox-network-api = "1.0"
proxmox-network-types = "1.1"
proxmox-installer-types = { version = "0.3", features = ["legacy"] }
# Local path overrides
# NOTE: You must run `cargo update` after changing this for it to take effect!
[patch.crates-io]
+# proxmox-network-api.path = "../proxmox/proxmox-network-api"
# proxmox-network-types.path = "../proxmox/proxmox-network-types"
# proxmox-installer-types.path = "../proxmox/proxmox-installer-types"
diff --git a/debian/control b/debian/control
index e860819..0f09c02 100644
--- a/debian/control
+++ b/debian/control
@@ -20,6 +20,7 @@ Build-Depends: cargo:native,
librust-pico-args-0.5-dev,
librust-pretty-assertions-1.4-dev,
librust-proxmox-installer-types-0.3+legacy-dev,
+ librust-proxmox-network-api-1-dev,
librust-proxmox-network-types-1-dev (>= 1.1-~~),
librust-proxmox-sys+crypt-dev,
librust-regex-1+default-dev (>= 1.7~~),
diff --git a/proxmox-auto-installer/Cargo.toml b/proxmox-auto-installer/Cargo.toml
index 5ef2f4f..b7ca85d 100644
--- a/proxmox-auto-installer/Cargo.toml
+++ b/proxmox-auto-installer/Cargo.toml
@@ -15,6 +15,7 @@ anyhow.workspace = true
log.workspace = true
proxmox-installer-common = { workspace = true, features = ["http"] }
proxmox-network-types.workspace = true
+proxmox-network-api.workspace = true
proxmox-installer-types.workspace = true
serde = { workspace = true, features = ["derive"] }
serde_json.workspace = true
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 9d27086..1a37314 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -1,31 +1,30 @@
use anyhow::{Context, Result, bail};
use glob::Pattern;
use log::{info, warn};
+use serde::{Deserialize, Serialize};
use std::{
collections::{BTreeMap, HashSet},
process::Command,
};
-use proxmox_installer_types::{
- UdevInfo,
- answer::{
- AutoInstallerConfig, DiskSelection, Filesystem, FilesystemOptions, FilesystemType,
- FilterMatch, FirstBootHookSourceMode, FqdnConfig, FqdnFromDhcpConfig, FqdnSourceMode,
- NetworkConfig,
- },
-};
-
use proxmox_installer_common::{
ROOT_PASSWORD_MIN_LENGTH,
disk_checks::check_swapsize,
options::{FilesystemDiskInfo, NetworkInterfacePinningOptions, NetworkOptions, email_validate},
setup::{
- InstallBtrfsOption, InstallConfig, InstallFirstBootSetup, InstallRootPassword,
- InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo,
+ InstallBtrfsOption, InstallConfig, InstallFirstBootSetup, InstallNetworkBondOptions,
+ InstallRootPassword, InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo,
},
};
-
-use serde::{Deserialize, Serialize};
+use proxmox_installer_types::{
+ UdevInfo,
+ answer::{
+ AutoInstallerConfig, DiskSelection, Filesystem, FilesystemOptions, FilesystemType,
+ FilterMatch, FirstBootHookSourceMode, FqdnConfig, FqdnFromDhcpConfig, FqdnSourceMode,
+ NetworkBondOptions, NetworkConfig,
+ },
+};
+use proxmox_network_types::MacAddress;
fn get_network_settings(
answer: &AutoInstallerConfig,
@@ -48,6 +47,7 @@ fn get_network_settings(
&runtime_info.network,
None,
pinning_opts.as_ref(),
+ answer.network.bond(),
);
opts.fqdn = name.to_owned();
opts
@@ -76,6 +76,7 @@ fn get_network_settings(
&runtime_info.network,
domain.as_deref(),
pinning_opts.as_ref(),
+ answer.network.bond(),
)
}
};
@@ -92,6 +93,37 @@ fn get_network_settings(
opts.verify()?;
}
+ if let Some(opts) = &network_options.bond_opts {
+ let members = match_selected_network_bond_members(opts, udev_info, runtime_info)?
+ .iter()
+ .fold(String::new(), |acc, mac| {
+ // Resolve either to the pinned name, if enabled, otherwise to the kernel-assigned one.
+ let name = network_options
+ .pinning_opts
+ .as_ref()
+ .and_then(|o| o.mapping.get(mac).cloned())
+ .or_else(|| {
+ runtime_info
+ .network
+ .interfaces
+ .iter()
+ .find_map(|(_, iface)| {
+ (iface.mac == *mac).then_some(iface.name.to_owned())
+ })
+ })
+ .unwrap_or("<unknown>".to_owned());
+
+ if acc.is_empty() {
+ name
+ } else {
+ format!("{acc}, {name}")
+ }
+ });
+
+ info!("Network interface bonding enabled on: {members}");
+ opts.verify()?;
+ }
+
info!("Network interface used is '{}'", &network_options.ifname);
Ok(network_options)
}
@@ -192,7 +224,7 @@ pub fn get_matched_udev_indexes(
let mut did_match_all = true;
for (filter_key, filter_value) in filter {
let filter_pattern =
- Pattern::new(filter_value).context("invalid glob in disk selection")?;
+ Pattern::new(filter_value).context("invalid glob in filter selection")?;
for (udev_key, udev_value) in dev_values {
if udev_key == filter_key && filter_pattern.matches(udev_value) {
did_match_once = true;
@@ -328,6 +360,30 @@ fn set_selected_disks(
Ok(())
}
+fn match_selected_network_bond_members(
+ bond: &NetworkBondOptions,
+ udev_info: &UdevInfo,
+ runtime_info: &RuntimeInfo,
+) -> Result<Vec<MacAddress>> {
+ if bond.filter.is_empty() {
+ Ok(bond.members.clone())
+ } else {
+ let filter_match = bond.filter_match.unwrap_or_default();
+ // list of kernel-assigned names
+ let selected = get_matched_udev_indexes(
+ &bond.filter,
+ &udev_info.nics,
+ filter_match == FilterMatch::All,
+ )?
+ .into_iter()
+ .filter_map(|ifname| runtime_info.network.interfaces.get(&ifname))
+ .map(|iface| iface.mac)
+ .collect();
+
+ Ok(selected)
+ }
+}
+
fn get_first_selected_disk(config: &InstallConfig) -> usize {
config
.disk_selection
@@ -493,6 +549,10 @@ pub fn verify_network_settings(
}
}
+ if let Some(opts) = network.bond() {
+ opts.verify()?;
+ }
+
Ok(())
}
@@ -552,6 +612,17 @@ pub fn parse_answer(
subscription_key: answer.global.subscription_key.clone(),
mngmt_nic: network_settings.ifname,
+ mngmt_bond: network_settings
+ .bond_opts
+ .map(|bond| -> Result<InstallNetworkBondOptions> {
+ Ok(InstallNetworkBondOptions {
+ members: match_selected_network_bond_members(&bond, udev_info, runtime_info)?,
+ mode: bond.mode,
+ hash_policy: bond.hash_policy,
+ primary_interface: bond.primary_interface,
+ })
+ })
+ .unwrap_or_else(|| Ok(InstallNetworkBondOptions::default()))?,
network_interface_pin_map: network_settings
.pinning_opts
.map(|o| o.mapping)
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index ae5b87d..0320bba 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -127,6 +127,7 @@ mod tests {
full_fqdn_from_dhcp_with_default_domain,
hashed_root_password,
minimal,
+ network_bond,
network_interface_pinning,
network_interface_pinning_mixed_case_mac_addresses,
nic_matching,
@@ -150,11 +151,15 @@ mod tests {
fqdn_hostname_only,
ipv4_and_subnet_mask_33,
lvm_swapsize_greater_than_hdsize,
+ network_bond_invalid_hash_policy,
+ network_bond_invalid_mode,
+ network_bond_primary_not_member,
+ network_bond_single_interface,
network_interface_pinning_overlong_interface_name,
no_fqdn_from_dhcp,
no_root_password_set,
short_password,
- zfs_raid_single_disk
+ zfs_raid_single_disk,
);
}
}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/network_bond.json b/proxmox-auto-installer/tests/resources/parse_answer/network_bond.json
new file mode 100644
index 0000000..cf0aa98
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/network_bond.json
@@ -0,0 +1,25 @@
+{
+ "autoreboot": 1,
+ "cidr": "192.168.1.114/24",
+ "country": "at",
+ "dns": "192.168.1.254",
+ "domain": "testinstall",
+ "filesys": "ext4",
+ "gateway": "192.168.1.1",
+ "hdsize": 223.57088470458984,
+ "existing_storage_auto_rename": 1,
+ "hostname": "pveauto",
+ "keymap": "de",
+ "mailto": "mail@no.invalid",
+ "mngmt_nic": "bond0",
+ "mngmt_bond": {
+ "members": ["B4:2E:99:AC:AD:B4", "B4:2E:99:AC:AD:B5"],
+ "mode": "active-backup",
+ "hash-policy": "layer3+4",
+ "primary-interface": "B4:2E:99:AC:AD:B5"
+ },
+ "root_password": { "plain": "12345678" },
+ "target_hd": "/dev/sda",
+ "timezone": "Europe/Vienna",
+ "first_boot": { "enabled": 0 }
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/network_bond.toml b/proxmox-auto-installer/tests/resources/parse_answer/network_bond.toml
new file mode 100644
index 0000000..dc6b7b0
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/network_bond.toml
@@ -0,0 +1,20 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-dhcp"
+
+[network.bond]
+members = ["b4:2e:99:ac:ad:b4", "b4:2e:99:ac:ad:b5"]
+mode = "active-backup"
+hash-policy = "layer3+4"
+primary-interface = "b4:2e:99:ac:ad:b5"
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/network_bond_with_filter.json b/proxmox-auto-installer/tests/resources/parse_answer/network_bond_with_filter.json
new file mode 100644
index 0000000..91a4723
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/network_bond_with_filter.json
@@ -0,0 +1,24 @@
+{
+ "autoreboot": 1,
+ "cidr": "192.168.1.114/24",
+ "country": "at",
+ "dns": "192.168.1.254",
+ "domain": "testinstall",
+ "filesys": "ext4",
+ "gateway": "192.168.1.1",
+ "hdsize": 223.57088470458984,
+ "existing_storage_auto_rename": 1,
+ "hostname": "pveauto",
+ "keymap": "de",
+ "mailto": "mail@no.invalid",
+ "mngmt_nic": "bond0",
+ "mngmt_bond": {
+ "members": ["B4:2E:99:AC:AD:B4", "B4:2E:99:AC:AD:B5"],
+ "mode": "active-backup",
+ "hash-policy": "layer2"
+ },
+ "root_password": { "plain": "12345678" },
+ "target_hd": "/dev/sda",
+ "timezone": "Europe/Vienna",
+ "first_boot": { "enabled": 0 }
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/network_bond_with_filter.toml b/proxmox-auto-installer/tests/resources/parse_answer/network_bond_with_filter.toml
new file mode 100644
index 0000000..849d93f
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/network_bond_with_filter.toml
@@ -0,0 +1,22 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-dhcp"
+
+[network.bond]
+# matches `eno1` and `eno2`
+filter.ID_NET_DRIVER = "igb"
+filter.ID_PATH = "pci-0000:c2:00.*"
+filter-match = "all"
+mode = "active-backup"
+hash-policy = "layer2"
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_hash_policy.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_hash_policy.json
new file mode 100644
index 0000000..b76c9d6
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_hash_policy.json
@@ -0,0 +1,3 @@
+{
+ "parse-error": "error parsing answer.toml: unknown variant `vlan+srcmac`, expected one of `layer2`, `layer2+3`, `layer3+4`"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_hash_policy.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_hash_policy.toml
new file mode 100644
index 0000000..af2789b
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_hash_policy.toml
@@ -0,0 +1,20 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-dhcp"
+
+[network.bond]
+members = ["b4:2e:99:ac:ad:b4", "b4:2e:99:ac:ad:b5"]
+mode = "active-backup"
+hash-policy = "vlan+srcmac"
+primary-interface = "b4:2e:99:ac:ad:b5"
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_mode.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_mode.json
new file mode 100644
index 0000000..95461dc
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_mode.json
@@ -0,0 +1,3 @@
+{
+ "parse-error": "error parsing answer.toml: unknown variant `non-existent-mode`, expected one of `balance-rr`, `active-backup`, `balance-xor`, `broadcast`, `802.3ad`, `balance-tlb`, `balance-alb`"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_mode.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_mode.toml
new file mode 100644
index 0000000..79695da
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_invalid_mode.toml
@@ -0,0 +1,18 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-dhcp"
+
+[network.bond]
+members = ["b4:2e:99:ac:ad:b4", "b4:2e:99:ac:ad:b5"]
+mode = "non-existent-mode"
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_primary_not_member.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_primary_not_member.json
new file mode 100644
index 0000000..7e64e92
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_primary_not_member.json
@@ -0,0 +1,3 @@
+{
+ "error": "primary bond interface B4:2E:99:AC:AD:B6 must be a member of the bond (members: B4:2E:99:AC:AD:B4, B4:2E:99:AC:AD:B5)"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_primary_not_member.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_primary_not_member.toml
new file mode 100644
index 0000000..fab400a
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_primary_not_member.toml
@@ -0,0 +1,19 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-dhcp"
+
+[network.bond]
+members = ["b4:2e:99:ac:ad:b4", "b4:2e:99:ac:ad:b5"]
+mode = "active-backup"
+primary-interface = "b4:2e:99:ac:ad:b6"
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_single_interface.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_single_interface.json
new file mode 100644
index 0000000..730fad6
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_single_interface.json
@@ -0,0 +1,3 @@
+{
+ "error": "bond setup requires at least 2 interfaces, got 1"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_single_interface.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_single_interface.toml
new file mode 100644
index 0000000..b035cef
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_bond_single_interface.toml
@@ -0,0 +1,18 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-dhcp"
+
+[network.bond]
+members = ["b4:2e:99:ac:ad:b4"]
+mode = "balance-rr"
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
diff --git a/proxmox-installer-common/Cargo.toml b/proxmox-installer-common/Cargo.toml
index 7682680..a283094 100644
--- a/proxmox-installer-common/Cargo.toml
+++ b/proxmox-installer-common/Cargo.toml
@@ -13,6 +13,7 @@ regex.workspace = true
serde = { workspace = true, features = [ "derive" ] }
serde_json.workspace = true
serde_plain.workspace = true
+proxmox-network-api.workspace = true
proxmox-network-types.workspace = true
proxmox-installer-types.workspace = true
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index fbbfe77..1add63b 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -15,8 +15,8 @@ use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo};
use proxmox_installer_types::{
EMAIL_DEFAULT_PLACEHOLDER,
answer::{
- BtrfsCompressOption, BtrfsRaidLevel, FilesystemType, NetworkInterfacePinningOptionsAnswer,
- ZfsChecksumOption, ZfsCompressOption, ZfsRaidLevel,
+ BtrfsCompressOption, BtrfsRaidLevel, FilesystemType, NetworkBondOptions,
+ NetworkInterfacePinningOptionsAnswer, ZfsChecksumOption, ZfsCompressOption, ZfsRaidLevel,
},
};
use proxmox_network_types::{MacAddress, fqdn::Fqdn, ip_address::Cidr};
@@ -395,6 +395,7 @@ pub struct NetworkOptions {
pub gateway: IpAddr,
pub dns_server: IpAddr,
pub pinning_opts: Option<NetworkInterfacePinningOptions>,
+ pub bond_opts: Option<NetworkBondOptions>,
}
impl NetworkOptions {
@@ -405,6 +406,7 @@ impl NetworkOptions {
network: &NetworkInfo,
default_domain: Option<&str>,
pinning_opts: Option<&NetworkInterfacePinningOptions>,
+ bond_opts: Option<&NetworkBondOptions>,
) -> Self {
// Sets up sensible defaults as much as possible, such that even in the
// worse case nothing breaks down *completely*.
@@ -417,6 +419,7 @@ impl NetworkOptions {
gateway: Ipv4Addr::new(192, 168, 100, 1).into(),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: pinning_opts.cloned(),
+ bond_opts: bond_opts.cloned(),
};
let iface = if let Some(gw) = &network.routes.gateway4
@@ -479,6 +482,10 @@ impl NetworkOptions {
}
}
+ if this.bond_opts.is_some() {
+ this.ifname = "bond0".to_owned();
+ }
+
this
}
@@ -632,7 +639,7 @@ mod tests {
let (setup, mut info) = mock_setup_network();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.bar.com").unwrap(),
@@ -640,12 +647,13 @@ mod tests {
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
+ bond_opts: None,
}
);
info.hostname = None;
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("pve.bar.com").unwrap(),
@@ -653,12 +661,13 @@ mod tests {
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
+ bond_opts: None,
}
);
info.dns.domain = None;
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("pve.example.invalid").unwrap(),
@@ -666,12 +675,13 @@ mod tests {
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
+ bond_opts: None,
}
);
info.hostname = Some("foo".to_owned());
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.example.invalid").unwrap(),
@@ -679,6 +689,7 @@ mod tests {
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
+ bond_opts: None,
}
);
}
@@ -726,7 +737,7 @@ mod tests {
let (setup, info) = mock_setup_network_v6_only();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.bar.com").unwrap(),
@@ -734,6 +745,7 @@ mod tests {
gateway: IpAddr::V6(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1)),
dns_server: IpAddr::V6(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 0x53)),
pinning_opts: None,
+ bond_opts: None,
}
);
}
@@ -743,7 +755,7 @@ mod tests {
let (setup, mut info) = mock_setup_network();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.bar.com").unwrap(),
@@ -751,12 +763,13 @@ mod tests {
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
+ bond_opts: None,
}
);
info.dns.domain = None;
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None),
+ NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.custom.local").unwrap(),
@@ -764,12 +777,13 @@ mod tests {
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
+ bond_opts: None,
}
);
info.dns.domain = Some("some.domain.local".to_owned());
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None),
+ NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.custom.local").unwrap(),
@@ -777,6 +791,7 @@ mod tests {
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
+ bond_opts: None,
}
);
}
@@ -813,7 +828,7 @@ mod tests {
let setup = SetupInfo::mocked();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("pve.example.invalid").unwrap(),
@@ -821,6 +836,7 @@ mod tests {
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 100, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
+ bond_opts: None,
}
);
}
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 14882bc..31a1195 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -16,8 +16,12 @@ use crate::options::{
};
use proxmox_installer_types::{
BootType, IsoInfo, ProductConfig,
- answer::{BtrfsCompressOption, FilesystemType, ZfsChecksumOption, ZfsCompressOption},
+ answer::{
+ BtrfsCompressOption, FilesystemType, ZfsChecksumOption,
+ ZfsCompressOption,
+ },
};
+use proxmox_network_api::{BondXmitHashPolicy, LinuxBondMode};
use proxmox_network_types::{Cidr, MacAddress};
/// Paths in the ISO environment containing installer data.
@@ -326,7 +330,8 @@ pub struct NetworkInfo {
pub dns: Dns,
pub routes: Routes,
- /// Maps devices to their configuration, if it has a usable configuration.
+ /// Maps devices to their configuration, if it has a usable configuration, keyed by their
+ /// kernel-assigned interface name.
/// (Contains no entries for devices with only link-local addresses.)
#[serde(default)]
pub interfaces: BTreeMap<String, Interface>,
@@ -488,6 +493,39 @@ pub fn spawn_low_level_installer(test_mode: bool) -> io::Result<process::Child>
.spawn()
}
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+/// Options for creating a bond for the management interface with the given
+/// physical interfaces and mode.
+pub struct InstallNetworkBondOptions {
+ /// List of interface MAC addresses to add as bond members.
+ pub members: Vec<MacAddress>,
+ /// Mode for the bond to set.
+ pub mode: LinuxBondMode,
+ /// Bond hash policy. Only used with '802.3ad' and 'balance-xor' mode.
+ pub hash_policy: BondXmitHashPolicy,
+ /// Primary bond interface. Only used with 'active-backup' mode.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub primary_interface: Option<MacAddress>,
+}
+
+impl Default for InstallNetworkBondOptions {
+ fn default() -> Self {
+ Self {
+ members: Vec::new(),
+ mode: LinuxBondMode::ActiveBackup,
+ hash_policy: BondXmitHashPolicy::Layer2,
+ primary_interface: None,
+ }
+ }
+}
+
+impl InstallNetworkBondOptions {
+ fn is_disabled(&self) -> bool {
+ self.members.is_empty()
+ }
+}
+
/// See Proxmox::Install::Config
#[derive(Debug, Deserialize, Serialize)]
pub struct InstallConfig {
@@ -530,6 +568,9 @@ pub struct InstallConfig {
pub subscription_key: Option<String>,
pub mngmt_nic: String,
+ #[serde(skip_serializing_if = "InstallNetworkBondOptions::is_disabled")]
+ pub mngmt_bond: InstallNetworkBondOptions,
+
// Maps MAC addresses -> custom name. If set, enables pinning for all
// interfaces present.
#[serde(
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 13/23] auto: pass through management network vlan options to low-level installer
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (11 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 12/23] auto: pass through management network bond options to low-level installer Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 14/23] tui: drop long obsolete comment about logo formatting Christoph Heiss
` (9 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Fixes #2164 [0].
The auto-installer gains a new section under [network] in its answer
file for setting up a VLAN on the management interface during the
installation:
[network.vlan]
vid = 5
Where `vid` is the desired VLAN ID.
This then just passes through the given VLAN options in the answer file
to the low-level installation configuration, verifying them beforehand
and printing a status message if the management interface will be put on
a VLAN.
[0] https://bugzilla.proxmox.com/show_bug.cgi?id=2164
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* make `InstallNetworkVlanOptions::enabled` de-/serialize from/to
an integer to make it compatible with the Perl-generated JSON
Changes v3 -> v4:
* skip serializing disabled vlan options to low-level config
Changes v2 -> v3:
* new patch
proxmox-auto-installer/src/utils.rs | 14 ++++++++
proxmox-auto-installer/tests/parse-answer.rs | 3 ++
.../parse_answer/network_bond_and_vlan.json | 29 ++++++++++++++++
.../parse_answer/network_bond_and_vlan.toml | 27 +++++++++++++++
.../resources/parse_answer/network_vlan.json | 23 +++++++++++++
.../resources/parse_answer/network_vlan.toml | 21 ++++++++++++
.../network_vlan_invalid_vid.json | 3 ++
.../network_vlan_invalid_vid.toml | 21 ++++++++++++
proxmox-installer-common/src/options.rs | 33 +++++++++++++------
proxmox-installer-common/src/setup.rs | 32 +++++++++++++++++-
10 files changed, 195 insertions(+), 11 deletions(-)
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_bond_and_vlan.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_bond_and_vlan.toml
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_vlan.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_vlan.toml
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_vlan_invalid_vid.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_vlan_invalid_vid.toml
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 1a37314..b24dc9d 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -48,6 +48,7 @@ fn get_network_settings(
None,
pinning_opts.as_ref(),
answer.network.bond(),
+ answer.network.vlan(),
);
opts.fqdn = name.to_owned();
opts
@@ -77,6 +78,7 @@ fn get_network_settings(
domain.as_deref(),
pinning_opts.as_ref(),
answer.network.bond(),
+ answer.network.vlan(),
)
}
};
@@ -124,6 +126,10 @@ fn get_network_settings(
opts.verify()?;
}
+ if let Some(opts) = &network_options.vlan_opts {
+ info!("Management interface reachable on VLAN {}", opts.vid);
+ }
+
info!("Network interface used is '{}'", &network_options.ifname);
Ok(network_options)
}
@@ -553,6 +559,10 @@ pub fn verify_network_settings(
opts.verify()?;
}
+ if let Some(opts) = network.vlan() {
+ opts.verify()?;
+ }
+
Ok(())
}
@@ -623,6 +633,10 @@ pub fn parse_answer(
})
})
.unwrap_or_else(|| Ok(InstallNetworkBondOptions::default()))?,
+ mngmt_vlan: network_settings
+ .vlan_opts
+ .map(|opts| opts.into())
+ .unwrap_or_default(),
network_interface_pin_map: network_settings
.pinning_opts
.map(|o| o.mapping)
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 0320bba..4776c4d 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -128,8 +128,10 @@ mod tests {
hashed_root_password,
minimal,
network_bond,
+ network_bond_and_vlan,
network_interface_pinning,
network_interface_pinning_mixed_case_mac_addresses,
+ network_vlan,
nic_matching,
no_network,
specific_nic,
@@ -156,6 +158,7 @@ mod tests {
network_bond_primary_not_member,
network_bond_single_interface,
network_interface_pinning_overlong_interface_name,
+ network_vlan_invalid_vid,
no_fqdn_from_dhcp,
no_root_password_set,
short_password,
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/network_bond_and_vlan.json b/proxmox-auto-installer/tests/resources/parse_answer/network_bond_and_vlan.json
new file mode 100644
index 0000000..ef67609
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/network_bond_and_vlan.json
@@ -0,0 +1,29 @@
+{
+ "autoreboot": 1,
+ "cidr": "192.168.1.114/24",
+ "country": "at",
+ "dns": "192.168.1.254",
+ "domain": "testinstall",
+ "filesys": "ext4",
+ "gateway": "192.168.1.1",
+ "hdsize": 223.57088470458984,
+ "existing_storage_auto_rename": 1,
+ "hostname": "pveauto",
+ "keymap": "de",
+ "mailto": "mail@no.invalid",
+ "mngmt_nic": "eno1",
+ "mngmt_bond": {
+ "members": ["B4:2E:99:AC:AD:B4", "B4:2E:99:AC:AD:B5"],
+ "mode": "active-backup",
+ "hash-policy": "layer3+4",
+ "primary-interface": "B4:2E:99:AC:AD:B5"
+ },
+ "mngmt_vlan": {
+ "enabled": 1,
+ "vid": 5
+ },
+ "root_password": { "plain": "12345678" },
+ "target_hd": "/dev/sda",
+ "timezone": "Europe/Vienna",
+ "first_boot": { "enabled": 0 }
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/network_bond_and_vlan.toml b/proxmox-auto-installer/tests/resources/parse_answer/network_bond_and_vlan.toml
new file mode 100644
index 0000000..4c76f8f
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/network_bond_and_vlan.toml
@@ -0,0 +1,27 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-answer"
+cidr = "192.168.1.114/24"
+gateway = "192.168.1.1"
+dns = "192.168.1.254"
+filter.ID_NET_NAME = "eno1"
+
+[network.bond]
+members = ["b4:2e:99:ac:ad:b4", "b4:2e:99:ac:ad:b5"]
+mode = "active-backup"
+hash-policy = "layer3+4"
+primary-interface = "b4:2e:99:ac:ad:b5"
+
+[network.vlan]
+vid = 5
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/network_vlan.json b/proxmox-auto-installer/tests/resources/parse_answer/network_vlan.json
new file mode 100644
index 0000000..e579eb0
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/network_vlan.json
@@ -0,0 +1,23 @@
+{
+ "autoreboot": 1,
+ "cidr": "192.168.1.114/24",
+ "country": "at",
+ "dns": "192.168.1.254",
+ "domain": "testinstall",
+ "filesys": "ext4",
+ "gateway": "192.168.1.1",
+ "hdsize": 223.57088470458984,
+ "existing_storage_auto_rename": 1,
+ "hostname": "pveauto",
+ "keymap": "de",
+ "mailto": "mail@no.invalid",
+ "mngmt_nic": "eno1",
+ "mngmt_vlan": {
+ "enabled": 1,
+ "vid": 5
+ },
+ "root_password": { "plain": "12345678" },
+ "target_hd": "/dev/sda",
+ "timezone": "Europe/Vienna",
+ "first_boot": { "enabled": 0 }
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/network_vlan.toml b/proxmox-auto-installer/tests/resources/parse_answer/network_vlan.toml
new file mode 100644
index 0000000..981aa80
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/network_vlan.toml
@@ -0,0 +1,21 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-answer"
+cidr = "192.168.1.114/24"
+gateway = "192.168.1.1"
+dns = "192.168.1.254"
+filter.ID_NET_NAME = "eno1"
+
+[network.vlan]
+vid = 5
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_vlan_invalid_vid.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_vlan_invalid_vid.json
new file mode 100644
index 0000000..91590fa
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_vlan_invalid_vid.json
@@ -0,0 +1,3 @@
+{
+ "error": "VLAN tag must be between 2 and 4094 inclusive, got 5000"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_vlan_invalid_vid.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_vlan_invalid_vid.toml
new file mode 100644
index 0000000..0e8cba0
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_vlan_invalid_vid.toml
@@ -0,0 +1,21 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-answer"
+cidr = "192.168.0.10/24"
+gateway = "192.168.0.1"
+dns = "192.168.0.1"
+filter.ID_NET_NAME = "eno1"
+
+[network.vlan]
+vid = 5000
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 1add63b..e32b77d 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -16,7 +16,8 @@ use proxmox_installer_types::{
EMAIL_DEFAULT_PLACEHOLDER,
answer::{
BtrfsCompressOption, BtrfsRaidLevel, FilesystemType, NetworkBondOptions,
- NetworkInterfacePinningOptionsAnswer, ZfsChecksumOption, ZfsCompressOption, ZfsRaidLevel,
+ NetworkInterfacePinningOptionsAnswer, NetworkVlanOptions, ZfsChecksumOption,
+ ZfsCompressOption, ZfsRaidLevel,
},
};
use proxmox_network_types::{MacAddress, fqdn::Fqdn, ip_address::Cidr};
@@ -396,6 +397,7 @@ pub struct NetworkOptions {
pub dns_server: IpAddr,
pub pinning_opts: Option<NetworkInterfacePinningOptions>,
pub bond_opts: Option<NetworkBondOptions>,
+ pub vlan_opts: Option<NetworkVlanOptions>,
}
impl NetworkOptions {
@@ -407,6 +409,7 @@ impl NetworkOptions {
default_domain: Option<&str>,
pinning_opts: Option<&NetworkInterfacePinningOptions>,
bond_opts: Option<&NetworkBondOptions>,
+ vlan_opts: Option<&NetworkVlanOptions>,
) -> Self {
// Sets up sensible defaults as much as possible, such that even in the
// worse case nothing breaks down *completely*.
@@ -420,6 +423,7 @@ impl NetworkOptions {
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: pinning_opts.cloned(),
bond_opts: bond_opts.cloned(),
+ vlan_opts: vlan_opts.cloned(),
};
let iface = if let Some(gw) = &network.routes.gateway4
@@ -639,7 +643,7 @@ mod tests {
let (setup, mut info) = mock_setup_network();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.bar.com").unwrap(),
@@ -648,12 +652,13 @@ mod tests {
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
bond_opts: None,
+ vlan_opts: None,
}
);
info.hostname = None;
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("pve.bar.com").unwrap(),
@@ -662,12 +667,13 @@ mod tests {
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
bond_opts: None,
+ vlan_opts: None,
}
);
info.dns.domain = None;
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("pve.example.invalid").unwrap(),
@@ -676,12 +682,13 @@ mod tests {
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
bond_opts: None,
+ vlan_opts: None,
}
);
info.hostname = Some("foo".to_owned());
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.example.invalid").unwrap(),
@@ -690,6 +697,7 @@ mod tests {
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
bond_opts: None,
+ vlan_opts: None,
}
);
}
@@ -737,7 +745,7 @@ mod tests {
let (setup, info) = mock_setup_network_v6_only();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.bar.com").unwrap(),
@@ -746,6 +754,7 @@ mod tests {
dns_server: IpAddr::V6(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 0x53)),
pinning_opts: None,
bond_opts: None,
+ vlan_opts: None,
}
);
}
@@ -755,7 +764,7 @@ mod tests {
let (setup, mut info) = mock_setup_network();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.bar.com").unwrap(),
@@ -764,12 +773,13 @@ mod tests {
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
bond_opts: None,
+ vlan_opts: None,
}
);
info.dns.domain = None;
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None, None),
+ NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.custom.local").unwrap(),
@@ -778,12 +788,13 @@ mod tests {
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
bond_opts: None,
+ vlan_opts: None,
}
);
info.dns.domain = Some("some.domain.local".to_owned());
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None, None),
+ NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.custom.local").unwrap(),
@@ -792,6 +803,7 @@ mod tests {
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
bond_opts: None,
+ vlan_opts: None,
}
);
}
@@ -828,7 +840,7 @@ mod tests {
let setup = SetupInfo::mocked();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None, None, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("pve.example.invalid").unwrap(),
@@ -837,6 +849,7 @@ mod tests {
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
pinning_opts: None,
bond_opts: None,
+ vlan_opts: None,
}
);
}
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 31a1195..c8888d2 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -17,7 +17,7 @@ use crate::options::{
use proxmox_installer_types::{
BootType, IsoInfo, ProductConfig,
answer::{
- BtrfsCompressOption, FilesystemType, ZfsChecksumOption,
+ BtrfsCompressOption, FilesystemType, NetworkVlanOptions, ZfsChecksumOption,
ZfsCompressOption,
},
};
@@ -526,6 +526,34 @@ impl InstallNetworkBondOptions {
}
}
+#[derive(Clone, Default, Debug, Deserialize, Serialize, PartialEq)]
+/// Low-level installer options for setting up a VLAN configuration on the management interface.
+pub struct InstallNetworkVlanOptions {
+ /// Whether to enable VLAN tagging on the management interface
+ #[serde(
+ serialize_with = "serialize_bool_as_u32",
+ deserialize_with = "deserialize_bool_from_int"
+ )]
+ enabled: bool,
+ /// VLAN tag to configure.
+ pub vid: u16,
+}
+
+impl From<NetworkVlanOptions> for InstallNetworkVlanOptions {
+ fn from(opts: NetworkVlanOptions) -> Self {
+ Self {
+ enabled: true,
+ vid: opts.vid,
+ }
+ }
+}
+
+impl InstallNetworkVlanOptions {
+ fn is_disabled(&self) -> bool {
+ !self.enabled
+ }
+}
+
/// See Proxmox::Install::Config
#[derive(Debug, Deserialize, Serialize)]
pub struct InstallConfig {
@@ -570,6 +598,8 @@ pub struct InstallConfig {
pub mngmt_nic: String,
#[serde(skip_serializing_if = "InstallNetworkBondOptions::is_disabled")]
pub mngmt_bond: InstallNetworkBondOptions,
+ #[serde(skip_serializing_if = "InstallNetworkVlanOptions::is_disabled")]
+ pub mngmt_vlan: InstallNetworkVlanOptions,
// Maps MAC addresses -> custom name. If set, enables pinning for all
// interfaces present.
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 14/23] tui: drop long obsolete comment about logo formatting
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (12 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 13/23] auto: pass through management network vlan " Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 15/23] tui: views: add widget for displaying a group of checkboxes Christoph Heiss
` (8 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
This was made obsolete by the changes in
08ad8ed ("tui: move installer logo into background and to the top")
which stopped centering the logo through `TextView::center()`.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* adapted commit message as suggested by Thomas
Changes v1 -> v2:
* no changes
proxmox-tui-installer/src/main.rs | 1 -
1 file changed, 1 deletion(-)
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index e9f47a9..61a06f7 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -36,7 +36,6 @@ use views::{
TableViewItem, TimezoneOptionsView,
};
-// TextView::center() seems to garble the first two lines, so fix it manually here.
const PROXMOX_LOGO: &str = r"
____
| _ \ _ __ _____ ___ __ ___ _____ __
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 15/23] tui: views: add widget for displaying a group of checkboxes
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (13 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 14/23] tui: drop long obsolete comment about logo formatting Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 16/23] tui: network: factor out name pinning into advanced options dialog Christoph Heiss
` (7 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Adds a simple view which displays multiple checkboxes, each optionally
associated with some user-defined data.
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Split this out into a separate commit to ease reviewing/applying.
Changes v4 -> v5:
* also enable/disable labels by matching the applied style
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* no changes
proxmox-tui-installer/src/views/mod.rs | 124 ++++++++++++++++++++++++-
1 file changed, 122 insertions(+), 2 deletions(-)
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index a343e60..a73dde3 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -3,9 +3,11 @@ use std::{collections::HashMap, net::IpAddr, str::FromStr, sync::Arc};
use cursive::{
Printer, Rect, Vec2, View,
event::{Event, EventResult},
- theme::BaseColor,
+ theme::{BaseColor, StyleType},
view::{Resizable, ViewWrapper},
- views::{EditView, LinearLayout, NamedView, ResizedView, SelectView, TextView},
+ views::{
+ Checkbox, DummyView, EditView, LinearLayout, NamedView, ResizedView, SelectView, TextView,
+ },
};
mod bootdisk;
@@ -626,3 +628,121 @@ impl CidrAddressEditView {
impl ViewWrapper for CidrAddressEditView {
cursive::wrap_impl!(self.view: LinearLayout);
}
+
+/// List of checkboxes, each with an associated label and optional user-defined
+/// data for identifying its meaning.
+pub struct CheckboxGroup<UDT = ()> {
+ view: LinearLayout,
+ user_data: HashMap<usize, UDT>,
+}
+
+impl<UDT> CheckboxGroup<UDT> {
+ pub fn new() -> Self {
+ Self {
+ view: LinearLayout::vertical(),
+ user_data: HashMap::new(),
+ }
+ }
+
+ pub fn add(&mut self, label: &str, checked: bool, data: UDT) {
+ self.user_data.insert(self.view.len(), data);
+ self.view.add_child(
+ LinearLayout::horizontal()
+ .child(Checkbox::new().with_checked(checked))
+ .child(DummyView)
+ .child(TextView::new(label)),
+ );
+ }
+
+ pub fn entries(&self) -> impl Iterator<Item = (bool, &UDT)> {
+ CheckboxGroupIterator {
+ group: self,
+ index: 0,
+ }
+ }
+
+ pub fn set_enabled(&mut self, enabled: bool) {
+ let label_style = if enabled {
+ StyleType::primary()
+ } else {
+ StyleType::secondary()
+ };
+
+ for i in 0..self.view.len() {
+ if let Some(label) = self
+ .view
+ .get_child_mut(i)
+ .and_then(|v| v.downcast_mut::<LinearLayout>())
+ .and_then(|v| v.get_child_mut(2))
+ .and_then(|v| v.downcast_mut::<TextView>())
+ {
+ label.set_style(label_style);
+ }
+
+ if let Some(checkbox) = self
+ .view
+ .get_child_mut(i)
+ .and_then(|v| v.downcast_mut::<LinearLayout>())
+ .and_then(|v| v.get_child_mut(0))
+ .and_then(|v| v.downcast_mut::<Checkbox>())
+ {
+ checkbox.set_enabled(enabled);
+ }
+ }
+ }
+
+ pub fn update_labels(&mut self, callback: &dyn Fn(bool, &mut TextView, &mut UDT)) {
+ for i in 0..self.view.len() {
+ if let Some(inner) = self
+ .view
+ .get_child_mut(i)
+ .and_then(|v| v.downcast_mut::<LinearLayout>())
+ {
+ let checked = inner
+ .get_child(0)
+ .and_then(|v| v.downcast_ref::<Checkbox>())
+ .map(|v| v.is_checked())
+ .unwrap_or(false);
+
+ let label = inner
+ .get_child_mut(2)
+ .and_then(|v| v.downcast_mut::<TextView>());
+
+ let data = self.user_data.get_mut(&i);
+
+ if let (Some(label), Some(data)) = (label, data) {
+ callback(checked, label, data);
+ }
+ }
+ }
+ }
+}
+
+impl<UDT: Send + Sync + 'static> ViewWrapper for CheckboxGroup<UDT> {
+ cursive::wrap_impl!(self.view: LinearLayout);
+}
+
+pub struct CheckboxGroupIterator<'a, UDT> {
+ group: &'a CheckboxGroup<UDT>,
+ index: usize,
+}
+
+impl<'a, UDT> Iterator for CheckboxGroupIterator<'a, UDT> {
+ type Item = (bool, &'a UDT);
+
+ fn next(&mut self) -> Option<Self::Item> {
+ let checked = self
+ .group
+ .view
+ .get_child(self.index)?
+ .downcast_ref::<LinearLayout>()?
+ .get_child(0)?
+ .downcast_ref::<Checkbox>()?
+ .is_checked();
+
+ let data = self.group.user_data.get(&self.index)?;
+
+ self.index += 1;
+ Some((checked, data))
+ }
+}
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 16/23] tui: network: factor out name pinning into advanced options dialog
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (14 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 15/23] tui: views: add widget for displaying a group of checkboxes Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 17/23] tui: network: match form label style when enabling/disabling elements Christoph Heiss
` (6 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
This dialog can now be easily extended with additional panels for other,
advanced network-related options.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* no changes
proxinstall | 4 +-
proxmox-tui-installer/src/views/network.rs | 272 +++++++++++----------
2 files changed, 140 insertions(+), 136 deletions(-)
diff --git a/proxinstall b/proxinstall
index 2622871..079b227 100755
--- a/proxinstall
+++ b/proxinstall
@@ -395,12 +395,12 @@ my sub create_network_interface_pin_frame {
my $enable_checkbox = Gtk3::CheckButton->new('Pin network interface names');
$enable_checkbox->set_active($gtk_state->{network_pinning_enabled});
- $vbox->pack_start($enable_checkbox, 1, 0, 5);
+ $vbox->pack_start($enable_checkbox, 0, 0, 5);
my $grid = Gtk3::Grid->new();
$grid->set_column_spacing(10);
$grid->set_row_spacing(10);
- $vbox->pack_start($grid, 1, 1, 5);
+ $vbox->pack_start($grid, 0, 0, 5);
my $interfaces = Proxmox::Install::RunEnv::get()->{network}->{interfaces};
my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
index 6401174..f040881 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -1,8 +1,9 @@
+use anyhow::{Result, anyhow, bail};
use cursive::{
Cursive, View,
view::{Nameable, Resizable, ViewWrapper},
views::{
- Button, Checkbox, Dialog, DummyView, EditView, LinearLayout, NamedView, ResizedView,
+ Button, Checkbox, Dialog, DummyView, EditView, LinearLayout, NamedView, Panel, ResizedView,
ScrollView, SelectView, TextView,
},
};
@@ -22,10 +23,10 @@ use proxmox_network_types::{MacAddress, fqdn::Fqdn, ip_address::Cidr};
use super::{CidrAddressEditView, FormView};
struct NetworkViewOptions {
- selected_mac: MacAddress,
pinning_enabled: bool,
- // For UI purposes, we want to always save the mapping, to save the state
- // between toggling the checkbox
+ interfaces: Vec<Interface>,
+ // 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,
}
@@ -40,32 +41,29 @@ pub struct NetworkOptionsView {
}
impl NetworkOptionsView {
- const PINNING_OPTIONS_BUTTON_NAME: &str = "network-pinning-options-button";
+ const FORM_VIEW_NAME: &str = "network-management-form";
const MGMT_IFNAME_SELECTVIEW_NAME: &str = "network-management-ifname-selectview";
+ const ADVANCED_OPTIONS_BUTTON_NAME: &str = "network-pinning-options-button";
+
+ const INTERFACE_PINNING_VIEW_NAME: &str = "network-name-pinning-view";
pub fn new(options: &NetworkOptions, network_info: &NetworkInfo) -> Self {
- let mut ifaces = network_info
+ let mut interfaces = network_info
.interfaces
.values()
- .collect::<Vec<&Interface>>();
+ .cloned()
+ .collect::<Vec<Interface>>();
// First, sort interfaces by their link state and then name
- ifaces.sort_unstable_by_key(|x| (&x.state, &x.name));
-
- let selected_mac = network_info
- .interfaces
- .get(&options.ifname)
- .map(|iface| iface.mac)
- .unwrap_or_else(|| ifaces.first().expect("at least one network interface").mac);
+ interfaces.sort_unstable_by(|a, b| (&a.state, &a.name).cmp(&(&b.state, &b.name)));
let options_ref = Arc::new(Mutex::new(NetworkViewOptions {
- selected_mac,
pinning_enabled: options.pinning_opts.is_some(),
+ interfaces,
pinning_options: options.pinning_opts.clone().unwrap_or_default(),
}));
- let iface_selection =
- Self::build_mgmt_ifname_selectview(ifaces.clone(), options_ref.clone());
+ let iface_selection = Self::build_mgmt_ifname_selectview(options_ref.clone(), None);
let form = FormView::<()>::new()
.child(
@@ -89,36 +87,13 @@ impl NetworkOptionsView {
EditView::new().content(options.dns_server.to_string()),
);
- 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())
+ let advanced_options = LinearLayout::horizontal()
.child(DummyView.full_width())
.child(
- Button::new("Pinning options", {
+ Button::new("Advanced 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());
+ let mut view = Self::advanced_options_view(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.
@@ -128,13 +103,13 @@ impl NetworkOptionsView {
siv.add_layer(view);
}
})
- .with_name(Self::PINNING_OPTIONS_BUTTON_NAME),
+ .with_name(Self::ADVANCED_OPTIONS_BUTTON_NAME),
);
let view = LinearLayout::vertical()
- .child(form)
+ .child(form.with_name(Self::FORM_VIEW_NAME))
.child(DummyView.full_width())
- .child(pinning_checkbox);
+ .child(advanced_options);
Self {
view,
@@ -145,13 +120,10 @@ impl NetworkOptionsView {
pub fn get_values(&mut self) -> Result<NetworkOptions, String> {
let form = self
.view
- .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")?;
+ .get_child_mut(0)
+ .and_then(|v| v.downcast_mut::<NamedView<FormView>>())
+ .ok_or("failed to retrieve network options form")?
+ .get_mut();
let fqdn = form
.get_value::<EditView, _>(1)
@@ -208,83 +180,69 @@ impl NetworkOptionsView {
}
}
- fn custom_name_mapping_view(
- network_info: &NetworkInfo,
- options_ref: NetworkViewOptionsRef,
- ) -> impl View {
- const DIALOG_NAME: &str = "network-interface-name-pinning-dialog";
+ 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),
+ )),
+ );
- 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;
+ Dialog::around(inner)
+ .title("Advanced Network Options")
+ .button("Ok", {
+ move |siv| {
+ match Self::get_view_values(siv) {
+ Ok(pin_opts) => {
+ siv.pop_layer();
+ let options = &mut options_ref.lock().expect("unpoisoned lock");
+ options.pinning_options = pin_opts;
+ }
+ Err(err) => {
+ siv.pop_layer();
+ siv.add_layer(Dialog::info(err.to_string()));
+ 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))
+ Self::refresh_ifname_selectview(siv, options_ref.clone());
+ }
+ })
+ .max_size((80, 40))
}
- fn refresh_ifname_selectview(
- siv: &mut Cursive,
- ifaces: &[Interface],
- options_ref: NetworkViewOptionsRef,
- ) {
+ fn get_view_values(siv: &mut Cursive) -> Result<NetworkInterfacePinningOptions> {
+ let pin_opts = siv
+ .call_on_name(
+ Self::INTERFACE_PINNING_VIEW_NAME,
+ InterfacePinningOptionsView::get_values,
+ )
+ .unwrap_or_else(|| {
+ bail!("failed to retrieve network interface name pinning options")
+ })?;
+
+ Ok(pin_opts)
+ }
+
+ fn refresh_ifname_selectview(siv: &mut Cursive, 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);
+ let selected_mac = view.selection().map(|iface| iface.mac);
+ *view = Self::build_mgmt_ifname_selectview(options_ref, selected_mac);
},
);
}
fn build_mgmt_ifname_selectview(
- ifaces: Vec<&Interface>,
options_ref: NetworkViewOptionsRef,
+ selected_mac: Option<MacAddress>,
) -> 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
+ let ifnames = options
+ .interfaces
.iter()
.map(|iface| {
if options.pinning_enabled
@@ -297,20 +255,20 @@ impl NetworkOptionsView {
})
.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;
- }
- });
+ 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
+ });
// Finally, (try to) select the current one
let selected = view
.iter()
- .position(|(_label, iface)| iface.mac == options.selected_mac)
+ .position(|(_label, iface)| iface.mac == selected_mac)
.unwrap_or(0); // we sort UP interfaces first, so select the first UP interface
//
view.set_selection(selected);
@@ -324,19 +282,44 @@ impl ViewWrapper for NetworkOptionsView {
}
struct InterfacePinningOptionsView {
- view: ScrollView<NamedView<FormView<MacAddress>>>,
+ view: LinearLayout,
}
impl InterfacePinningOptionsView {
+ const ENABLED_CHECKBOX_NAME: &str = "network-interface-name-pinning-enabled";
const FORM_NAME: &str = "network-interface-name-pinning-form";
- fn new(interfaces: &[&Interface], options_ref: NetworkViewOptionsRef) -> Self {
+ fn new(options_ref: NetworkViewOptionsRef) -> Self {
let options = options_ref.lock().expect("unpoisoned lock");
// Filter out all non-physical links, as it does not make sense to pin their names
// in this way.
// The low-level installer will skip them anyway.
- let interfaces = interfaces.iter().filter(|iface| iface.pinned_id.is_some());
+ let interfaces = options
+ .interfaces
+ .iter()
+ .filter(|iface| iface.pinned_id.is_some());
+
+ let mut view = LinearLayout::vertical()
+ .child(
+ LinearLayout::horizontal()
+ .child(
+ Checkbox::new()
+ .with_checked(options.pinning_enabled)
+ .on_change({
+ let options_ref = options_ref.clone();
+ move |siv, enabled| {
+ options_ref.lock().expect("unpoisoned lock").pinning_enabled =
+ enabled;
+
+ Self::toggle_all(siv, enabled);
+ }
+ })
+ .with_name(Self::ENABLED_CHECKBOX_NAME),
+ )
+ .child(TextView::new(" Pin network interface names").no_wrap()),
+ )
+ .child(DummyView::new());
let mut form = FormView::<MacAddress>::new();
@@ -356,7 +339,7 @@ impl InterfacePinningOptionsView {
.expect("always pinnable interface")
.name,
)
- .max_content_width(MAX_IFNAME_LEN)
+ .with_enabled(options.pinning_enabled)
.fixed_width(MAX_IFNAME_LEN),
);
@@ -375,13 +358,31 @@ impl InterfacePinningOptionsView {
}
}
- Self {
- view: ScrollView::new(form.with_name(Self::FORM_NAME)),
- }
+ view.add_child(form.with_name(Self::FORM_NAME));
+
+ Self { view }
}
- fn get_values(&mut self) -> Result<NetworkInterfacePinningOptions, String> {
- let form = self.view.get_inner_mut().get_mut();
+ fn toggle_all(siv: &mut Cursive, enabled: bool) {
+ siv.call_on_name(Self::FORM_NAME, |v: &mut FormView<MacAddress>| {
+ v.call_on_childs(&|v: &mut LinearLayout| {
+ if let Some(v) = v
+ .get_child_mut(1)
+ .and_then(|v| v.downcast_mut::<ResizedView<EditView>>())
+ {
+ v.get_inner_mut().set_enabled(enabled);
+ }
+ });
+ });
+ }
+
+ fn get_values(&mut self) -> Result<NetworkInterfacePinningOptions> {
+ let form = self
+ .view
+ .get_child_mut(2)
+ .and_then(|v| v.downcast_mut::<NamedView<FormView<MacAddress>>>())
+ .ok_or(anyhow!("failed to retrieve interface pinning form"))?
+ .get_mut();
let mut mapping = HashMap::new();
@@ -395,18 +396,21 @@ impl InterfacePinningOptionsView {
.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))?;
+ .ok_or(anyhow!(
+ "failed to retrieve pinning ID for interface {}",
+ mac
+ ))?;
mapping.insert(*mac, (*name).clone());
}
let opts = NetworkInterfacePinningOptions { mapping };
- opts.verify().map_err(|err| err.to_string())?;
+ opts.verify()?;
Ok(opts)
}
}
impl ViewWrapper for InterfacePinningOptionsView {
- cursive::wrap_impl!(self.view: ScrollView<NamedView<FormView<MacAddress>>>);
+ cursive::wrap_impl!(self.view: LinearLayout);
}
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 17/23] tui: network: match form label style when enabling/disabling elements
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (15 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 16/23] tui: network: factor out name pinning into advanced options dialog Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 18/23] tui: network: add bond setup options to advanced network dialog Christoph Heiss
` (5 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
This follows the GUI more closely by also changing the style of the
labels to make them the same coloring as disabled interactive form
elements.
Suggested-by: Lukas Sichert <l.sichert@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* new patch
proxmox-tui-installer/src/views/mod.rs | 27 ++++++++++++++++++++++
proxmox-tui-installer/src/views/network.rs | 1 +
2 files changed, 28 insertions(+)
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index a73dde3..3a6a00d 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -520,6 +520,33 @@ impl<UDT> FormView<UDT> {
}
}
+ pub fn set_labels_enabled(&mut self, enabled: bool) {
+ let style = if enabled {
+ StyleType::primary()
+ } else {
+ StyleType::secondary()
+ };
+
+ let len = self.len();
+ let view = self
+ .view
+ .get_child_mut(0)
+ .and_then(|v| v.downcast_mut::<ResizedView<LinearLayout>>())
+ .map(|v| v.get_inner_mut());
+
+ if let Some(view) = view {
+ for i in 0..len {
+ let label = view
+ .get_child_mut(i)
+ .and_then(|v| v.downcast_mut::<TextView>());
+
+ if let Some(label) = label {
+ label.set_style(style);
+ }
+ }
+ }
+ }
+
pub fn len(&self) -> usize {
self.view
.get_child(1)
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
index f040881..b8f938c 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -365,6 +365,7 @@ impl InterfacePinningOptionsView {
fn toggle_all(siv: &mut Cursive, enabled: bool) {
siv.call_on_name(Self::FORM_NAME, |v: &mut FormView<MacAddress>| {
+ v.set_labels_enabled(enabled);
v.call_on_childs(&|v: &mut LinearLayout| {
if let Some(v) = v
.get_child_mut(1)
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 18/23] tui: network: add bond setup options to advanced network dialog
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (16 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 17/23] tui: network: match form label style when enabling/disabling elements Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 19/23] tui: network: add vlan " Christoph Heiss
` (4 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
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 <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* also toggle form labels with the state
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* new patch
proxmox-tui-installer/Cargo.toml | 1 +
proxmox-tui-installer/src/main.rs | 7 +-
proxmox-tui-installer/src/options.rs | 47 ++-
proxmox-tui-installer/src/setup.rs | 15 +-
proxmox-tui-installer/src/views/network.rs | 422 ++++++++++++++++++---
5 files changed, 441 insertions(+), 51 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..6fcdecb 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<SummaryOption> {
+ pub fn to_summary(
+ &self,
+ runtime_info: &RuntimeInfo,
+ locales: &LocaleInfo,
+ ) -> Vec<SummaryOption> {
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,36 @@ 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()),
]
}
+
+ /// Resolves all fixed bond members into their human-readable name, either the pinned one or
+ /// kernel-assigned one, if name pinning is enabled or not respectively.
+ fn resolve_bond_member_names(
+ &self,
+ bond: &NetworkBondOptions,
+ network: &NetworkInfo,
+ ) -> Vec<String> {
+ 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("<unknown>".to_owned())
+ })
+ .collect()
+ }
}
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index ae4b717..aecbe10 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -3,7 +3,7 @@ use std::collections::BTreeMap;
use crate::options::InstallerOptions;
use proxmox_installer_common::{
options::AdvancedBootdiskOptions,
- setup::{InstallConfig, InstallFirstBootSetup, InstallRootPassword},
+ setup::{InstallConfig, InstallFirstBootSetup, InstallNetworkBondOptions, InstallRootPassword},
};
impl From<InstallerOptions> for InstallConfig {
@@ -35,6 +35,19 @@ impl From<InstallerOptions> for InstallConfig {
subscription_key: None,
mngmt_nic: options.network.ifname,
+ mngmt_bond: options
+ .network
+ .bond_opts
+ .map(|bond| {
+ InstallNetworkBondOptions {
+ // We never use the udev filter here, so no need to pay attention to that.
+ members: bond.members,
+ mode: bond.mode,
+ hash_policy: bond.hash_policy,
+ primary_interface: bond.primary_interface,
+ }
+ })
+ .unwrap_or_default(),
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 b8f938c..785f957 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -8,7 +8,7 @@ use cursive::{
},
};
use std::{
- collections::HashMap,
+ collections::{BTreeMap, HashMap},
net::IpAddr,
sync::{Arc, Mutex},
};
@@ -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<Interface>,
+ /// 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,27 @@ 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_default(),
}));
- 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 +171,26 @@ impl NetworkOptionsView {
.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 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::<NamedView<SelectView<Interface>>, _>(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 +207,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<NetworkInterfacePinningOptions> {
+ 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 +261,46 @@ 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<Interface>| {
- 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())
+ .with_name(Self::MGMT_IFNAME_SELECTVIEW_NAME),
+ );
+ }
+ });
}
- fn build_mgmt_ifname_selectview(
- options_ref: NetworkViewOptionsRef,
- selected_mac: Option<MacAddress>,
- ) -> SelectView<Interface> {
+ fn build_mgmt_ifname_selectview(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
@@ -255,20 +318,21 @@ impl NetworkOptionsView {
})
.collect::<Vec<(String, Interface)>>();
- 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 +377,7 @@ impl InterfacePinningOptionsView {
enabled;
Self::toggle_all(siv, enabled);
+ BondOptionsView::refresh_ifnames(siv, options_ref.clone());
}
})
.with_name(Self::ENABLED_CHECKBOX_NAME),
@@ -415,3 +480,270 @@ 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::<MacAddress>::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| {
+ form.set_labels_enabled(enabled);
+
+ if let Some(v) = form.get_child_mut::<SelectView<LinuxBondMode>>(0) {
+ v.set_enabled(enabled);
+ }
+ if let Some(v) = form.get_child_mut::<SelectView<BondXmitHashPolicy>>(1) {
+ v.set_enabled(enabled);
+ }
+ if let Some(v) = form.get_child_mut::<EditView>(2) {
+ v.set_enabled(enabled);
+ }
+ });
+
+ siv.call_on_name(
+ Self::MEMBERS_VIEW_NAME,
+ |group: &mut CheckboxGroup<MacAddress>| {
+ 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::<SelectView<BondXmitHashPolicy>>(1) {
+ v.set_enabled(Self::hash_policy_view_enabled(*selected));
+ }
+ if let Some(v) = form.get_child_mut::<EditView>(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<MacAddress>| {
+ 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<NetworkBondOptions> {
+ let form = self
+ .view
+ .get_child_mut(2)
+ .and_then(|v| v.downcast_mut::<NamedView<FormView<()>>>())
+ .ok_or(anyhow!("failed to retrieve bond options form"))?
+ .get_mut();
+
+ let mode = form
+ .get_value::<SelectView<LinuxBondMode>, _>(0)
+ .ok_or(anyhow!("failed to retrieve bond mode"))?;
+
+ let hash_policy = form
+ .get_value::<SelectView<BondXmitHashPolicy>, _>(1)
+ .ok_or(anyhow!("failed to retrieve bond hash policy"))?;
+
+ let primary_interface = form
+ .get_value::<EditView, String>(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::<NamedView<CheckboxGroup<MacAddress>>>())
+ .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,
+ filter: BTreeMap::new(),
+ filter_match: None,
+ 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<MacAddress> {
+ 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.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 19/23] tui: network: add vlan setup options to advanced network dialog
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (17 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 18/23] tui: network: add bond setup options to advanced network dialog Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 20/23] gui: network: split advanced options dialog into tabs Christoph Heiss
` (3 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Fixes #2164 [0].
Adds a third panel to the advanced network options dialog, allowing the
user to set a VLAN ID for the management interface.
[0] https://bugzilla.proxmox.com/show_bug.cgi?id=2164
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* also toggle form labels with the state
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* no changes
Changes v1 -> v2:
* new patch
proxmox-tui-installer/src/main.rs | 1 +
proxmox-tui-installer/src/options.rs | 6 +-
proxmox-tui-installer/src/setup.rs | 5 +
proxmox-tui-installer/src/views/mod.rs | 35 +++++--
proxmox-tui-installer/src/views/network.rs | 116 ++++++++++++++++++++-
5 files changed, 151 insertions(+), 12 deletions(-)
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 6e7c691..cd3b0ec 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -178,6 +178,7 @@ fn main() {
// We enable network interface pinning by default in the TUI
Some(&NetworkInterfacePinningOptions::default()),
None,
+ None,
),
autoreboot: true,
},
diff --git a/proxmox-tui-installer/src/options.rs b/proxmox-tui-installer/src/options.rs
index 6fcdecb..fc54b99 100644
--- a/proxmox-tui-installer/src/options.rs
+++ b/proxmox-tui-installer/src/options.rs
@@ -42,7 +42,7 @@ impl InstallerOptions {
.map(|l| &l.name)
.unwrap_or(&self.timezone.kb_layout);
- let mngmt_interface = if let Some(opts) = &self.network.bond_opts {
+ let mut mngmt_interface = if let Some(opts) = &self.network.bond_opts {
let members = self
.resolve_bond_member_names(opts, &runtime_info.network)
.join(" | ");
@@ -52,6 +52,10 @@ impl InstallerOptions {
self.network.ifname.to_owned()
};
+ if let Some(opts) = &self.network.vlan_opts {
+ mngmt_interface += &format!(", VLAN {}", opts.vid);
+ }
+
vec![
SummaryOption::new("Bootdisk filesystem", self.bootdisk.fstype.to_string()),
SummaryOption::new(
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index aecbe10..9410bfe 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -48,6 +48,11 @@ impl From<InstallerOptions> for InstallConfig {
}
})
.unwrap_or_default(),
+ mngmt_vlan: options
+ .network
+ .vlan_opts
+ .map(|opts| opts.into())
+ .unwrap_or_default(),
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/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 3a6a00d..760b467 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -31,6 +31,7 @@ pub use timezone::*;
pub struct NumericEditView<T> {
view: LinearLayout,
+ min_value: Option<T>,
max_value: Option<T>,
placeholder: Option<T>,
max_content_width: Option<usize>,
@@ -44,6 +45,7 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
Self {
view,
+ min_value: None,
max_value: None,
placeholder: None,
max_content_width: None,
@@ -63,6 +65,7 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
Self {
view,
+ min_value: None,
max_value: None,
placeholder: None,
max_content_width: None,
@@ -70,6 +73,16 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
}
}
+ pub fn enabled(mut self, enabled: bool) -> Self {
+ self.inner_mut().set_enabled(enabled);
+ self
+ }
+
+ pub fn min_value(mut self, min: T) -> Self {
+ self.min_value = Some(min);
+ self
+ }
+
pub fn max_value(mut self, max: T) -> Self {
self.max_value = Some(max);
self
@@ -139,13 +152,23 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
self.max_value = Some(max);
}
+ pub fn set_enabled(&mut self, enabled: bool) {
+ self.inner_mut().set_enabled(enabled);
+ }
+
+ /// Checks if the new value is actually valid according to the min and max value, if set
fn check_bounds(&mut self, original: Arc<String>, result: EventResult) -> EventResult {
- // Check if the new value is actually valid according to the max value, if set
- if let Some(max) = self.max_value
- && let Ok(val) = self.get_content()
- && result.is_consumed()
- && val > max
- {
+ let above_min = self
+ .min_value
+ .map(|min| self.get_content().map(|val| val >= min).unwrap_or(true))
+ .unwrap_or(true);
+
+ let below_max = self
+ .max_value
+ .map(|max| self.get_content().map(|val| val <= max).unwrap_or(true))
+ .unwrap_or(true);
+
+ if result.is_consumed() && (!above_min || !below_max) {
// Restore the original value, before the insert
let cb = self.inner_mut().set_content((*original).clone());
return EventResult::with_cb_once(move |siv| {
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
index 785f957..929cb9c 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -18,15 +18,16 @@ use proxmox_installer_common::{
options::{NetworkInterfacePinningOptions, NetworkOptions},
setup::{Interface, NetworkInfo},
};
-use proxmox_installer_types::answer::NetworkBondOptions;
+use proxmox_installer_types::answer::{NetworkBondOptions, NetworkVlanOptions};
use proxmox_network_api::{BondXmitHashPolicy, LinuxBondMode};
use proxmox_network_types::{MacAddress, fqdn::Fqdn, ip_address::Cidr};
-use super::{CheckboxGroup, CidrAddressEditView, FormView};
+use super::{CheckboxGroup, CidrAddressEditView, FormView, IntegerEditView};
struct NetworkViewOptions {
pinning_enabled: bool,
bond_enabled: bool,
+ vlan_enabled: bool,
interfaces: Vec<Interface>,
/// Last selected interface (by its MAC address) in the [SelectView].
/// Keeps track of the selected interface across enabling/disabling bonding, which replaces the
@@ -36,6 +37,7 @@ struct NetworkViewOptions {
// by the user - to preserve the state between toggling the checkbox
pinning_options: NetworkInterfacePinningOptions,
bond_options: NetworkBondOptions,
+ vlan_options: NetworkVlanOptions,
}
/// Convenience wrapper when needing to take a (interior-mutable) reference to
@@ -55,6 +57,7 @@ impl NetworkOptionsView {
const INTERFACE_PINNING_VIEW_NAME: &str = "network-name-pinning-view";
const BOND_OPTIONS_VIEW_NAME: &str = "network-bond-options-view";
+ const VLAN_OPTIONS_VIEW_NAME: &str = "network-vlan-options-view";
pub fn new(options: &NetworkOptions, network_info: &NetworkInfo) -> Self {
let mut interfaces = network_info
@@ -80,10 +83,15 @@ impl NetworkOptionsView {
let options_ref = Arc::new(Mutex::new(NetworkViewOptions {
pinning_enabled: options.pinning_opts.is_some(),
bond_enabled: options.bond_opts.is_some(),
+ vlan_enabled: options.vlan_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_default(),
+ vlan_options: options
+ .vlan_opts
+ .clone()
+ .unwrap_or(NetworkVlanOptions { vid: 0 }),
}));
let iface_selection = Self::build_mgmt_ifname_selectview(options_ref.clone());
@@ -178,6 +186,7 @@ impl NetworkOptionsView {
.then_some(options.pinning_options.clone());
let bond_opts = options.bond_enabled.then_some(options.bond_options.clone());
+ let vlan_opts = options.vlan_enabled.then_some(options.vlan_options.clone());
let ifname = if options.bond_enabled {
"bond0".to_owned()
@@ -208,6 +217,7 @@ impl NetworkOptionsView {
dns_server,
pinning_opts,
bond_opts,
+ vlan_opts,
})
}
}
@@ -222,6 +232,10 @@ impl NetworkOptionsView {
.child(Panel::new(
BondOptionsView::new(options_ref.clone())
.with_name(Self::BOND_OPTIONS_VIEW_NAME),
+ ))
+ .child(Panel::new(
+ VlanOptionsView::new(options_ref.clone())
+ .with_name(Self::VLAN_OPTIONS_VIEW_NAME),
)),
);
@@ -230,11 +244,12 @@ impl NetworkOptionsView {
.button("Ok", {
move |siv| {
match Self::get_view_values(siv, options_ref.clone()) {
- Ok((pin_opts, bond_opts)) => {
+ Ok((pin_opts, bond_opts, vlan_opts)) => {
siv.pop_layer();
let options = &mut options_ref.lock().expect("unpoisoned lock");
options.pinning_options = pin_opts;
options.bond_options = bond_opts;
+ options.vlan_options = vlan_opts;
}
Err(err) => {
siv.add_layer(Dialog::info(err.to_string()));
@@ -251,7 +266,11 @@ impl NetworkOptionsView {
fn get_view_values(
siv: &mut Cursive,
options_ref: NetworkViewOptionsRef,
- ) -> Result<(NetworkInterfacePinningOptions, NetworkBondOptions)> {
+ ) -> Result<(
+ NetworkInterfacePinningOptions,
+ NetworkBondOptions,
+ NetworkVlanOptions,
+ )> {
let pin_opts = siv
.call_on_name(
Self::INTERFACE_PINNING_VIEW_NAME,
@@ -273,7 +292,19 @@ impl NetworkOptionsView {
bond_opts.verify()?;
}
- Ok((pin_opts, bond_opts))
+ let vlan_opts = siv
+ .call_on_name(Self::VLAN_OPTIONS_VIEW_NAME, VlanOptionsView::get_values)
+ .unwrap_or_else(|| bail!("failed to retrieve management network VLAN options"))?;
+
+ if options_ref
+ .lock()
+ .map(|opts| opts.vlan_enabled)
+ .unwrap_or(false)
+ {
+ vlan_opts.verify()?;
+ }
+
+ Ok((pin_opts, bond_opts, vlan_opts))
}
fn refresh_ifname_view(siv: &mut Cursive, options_ref: NetworkViewOptionsRef) {
@@ -747,3 +778,78 @@ impl BondOptionsView {
impl ViewWrapper for BondOptionsView {
cursive::wrap_impl!(self.view: LinearLayout);
}
+
+struct VlanOptionsView {
+ view: LinearLayout,
+}
+
+impl VlanOptionsView {
+ const FORM_NAME: &str = "network-vlan-form-view";
+
+ fn new(options_ref: NetworkViewOptionsRef) -> Self {
+ let options = &options_ref.lock().expect("unpoisoned lock");
+
+ let view = LinearLayout::vertical()
+ .child(
+ LinearLayout::horizontal()
+ .child(
+ Checkbox::new()
+ .with_checked(options.vlan_enabled)
+ .on_change({
+ let options_ref = options_ref.clone();
+ move |siv, enabled| {
+ options_ref.lock().expect("unpoisoned lock").vlan_enabled =
+ enabled;
+ Self::toggle_all(siv, enabled);
+ }
+ }),
+ )
+ .child(TextView::new(" Enable VLAN setup on management interface").no_wrap()),
+ )
+ .child(DummyView::new())
+ .child(
+ FormView::<()>::new()
+ .child(
+ "VLAN ID",
+ IntegerEditView::new()
+ .min_value(1)
+ .max_value(4094)
+ .content(options.vlan_options.vid as usize)
+ .enabled(options.vlan_enabled),
+ )
+ .with_name(Self::FORM_NAME),
+ );
+
+ Self { view }
+ }
+
+ fn toggle_all(siv: &mut Cursive, enabled: bool) {
+ siv.call_on_name(Self::FORM_NAME, |form: &mut FormView| {
+ form.set_labels_enabled(enabled);
+ if let Some(v) = form.get_child_mut::<IntegerEditView>(0) {
+ v.set_enabled(enabled);
+ }
+ });
+ }
+
+ fn get_values(&mut self) -> Result<NetworkVlanOptions> {
+ let form = self
+ .view
+ .get_child_mut(2)
+ .and_then(|v| v.downcast_mut::<NamedView<FormView<()>>>())
+ .ok_or(anyhow!("failed to retrieve bond options form"))?
+ .get_mut();
+
+ let vid = form
+ .get_value::<IntegerEditView, _>(0)
+ .ok_or(anyhow!("failed to retrieve VLAN ID"))?;
+
+ Ok(NetworkVlanOptions {
+ vid: vid.try_into()?,
+ })
+ }
+}
+
+impl ViewWrapper for VlanOptionsView {
+ cursive::wrap_impl!(self.view: LinearLayout);
+}
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 20/23] gui: network: split advanced options dialog into tabs
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (18 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 19/23] tui: network: add vlan " Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 21/23] tui: " Christoph Heiss
` (2 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
With the bond and VLAN options, the advanced network options dialog
stacks three sections, and both the pinning and the bond section list
every physical network interface. Servers with six or more ports, for
example two each of 1G, 10G and 100G, are not unusual, which makes the
dialog taller than smaller screen resolutions can show.
Put each section on its own tab and limit the page height relative to
the screen height, so that a page with many interfaces scrolls instead
of pushing the dialog buttons off the screen. If an input is invalid,
switch to the tab that contains it before showing the error, as it
could otherwise be on a page that is not visible.
Originally-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* new patch
proxinstall | 80 ++++++++++++++++++++++++++---------------------------
1 file changed, 40 insertions(+), 40 deletions(-)
diff --git a/proxinstall b/proxinstall
index 079b227..6b8c7e8 100755
--- a/proxinstall
+++ b/proxinstall
@@ -381,13 +381,10 @@ my $create_label_widget_grid = sub {
return $grid;
};
-my sub create_network_interface_pin_frame {
+my sub create_network_interface_pin_page {
my ($on_enable_changed) = @_;
- my $frame = Gtk3::Frame->new('Interface Name Pinning');
-
my $vbox = Gtk3::Box->new('vertical', 5);
- $frame->add($vbox);
$vbox->set_margin_start(10);
$vbox->set_margin_end(10);
$vbox->set_margin_top(5);
@@ -468,14 +465,11 @@ my sub create_network_interface_pin_frame {
Proxmox::Install::Config::set_network_interface_pin_map(\%new_mapping);
};
- return ($frame, $on_done);
+ return ($vbox, $on_done);
}
-my sub create_network_bond_frame {
- my $frame = Gtk3::Frame->new('Management Bond');
-
+my sub create_network_bond_page {
my $vbox = Gtk3::Box->new('vertical', 5);
- $frame->add($vbox);
$vbox->set_margin_start(10);
$vbox->set_margin_end(10);
$vbox->set_margin_top(5);
@@ -483,7 +477,7 @@ my sub create_network_bond_frame {
my $enable_checkbox = Gtk3::CheckButton->new('Enable bonding for management interface');
$enable_checkbox->set_active($gtk_state->{network_bonding_enabled});
- $vbox->pack_start($enable_checkbox, 1, 0, 5);
+ $vbox->pack_start($enable_checkbox, 0, 0, 5);
my $bond_opts = Proxmox::Install::Config::get_mngmt_bond_opt();
my $interfaces = Proxmox::Install::RunEnv::get()->{network}->{interfaces};
@@ -639,14 +633,11 @@ my sub create_network_bond_frame {
Proxmox::Install::Config::set_mngmt_bond_opt('primary_interface', $primary_mac);
};
- return ($frame, $refresh_if_names, $on_done);
+ return ($vbox, $refresh_if_names, $on_done);
}
-my sub create_network_vlan_frame {
- my $frame = Gtk3::Frame->new('Management VLAN');
-
+my sub create_network_vlan_page {
my $vbox = Gtk3::Box->new('vertical', 5);
- $frame->add($vbox);
$vbox->set_margin_start(10);
$vbox->set_margin_end(10);
$vbox->set_margin_top(5);
@@ -654,7 +645,7 @@ my sub create_network_vlan_frame {
my $enable_checkbox = Gtk3::CheckButton->new('Enable VLAN setup on management interface');
$enable_checkbox->set_active($gtk_state->{network_vlan_enabled});
- $vbox->pack_start($enable_checkbox, 1, 0, 5);
+ $vbox->pack_start($enable_checkbox, 0, 0, 5);
my $vlan_opts = Proxmox::Install::Config::get_mngmt_vlan_opt();
@@ -697,7 +688,7 @@ my sub create_network_vlan_frame {
Proxmox::Install::Config::set_mngmt_vlan_opt('enabled', 1);
};
- return ($frame, $on_done);
+ return ($vbox, $on_done);
}
my sub create_network_advanced_options_view {
@@ -709,37 +700,46 @@ my sub create_network_advanced_options_view {
my $content = $dialog->get_content_area();
- my $scrolled_window = Gtk3::ScrolledWindow->new();
- $scrolled_window->set_hexpand(1);
- $scrolled_window->set_vexpand(1);
- $scrolled_window->set_propagate_natural_height(1);
- $scrolled_window->set_policy('never', 'automatic');
- $scrolled_window->set_min_content_height(200);
+ my ($bond_page, $bond_refresh, $bond_on_done) = create_network_bond_page();
+ my ($pin_page, $pin_on_done) = create_network_interface_pin_page($bond_refresh);
+ my ($vlan_page, $vlan_on_done) = create_network_vlan_page();
- my $hbox = Gtk3::Box->new('vertical', 0);
+ # leave room for the title, tabs and buttons, so the dialog also fits smaller screens
+ my $max_page_height = Gtk3::Gdk::Screen::get_default()->get_height() - 250;
+ $max_page_height = 200 if $max_page_height < 200;
- my ($bond_frame, $bond_refresh, $bond_on_done) = create_network_bond_frame();
- my ($pin_frame, $pin_on_done) = create_network_interface_pin_frame($bond_refresh);
- my ($vlan_frame, $vlan_on_done) = create_network_vlan_frame();
+ my $notebook = Gtk3::Notebook->new();
+ my @pages = (
+ ['Interface Name Pinning', $pin_page, $pin_on_done],
+ ['Management Bond', $bond_page, $bond_on_done],
+ ['Management VLAN', $vlan_page, $vlan_on_done],
+ );
+ for my $page (@pages) {
+ my ($title, $widget) = $page->@*;
- $hbox->pack_start($pin_frame, 0, 0, 10);
- $hbox->pack_start($bond_frame, 0, 0, 10);
- $hbox->pack_start($vlan_frame, 0, 0, 10);
+ my $scrolled_window = Gtk3::ScrolledWindow->new();
+ $scrolled_window->set_hexpand(1);
+ $scrolled_window->set_vexpand(1);
+ $scrolled_window->set_propagate_natural_height(1);
+ $scrolled_window->set_policy('never', 'automatic');
+ $scrolled_window->set_min_content_height(200);
+ $scrolled_window->set_max_content_height($max_page_height);
+ $scrolled_window->add($widget);
- $scrolled_window->add($hbox);
- $content->pack_start($scrolled_window, 1, 1, 10);
+ $notebook->append_page($scrolled_window, Gtk3::Label->new($title));
+ }
+ $content->pack_start($notebook, 1, 1, 10);
my $inputs = {};
$dialog->signal_connect(
response => sub {
- eval {
- $pin_on_done->();
- $bond_on_done->();
- $vlan_on_done->();
- };
- if ($@) {
- Proxmox::UI::message($@, $dialog);
- return;
+ for my $i (0 .. $#pages) {
+ eval { $pages[$i]->[2]->() };
+ if (my $err = $@) {
+ $notebook->set_current_page($i);
+ Proxmox::UI::message($err, $dialog);
+ return;
+ }
}
$dialog->destroy();
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 21/23] tui: network: split advanced options dialog into tabs
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (19 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 20/23] gui: network: split advanced options dialog into tabs Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 22/23] gui: network: list bond member candidates sharing a MAC address once Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 23/23] tui: " Christoph Heiss
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
With the bond and VLAN options, the advanced network options dialog
stacks three sections, and both the pinning and the bond section list
every physical network interface. Servers with six or more ports are
not unusual, so the dialog needs a lot of scrolling to get through, in
particular on an 80x24 console.
Put each section on its own tab, as in the graphical installer, and let
each tab scroll on its own. If an input is invalid, switch to the tab
that contains it before showing the error, as it could otherwise be on
a tab that is not visible.
Originally-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* new patch
proxmox-tui-installer/src/views/network.rs | 85 +++++++++++++------
.../src/views/tabbed_view.rs | 7 ++
2 files changed, 66 insertions(+), 26 deletions(-)
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
index 929cb9c..4a5d334 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -3,8 +3,8 @@ use cursive::{
Cursive, View,
view::{Nameable, Resizable, ViewWrapper},
views::{
- Button, Checkbox, Dialog, DummyView, EditView, LinearLayout, NamedView, Panel, ResizedView,
- ScrollView, SelectView, TextView,
+ Button, Checkbox, Dialog, DummyView, EditView, LinearLayout, NamedView, PaddedView,
+ ResizedView, ScrollView, SelectView, TextView,
},
};
use std::{
@@ -22,7 +22,7 @@ use proxmox_installer_types::answer::{NetworkBondOptions, NetworkVlanOptions};
use proxmox_network_api::{BondXmitHashPolicy, LinuxBondMode};
use proxmox_network_types::{MacAddress, fqdn::Fqdn, ip_address::Cidr};
-use super::{CheckboxGroup, CidrAddressEditView, FormView, IntegerEditView};
+use super::{CheckboxGroup, CidrAddressEditView, FormView, IntegerEditView, TabbedView};
struct NetworkViewOptions {
pinning_enabled: bool,
@@ -58,6 +58,12 @@ impl NetworkOptionsView {
const INTERFACE_PINNING_VIEW_NAME: &str = "network-name-pinning-view";
const BOND_OPTIONS_VIEW_NAME: &str = "network-bond-options-view";
const VLAN_OPTIONS_VIEW_NAME: &str = "network-vlan-options-view";
+ const ADVANCED_OPTIONS_TABS_NAME: &str = "network-advanced-options-tabs";
+
+ // tab indices in the advanced options dialog
+ const INTERFACE_PINNING_TAB: usize = 0;
+ const BOND_OPTIONS_TAB: usize = 1;
+ const VLAN_OPTIONS_TAB: usize = 2;
pub fn new(options: &NetworkOptions, network_info: &NetworkInfo) -> Self {
let mut interfaces = network_info
@@ -223,23 +229,35 @@ impl NetworkOptionsView {
}
fn advanced_options_view(options_ref: NetworkViewOptionsRef) -> impl View {
- let inner = ScrollView::new(
- LinearLayout::vertical()
- .child(Panel::new(
+ // with many interfaces, the pinning and bond pages can be taller than the screen
+ fn page(view: impl View) -> PaddedView<ScrollView<impl View>> {
+ PaddedView::lrtb(1, 1, 1, 0, ScrollView::new(view))
+ }
+
+ let tabs = TabbedView::new()
+ .tab(
+ "Interface Name Pinning",
+ page(
InterfacePinningOptionsView::new(options_ref.clone())
.with_name(Self::INTERFACE_PINNING_VIEW_NAME),
- ))
- .child(Panel::new(
+ ),
+ )
+ .tab(
+ "Management Bond",
+ page(
BondOptionsView::new(options_ref.clone())
.with_name(Self::BOND_OPTIONS_VIEW_NAME),
- ))
- .child(Panel::new(
+ ),
+ )
+ .tab(
+ "Management VLAN",
+ page(
VlanOptionsView::new(options_ref.clone())
.with_name(Self::VLAN_OPTIONS_VIEW_NAME),
- )),
- );
+ ),
+ );
- Dialog::around(inner)
+ Dialog::around(tabs.with_name(Self::ADVANCED_OPTIONS_TABS_NAME))
.title("Advanced Network Options")
.button("Ok", {
move |siv| {
@@ -251,7 +269,12 @@ impl NetworkOptionsView {
options.bond_options = bond_opts;
options.vlan_options = vlan_opts;
}
- Err(err) => {
+ Err((tab, err)) => {
+ // the invalid input might be on a tab that is not shown currently
+ siv.call_on_name(
+ Self::ADVANCED_OPTIONS_TABS_NAME,
+ |view: &mut TabbedView| view.set_current(tab),
+ );
siv.add_layer(Dialog::info(err.to_string()));
return;
}
@@ -263,45 +286,55 @@ impl NetworkOptionsView {
.max_size((80, 40))
}
+ /// Returns the values of all advanced options, or the error together with the index of the
+ /// tab containing the invalid input.
fn get_view_values(
siv: &mut Cursive,
options_ref: NetworkViewOptionsRef,
- ) -> Result<(
- NetworkInterfacePinningOptions,
- NetworkBondOptions,
- NetworkVlanOptions,
- )> {
+ ) -> Result<
+ (
+ NetworkInterfacePinningOptions,
+ NetworkBondOptions,
+ NetworkVlanOptions,
+ ),
+ (usize, anyhow::Error),
+ > {
let pin_opts = siv
.call_on_name(
Self::INTERFACE_PINNING_VIEW_NAME,
InterfacePinningOptionsView::get_values,
)
- .unwrap_or_else(|| {
- bail!("failed to retrieve network interface name pinning options")
- })?;
+ .unwrap_or_else(|| bail!("failed to retrieve network interface name pinning options"))
+ .map_err(|err| (Self::INTERFACE_PINNING_TAB, err))?;
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"))?;
+ .unwrap_or_else(|| bail!("failed to retrieve management network bond options"))
+ .map_err(|err| (Self::BOND_OPTIONS_TAB, err))?;
if options_ref
.lock()
.map(|opts| opts.bond_enabled)
.unwrap_or(false)
{
- bond_opts.verify()?;
+ bond_opts
+ .verify()
+ .map_err(|err| (Self::BOND_OPTIONS_TAB, err))?;
}
let vlan_opts = siv
.call_on_name(Self::VLAN_OPTIONS_VIEW_NAME, VlanOptionsView::get_values)
- .unwrap_or_else(|| bail!("failed to retrieve management network VLAN options"))?;
+ .unwrap_or_else(|| bail!("failed to retrieve management network VLAN options"))
+ .map_err(|err| (Self::VLAN_OPTIONS_TAB, err))?;
if options_ref
.lock()
.map(|opts| opts.vlan_enabled)
.unwrap_or(false)
{
- vlan_opts.verify()?;
+ vlan_opts
+ .verify()
+ .map_err(|err| (Self::VLAN_OPTIONS_TAB, err))?;
}
Ok((pin_opts, bond_opts, vlan_opts))
diff --git a/proxmox-tui-installer/src/views/tabbed_view.rs b/proxmox-tui-installer/src/views/tabbed_view.rs
index 1331713..4c849ea 100644
--- a/proxmox-tui-installer/src/views/tabbed_view.rs
+++ b/proxmox-tui-installer/src/views/tabbed_view.rs
@@ -40,6 +40,13 @@ impl TabbedView {
self
}
+ /// Makes the tab at `index` the active one, if it exists.
+ pub fn set_current(&mut self, index: usize) {
+ if index < self.views.len() {
+ self.current = index;
+ }
+ }
+
/// Returns a reference to the specified tab content view.
pub fn get(&self, index: usize) -> Option<&dyn View> {
self.views.get(index).map(|(_, view)| view.borrow())
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 22/23] gui: network: list bond member candidates sharing a MAC address once
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (20 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 21/23] tui: " Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
2026-11-06 13:10 ` [PATCH installer v5 23/23] tui: " Christoph Heiss
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Bond members are identified by their MAC address, so physical links
sharing one, for example on hardware that does not assign each port a
unique address, cannot be told apart in the bond configuration anyway.
Such links got a checkbox each, but only the last one was kept for the
MAC address, so the others were never labeled and showed up as empty
checkboxes without any effect. Also, the label of the kept checkbox was
taken from whichever of these links came first in hash order.
Only list the first of these links by name, and label each checkbox
from the link it was created for.
Originally-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* new patch
proxinstall | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/proxinstall b/proxinstall
index 6b8c7e8..c0f2664 100755
--- a/proxinstall
+++ b/proxinstall
@@ -545,9 +545,11 @@ my sub create_network_bond_page {
$vbox->pack_start($if_label, 0, 0, 0);
my $checkboxes = {};
+ my $member_ifaces = {};
for my $ifname (sort keys $interfaces->%*) {
my $iface = $interfaces->{$ifname};
- next if !defined($iface->{pinned_id});
+ # members are identified by MAC address, so links sharing one cannot be told apart
+ next if !defined($iface->{pinned_id}) || defined($checkboxes->{ $iface->{mac} });
# without a label, as one passed here would be parsed for mnemonics, eating underscores
my $checkbox = Gtk3::CheckButton->new();
@@ -557,6 +559,7 @@ my sub create_network_bond_page {
$vbox->pack_start($checkbox, 0, 0, 5);
$checkboxes->{ $iface->{mac} } = $checkbox;
+ $member_ifaces->{ $iface->{mac} } = $iface;
}
my $set_sensitive = sub {
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH installer v5 23/23] tui: network: list bond member candidates sharing a MAC address once
2026-11-06 13:10 [PATCH installer v5 00/23] fix #2164: add bond and vlan setup across all installers Christoph Heiss
` (21 preceding siblings ...)
2026-11-06 13:10 ` [PATCH installer v5 22/23] gui: network: list bond member candidates sharing a MAC address once Christoph Heiss
@ 2026-11-06 13:10 ` Christoph Heiss
22 siblings, 0 replies; 24+ messages in thread
From: Christoph Heiss @ 2026-11-06 13:10 UTC (permalink / raw)
To: pve-devel
Bond members are identified by their MAC address, so physical links
sharing one, for example on hardware that does not assign each port a
unique address, cannot be told apart in the bond configuration anyway.
Listing each of them only results in entries that toggle the same
member, which then also ends up twice in the member list if both are
checked.
Only list the first of these links. When refreshing the labels after
the pinned names changed, only consider physical links, as virtual ones
like bridges or bonds can share the MAC address of a physical link and
would show up with their name instead.
Originally-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v4 -> v5:
* no changes
Changes v3 -> v4:
* no changes
Changes v2 -> v3:
* new patch
proxmox-tui-installer/src/views/network.rs | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
index 4a5d334..80e9d26 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -8,7 +8,7 @@ use cursive::{
},
};
use std::{
- collections::{BTreeMap, HashMap},
+ collections::{BTreeMap, HashMap, HashSet},
net::IpAddr,
sync::{Arc, Mutex},
};
@@ -642,10 +642,12 @@ impl BondOptionsView {
.child(DummyView::new());
let mut checkboxes = CheckboxGroup::<MacAddress>::new();
+ // members are identified by MAC address, so links sharing one cannot be told apart
+ let mut listed_macs = HashSet::new();
for iface in options
.interfaces
.iter()
- .filter(|iface| iface.pinned_id.is_some())
+ .filter(|iface| iface.pinned_id.is_some() && listed_macs.insert(iface.mac))
{
let label = Self::format_interface_label(options, iface);
let is_member = options.bond_options.members.contains(&iface.mac);
@@ -709,7 +711,11 @@ impl BondOptionsView {
|group: &mut CheckboxGroup<MacAddress>| {
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) {
+ if let Some(iface) = options
+ .interfaces
+ .iter()
+ .find(|iface| iface.pinned_id.is_some() && iface.mac == *mac)
+ {
label.set_content(Self::format_interface_label(&options, iface));
}
});
--
2.56.0
^ permalink raw reply related [flat|nested] 24+ messages in thread