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.hanre...@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
