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 CA1051FF16B for ; Tue, 15 Jul 2025 11:29:24 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2FF503767A; Tue, 15 Jul 2025 11:30:21 +0200 (CEST) Message-ID: <2dd7d909-fdfd-42e9-a47c-c9c04c4a8891@proxmox.com> Date: Tue, 15 Jul 2025 11:30:17 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Stefan Hanreich , Proxmox VE development discussion References: <20250715090749.1608768-1-d.csapak@proxmox.com> <20250715090749.1608768-3-d.csapak@proxmox.com> <1f39b2d5-4c02-44fd-aabb-2e0a585547f3@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: <1f39b2d5-4c02-44fd-aabb-2e0a585547f3@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.020 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 RCVD_IN_MSPIKE_H2 0.001 Average reputation (+2) RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [network.pm, proxmox.com, i.new] Subject: Re: [pve-devel] [PATCH pve-manager v2 1/1] api/ui: show/return alternative interface names 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On 7/15/25 11:21, Stefan Hanreich wrote: > Was formulating my response to v1, but you were too fast with sending a > v2 for me. I think we need to also consider the case where we pinned and > replaced the names in /e/n/i via pveeth already, but the changes to the > network interface names haven't yet been applied. The problem here is > that we also return interfaces contained in /proc/net/dev in the > overview as well [1] - which would lead to duplicate interface names in > the view. > > I considered applying the changes to the network interface names > immediately via `udevadm trigger`. Alternatively, I thought of adding > the old names as altnames when pinning network interfaces, but not in a > persistent manner. I think both would solve this state between > transitioning from the old names to the new names. What do you think? ok, so from what i can tell 'pveeth pin' writes directly into the /e/n/i file? shouldn't it write to /e/n/i.new file (or whatever it's named) so we show it as pending change? then we could do the udevadm trigger in the 'apply config' api handler? also, basically the 'altname' lookup code would also have to lookup the .link files ? (seems like it's a lot of work/disk reading to do?) > > Generally I agree with using the names from the /e/n/i file as our > 'real' name, since the view is basically the interfaces file, but with > the important caveat that we also include interfaces from /proc/net/dev. > > [1] > https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/INotify.pm;h=dbad08c3fc5494403a061b9d77d587314477a9f5;hb=HEAD#l897 > > On 7/15/25 11:07, Dominik Csapak wrote: >> in api listing for all interfaces, and in the GET call for an individual >> interface. >> >> For intefaces that are already using an 'altname' in the iface stanza, >> we look up the 'legacy' interface name and the remaining altnames and >> show those. >> >> If we don't show them, the user might be confused if the bridge ports >> and interface names don't correlate. >> >> enables the additional alternative names column in the network view on >> the gui. >> >> Signed-off-by: Dominik Csapak >> --- >> changes from v1: >> * also show alternative names for ifaces with altname stanza >> (include legacy iface in that case) >> * sort the altnames >> >> PVE/API2/Network.pm | 41 +++++++++++++++++++++++++++++++++++-- >> www/manager6/node/Config.js | 1 + >> 2 files changed, 40 insertions(+), 2 deletions(-) >> >> diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm >> index a8cd7649..53e0ea0e 100644 >> --- a/PVE/API2/Network.pm >> +++ b/PVE/API2/Network.pm >> @@ -12,6 +12,7 @@ use PVE::RESTHandler; >> use PVE::RPCEnvironment; >> use PVE::JSONSchema qw(get_standard_option); >> use PVE::AccessControl; >> +use PVE::Network; >> use IO::File; >> >> use base qw(PVE::RESTHandler); >> @@ -231,6 +232,32 @@ sub json_config_properties { >> return $prop; >> } >> >> +sub extract_altnames { >> + my ($iface, $altnames) = @_; >> + >> + $altnames = PVE::Network::altname_mapping() if !defined($altnames); >> + >> + my $iface_altnames = []; >> + my $original_iface; >> + >> + # when we get an altname, first extract the legacy iface name >> + if (my $legacy_iface = $altnames->{$iface}) { >> + push $iface_altnames->@*, $legacy_iface; >> + $original_iface = $iface; >> + $iface = $legacy_iface; >> + } >> + >> + for my $altname (keys $altnames->%*) { >> + next if defined($original_iface) && $original_iface eq $altname; >> + if ($altnames->{$altname} eq $iface) { >> + push $iface_altnames->@*, $altname; >> + } >> + } >> + >> + return [sort $iface_altnames->@*] if scalar($iface_altnames->@*) > 0; >> + return undef; >> +} >> + >> __PACKAGE__->register_method({ >> name => 'index', >> path => '', >> @@ -390,6 +417,8 @@ __PACKAGE__->register_method({ >> >> my $ifaces = $config->{ifaces}; >> >> + my $altnames = PVE::Network::altname_mapping(); >> + >> delete $ifaces->{lo}; # do not list the loopback device >> >> if (my $tfilter = $param->{type}) { >> @@ -424,6 +453,9 @@ __PACKAGE__->register_method({ >> my $type = $ifaces->{$k}->{type}; >> delete $ifaces->{$k} >> if ($type eq 'bridge' || $type eq 'OVSBridge') && !$can_access_vnet->($k); >> + >> + my $iface_altnames = extract_altnames($k, $altnames); >> + $ifaces->{$k}->{altnames} = $iface_altnames if defined($iface_altnames); >> } >> >> return PVE::RESTHandler::hash_to_array($ifaces, 'iface'); >> @@ -789,10 +821,15 @@ __PACKAGE__->register_method({ >> my $config = PVE::INotify::read_file('interfaces'); >> my $ifaces = $config->{ifaces}; >> >> + my $iface = $param->{iface}; >> + >> raise_param_exc({ iface => "interface does not exist" }) >> - if !$ifaces->{ $param->{iface} }; >> + if !$ifaces->{$iface}; >> + >> + my $altnames = extract_altnames($iface); >> + $ifaces->{$iface}->{altnames} = $altnames if defined($altnames); >> >> - return $ifaces->{ $param->{iface} }; >> + return $ifaces->{$iface}; >> }, >> }); >> >> diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js >> index 4cd1671c..f6cd8749 100644 >> --- a/www/manager6/node/Config.js >> +++ b/www/manager6/node/Config.js >> @@ -190,6 +190,7 @@ Ext.define('PVE.node.Config', { >> iconCls: 'fa fa-exchange', >> itemId: 'network', >> showApplyBtn: true, >> + showAltNames: true, >> groups: ['services'], >> nodename: nodename, >> editOptions: { > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel