all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH pve-common 1/1] inotify/interfaces: use 'ip link' instead of /proc/net/dev
@ 2025-07-23 14:21 Stefan Hanreich
  2025-07-23 15:02 ` [pve-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Hanreich @ 2025-07-23 14:21 UTC (permalink / raw)
  To: pve-devel

The function reading /etc/network/interfaces used /proc/net/dev to
determine pre-existing physical interfaces. Since the introduction of
altnames, /proc/net/dev returns insufficient information for
determining if an interface is already contained in /e/n/i, since it
does not include altnames.

The interfaces parser added all interfaces from /proc/net/dev as
configurable network devices. If altnames were used in the
configuration, then the same interface would be listed twice: once
with its 'real' name (from /proc/net/dev) and once with its altname
(from the interfaces file).

Adapt the interfaces file parser to utilize 'ip link' instead of
/proc/net/dev as its source of truth regarding configured network
devices. The parser now only adds a physical interface if neither its
'real' name nor any of its altnames are contained in
/etc/network/interfaces.

This patch makes the assumption that a interface is always referred to
by either its name or one of its altnames, but not mixed (e.g. the
stanza uses the 'real' name but bridge_ports uses the altname). In its
current state this will already lead to problems with validation,
since we do not consider altnames when looking up the availability of
bridge ports or VLAN parent devices for instance.

As long as our tooling is used, this assumption should always hold, it
can only be broken by manually editing the interfaces file.

There is one special case still where two entries exist for a single
network interface: If a pinning is generated by
proxmox-network-interface-pinning, but not yet applied. In this case,
the UI will show both network interfaces (nicX and ethX). It will also
show the pending changes that replace ethX with nicX. A possible
solution for improving this case could look at the existing .link
files, check if any not-yet-applied pin exists there and use the
pinned name as the 'real' name when parsing the interfaces file.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/INotify.pm                            | 31 ++++++++++++-------
 test/etc_network_interfaces/ip_link_details   | 20 ++++++++++++
 test/etc_network_interfaces/proc_net_dev      |  5 ---
 test/etc_network_interfaces/runtest.pl        |  7 ++---
 .../t.base-auto-allow-hotplug.pl              | 14 ++++++---
 .../t.create_network.pl                       | 27 +++++++++-------
 .../t.keep-option-order.pl                    | 21 ++++++++-----
 .../t.list-interfaces.pl                      | 27 ++++++++++------
 .../t.ovs_bridge_allow.pl                     | 25 +++++++++------
 .../t.unhandled-interfaces-to-manual.pl       | 28 ++++++++---------
 .../etc_network_interfaces/t.unknown_order.pl | 14 ++++++++-
 11 files changed, 138 insertions(+), 81 deletions(-)
 create mode 100644 test/etc_network_interfaces/ip_link_details
 delete mode 100644 test/etc_network_interfaces/proc_net_dev

diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
index dbad08c..93f8f2d 100644
--- a/src/PVE/INotify.pm
+++ b/src/PVE/INotify.pm
@@ -866,13 +866,13 @@ my $check_mtu = sub {
 # }
 sub read_etc_network_interfaces {
     my ($filename, $fh) = @_;
-    my $proc_net_dev = IO::File->new('/proc/net/dev', 'r');
+    my $ip_links = PVE::Network::ip_link_details();
     my $active = PVE::ProcFSTools::get_active_network_interfaces();
-    return __read_etc_network_interfaces($fh, $proc_net_dev, $active);
+    return __read_etc_network_interfaces($fh, $ip_links, $active);
 }
 
 sub __read_etc_network_interfaces {
-    my ($fh, $proc_net_dev, $active_ifaces) = @_;
+    my ($fh, $ip_links, $active_ifaces) = @_;
 
     my $config = {};
     my $ifaces = $config->{ifaces} = {};
@@ -894,15 +894,6 @@ sub __read_etc_network_interfaces {
 
     my $line;
 
-    if ($proc_net_dev) {
-        while (defined($line = <$proc_net_dev>)) {
-            if ($line =~ m/^\s*($PVE::Network::PHYSICAL_NIC_RE):.*/) {
-                $ifaces->{$1}->{exists} = 1;
-            }
-        }
-        close($proc_net_dev);
-    }
-
     # we try to keep order inside the file
     my $priority = 2; # 1 is reserved for lo
 
@@ -1047,6 +1038,22 @@ SECTION: while (defined($line = <$fh>)) {
         }
     }
 
+    OUTER:
+    for my $iface_name (keys $ip_links->%*) {
+        my $ip_link = $ip_links->{$iface_name};
+
+        next if $iface_name !~ m/^$PVE::Network::PHYSICAL_NIC_RE$/;
+
+        for my $altname ($ip_link->{altnames}->@*) {
+            if ($ifaces->{$altname}) {
+                $ifaces->{$altname}->{exists} = 1;
+                next OUTER;
+            }
+        }
+
+        $ifaces->{$iface_name}->{exists} = 1;
+    }
+
     foreach my $ifname (@$active_ifaces) {
         if (my $iface = $ifaces->{$ifname}) {
             $iface->{active} = 1;
diff --git a/test/etc_network_interfaces/ip_link_details b/test/etc_network_interfaces/ip_link_details
new file mode 100644
index 0000000..93b5bf6
--- /dev/null
+++ b/test/etc_network_interfaces/ip_link_details
@@ -0,0 +1,20 @@
+{
+  "lo": {
+    "ifindex": 1,
+    "ifname": "lo",
+    "link_type": "loopback"
+  },
+  "eth0": {
+    "ifindex": 2,
+    "ifname": "eth0",
+    "link_type": "ether"
+  },
+  "vmbr0": {
+    "ifindex": 3,
+    "ifname": "vmbr0",
+    "link_type": "ether",
+    "linkinfo": {
+      "info_kind": "bridge"
+    }
+  }
+}
diff --git a/test/etc_network_interfaces/proc_net_dev b/test/etc_network_interfaces/proc_net_dev
deleted file mode 100644
index 01df936..0000000
--- a/test/etc_network_interfaces/proc_net_dev
+++ /dev/null
@@ -1,5 +0,0 @@
-Inter-|   Receive                                                |  Transmit
- face |bytes    packets errs drop fifo frame compressed multicast|bytes    packets errs drop fifo colls carrier compressed
- vmbr0:    0          0    0    0    0     0          0         0     0          0    0    0    0     0       0          0
-  eth0:    0          0    0    0    0     0          0         0     0          0    0    0    0     0       0          0
-    lo:    0          0    0    0    0     0          0         0     0          0    0    0    0     0       0          0
diff --git a/test/etc_network_interfaces/runtest.pl b/test/etc_network_interfaces/runtest.pl
index 814b331..b5ef2fd 100755
--- a/test/etc_network_interfaces/runtest.pl
+++ b/test/etc_network_interfaces/runtest.pl
@@ -65,12 +65,11 @@ sub flush_files() {
 # Read an interfaces file with optional /proc/net/dev file content string and
 # the list of active interfaces, which otherwise default
 sub r($;$$) {
-    my ($ifaces, $proc_net_dev, $active) = @_;
-    $proc_net_dev //= load('proc_net_dev');
+    my ($ifaces, $ip_links, $active) = @_;
+    $ip_links //= decode_json(load('ip_link_details'));
     $active //= [split(/\s+/, load('active_interfaces'))];
     open my $fh1, '<', \$ifaces;
-    open my $fh2, '<', \$proc_net_dev;
-    $config = PVE::INotify::__read_etc_network_interfaces($fh1, $fh2, $active);
+    $config = PVE::INotify::__read_etc_network_interfaces($fh1, $ip_links, $active);
     close $fh1;
 }
 
diff --git a/test/etc_network_interfaces/t.base-auto-allow-hotplug.pl b/test/etc_network_interfaces/t.base-auto-allow-hotplug.pl
index 772da83..628146c 100644
--- a/test/etc_network_interfaces/t.base-auto-allow-hotplug.pl
+++ b/test/etc_network_interfaces/t.base-auto-allow-hotplug.pl
@@ -1,23 +1,27 @@
+use JSON;
+
 my $active_ifaces = ['lo', 'ens18', 'ens'];
-my $proc_net = load('proc_net_dev');
-$proc_net =~ s/eth0/ens18/;
+
+my $ip_links = decode_json(load('ip_link_details'));
+$ip_links->{ens18} = delete $ip_links->{eth0};
+$ip_links->{ens18}->{ifname} = ens18;
 
 my $wanted = load('base-allow-hotplug');
 
 # parse the config
-r($wanted, $proc_net, $active_ifaces);
+r($wanted, $ip_links, $active_ifaces);
 
 $wanted =~ s/allow-hotplug ens18/auto ens18/; # FIXME: hack! rather we need to keep allow-hotplug!
 
 expect $wanted;
 
 # idempotency (save, re-parse, and re-check)
-r(w(), $proc_net, $active_ifaces);
+r(w(), $ip_links, $active_ifaces);
 expect $wanted;
 
 # parse one with both, "auto" and "allow-hotplug"
 my $bad = load('base-auto-allow-hotplug');
-r($bad, $proc_net, $active_ifaces);
+r($bad, $ip_links, $active_ifaces);
 
 # should drop the first occuring one of the conflicting options ("auto" currently)
 expect $wanted;
diff --git a/test/etc_network_interfaces/t.create_network.pl b/test/etc_network_interfaces/t.create_network.pl
index 7b5a12f..ef63d0f 100644
--- a/test/etc_network_interfaces/t.create_network.pl
+++ b/test/etc_network_interfaces/t.create_network.pl
@@ -1,13 +1,16 @@
-save('proc_net_dev', <<'/proc/net/dev');
-eth0:
-eth1:
-eth2:
-eth3:
-eth4:
-eth5:
-/proc/net/dev
+use JSON;
+use Storable qw(dclone);
 
-r(load('brbase'));
+my $ip_links = decode_json(load('ip_link_details'));
+
+for my $idx (1..5) {
+    my $entry = dclone($ip_links->{eth0});
+    $entry->{ifname} = "eth$idx";
+
+    $ip_links->{"eth$idx"} = $entry;
+}
+
+r(load('brbase'), $ip_links);
 
 #
 # Variables used for the various interfaces:
@@ -483,14 +486,14 @@ CHECK
 #
 
 save('if', w());
-r(load('if'));
+r(load('if'), $ip_links);
 expect load('if');
 
 #
 # Check a brbase with an ipv6 address on eth1
 #
 
-r(load('brbase'));
+r(load('brbase'), $ip_links);
 
 my $ip = 'fc05::2';
 my $nm = '112';
@@ -535,7 +538,7 @@ iface vmbr0 inet static
 CHECK
 
 save('if', w());
-r(load('if'));
+r(load('if'), $ip_links);
 expect load('if');
 
 1;
diff --git a/test/etc_network_interfaces/t.keep-option-order.pl b/test/etc_network_interfaces/t.keep-option-order.pl
index 6651871..8b506c1 100644
--- a/test/etc_network_interfaces/t.keep-option-order.pl
+++ b/test/etc_network_interfaces/t.keep-option-order.pl
@@ -1,3 +1,15 @@
+use JSON;
+use Storable qw(dclone);
+
+my $ip_links = decode_json(load('ip_link_details'));
+
+for my $idx (1..3) {
+    my $entry = dclone($ip_links->{eth0});
+    $entry->{ifname} = "eth$idx";
+
+    $ip_links->{"eth$idx"} = $entry;
+}
+
 #
 # Order of option lines between interfaces should be preserved:
 # eth0 is unconfigured and will thus end up at the end as 'manual'
@@ -15,14 +27,7 @@ iface eth3 inet manual
 
 ORDERED
 
-r(
-    "$ordered", <<'/proc/net/dev',
-eth0:
-eth1:
-eth2:
-eth3:
-/proc/net/dev
-);
+r($ordered, $ip_links);
 
 expect(load('loopback') . $ordered . "iface eth0 inet manual\n\n");
 
diff --git a/test/etc_network_interfaces/t.list-interfaces.pl b/test/etc_network_interfaces/t.list-interfaces.pl
index 638bc42..d8d1450 100644
--- a/test/etc_network_interfaces/t.list-interfaces.pl
+++ b/test/etc_network_interfaces/t.list-interfaces.pl
@@ -7,13 +7,22 @@
 # The expected behavior is to notice their existance and treat them as manually
 # configured interfaces.
 # Saving the file after reading would add the corresponding 'manual' lines.
-save('proc_net_dev', <<'/proc/net/dev');
-eth0:
-eth1:
-eth2:
-eth3:
-eth100:
-/proc/net/dev
+
+use JSON;
+use Storable qw(dclone);
+
+my $ip_links = decode_json(load('ip_link_details'));
+
+for my $idx (1..3) {
+    my $entry = dclone($ip_links->{eth0});
+    $entry->{ifname} = "eth$idx";
+
+    $ip_links->{"eth$idx"} = $entry;
+}
+
+my $entry = dclone($ip_links->{eth0});
+$entry->{ifname} = "eth100";
+$ip_links->{"eth100"} = $entry;
 
 my %wanted = (
     vmbr0 => {
@@ -85,7 +94,7 @@ source after-ovs
 
 /etc/network/interfaces
 
-r(load('interfaces'));
+r(load('interfaces'), $ip_links);
 save('2', w());
 
 my $ifaces = $config->{ifaces};
@@ -125,7 +134,7 @@ die "invalid families defined for vmbr0"
     if (scalar(@f100) != 2) || ($f100[0] ne 'inet') || ($f100[1] ne 'inet6');
 
 # idempotency
-r(w());
+r(w(), $ip_links);
 expect load('2');
 
 1;
diff --git a/test/etc_network_interfaces/t.ovs_bridge_allow.pl b/test/etc_network_interfaces/t.ovs_bridge_allow.pl
index fabaa8b..f4fbf4c 100644
--- a/test/etc_network_interfaces/t.ovs_bridge_allow.pl
+++ b/test/etc_network_interfaces/t.ovs_bridge_allow.pl
@@ -1,17 +1,22 @@
 use strict;
 
+use JSON;
+use Storable qw(dclone);
+
+my $ip_links = decode_json(load('ip_link_details'));
+
+for my $idx (1..3) {
+    my $entry = dclone($ip_links->{eth0});
+    $entry->{ifname} = "eth$idx";
+
+    $ip_links->{"eth$idx"} = $entry;
+}
+
+
 my $ip = '192.168.0.100/24';
 my $gw = '192.168.0.1';
 
-# replace proc_net_dev with one with a bunch of interfaces
-save('proc_net_dev', <<'/proc/net/dev');
-eth0:
-eth1:
-eth2:
-eth3:
-/proc/net/dev
-
-r('');
+r('', $ip_links);
 
 new_iface(
     'vmbr0',
@@ -82,7 +87,7 @@ iface vmbr0 inet static
 # they're stripped from $config->{options} at load-time since they're
 # auto-generated when writing OVSPorts.
 save('idem', w());
-r(load('idem'));
+r(load('idem'), $ip_links);
 expect load('idem');
 
 # Removing an ovs_port also has to remove the corresponding allow- line!
diff --git a/test/etc_network_interfaces/t.unhandled-interfaces-to-manual.pl b/test/etc_network_interfaces/t.unhandled-interfaces-to-manual.pl
index fd79251..c2dc602 100644
--- a/test/etc_network_interfaces/t.unhandled-interfaces-to-manual.pl
+++ b/test/etc_network_interfaces/t.unhandled-interfaces-to-manual.pl
@@ -1,18 +1,16 @@
-r(
-    '', <<'/proc/net/dev',
-Inter-|   Receive                                                |  Transmit
- face |bytes    packets errs drop fifo frame compressed multicast|bytes    packets errs drop fifo colls carrier compressed
-These eth interfaces show up:
-  eth0:
-eth1:
-       eth2:
-  eth3:
-    lo:
-All other stuff is being ignored eth99:
-eth100 is not actually available:
- ethBAD: this one's now allowed either
-/proc/net/dev
-);
+use JSON;
+use Storable qw(dclone);
+
+my $ip_links = decode_json(load('ip_link_details'));
+
+for my $idx (1..3) {
+    my $entry = dclone($ip_links->{eth0});
+    $entry->{ifname} = "eth$idx";
+
+    $ip_links->{"eth$idx"} = $entry;
+}
+
+r('', $ip_links);
 
 expect load('base') . <<'IFACES';
 iface eth1 inet manual
diff --git a/test/etc_network_interfaces/t.unknown_order.pl b/test/etc_network_interfaces/t.unknown_order.pl
index 291b858..dd73069 100644
--- a/test/etc_network_interfaces/t.unknown_order.pl
+++ b/test/etc_network_interfaces/t.unknown_order.pl
@@ -1,3 +1,15 @@
+use JSON;
+use Storable qw(dclone);
+
+my $ip_links = decode_json(load('ip_link_details'));
+
+for my $idx (1..5) {
+    my $entry = dclone($ip_links->{eth0});
+    $entry->{ifname} = "eth$idx";
+
+    $ip_links->{"eth$idx"} = $entry;
+}
+
 my $base = load('loopback');
 
 sub wanted($) {
@@ -91,7 +103,7 @@ iface vmbr5 inet manual
 IFACES
 }
 
-r(wanted(13));
+r(wanted(13), $ip_links);
 update_iface('bond1', [{ family => 'inet', address => '10.10.10.11/24' }]);
 expect wanted(11);
 
-- 
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] 6+ messages in thread

* [pve-devel] applied: [PATCH pve-common 1/1] inotify/interfaces: use 'ip link' instead of /proc/net/dev
  2025-07-23 14:21 [pve-devel] [PATCH pve-common 1/1] inotify/interfaces: use 'ip link' instead of /proc/net/dev Stefan Hanreich
@ 2025-07-23 15:02 ` Thomas Lamprecht
  2025-07-23 15:16   ` Stefan Hanreich
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Lamprecht @ 2025-07-23 15:02 UTC (permalink / raw)
  To: pve-devel, Stefan Hanreich

On Wed, 23 Jul 2025 16:21:06 +0200, Stefan Hanreich wrote:
> The function reading /etc/network/interfaces used /proc/net/dev to
> determine pre-existing physical interfaces. Since the introduction of
> altnames, /proc/net/dev returns insufficient information for
> determining if an interface is already contained in /e/n/i, since it
> does not include altnames.
> 
> The interfaces parser added all interfaces from /proc/net/dev as
> configurable network devices. If altnames were used in the
> configuration, then the same interface would be listed twice: once
> with its 'real' name (from /proc/net/dev) and once with its altname
> (from the interfaces file).
> 
> [...]

I tested this one some configurations where /e/n/i used different names
compared to the "primary" interface name the kernels uses, made it work fine
now. Implementation wise it looks relative straight forward.

I do not recall for sure anymore, but do differing bridge-ports work
transparently with the ifupdown2 changes from Christoph. With that it might be
nice to support it here too in the midterm, but that is certainly not a blocker
for now.

Applied with perltidy formatting changes squashed in, thanks!

[1/1] inotify/interfaces: use 'ip link' instead of /proc/net/dev
      commit: 346ff9744b036c529373cab5b503eaaf37a5bf03


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


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

* Re: [pve-devel] applied: [PATCH pve-common 1/1] inotify/interfaces: use 'ip link' instead of /proc/net/dev
  2025-07-23 15:02 ` [pve-devel] applied: " Thomas Lamprecht
@ 2025-07-23 15:16   ` Stefan Hanreich
  2025-07-23 15:33     ` Thomas Lamprecht
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Hanreich @ 2025-07-23 15:16 UTC (permalink / raw)
  To: Thomas Lamprecht, pve-devel

On 7/23/25 17:06, Thomas Lamprecht wrote:

[snip]

> I do not recall for sure anymore, but do differing bridge-ports work
> transparently with the ifupdown2 changes from Christoph. With that it might be
> nice to support it here too in the midterm, but that is certainly not a blocker
> for now.

I'm not sure if I understand 100% - do you mean if the name used in
bridge-ports differs from the name of the referenced interface in
/e/n/i? That doesn't work currently, since the validation breaks.

I've discussed this initially with Dominik today, and we'd need to
resolve altnames every time we look up names from bridge-ports, etc.

If we want to support mixing names in the configuration, then we'd
additionally have to construct a temporary, merged, configuration and
validate against that.

> Applied with perltidy formatting changes squashed in, thanks!

I will pay more attention to running `make tidy` in the future, still
not used to the fact that we actually have a formatter now - sorry.


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


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

* Re: [pve-devel] applied: [PATCH pve-common 1/1] inotify/interfaces: use 'ip link' instead of /proc/net/dev
  2025-07-23 15:16   ` Stefan Hanreich
@ 2025-07-23 15:33     ` Thomas Lamprecht
  2025-07-23 15:53       ` Stefan Hanreich
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Lamprecht @ 2025-07-23 15:33 UTC (permalink / raw)
  To: Stefan Hanreich, pve-devel

Am 23.07.25 um 17:16 schrieb Stefan Hanreich:
> On 7/23/25 17:06, Thomas Lamprecht wrote:
> 
> [snip]
> 
>> I do not recall for sure anymore, but do differing bridge-ports work
>> transparently with the ifupdown2 changes from Christoph. With that it might be
>> nice to support it here too in the midterm, but that is certainly not a blocker
>> for now.
> 
> I'm not sure if I understand 100% - do you mean if the name used in
> bridge-ports differs from the name of the referenced interface in
> /e/n/i? That doesn't work currently, since the validation breaks.

Yeah no, I mean does it work for ifupdown2 now? As every situation
that works there but confuses our e/n/i parser is naturally not ideal
(for the long term).

> I've discussed this initially with Dominik today, and we'd need to
> resolve altnames every time we look up names from bridge-ports, etc.

That sounds like this is some expensive and high frequency operation,
but isn't it really only required when managing the node network or
wanting to do getting all available interfaces or doing some sanity
checks, i.e. things that invovle parsing the network config, where we
now have the altname map queried anyway?

Or am I overlooking something?

> If we want to support mixing names in the configuration, then we'd
> additionally have to construct a temporary, merged, configuration and
> validate against that.

If ifupdown2 currently still doesn't supports that we're good in any
case, if it supports it would be nice to keep our stack consistent with
ifudpown2, but as hinted, not pressing now especially as our stack, as
you mentioned, never creates such a mixed config anyway.

> 
>> Applied with perltidy formatting changes squashed in, thanks!
> 
> I will pay more attention to running `make tidy` in the future, still
> not used to the fact that we actually have a formatter now - sorry.
> 

No worries, I forget it myself more often than I'd like to
admit, but it gets better with time ^^


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


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

* Re: [pve-devel] applied: [PATCH pve-common 1/1] inotify/interfaces: use 'ip link' instead of /proc/net/dev
  2025-07-23 15:33     ` Thomas Lamprecht
@ 2025-07-23 15:53       ` Stefan Hanreich
  2025-07-23 15:57         ` Thomas Lamprecht
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Hanreich @ 2025-07-23 15:53 UTC (permalink / raw)
  To: Thomas Lamprecht, pve-devel



On 7/23/25 17:33, Thomas Lamprecht wrote:
> Am 23.07.25 um 17:16 schrieb Stefan Hanreich:
>> On 7/23/25 17:06, Thomas Lamprecht wrote:
>>
>> [snip]
>>
>>> I do not recall for sure anymore, but do differing bridge-ports work
>>> transparently with the ifupdown2 changes from Christoph. With that it might be
>>> nice to support it here too in the midterm, but that is certainly not a blocker
>>> for now.
>>
>> I'm not sure if I understand 100% - do you mean if the name used in
>> bridge-ports differs from the name of the referenced interface in
>> /e/n/i? That doesn't work currently, since the validation breaks.
> 
> Yeah no, I mean does it work for ifupdown2 now? As every situation
> that works there but confuses our e/n/i parser is naturally not ideal
> (for the long term).

It does work there atm (just re-checked).


>> I've discussed this initially with Dominik today, and we'd need to
>> resolve altnames every time we look up names from bridge-ports, etc.
> 
> That sounds like this is some expensive and high frequency operation,
> but isn't it really only required when managing the node network or
> wanting to do getting all available interfaces or doing some sanity
> checks, i.e. things that invovle parsing the network config, where we
> now have the altname map queried anyway?
> 
> Or am I overlooking something?

I meant in the context of reading/writing the interfaces file. I haven't
100% thought it through, and I might be missing something critical, but
afaict it is as you said: We'd have to do it when reading / writing the
network configuration from /e/n/i and in the pve-manager Network API
when updating, since it does some additional checks (e.g.
check_duplicate_ports) before writing via the function in INotify.


>> If we want to support mixing names in the configuration, then we'd
>> additionally have to construct a temporary, merged, configuration and
>> validate against that.
> 
> If ifupdown2 currently still doesn't supports that we're good in any
> case, if it supports it would be nice to keep our stack consistent with
> ifudpown2, but as hinted, not pressing now especially as our stack, as
> you mentioned, never creates such a mixed config anyway.

see above, fully supporting the current ifupdown2 capabilities will
require untangling.


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


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

* Re: [pve-devel] applied: [PATCH pve-common 1/1] inotify/interfaces: use 'ip link' instead of /proc/net/dev
  2025-07-23 15:53       ` Stefan Hanreich
@ 2025-07-23 15:57         ` Thomas Lamprecht
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Lamprecht @ 2025-07-23 15:57 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich

Am 23.07.25 um 17:53 schrieb Stefan Hanreich:
> see above, fully supporting the current ifupdown2 capabilities will
> require untangling.

Ack, thanks for looking into this. Might be best done with switching
more of this to rust, ideally with some more tests added upfront and
using the same test as closely as possible for old and new.


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


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

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

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-23 14:21 [pve-devel] [PATCH pve-common 1/1] inotify/interfaces: use 'ip link' instead of /proc/net/dev Stefan Hanreich
2025-07-23 15:02 ` [pve-devel] applied: " Thomas Lamprecht
2025-07-23 15:16   ` Stefan Hanreich
2025-07-23 15:33     ` Thomas Lamprecht
2025-07-23 15:53       ` Stefan Hanreich
2025-07-23 15:57         ` Thomas Lamprecht

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal