[pve-devel] [PATCH manager v2 0/4] improvements to proxmox-network-interface-pinning

[pve-devel] [PATCH manager v2 0/4] improvements to proxmox-network-interface-pinning

[Stefan Hanreich]

Several improvements to the network interface pinning tool:
* interfaces are now iterated according to ifindex
* output of the mapping is now sorted and less verbose
* users can now set a specific target name
* if has been introduced as an additional legal prefix

Still looking into removing the PHYSICAL_NIC_RE altogether, but sending this
patch series as-is nevertheless for review / feedback. If I manage to remove the
PHYSICAL_NIC_RE in time, we could get rid of the constraints of prefix /
target-name altogether...

Changes from v1:
* ran make tidy

pve-manager:

Stefan Hanreich (4):
  network-interface-pinning: use ifindex as order for pinning
  network-interface-pinning: improve printing mapping
  network-interface-pinning: add target-name parameter
  network-interface-pinning: add if prefix to list of allowed prefixes

 PVE/CLI/proxmox_network_interface_pinning.pm | 78 +++++++++++++++++---
 1 file changed, 67 insertions(+), 11 deletions(-)


Summary over all repositories:
  1 files changed, 67 insertions(+), 11 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-manager v2 1/4] network-interface-pinning: use ifindex as order for pinning

[Stefan Hanreich]

While ifindex is not guaranteed to be stable across reboots, it seems
like a good enough heuristic for making sure interfaces with multiple
ports are clamped together when pinning.

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

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index 3f1a83f10..05b1de38a 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -257,7 +257,11 @@ package PVE::CLI::proxmox_network_interface_pinning::InterfaceMapping {
 
         my %existing_names = map { $_ => 1 } values $pinned->%*;
 
-        for my $ifname (sort keys $ip_links->%*) {
+        my @sorted_links = sort {
+            $ip_links->{$a}->{ifindex} <=> $ip_links->{$b}->{ifindex};
+        } keys $ip_links->%*;
+
+        for my $ifname (@sorted_links) {
             my $ip_link = $ip_links->{$ifname};
             my $generated_name;
 
-- 
2.39.5


_______________________________________________
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/4] network-interface-pinning: improve printing mapping

[Stefan Hanreich]

Instead of printing a separate line for each altname, the tool now
only prints one line per physical interface. The primary name is used
as an identifier and the altnames are printed additionally in
parentheses (if they exist). Additionally, the output is now sorted by
ifindex (just as the pin order), so interfaces should now be printed
in ascending order.

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

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index 05b1de38a..9025cd835 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -414,8 +414,28 @@ __PACKAGE__->register_method({
                 exit 0;
             }
 
-            for my $old_name (sort keys $mapping->%*) {
-                print "Name for link '$old_name' will change to '$mapping->{$old_name}'\n";
+            my $altnames = PVE::Network::altname_mapping($ip_links);
+
+            my @sorted_links = sort {
+                my $a_name = $altnames->{$a} // $a;
+                my $b_name = $altnames->{$b} // $b;
+
+                $ip_links->{$a_name}->{ifindex} <=> $ip_links->{$b_name}->{ifindex};
+            } grep {
+                $ip_links->{$_}
+            } keys $mapping->%*;
+
+            for my $old_name (@sorted_links) {
+                my $altname_string = '';
+
+                if (my $interface_altnames = $ip_links->{$old_name}->{altnames}) {
+                    $altname_string = join(', ', $interface_altnames->@*);
+                }
+
+                print "Name for link '$old_name' ";
+                print "($altname_string) " if $altname_string;
+                print "will change to '$mapping->{$old_name}'\n";
+
             }
 
             generate_link_files($ip_links, $mapping);
-- 
2.39.5


_______________________________________________
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 3/4] network-interface-pinning: add target-name parameter

[Stefan Hanreich]

If a specific interface is specified via the interface parameter,
users can now additionally specify a target-name. This makes it easier
for users to assign specific names to specific interfaces, according
to their preferences.

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

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index 9025cd835..357a78544 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -249,6 +249,11 @@ package PVE::CLI::proxmox_network_interface_pinning::InterfaceMapping {
     use PVE::CLI::proxmox_network_interface_pinning;
     use PVE::Tools;
 
+    sub new {
+        my ($class, $mapping) = @_;
+        bless $mapping, $class;
+    }
+
     sub generate {
         my ($class, $ip_links, $pinned, $prefix) = @_;
 
@@ -362,6 +367,12 @@ __PACKAGE__->register_method({
                 default => '<all>', # just for the docs.
                 optional => 1,
             },
+            'target-name' => {
+                description => 'Pin the interface to a specific name',
+                type => 'string',
+                pattern => 'nic\d+',
+                optional => 1,
+            },
         },
     },
     returns => {
@@ -371,6 +382,10 @@ __PACKAGE__->register_method({
         my ($params) = @_;
 
         my $iface = $params->{interface}; # undef means all.
+        my $target_name = $params->{'target-name'};
+
+        die "target-name can only be set, if interface is set as well!\n"
+            if $params->{'target-name'} && !$params->{interface};
 
         if (-t STDOUT) {
             my $target = defined($iface) ? "the interface '$iface'" : 'all interfaces';
@@ -402,12 +417,23 @@ __PACKAGE__->register_method({
                 }
             }
 
-            my $mapping =
-                PVE::CLI::proxmox_network_interface_pinning::InterfaceMapping->generate(
-                    $ip_links,
-                    $pinned,
-                    $prefix,
-                );
+            my $mapping;
+
+            if ($target_name) {
+                die "target-name already exists as link or pin!\n"
+                    if $ip_links->{$target_name} || grep { $target_name eq $_ } values $pinned->%*;
+
+                $mapping = PVE::CLI::proxmox_network_interface_pinning::InterfaceMapping->new({
+                    $iface => $target_name,
+                });
+            } else {
+                $mapping =
+                    PVE::CLI::proxmox_network_interface_pinning::InterfaceMapping->generate(
+                        $ip_links,
+                        $pinned,
+                        $prefix,
+                    );
+            }
 
             if (!$mapping->%*) {
                 print "Nothing to do, aborting.\n";
-- 
2.39.5


_______________________________________________
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 4/4] network-interface-pinning: add if prefix to list of allowed prefixes

[Stefan Hanreich]

'if' has been added as a possible prefix for physical nics in
pve-common. Add it as a possible prefix for pinning network interfaces
here as well.

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

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index 357a78544..e2ad8c69f 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -359,7 +359,6 @@ __PACKAGE__->register_method({
     parameters => {
         additionalProperties => 0,
         properties => {
-            # TODO: support a target name or prefix once pve-common supports generic physical ifaces
             interface => {
                 description => 'Only pin a specific interface.',
                 type => 'string',
@@ -367,10 +366,17 @@ __PACKAGE__->register_method({
                 default => '<all>', # just for the docs.
                 optional => 1,
             },
+            prefix => {
+                description => 'Only pin a specific interface.',
+                type => 'string',
+                enum => ['nic', 'if'],
+                default => 'nic', # just for the docs.
+                optional => 1,
+            },
             'target-name' => {
                 description => 'Pin the interface to a specific name',
                 type => 'string',
-                pattern => 'nic\d+',
+                pattern => '(?:nic|if)\d+',
                 optional => 1,
             },
         },
@@ -398,7 +404,7 @@ __PACKAGE__->register_method({
         }
 
         my $code = sub {
-            my $prefix = 'nic'; # TODO: make flexible once pve-common supports that.
+            my $prefix = $params->{prefix} // 'nic';
 
             my $ip_links = get_ip_links();
             my $pinned = get_pinned();
-- 
2.39.5


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

[pve-devel] applied: [PATCH manager v2 0/4] improvements to proxmox-network-interface-pinning

[Thomas Lamprecht]

On Thu, 24 Jul 2025 11:34:55 +0200, Stefan Hanreich wrote:
> Several improvements to the network interface pinning tool:
> * interfaces are now iterated according to ifindex
> * output of the mapping is now sorted and less verbose
> * users can now set a specific target name
> * if has been introduced as an additional legal prefix
> 
> Still looking into removing the PHYSICAL_NIC_RE altogether, but sending this
> patch series as-is nevertheless for review / feedback. If I manage to remove the
> PHYSICAL_NIC_RE in time, we could get rid of the constraints of prefix /
> target-name altogether...
> 
> [...]

Applied, thanks!

Made two small follow-up commits, one to adapt the 'prefix' parameter
description (was copied from the interface parameter) and replacing the
explicit check for the target-name -> interface parameter dependency with the
built-in support from our JSON schema implementation.

[1/4] network-interface-pinning: use ifindex as order for pinning
      commit: 595b69b02474e7ad8d159cf757249a691c1f7ebb
[2/4] network-interface-pinning: improve printing mapping
      commit: e19c1f48b5d747eb0232e6616e2c938271bf3924
[3/4] network-interface-pinning: add target-name parameter
      commit: 63dddce3cb7c7b4adae5897376bf4aa09b14d61b
[4/4] network-interface-pinning: add if prefix to list of allowed prefixes
      commit: 90057e0ab947804704de91dccab7b8d2b219373f


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