From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id B21C21FF17E for ; Thu, 13 Nov 2025 14:49:50 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 61FB81CEB3; Thu, 13 Nov 2025 14:50:36 +0100 (CET) From: Christoph Heiss To: pve-devel@lists.proxmox.com Date: Thu, 13 Nov 2025 14:49:52 +0100 Message-ID: <20251113135023.1038305-5-c.heiss@proxmox.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20251113135023.1038305-1-c.heiss@proxmox.com> References: <20251113135023.1038305-1-c.heiss@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1763041805756 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.048 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH installer 4/6] gui: network: only display pinnable devices in pinning options dialog X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Virtual devices such as e.g. bridges, bonds etc. otherwise also show up, where name pinning does not make sense. By making the `pinned_id` property on our internal network device structure optional, any other code can easily determine whether an interface is pinnable or not. Not really an issue during bare-metal installations, as there aren't any bridges/bonds set up yet - no changes there. But it's a nice future-proofing and improves developing/testing experience, as such interfaces are now suppressed). Signed-off-by: Christoph Heiss --- Proxmox/Install.pm | 9 +++------ Proxmox/Install/RunEnv.pm | 12 +++++++++--- Proxmox/Sys/Net.pm | 18 ------------------ proxinstall | 10 ++++++++-- 4 files changed, 20 insertions(+), 29 deletions(-) diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm index 1704ec4..bfc1769 100644 --- a/Proxmox/Install.pm +++ b/Proxmox/Install.pm @@ -1108,15 +1108,12 @@ sub extract_data { 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}; + # skip unpinnable interfaces, i.e. virtual links + next if !defined($if->{pinned_id}); + if (!defined($netif_pin_map->{ $if->{mac} })) { # each installer frontend must ensure that either # - pinning is disabled by never setting the map or diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm index 40a9621..fd3f74c 100644 --- a/Proxmox/Install/RunEnv.pm +++ b/Proxmox/Install/RunEnv.pm @@ -87,7 +87,7 @@ my sub query_netdevs : prototype() { my $default; # 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 $interfaces = fromjs(qx/ip --details --json address show/); my $pinned_counter = 0; for my $if (@$interfaces) { @@ -122,14 +122,20 @@ my sub query_netdevs : prototype() { $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; - $pinned_counter++; + # only set the `pinned_id` property if the interface actually can be pinned, + # i.e. is a physical link + my $is_pinnable = + Proxmox::Sys::Net::ip_link_is_physical($if) && !Proxmox::Sys::Net::iface_is_vf($name); + if ($is_pinnable) { + $ifs->{$name}->{pinned_id} = "${pinned_counter}"; + $pinned_counter++; + } } return $ifs; diff --git a/Proxmox/Sys/Net.pm b/Proxmox/Sys/Net.pm index 016a7f8..f6a0940 100644 --- a/Proxmox/Sys/Net.pm +++ b/Proxmox/Sys/Net.pm @@ -153,24 +153,6 @@ sub iface_is_vf { 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) = @_; diff --git a/proxinstall b/proxinstall index e3ea22e..e076c25 100755 --- a/proxinstall +++ b/proxinstall @@ -374,9 +374,15 @@ my sub create_network_interface_pin_view { my $inputs = {}; my $row = 0; + for my $ifname (sort keys $interfaces->%*) { my $iface = $interfaces->{$ifname}; + # exclude all non-pinnable interfaces, i.e. non-physical links, which won't have their + # `pinned_id` set. + # It does not make sense to pin those names, and the low-level installer will also skip them + next if !defined($iface->{pinned_id}); + my $name = $mapping->{ $iface->{mac} }; my $label_text = "$iface->{mac} ($ifname, $iface->{driver}, $iface->{state})"; @@ -463,7 +469,7 @@ sub create_ipconf_view { my $symbol = "$iface->{state}" eq "UP" ? "\x{25CF}" : ' '; my $name = - $gtk_state->{network_pinning_enabled} + $gtk_state->{network_pinning_enabled} && defined($mapping->{ $iface->{mac} }) ? $mapping->{ $iface->{mac} } : $iface->{name}; @@ -1920,7 +1926,7 @@ if (!$initial_error && (scalar keys $run_env->{ipconf}->{ifaces}->%* == 0)) { # pre-fill the name mapping before starting my %mapping = map { - $_->{mac} => DEFAULT_PIN_PREFIX . $_->{pinned_id} + defined($_->{pinned_id}) ? ($_->{mac} => DEFAULT_PIN_PREFIX . $_->{pinned_id}) : () } values $run_env->{network}->{interfaces}->%*; Proxmox::Install::Config::set_network_interface_pin_map(\%mapping); } -- 2.51.0 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel