[pve-devel] [PATCH common/manager v2 0/3] arbitrary prefixes for pinning network interfaces

[pve-devel] [PATCH common/manager v2 0/3] arbitrary prefixes for pinning network interfaces

[Stefan Hanreich]

This patch series lifts the restriction for naming physical interfaces.
Previously we relied on a regex (PHYSICAL_NIC_RE) for determining whether an
interface was physical or not. This patch series changes that, by querying the
kernel for the type of the interface and using that to determine whether an
interface is a physical interface or not. This allows us to use arbitrary names
for physical interfaces, which in turn allows proxmox-network-interface-pinning
to use arbitrary prefixes / target-names when pinning network interfaces.

The main change here is with pvestatd and metric collection, where we used the
regex to determine the type of interface in PullMetric. I introduced a new key
in the netdev hash that indicates the type of interface. Since my knowledge of
this component is almost zero, I'm not sure if this is the proper approach.
Maybe someone with more experience can chime in on possible issues /
improvements.

pve-manager depends on pve-common

Changes from v1 to v2:
* Cache ip link output
* add dependency for pvenetcommit to pvestatd
* use type only to distinguish between physical / virtual interfaces

pve-common:

Stefan Hanreich (1):
  inotify/interfaces: use ip link for detecting physical interfaces

 src/PVE/INotify.pm | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)


pve-manager:

Stefan Hanreich (2):
  pvestatd: pull metric: use ip link to detect physical interfaces
  network-interface-pinning: allow arbitrary names

 PVE/CLI/proxmox_network_interface_pinning.pm |  4 ++--
 PVE/PullMetric.pm                            | 15 ++++++++++++---
 PVE/Service/pvestatd.pm                      | 15 ++++++++++++++-
 services/pvestatd.service                    |  2 +-
 4 files changed, 29 insertions(+), 7 deletions(-)


Summary over all repositories:
  5 files changed, 42 insertions(+), 19 deletions(-)

-- 
Generated by git-murpp 0.8.0

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

[pve-devel] [PATCH pve-common v2 1/1] inotify/interfaces: use ip link for detecting physical interfaces

[Stefan Hanreich]

The parser for /e/n/i relied on PHYSICAL_NIC_RE for detecting physical
interfaces. In order to allow arbitrary interface names for pinning
physical interfaces, switch over to detecting physical interfaces via
'ip link' instead.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/INotify.pm | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
index 7466c40..d7e5add 100644
--- a/src/PVE/INotify.pm
+++ b/src/PVE/INotify.pm
@@ -878,6 +878,8 @@ sub __read_etc_network_interfaces {
     my $ifaces = $config->{ifaces} = {};
     my $options = $config->{options} = [];
 
+    my $altnames = PVE::Network::altname_mapping($ip_links);
+
     my $options_alternatives = {
         'ovs_mtu' => 'mtu',
         'bond-slaves' => 'slaves',
@@ -1042,7 +1044,7 @@ 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$/;
+        next if !PVE::Network::ip_link_is_physical($ip_link);
 
         for my $altname ($ip_link->{altnames}->@*) {
             if ($ifaces->{$altname}) {
@@ -1072,6 +1074,9 @@ OUTER:
     foreach my $iface (sort keys %$ifaces) {
         my $d = $ifaces->{$iface};
         $d->{type} = 'unknown';
+
+        my $ip_link = $ip_links->{$altnames->{$iface} // $iface};
+
         if (defined $d->{'bridge_ports'}) {
             $d->{type} = 'bridge';
             if (!defined($d->{bridge_stp})) {
@@ -1140,7 +1145,7 @@ OUTER:
                 $ifaces->{$raw_iface}->{exists} = 0;
                 $d->{exists} = 0;
             }
-        } elsif ($iface =~ m/^$PVE::Network::PHYSICAL_NIC_RE$/) {
+        } elsif ($ip_link && PVE::Network::ip_link_is_physical($ip_link)) {
             if (!$d->{ovs_type}) {
                 $d->{type} = 'eth';
             } elsif ($d->{ovs_type} eq 'OVSPort') {
@@ -1550,16 +1555,12 @@ sub __write_etc_network_interfaces {
         if ($d->{type} eq 'OVSPort' || $d->{type} eq 'OVSIntPort' || $d->{type} eq 'OVSBond') {
             my $brname = $used_ports->{$iface};
             if (!$brname || !$ifaces->{$brname}) {
-                if ($iface =~ /^$PVE::Network::PHYSICAL_NIC_RE/) {
-                    $ifaces->{$iface} = {
-                        type => 'eth',
-                        exists => 1,
-                        method => 'manual',
-                        families => ['inet'],
-                    };
-                } else {
-                    delete $ifaces->{$iface};
-                }
+                $ifaces->{$iface} = {
+                    type => 'eth',
+                    exists => 1,
+                    method => 'manual',
+                    families => ['inet'],
+                };
                 next;
             }
             my $bd = $ifaces->{$brname};
-- 
2.47.2


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

[pve-devel] [PATCH pve-manager v2 1/2] pvestatd: pull metric: use ip link to detect physical interfaces

[Stefan Hanreich]

pve-common now allows arbitrary names for physical interfaces, without
being restricted by PHYSICAL_NIC_RE. In order to detect physical
interfaces, pvestatd now needs to query 'ip link' for the type of an
interface instead of relying on the regular expression.

On the receiving end, PullMetric cannot consult 'ip link' for
determining which interface is physical or not. To work around that,
introduce a new type key, that carries information about the type of
an interface. When aggregating the metrics, PullMetric can now read
this additional parameter, to infer the type of the interface (either
physical or virtual).

To avoid spawning a process in every update loop of pvestatd, cache
the output once and then use it throughout the lifecycle of pvestatd.
Physical interfaces rarely get added / removed without a reboot, so we
can cache this indefinitely. Users can always restart the service to
refresh the information about physical interfaces.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 PVE/PullMetric.pm         | 15 ++++++++++++---
 PVE/Service/pvestatd.pm   | 15 ++++++++++++++-
 services/pvestatd.service |  2 +-
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/PVE/PullMetric.pm b/PVE/PullMetric.pm
index f55653505..24310c30e 100644
--- a/PVE/PullMetric.pm
+++ b/PVE/PullMetric.pm
@@ -91,9 +91,18 @@ my sub get_node_metrics {
     push @$metrics, gauge($id, $timestamp, "uptime", $data->{uptime});
 
     my ($netin, $netout) = (0, 0);
-    for my $dev (grep { /^$PVE::Network::PHYSICAL_NIC_RE$/ } keys $data->{nics}->%*) {
-        $netin += $data->{nics}->{$dev}->{receive};
-        $netout += $data->{nics}->{$dev}->{transmit};
+
+    for my $dev (keys $data->{nics}->%*) {
+        my $nic_data = $data->{nics}->{$dev};
+
+        if ($nic_data->{type}) {
+            next if $nic_data->{type} ne 'physical';
+        } else {
+            next if $dev !~ /^$PVE::Network::PHYSICAL_NIC_RE$/;
+        }
+
+        $netin += $nic_data->{receive};
+        $netout += $nic_data->{transmit};
     }
     push @$metrics, derive($id, $timestamp, "net_in", $netin);
     push @$metrics, derive($id, $timestamp, "net_out", $netout);
diff --git a/PVE/Service/pvestatd.pm b/PVE/Service/pvestatd.pm
index e645eec3c..d662e3070 100755
--- a/PVE/Service/pvestatd.pm
+++ b/PVE/Service/pvestatd.pm
@@ -155,6 +155,8 @@ my sub broadcast_static_node_info {
     }
 }
 
+my $cached_ip_links = undef;
+
 sub update_node_status {
     my ($status_cfg, $pull_txn) = @_;
 
@@ -171,9 +173,20 @@ sub update_node_status {
     my $sublevel = $subinfo->{level} || '';
 
     my $netdev = PVE::ProcFSTools::read_proc_net_dev();
+    $cached_ip_links = PVE::Network::ip_link_details() if !$cached_ip_links;
+
     # traffic from/to physical interface cards
     my ($netin, $netout) = (0, 0);
-    for my $dev (grep { /^$PVE::Network::PHYSICAL_NIC_RE$/ } keys %$netdev) {
+    for my $dev (keys %$netdev) {
+        my $ip_link = $cached_ip_links->{$dev};
+
+        if (PVE::Network::ip_link_is_physical($ip_link)) {
+            $netdev->{$dev}->{type} = 'physical';
+        } else {
+            $netdev->{$dev}->{type} = 'virtual';
+            next;
+        }
+
         $netin += $netdev->{$dev}->{receive};
         $netout += $netdev->{$dev}->{transmit};
     }
diff --git a/services/pvestatd.service b/services/pvestatd.service
index d7db50f6d..68a05305f 100644
--- a/services/pvestatd.service
+++ b/services/pvestatd.service
@@ -2,7 +2,7 @@
 Description=PVE Status Daemon
 ConditionPathExists=/usr/bin/pvestatd
 Wants=pve-cluster.service
-After=pve-cluster.service
+After=pve-cluster.service pvenetcommit.service
 
 [Service]
 ExecStart=/usr/bin/pvestatd start
-- 
2.47.2


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

[pve-devel] [PATCH pve-manager v2 2/2] network-interface-pinning: allow arbitrary names

[Stefan Hanreich]

With the changes to physical interface detection in pve-common and
pve-manager, it is now possible to use arbitrary names for physical
interfaces in our network stack. This allows the removal of the
existing, hardcoded, prefixes.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 PVE/CLI/proxmox_network_interface_pinning.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index 9bbc2929d..adc0acd52 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -370,14 +370,14 @@ __PACKAGE__->register_method({
                 description =>
                     'Use a specific prefix for automatically choosing the pinned name.',
                 type => 'string',
-                enum => ['nic', 'if'],
+                pattern => '^[a-zA-Z][a-zA-Z0-9-_]{0,7}$',
                 default => 'nic', # just for the docs.
                 optional => 1,
             },
             'target-name' => {
                 description => 'Pin the interface to a specific name.',
                 type => 'string',
-                pattern => '(?:nic|if)\d+',
+                format => 'pve-iface',
                 optional => 1,
                 requires => 'interface',
             },
-- 
2.47.2


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

[pve-devel] applied: [PATCH pve-common v2 1/1] inotify/interfaces: use ip link for detecting physical interfaces

[Thomas Lamprecht]

On Tue, 29 Jul 2025 19:16:43 +0200, Stefan Hanreich wrote:
> The parser for /e/n/i relied on PHYSICAL_NIC_RE for detecting physical
> interfaces. In order to allow arbitrary interface names for pinning
> physical interfaces, switch over to detecting physical interfaces via
> 'ip link' instead.
> 
> 

Applied, thanks!

[1/1] inotify/interfaces: use ip link for detecting physical interfaces
      commit: 644934527ac118f5bfc519b3c2c3ecdc6218288b


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

[pve-devel] applied: [PATCH pve-manager v2 1/2] pvestatd: pull metric: use ip link to detect physical interfaces

[Thomas Lamprecht]

On Tue, 29 Jul 2025 19:16:44 +0200, Stefan Hanreich wrote:
> pve-common now allows arbitrary names for physical interfaces, without
> being restricted by PHYSICAL_NIC_RE. In order to detect physical
> interfaces, pvestatd now needs to query 'ip link' for the type of an
> interface instead of relying on the regular expression.
> 
> On the receiving end, PullMetric cannot consult 'ip link' for
> determining which interface is physical or not. To work around that,
> introduce a new type key, that carries information about the type of
> an interface. When aggregating the metrics, PullMetric can now read
> this additional parameter, to infer the type of the interface (either
> physical or virtual).
> 
> [...]

Applied, thanks!

I added a time-based expiry check to re-cache this every 15 minutes in a follow
up, it's a very simple check and this way we converge relatively fast while not
doing that many updates.

[1/2] pvestatd: pull metric: use ip link to detect physical interfaces
      commit: 83d0b982e6ad498031f0f1d97511b441c98a7706


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

[pve-devel] applied: [PATCH pve-manager v2 2/2] network-interface-pinning: allow arbitrary names

[Thomas Lamprecht]

On Tue, 29 Jul 2025 19:16:45 +0200, Stefan Hanreich wrote:
> With the changes to physical interface detection in pve-common and
> pve-manager, it is now possible to use arbitrary names for physical
> interfaces in our network stack. This allows the removal of the
> existing, hardcoded, prefixes.
> 
> 

Applied, thanks!

[2/2] network-interface-pinning: allow arbitrary names
      commit: 57135496bad52c59062857ecdd3a7577fef7b495


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