From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 5B4A81FF09C for ; Mon, 02 Nov 2026 13:47:34 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 3F3FD217E8; Mon, 02 Nov 2026 13:47:14 +0100 (CET) From: Christoph Heiss To: pve-devel@lists.proxmox.com Subject: [PATCH installer v4 11/30] install: factor out ifupdown2 config generation into own subroutine Date: Mon, 2 Nov 2026 13:45:17 +0100 Message-ID: <20261102124539.1008075-12-c.heiss@proxmox.com> X-Mailer: git-send-email 2.55.0 In-Reply-To: <20261102124539.1008075-1-c.heiss@proxmox.com> References: <20261102124539.1008075-1-c.heiss@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793623600367 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.212 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: U7RCJTLJSCL2ROR3EMTQU3LFWNYXH4C6 X-Message-ID-Hash: U7RCJTLJSCL2ROR3EMTQU3LFWNYXH4C6 X-MailFrom: c.heiss@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Makes it more readable and to add more logic in the future, as well as being now unit-testable. While at it, unify the `address` and `gateway` and keep it of the branched paths for bridged/unbridged. Makes things simpler and the final configuration more predictable in layout. Signed-off-by: Christoph Heiss --- Changes v3 -> v4: * no changes Changes v2 -> v3: * only generate config in subroutine, keep the writing of /etc/network/interfaces in the main logic * fix missing translation of $ethdev to pinned name, by passing it directly as a parameter * add initial testsuite for ifupdown2 config generation (including additional tests from @Thomas) * simplify config generation a bit by moving address and gateway setup into common path Changes v1 -> v2: * no changes Proxmox/Install.pm | 134 ++++++++++---------- test/Makefile | 6 +- test/generate-ifupdown2-config.pl | 197 ++++++++++++++++++++++++++++++ 3 files changed, 270 insertions(+), 67 deletions(-) create mode 100755 test/generate-ifupdown2-config.pl diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm index 7bb8e29..a98571f 100644 --- a/Proxmox/Install.pm +++ b/Proxmox/Install.pm @@ -798,6 +798,71 @@ my sub setup_proxmox_first_boot_service { } } +sub generate_ifupdown2_config { + my $run_env = Proxmox::Install::RunEnv::get(); + my $iso_env = Proxmox::Install::ISOEnv::get(); + + my $ip_version = Proxmox::Install::Config::get_ip_version(); + my $ntype = $ip_version == 4 ? 'inet' : 'inet6'; + + my $interfaces = $run_env->{network}->{interfaces}; + my $ethdev = Proxmox::Install::Config::get_mngmt_nic(); + my $cidr = Proxmox::Install::Config::get_cidr(); + my $gateway = Proxmox::Install::Config::get_gateway(); + + my $netif_pin_map = Proxmox::Install::Config::get_network_interface_pin_map(); + + # frontends may pass the original kernel name of an interface that gets pinned + if (defined($netif_pin_map) && defined($interfaces->{$ethdev})) { + $ethdev = $netif_pin_map->{ $interfaces->{$ethdev}->{mac} } // $ethdev; + } + + my $ifaces = "auto lo\niface lo inet loopback\n\n"; + + # iproute2 omits the gateway and protocol keys for some routes; route devices are original + # kernel interface names, while $ethdev can be the pinned one + my $gateways6 = $run_env->{network}->{routes}->{gateways6} // []; + my $is_gateway6_from_ra = grep { + my $dev = $_->{dev} // ''; + if (defined($netif_pin_map) && defined($interfaces->{$dev})) { + $dev = $netif_pin_map->{ $interfaces->{$dev}->{mac} } // $dev; + } + $dev eq $ethdev + && ($_->{gateway} // '') eq $gateway + && ($_->{protocol} // '') eq 'ra'; + } $gateways6->@*; + + if ($iso_env->{cfg}->{bridged_network}) { + $ifaces .= "iface $ethdev $ntype manual\n"; + + $ifaces .= + "\nauto vmbr0\niface vmbr0 $ntype static\n" + . "\tbridge-ports $ethdev\n" + . "\tbridge-stp off\n" + . "\tbridge-fd 0\n"; + } else { + $ifaces .= "auto $ethdev\n" . # + "iface $ethdev $ntype static\n"; + } + + $ifaces .= "\taddress $cidr\n"; + if (!$is_gateway6_from_ra) { + $ifaces .= "\tgateway $gateway\n"; + } + + foreach my $iface (sort keys $interfaces->%*) { + my $if = $interfaces->{$iface}; + my $name = defined($netif_pin_map) ? $netif_pin_map->{ $if->{mac} } : $if->{name}; + + next if $name eq $ethdev; + + $ifaces .= "\niface $name $ntype manual\n"; + } + + $ifaces .= "\n\nsource /etc/network/interfaces.d/*\n"; + return $ifaces; +} + sub extract_data { my $iso_env = Proxmox::Install::ISOEnv::get(); my $run_env = Proxmox::Install::RunEnv::get(); @@ -1138,21 +1203,11 @@ sub extract_data { syscmd("/bin/hostname $hostname") if !is_test_mode(); - # configure interfaces - - my $ifaces = "auto lo\niface lo inet loopback\n\n"; - - my $ip_version = Proxmox::Install::Config::get_ip_version(); - my $ntype = $ip_version == 4 ? 'inet' : 'inet6'; - - my $ethdev = Proxmox::Install::Config::get_mngmt_nic(); - my $cidr = Proxmox::Install::Config::get_cidr(); - my $gateway = Proxmox::Install::Config::get_gateway(); - # configure pinned names for all network interfaces, if enabled my $netif_pin_map = Proxmox::Install::Config::get_network_interface_pin_map(); if (defined($netif_pin_map)) { + mkdir "$targetdir/usr/local/lib/systemd", 0755; mkdir "$targetdir/usr/local/lib/systemd/network", 0755; @@ -1178,64 +1233,11 @@ sub extract_data { "$targetdir/usr/local/lib/systemd/network/50-pmx-${pinned_name}.link", $link_file_content, ); - - # ensure that we use the correct interface name when writing out - # /etc/network/interfaces below - just in case the interface name - # is pinned and get_mngmt_nic() returned the original interface name. - # better be safe than sorry - if ($ethdev eq $ifname) { - $ethdev = $pinned_name; - } } } - my $interfaces = $run_env->{network}->{interfaces}; - - # iproute2 omits the gateway and protocol keys for some routes; route devices are original - # kernel interface names, while $ethdev can be the pinned one - my $gateways6 = $run_env->{network}->{routes}->{gateways6} // []; - my $is_gateway6_from_ra = grep { - my $dev = $_->{dev} // ''; - if (defined($netif_pin_map) && defined($interfaces->{$dev})) { - $dev = $netif_pin_map->{ $interfaces->{$dev}->{mac} } // $dev; - } - $dev eq $ethdev - && ($_->{gateway} // '') eq $gateway - && ($_->{protocol} // '') eq 'ra'; - } $gateways6->@*; - - if ($iso_env->{cfg}->{bridged_network}) { - $ifaces .= "iface $ethdev $ntype manual\n"; - - $ifaces .= - "\nauto vmbr0\niface vmbr0 $ntype static\n" - . "\taddress $cidr\n" - . "\tgateway $gateway\n" - . "\tbridge-ports $ethdev\n" - . "\tbridge-stp off\n" - . "\tbridge-fd 0\n"; - } else { - $ifaces .= "auto $ethdev\n" . # - "iface $ethdev $ntype static\n" . # - "\taddress $cidr\n"; - - if (!$is_gateway6_from_ra) { - $ifaces .= "\tgateway $gateway\n"; - } - } - - foreach my $iface (sort keys $interfaces->%*) { - my $if = $interfaces->{$iface}; - my $name = defined($netif_pin_map) ? $netif_pin_map->{ $if->{mac} } : $if->{name}; - - next if $name eq $ethdev; - - $ifaces .= "\niface $name $ntype manual\n"; - } - - $ifaces .= "\n\nsource /etc/network/interfaces.d/*\n"; - - file_write_all("$targetdir/etc/network/interfaces", $ifaces); + # configure interfaces + file_write_all("$targetdir/etc/network/interfaces", generate_ifupdown2_config()); # configure dns diff --git a/test/Makefile b/test/Makefile index bfe7674..9727244 100644 --- a/test/Makefile +++ b/test/Makefile @@ -5,7 +5,7 @@ export PERLLIB=.. .PHONY: check check: test-zfs-arc-max test-run-command test-parse-fqdn test-ui2-stdio \ test-zfs-get-pool-list test-parse-kernel-cmdline \ - test-validate-link-pin-map + test-validate-link-pin-map test-generate-ifupdown2-config .PHONY: test-zfs-arc-max test-zfs-arc-max: @@ -34,3 +34,7 @@ test-parse-kernel-cmdline: .PHONY: test-validate-link-pin-map test-validate-link-pin-map: ./validate-link-pin-map.pl + +.PHONY: test-generate-ifupdown2-config +test-generate-ifupdown2-config: + ./generate-ifupdown2-config.pl diff --git a/test/generate-ifupdown2-config.pl b/test/generate-ifupdown2-config.pl new file mode 100755 index 0000000..5c1794e --- /dev/null +++ b/test/generate-ifupdown2-config.pl @@ -0,0 +1,197 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use Test::More; +use Test::MockModule qw(strict); + +use Proxmox::Install; +use Proxmox::Install::Config; + +use Proxmox::Log; +Proxmox::Log::init("/tmp/install.log"); + +my $proxmox_install_runenv = Test::MockModule->new('Proxmox::Install::RunEnv'); +my $proxmox_install_isoenv = Test::MockModule->new('Proxmox::Install::ISOEnv'); + +my $current_cfg; +my sub set_config { + my (%cfg) = @_; + + $current_cfg = \%cfg; + $proxmox_install_isoenv->redefine( + get => sub { + return { + product => $cfg{bridge} ? 'pve' : 'pbs', + cfg => { + product => $cfg{bridge} ? 'pve' : 'pbs', + bridged_network => $cfg{bridge}, + }, + }; + }, + ); +} + +my sub is_generated { + my ($expected, $msg) = @_; + + is( + Proxmox::Install::generate_ifupdown2_config(), <{bridge})", + ); +} + +# global config +$proxmox_install_runenv->redefine( + get => sub { + return { + network => { + interfaces => { + eno1 => { + mac => '12:34:56:ab:cd:ef', + index => 1, + name => 'eno1', + }, + eno2 => { + mac => 'ab:cd:ef:12:34:56', + index => 2, + name => 'eno2', + }, + }, + }, + }; + }, +); +set_config(bridge => 0); +Proxmox::Install::Config::merge({ + mngmt_nic => 'eno1', + cidr => '192.168.0.10/24', + gateway => '192.168.0.1', +}); + +### +# There's essentially a matrix to test w.r.t the overall networking setup: +# +# +-------+--------+ +# | test# | bridge | +# +=======+========+ +# | 1 | 0 | +# | 2 | 1 | +# +-------+--------+ +### + +# 1. +set_config(bridge => 0); +is_generated( + < 1); +is_generated( + < { + '12:34:56:ab:cd:ef' => 'nic0', + 'ab:cd:ef:12:34:56' => 'nic1', + }, +}); +is_generated( + < undef, +}); + +# A gateway learned via IPv6 router advertisements is not written out +Proxmox::Install::Config::merge({ + mngmt_nic => 'eno1', + cidr => '2001:db8::1/64', + gateway => 'fe80::1', +}); +$proxmox_install_runenv->redefine( + get => sub { + return { + network => { + interfaces => { + eno1 => { + mac => '12:34:56:ab:cd:ef', + index => 1, + name => 'eno1', + }, + eno2 => { + mac => 'ab:cd:ef:12:34:56', + index => 2, + name => 'eno2', + }, + }, + routes => { + gateways6 => [{ dev => 'eno1', gateway => 'fe80::1', protocol => 'ra' }], + }, + }, + }; + }, +); + +is_generated( + <