public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui
@ 2025-07-14 10:42 Dominik Csapak
  2025-07-14 10:42 ` [pve-devel] [PATCH widget-toolkit 1/1] network: optionally show alternative interface names Dominik Csapak
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Dominik Csapak @ 2025-07-14 10:42 UTC (permalink / raw)
  To: pve-devel

This series goes on top of Stefan Hahnreich's last series[0], but it seems
there is still some things missing from INotify (to e.g. set the altnames via
the api), so maybe the helper I introduce in the pve-manager patch will be
unnecessary, depending if parsing the interfaces file will already return the
altnames.

Note that the pve-manager patch depends on the widget-toolkit patch

0: https://lore.proxmox.com/pve-devel/20250709194526.560709-2-s.hanreich@proxmox.com/

proxmox-widget-toolkit:

Dominik Csapak (1):
  network: optionally show alternative interface names

 src/node/NetworkEdit.js | 19 +++++++++++++++++++
 src/node/NetworkView.js | 20 ++++++++++++++++++++
 2 files changed, 39 insertions(+)


pve-manager:

Dominik Csapak (1):
  api/ui: show/return alternative interface names

 PVE/API2/Network.pm         | 32 ++++++++++++++++++++++++++++++--
 www/manager6/node/Config.js |  1 +
 2 files changed, 31 insertions(+), 2 deletions(-)


Summary over all repositories:
  4 files changed, 70 insertions(+), 2 deletions(-)

-- 
Generated by git-murpp 0.8.1


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] [PATCH widget-toolkit 1/1] network: optionally show alternative interface names
  2025-07-14 10:42 [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui Dominik Csapak
@ 2025-07-14 10:42 ` Dominik Csapak
  2025-07-14 10:42 ` [pve-devel] [PATCH pve-manager 1/1] api/ui: show/return " Dominik Csapak
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Dominik Csapak @ 2025-07-14 10:42 UTC (permalink / raw)
  To: pve-devel

add a new (hidden by default) column for the interface names, and show
them when editing an existing interface with such alternative names.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/node/NetworkEdit.js | 19 +++++++++++++++++++
 src/node/NetworkView.js | 20 ++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/src/node/NetworkEdit.js b/src/node/NetworkEdit.js
index b8e23f9..cd9dc7c 100644
--- a/src/node/NetworkEdit.js
+++ b/src/node/NetworkEdit.js
@@ -5,6 +5,9 @@ Ext.define('Proxmox.node.NetworkEdit', {
     // Enable to show the VLAN ID field
     enableBridgeVlanIds: false,
 
+    // Show alternative names below normal name
+    showAltNames: false,
+
     initComponent: function () {
         let me = this;
 
@@ -363,6 +366,14 @@ Ext.define('Proxmox.node.NetworkEdit', {
             },
         );
 
+        if (me.showAltNames && !me.isCreate) {
+            column1.push({
+                xtype: 'displayfield',
+                name: 'altnames',
+                fieldLabel: gettext("Alternative Names"),
+            });
+        }
+
         if (me.iftype === 'OVSBond') {
             column1.push(
                 {
@@ -449,6 +460,14 @@ Ext.define('Proxmox.node.NetworkEdit', {
                         });
                         return;
                     }
+
+                    if (data.altnames) {
+                        if (Ext.isArray(data.altnames)) {
+                            data.altnames = data.altnames.join('<br>');
+                        }
+                    } else {
+                        me.down('field[name=altnames]').setVisible(false);
+                    }
                     me.setValues(data);
                     me.isValid(); // trigger validation
                 },
diff --git a/src/node/NetworkView.js b/src/node/NetworkView.js
index cf8e6b7..0c62388 100644
--- a/src/node/NetworkView.js
+++ b/src/node/NetworkView.js
@@ -4,6 +4,7 @@ Ext.define('proxmox-networks', {
         'active',
         'address',
         'address6',
+        'altnames',
         'autostart',
         'bridge_ports',
         'cidr',
@@ -33,6 +34,9 @@ Ext.define('Proxmox.node.NetworkView', {
 
     showApplyBtn: false,
 
+    // if true, the altnames column will be shown by default
+    showAltNames: false,
+
     // for options passed down to the network edit window
     editOptions: {},
 
@@ -103,6 +107,7 @@ Ext.define('Proxmox.node.NetworkView', {
                 nodename: me.nodename,
                 iface: rec.data.iface,
                 iftype: rec.data.type,
+                showAltNames: me.showAltNames,
                 ...me.editOptions,
                 listeners: {
                     destroy: () => reload(),
@@ -278,6 +283,21 @@ Ext.define('Proxmox.node.NetworkView', {
                             sortable: true,
                             dataIndex: 'iface',
                         },
+                        {
+                            header: gettext("Alternative Names"),
+                            dataIndex: 'altnames',
+                            hidden: !me.showAltNames,
+                            width: 140, // enough space for 'enx<MAC>'
+                            renderer: (value) => {
+                                if (!value) {
+                                    return '';
+                                }
+                                if (Ext.isArray(value)) {
+                                    return value.map(Ext.htmlEncode).join('<br>');
+                                }
+                                return Ext.htmlEncode(value);
+                            },
+                        },
                         {
                             header: gettext('Type'),
                             sortable: true,
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] [PATCH pve-manager 1/1] api/ui: show/return alternative interface names
  2025-07-14 10:42 [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui Dominik Csapak
  2025-07-14 10:42 ` [pve-devel] [PATCH widget-toolkit 1/1] network: optionally show alternative interface names Dominik Csapak
@ 2025-07-14 10:42 ` Dominik Csapak
  2025-07-14 13:49 ` [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui Christoph Heiss
  2025-07-15  9:08 ` [pve-devel] superseded: " Dominik Csapak
  3 siblings, 0 replies; 13+ messages in thread
From: Dominik Csapak @ 2025-07-14 10:42 UTC (permalink / raw)
  To: pve-devel

in api listing for all interfaces, and in the GET call for an individual
interface.

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 <d.csapak@proxmox.com>
---
 PVE/API2/Network.pm         | 32 ++++++++++++++++++++++++++++++--
 www/manager6/node/Config.js |  1 +
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm
index a8cd7649..19611cb9 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,23 @@ sub json_config_properties {
     return $prop;
 }
 
+sub extract_altnames {
+    my ($iface, $altnames) = @_;
+
+    $altnames = PVE::Network::altname_mapping() if !defined($altnames);
+
+    my $iface_altnames = [];
+
+    for my $altname (keys $altnames->%*) {
+        if ($altnames->{$altname} eq $iface) {
+            push $iface_altnames->@*, $altname;
+        }
+    }
+
+    return $iface_altnames if scalar($iface_altnames->@*) > 0;
+    return undef;
+}
+
 __PACKAGE__->register_method({
     name => 'index',
     path => '',
@@ -390,6 +408,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 +444,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 +812,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: {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui
  2025-07-14 10:42 [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui Dominik Csapak
  2025-07-14 10:42 ` [pve-devel] [PATCH widget-toolkit 1/1] network: optionally show alternative interface names Dominik Csapak
  2025-07-14 10:42 ` [pve-devel] [PATCH pve-manager 1/1] api/ui: show/return " Dominik Csapak
@ 2025-07-14 13:49 ` Christoph Heiss
  2025-07-14 13:53   ` Dominik Csapak
  2025-07-15  9:08 ` [pve-devel] superseded: " Dominik Csapak
  3 siblings, 1 reply; 13+ messages in thread
From: Christoph Heiss @ 2025-07-14 13:49 UTC (permalink / raw)
  To: Dominik Csapak; +Cc: Proxmox VE development discussion

Tested the series, came across two thing:

Given e.g. the following interface:

    11: ens8: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc [..]
	link/ether 01:02:03:04:05:06 brd ff:ff:ff:ff:ff:ff
	altname nic8
	altname enxdeadff02d073

and the following /etc/network/interfaces snippet:

    auto nic8
    iface nic8 inet manual

the interface will be displayed twice in the network page. If in the
above snippet s/nic8/ens8/ is done (i.e. using the primary name), it
works correctly.

I guess between our /e/n/i parser and the altname mapping the interface
is picked up twice, so this will need some sort of "de-duplication" in
the backend, from what I can gather.

---

Further, if an interface has at least two altnames (like above), the
ordering doesn't seem stable across API requests.

E.g. double-click on an interface, close the dialog -> the
altnames will switch around randomly in the table column.
Opening the dialog multiple times yields the same behaviour for the
altname textfield.

On Mon Jul 14, 2025 at 12:42 PM CEST, Dominik Csapak wrote:
> This series goes on top of Stefan Hahnreich's last series[0], but it seems
> there is still some things missing from INotify (to e.g. set the altnames via
> the api), so maybe the helper I introduce in the pve-manager patch will be
> unnecessary, depending if parsing the interfaces file will already return the
> altnames.
>
> Note that the pve-manager patch depends on the widget-toolkit patch
>
> 0: https://lore.proxmox.com/pve-devel/20250709194526.560709-2-s.hanreich@proxmox.com/
>
> proxmox-widget-toolkit:
>
> Dominik Csapak (1):
>   network: optionally show alternative interface names
>
>  src/node/NetworkEdit.js | 19 +++++++++++++++++++
>  src/node/NetworkView.js | 20 ++++++++++++++++++++
>  2 files changed, 39 insertions(+)
>
>
> pve-manager:
>
> Dominik Csapak (1):
>   api/ui: show/return alternative interface names
>
>  PVE/API2/Network.pm         | 32 ++++++++++++++++++++++++++++++--
>  www/manager6/node/Config.js |  1 +
>  2 files changed, 31 insertions(+), 2 deletions(-)
>
>
> Summary over all repositories:
>   4 files changed, 70 insertions(+), 2 deletions(-)



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui
  2025-07-14 13:49 ` [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui Christoph Heiss
@ 2025-07-14 13:53   ` Dominik Csapak
  2025-07-14 14:31     ` Thomas Lamprecht
  0 siblings, 1 reply; 13+ messages in thread
From: Dominik Csapak @ 2025-07-14 13:53 UTC (permalink / raw)
  To: Christoph Heiss; +Cc: Proxmox VE development discussion

On 7/14/25 15:49, Christoph Heiss wrote:
> Tested the series, came across two thing:
> 
> Given e.g. the following interface:
> 
>      11: ens8: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc [..]
> 	link/ether 01:02:03:04:05:06 brd ff:ff:ff:ff:ff:ff
> 	altname nic8
> 	altname enxdeadff02d073
> 
> and the following /etc/network/interfaces snippet:
> 
>      auto nic8
>      iface nic8 inet manual
> 
> the interface will be displayed twice in the network page. If in the
> above snippet s/nic8/ens8/ is done (i.e. using the primary name), it
> works correctly.
> 
> I guess between our /e/n/i parser and the altname mapping the interface
> is picked up twice, so this will need some sort of "de-duplication" in
> the backend, from what I can gather.

yeah i don't touch the which interfaces will be shown, so
that's on the /e/n/i parser

> 
> ---
> 
> Further, if an interface has at least two altnames (like above), the
> ordering doesn't seem stable across API requests.
> 
> E.g. double-click on an interface, close the dialog -> the
> altnames will switch around randomly in the table column.
> Opening the dialog multiple times yields the same behaviour for the
> altname textfield.

yeah that's true, we probably want a 'sort keys' here, i'll wait until
tomorrow for a v2 if there are other remarks

> 
> On Mon Jul 14, 2025 at 12:42 PM CEST, Dominik Csapak wrote:
>> This series goes on top of Stefan Hahnreich's last series[0], but it seems
>> there is still some things missing from INotify (to e.g. set the altnames via
>> the api), so maybe the helper I introduce in the pve-manager patch will be
>> unnecessary, depending if parsing the interfaces file will already return the
>> altnames.
>>
>> Note that the pve-manager patch depends on the widget-toolkit patch
>>
>> 0: https://lore.proxmox.com/pve-devel/20250709194526.560709-2-s.hanreich@proxmox.com/
>>
>> proxmox-widget-toolkit:
>>
>> Dominik Csapak (1):
>>    network: optionally show alternative interface names
>>
>>   src/node/NetworkEdit.js | 19 +++++++++++++++++++
>>   src/node/NetworkView.js | 20 ++++++++++++++++++++
>>   2 files changed, 39 insertions(+)
>>
>>
>> pve-manager:
>>
>> Dominik Csapak (1):
>>    api/ui: show/return alternative interface names
>>
>>   PVE/API2/Network.pm         | 32 ++++++++++++++++++++++++++++++--
>>   www/manager6/node/Config.js |  1 +
>>   2 files changed, 31 insertions(+), 2 deletions(-)
>>
>>
>> Summary over all repositories:
>>    4 files changed, 70 insertions(+), 2 deletions(-)
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui
  2025-07-14 13:53   ` Dominik Csapak
@ 2025-07-14 14:31     ` Thomas Lamprecht
  2025-07-15  7:41       ` Dominik Csapak
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Lamprecht @ 2025-07-14 14:31 UTC (permalink / raw)
  To: Proxmox VE development discussion, Dominik Csapak, Christoph Heiss

Am 14.07.25 um 15:53 schrieb Dominik Csapak:
> On 7/14/25 15:49, Christoph Heiss wrote:
>> Tested the series, came across two thing:
>>
>> Given e.g. the following interface:
>>
>>      11: ens8: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc [..]
>> 	link/ether 01:02:03:04:05:06 brd ff:ff:ff:ff:ff:ff
>> 	altname nic8
>> 	altname enxdeadff02d073
>>
>> and the following /etc/network/interfaces snippet:
>>
>>      auto nic8
>>      iface nic8 inet manual
>>
>> the interface will be displayed twice in the network page. If in the
>> above snippet s/nic8/ens8/ is done (i.e. using the primary name), it
>> works correctly.
>>
>> I guess between our /e/n/i parser and the altname mapping the interface
>> is picked up twice, so this will need some sort of "de-duplication" in
>> the backend, from what I can gather.
> 
> yeah i don't touch the which interfaces will be shown, so
> that's on the /e/n/i parser

Fwiw, it's not really wrong, one is the name in /e/n/i and the others
are interface names per `ip link`, but it naturally can be a bit
confusing as is.

Depends also a bit on what we want, i.e., it probably makes sense to always
show the name used in the /e/n/i config in the name column, and always filter
that out from the alternative names displayed, as that allows the easiest
correlation to the /e/n/i config, which is the main source of true for this
panel.
The small downside for that is then that the Alternative Names column is not
a 1:1 mapping of what `ip link` shows as altname, but that probably is not
an actual issue, after all they all live in the same namespace and are
interchangeable for usage with the iproute2 tools.
But that might also mean that we have to treat duplicates explicitly here
in the API call enhancing that info. Waiting until tomorrow makes sense in
any way though, as Stefan will be back from his short vacation then and 
might have some input here.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui
  2025-07-14 14:31     ` Thomas Lamprecht
@ 2025-07-15  7:41       ` Dominik Csapak
  2025-07-15  8:17         ` Thomas Lamprecht
  0 siblings, 1 reply; 13+ messages in thread
From: Dominik Csapak @ 2025-07-15  7:41 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion, Christoph Heiss

On 7/14/25 16:31, Thomas Lamprecht wrote:
> Am 14.07.25 um 15:53 schrieb Dominik Csapak:
>> On 7/14/25 15:49, Christoph Heiss wrote:
>>> Tested the series, came across two thing:
>>>
>>> Given e.g. the following interface:
>>>
>>>       11: ens8: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc [..]
>>> 	link/ether 01:02:03:04:05:06 brd ff:ff:ff:ff:ff:ff
>>> 	altname nic8
>>> 	altname enxdeadff02d073
>>>
>>> and the following /etc/network/interfaces snippet:
>>>
>>>       auto nic8
>>>       iface nic8 inet manual
>>>
>>> the interface will be displayed twice in the network page. If in the
>>> above snippet s/nic8/ens8/ is done (i.e. using the primary name), it
>>> works correctly.
>>>
>>> I guess between our /e/n/i parser and the altname mapping the interface
>>> is picked up twice, so this will need some sort of "de-duplication" in
>>> the backend, from what I can gather.
>>
>> yeah i don't touch the which interfaces will be shown, so
>> that's on the /e/n/i parser
> 
> Fwiw, it's not really wrong, one is the name in /e/n/i and the others
> are interface names per `ip link`, but it naturally can be a bit
> confusing as is.
> 
> Depends also a bit on what we want, i.e., it probably makes sense to always
> show the name used in the /e/n/i config in the name column, and always filter
> that out from the alternative names displayed, as that allows the easiest
> correlation to the /e/n/i config, which is the main source of true for this
> panel.
> The small downside for that is then that the Alternative Names column is not
> a 1:1 mapping of what `ip link` shows as altname, but that probably is not
> an actual issue, after all they all live in the same namespace and are
> interchangeable for usage with the iproute2 tools.
> But that might also mean that we have to treat duplicates explicitly here
> in the API call enhancing that info. Waiting until tomorrow makes sense in
> any way though, as Stefan will be back from his short vacation then and
> might have some input here.

i thought a bit about this, and I think there are some disadvantages when
just taking the names from /e/n/i:

* not sure if it would work at all (depends on ifupdown2), but some
   weird configs could make the ui even more confusing, e.g. (assuming nicX is an altname of nicY):

         auto nicX
         iface nicY inet manual


   we now would probably pick up both nicX and nicY, and show both in addition to what ip link says

* having any custom altname in the /e/n/i would require the computed/shown altnames to be
   calculated for *each*, so e.g. if i have
    nicX, nicY and nicZ all point to the same device, we'd have to make the altname list generation
   more complicated


IMHO it would be ok to always show the 'canonical' name (whatever ip link/kernel says)
in the 'name' column and everything else in the 'altname' column regardless what is
configured in /e/n/i


Disadvantage is of course that for some this might be more confusing as some will be
taking /e/n/i as a reference and not 'ip link'....


I don't think there is a good way to make it super clear for all situations/users, but in the end
we have to decide what is the 'ground truth' (i.e. either /e/n/i or 'ip link')






_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui
  2025-07-15  7:41       ` Dominik Csapak
@ 2025-07-15  8:17         ` Thomas Lamprecht
  2025-07-15  8:36           ` Dominik Csapak
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Lamprecht @ 2025-07-15  8:17 UTC (permalink / raw)
  To: Dominik Csapak, Proxmox VE development discussion, Christoph Heiss

Am 15.07.25 um 09:41 schrieb Dominik Csapak:
> On 7/14/25 16:31, Thomas Lamprecht wrote:
>> Fwiw, it's not really wrong, one is the name in /e/n/i and the others
>> are interface names per `ip link`, but it naturally can be a bit
>> confusing as is.
>>
>> Depends also a bit on what we want, i.e., it probably makes sense to always
>> show the name used in the /e/n/i config in the name column, and always filter
>> that out from the alternative names displayed, as that allows the easiest
>> correlation to the /e/n/i config, which is the main source of true for this
>> panel.
>> The small downside for that is then that the Alternative Names column is not
>> a 1:1 mapping of what `ip link` shows as altname, but that probably is not
>> an actual issue, after all they all live in the same namespace and are
>> interchangeable for usage with the iproute2 tools.
>> But that might also mean that we have to treat duplicates explicitly here
>> in the API call enhancing that info. Waiting until tomorrow makes sense in
>> any way though, as Stefan will be back from his short vacation then and
>> might have some input here.
> 
> i thought a bit about this, and I think there are some disadvantages when
> just taking the names from /e/n/i:
> 
> * not sure if it would work at all (depends on ifupdown2), but some
>    weird configs could make the ui even more confusing, e.g. (assuming nicX is an altname of nicY):
> 
>          auto nicX
>          iface nicY inet manual
> 
> 
>    we now would probably pick up both nicX and nicY, and show both in addition to what ip link says

Just test it? I'd like to avoid theoretical conversations, this can be
already confusing enough without that. In anyway, we can normalize that
on serialization and it's a edge case the user had to manually cause
themselves, so would not bother to much at all with this case.

> 
> * having any custom altname in the /e/n/i would require the computed/shown altnames to be

What does "custom altname" mean?

>    calculated for *each*, so e.g. if i have
>     nicX, nicY and nicZ all point to the same device, we'd have to make the altname list generation
>    more complicated
> 
> 
> IMHO it would be ok to always show the 'canonical' name (whatever ip link/kernel says)
> in the 'name' column and everything else in the 'altname' column regardless what is
> configured in /e/n/i

There is no real canonical name, there's a legacy name that might be
called primary one, but as one can use the others transparently for it
we really must not think to much into that terminology, it's as first
class as the other names.

> Disadvantage is of course that for some this might be more confusing as some will be
> taking /e/n/i as a reference and not 'ip link'....

Yeah, and ip link shows all names, /e/n/i has normally just the one
name, so one can easily map any /e/n/i variant to ip link but not vice
versa.

> 
> 
> I don't think there is a good way to make it super clear for all situations/users, but in the end
> we have to decide what is the 'ground truth' (i.e. either /e/n/i or 'ip link')

Writing this I'm a bit more certain about:

"iface" entries are our source of truth and the name from POV of the API
and UI, outdated "auto" statements can be fixed on serialization and also
normalized on parsing, they can only stem from manual editing and are not
a real issue.
All other names pointing to the same interface should be picked up as
alternative names.

And if we want to highlight the primary name ip link shows, this could be
done either inline in the UI, or through by placing it in a dedicated
column. Both could be followed up on depending on the user feedback we get,
so I do not think we need to decide that now.

The /e/n/i config always was the source for the network config view–which
isn't a network status view–so that should IMO really stick with /e/n/i as
prime source of truth.

Seeing status of all interfaces  including statistics and the like might be
nice, but that probably is handled as new API and component, potentially
even a cluster wide one. There we could then make iproute2/kernel the primary
source of truth, and only show any config name a interface is used as.
 But I'd not think about this to much for the transparent altname support.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui
  2025-07-15  8:17         ` Thomas Lamprecht
@ 2025-07-15  8:36           ` Dominik Csapak
  2025-07-15  8:54             ` Thomas Lamprecht
  0 siblings, 1 reply; 13+ messages in thread
From: Dominik Csapak @ 2025-07-15  8:36 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion, Christoph Heiss

On 7/15/25 10:17, Thomas Lamprecht wrote:
> Am 15.07.25 um 09:41 schrieb Dominik Csapak:
>> On 7/14/25 16:31, Thomas Lamprecht wrote:
>>> Fwiw, it's not really wrong, one is the name in /e/n/i and the others
>>> are interface names per `ip link`, but it naturally can be a bit
>>> confusing as is.
>>>
>>> Depends also a bit on what we want, i.e., it probably makes sense to always
>>> show the name used in the /e/n/i config in the name column, and always filter
>>> that out from the alternative names displayed, as that allows the easiest
>>> correlation to the /e/n/i config, which is the main source of true for this
>>> panel.
>>> The small downside for that is then that the Alternative Names column is not
>>> a 1:1 mapping of what `ip link` shows as altname, but that probably is not
>>> an actual issue, after all they all live in the same namespace and are
>>> interchangeable for usage with the iproute2 tools.
>>> But that might also mean that we have to treat duplicates explicitly here
>>> in the API call enhancing that info. Waiting until tomorrow makes sense in
>>> any way though, as Stefan will be back from his short vacation then and
>>> might have some input here.
>>
>> i thought a bit about this, and I think there are some disadvantages when
>> just taking the names from /e/n/i:
>>
>> * not sure if it would work at all (depends on ifupdown2), but some
>>     weird configs could make the ui even more confusing, e.g. (assuming nicX is an altname of nicY):
>>
>>           auto nicX
>>           iface nicY inet manual
>>
>>
>>     we now would probably pick up both nicX and nicY, and show both in addition to what ip link says
> 
> Just test it? I'd like to avoid theoretical conversations, this can be
> already confusing enough without that. In anyway, we can normalize that
> on serialization and it's a edge case the user had to manually cause
> themselves, so would not bother to much at all with this case.
> 

yeah, sure. just wanted to write down my thoughts before doing further work

>>
>> * having any custom altname in the /e/n/i would require the computed/shown altnames to be
> 
> What does "custom altname" mean?

i just meant altname instead of what ip link reports as default (sorry for the confusing wording)

> 
>>     calculated for *each*, so e.g. if i have
>>      nicX, nicY and nicZ all point to the same device, we'd have to make the altname list generation
>>     more complicated
>>
>>
>> IMHO it would be ok to always show the 'canonical' name (whatever ip link/kernel says)
>> in the 'name' column and everything else in the 'altname' column regardless what is
>> configured in /e/n/i
> 
> There is no real canonical name, there's a legacy name that might be
> called primary one, but as one can use the others transparently for it
> we really must not think to much into that terminology, it's as first
> class as the other names.

i would interpret the name the kernel has (e.g. what is in /proc/net/dev) as the 'canonical name' ?
please correct me if i'm wrong about that though

> 
>> Disadvantage is of course that for some this might be more confusing as some will be
>> taking /e/n/i as a reference and not 'ip link'....
> 
> Yeah, and ip link shows all names, /e/n/i has normally just the one
> name, so one can easily map any /e/n/i variant to ip link but not vice
> versa.
> 
>>
>>
>> I don't think there is a good way to make it super clear for all situations/users, but in the end
>> we have to decide what is the 'ground truth' (i.e. either /e/n/i or 'ip link')
> 
> Writing this I'm a bit more certain about:
> 
> "iface" entries are our source of truth and the name from POV of the API
> and UI, outdated "auto" statements can be fixed on serialization and also
> normalized on parsing, they can only stem from manual editing and are not
> a real issue.
> All other names pointing to the same interface should be picked up as
> alternative names.
> 
> And if we want to highlight the primary name ip link shows, this could be
> done either inline in the UI, or through by placing it in a dedicated
> column. Both could be followed up on depending on the user feedback we get,
> so I do not think we need to decide that now.
> 
> The /e/n/i config always was the source for the network config view–which
> isn't a network status view–so that should IMO really stick with /e/n/i as
> prime source of truth.
> 
> Seeing status of all interfaces  including statistics and the like might be
> nice, but that probably is handled as new API and component, potentially
> even a cluster wide one. There we could then make iproute2/kernel the primary
> source of truth, and only show any config name a interface is used as.
>   But I'd not think about this to much for the transparent altname support.

ok then we have to think about how to organize the altnames structure properly
otherwise it's a PITA to map from the name in the /e/n/i to all others


(this is all implementation detail of course, but does make a bit of a headache)
currently we have a hash with collections of 'altname' => 'what ip link reports'
but in the case we have altnames in the /e/n/i, we'd actually need the a list of names that are 
equivalent, and that either as a list to search through, or duplicated for each iface name that it 
can reference, so e.g.


if we have nicX, nicY, nicZ all alternatives to each other, now we have:
{
    'nicY' => 'nicX',
    'nicZ' => 'nicX',
}

but we actually want something like this?

{
    'nicX' => ['nixY', 'nicZ'],
    'nicY' => ['nicX', 'nicZ'],
    'nicZ' => ['nicX', 'nicY'],
}


because when we want to show the alternative names of e.g. nicY the first structure is a bit bad to 
search through


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui
  2025-07-15  8:36           ` Dominik Csapak
@ 2025-07-15  8:54             ` Thomas Lamprecht
  2025-07-15  9:00               ` Dominik Csapak
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Lamprecht @ 2025-07-15  8:54 UTC (permalink / raw)
  To: Dominik Csapak, Proxmox VE development discussion, Christoph Heiss

Am 15.07.25 um 10:36 schrieb Dominik Csapak:
> On 7/15/25 10:17, Thomas Lamprecht wrote:
>>
>>>     calculated for *each*, so e.g. if i have
>>>      nicX, nicY and nicZ all point to the same device, we'd have to make the altname list generation
>>>     more complicated
>>>
>>>
>>> IMHO it would be ok to always show the 'canonical' name (whatever ip link/kernel says)
>>> in the 'name' column and everything else in the 'altname' column regardless what is
>>> configured in /e/n/i
>>
>> There is no real canonical name, there's a legacy name that might be
>> called primary one, but as one can use the others transparently for it
>> we really must not think to much into that terminology, it's as first
>> class as the other names.
> 
> i would interpret the name the kernel has (e.g. what is in /proc/net/dev) as the 'canonical name' ?
> please correct me if i'm wrong about that though

Those files and folders will always use the legacy name with the
stricter naming length limits for not surprising any older tool
with the more flexible newer naming,

That's why legacy name might slightly the better term for these,
but as they will stay basically forever it's probably not really
useful to make that distinction either.

Most users won't do much directly with these files, and if they are
likely to be a bit more experienced to figure out this distinction.
Most users, even technical ones, will use iproute2 tools if they
can avoid poking /proc or /sys and especially the less experienced
one only will check /e/n/i and ip addr/link when they run into trouble,
which is they main underlying reason I want to show the altnames in
that config view, so that their live gets a little bit easier by seeing
what "iface XYZ" stanza they will need to adapt independent of the (alt)
name used. And as one can use all names with iproute2 already this has
the nice side effect of adding feature parity and consistency with a
side benefit of a smaller chance for broken upgrades on systemd naming
scheme policy changes. 


> ok then we have to think about how to organize the altnames structure properly
> otherwise it's a PITA to map from the name in the /e/n/i to all others
> 
> 
> (this is all implementation detail of course, but does make a bit of a headache)
> currently we have a hash with collections of 'altname' => 'what ip link reports'
> but in the case we have altnames in the /e/n/i, we'd actually need the a list of names that are 
> equivalent, and that either as a list to search through, or duplicated for each iface name that it 
> can reference, so e.g.
> 
> 
> if we have nicX, nicY, nicZ all alternatives to each other, now we have:
> {
>     'nicY' => 'nicX',
>     'nicZ' => 'nicX',
> }
> 
> but we actually want something like this?
> 
> {
>     'nicX' => ['nixY', 'nicZ'],
>     'nicY' => ['nicX', 'nicZ'],
>     'nicZ' => ['nicX', 'nicY'],
> }
> 
> 
> because when we want to show the alternative names of e.g. nicY the first structure is a bit bad to 
> search through
> 

So we basically have to start with a hash of all names from /e/n/i as keys
and an empty array, then go through the keys and values of the altname map
and add every name (independnt of "primary" = key or "altname" one of the
values for that entry where the /e/n/i name matches one of the keys or values.

fwiw, the API could already add more info if you want to future proof it
w.r.t. putting a distinction between legacy/primary name and altnames.

{

   'e-n-i-name' => {
        'primary' => '...',
         'altnames' => ['...'],
    },
    ...
}

That way we would be flexible with adapting the UI to user feedback while
keeping the API as is. For now we could just merge all primary and altname
with the e-n-i name filtered out in a renderer for the new column.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui
  2025-07-15  8:54             ` Thomas Lamprecht
@ 2025-07-15  9:00               ` Dominik Csapak
  2025-07-15  9:01                 ` Thomas Lamprecht
  0 siblings, 1 reply; 13+ messages in thread
From: Dominik Csapak @ 2025-07-15  9:00 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion, Christoph Heiss

btw. tested using

auto nicX
iface nicY

etc.

it works AFAICT, our parser picks it up as two different ifaces (which makes sense)
and writes out a seperate 'iface' stanza for nicX in this case the next
time it's edited... so fine i guess

On 7/15/25 10:54, Thomas Lamprecht wrote:
> Am 15.07.25 um 10:36 schrieb Dominik Csapak:
>> On 7/15/25 10:17, Thomas Lamprecht wrote:
>>>
>>>>      calculated for *each*, so e.g. if i have
>>>>       nicX, nicY and nicZ all point to the same device, we'd have to make the altname list generation
>>>>      more complicated
>>>>
>>>>
>>>> IMHO it would be ok to always show the 'canonical' name (whatever ip link/kernel says)
>>>> in the 'name' column and everything else in the 'altname' column regardless what is
>>>> configured in /e/n/i
>>>
>>> There is no real canonical name, there's a legacy name that might be
>>> called primary one, but as one can use the others transparently for it
>>> we really must not think to much into that terminology, it's as first
>>> class as the other names.
>>
>> i would interpret the name the kernel has (e.g. what is in /proc/net/dev) as the 'canonical name' ?
>> please correct me if i'm wrong about that though
> 
> Those files and folders will always use the legacy name with the
> stricter naming length limits for not surprising any older tool
> with the more flexible newer naming,
> 
> That's why legacy name might slightly the better term for these,
> but as they will stay basically forever it's probably not really
> useful to make that distinction either.
> 
> Most users won't do much directly with these files, and if they are
> likely to be a bit more experienced to figure out this distinction.
> Most users, even technical ones, will use iproute2 tools if they
> can avoid poking /proc or /sys and especially the less experienced
> one only will check /e/n/i and ip addr/link when they run into trouble,
> which is they main underlying reason I want to show the altnames in
> that config view, so that their live gets a little bit easier by seeing
> what "iface XYZ" stanza they will need to adapt independent of the (alt)
> name used. And as one can use all names with iproute2 already this has
> the nice side effect of adding feature parity and consistency with a
> side benefit of a smaller chance for broken upgrades on systemd naming
> scheme policy changes.
> 

thanks for clarification


> 
>> ok then we have to think about how to organize the altnames structure properly
>> otherwise it's a PITA to map from the name in the /e/n/i to all others
>>
>>
>> (this is all implementation detail of course, but does make a bit of a headache)
>> currently we have a hash with collections of 'altname' => 'what ip link reports'
>> but in the case we have altnames in the /e/n/i, we'd actually need the a list of names that are
>> equivalent, and that either as a list to search through, or duplicated for each iface name that it
>> can reference, so e.g.
>>
>>
>> if we have nicX, nicY, nicZ all alternatives to each other, now we have:
>> {
>>      'nicY' => 'nicX',
>>      'nicZ' => 'nicX',
>> }
>>
>> but we actually want something like this?
>>
>> {
>>      'nicX' => ['nixY', 'nicZ'],
>>      'nicY' => ['nicX', 'nicZ'],
>>      'nicZ' => ['nicX', 'nicY'],
>> }
>>
>>
>> because when we want to show the alternative names of e.g. nicY the first structure is a bit bad to
>> search through
>>
> 
> So we basically have to start with a hash of all names from /e/n/i as keys
> and an empty array, then go through the keys and values of the altname map
> and add every name (independnt of "primary" = key or "altname" one of the
> values for that entry where the /e/n/i name matches one of the keys or values.
> 
> fwiw, the API could already add more info if you want to future proof it
> w.r.t. putting a distinction between legacy/primary name and altnames.
> 
> {
> 
>     'e-n-i-name' => {
>          'primary' => '...',
>           'altnames' => ['...'],
>      },
>      ...
> }
> 
> That way we would be flexible with adapting the UI to user feedback while
> keeping the API as is. For now we could just merge all primary and altname
> with the e-n-i name filtered out in a renderer for the new column.


while that could work, i found an easier way (at least for my code) that
works with the current structure too (i'll send a v2 shortly)



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui
  2025-07-15  9:00               ` Dominik Csapak
@ 2025-07-15  9:01                 ` Thomas Lamprecht
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2025-07-15  9:01 UTC (permalink / raw)
  To: Dominik Csapak, Proxmox VE development discussion, Christoph Heiss

Am 15.07.25 um 11:00 schrieb Dominik Csapak:
> btw. tested using
> 
> auto nicX
> iface nicY
> 
> etc.
> 
> it works AFAICT, our parser picks it up as two different ifaces (which makes sense)
> and writes out a seperate 'iface' stanza for nicX in this case the next
> time it's edited... so fine i guess

Alright, thanks for testing and getting back to me with the results!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] superseded: [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui
  2025-07-14 10:42 [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui Dominik Csapak
                   ` (2 preceding siblings ...)
  2025-07-14 13:49 ` [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui Christoph Heiss
@ 2025-07-15  9:08 ` Dominik Csapak
  3 siblings, 0 replies; 13+ messages in thread
From: Dominik Csapak @ 2025-07-15  9:08 UTC (permalink / raw)
  To: pve-devel

superseded by v2:
https://lore.proxmox.com/all/20250715090749.1608768-1-d.csapak@proxmox.com/T/#t


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-07-15  9:07 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-14 10:42 [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui Dominik Csapak
2025-07-14 10:42 ` [pve-devel] [PATCH widget-toolkit 1/1] network: optionally show alternative interface names Dominik Csapak
2025-07-14 10:42 ` [pve-devel] [PATCH pve-manager 1/1] api/ui: show/return " Dominik Csapak
2025-07-14 13:49 ` [pve-devel] [PATCH manager/widget-toolkit 0/2] show altenative interface names in the web ui Christoph Heiss
2025-07-14 13:53   ` Dominik Csapak
2025-07-14 14:31     ` Thomas Lamprecht
2025-07-15  7:41       ` Dominik Csapak
2025-07-15  8:17         ` Thomas Lamprecht
2025-07-15  8:36           ` Dominik Csapak
2025-07-15  8:54             ` Thomas Lamprecht
2025-07-15  9:00               ` Dominik Csapak
2025-07-15  9:01                 ` Thomas Lamprecht
2025-07-15  9:08 ` [pve-devel] superseded: " Dominik Csapak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal