* [pve-devel] [PATCH installer 01/14] test: parse-kernel-cmdline: fix module import statement
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 02/14] install: add support for network interface name pinning Christoph Heiss
` (13 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
The subroutine is called with its fully-qualified path below, so drop
the explicit import - as it otherwise fails to find it.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
test/parse-kernel-cmdline.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/parse-kernel-cmdline.pl b/test/parse-kernel-cmdline.pl
index b236734..7c849c5 100755
--- a/test/parse-kernel-cmdline.pl
+++ b/test/parse-kernel-cmdline.pl
@@ -7,7 +7,7 @@ use Test::More;
use Test::MockModule qw(strict);
use Proxmox::Install::RunEnv;
-use Proxmox::Install::Config qw(parse_kernel_cmdline);
+use Proxmox::Install::Config;
my $proxmox_install_runenv = Test::MockModule->new('Proxmox::Install::RunEnv');
my $proxmox_install_isoenv = Test::MockModule->new('Proxmox::Install::ISOEnv');
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* [pve-devel] [PATCH installer 02/14] install: add support for network interface name pinning
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 01/14] test: parse-kernel-cmdline: fix module import statement Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 03/14] run env: network: add kernel driver name to network interface info Christoph Heiss
` (12 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
Adds the (low-level) part for pinning network interface names during
installation based on their MAC address.
Adds a new option `pin_interface_name_map`, which maps MAC addresses to
their respective new interface name.
Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Proxmox/Install.pm | 47 +++++++++++++++-
Proxmox/Install/Config.pm | 8 +++
Proxmox/Install/RunEnv.pm | 4 ++
Proxmox/Sys/Net.pm | 54 +++++++++++++++++++
...n_from_dhcp_no_default_domain.run-env.json | 27 ++++++----
.../no_fqdn_from_dhcp.run-env.json | 27 ++++++----
.../tests/resources/run-env-info.json | 29 ++++++----
7 files changed, 167 insertions(+), 29 deletions(-)
diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index 2ebd376..8d02c88 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -1101,6 +1101,49 @@ sub extract_data {
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;
+
+ my $ip_links = Proxmox::Sys::Net::ip_link_details();
+
+ for my $ifname (sort keys $run_env->{network}->{interfaces}->%*) {
+ next
+ if !Proxmox::Sys::Net::ip_link_is_physical($ip_links->{$ifname})
+ || Proxmox::Sys::Net::iface_is_vf($ifname);
+
+ my $if = $run_env->{network}->{interfaces}->{$ifname};
+
+ if (!defined($netif_pin_map->{ $if->{mac} })) {
+ # each installer frontend must ensure that either
+ # - pinning is disabled by never setting the map or
+ # - setting a pinned name for each
+ die "no network interface name set for '$if->{mac}'!";
+ }
+
+ my $pinned_name = $netif_pin_map->{ $if->{mac} };
+ my $link_file_content = Proxmox::Sys::Net::get_pin_link_file_content(
+ $if->{mac}, $pinned_name,
+ );
+
+ file_write_all(
+ "$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;
+ }
+ }
+ }
+
if ($iso_env->{cfg}->{bridged_network}) {
$ifaces .= "iface $ethdev $ntype manual\n";
@@ -1121,7 +1164,9 @@ sub extract_data {
my $ipconf = $run_env->{ipconf};
foreach my $iface (sort keys %{ $ipconf->{ifaces} }) {
- my $name = $ipconf->{ifaces}->{$iface}->{name};
+ my $if = $ipconf->{ifaces}->{$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";
diff --git a/Proxmox/Install/Config.pm b/Proxmox/Install/Config.pm
index da476da..7b81fcf 100644
--- a/Proxmox/Install/Config.pm
+++ b/Proxmox/Install/Config.pm
@@ -101,6 +101,11 @@ my sub init_cfg {
# network related
mngmt_nic => undef,
+
+ # maps mac address -> custom name
+ # if set to a hash, enables interface name pinning for all interfaces
+ network_interface_pin_map => undef,
+
hostname => undef,
domain => undef,
cidr => undef,
@@ -248,6 +253,9 @@ sub get_root_ssh_keys { return get('root_ssh_keys'); }
sub set_mngmt_nic { set_key('mngmt_nic', $_[0]); }
sub get_mngmt_nic { return get('mngmt_nic'); }
+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'); }
+
sub set_hostname { set_key('hostname', $_[0]); }
sub get_hostname { return get('hostname'); }
diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
index 540bec7..f0fa6ec 100644
--- a/Proxmox/Install/RunEnv.pm
+++ b/Proxmox/Install/RunEnv.pm
@@ -86,6 +86,7 @@ my sub query_netdevs : prototype() {
# FIXME: not the same as the battle proven way we used in the installer for years?
my $interfaces = fromjs(qx/ip --json address show/);
+ my $pinned_counter = 0;
for my $if (@$interfaces) {
my ($index, $name, $state, $mac, $addresses) =
$if->@{qw(ifindex ifname operstate address addr_info)};
@@ -115,10 +116,13 @@ my sub query_netdevs : prototype() {
$ifs->{$name} = {
index => $index,
name => $name,
+ pinned_id => "${pinned_counter}",
mac => $mac,
state => uc($state),
};
$ifs->{$name}->{addresses} = \@valid_addrs if @valid_addrs;
+
+ $pinned_counter++;
}
return $ifs;
diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
index 6fe99ec..2183d27 100644
--- a/Proxmox/Sys/Net.pm
+++ b/Proxmox/Sys/Net.pm
@@ -3,7 +3,9 @@ package Proxmox::Sys::Net;
use strict;
use warnings;
+use Proxmox::Sys::Command;
use Proxmox::Sys::Udev;
+use JSON qw();
use base qw(Exporter);
our @EXPORT_OK = qw(parse_ip_address parse_ip_mask parse_fqdn);
@@ -129,9 +131,59 @@ sub parse_ip_mask {
return;
}
+sub iface_is_vf {
+ my ($iface_name) = @_;
+ return -l "/sys/class/net/$iface_name/device/physfn";
+}
+
+# Duplicated from pve-common/src/PVE/Network.pm for now
+sub ip_link_details {
+ my $link_json = '';
+
+ Proxmox::Sys::Command::run_command(
+ ['ip', '-details', '-json', 'link', 'show'],
+ sub {
+ $link_json .= shift;
+ return;
+ },
+ );
+
+ my $links = JSON::decode_json($link_json);
+ my %ip_links = map { $_->{ifname} => $_ } $links->@*;
+
+ return \%ip_links;
+}
+
+# Duplicated from pve-common/src/PVE/Network.pm from now
+sub ip_link_is_physical {
+ my ($ip_link) = @_;
+
+ # ether alone isn't enough, as virtual interfaces can also have link_type
+ # ether
+ return $ip_link->{link_type} eq 'ether'
+ && (!defined($ip_link->{linkinfo}) || !defined($ip_link->{linkinfo}->{info_kind}));
+}
+
+my $LINK_FILE_TEMPLATE = <<EOF;
+# setup by the Proxmox installer.
+[Match]
+MACAddress=%s
+Type=ether
+
+[Link]
+Name=%s
+EOF
+
+sub get_pin_link_file_content {
+ my ($mac, $pin_name) = @_;
+
+ return sprintf($LINK_FILE_TEMPLATE, $mac, $pin_name);
+}
+
sub get_ip_config {
my $ifaces = {};
my $default;
+ my $pinned_counter = 0;
my $links = `ip -o l`;
foreach my $l (split /\n/, $links) {
@@ -144,11 +196,13 @@ sub get_ip_config {
$ifaces->{"$index"} = {
name => $name,
+ pinned_id => "${pinned_counter}",
driver => $driver,
flags => $flags,
state => $state,
mac => $mac,
};
+ $pinned_counter++;
my $addresses = `ip -o a s $name`;
for my $addr_line (split /\n/, $addresses) {
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json
index ef14419..e7c2205 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json
@@ -181,55 +181,64 @@
"index": 4,
"mac": "b4:2e:99:ac:ad:b4",
"name": "eno1",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "0"
},
"eno2": {
"index": 6,
"mac": "b4:2e:99:ac:ad:b5",
"name": "eno2",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "1"
},
"enp129s0f0np0": {
"index": 7,
"mac": "1c:34:da:5c:5e:24",
"name": "enp129s0f0np0",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "2"
},
"enp129s0f1np1": {
"index": 8,
"mac": "1c:34:da:5c:5e:25",
"name": "enp129s0f1np1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "3"
},
"enp193s0f0np0": {
"index": 9,
"mac": "24:8a:07:1e:05:bc",
"name": "enp193s0f0np0",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "4"
},
"enp193s0f1np1": {
"index": 10,
"mac": "24:8a:07:1e:05:bd",
"name": "enp193s0f1np1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "5"
},
"enp65s0f0": {
"index": 2,
"mac": "a0:36:9f:0a:b3:82",
"name": "enp65s0f0",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "6"
},
"enp65s0f1": {
"index": 3,
"mac": "a0:36:9f:0a:b3:83",
"name": "enp65s0f1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "7"
},
"enx5a4732ddc747": {
"index": 5,
"mac": "5a:47:32:dd:c7:47",
"name": "enx5a4732ddc747",
- "state": "UNKNOWN"
+ "state": "UNKNOWN",
+ "pinned_id": "8"
}
},
"routes": {
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json
index e3cea3e..47d7bde 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json
@@ -180,55 +180,64 @@
"index": 4,
"mac": "b4:2e:99:ac:ad:b4",
"name": "eno1",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "0"
},
"eno2": {
"index": 6,
"mac": "b4:2e:99:ac:ad:b5",
"name": "eno2",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "1"
},
"enp129s0f0np0": {
"index": 7,
"mac": "1c:34:da:5c:5e:24",
"name": "enp129s0f0np0",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "2"
},
"enp129s0f1np1": {
"index": 8,
"mac": "1c:34:da:5c:5e:25",
"name": "enp129s0f1np1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "3"
},
"enp193s0f0np0": {
"index": 9,
"mac": "24:8a:07:1e:05:bc",
"name": "enp193s0f0np0",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "4"
},
"enp193s0f1np1": {
"index": 10,
"mac": "24:8a:07:1e:05:bd",
"name": "enp193s0f1np1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "5"
},
"enp65s0f0": {
"index": 2,
"mac": "a0:36:9f:0a:b3:82",
"name": "enp65s0f0",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "6"
},
"enp65s0f1": {
"index": 3,
"mac": "a0:36:9f:0a:b3:83",
"name": "enp65s0f1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "7"
},
"enx5a4732ddc747": {
"index": 5,
"mac": "5a:47:32:dd:c7:47",
"name": "enx5a4732ddc747",
- "state": "UNKNOWN"
+ "state": "UNKNOWN",
+ "pinned_id": "8"
}
},
"routes": {
diff --git a/proxmox-auto-installer/tests/resources/run-env-info.json b/proxmox-auto-installer/tests/resources/run-env-info.json
index 5a8d80a..5291721 100644
--- a/proxmox-auto-installer/tests/resources/run-env-info.json
+++ b/proxmox-auto-installer/tests/resources/run-env-info.json
@@ -181,55 +181,64 @@
"index": 4,
"mac": "b4:2e:99:ac:ad:b4",
"name": "eno1",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "0"
},
"eno2": {
"index": 6,
"mac": "b4:2e:99:ac:ad:b5",
"name": "eno2",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "1"
},
"enp129s0f0np0": {
"index": 7,
"mac": "1c:34:da:5c:5e:24",
"name": "enp129s0f0np0",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "2"
},
"enp129s0f1np1": {
"index": 8,
"mac": "1c:34:da:5c:5e:25",
"name": "enp129s0f1np1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "3"
},
"enp193s0f0np0": {
"index": 9,
"mac": "24:8a:07:1e:05:bc",
"name": "enp193s0f0np0",
- "state": "UP"
+ "state": "UP",
+ "pinned_id": "4"
},
"enp193s0f1np1": {
"index": 10,
"mac": "24:8a:07:1e:05:bd",
"name": "enp193s0f1np1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "5"
},
"enp65s0f0": {
"index": 2,
"mac": "a0:36:9f:0a:b3:82",
"name": "enp65s0f0",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "6"
},
- "en p65s0f1": {
+ "enp65s0f1": {
"index": 3,
"mac": "a0:36:9f:0a:b3:83",
"name": "enp65s0f1",
- "state": "DOWN"
+ "state": "DOWN",
+ "pinned_id": "7"
},
"enx5a4732ddc747": {
"index": 5,
"mac": "5a:47:32:dd:c7:47",
"name": "enx5a4732ddc747",
- "state": "UNKNOWN"
+ "state": "UNKNOWN",
+ "pinned_id": "8"
}
},
"routes": {
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* [pve-devel] [PATCH installer 03/14] run env: network: add kernel driver name to network interface info
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 01/14] test: parse-kernel-cmdline: fix module import statement Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 02/14] install: add support for network interface name pinning Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 04/14] common: utils: fix clippy warnings Christoph Heiss
` (11 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
This information is already available in the GUI, this makes it possible
to show it in the TUI too.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Proxmox/Install/RunEnv.pm | 7 +++++++
.../fqdn_from_dhcp_no_default_domain.run-env.json | 9 +++++++++
.../parse_answer_fail/no_fqdn_from_dhcp.run-env.json | 9 +++++++++
proxmox-auto-installer/tests/resources/run-env-info.json | 9 +++++++++
proxmox-installer-common/src/options.rs | 2 ++
proxmox-installer-common/src/setup.rs | 3 +++
6 files changed, 39 insertions(+)
diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
index f0fa6ec..40a9621 100644
--- a/Proxmox/Install/RunEnv.pm
+++ b/Proxmox/Install/RunEnv.pm
@@ -72,6 +72,9 @@ sub query_cpu_info : prototype() {
# mac => <mac address>,
# index => <index>,
# name => <ifname>,
+# state => <UP|DOWN|LOWERLAYERDOWN|DORMANT|TESTING|NOTPRESENT|UNKNOWN>,
+# pinned_id => <sequential numerical ID>,
+# driver => <driver name>,
# addresses => [
# family => <inet|inet6>,
# address => <mac address>,
@@ -113,12 +116,16 @@ my sub query_netdevs : prototype() {
}
}
+ my $driver = readlink "/sys/class/net/$name/device/driver" || 'unknown';
+ $driver =~ s!^.*/!!;
+
$ifs->{$name} = {
index => $index,
name => $name,
pinned_id => "${pinned_counter}",
mac => $mac,
state => uc($state),
+ driver => $driver,
};
$ifs->{$name}->{addresses} = \@valid_addrs if @valid_addrs;
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json
index e7c2205..4c41c13 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/fqdn_from_dhcp_no_default_domain.run-env.json
@@ -182,6 +182,7 @@
"mac": "b4:2e:99:ac:ad:b4",
"name": "eno1",
"state": "UP",
+ "driver": "igb",
"pinned_id": "0"
},
"eno2": {
@@ -189,6 +190,7 @@
"mac": "b4:2e:99:ac:ad:b5",
"name": "eno2",
"state": "UP",
+ "driver": "igb",
"pinned_id": "1"
},
"enp129s0f0np0": {
@@ -196,6 +198,7 @@
"mac": "1c:34:da:5c:5e:24",
"name": "enp129s0f0np0",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "2"
},
"enp129s0f1np1": {
@@ -203,6 +206,7 @@
"mac": "1c:34:da:5c:5e:25",
"name": "enp129s0f1np1",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "3"
},
"enp193s0f0np0": {
@@ -210,6 +214,7 @@
"mac": "24:8a:07:1e:05:bc",
"name": "enp193s0f0np0",
"state": "UP",
+ "driver": "mlx5_core",
"pinned_id": "4"
},
"enp193s0f1np1": {
@@ -217,6 +222,7 @@
"mac": "24:8a:07:1e:05:bd",
"name": "enp193s0f1np1",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "5"
},
"enp65s0f0": {
@@ -224,6 +230,7 @@
"mac": "a0:36:9f:0a:b3:82",
"name": "enp65s0f0",
"state": "DOWN",
+ "driver": "igb",
"pinned_id": "6"
},
"enp65s0f1": {
@@ -231,6 +238,7 @@
"mac": "a0:36:9f:0a:b3:83",
"name": "enp65s0f1",
"state": "DOWN",
+ "driver": "igb",
"pinned_id": "7"
},
"enx5a4732ddc747": {
@@ -238,6 +246,7 @@
"mac": "5a:47:32:dd:c7:47",
"name": "enx5a4732ddc747",
"state": "UNKNOWN",
+ "driver": "cdc_ether",
"pinned_id": "8"
}
},
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json
index 47d7bde..fa4e834 100644
--- a/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/no_fqdn_from_dhcp.run-env.json
@@ -181,6 +181,7 @@
"mac": "b4:2e:99:ac:ad:b4",
"name": "eno1",
"state": "UP",
+ "driver": "igb",
"pinned_id": "0"
},
"eno2": {
@@ -188,6 +189,7 @@
"mac": "b4:2e:99:ac:ad:b5",
"name": "eno2",
"state": "UP",
+ "driver": "igb",
"pinned_id": "1"
},
"enp129s0f0np0": {
@@ -195,6 +197,7 @@
"mac": "1c:34:da:5c:5e:24",
"name": "enp129s0f0np0",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "2"
},
"enp129s0f1np1": {
@@ -202,6 +205,7 @@
"mac": "1c:34:da:5c:5e:25",
"name": "enp129s0f1np1",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "3"
},
"enp193s0f0np0": {
@@ -209,6 +213,7 @@
"mac": "24:8a:07:1e:05:bc",
"name": "enp193s0f0np0",
"state": "UP",
+ "driver": "mlx5_core",
"pinned_id": "4"
},
"enp193s0f1np1": {
@@ -216,6 +221,7 @@
"mac": "24:8a:07:1e:05:bd",
"name": "enp193s0f1np1",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "5"
},
"enp65s0f0": {
@@ -223,6 +229,7 @@
"mac": "a0:36:9f:0a:b3:82",
"name": "enp65s0f0",
"state": "DOWN",
+ "driver": "igb",
"pinned_id": "6"
},
"enp65s0f1": {
@@ -230,6 +237,7 @@
"mac": "a0:36:9f:0a:b3:83",
"name": "enp65s0f1",
"state": "DOWN",
+ "driver": "igb",
"pinned_id": "7"
},
"enx5a4732ddc747": {
@@ -237,6 +245,7 @@
"mac": "5a:47:32:dd:c7:47",
"name": "enx5a4732ddc747",
"state": "UNKNOWN",
+ "driver": "cdc_ether",
"pinned_id": "8"
}
},
diff --git a/proxmox-auto-installer/tests/resources/run-env-info.json b/proxmox-auto-installer/tests/resources/run-env-info.json
index 5291721..add5f2a 100644
--- a/proxmox-auto-installer/tests/resources/run-env-info.json
+++ b/proxmox-auto-installer/tests/resources/run-env-info.json
@@ -182,6 +182,7 @@
"mac": "b4:2e:99:ac:ad:b4",
"name": "eno1",
"state": "UP",
+ "driver": "igb",
"pinned_id": "0"
},
"eno2": {
@@ -189,6 +190,7 @@
"mac": "b4:2e:99:ac:ad:b5",
"name": "eno2",
"state": "UP",
+ "driver": "igb",
"pinned_id": "1"
},
"enp129s0f0np0": {
@@ -196,6 +198,7 @@
"mac": "1c:34:da:5c:5e:24",
"name": "enp129s0f0np0",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "2"
},
"enp129s0f1np1": {
@@ -203,6 +206,7 @@
"mac": "1c:34:da:5c:5e:25",
"name": "enp129s0f1np1",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "3"
},
"enp193s0f0np0": {
@@ -210,6 +214,7 @@
"mac": "24:8a:07:1e:05:bc",
"name": "enp193s0f0np0",
"state": "UP",
+ "driver": "mlx5_core",
"pinned_id": "4"
},
"enp193s0f1np1": {
@@ -217,6 +222,7 @@
"mac": "24:8a:07:1e:05:bd",
"name": "enp193s0f1np1",
"state": "DOWN",
+ "driver": "mlx5_core",
"pinned_id": "5"
},
"enp65s0f0": {
@@ -224,6 +230,7 @@
"mac": "a0:36:9f:0a:b3:82",
"name": "enp65s0f0",
"state": "DOWN",
+ "driver": "igb",
"pinned_id": "6"
},
"enp65s0f1": {
@@ -231,6 +238,7 @@
"mac": "a0:36:9f:0a:b3:83",
"name": "enp65s0f1",
"state": "DOWN",
+ "driver": "igb",
"pinned_id": "7"
},
"enx5a4732ddc747": {
@@ -238,6 +246,7 @@
"mac": "5a:47:32:dd:c7:47",
"name": "enx5a4732ddc747",
"state": "UNKNOWN",
+ "driver": "cdc_ether",
"pinned_id": "8"
}
},
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 48c77c9..50fd74e 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -677,6 +677,7 @@ mod tests {
name: "eth0".to_owned(),
index: 0,
state: InterfaceState::Up,
+ driver: "dummy".to_owned(),
mac: "01:23:45:67:89:ab".to_owned(),
addresses: Some(vec![
CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
@@ -804,6 +805,7 @@ mod tests {
name: "eth0".to_owned(),
index: 0,
state: InterfaceState::Up,
+ driver: "dummy".to_owned(),
mac: "01:23:45:67:89:ab".to_owned(),
addresses: None,
},
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 66cea72..4fda39d 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -473,6 +473,9 @@ pub struct Interface {
pub state: InterfaceState,
+ /// Kernel driver name that claims this interface.
+ pub driver: String,
+
#[serde(default)]
#[serde(deserialize_with = "deserialize_cidr_list")]
pub addresses: Option<Vec<CidrAddress>>,
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* [pve-devel] [PATCH installer 04/14] common: utils: fix clippy warnings
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
` (2 preceding siblings ...)
2025-10-14 13:21 ` [pve-devel] [PATCH installer 03/14] run env: network: add kernel driver name to network interface info Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 05/14] common: setup: simplify network address list serialization Christoph Heiss
` (10 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-installer-common/src/utils.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/proxmox-installer-common/src/utils.rs b/proxmox-installer-common/src/utils.rs
index 054a0fd..ffc862e 100644
--- a/proxmox-installer-common/src/utils.rs
+++ b/proxmox-installer-common/src/utils.rs
@@ -130,14 +130,14 @@ fn mask_limit(addr: &IpAddr) -> usize {
}
fn check_mask_limit(addr: &IpAddr, mask: usize) -> Result<(), CidrAddressParseError> {
- let limit = mask_limit(&addr);
- return if mask > limit {
+ let limit = mask_limit(addr);
+ if mask > limit {
Err(CidrAddressParseError::InvalidMask(
format!("mask cannot be greater than {limit}").into(),
))
} else {
Ok(())
- };
+ }
}
/// Possible errors that might occur when parsing FQDNs.
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* [pve-devel] [PATCH installer 05/14] common: setup: simplify network address list serialization
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
` (3 preceding siblings ...)
2025-10-14 13:21 ` [pve-devel] [PATCH installer 04/14] common: utils: fix clippy warnings Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 06/14] common: implement support for `network_interface_pin_map` config Christoph Heiss
` (9 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
Option<Vec<..>> is redundant in combination with #[serde(default)],
as the latter already defaults to an empty Vec<..> if the key does not
exist.
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-installer-common/src/options.rs | 26 ++++++++++---------------
proxmox-installer-common/src/setup.rs | 6 +++---
2 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 50fd74e..59e8560 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -518,24 +518,20 @@ impl NetworkOptions {
if let Some(gw) = &routes.gateway4 {
if let Some(iface) = network.interfaces.get(&gw.dev) {
this.ifname.clone_from(&iface.name);
- if let Some(addresses) = &iface.addresses {
- if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv4()) {
- this.gateway = gw.gateway;
- this.address = addr.clone();
- filled = true;
- }
+ if let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv4()) {
+ this.gateway = gw.gateway;
+ this.address = addr.clone();
+ filled = true;
}
}
}
if !filled {
if let Some(gw) = &routes.gateway6 {
if let Some(iface) = network.interfaces.get(&gw.dev) {
- if let Some(addresses) = &iface.addresses {
- if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv6()) {
- this.ifname.clone_from(&iface.name);
- this.gateway = gw.gateway;
- this.address = addr.clone();
- }
+ if let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv6()) {
+ this.ifname.clone_from(&iface.name);
+ this.gateway = gw.gateway;
+ this.address = addr.clone();
}
}
}
@@ -679,9 +675,7 @@ mod tests {
state: InterfaceState::Up,
driver: "dummy".to_owned(),
mac: "01:23:45:67:89:ab".to_owned(),
- addresses: Some(vec![
- CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
- ]),
+ addresses: vec![CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap()],
},
);
@@ -807,7 +801,7 @@ mod tests {
state: InterfaceState::Up,
driver: "dummy".to_owned(),
mac: "01:23:45:67:89:ab".to_owned(),
- addresses: None,
+ addresses: vec![],
},
);
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 4fda39d..4873fff 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -321,7 +321,7 @@ where
.collect())
}
-fn deserialize_cidr_list<'de, D>(deserializer: D) -> Result<Option<Vec<CidrAddress>>, D::Error>
+fn deserialize_cidr_list<'de, D>(deserializer: D) -> Result<Vec<CidrAddress>, D::Error>
where
D: Deserializer<'de>,
{
@@ -347,7 +347,7 @@ where
);
}
- Ok(Some(result))
+ Ok(result)
}
fn serialize_as_display<S, T>(value: &T, serializer: S) -> Result<S::Ok, S::Error>
@@ -478,7 +478,7 @@ pub struct Interface {
#[serde(default)]
#[serde(deserialize_with = "deserialize_cidr_list")]
- pub addresses: Option<Vec<CidrAddress>>,
+ pub addresses: Vec<CidrAddress>,
}
impl Interface {
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* [pve-devel] [PATCH installer 06/14] common: implement support for `network_interface_pin_map` config
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
` (4 preceding siblings ...)
2025-10-14 13:21 ` [pve-devel] [PATCH installer 05/14] common: setup: simplify network address list serialization Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-21 14:05 ` Michael Köppl
2025-10-14 13:21 ` [pve-devel] [PATCH installer 07/14] auto: add support for pinning network interface names Christoph Heiss
` (8 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
Adds all the pieces for installer frontends to wire up pinning support,
i.e. deserializing from the runtime environment, doing verification and
serializing it out to the low-level installer.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-auto-installer/src/utils.rs | 3 +-
proxmox-installer-common/src/lib.rs | 5 +
proxmox-installer-common/src/options.rs | 158 ++++++++++++++++++++++--
proxmox-installer-common/src/setup.rs | 51 +++++++-
proxmox-tui-installer/src/setup.rs | 3 +-
5 files changed, 203 insertions(+), 17 deletions(-)
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 7d42f2c..eb666d1 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -2,7 +2,7 @@ use anyhow::{Context, Result, bail};
use glob::Pattern;
use log::info;
use std::{
- collections::{BTreeMap, HashSet},
+ collections::{BTreeMap, HashMap, HashSet},
process::Command,
};
@@ -485,6 +485,7 @@ pub fn parse_answer(
root_ssh_keys: answer.global.root_ssh_keys.clone(),
mngmt_nic: network_settings.ifname,
+ network_interface_pin_map: HashMap::new(),
hostname: network_settings
.fqdn
diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
index ea907a0..a85d5f8 100644
--- a/proxmox-installer-common/src/lib.rs
+++ b/proxmox-installer-common/src/lib.rs
@@ -10,6 +10,11 @@ pub mod http;
#[cfg(feature = "cli")]
pub mod cli;
+pub mod net {
+ /// Maximum length of the (primary) name of a network interface
+ pub const MAX_IFNAME_LEN: usize = 15; // IFNAMSIZ - 1 to account for NUL byte
+}
+
pub const RUNTIME_DIR: &str = "/run/proxmox-installer";
/// Default placeholder value for the administrator email address.
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 59e8560..60ea227 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -1,12 +1,14 @@
use anyhow::{Result, bail};
use regex::Regex;
use serde::{Deserialize, Serialize};
+use std::collections::HashMap;
use std::net::{IpAddr, Ipv4Addr};
use std::str::FromStr;
use std::sync::OnceLock;
use std::{cmp, fmt};
use crate::disk_checks::check_raid_min_disks;
+use crate::net::MAX_IFNAME_LEN;
use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo};
use crate::utils::{CidrAddress, Fqdn};
@@ -476,6 +478,54 @@ impl TimezoneOptions {
}
}
+/// Options controlling the behaviour of the network interface pinning (by
+/// creating appropriate systemd.link files) during the installation.
+#[derive(Clone, Debug, Default, PartialEq, Deserialize)]
+#[serde(rename_all = "kebab-case", deny_unknown_fields)]
+pub struct NetworkInterfacePinningOptions {
+ /// Maps MAC address to custom name
+ #[serde(default)]
+ pub mapping: HashMap<String, String>,
+}
+
+impl NetworkInterfacePinningOptions {
+ /// Default prefix to prepend to the pinned interface ID as received from the low-level
+ /// installer.
+ pub const DEFAULT_PREFIX: &str = "nic";
+
+ /// Do some basic checks on the options.
+ ///
+ /// This checks for:
+ /// - empty interface names
+ /// - overlong interface names
+ /// - duplicate interface names
+ pub fn verify(&self) -> Result<()> {
+ let mut reverse_mapping = HashMap::<String, String>::new();
+ for (mac, name) in self.mapping.iter() {
+ if name.is_empty() {
+ bail!("interface name mapping for '{mac}' cannot be empty");
+ }
+
+ if name.len() > MAX_IFNAME_LEN {
+ bail!(
+ "interface name mapping '{name}' for '{mac}' cannot be longer than {} characters",
+ MAX_IFNAME_LEN
+ );
+ }
+
+ if let Some(duplicate_mac) = reverse_mapping.get(name) {
+ if mac != duplicate_mac {
+ bail!("duplicate interface name mapping '{name}' for: {mac}, {duplicate_mac}");
+ }
+ }
+
+ reverse_mapping.insert(name.clone(), mac.clone());
+ }
+
+ Ok(())
+ }
+}
+
#[derive(Clone, Debug, PartialEq)]
pub struct NetworkOptions {
pub ifname: String,
@@ -483,6 +533,7 @@ pub struct NetworkOptions {
pub address: CidrAddress,
pub gateway: IpAddr,
pub dns_server: IpAddr,
+ pub pinning_opts: Option<NetworkInterfacePinningOptions>,
}
impl NetworkOptions {
@@ -492,6 +543,7 @@ impl NetworkOptions {
setup: &SetupInfo,
network: &NetworkInfo,
default_domain: Option<&str>,
+ pinning_opts: Option<&NetworkInterfacePinningOptions>,
) -> Self {
// Sets up sensible defaults as much as possible, such that even in the
// worse case nothing breaks down *completely*.
@@ -507,6 +559,7 @@ impl NetworkOptions {
address: CidrAddress::new(Ipv4Addr::new(192, 168, 100, 2), 24).unwrap(),
gateway: Ipv4Addr::new(192, 168, 100, 1).into(),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: pinning_opts.cloned(),
};
if let Some(ip) = network.dns.dns.first() {
@@ -517,7 +570,11 @@ impl NetworkOptions {
let mut filled = false;
if let Some(gw) = &routes.gateway4 {
if let Some(iface) = network.interfaces.get(&gw.dev) {
- this.ifname.clone_from(&iface.name);
+ if let Some(opts) = pinning_opts {
+ this.ifname.clone_from(&iface.to_pinned(opts).name);
+ } else {
+ this.ifname.clone_from(&iface.name);
+ }
if let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv4()) {
this.gateway = gw.gateway;
this.address = addr.clone();
@@ -529,7 +586,11 @@ impl NetworkOptions {
if let Some(gw) = &routes.gateway6 {
if let Some(iface) = network.interfaces.get(&gw.dev) {
if let Some(addr) = iface.addresses.iter().find(|addr| addr.is_ipv6()) {
- this.ifname.clone_from(&iface.name);
+ if let Some(opts) = pinning_opts {
+ this.ifname.clone_from(&iface.to_pinned(opts).name);
+ } else {
+ this.ifname.clone_from(&iface.name);
+ }
this.gateway = gw.gateway;
this.address = addr.clone();
}
@@ -544,7 +605,20 @@ impl NetworkOptions {
// earlier in that case, so use the first one enumerated.
if this.ifname.is_empty() {
if let Some(iface) = network.interfaces.values().min_by_key(|v| v.index) {
- this.ifname.clone_from(&iface.name);
+ if let Some(opts) = pinning_opts {
+ this.ifname.clone_from(&iface.to_pinned(opts).name);
+ } else {
+ this.ifname.clone_from(&iface.name);
+ }
+ }
+ }
+
+ if let Some(ref mut opts) = this.pinning_opts {
+ // Ensure that all unique interfaces indeed have an entry in the map,
+ // as required by the low-level installer
+ for iface in network.interfaces.values() {
+ let pinned_name = iface.to_pinned(opts).name;
+ opts.mapping.entry(iface.mac.clone()).or_insert(pinned_name);
}
}
@@ -672,6 +746,7 @@ mod tests {
Interface {
name: "eth0".to_owned(),
index: 0,
+ pinned_id: "0".to_owned(),
state: InterfaceState::Up,
driver: "dummy".to_owned(),
mac: "01:23:45:67:89:ab".to_owned(),
@@ -703,49 +778,53 @@ mod tests {
let (setup, mut info) = mock_setup_network();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.bar.com").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
info.hostname = None;
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("pve.bar.com").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
info.dns.domain = None;
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("pve.example.invalid").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
info.hostname = Some("foo".to_owned());
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.example.invalid").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
}
@@ -755,37 +834,40 @@ mod tests {
let (setup, mut info) = mock_setup_network();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.bar.com").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
info.dns.domain = None;
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, Some("custom.local")),
+ NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.custom.local").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
info.dns.domain = Some("some.domain.local".to_owned());
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, Some("custom.local")),
+ NetworkOptions::defaults_from(&setup, &info, Some("custom.local"), None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("foo.custom.local").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 0, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
}
@@ -798,6 +880,7 @@ mod tests {
Interface {
name: "eth0".to_owned(),
index: 0,
+ pinned_id: "0".to_owned(),
state: InterfaceState::Up,
driver: "dummy".to_owned(),
mac: "01:23:45:67:89:ab".to_owned(),
@@ -818,14 +901,67 @@ mod tests {
let setup = SetupInfo::mocked();
pretty_assertions::assert_eq!(
- NetworkOptions::defaults_from(&setup, &info, None),
+ NetworkOptions::defaults_from(&setup, &info, None, None),
NetworkOptions {
ifname: "eth0".to_owned(),
fqdn: Fqdn::from("pve.example.invalid").unwrap(),
address: CidrAddress::new(Ipv4Addr::new(192, 168, 100, 2), 24).unwrap(),
gateway: IpAddr::V4(Ipv4Addr::new(192, 168, 100, 1)),
dns_server: Ipv4Addr::new(192, 168, 100, 1).into(),
+ pinning_opts: None,
}
);
}
+
+ #[test]
+ fn network_interface_pinning_options_fail_on_empty_name() {
+ let mut options = NetworkInterfacePinningOptions::default();
+ options
+ .mapping
+ .insert("ab:cd:ef:12:34:56".to_owned(), String::new());
+
+ let res = options.verify();
+ assert!(res.is_err());
+ assert_eq!(
+ res.unwrap_err().to_string(),
+ "interface name mapping for 'ab:cd:ef:12:34:56' cannot be empty"
+ )
+ }
+
+ #[test]
+ fn network_interface_pinning_options_fail_on_overlong_name() {
+ let mut options = NetworkInterfacePinningOptions::default();
+ options.mapping.insert(
+ "ab:cd:ef:12:34:56".to_owned(),
+ "waytoolonginterfacename".to_owned(),
+ );
+
+ let res = options.verify();
+ assert!(res.is_err());
+ assert_eq!(
+ res.unwrap_err().to_string(),
+ "interface name mapping 'waytoolonginterfacename' for 'ab:cd:ef:12:34:56' cannot be longer than 15 characters"
+ )
+ }
+
+ #[test]
+ fn network_interface_pinning_options_fail_on_duplicate_name() {
+ let mut options = NetworkInterfacePinningOptions::default();
+ options
+ .mapping
+ .insert("ab:cd:ef:12:34:56".to_owned(), "nic0".to_owned());
+ options
+ .mapping
+ .insert("12:34:56:ab:cd:ef".to_owned(), "nic0".to_owned());
+
+ let res = options.verify();
+ assert!(res.is_err());
+ let err = res.unwrap_err().to_string();
+
+ // [HashMap] does not guarantee iteration order, so just check for the substrings
+ // we expect to find
+ assert!(err.contains("duplicate interface name mapping 'nic0' for: "));
+ assert!(err.contains("12:34:56:ab:cd:ef"));
+ assert!(err.contains("ab:cd:ef:12:34:56"));
+ }
}
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 4873fff..3e99576 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -3,6 +3,7 @@ use std::{
collections::{BTreeMap, HashMap},
fmt,
fs::File,
+ hash::Hash,
io::{self, BufReader},
net::IpAddr,
path::{Path, PathBuf},
@@ -13,8 +14,8 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer, de};
use crate::{
options::{
- BtrfsBootdiskOptions, BtrfsCompressOption, Disk, FsType, ZfsBootdiskOptions,
- ZfsChecksumOption, ZfsCompressOption,
+ BtrfsBootdiskOptions, BtrfsCompressOption, Disk, FsType, NetworkInterfacePinningOptions,
+ ZfsBootdiskOptions, ZfsChecksumOption, ZfsCompressOption,
},
utils::CidrAddress,
};
@@ -443,8 +444,9 @@ pub struct Gateway {
pub gateway: IpAddr,
}
-#[derive(Clone, Deserialize)]
+#[derive(Clone, Deserialize, Serialize)]
#[serde(rename_all = "UPPERCASE")]
+/// The current interface state as reported by the kernel.
pub enum InterfaceState {
Up,
Down,
@@ -452,6 +454,8 @@ pub enum InterfaceState {
Unknown,
}
+serde_plain::derive_display_from_serialize!(InterfaceState);
+
impl InterfaceState {
// avoid display trait as this is not the string representation for a serializer
pub fn render(&self) -> String {
@@ -469,6 +473,9 @@ pub struct Interface {
pub index: usize,
+ /// Sequential interface ID for pinning interface names.
+ pub pinned_id: String,
+
pub mac: String,
pub state: InterfaceState,
@@ -484,7 +491,22 @@ pub struct Interface {
impl Interface {
// avoid display trait as this is not the string representation for a serializer
pub fn render(&self) -> String {
- format!("{} {}", self.state.render(), self.name)
+ format!("{} {} ({})", self.state.render(), self.name, self.mac)
+ }
+
+ pub fn to_pinned(&self, options: &NetworkInterfacePinningOptions) -> Self {
+ let mut this = self.clone();
+ this.name = options
+ .mapping
+ .get(&this.mac)
+ .unwrap_or(&format!(
+ "{}{}",
+ NetworkInterfacePinningOptions::DEFAULT_PREFIX,
+ this.pinned_id
+ ))
+ .clone();
+
+ this
}
}
@@ -577,6 +599,14 @@ pub struct InstallConfig {
pub root_ssh_keys: Vec<String>,
pub mngmt_nic: String,
+ // Maps MAC addresses -> custom name. If set, enables pinning for all
+ // interfaces present.
+ #[serde(
+ default,
+ skip_serializing_if = "HashMap::is_empty",
+ deserialize_with = "deserialize_optional_map"
+ )]
+ pub network_interface_pin_map: HashMap<String, String>,
pub hostname: String,
pub domain: String,
@@ -610,3 +640,16 @@ pub enum LowLevelMessage {
text: Option<String>,
},
}
+
+/// Deserializes an optional [HashMap].
+///
+/// If missing, it returns an empty map, otherwise the deserialized map.
+fn deserialize_optional_map<'de, D, K, V>(deserializer: D) -> Result<HashMap<K, V>, D::Error>
+where
+ D: Deserializer<'de>,
+ K: Eq + Hash + Deserialize<'de>,
+ V: Deserialize<'de>,
+{
+ let map: Option<HashMap<K, V>> = Deserialize::deserialize(deserializer)?;
+ Ok(map.unwrap_or_default())
+}
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index b90c7dc..d2ffb70 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -1,4 +1,4 @@
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashMap};
use crate::options::InstallerOptions;
use proxmox_installer_common::{
@@ -32,6 +32,7 @@ impl From<InstallerOptions> for InstallConfig {
root_ssh_keys: vec![],
mngmt_nic: options.network.ifname,
+ network_interface_pin_map: HashMap::new(),
// Safety: At this point, it is know that we have a valid FQDN, as
// this is set by the TUI network panel, which only lets the user
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [pve-devel] [PATCH installer 06/14] common: implement support for `network_interface_pin_map` config
2025-10-14 13:21 ` [pve-devel] [PATCH installer 06/14] common: implement support for `network_interface_pin_map` config Christoph Heiss
@ 2025-10-21 14:05 ` Michael Köppl
2025-10-22 9:37 ` Christoph Heiss
0 siblings, 1 reply; 23+ messages in thread
From: Michael Köppl @ 2025-10-21 14:05 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
1 comment inline
On Tue Oct 14, 2025 at 3:21 PM CEST, Christoph Heiss wrote:
> Adds all the pieces for installer frontends to wire up pinning support,
> i.e. deserializing from the runtime environment, doing verification and
> serializing it out to the low-level installer.
>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> proxmox-auto-installer/src/utils.rs | 3 +-
> proxmox-installer-common/src/lib.rs | 5 +
> proxmox-installer-common/src/options.rs | 158 ++++++++++++++++++++++--
> proxmox-installer-common/src/setup.rs | 51 +++++++-
> proxmox-tui-installer/src/setup.rs | 3 +-
> 5 files changed, 203 insertions(+), 17 deletions(-)
>
> diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
> index 7d42f2c..eb666d1 100644
> --- a/proxmox-auto-installer/src/utils.rs
> +++ b/proxmox-auto-installer/src/utils.rs
> @@ -2,7 +2,7 @@ use anyhow::{Context, Result, bail};
> use glob::Pattern;
> use log::info;
> use std::{
> - collections::{BTreeMap, HashSet},
> + collections::{BTreeMap, HashMap, HashSet},
> process::Command,
> };
>
> @@ -485,6 +485,7 @@ pub fn parse_answer(
> root_ssh_keys: answer.global.root_ssh_keys.clone(),
>
> mngmt_nic: network_settings.ifname,
> + network_interface_pin_map: HashMap::new(),
>
> hostname: network_settings
> .fqdn
> diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
> index ea907a0..a85d5f8 100644
> --- a/proxmox-installer-common/src/lib.rs
> +++ b/proxmox-installer-common/src/lib.rs
> @@ -10,6 +10,11 @@ pub mod http;
> #[cfg(feature = "cli")]
> pub mod cli;
>
> +pub mod net {
> + /// Maximum length of the (primary) name of a network interface
> + pub const MAX_IFNAME_LEN: usize = 15; // IFNAMSIZ - 1 to account for NUL byte
> +}
> +
> pub const RUNTIME_DIR: &str = "/run/proxmox-installer";
>
> /// Default placeholder value for the administrator email address.
> diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
> index 59e8560..60ea227 100644
> --- a/proxmox-installer-common/src/options.rs
> +++ b/proxmox-installer-common/src/options.rs
> @@ -1,12 +1,14 @@
> use anyhow::{Result, bail};
> use regex::Regex;
> use serde::{Deserialize, Serialize};
> +use std::collections::HashMap;
> use std::net::{IpAddr, Ipv4Addr};
> use std::str::FromStr;
> use std::sync::OnceLock;
> use std::{cmp, fmt};
>
> use crate::disk_checks::check_raid_min_disks;
> +use crate::net::MAX_IFNAME_LEN;
> use crate::setup::{LocaleInfo, NetworkInfo, RuntimeInfo, SetupInfo};
> use crate::utils::{CidrAddress, Fqdn};
>
> @@ -476,6 +478,54 @@ impl TimezoneOptions {
> }
> }
>
> +/// Options controlling the behaviour of the network interface pinning (by
> +/// creating appropriate systemd.link files) during the installation.
> +#[derive(Clone, Debug, Default, PartialEq, Deserialize)]
> +#[serde(rename_all = "kebab-case", deny_unknown_fields)]
> +pub struct NetworkInterfacePinningOptions {
> + /// Maps MAC address to custom name
> + #[serde(default)]
> + pub mapping: HashMap<String, String>,
> +}
> +
> +impl NetworkInterfacePinningOptions {
> + /// Default prefix to prepend to the pinned interface ID as received from the low-level
> + /// installer.
> + pub const DEFAULT_PREFIX: &str = "nic";
> +
> + /// Do some basic checks on the options.
> + ///
> + /// This checks for:
> + /// - empty interface names
> + /// - overlong interface names
> + /// - duplicate interface names
imo it would make sense to additionally also have the check that the
interface name is not fully numeric, since according to the docs, this
is not allowed [0].
[0] https://manpages.debian.org/testing/udev/systemd.link.5.en.html#%5BLINK%5D_SECTION_OPTIONS
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [pve-devel] [PATCH installer 06/14] common: implement support for `network_interface_pin_map` config
2025-10-21 14:05 ` Michael Köppl
@ 2025-10-22 9:37 ` Christoph Heiss
0 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-22 9:37 UTC (permalink / raw)
To: Michael Köppl; +Cc: Proxmox VE development discussion
On Tue Oct 21, 2025 at 4:05 PM CEST, Michael Köppl wrote:
> 1 comment inline
>
> On Tue Oct 14, 2025 at 3:21 PM CEST, Christoph Heiss wrote:
[..]
>> +impl NetworkInterfacePinningOptions {
>> + /// Default prefix to prepend to the pinned interface ID as received from the low-level
>> + /// installer.
>> + pub const DEFAULT_PREFIX: &str = "nic";
>> +
>> + /// Do some basic checks on the options.
>> + ///
>> + /// This checks for:
>> + /// - empty interface names
>> + /// - overlong interface names
>> + /// - duplicate interface names
>
> imo it would make sense to additionally also have the check that the
> interface name is not fully numeric, since according to the docs, this
> is not allowed [0].
>
> [0] https://manpages.debian.org/testing/udev/systemd.link.5.en.html#%5BLINK%5D_SECTION_OPTIONS
Good catch, thanks!
I'll add it for v2, in addition to the other restrictions mentioned for
Name= in the man page.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* [pve-devel] [PATCH installer 07/14] auto: add support for pinning network interface names
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
` (5 preceding siblings ...)
2025-10-14 13:21 ` [pve-devel] [PATCH installer 06/14] common: implement support for `network_interface_pin_map` config Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 08/14] assistant: verify network settings in `validate-answer` subcommand Christoph Heiss
` (7 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
Introduce a new `[network.interface-name-pinning]` section in the answer
file, which is just a (TOML) table mapping MAC addresses to interface
names.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-auto-installer/src/answer.rs | 63 ++++++++++++++-----
proxmox-auto-installer/src/utils.rs | 40 ++++++++++--
proxmox-auto-installer/tests/parse-answer.rs | 2 +
.../network_interface_pinning.json | 30 +++++++++
.../network_interface_pinning.toml | 22 +++++++
...rface_pinning_overlong_interface_name.json | 3 +
...rface_pinning_overlong_interface_name.toml | 18 ++++++
7 files changed, 159 insertions(+), 19 deletions(-)
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.toml
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.toml
diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
index 88f4c87..1e455ca 100644
--- a/proxmox-auto-installer/src/answer.rs
+++ b/proxmox-auto-installer/src/answer.rs
@@ -1,13 +1,17 @@
-use anyhow::{Result, format_err};
+use anyhow::{Result, bail, format_err};
use proxmox_installer_common::{
options::{
- BtrfsCompressOption, BtrfsRaidLevel, FsType, ZfsChecksumOption, ZfsCompressOption,
- ZfsRaidLevel,
+ BtrfsCompressOption, BtrfsRaidLevel, FsType, NetworkInterfacePinningOptions,
+ ZfsChecksumOption, ZfsCompressOption, ZfsRaidLevel,
},
utils::{CidrAddress, Fqdn},
};
use serde::{Deserialize, Serialize};
-use std::{collections::BTreeMap, io::BufRead, net::IpAddr};
+use std::{
+ collections::{BTreeMap, HashMap},
+ io::BufRead,
+ net::IpAddr,
+};
// NOTE New answer file properties must use kebab-case, but should allow snake_case for backwards
// compatibility. TODO Remove the snake_cased variants in a future major version (e.g. PVE 10).
@@ -178,6 +182,18 @@ enum NetworkConfigMode {
FromAnswer,
}
+/// Options controlling the behaviour of the network interface pinning (by
+/// creating appropriate systemd.link files) during the installation.
+#[derive(Clone, Debug, Default, PartialEq, Deserialize)]
+#[serde(rename_all = "kebab-case", deny_unknown_fields)]
+pub struct NetworkInterfacePinningOptionsAnswer {
+ /// Whether interfaces should be pinned during the installation.
+ pub enabled: bool,
+ /// Maps MAC address to custom name
+ #[serde(default)]
+ pub mapping: HashMap<String, String>,
+}
+
#[derive(Clone, Deserialize, Debug)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
struct NetworkInAnswer {
@@ -188,30 +204,47 @@ struct NetworkInAnswer {
pub gateway: Option<IpAddr>,
#[serde(default)]
pub filter: BTreeMap<String, String>,
+ /// Controls network interface pinning behaviour during installation.
+ /// Off by default. Allowed for both `from-dhcp` and `from-answer` modes.
+ #[serde(default)]
+ pub interface_name_pinning: Option<NetworkInterfacePinningOptionsAnswer>,
}
#[derive(Clone, Deserialize, Debug)]
#[serde(try_from = "NetworkInAnswer", deny_unknown_fields)]
pub struct Network {
pub network_settings: NetworkSettings,
+ /// Controls network interface pinning behaviour during installation.
+ pub interface_name_pinning: Option<NetworkInterfacePinningOptions>,
}
impl TryFrom<NetworkInAnswer> for Network {
- type Error = &'static str;
+ type Error = anyhow::Error;
+
+ fn try_from(network: NetworkInAnswer) -> Result<Self> {
+ let interface_name_pinning = match network.interface_name_pinning {
+ Some(opts) if opts.enabled => {
+ let opts = NetworkInterfacePinningOptions {
+ mapping: opts.mapping,
+ };
+ opts.verify()?;
+ Some(opts)
+ }
+ _ => None,
+ };
- fn try_from(network: NetworkInAnswer) -> Result<Self, Self::Error> {
if network.source == NetworkConfigMode::FromAnswer {
if network.cidr.is_none() {
- return Err("Field 'cidr' must be set.");
+ bail!("Field 'cidr' must be set.");
}
if network.dns.is_none() {
- return Err("Field 'dns' must be set.");
+ bail!("Field 'dns' must be set.");
}
if network.gateway.is_none() {
- return Err("Field 'gateway' must be set.");
+ bail!("Field 'gateway' must be set.");
}
if network.filter.is_empty() {
- return Err("Field 'filter' must be set.");
+ bail!("Field 'filter' must be set.");
}
Ok(Network {
@@ -221,23 +254,25 @@ impl TryFrom<NetworkInAnswer> for Network {
gateway: network.gateway.unwrap(),
filter: network.filter,
}),
+ interface_name_pinning,
})
} else {
if network.cidr.is_some() {
- return Err("Field 'cidr' not supported for 'from-dhcp' config.");
+ bail!("Field 'cidr' not supported for 'from-dhcp' config.");
}
if network.dns.is_some() {
- return Err("Field 'dns' not supported for 'from-dhcp' config.");
+ bail!("Field 'dns' not supported for 'from-dhcp' config.");
}
if network.gateway.is_some() {
- return Err("Field 'gateway' not supported for 'from-dhcp' config.");
+ bail!("Field 'gateway' not supported for 'from-dhcp' config.");
}
if !network.filter.is_empty() {
- return Err("Field 'filter' not supported for 'from-dhcp' config.");
+ bail!("Field 'filter' not supported for 'from-dhcp' config.");
}
Ok(Network {
network_settings: NetworkSettings::FromDhcp,
+ interface_name_pinning,
})
}
}
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index eb666d1..14085a4 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -2,14 +2,14 @@ use anyhow::{Context, Result, bail};
use glob::Pattern;
use log::info;
use std::{
- collections::{BTreeMap, HashMap, HashSet},
+ collections::{BTreeMap, HashSet},
process::Command,
};
use crate::{
answer::{
self, Answer, DiskSelection, FirstBootHookSourceMode, FqdnConfig, FqdnExtendedConfig,
- FqdnSourceMode,
+ FqdnSourceMode, Network,
},
udevinfo::UdevInfo,
};
@@ -35,7 +35,12 @@ fn get_network_settings(
let mut network_options = match &answer.global.fqdn {
// If the user set a static FQDN in the answer file, override it
FqdnConfig::Simple(name) => {
- let mut opts = NetworkOptions::defaults_from(setup_info, &runtime_info.network, None);
+ let mut opts = NetworkOptions::defaults_from(
+ setup_info,
+ &runtime_info.network,
+ None,
+ answer.network.interface_name_pinning.as_ref(),
+ );
opts.fqdn = name.to_owned();
opts
}
@@ -58,7 +63,12 @@ fn get_network_settings(
bail!("no domain received from DHCP server and `global.fqdn.domain` is unset!");
}
- NetworkOptions::defaults_from(setup_info, &runtime_info.network, domain.as_deref())
+ NetworkOptions::defaults_from(
+ setup_info,
+ &runtime_info.network,
+ domain.as_deref(),
+ answer.network.interface_name_pinning.as_ref(),
+ )
}
};
@@ -68,6 +78,12 @@ fn get_network_settings(
network_options.gateway = settings.gateway;
network_options.ifname = get_single_udev_index(&settings.filter, &udev_info.nics)?;
}
+
+ if let Some(opts) = &network_options.pinning_opts {
+ info!("Network interface name pinning is enabled");
+ opts.verify()?;
+ }
+
info!("Network interface used is '{}'", &network_options.ifname);
Ok(network_options)
}
@@ -430,6 +446,16 @@ pub fn verify_first_boot_settings(answer: &Answer) -> Result<()> {
Ok(())
}
+pub fn verify_network_settings(network: &Network) -> Result<()> {
+ info!("Verifying network settings");
+
+ if let Some(pin_opts) = &network.interface_name_pinning {
+ pin_opts.verify()?;
+ }
+
+ Ok(())
+}
+
pub fn parse_answer(
answer: &Answer,
udev_info: &UdevInfo,
@@ -451,6 +477,7 @@ pub fn parse_answer(
verify_disks_settings(answer)?;
verify_email_and_root_password_settings(answer)?;
verify_first_boot_settings(answer)?;
+ verify_network_settings(&answer.network)?;
let root_password = match (
&answer.global.root_password,
@@ -485,7 +512,10 @@ pub fn parse_answer(
root_ssh_keys: answer.global.root_ssh_keys.clone(),
mngmt_nic: network_settings.ifname,
- network_interface_pin_map: HashMap::new(),
+ network_interface_pin_map: network_settings
+ .pinning_opts
+ .map(|o| o.mapping)
+ .unwrap_or_default(),
hostname: network_settings
.fqdn
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 6754374..696fe1f 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -129,6 +129,7 @@ mod tests {
full_fqdn_from_dhcp_with_default_domain,
hashed_root_password,
minimal,
+ network_interface_pinning,
nic_matching,
specific_nic,
zfs,
@@ -149,6 +150,7 @@ mod tests {
fqdn_hostname_only,
ipv4_and_subnet_mask_33,
lvm_swapsize_greater_than_hdsize,
+ network_interface_pinning_overlong_interface_name,
no_fqdn_from_dhcp,
no_root_password_set,
short_password,
diff --git a/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.json b/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.json
new file mode 100644
index 0000000..76723c8
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.json
@@ -0,0 +1,30 @@
+{
+ "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": "mgmt",
+ "network_interface_pin_map": {
+ "1c:34:da:5c:5e:24": "nic2",
+ "1c:34:da:5c:5e:25": "nic3",
+ "24:8a:07:1e:05:bc": "lan0",
+ "24:8a:07:1e:05:bd": "lan1",
+ "5a:47:32:dd:c7:47": "nic8",
+ "a0:36:9f:0a:b3:82": "nic6",
+ "a0:36:9f:0a:b3:83": "nic7",
+ "b4:2e:99:ac:ad:b4": "mgmt",
+ "b4:2e:99:ac:ad:b5": "nic1"
+ },
+ "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_interface_pinning.toml b/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.toml
new file mode 100644
index 0000000..d9a2110
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.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.interface-name-pinning]
+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"
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
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
new file mode 100644
index 0000000..70e196c
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json
@@ -0,0 +1,3 @@
+{
+ "parse-error": "error parsing answer.toml: interface name mapping 'waytoolonginterfacename' for 'ab:cd:ef:12:34:56' cannot be longer than 15 characters"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.toml
new file mode 100644
index 0000000..e82b47d
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.toml
@@ -0,0 +1,18 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "pveauto.fail.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-dhcp"
+interface-name-pinning.enabled = true
+
+[network.interface-name-pinning.mapping]
+"ab:cd:ef:12:34:56" = "waytoolonginterfacename"
+
+[disk-setup]
+filesystem = "ext4"
+disk-list = ["sda"]
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* [pve-devel] [PATCH installer 08/14] assistant: verify network settings in `validate-answer` subcommand
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
` (6 preceding siblings ...)
2025-10-14 13:21 ` [pve-devel] [PATCH installer 07/14] auto: add support for pinning network interface names Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 09/14] post-hook: avoid redundant Option<bool> for (de-)serialization Christoph Heiss
` (6 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-auto-install-assistant/src/main.rs | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index c0d932c..093169a 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -23,7 +23,7 @@ use proxmox_auto_installer::{
AutoInstSettings, FetchAnswerFrom, HttpOptions, default_partition_label,
get_matched_udev_indexes, get_nic_list, get_single_udev_index, verify_disks_settings,
verify_email_and_root_password_settings, verify_first_boot_settings,
- verify_locale_settings,
+ verify_locale_settings, verify_network_settings,
},
};
use proxmox_installer_common::{FIRST_BOOT_EXEC_MAX_SIZE, FIRST_BOOT_EXEC_NAME, cli};
@@ -909,6 +909,7 @@ fn parse_answer(path: impl AsRef<Path> + fmt::Debug) -> Result<Answer> {
verify_disks_settings(&answer)?;
verify_first_boot_settings(&answer)?;
verify_email_and_root_password_settings(&answer)?;
+ verify_network_settings(&answer.network)?;
Ok(answer)
}
Err(err) => bail!("Error parsing answer file: {err}"),
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* [pve-devel] [PATCH installer 09/14] post-hook: avoid redundant Option<bool> for (de-)serialization
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
` (7 preceding siblings ...)
2025-10-14 13:21 ` [pve-devel] [PATCH installer 08/14] assistant: verify network settings in `validate-answer` subcommand Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 10/14] post-hook: add network interface name and pinning status Christoph Heiss
` (5 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
Instead, for the serialization case just skip it if the value is falsy,
for deserialization default-initialize it with `false`.
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-installer-common/src/setup.rs | 12 ++----------
proxmox-post-hook/src/main.rs | 27 +++++++++++++++------------
2 files changed, 17 insertions(+), 22 deletions(-)
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 3e99576..1a584ba 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -275,14 +275,6 @@ where
Ok(val != 0)
}
-fn deserialize_bool_from_int_maybe<'de, D>(deserializer: D) -> Result<Option<bool>, D::Error>
-where
- D: Deserializer<'de>,
-{
- let val: Option<u32> = Deserialize::deserialize(deserializer)?;
- Ok(val.map(|v| v != 0))
-}
-
fn deserialize_cczones_map<'de, D>(
deserializer: D,
) -> Result<HashMap<String, Vec<String>>, D::Error>
@@ -389,8 +381,8 @@ pub struct RuntimeInfo {
pub hvm_supported: bool,
/// Whether the system was booted with SecureBoot enabled
- #[serde(default, deserialize_with = "deserialize_bool_from_int_maybe")]
- pub secure_boot: Option<bool>,
+ #[serde(default, deserialize_with = "deserialize_bool_from_int")]
+ pub secure_boot: bool,
/// Default upper limit for the ZFS ARC size, in MiB.
pub default_zfs_arc_max: usize,
diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
index bd27121..0a9e661 100644
--- a/proxmox-post-hook/src/main.rs
+++ b/proxmox-post-hook/src/main.rs
@@ -44,8 +44,8 @@ struct BootInfo {
/// Whether the system is booted using UEFI or legacy BIOS.
mode: BootType,
/// Whether SecureBoot is enabled for the installation.
- #[serde(skip_serializing_if = "Option::is_none")]
- secureboot: Option<bool>,
+ #[serde(skip_serializing_if = "bool_is_false")]
+ secureboot: bool,
}
/// Holds all the public keys for the different algorithms available.
@@ -66,8 +66,8 @@ struct DiskInfo {
/// Size in bytes
size: usize,
/// Set to true if the disk is used for booting.
- #[serde(skip_serializing_if = "Option::is_none")]
- is_bootdisk: Option<bool>,
+ #[serde(skip_serializing_if = "bool_is_false")]
+ is_bootdisk: bool,
/// Properties about the device as given by udev.
udev_properties: UdevProperties,
}
@@ -83,12 +83,16 @@ struct NetworkInterfaceInfo {
address: Option<CidrAddress>,
/// Set to true if the interface is the chosen management interface during
/// installation.
- #[serde(skip_serializing_if = "Option::is_none")]
- is_management: Option<bool>,
+ #[serde(skip_serializing_if = "bool_is_false")]
+ is_management: bool,
/// Properties about the device as given by udev.
udev_properties: UdevProperties,
}
+fn bool_is_false(value: &bool) -> bool {
+ !value
+}
+
/// Information about the installed product itself.
#[derive(Serialize)]
#[serde(rename_all = "kebab-case")]
@@ -323,7 +327,8 @@ impl PostHookInfo {
let is_bootdisk = config
.target_hd
.as_ref()
- .and_then(|hd| (*hd == disk.path).then_some(true));
+ .map(|hd| *hd == disk.path)
+ .unwrap_or_default();
anyhow::Ok(DiskInfo {
size: (config.hdsize * (SIZE_GIB as f64)) as usize,
@@ -341,9 +346,7 @@ impl PostHookInfo {
.disks
.iter()
.flat_map(|disk| {
- let is_bootdisk = selected_disks_indices
- .contains(&&disk.index)
- .then_some(true);
+ let is_bootdisk = selected_disks_indices.contains(&&disk.index);
anyhow::Ok(DiskInfo {
size: (config.hdsize * (SIZE_GIB as f64)) as usize,
@@ -389,14 +392,14 @@ impl PostHookInfo {
anyhow::Ok(NetworkInterfaceInfo {
mac: nic.mac.clone(),
address: Some(config.cidr.clone()),
- is_management: Some(true),
+ is_management: true,
udev_properties,
})
} else {
anyhow::Ok(NetworkInterfaceInfo {
mac: nic.mac.clone(),
address: None,
- is_management: None,
+ is_management: false,
udev_properties,
})
}
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* [pve-devel] [PATCH installer 10/14] post-hook: add network interface name and pinning status
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
` (8 preceding siblings ...)
2025-10-14 13:21 ` [pve-devel] [PATCH installer 09/14] post-hook: avoid redundant Option<bool> for (de-)serialization Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 11/14] tui: views: move network options view to own module Christoph Heiss
` (4 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
Adds a new `name` and `is-pinned` attribute to each reported network interface in
the post-hook data.
Also increments the minor schema version by one, as it is a
backwards-compatible change.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-post-hook/src/main.rs | 39 +++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
index 0a9e661..ce64533 100644
--- a/proxmox-post-hook/src/main.rs
+++ b/proxmox-post-hook/src/main.rs
@@ -76,6 +76,8 @@ struct DiskInfo {
#[derive(Serialize)]
#[serde(rename_all = "kebab-case")]
struct NetworkInterfaceInfo {
+ /// Name of the interface
+ name: String,
/// MAC address of the interface
mac: String,
/// (Designated) IP address of the interface
@@ -85,6 +87,10 @@ struct NetworkInterfaceInfo {
/// installation.
#[serde(skip_serializing_if = "bool_is_false")]
is_management: bool,
+ /// Set to true if the network interface name was pinned based on the MAC
+ /// address during the installation.
+ #[serde(skip_serializing_if = "bool_is_false")]
+ is_pinned: bool,
/// Properties about the device as given by udev.
udev_properties: UdevProperties,
}
@@ -151,7 +157,7 @@ struct PostHookInfoSchema {
}
impl PostHookInfoSchema {
- const SCHEMA_VERSION: &str = "1.1";
+ const SCHEMA_VERSION: &str = "1.2";
}
impl Default for PostHookInfoSchema {
@@ -386,23 +392,24 @@ impl PostHookInfo {
})?
.clone();
- if config.mngmt_nic == nic.name {
+ let is_pinned = config.network_interface_pin_map.contains_key(&nic.mac);
+ let ifname = config
+ .network_interface_pin_map
+ .get(&nic.mac)
+ .unwrap_or(&nic.name);
+
+ let is_management = config.mngmt_nic == *ifname;
+
+ anyhow::Ok(NetworkInterfaceInfo {
+ name: ifname.clone(),
+ mac: nic.mac.clone(),
// Use the actual IP address from the low-level install config, as the runtime info
// contains the original IP address from DHCP.
- anyhow::Ok(NetworkInterfaceInfo {
- mac: nic.mac.clone(),
- address: Some(config.cidr.clone()),
- is_management: true,
- udev_properties,
- })
- } else {
- anyhow::Ok(NetworkInterfaceInfo {
- mac: nic.mac.clone(),
- address: None,
- is_management: false,
- udev_properties,
- })
- }
+ address: is_management.then_some(config.cidr.clone()),
+ is_management,
+ is_pinned,
+ udev_properties,
+ })
})
.collect())
}
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* [pve-devel] [PATCH installer 11/14] tui: views: move network options view to own module
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
` (9 preceding siblings ...)
2025-10-14 13:21 ` [pve-devel] [PATCH installer 10/14] post-hook: add network interface name and pinning status Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 12/14] tui: views: form: allow attaching user-defined data to children Christoph Heiss
` (3 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
In preparation for adding network interface name pinning, which will
introduce quite a bit more functionality.
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-tui-installer/src/main.rs | 90 +---------------
proxmox-tui-installer/src/views/mod.rs | 3 +
proxmox-tui-installer/src/views/network.rs | 113 +++++++++++++++++++++
3 files changed, 121 insertions(+), 85 deletions(-)
create mode 100644 proxmox-tui-installer/src/views/network.rs
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 15ee5d3..b24f90b 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -1,6 +1,6 @@
#![forbid(unsafe_code)]
-use std::{collections::HashMap, env, net::IpAddr};
+use std::{collections::HashMap, env};
use cursive::{
Cursive, CursiveRunnable, ScreenId, View, XY,
@@ -9,7 +9,7 @@ use cursive::{
view::{Nameable, Offset, Resizable, ViewWrapper},
views::{
Button, Checkbox, Dialog, DummyView, EditView, Layer, LinearLayout, PaddedView, Panel,
- ResizedView, ScrollView, SelectView, StackView, TextView,
+ ResizedView, ScrollView, StackView, TextView,
},
};
@@ -20,7 +20,6 @@ use proxmox_installer_common::{
ROOT_PASSWORD_MIN_LENGTH,
options::{BootdiskOptions, NetworkOptions, TimezoneOptions, email_validate},
setup::{LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo, installer_setup},
- utils::{CidrAddress, Fqdn},
};
mod setup;
@@ -28,7 +27,7 @@ mod system;
mod views;
use views::{
- BootdiskOptionsView, CidrAddressEditView, FormView, InstallProgressView, TableView,
+ BootdiskOptionsView, FormView, InstallProgressView, NetworkOptionsView, TableView,
TableViewItem, TimezoneOptionsView,
};
@@ -483,91 +482,12 @@ fn password_dialog(siv: &mut Cursive) -> InstallerView {
fn network_dialog(siv: &mut Cursive) -> InstallerView {
let state = siv.user_data::<InstallerState>().unwrap();
let options = &state.options.network;
- let ifaces = state.runtime_info.network.interfaces.values();
- let ifnames = ifaces
- .clone()
- .map(|iface| (iface.render(), iface.name.clone()));
- let mut ifaces_selection = SelectView::new().popup().with_all(ifnames.clone());
-
- // sort first to always have stable view
- ifaces_selection.sort();
- let selected = ifaces_selection
- .iter()
- .position(|(_label, iface)| *iface == options.ifname)
- .unwrap_or(ifaces.len() - 1);
-
- ifaces_selection.set_selection(selected);
-
- let inner = FormView::new()
- .child("Management interface", ifaces_selection)
- .child(
- "Hostname (FQDN)",
- EditView::new().content(options.fqdn.to_string()),
- )
- .child(
- "IP address (CIDR)",
- CidrAddressEditView::new().content(options.address.clone()),
- )
- .child(
- "Gateway address",
- EditView::new().content(options.gateway.to_string()),
- )
- .child(
- "DNS server address",
- EditView::new().content(options.dns_server.to_string()),
- )
- .with_name("network-options");
InstallerView::new(
state,
- inner,
+ NetworkOptionsView::new(options, &state.runtime_info.network).with_name("network-options"),
Box::new(|siv| {
- let options = siv.call_on_name("network-options", |view: &mut FormView| {
- let ifname = view
- .get_value::<SelectView, _>(0)
- .ok_or("failed to retrieve management interface name")?;
-
- let fqdn = view
- .get_value::<EditView, _>(1)
- .ok_or("failed to retrieve host FQDN")?
- .parse::<Fqdn>()
- .map_err(|err| format!("hostname does not look valid:\n\n{err}"))?;
-
- let address = view
- .get_value::<CidrAddressEditView, _>(2)
- .ok_or("failed to retrieve host address".to_string())
- .and_then(|(ip_addr, mask)| {
- CidrAddress::new(ip_addr, mask).map_err(|err| err.to_string())
- })?;
-
- let gateway = view
- .get_value::<EditView, _>(3)
- .ok_or("failed to retrieve gateway address")?
- .parse::<IpAddr>()
- .map_err(|err| err.to_string())?;
-
- let dns_server = view
- .get_value::<EditView, _>(4)
- .ok_or("failed to retrieve DNS server address")?
- .parse::<IpAddr>()
- .map_err(|err| err.to_string())?;
-
- if address.addr().is_ipv4() != gateway.is_ipv4() {
- Err("host and gateway IP address version must not differ".to_owned())
- } else if address.addr().is_ipv4() != dns_server.is_ipv4() {
- Err("host and DNS IP address version must not differ".to_owned())
- } else if fqdn.to_string().ends_with(".invalid") {
- Err("hostname does not look valid".to_owned())
- } else {
- Ok(NetworkOptions {
- ifname,
- fqdn,
- address,
- gateway,
- dns_server,
- })
- }
- });
+ let options = siv.call_on_name("network-options", NetworkOptionsView::get_values);
match options {
Some(Ok(options)) => {
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 537e3ed..43ca999 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -16,6 +16,9 @@ pub use bootdisk::*;
mod install_progress;
pub use install_progress::*;
+mod network;
+pub use network::*;
+
mod tabbed_view;
pub use tabbed_view::*;
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
new file mode 100644
index 0000000..5e3e258
--- /dev/null
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -0,0 +1,113 @@
+use std::net::IpAddr;
+
+use cursive::{
+ view::ViewWrapper,
+ views::{EditView, SelectView},
+};
+
+use super::{CidrAddressEditView, FormView};
+use proxmox_installer_common::{
+ options::NetworkOptions,
+ setup::NetworkInfo,
+ utils::{CidrAddress, Fqdn},
+};
+
+pub struct NetworkOptionsView {
+ view: FormView,
+}
+
+impl NetworkOptionsView {
+ pub fn new(options: &NetworkOptions, network_info: &NetworkInfo) -> Self {
+ let ifaces = network_info.interfaces.values();
+ let ifnames = ifaces
+ .clone()
+ .map(|iface| (iface.render(), iface.name.clone()));
+ let mut ifaces_selection = SelectView::new().popup().with_all(ifnames.clone());
+
+ // sort first to always have stable view
+ ifaces_selection.sort();
+ let selected = ifaces_selection
+ .iter()
+ .position(|(_label, iface)| *iface == options.ifname)
+ .unwrap_or(ifaces.len() - 1);
+
+ ifaces_selection.set_selection(selected);
+
+ let view = FormView::new()
+ .child("Management interface", ifaces_selection)
+ .child(
+ "Hostname (FQDN)",
+ EditView::new().content(options.fqdn.to_string()),
+ )
+ .child(
+ "IP address (CIDR)",
+ CidrAddressEditView::new().content(options.address.clone()),
+ )
+ .child(
+ "Gateway address",
+ EditView::new().content(options.gateway.to_string()),
+ )
+ .child(
+ "DNS server address",
+ EditView::new().content(options.dns_server.to_string()),
+ );
+
+ Self { view }
+ }
+
+ pub fn get_values(&mut self) -> Result<NetworkOptions, String> {
+ let ifname = self
+ .view
+ .get_value::<SelectView, _>(0)
+ .ok_or("failed to retrieve management interface name")?;
+
+ let fqdn = self
+ .view
+ .get_value::<EditView, _>(1)
+ .ok_or("failed to retrieve host FQDN")?
+ .parse::<Fqdn>()
+ .map_err(|err| format!("hostname does not look valid:\n\n{err}"))?;
+
+ let address = self
+ .view
+ .get_value::<CidrAddressEditView, _>(2)
+ .ok_or("failed to retrieve host address".to_string())
+ .and_then(|(ip_addr, mask)| {
+ CidrAddress::new(ip_addr, mask).map_err(|err| err.to_string())
+ })?;
+
+ let gateway = self
+ .view
+ .get_value::<EditView, _>(3)
+ .ok_or("failed to retrieve gateway address")?
+ .parse::<IpAddr>()
+ .map_err(|err| err.to_string())?;
+
+ let dns_server = self
+ .view
+ .get_value::<EditView, _>(4)
+ .ok_or("failed to retrieve DNS server address")?
+ .parse::<IpAddr>()
+ .map_err(|err| err.to_string())?;
+
+ if address.addr().is_ipv4() != gateway.is_ipv4() {
+ Err("host and gateway IP address version must not differ".to_owned())
+ } else if address.addr().is_ipv4() != dns_server.is_ipv4() {
+ Err("host and DNS IP address version must not differ".to_owned())
+ } else if fqdn.to_string().ends_with(".invalid") {
+ Err("hostname does not look valid".to_owned())
+ } else {
+ Ok(NetworkOptions {
+ ifname,
+ fqdn,
+ address,
+ gateway,
+ dns_server,
+ })
+ }
+ }
+}
+
+impl ViewWrapper for NetworkOptionsView {
+ cursive::wrap_impl!(self.view: FormView);
+}
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* [pve-devel] [PATCH installer 12/14] tui: views: form: allow attaching user-defined data to children
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
` (10 preceding siblings ...)
2025-10-14 13:21 ` [pve-devel] [PATCH installer 11/14] tui: views: move network options view to own module Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 13/14] tui: add support for pinning network interface names Christoph Heiss
` (2 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
Allows storing (typesafe) arbitrary data together with each child.
No functional changes for existing code.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-tui-installer/src/main.rs | 2 +-
proxmox-tui-installer/src/views/bootdisk.rs | 6 ++--
proxmox-tui-installer/src/views/mod.rs | 33 +++++++++++++++++----
proxmox-tui-installer/src/views/network.rs | 2 +-
4 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index b24f90b..fce9fc2 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -420,7 +420,7 @@ fn password_dialog(siv: &mut Cursive) -> InstallerView {
let state = siv.user_data::<InstallerState>().unwrap();
let options = &state.options.password;
- let inner = FormView::new()
+ let inner = FormView::<()>::new()
.child(
"Root password [at least 8 characters]",
EditView::new().secret(),
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 9f6d235..3566814 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -47,7 +47,7 @@ impl BootdiskOptionsView {
pub fn new(siv: &mut Cursive, runinfo: &RuntimeInfo, options: &BootdiskOptions) -> Self {
let advanced_options = Arc::new(Mutex::new(options.clone()));
- let bootdisk_form = FormView::new()
+ let bootdisk_form = FormView::<()>::new()
.child(
"Target harddisk",
target_bootdisk_selectview(
@@ -152,7 +152,7 @@ impl AdvancedBootdiskOptionsView {
let mut view = LinearLayout::vertical()
.child(DummyView.full_width())
- .child(FormView::new().child("Filesystem", fstype_select))
+ .child(FormView::<()>::new().child("Filesystem", fstype_select))
.child(DummyView.full_width());
// Create the appropriate (inner) advanced options view
@@ -493,7 +493,7 @@ impl<T: View> MultiDiskOptionsView<T> {
selectable_disks.push(("-- do not use --".to_owned(), None));
- let mut disk_form = FormView::new();
+ let mut disk_form = FormView::<()>::new();
for (i, _) in avail_disks.iter().enumerate() {
disk_form.add_child(
&format!("Harddisk {i}"),
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 43ca999..5723fed 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -1,4 +1,4 @@
-use std::{net::IpAddr, str::FromStr, sync::Arc};
+use std::{collections::HashMap, net::IpAddr, str::FromStr, sync::Arc};
use cursive::{
Printer, Rect, Vec2, View,
@@ -414,17 +414,21 @@ impl FormViewGetValue<f64> for DiskSizeEditView {
}
}
-pub struct FormView {
+pub struct FormView<UDT = ()> {
view: LinearLayout,
+ user_data: HashMap<usize, UDT>,
}
-impl FormView {
+impl<UDT> FormView<UDT> {
pub fn new() -> Self {
let view = LinearLayout::horizontal()
.child(LinearLayout::vertical().full_width())
.child(LinearLayout::vertical().full_width());
- Self { view }
+ Self {
+ view,
+ user_data: HashMap::new(),
+ }
}
pub fn add_child(&mut self, label: &str, view: impl View) {
@@ -432,6 +436,12 @@ impl FormView {
self.add_to_column(1, view);
}
+ pub fn add_child_with_data(&mut self, label: &str, view: impl View, data: UDT) {
+ self.add_to_column(0, TextView::new(format!("{label}: ")).no_wrap());
+ self.add_to_column(1, view);
+ self.user_data.insert(self.len() - 1, data);
+ }
+
pub fn child(mut self, label: &str, view: impl View) -> Self {
self.add_child(label, view);
self
@@ -453,6 +463,19 @@ impl FormView {
.downcast_ref::<T>()
}
+ pub fn get_child_with_data<T: View>(&self, index: usize) -> Option<(&T, &UDT)> {
+ let view = self
+ .view
+ .get_child(1)?
+ .downcast_ref::<ResizedView<LinearLayout>>()?
+ .get_inner()
+ .get_child(index)?
+ .downcast_ref::<T>()?;
+
+ let data = self.user_data.get(&index)?;
+ Some((view, data))
+ }
+
pub fn get_child_mut<T: View>(&mut self, index: usize) -> Option<&mut T> {
self.view
.get_child_mut(1)?
@@ -509,7 +532,7 @@ impl FormView {
}
}
-impl ViewWrapper for FormView {
+impl<UDT: Send + Sync + 'static> ViewWrapper for FormView<UDT> {
cursive::wrap_impl!(self.view: LinearLayout);
fn wrap_important_area(&self, size: Vec2) -> Rect {
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
index 5e3e258..960c25e 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -33,7 +33,7 @@ impl NetworkOptionsView {
ifaces_selection.set_selection(selected);
- let view = FormView::new()
+ let form = FormView::<()>::new()
.child("Management interface", ifaces_selection)
.child(
"Hostname (FQDN)",
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* [pve-devel] [PATCH installer 13/14] tui: add support for pinning network interface names
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
` (11 preceding siblings ...)
2025-10-14 13:21 ` [pve-devel] [PATCH installer 12/14] tui: views: form: allow attaching user-defined data to children Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-14 13:21 ` [pve-devel] [PATCH installer 14/14] gui: " Christoph Heiss
2025-10-21 14:04 ` [pve-devel] [PATCH installer 00/14] support network interface name pinning Michael Köppl
14 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
Adds an additional checkbox and option button in the network panel, the
latter triggering a dialog for setting custom names per network
interface present on the system.
Pinning is enabled by default.
Each pinned network interface name defaults to `nicN`, where N is the
pinned ID from the low-level installer.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
proxmox-installer-common/src/setup.rs | 10 +-
proxmox-tui-installer/src/main.rs | 13 +-
proxmox-tui-installer/src/setup.rs | 6 +-
proxmox-tui-installer/src/views/mod.rs | 5 +
proxmox-tui-installer/src/views/network.rs | 357 +++++++++++++++++++--
5 files changed, 353 insertions(+), 38 deletions(-)
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 1a584ba..c93ee30 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -436,7 +436,7 @@ pub struct Gateway {
pub gateway: IpAddr,
}
-#[derive(Clone, Deserialize, Serialize)]
+#[derive(Clone, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)]
#[serde(rename_all = "UPPERCASE")]
/// The current interface state as reported by the kernel.
pub enum InterfaceState {
@@ -483,7 +483,13 @@ pub struct Interface {
impl Interface {
// avoid display trait as this is not the string representation for a serializer
pub fn render(&self) -> String {
- format!("{} {} ({})", self.state.render(), self.name, self.mac)
+ format!(
+ "{} {} ({}, {})",
+ self.state.render(),
+ self.name,
+ self.mac,
+ self.driver
+ )
}
pub fn to_pinned(&self, options: &NetworkInterfacePinningOptions) -> Self {
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index fce9fc2..cd590b8 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -18,7 +18,10 @@ use options::{InstallerOptions, PasswordOptions};
use proxmox_installer_common::{
ROOT_PASSWORD_MIN_LENGTH,
- options::{BootdiskOptions, NetworkOptions, TimezoneOptions, email_validate},
+ options::{
+ BootdiskOptions, NetworkInterfacePinningOptions, NetworkOptions, TimezoneOptions,
+ email_validate,
+ },
setup::{LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo, installer_setup},
};
mod setup;
@@ -167,7 +170,13 @@ fn main() {
bootdisk: BootdiskOptions::defaults_from(&runtime_info.disks[0]),
timezone: TimezoneOptions::defaults_from(&runtime_info, &locales),
password: Default::default(),
- network: NetworkOptions::defaults_from(&setup_info, &runtime_info.network, None),
+ network: NetworkOptions::defaults_from(
+ &setup_info,
+ &runtime_info.network,
+ None,
+ // We enable network interface pinning by default in the TUI
+ Some(&NetworkInterfacePinningOptions::default()),
+ ),
autoreboot: true,
},
setup_info,
diff --git a/proxmox-tui-installer/src/setup.rs b/proxmox-tui-installer/src/setup.rs
index d2ffb70..3ab1869 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -1,4 +1,4 @@
-use std::collections::{BTreeMap, HashMap};
+use std::collections::BTreeMap;
use crate::options::InstallerOptions;
use proxmox_installer_common::{
@@ -8,6 +8,8 @@ use proxmox_installer_common::{
impl From<InstallerOptions> for InstallConfig {
fn from(options: InstallerOptions) -> Self {
+ let pinning_opts = options.network.pinning_opts.as_ref();
+
let mut config = Self {
autoreboot: options.autoreboot as usize,
@@ -32,7 +34,7 @@ impl From<InstallerOptions> for InstallConfig {
root_ssh_keys: vec![],
mngmt_nic: options.network.ifname,
- network_interface_pin_map: HashMap::new(),
+ network_interface_pin_map: pinning_opts.map(|o| o.mapping.clone()).unwrap_or_default(),
// Safety: At this point, it is know that we have a valid FQDN, as
// this is set by the TUI network panel, which only lets the user
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 5723fed..5784681 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -436,6 +436,11 @@ impl<UDT> FormView<UDT> {
self.add_to_column(1, view);
}
+ pub fn add_child_with_custom_label(&mut self, label: &str, view: impl View) {
+ self.add_to_column(0, TextView::new(label).no_wrap());
+ self.add_to_column(1, view);
+ }
+
pub fn add_child_with_data(&mut self, label: &str, view: impl View, data: UDT) {
self.add_to_column(0, TextView::new(format!("{label}: ")).no_wrap());
self.add_to_column(1, view);
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
index 960c25e..4388c7d 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -1,40 +1,82 @@
-use std::net::IpAddr;
-
use cursive::{
- view::ViewWrapper,
- views::{EditView, SelectView},
+ Cursive, View,
+ view::{Nameable, Resizable, ViewWrapper},
+ views::{
+ Button, Checkbox, Dialog, DummyView, EditView, LinearLayout, NamedView, ResizedView,
+ ScrollView, SelectView, TextView,
+ },
+};
+use std::{
+ collections::HashMap,
+ net::IpAddr,
+ sync::{Arc, Mutex},
};
use super::{CidrAddressEditView, FormView};
use proxmox_installer_common::{
- options::NetworkOptions,
- setup::NetworkInfo,
+ net::MAX_IFNAME_LEN,
+ options::{NetworkInterfacePinningOptions, NetworkOptions},
+ setup::{Interface, NetworkInfo},
utils::{CidrAddress, Fqdn},
};
+struct NetworkViewOptions {
+ selected_mac: String,
+ pinning_enabled: bool,
+ // For UI purposes, we want to always save the mapping, to save the state
+ // between toggling the checkbox
+ pinning_options: NetworkInterfacePinningOptions,
+}
+
+/// Convenience wrapper when needing to take a (interior-mutable) reference to
+/// `NetworkViewOptions`.
+type NetworkViewOptionsRef = Arc<Mutex<NetworkViewOptions>>;
+
+/// View for configuring anything related to network setup.
pub struct NetworkOptionsView {
- view: FormView,
+ view: LinearLayout,
+ options: NetworkViewOptionsRef,
}
impl NetworkOptionsView {
+ const PINNING_OPTIONS_BUTTON_NAME: &str = "network-pinning-options-button";
+ const MGMT_IFNAME_SELECTVIEW_NAME: &str = "network-management-ifname-selectview";
+
pub fn new(options: &NetworkOptions, network_info: &NetworkInfo) -> Self {
- let ifaces = network_info.interfaces.values();
- let ifnames = ifaces
- .clone()
- .map(|iface| (iface.render(), iface.name.clone()));
- let mut ifaces_selection = SelectView::new().popup().with_all(ifnames.clone());
+ let mut ifaces = network_info
+ .interfaces
+ .values()
+ .collect::<Vec<&Interface>>();
- // sort first to always have stable view
- ifaces_selection.sort();
- let selected = ifaces_selection
- .iter()
- .position(|(_label, iface)| *iface == options.ifname)
- .unwrap_or(ifaces.len() - 1);
+ // First, sort interfaces by their link state and then name
+ ifaces.sort_unstable_by_key(|x| (&x.state, &x.name));
- ifaces_selection.set_selection(selected);
+ let selected_mac = network_info
+ .interfaces
+ .get(&options.ifname)
+ .map(|iface| iface.mac.clone())
+ .unwrap_or_else(|| {
+ ifaces
+ .first()
+ .expect("at least one network interface")
+ .mac
+ .clone()
+ });
+
+ let options_ref = Arc::new(Mutex::new(NetworkViewOptions {
+ selected_mac,
+ pinning_enabled: options.pinning_opts.is_some(),
+ pinning_options: options.pinning_opts.clone().unwrap_or_default(),
+ }));
+
+ let iface_selection =
+ Self::build_mgmt_ifname_selectview(ifaces.clone(), options_ref.clone());
let form = FormView::<()>::new()
- .child("Management interface", ifaces_selection)
+ .child(
+ "Management interface",
+ iface_selection.with_name(Self::MGMT_IFNAME_SELECTVIEW_NAME),
+ )
.child(
"Hostname (FQDN)",
EditView::new().content(options.fqdn.to_string()),
@@ -52,44 +94,106 @@ impl NetworkOptionsView {
EditView::new().content(options.dns_server.to_string()),
);
- Self { view }
+ let pinning_checkbox = LinearLayout::horizontal()
+ .child(Checkbox::new().checked().on_change({
+ let ifaces = ifaces
+ .iter()
+ .map(|iface| (*iface).clone())
+ .collect::<Vec<Interface>>();
+ let options_ref = options_ref.clone();
+ move |siv, enable_pinning| {
+ siv.call_on_name(Self::PINNING_OPTIONS_BUTTON_NAME, {
+ let options_ref = options_ref.clone();
+ move |view: &mut Button| {
+ view.set_enabled(enable_pinning);
+
+ options_ref.lock().expect("unpoisoned lock").pinning_enabled =
+ enable_pinning;
+ }
+ });
+
+ Self::refresh_ifname_selectview(siv, &ifaces, options_ref.clone());
+ }
+ }))
+ .child(TextView::new(" Pin network interface names").no_wrap())
+ .child(DummyView.full_width())
+ .child(
+ Button::new("Pinning options", {
+ let options_ref = options_ref.clone();
+ let network_info = network_info.clone();
+ move |siv| {
+ let mut view =
+ Self::custom_name_mapping_view(&network_info, options_ref.clone());
+
+ // Pre-compute the child's layout, since it might depend on the size. Without this,
+ // the view will be empty until focused.
+ // The screen size never changes in our case, so this is completely OK.
+ view.layout(siv.screen_size());
+
+ siv.add_layer(view);
+ }
+ })
+ .with_name(Self::PINNING_OPTIONS_BUTTON_NAME),
+ );
+
+ let view = LinearLayout::vertical()
+ .child(form)
+ .child(DummyView.full_width())
+ .child(pinning_checkbox);
+
+ Self {
+ view,
+ options: options_ref,
+ }
}
pub fn get_values(&mut self) -> Result<NetworkOptions, String> {
- let ifname = self
+ let form = self
.view
- .get_value::<SelectView, _>(0)
+ .get_child(0)
+ .and_then(|v| v.downcast_ref::<FormView>())
+ .ok_or("failed to retrieve network options form")?;
+
+ let iface = form
+ .get_value::<NamedView<SelectView<Interface>>, _>(0)
.ok_or("failed to retrieve management interface name")?;
- let fqdn = self
- .view
+ let fqdn = form
.get_value::<EditView, _>(1)
.ok_or("failed to retrieve host FQDN")?
.parse::<Fqdn>()
.map_err(|err| format!("hostname does not look valid:\n\n{err}"))?;
- let address = self
- .view
+ let address = form
.get_value::<CidrAddressEditView, _>(2)
.ok_or("failed to retrieve host address".to_string())
.and_then(|(ip_addr, mask)| {
CidrAddress::new(ip_addr, mask).map_err(|err| err.to_string())
})?;
- let gateway = self
- .view
+ let gateway = form
.get_value::<EditView, _>(3)
.ok_or("failed to retrieve gateway address")?
.parse::<IpAddr>()
.map_err(|err| err.to_string())?;
- let dns_server = self
- .view
+ let dns_server = form
.get_value::<EditView, _>(4)
.ok_or("failed to retrieve DNS server address")?
.parse::<IpAddr>()
.map_err(|err| err.to_string())?;
+ let pinning_opts = self
+ .options
+ .lock()
+ .map(|opt| opt.pinning_enabled.then_some(opt.pinning_options.clone()))
+ .map_err(|err| err.to_string())?;
+
+ let ifname = match &pinning_opts {
+ Some(opts) => iface.to_pinned(opts).name,
+ None => iface.name,
+ };
+
if address.addr().is_ipv4() != gateway.is_ipv4() {
Err("host and gateway IP address version must not differ".to_owned())
} else if address.addr().is_ipv4() != dns_server.is_ipv4() {
@@ -103,11 +207,200 @@ impl NetworkOptionsView {
address,
gateway,
dns_server,
+ pinning_opts,
})
}
}
+
+ fn custom_name_mapping_view(
+ network_info: &NetworkInfo,
+ options_ref: NetworkViewOptionsRef,
+ ) -> impl View {
+ const DIALOG_NAME: &str = "network-interface-name-pinning-dialog";
+
+ let mut interfaces = network_info
+ .interfaces
+ .values()
+ .collect::<Vec<&Interface>>();
+
+ interfaces.sort_by(|a, b| (&a.state, &a.name).cmp(&(&b.state, &b.name)));
+
+ Dialog::around(InterfacePinningOptionsView::new(
+ &interfaces,
+ options_ref.clone(),
+ ))
+ .title("Interface Name Pinning Options")
+ .button("Ok", {
+ let interfaces = interfaces
+ .iter()
+ .map(|v| (*v).clone())
+ .collect::<Vec<Interface>>();
+ move |siv| {
+ let options = siv
+ .call_on_name(DIALOG_NAME, |view: &mut Dialog| {
+ view.get_content_mut()
+ .downcast_mut::<InterfacePinningOptionsView>()
+ .map(InterfacePinningOptionsView::get_values)
+ })
+ .flatten();
+
+ let options = match options {
+ Some(Ok(options)) => options,
+ Some(Err(err)) => {
+ siv.add_layer(Dialog::info(err));
+ return;
+ }
+ None => {
+ siv.add_layer(Dialog::info(
+ "Failed to retrieve network interface name pinning options view",
+ ));
+ return;
+ }
+ };
+
+ siv.pop_layer();
+ options_ref.lock().expect("unpoisoned lock").pinning_options = options;
+
+ Self::refresh_ifname_selectview(siv, &interfaces, options_ref.clone());
+ }
+ })
+ .with_name(DIALOG_NAME)
+ .max_size((80, 40))
+ }
+
+ fn refresh_ifname_selectview(
+ siv: &mut Cursive,
+ ifaces: &[Interface],
+ options_ref: NetworkViewOptionsRef,
+ ) {
+ siv.call_on_name(
+ Self::MGMT_IFNAME_SELECTVIEW_NAME,
+ |view: &mut SelectView<Interface>| {
+ *view = Self::build_mgmt_ifname_selectview(ifaces.iter().collect(), options_ref);
+ },
+ );
+ }
+
+ fn build_mgmt_ifname_selectview(
+ ifaces: Vec<&Interface>,
+ options_ref: NetworkViewOptionsRef,
+ ) -> SelectView<Interface> {
+ let options = options_ref.lock().expect("unpoisoned lock");
+
+ // Map all interfaces to a list of (human-readable interface name, [Interface]) pairs
+ let ifnames = ifaces
+ .iter()
+ .map(|iface| {
+ let iface = if options.pinning_enabled {
+ &iface.to_pinned(&options.pinning_options)
+ } else {
+ iface
+ };
+
+ (iface.render(), iface.clone())
+ })
+ .collect::<Vec<(String, Interface)>>();
+
+ let mut view = SelectView::new()
+ .popup()
+ .with_all(ifnames.clone())
+ .on_submit({
+ let options_ref = options_ref.clone();
+ move |_, iface| {
+ options_ref.lock().expect("unpoisoned lock").selected_mac = iface.mac.clone();
+ }
+ });
+
+ // Finally, (try to) select the current one
+ let selected = view
+ .iter()
+ .position(|(_label, iface)| iface.mac == options.selected_mac)
+ .unwrap_or(0); // we sort UP interfaces first, so select the first UP interface
+ //
+ view.set_selection(selected);
+
+ view
+ }
}
impl ViewWrapper for NetworkOptionsView {
- cursive::wrap_impl!(self.view: FormView);
+ cursive::wrap_impl!(self.view: LinearLayout);
+}
+
+struct InterfacePinningOptionsView {
+ view: ScrollView<NamedView<FormView<String>>>,
+}
+
+impl InterfacePinningOptionsView {
+ const FORM_NAME: &str = "network-interface-name-pinning-form";
+
+ fn new(interfaces: &[&Interface], options_ref: NetworkViewOptionsRef) -> Self {
+ let options = options_ref.lock().expect("unpoisoned lock");
+
+ let mut form = FormView::<String>::new();
+
+ for iface in interfaces {
+ let label = format!(
+ "{} ({}, {}, {})",
+ iface.mac, iface.name, iface.driver, iface.state
+ );
+
+ let view = LinearLayout::horizontal()
+ .child(DummyView.full_width()) // right align below form elements
+ .child(
+ EditView::new()
+ .content(iface.to_pinned(&options.pinning_options).name)
+ .max_content_width(MAX_IFNAME_LEN)
+ .fixed_width(MAX_IFNAME_LEN),
+ );
+
+ form.add_child_with_data(&label, view, iface.mac.clone());
+
+ if !iface.addresses.is_empty() {
+ for chunk in iface.addresses.chunks(2) {
+ let addrs = chunk
+ .iter()
+ .map(|v| v.to_string())
+ .collect::<Vec<String>>()
+ .join(", ");
+
+ form.add_child_with_custom_label(&format!(" {addrs}\n"), DummyView);
+ }
+ }
+ }
+
+ Self {
+ view: ScrollView::new(form.with_name(Self::FORM_NAME)),
+ }
+ }
+
+ fn get_values(&mut self) -> Result<NetworkInterfacePinningOptions, String> {
+ let form = self.view.get_inner_mut().get_mut();
+
+ let mut mapping = HashMap::new();
+
+ for i in 0..form.len() {
+ let (inner, mac) = match form.get_child_with_data::<LinearLayout>(i) {
+ Some(formdata) => formdata,
+ None => continue,
+ };
+
+ let name = inner
+ .get_child(1)
+ .and_then(|v| v.downcast_ref::<ResizedView<EditView>>())
+ .map(|v| v.get_inner().get_content())
+ .ok_or_else(|| format!("failed to retrieve pinning ID for interface {}", mac))?;
+
+ mapping.insert(mac.clone(), (*name).clone());
+ }
+
+ let opts = NetworkInterfacePinningOptions { mapping };
+ opts.verify().map_err(|err| err.to_string())?;
+
+ Ok(opts)
+ }
+}
+
+impl ViewWrapper for InterfacePinningOptionsView {
+ cursive::wrap_impl!(self.view: ScrollView<NamedView<FormView<String>>>);
}
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* [pve-devel] [PATCH installer 14/14] gui: add support for pinning network interface names
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
` (12 preceding siblings ...)
2025-10-14 13:21 ` [pve-devel] [PATCH installer 13/14] tui: add support for pinning network interface names Christoph Heiss
@ 2025-10-14 13:21 ` Christoph Heiss
2025-10-14 15:04 ` Maximiliano Sandoval
2025-10-21 14:05 ` Michael Köppl
2025-10-21 14:04 ` [pve-devel] [PATCH installer 00/14] support network interface name pinning Michael Köppl
14 siblings, 2 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-14 13:21 UTC (permalink / raw)
To: pve-devel
Adds an additional checkbox and option button in the network panel, the
latter triggering a dialog for setting custom names per network
interface present on the system.
Pinning is enabled by default.
Each pinned network interface name defaults to `nicN`, where N is the
pinned ID from the low-level installer.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Proxmox/Sys/Net.pm | 9 +-
proxinstall | 209 ++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 197 insertions(+), 21 deletions(-)
diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
index 2183d27..7fe800c 100644
--- a/Proxmox/Sys/Net.pm
+++ b/Proxmox/Sys/Net.pm
@@ -8,7 +8,14 @@ use Proxmox::Sys::Udev;
use JSON qw();
use base qw(Exporter);
-our @EXPORT_OK = qw(parse_ip_address parse_ip_mask parse_fqdn);
+our @EXPORT_OK = qw(parse_ip_address parse_ip_mask parse_fqdn MAX_IFNAME_LEN DEFAULT_PIN_PREFIX);
+
+# Maximum length of the (primary) name of a network interface
+# IFNAMSIZ - 1 to account for NUL byte
+use constant {
+ MAX_IFNAME_LEN => 15,
+ DEFAULT_PIN_PREFIX => 'nic',
+};
our $HOSTNAME_RE = "(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]{,61}?[a-zA-Z0-9])?)";
our $FQDN_RE = "(?:${HOSTNAME_RE}\\.)*${HOSTNAME_RE}";
diff --git a/proxinstall b/proxinstall
index 5ba65fa..35e948a 100755
--- a/proxinstall
+++ b/proxinstall
@@ -37,7 +37,7 @@ use Proxmox::Sys;
use Proxmox::Sys::Block qw(get_cached_disks);
use Proxmox::Sys::Command qw(syscmd);
use Proxmox::Sys::File qw(file_read_all file_write_all);
-use Proxmox::Sys::Net qw(parse_ip_address parse_ip_mask);
+use Proxmox::Sys::Net qw(parse_ip_address parse_ip_mask MAX_IFNAME_LEN DEFAULT_PIN_PREFIX);
use Proxmox::UI;
my $step_number = 0; # Init number for global function list
@@ -340,11 +340,117 @@ my $create_basic_grid = sub {
return $grid;
};
+my sub create_network_interface_pin_view {
+ my ($done_cb) = @_;
+
+ my $dialog = Gtk3::Dialog->new();
+ $dialog->set_title('Interface Name Pinning Options');
+ $dialog->add_button('_OK', 1);
+
+ my $content = $dialog->get_content_area();
+
+ my $hbox = Gtk3::Box->new('horizontal', 0);
+ $content->pack_start($hbox, 1, 1, 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_visible(1);
+ $scrolled_window->set_min_content_height(200);
+ $scrolled_window->set_margin_end(10);
+
+ $hbox->pack_start($scrolled_window, 1, 0, 5);
+
+ my $interfaces = Proxmox::Install::RunEnv::get()->{network}->{interfaces};
+ my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
+
+ my $inputs = {};
+ my $row = 0;
+ for my $ifname (sort keys $interfaces->%*) {
+ my $iface = $interfaces->{$ifname};
+
+ my $name = $mapping->{ $iface->{mac} };
+ my $label_text = "$iface->{mac} ($ifname, $iface->{driver}, $iface->{state})";
+
+ # if the interface has addresses assigned through DHCP, show them for
+ # reference
+ if (defined($iface->{addresses})) {
+ $label_text .=
+ "\n " . join(', ', map { "$_->{address}/$_->{prefix}" } @{ $iface->{addresses} });
+ }
+
+ my ($label, $input) = create_text_input($name, $label_text);
+ $label->set_xalign(0.);
+
+ $grid->attach($label, 0, $row, 1, 1);
+ $grid->attach($input, 1, $row, 1, 1);
+ $row++;
+
+ $inputs->{ $iface->{mac} } = $input;
+ }
+
+ $hbox->show_all();
+
+ $dialog->signal_connect(
+ response => sub {
+ my $new_mapping = {};
+ my $reverse_mapping = {};
+ foreach my $mac (keys %$inputs) {
+ my $name = $inputs->{$mac}->get_text();
+
+ if (!defined($name) || $name eq '') {
+ Proxmox::UI::message("interface name mapping for $mac cannot be empty");
+ $inputs->{$mac}->grab_focus();
+ return;
+ }
+
+ if ($reverse_mapping->{$name}) {
+ Proxmox::UI::message(
+ "duplicate interface name mapping '$name' for: $mac, $reverse_mapping->{$name}"
+ );
+ $inputs->{$mac}->grab_focus();
+ return;
+ }
+
+ if (length($name) > MAX_IFNAME_LEN) {
+ Proxmox::UI::message(
+ "interface name mapping '$name' for $mac cannot be longer than "
+ . MAX_IFNAME_LEN
+ . " characters");
+ $inputs->{$mac}->grab_focus();
+ return;
+ }
+
+ $new_mapping->{$mac} = $name;
+ $reverse_mapping->{$name} = $mac;
+ }
+
+ Proxmox::Install::Config::set_network_interface_pin_map($new_mapping);
+ $dialog->destroy();
+ $done_cb->();
+ },
+ );
+
+ $dialog->show();
+ $dialog->run();
+}
+
sub create_ipconf_view {
cleanup_view();
Proxmox::UI::display_html('ipconf.htm');
+ my $run_env = Proxmox::Install::RunEnv::get();
+ my $ipconf = $run_env->{ipconf};
+
my $grid = &$create_basic_grid();
$grid->set_row_spacing(10);
$grid->set_column_spacing(10);
@@ -355,7 +461,7 @@ sub create_ipconf_view {
my ($cidr_label, $cidr_box, $ipconf_entry_addr, $ipconf_entry_mask) = create_cidr_inputs($cidr);
- my $device_model = Gtk3::ListStore->new('Glib::String', 'Glib::String');
+ my $device_model = Gtk3::ListStore->new('Glib::String', 'Glib::String', 'Glib::String');
my $device_cb = Gtk3::ComboBox->new_with_model($device_model);
$device_cb->set_active(0);
$device_cb->set_visible(1);
@@ -369,19 +475,59 @@ sub create_ipconf_view {
$device_cb->pack_start($cell, 0);
$device_cb->add_attribute($cell, 'text', 1);
- my $get_device_desc = sub {
- my $iface = shift;
- return "$iface->{name} - $iface->{mac} ($iface->{driver})";
+ my $refresh_device_cb = sub {
+ # clear all entries and re-add them with their new names
+ my $active = $device_cb->get_active();
+ $device_model->clear();
+
+ my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
+ my $i = 0;
+ for my $index (sort keys $ipconf->{ifaces}->%*) {
+ my $iface = $ipconf->{ifaces}->{$index};
+ my $iter = $device_model->append();
+
+ my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' ';
+ my $name = $gtk_state->{network_pinning_enabled} ? $mapping->{ $iface->{mac} } : $iface->{name};
+
+ $device_model->set(
+ $iter,
+ 0 => $symbol,
+ 1 => "$name - $iface->{mac} ($iface->{driver})",
+ );
+ $i++;
+ }
+
+ # re-set the currently active entry to keep the users selection
+ $device_cb->set_active($active);
};
- my $run_env = Proxmox::Install::RunEnv::get();
- my $ipconf = $run_env->{ipconf};
+ 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(
+ 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->();
+ },
+ );
my ($device_active_map, $device_active_reverse_map) = ({}, {});
my $device_change_handler = sub {
my $current = shift;
+ # happens during the clear + re-insertion of all interfaces after
+ # the pinning changed, can be safely ignored
+ return if $current->get_active() == -1;
+
my $new = $device_active_map->{ $current->get_active() };
my $iface = $ipconf->{ifaces}->{$new};
@@ -389,6 +535,7 @@ sub create_ipconf_view {
return if defined($selected) && $iface->{name} eq $selected;
Proxmox::Install::Config::set_mngmt_nic($iface->{name});
+
$ipconf_entry_addr->set_text($iface->{inet}->{addr} || $iface->{inet6}->{addr})
if $iface->{inet}->{addr} || $iface->{inet6}->{addr};
$ipconf_entry_mask->set_text($iface->{inet}->{prefix} || $iface->{inet6}->{prefix})
@@ -400,13 +547,6 @@ sub create_ipconf_view {
my $i = 0;
for my $index (sort keys $ipconf->{ifaces}->%*) {
my $iface = $ipconf->{ifaces}->{$index};
- my $iter = $device_model->append();
- my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' ';
- $device_model->set(
- $iter,
- 0 => $symbol,
- 1 => $get_device_desc->($iface),
- );
$device_active_map->{$i} = $index;
$device_active_reverse_map->{ $iface->{name} } = $i;
@@ -418,6 +558,9 @@ sub create_ipconf_view {
$i++;
}
+ # fill the combobox with entries
+ $refresh_device_cb->();
+
if (my $nic = Proxmox::Install::Config::get_mngmt_nic()) {
$initial_active_device_pos = $device_active_reverse_map->{$nic};
} else {
@@ -443,7 +586,7 @@ sub create_ipconf_view {
$label->set_xalign(1.0);
$grid->attach($label, 0, 0, 1, 1);
- $grid->attach($device_cb, 1, 0, 1, 1);
+ $grid->attach($device_cb, 1, 0, 2, 1);
my $fqdn = Proxmox::Install::Config::get_fqdn();
my $hostname = $run_env->{network}->{hostname} || $iso_env->{product};
@@ -452,17 +595,17 @@ sub create_ipconf_view {
my ($host_label, $hostentry) = create_text_input($fqdn, 'Hostname (FQDN)');
$grid->attach($host_label, 0, 1, 1, 1);
- $grid->attach($hostentry, 1, 1, 1, 1);
+ $grid->attach($hostentry, 1, 1, 2, 1);
$grid->attach($cidr_label, 0, 2, 1, 1);
- $grid->attach($cidr_box, 1, 2, 1, 1);
+ $grid->attach($cidr_box, 1, 2, 2, 1);
my $cfg_gateway = Proxmox::Install::Config::get_gateway();
my $gateway = $cfg_gateway // $ipconf->{gateway} || '192.168.100.1';
my ($gw_label, $ipconf_entry_gw) = create_text_input($gateway, 'Gateway');
$grid->attach($gw_label, 0, 3, 1, 1);
- $grid->attach($ipconf_entry_gw, 1, 3, 1, 1);
+ $grid->attach($ipconf_entry_gw, 1, 3, 2, 1);
my $cfg_dns = Proxmox::Install::Config::get_dns();
my $dnsserver = $cfg_dns // $ipconf->{dnsserver} || $gateway;
@@ -470,7 +613,10 @@ sub create_ipconf_view {
my ($dns_label, $ipconf_entry_dns) = create_text_input($dnsserver, 'DNS Server');
$grid->attach($dns_label, 0, 4, 1, 1);
- $grid->attach($ipconf_entry_dns, 1, 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);
$gtk_state->{inbox}->show_all;
set_next(
@@ -538,6 +684,8 @@ 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++;
@@ -573,6 +721,12 @@ 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 $nic_mapping = Proxmox::Install::Config::get_network_interface_pin_map();
+ my $interface = $gtk_state->{network_pinning_enabled} ? $nic_mapping->{ $iface->{mac} } : $iface->{name};
+
my %config_values = (
__target_hd__ => join(' | ', $target_hds->@*),
__target_fs__ => Proxmox::Install::Config::get_filesys(),
@@ -580,7 +734,7 @@ sub create_ack_view {
__timezone__ => Proxmox::Install::Config::get_timezone(),
__keymap__ => Proxmox::Install::Config::get_keymap(),
__mailto__ => Proxmox::Install::Config::get_mailto(),
- __interface__ => Proxmox::Install::Config::get_mngmt_nic(),
+ __interface__ => $interface,
__hostname__ => Proxmox::Install::Config::get_hostname(),
__cidr__ => Proxmox::Install::Config::get_cidr(),
__gateway__ => Proxmox::Install::Config::get_gateway(),
@@ -609,6 +763,12 @@ sub create_ack_view {
set_next(
undef,
sub {
+ # before starting the install, unset the name pinning map if it
+ # is disabled
+ if (!$gtk_state->{network_pinning_enabled}) {
+ Proxmox::Install::Config::set_network_interface_pin_map(undef);
+ }
+
$step_number++;
create_extract_view();
},
@@ -1775,6 +1935,15 @@ if (!$initial_error && (scalar keys $run_env->{ipconf}->{ifaces}->%* == 0)) {
$initial_error = 1;
Proxmox::UI::display_html("nonics.htm");
set_next("Reboot", sub { app_quit(0); });
+} else {
+ # we enable it by default for new installation
+ $gtk_state->{network_pinning_enabled} = 1;
+
+ # pre-fill the name mapping before starting
+ my %mapping = map {
+ $_->{mac} => DEFAULT_PIN_PREFIX . $_->{pinned_id}
+ } values $run_env->{network}->{interfaces}->%*;
+ Proxmox::Install::Config::set_network_interface_pin_map(\%mapping);
}
create_intro_view() if !$initial_error;
--
2.51.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [pve-devel] [PATCH installer 14/14] gui: add support for pinning network interface names
2025-10-14 13:21 ` [pve-devel] [PATCH installer 14/14] gui: " Christoph Heiss
@ 2025-10-14 15:04 ` Maximiliano Sandoval
2025-10-16 12:01 ` Christoph Heiss
2025-10-21 14:05 ` Michael Köppl
1 sibling, 1 reply; 23+ messages in thread
From: Maximiliano Sandoval @ 2025-10-14 15:04 UTC (permalink / raw)
To: Christoph Heiss; +Cc: pve-devel
Christoph Heiss <c.heiss@proxmox.com> writes:
Some comments bellow:
> Adds an additional checkbox and option button in the network panel, the
> latter triggering a dialog for setting custom names per network
> interface present on the system.
>
> Pinning is enabled by default.
>
> Each pinned network interface name defaults to `nicN`, where N is the
> pinned ID from the low-level installer.
>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> Proxmox/Sys/Net.pm | 9 +-
> proxinstall | 209 ++++++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 197 insertions(+), 21 deletions(-)
>
> diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
> index 2183d27..7fe800c 100644
> --- a/Proxmox/Sys/Net.pm
> +++ b/Proxmox/Sys/Net.pm
> @@ -8,7 +8,14 @@ use Proxmox::Sys::Udev;
> use JSON qw();
>
> use base qw(Exporter);
> -our @EXPORT_OK = qw(parse_ip_address parse_ip_mask parse_fqdn);
> +our @EXPORT_OK = qw(parse_ip_address parse_ip_mask parse_fqdn MAX_IFNAME_LEN DEFAULT_PIN_PREFIX);
> +
> +# Maximum length of the (primary) name of a network interface
> +# IFNAMSIZ - 1 to account for NUL byte
> +use constant {
> + MAX_IFNAME_LEN => 15,
> + DEFAULT_PIN_PREFIX => 'nic',
> +};
>
> our $HOSTNAME_RE = "(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]{,61}?[a-zA-Z0-9])?)";
> our $FQDN_RE = "(?:${HOSTNAME_RE}\\.)*${HOSTNAME_RE}";
> diff --git a/proxinstall b/proxinstall
> index 5ba65fa..35e948a 100755
> --- a/proxinstall
> +++ b/proxinstall
> @@ -37,7 +37,7 @@ use Proxmox::Sys;
> use Proxmox::Sys::Block qw(get_cached_disks);
> use Proxmox::Sys::Command qw(syscmd);
> use Proxmox::Sys::File qw(file_read_all file_write_all);
> -use Proxmox::Sys::Net qw(parse_ip_address parse_ip_mask);
> +use Proxmox::Sys::Net qw(parse_ip_address parse_ip_mask MAX_IFNAME_LEN DEFAULT_PIN_PREFIX);
> use Proxmox::UI;
>
> my $step_number = 0; # Init number for global function list
> @@ -340,11 +340,117 @@ my $create_basic_grid = sub {
> return $grid;
> };
>
> +my sub create_network_interface_pin_view {
> + my ($done_cb) = @_;
> +
> + my $dialog = Gtk3::Dialog->new();
> + $dialog->set_title('Interface Name Pinning Options');
> + $dialog->add_button('_OK', 1);
The response argument is indeed a number (gint), but there is an enum
[1] for this. In perl one can use the string 'ok' instead of
GTK_RESPONSE_OK, for example.
I do not see the value of the response being used during the
`GtkDialog::response` signal handler, note that a dialog can be closed
either be pressing ESC, clicking the X button, or by clicking the `OK`
button as per the callback bellow. As it stands, all the methods I
described above would run the handler equally, is this intended?
[1] https://docs.gtk.org/gtk3/enum.ResponseType.html
> +
> + my $content = $dialog->get_content_area();
> +
> + my $hbox = Gtk3::Box->new('horizontal', 0);
> + $content->pack_start($hbox, 1, 1, 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_visible(1);
The scrolled window is the child of hbox and gtk_widget_show_all is
called on the later, it should not be necessary to call
gtk_widget_set_visible on this one.
> + $scrolled_window->set_min_content_height(200);
> + $scrolled_window->set_margin_end(10);
It is a bit asymmetrical that there is no margin on the start.
> +
> + $hbox->pack_start($scrolled_window, 1, 0, 5);
> +
> + my $interfaces = Proxmox::Install::RunEnv::get()->{network}->{interfaces};
> + my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
> +
> + my $inputs = {};
> + my $row = 0;
> + for my $ifname (sort keys $interfaces->%*) {
> + my $iface = $interfaces->{$ifname};
> +
> + my $name = $mapping->{ $iface->{mac} };
> + my $label_text = "$iface->{mac} ($ifname, $iface->{driver}, $iface->{state})";
> +
> + # if the interface has addresses assigned through DHCP, show them for
> + # reference
> + if (defined($iface->{addresses})) {
> + $label_text .=
> + "\n " . join(', ', map { "$_->{address}/$_->{prefix}" } @{ $iface->{addresses} });
> + }
> +
> + my ($label, $input) = create_text_input($name, $label_text);
> + $label->set_xalign(0.);
> +
> + $grid->attach($label, 0, $row, 1, 1);
> + $grid->attach($input, 1, $row, 1, 1);
> + $row++;
> +
> + $inputs->{ $iface->{mac} } = $input;
> + }
> +
> + $hbox->show_all();
> +
> + $dialog->signal_connect(
> + response => sub {
> + my $new_mapping = {};
> + my $reverse_mapping = {};
> + foreach my $mac (keys %$inputs) {
> + my $name = $inputs->{$mac}->get_text();
> +
> + if (!defined($name) || $name eq '') {
> + Proxmox::UI::message("interface name mapping for $mac cannot be empty");
> + $inputs->{$mac}->grab_focus();
> + return;
> + }
> +
> + if ($reverse_mapping->{$name}) {
> + Proxmox::UI::message(
> + "duplicate interface name mapping '$name' for: $mac, $reverse_mapping->{$name}"
> + );
> + $inputs->{$mac}->grab_focus();
> + return;
> + }
> +
> + if (length($name) > MAX_IFNAME_LEN) {
> + Proxmox::UI::message(
> + "interface name mapping '$name' for $mac cannot be longer than "
> + . MAX_IFNAME_LEN
> + . " characters");
> + $inputs->{$mac}->grab_focus();
> + return;
> + }
> +
> + $new_mapping->{$mac} = $name;
> + $reverse_mapping->{$name} = $mac;
> + }
> +
> + Proxmox::Install::Config::set_network_interface_pin_map($new_mapping);
> + $dialog->destroy();
> + $done_cb->();
> + },
> + );
> +
> + $dialog->show();
> + $dialog->run();
There are two ways to present dialogs, either by running
`gtk_dialog_run` which will block until the dialog is done and will
return the response (deprecated) and then close/destroy the dialog, or
connect to the response signal which will be emitted once there is a
response and the dialog can be closed (as done above) but instead of
calling `gtk_dialog_run()` one would call `gtk_window_present()` on it.
So please run `present` instead of `run` here.
In general `present()` and `gtk_widget_show()` are kinda similar but the
former is a wrapper around the later (among other things) and is
preferable (might have a different effect depending on the compositor).
Incidentally using gtk_widget_show() for the purpose of displaying a
window/dialog is deprecated in GTK 4.
> +}
> +
> sub create_ipconf_view {
>
> cleanup_view();
> Proxmox::UI::display_html('ipconf.htm');
>
> + my $run_env = Proxmox::Install::RunEnv::get();
> + my $ipconf = $run_env->{ipconf};
> +
> my $grid = &$create_basic_grid();
> $grid->set_row_spacing(10);
> $grid->set_column_spacing(10);
> @@ -355,7 +461,7 @@ sub create_ipconf_view {
>
> my ($cidr_label, $cidr_box, $ipconf_entry_addr, $ipconf_entry_mask) = create_cidr_inputs($cidr);
>
> - my $device_model = Gtk3::ListStore->new('Glib::String', 'Glib::String');
> + my $device_model = Gtk3::ListStore->new('Glib::String', 'Glib::String', 'Glib::String');
> my $device_cb = Gtk3::ComboBox->new_with_model($device_model);
> $device_cb->set_active(0);
> $device_cb->set_visible(1);
> @@ -369,19 +475,59 @@ sub create_ipconf_view {
> $device_cb->pack_start($cell, 0);
> $device_cb->add_attribute($cell, 'text', 1);
>
> - my $get_device_desc = sub {
> - my $iface = shift;
> - return "$iface->{name} - $iface->{mac} ($iface->{driver})";
> + my $refresh_device_cb = sub {
> + # clear all entries and re-add them with their new names
> + my $active = $device_cb->get_active();
> + $device_model->clear();
> +
> + my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
> + my $i = 0;
> + for my $index (sort keys $ipconf->{ifaces}->%*) {
> + my $iface = $ipconf->{ifaces}->{$index};
> + my $iter = $device_model->append();
> +
> + my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' ';
> + my $name = $gtk_state->{network_pinning_enabled} ? $mapping->{ $iface->{mac} } : $iface->{name};
> +
> + $device_model->set(
> + $iter,
> + 0 => $symbol,
> + 1 => "$name - $iface->{mac} ($iface->{driver})",
> + );
> + $i++;
> + }
> +
> + # re-set the currently active entry to keep the users selection
> + $device_cb->set_active($active);
> };
>
> - my $run_env = Proxmox::Install::RunEnv::get();
> - my $ipconf = $run_env->{ipconf};
> + 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(
> + 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->();
> + },
> + );
>
> my ($device_active_map, $device_active_reverse_map) = ({}, {});
>
> my $device_change_handler = sub {
> my $current = shift;
>
> + # happens during the clear + re-insertion of all interfaces after
> + # the pinning changed, can be safely ignored
> + return if $current->get_active() == -1;
> +
> my $new = $device_active_map->{ $current->get_active() };
> my $iface = $ipconf->{ifaces}->{$new};
>
> @@ -389,6 +535,7 @@ sub create_ipconf_view {
> return if defined($selected) && $iface->{name} eq $selected;
>
> Proxmox::Install::Config::set_mngmt_nic($iface->{name});
> +
> $ipconf_entry_addr->set_text($iface->{inet}->{addr} || $iface->{inet6}->{addr})
> if $iface->{inet}->{addr} || $iface->{inet6}->{addr};
> $ipconf_entry_mask->set_text($iface->{inet}->{prefix} || $iface->{inet6}->{prefix})
> @@ -400,13 +547,6 @@ sub create_ipconf_view {
> my $i = 0;
> for my $index (sort keys $ipconf->{ifaces}->%*) {
> my $iface = $ipconf->{ifaces}->{$index};
> - my $iter = $device_model->append();
> - my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' ';
> - $device_model->set(
> - $iter,
> - 0 => $symbol,
> - 1 => $get_device_desc->($iface),
> - );
> $device_active_map->{$i} = $index;
> $device_active_reverse_map->{ $iface->{name} } = $i;
>
> @@ -418,6 +558,9 @@ sub create_ipconf_view {
> $i++;
> }
>
> + # fill the combobox with entries
> + $refresh_device_cb->();
> +
> if (my $nic = Proxmox::Install::Config::get_mngmt_nic()) {
> $initial_active_device_pos = $device_active_reverse_map->{$nic};
> } else {
> @@ -443,7 +586,7 @@ sub create_ipconf_view {
> $label->set_xalign(1.0);
>
> $grid->attach($label, 0, 0, 1, 1);
> - $grid->attach($device_cb, 1, 0, 1, 1);
> + $grid->attach($device_cb, 1, 0, 2, 1);
>
> my $fqdn = Proxmox::Install::Config::get_fqdn();
> my $hostname = $run_env->{network}->{hostname} || $iso_env->{product};
> @@ -452,17 +595,17 @@ sub create_ipconf_view {
>
> my ($host_label, $hostentry) = create_text_input($fqdn, 'Hostname (FQDN)');
> $grid->attach($host_label, 0, 1, 1, 1);
> - $grid->attach($hostentry, 1, 1, 1, 1);
> + $grid->attach($hostentry, 1, 1, 2, 1);
>
> $grid->attach($cidr_label, 0, 2, 1, 1);
> - $grid->attach($cidr_box, 1, 2, 1, 1);
> + $grid->attach($cidr_box, 1, 2, 2, 1);
>
> my $cfg_gateway = Proxmox::Install::Config::get_gateway();
> my $gateway = $cfg_gateway // $ipconf->{gateway} || '192.168.100.1';
>
> my ($gw_label, $ipconf_entry_gw) = create_text_input($gateway, 'Gateway');
> $grid->attach($gw_label, 0, 3, 1, 1);
> - $grid->attach($ipconf_entry_gw, 1, 3, 1, 1);
> + $grid->attach($ipconf_entry_gw, 1, 3, 2, 1);
>
> my $cfg_dns = Proxmox::Install::Config::get_dns();
> my $dnsserver = $cfg_dns // $ipconf->{dnsserver} || $gateway;
> @@ -470,7 +613,10 @@ sub create_ipconf_view {
> my ($dns_label, $ipconf_entry_dns) = create_text_input($dnsserver, 'DNS Server');
>
> $grid->attach($dns_label, 0, 4, 1, 1);
> - $grid->attach($ipconf_entry_dns, 1, 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);
>
> $gtk_state->{inbox}->show_all;
> set_next(
> @@ -538,6 +684,8 @@ 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++;
> @@ -573,6 +721,12 @@ 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 $nic_mapping = Proxmox::Install::Config::get_network_interface_pin_map();
> + my $interface = $gtk_state->{network_pinning_enabled} ? $nic_mapping->{ $iface->{mac} } : $iface->{name};
> +
> my %config_values = (
> __target_hd__ => join(' | ', $target_hds->@*),
> __target_fs__ => Proxmox::Install::Config::get_filesys(),
> @@ -580,7 +734,7 @@ sub create_ack_view {
> __timezone__ => Proxmox::Install::Config::get_timezone(),
> __keymap__ => Proxmox::Install::Config::get_keymap(),
> __mailto__ => Proxmox::Install::Config::get_mailto(),
> - __interface__ => Proxmox::Install::Config::get_mngmt_nic(),
> + __interface__ => $interface,
> __hostname__ => Proxmox::Install::Config::get_hostname(),
> __cidr__ => Proxmox::Install::Config::get_cidr(),
> __gateway__ => Proxmox::Install::Config::get_gateway(),
> @@ -609,6 +763,12 @@ sub create_ack_view {
> set_next(
> undef,
> sub {
> + # before starting the install, unset the name pinning map if it
> + # is disabled
> + if (!$gtk_state->{network_pinning_enabled}) {
> + Proxmox::Install::Config::set_network_interface_pin_map(undef);
> + }
> +
> $step_number++;
> create_extract_view();
> },
> @@ -1775,6 +1935,15 @@ if (!$initial_error && (scalar keys $run_env->{ipconf}->{ifaces}->%* == 0)) {
> $initial_error = 1;
> Proxmox::UI::display_html("nonics.htm");
> set_next("Reboot", sub { app_quit(0); });
> +} else {
> + # we enable it by default for new installation
> + $gtk_state->{network_pinning_enabled} = 1;
> +
> + # pre-fill the name mapping before starting
> + my %mapping = map {
> + $_->{mac} => DEFAULT_PIN_PREFIX . $_->{pinned_id}
> + } values $run_env->{network}->{interfaces}->%*;
> + Proxmox::Install::Config::set_network_interface_pin_map(\%mapping);
> }
>
> create_intro_view() if !$initial_error;
--
Maximiliano
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [pve-devel] [PATCH installer 14/14] gui: add support for pinning network interface names
2025-10-14 15:04 ` Maximiliano Sandoval
@ 2025-10-16 12:01 ` Christoph Heiss
0 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-16 12:01 UTC (permalink / raw)
To: Maximiliano Sandoval; +Cc: pve-devel
Thanks for the review!
On Tue Oct 14, 2025 at 5:04 PM CEST, Maximiliano Sandoval wrote:
> Christoph Heiss <c.heiss@proxmox.com> writes:
>
> Some comments bellow:
>> diff --git a/proxinstall b/proxinstall
>> index 5ba65fa..35e948a 100755
>> --- a/proxinstall
>> +++ b/proxinstall
[..]
>> +my sub create_network_interface_pin_view {
>> + my ($done_cb) = @_;
>> +
>> + my $dialog = Gtk3::Dialog->new();
>> + $dialog->set_title('Interface Name Pinning Options');
>> + $dialog->add_button('_OK', 1);
>
> The response argument is indeed a number (gint), but there is an enum
> [1] for this. In perl one can use the string 'ok' instead of
> GTK_RESPONSE_OK, for example.
I'll change it to 'ok', thanks!
>
> I do not see the value of the response being used during the
> `GtkDialog::response` signal handler, note that a dialog can be closed
> either be pressing ESC, clicking the X button, or by clicking the `OK`
> button as per the callback bellow. As it stands, all the methods I
> described above would run the handler equally, is this intended?
I tried to be consistent with the advanced disk dialog, which has the
exact same behaviour - so yes.
[..]
>> +
>> + $scrolled_window->add($grid);
>> + $scrolled_window->set_policy('never', 'automatic');
>> + $scrolled_window->set_visible(1);
>
> The scrolled window is the child of hbox and gtk_widget_show_all is
> called on the later, it should not be necessary to call
> gtk_widget_set_visible on this one.
I see, I'll remove it.
>
>> + $scrolled_window->set_min_content_height(200);
>> + $scrolled_window->set_margin_end(10);
>
> It is a bit asymmetrical that there is no margin on the start.
Right, thanks for noticing!
[..]
>> +
>> + $dialog->show();
>> + $dialog->run();
>
> There are two ways to present dialogs, either by running
> `gtk_dialog_run` which will block until the dialog is done and will
> return the response (deprecated) and then close/destroy the dialog, or
> connect to the response signal which will be emitted once there is a
> response and the dialog can be closed (as done above) but instead of
> calling `gtk_dialog_run()` one would call `gtk_window_present()` on it.
> So please run `present` instead of `run` here.
Will do in v2.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pve-devel] [PATCH installer 14/14] gui: add support for pinning network interface names
2025-10-14 13:21 ` [pve-devel] [PATCH installer 14/14] gui: " Christoph Heiss
2025-10-14 15:04 ` Maximiliano Sandoval
@ 2025-10-21 14:05 ` Michael Köppl
2025-10-22 9:40 ` Christoph Heiss
1 sibling, 1 reply; 23+ messages in thread
From: Michael Köppl @ 2025-10-21 14:05 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
3 comments inline
On Tue Oct 14, 2025 at 3:21 PM CEST, Christoph Heiss wrote:
> Adds an additional checkbox and option button in the network panel, the
> latter triggering a dialog for setting custom names per network
> interface present on the system.
>
> Pinning is enabled by default.
>
> Each pinned network interface name defaults to `nicN`, where N is the
> pinned ID from the low-level installer.
>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> Proxmox/Sys/Net.pm | 9 +-
> proxinstall | 209 ++++++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 197 insertions(+), 21 deletions(-)
>
> diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm
> index 2183d27..7fe800c 100644
> --- a/Proxmox/Sys/Net.pm
> +++ b/Proxmox/Sys/Net.pm
> @@ -8,7 +8,14 @@ use Proxmox::Sys::Udev;
> use JSON qw();
>
> use base qw(Exporter);
> -our @EXPORT_OK = qw(parse_ip_address parse_ip_mask parse_fqdn);
> +our @EXPORT_OK = qw(parse_ip_address parse_ip_mask parse_fqdn MAX_IFNAME_LEN DEFAULT_PIN_PREFIX);
> +
> +# Maximum length of the (primary) name of a network interface
> +# IFNAMSIZ - 1 to account for NUL byte
> +use constant {
> + MAX_IFNAME_LEN => 15,
> + DEFAULT_PIN_PREFIX => 'nic',
> +};
>
> our $HOSTNAME_RE = "(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]{,61}?[a-zA-Z0-9])?)";
> our $FQDN_RE = "(?:${HOSTNAME_RE}\\.)*${HOSTNAME_RE}";
> diff --git a/proxinstall b/proxinstall
> index 5ba65fa..35e948a 100755
> --- a/proxinstall
> +++ b/proxinstall
> @@ -37,7 +37,7 @@ use Proxmox::Sys;
> use Proxmox::Sys::Block qw(get_cached_disks);
> use Proxmox::Sys::Command qw(syscmd);
> use Proxmox::Sys::File qw(file_read_all file_write_all);
> -use Proxmox::Sys::Net qw(parse_ip_address parse_ip_mask);
> +use Proxmox::Sys::Net qw(parse_ip_address parse_ip_mask MAX_IFNAME_LEN DEFAULT_PIN_PREFIX);
> use Proxmox::UI;
>
> my $step_number = 0; # Init number for global function list
> @@ -340,11 +340,117 @@ my $create_basic_grid = sub {
> return $grid;
> };
>
> +my sub create_network_interface_pin_view {
> + my ($done_cb) = @_;
> +
> + my $dialog = Gtk3::Dialog->new();
> + $dialog->set_title('Interface Name Pinning Options');
> + $dialog->add_button('_OK', 1);
> +
> + my $content = $dialog->get_content_area();
> +
> + my $hbox = Gtk3::Box->new('horizontal', 0);
> + $content->pack_start($hbox, 1, 1, 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_visible(1);
> + $scrolled_window->set_min_content_height(200);
> + $scrolled_window->set_margin_end(10);
> +
> + $hbox->pack_start($scrolled_window, 1, 0, 5);
> +
> + my $interfaces = Proxmox::Install::RunEnv::get()->{network}->{interfaces};
> + my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
> +
> + my $inputs = {};
> + my $row = 0;
> + for my $ifname (sort keys $interfaces->%*) {
> + my $iface = $interfaces->{$ifname};
> +
> + my $name = $mapping->{ $iface->{mac} };
> + my $label_text = "$iface->{mac} ($ifname, $iface->{driver}, $iface->{state})";
> +
> + # if the interface has addresses assigned through DHCP, show them for
> + # reference
> + if (defined($iface->{addresses})) {
> + $label_text .=
> + "\n " . join(', ', map { "$_->{address}/$_->{prefix}" } @{ $iface->{addresses} });
> + }
> +
> + my ($label, $input) = create_text_input($name, $label_text);
> + $label->set_xalign(0.);
> +
> + $grid->attach($label, 0, $row, 1, 1);
> + $grid->attach($input, 1, $row, 1, 1);
> + $row++;
> +
> + $inputs->{ $iface->{mac} } = $input;
> + }
> +
> + $hbox->show_all();
> +
> + $dialog->signal_connect(
> + response => sub {
> + my $new_mapping = {};
> + my $reverse_mapping = {};
> + foreach my $mac (keys %$inputs) {
> + my $name = $inputs->{$mac}->get_text();
> +
> + if (!defined($name) || $name eq '') {
> + Proxmox::UI::message("interface name mapping for $mac cannot be empty");
> + $inputs->{$mac}->grab_focus();
> + return;
> + }
> +
> + if ($reverse_mapping->{$name}) {
> + Proxmox::UI::message(
> + "duplicate interface name mapping '$name' for: $mac, $reverse_mapping->{$name}"
> + );
> + $inputs->{$mac}->grab_focus();
> + return;
> + }
> +
> + if (length($name) > MAX_IFNAME_LEN) {
> + Proxmox::UI::message(
> + "interface name mapping '$name' for $mac cannot be longer than "
> + . MAX_IFNAME_LEN
> + . " characters");
> + $inputs->{$mac}->grab_focus();
> + return;
> + }
same as for the TUI installer, imo the check for only-numeric interface
names would make sense. Or is there a reason it was omitted?
> +
> + $new_mapping->{$mac} = $name;
> + $reverse_mapping->{$name} = $mac;
> + }
> +
> + Proxmox::Install::Config::set_network_interface_pin_map($new_mapping);
> + $dialog->destroy();
> + $done_cb->();
> + },
> + );
> +
> + $dialog->show();
> + $dialog->run();
> +}
> +
> sub create_ipconf_view {
>
> cleanup_view();
> Proxmox::UI::display_html('ipconf.htm');
>
> + my $run_env = Proxmox::Install::RunEnv::get();
> + my $ipconf = $run_env->{ipconf};
> +
> my $grid = &$create_basic_grid();
> $grid->set_row_spacing(10);
> $grid->set_column_spacing(10);
> @@ -355,7 +461,7 @@ sub create_ipconf_view {
>
> my ($cidr_label, $cidr_box, $ipconf_entry_addr, $ipconf_entry_mask) = create_cidr_inputs($cidr);
>
> - my $device_model = Gtk3::ListStore->new('Glib::String', 'Glib::String');
> + my $device_model = Gtk3::ListStore->new('Glib::String', 'Glib::String', 'Glib::String');
> my $device_cb = Gtk3::ComboBox->new_with_model($device_model);
> $device_cb->set_active(0);
> $device_cb->set_visible(1);
> @@ -369,19 +475,59 @@ sub create_ipconf_view {
> $device_cb->pack_start($cell, 0);
> $device_cb->add_attribute($cell, 'text', 1);
>
> - my $get_device_desc = sub {
> - my $iface = shift;
> - return "$iface->{name} - $iface->{mac} ($iface->{driver})";
> + my $refresh_device_cb = sub {
> + # clear all entries and re-add them with their new names
> + my $active = $device_cb->get_active();
> + $device_model->clear();
> +
> + my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
> + my $i = 0;
> + for my $index (sort keys $ipconf->{ifaces}->%*) {
> + my $iface = $ipconf->{ifaces}->{$index};
> + my $iter = $device_model->append();
> +
> + my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' ';
> + my $name = $gtk_state->{network_pinning_enabled} ? $mapping->{ $iface->{mac} } : $iface->{name};
This seems to need a `make tidy`?
> +
> + $device_model->set(
> + $iter,
> + 0 => $symbol,
> + 1 => "$name - $iface->{mac} ($iface->{driver})",
> + );
> + $i++;
> + }
> +
> + # re-set the currently active entry to keep the users selection
> + $device_cb->set_active($active);
> };
>
> - my $run_env = Proxmox::Install::RunEnv::get();
> - my $ipconf = $run_env->{ipconf};
> + 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(
> + 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->();
> + },
> + );
>
> my ($device_active_map, $device_active_reverse_map) = ({}, {});
>
> my $device_change_handler = sub {
> my $current = shift;
>
> + # happens during the clear + re-insertion of all interfaces after
> + # the pinning changed, can be safely ignored
> + return if $current->get_active() == -1;
> +
> my $new = $device_active_map->{ $current->get_active() };
> my $iface = $ipconf->{ifaces}->{$new};
>
> @@ -389,6 +535,7 @@ sub create_ipconf_view {
> return if defined($selected) && $iface->{name} eq $selected;
>
> Proxmox::Install::Config::set_mngmt_nic($iface->{name});
> +
> $ipconf_entry_addr->set_text($iface->{inet}->{addr} || $iface->{inet6}->{addr})
> if $iface->{inet}->{addr} || $iface->{inet6}->{addr};
> $ipconf_entry_mask->set_text($iface->{inet}->{prefix} || $iface->{inet6}->{prefix})
> @@ -400,13 +547,6 @@ sub create_ipconf_view {
> my $i = 0;
> for my $index (sort keys $ipconf->{ifaces}->%*) {
> my $iface = $ipconf->{ifaces}->{$index};
> - my $iter = $device_model->append();
> - my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' ';
> - $device_model->set(
> - $iter,
> - 0 => $symbol,
> - 1 => $get_device_desc->($iface),
> - );
> $device_active_map->{$i} = $index;
> $device_active_reverse_map->{ $iface->{name} } = $i;
>
> @@ -418,6 +558,9 @@ sub create_ipconf_view {
> $i++;
> }
>
> + # fill the combobox with entries
> + $refresh_device_cb->();
> +
> if (my $nic = Proxmox::Install::Config::get_mngmt_nic()) {
> $initial_active_device_pos = $device_active_reverse_map->{$nic};
> } else {
> @@ -443,7 +586,7 @@ sub create_ipconf_view {
> $label->set_xalign(1.0);
>
> $grid->attach($label, 0, 0, 1, 1);
> - $grid->attach($device_cb, 1, 0, 1, 1);
> + $grid->attach($device_cb, 1, 0, 2, 1);
>
> my $fqdn = Proxmox::Install::Config::get_fqdn();
> my $hostname = $run_env->{network}->{hostname} || $iso_env->{product};
> @@ -452,17 +595,17 @@ sub create_ipconf_view {
>
> my ($host_label, $hostentry) = create_text_input($fqdn, 'Hostname (FQDN)');
> $grid->attach($host_label, 0, 1, 1, 1);
> - $grid->attach($hostentry, 1, 1, 1, 1);
> + $grid->attach($hostentry, 1, 1, 2, 1);
>
> $grid->attach($cidr_label, 0, 2, 1, 1);
> - $grid->attach($cidr_box, 1, 2, 1, 1);
> + $grid->attach($cidr_box, 1, 2, 2, 1);
>
> my $cfg_gateway = Proxmox::Install::Config::get_gateway();
> my $gateway = $cfg_gateway // $ipconf->{gateway} || '192.168.100.1';
>
> my ($gw_label, $ipconf_entry_gw) = create_text_input($gateway, 'Gateway');
> $grid->attach($gw_label, 0, 3, 1, 1);
> - $grid->attach($ipconf_entry_gw, 1, 3, 1, 1);
> + $grid->attach($ipconf_entry_gw, 1, 3, 2, 1);
>
> my $cfg_dns = Proxmox::Install::Config::get_dns();
> my $dnsserver = $cfg_dns // $ipconf->{dnsserver} || $gateway;
> @@ -470,7 +613,10 @@ sub create_ipconf_view {
> my ($dns_label, $ipconf_entry_dns) = create_text_input($dnsserver, 'DNS Server');
>
> $grid->attach($dns_label, 0, 4, 1, 1);
> - $grid->attach($ipconf_entry_dns, 1, 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);
>
> $gtk_state->{inbox}->show_all;
> set_next(
> @@ -538,6 +684,8 @@ 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++;
> @@ -573,6 +721,12 @@ 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 $nic_mapping = Proxmox::Install::Config::get_network_interface_pin_map();
> + my $interface = $gtk_state->{network_pinning_enabled} ? $nic_mapping->{ $iface->{mac} } : $iface->{name};
also `make tidy`
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [pve-devel] [PATCH installer 14/14] gui: add support for pinning network interface names
2025-10-21 14:05 ` Michael Köppl
@ 2025-10-22 9:40 ` Christoph Heiss
0 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-22 9:40 UTC (permalink / raw)
To: Michael Köppl; +Cc: Proxmox VE development discussion
On Tue Oct 21, 2025 at 4:05 PM CEST, Michael Köppl wrote:
> 3 comments inline
>
> On Tue Oct 14, 2025 at 3:21 PM CEST, Christoph Heiss wrote:
[..]
>> + $dialog->signal_connect(
>> + response => sub {
>> + my $new_mapping = {};
>> + my $reverse_mapping = {};
>> + foreach my $mac (keys %$inputs) {
>> + my $name = $inputs->{$mac}->get_text();
>> +
>> + if (!defined($name) || $name eq '') {
>> + Proxmox::UI::message("interface name mapping for $mac cannot be empty");
>> + $inputs->{$mac}->grab_focus();
>> + return;
>> + }
>> +
>> + if ($reverse_mapping->{$name}) {
>> + Proxmox::UI::message(
>> + "duplicate interface name mapping '$name' for: $mac, $reverse_mapping->{$name}"
>> + );
>> + $inputs->{$mac}->grab_focus();
>> + return;
>> + }
>> +
>> + if (length($name) > MAX_IFNAME_LEN) {
>> + Proxmox::UI::message(
>> + "interface name mapping '$name' for $mac cannot be longer than "
>> + . MAX_IFNAME_LEN
>> + . " characters");
>> + $inputs->{$mac}->grab_focus();
>> + return;
>> + }
>
> same as for the TUI installer, imo the check for only-numeric interface
> names would make sense. Or is there a reason it was omitted?
No particular reason, I'll add it here too for v2.
IIRC that seems to just have slipped through, as the first draft was
with a fixed prefix, since our tooling didn't support fully custom names
back then.
[..]
>> - my $get_device_desc = sub {
>> - my $iface = shift;
>> - return "$iface->{name} - $iface->{mac} ($iface->{driver})";
>> + my $refresh_device_cb = sub {
>> + # clear all entries and re-add them with their new names
>> + my $active = $device_cb->get_active();
>> + $device_model->clear();
>> +
>> + my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
>> + my $i = 0;
>> + for my $index (sort keys $ipconf->{ifaces}->%*) {
>> + my $iface = $ipconf->{ifaces}->{$index};
>> + my $iter = $device_model->append();
>> +
>> + my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' ';
>> + my $name = $gtk_state->{network_pinning_enabled} ? $mapping->{ $iface->{mac} } : $iface->{name};
>
> This seems to need a `make tidy`?
>
I'll run it before sending v2, thanks :^)
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pve-devel] [PATCH installer 00/14] support network interface name pinning
2025-10-14 13:21 [pve-devel] [PATCH installer 00/14] support network interface name pinning Christoph Heiss
` (13 preceding siblings ...)
2025-10-14 13:21 ` [pve-devel] [PATCH installer 14/14] gui: " Christoph Heiss
@ 2025-10-21 14:04 ` Michael Köppl
2025-10-22 9:46 ` Christoph Heiss
14 siblings, 1 reply; 23+ messages in thread
From: Michael Köppl @ 2025-10-21 14:04 UTC (permalink / raw)
To: Proxmox VE development discussion; +Cc: pve-devel
Tested this in both the GUI and TUI installers and the autoinstaller.
Tried the following:
- Pinning names for multiple interfaces
- Pinning names for interface with invalid MAC in autoinstaller
Noticed that in this case, the default of nic0 is used since the map
from MACs to interface names is pre-populated with the default
values. So if I use a MAC address in my answer file that does not
exist, the interface name for any interface not mentioned in the
answer file will be set to nic0, since if
network.interface-name-pinning.enabled is set to true, the pinning
is done in any case. I suppose this is on purpose, since there's no
way to check for the existence of the MAC address beforehand and
once the installation is running, the .link file has already been
written. Just wanted to mention it nonetheless.
- Testing invalid interface names in GUI, TUI, autoinstaller
Noticed that in the GUI, the error dialog will show up behind the
form where users enter the interface name. The form cannot really be
interacted with and the window has to be moved to the side to reveal
the error message.
- Explicitly test the case of pinning a fully numeric interface, since
it is allowed by the installers, but systemd does not allow this [0].
The result of this is that the interface that's affected does not
come up on first boot, since /etc/network/interfaces configures the
numeric-only interface which does not exist.
- Checked that disabling pinning actually does not set any interface
names
Other than the above, this seems to work as advertised. Nice work!
Also had a look at the code and left a few comments on the individual
patches, but did not notice any real problems with it. A single patch
seems to add code that is not correctly formatted and other than that
just added some suggestions.
With the comments on the individual patches addressed, consider this:
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
[0] https://manpages.debian.org/testing/udev/systemd.link.5.en.html#%5BLINK%5D_SECTION_OPTIONS
On Tue Oct 14, 2025 at 3:21 PM CEST, Christoph Heiss wrote:
> This series adds support for pinning the names of network interfaces
> directly during the installation, for all of GUI, TUI and auto-installer.
>
> There are also some smaller clean-ups and quality-of-life improvements
> interspersed - to keep the patches for each part of the installer
> together - in the series, which can also be applied
> separately/beforehand if wanted.
>
> Tested all combinations, i.e. for each of GUI, TUI and auto-installer,
> installed with pinning disabled (checking for regressions) and with
> pinning enabled at some custom interface names set.
>
> The auto-installer changes can be tested by specifying e.g.
>
> [network.interface-name-pinning]
> enabled = true
>
> [network.interface-name-pinning.mapping]
> "ab:cd:ef:12:34:56" = "mgmt"
> "12:34:56:ab:cd:ef" = "lan0"
>
> in the answer file.
>
> Christoph Heiss (13):
> test: parse-kernel-cmdline: fix module import statement
> install: add support for network interface name pinning
> run env: network: add kernel driver name to network interface info
> common: utils: fix clippy warnings
> common: setup: simplify network address list serialization
> common: implement support for `network_interface_pin_map` config
> auto: add support for pinning network interface names
> assistant: verify network settings in `validate-answer` subcommand
> post-hook: avoid redundant Option<bool> for (de-)serialization
> post-hook: add network interface name and pinning status
> tui: views: move network options view to own module
> tui: views: form: allow attaching user-defined data to children
> tui: add support for pinning network interface names
> gui: add support for pinning network interface names
>
> Proxmox/Install.pm | 47 +-
> Proxmox/Install/Config.pm | 8 +
> Proxmox/Install/RunEnv.pm | 11 +
> Proxmox/Sys/Net.pm | 63 ++-
> proxinstall | 209 ++++++++-
> proxmox-auto-install-assistant/src/main.rs | 3 +-
> proxmox-auto-installer/src/answer.rs | 63 ++-
> proxmox-auto-installer/src/utils.rs | 36 +-
> proxmox-auto-installer/tests/parse-answer.rs | 2 +
> .../network_interface_pinning.json | 30 ++
> .../network_interface_pinning.toml | 22 +
> ...n_from_dhcp_no_default_domain.run-env.json | 36 +-
> ...rface_pinning_overlong_interface_name.json | 3 +
> ...rface_pinning_overlong_interface_name.toml | 18 +
> .../no_fqdn_from_dhcp.run-env.json | 36 +-
> .../tests/resources/run-env-info.json | 38 +-
> proxmox-installer-common/src/lib.rs | 5 +
> proxmox-installer-common/src/options.rs | 174 ++++++--
> proxmox-installer-common/src/setup.rs | 74 +++-
> proxmox-installer-common/src/utils.rs | 6 +-
> proxmox-post-hook/src/main.rs | 62 +--
> proxmox-tui-installer/src/main.rs | 105 +----
> proxmox-tui-installer/src/setup.rs | 3 +
> proxmox-tui-installer/src/views/bootdisk.rs | 6 +-
> proxmox-tui-installer/src/views/mod.rs | 41 +-
> proxmox-tui-installer/src/views/network.rs | 406 ++++++++++++++++++
> test/parse-kernel-cmdline.pl | 2 +-
> 27 files changed, 1274 insertions(+), 235 deletions(-)
> create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.json
> create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/network_interface_pinning.toml
> create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.json
> create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/network_interface_pinning_overlong_interface_name.toml
> create mode 100644 proxmox-tui-installer/src/views/network.rs
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [pve-devel] [PATCH installer 00/14] support network interface name pinning
2025-10-21 14:04 ` [pve-devel] [PATCH installer 00/14] support network interface name pinning Michael Köppl
@ 2025-10-22 9:46 ` Christoph Heiss
0 siblings, 0 replies; 23+ messages in thread
From: Christoph Heiss @ 2025-10-22 9:46 UTC (permalink / raw)
To: Proxmox VE development discussion
On Tue Oct 21, 2025 at 4:04 PM CEST, Michael Köppl wrote:
> Tested this in both the GUI and TUI installers and the autoinstaller.
> Tried the following:
>
> - Pinning names for multiple interfaces
> - Pinning names for interface with invalid MAC in autoinstaller
> Noticed that in this case, the default of nic0 is used since the map
> from MACs to interface names is pre-populated with the default
> values. So if I use a MAC address in my answer file that does not
> exist, the interface name for any interface not mentioned in the
> answer file will be set to nic0, since if
> network.interface-name-pinning.enabled is set to true, the pinning
> is done in any case. I suppose this is on purpose, since there's no
> way to check for the existence of the MAC address beforehand and
> once the installation is running, the .link file has already been
> written. Just wanted to mention it nonetheless.
Yeah, it's intended. In the auto-installer case, we basically have to
trust the administrator to do things (more) diligently, as checking
whether MAC addresses actually exist can only be done at installation
time, really.
Although; I'll add a warning when an invalid/unknown MAC address is
encountered at the start of the installation, so that there is at least
_some_ notice.
> - Testing invalid interface names in GUI, TUI, autoinstaller
> Noticed that in the GUI, the error dialog will show up behind the
> form where users enter the interface name. The form cannot really be
> interacted with and the window has to be moved to the side to reveal
> the error message.
Oh, that's interesting. I'll look into it, thanks!
> - Explicitly test the case of pinning a fully numeric interface, since
> it is allowed by the installers, but systemd does not allow this [0].
> The result of this is that the interface that's affected does not
> come up on first boot, since /etc/network/interfaces configures the
> numeric-only interface which does not exist.
Yep, going to add some additional validation for interface names.
> - Checked that disabling pinning actually does not set any interface
> names
>
> Other than the above, this seems to work as advertised. Nice work!
>
> Also had a look at the code and left a few comments on the individual
> patches, but did not notice any real problems with it. A single patch
> seems to add code that is not correctly formatted and other than that
> just added some suggestions.
>
> With the comments on the individual patches addressed, consider this:
> Tested-by: Michael Köppl <m.koeppl@proxmox.com>
> Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
Thanks!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 23+ messages in thread