[pve-devel] [PATCH common/manager v2 00/18] backport 'proxmox-network-interface-pinning fixes'

[pve-devel] [PATCH common/manager v2 00/18] backport 'proxmox-network-interface-pinning fixes'

[Stefan Hanreich]

Contains the changes from the original series minus the changes made because of
the SDN fabrics. For more details see the respective commits / original series.

I've decided to leave the complete history intact, but we could squash a lot of
the bugfixes into the initial two commits, if so desired.

pve-manager depends on pve-common

pve-common:

Stefan Hanreich (2):
  network: add ip link and altname helpers
  network: add nic prefix to physical nic regex

 src/PVE/Network.pm | 47 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)


pve-manager:

Stefan Hanreich (10):
  cli: add proxmox-network-interface-pinning tool
  services: add pvesdncommit and pvefirewallcommit
  pve-sdn-commit: fix reloading logic
  proxmox-network-interface-pinning: die on failing to write interfaces
  proxmox-network-interface-pinning: fix pinning after reboot
  network-interface-pinning: avoid comparing undefined string
  {sdn, firewall}-commit: wait for quorum
  sdn-commit: only reload ifupdown if sdn configuration changed
  network-interface-pinning: fix subsequent invocations
  network-interface-pinning: early exit if nothing to do

Thomas Lamprecht (6):
  use kebab-case spelling for new SDN and firewall config-commit
    services
  firewall on-boot commit: report errors if rename fails
  nic pinning: prompt before continuing if connected to TTY
  nic pinning: update description for generate command
  nic pinning: rename 'nic' parameter to 'interface'
  nic pinning: improve some informational and error output
    wording/formatting

 PVE/CLI/Makefile                             |   1 +
 PVE/CLI/proxmox_network_interface_pinning.pm | 415 +++++++++++++++++++
 bin/Makefile                                 |  21 +-
 bin/proxmox-network-interface-pinning        |   8 +
 bin/pve-firewall-commit                      |  27 ++
 bin/pve-sdn-commit                           |  77 ++++
 debian/postinst                              |   2 +-
 services/Makefile                            |   4 +-
 services/pve-firewall-commit.service         |  13 +
 services/pve-sdn-commit.service              |  13 +
 10 files changed, 578 insertions(+), 3 deletions(-)
 create mode 100644 PVE/CLI/proxmox_network_interface_pinning.pm
 create mode 100644 bin/proxmox-network-interface-pinning
 create mode 100644 bin/pve-firewall-commit
 create mode 100644 bin/pve-sdn-commit
 create mode 100644 services/pve-firewall-commit.service
 create mode 100644 services/pve-sdn-commit.service


Summary over all repositories:
  11 files changed, 624 insertions(+), 4 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/2] network: add ip link and altname helpers

[Stefan Hanreich]

Those helpers will be used by several other packages to implement the
altname support. Those helpers will also be used by the new pveeth
tool which can be used for pinning interface names.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250709194526.560709-2-s.hanreich@proxmox.com
---
 src/PVE/Network.pm | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
index d084b36..ce87b93 100644
--- a/src/PVE/Network.pm
+++ b/src/PVE/Network.pm
@@ -973,4 +973,49 @@ sub is_ovs_bridge {
     die "failed to query OVS to determine type of '$bridge': $res\n";
 }
 
+sub ip_link_details {
+    my $link_json = '';
+
+    PVE::Tools::run_command(
+        ['ip', '-details', '-json', 'link', 'show'],
+        outfunc => sub {
+            $link_json .= shift;
+        }
+    );
+
+    my $links = JSON::decode_json($link_json);
+    my %ip_links = map { $_->{ifname} => $_ } $links->@*;
+
+    return \%ip_links;
+}
+
+sub ip_link_is_physical {
+    my ($ip_link) = @_;
+
+    # ether alone isn't enough, as virtual interfaces can also have link_type
+    # ether
+    return $ip_link->{link_type} eq 'ether'
+        && (!defined($ip_link->{linkinfo}) || !defined($ip_link->{linkinfo}->{info_kind}));
+}
+
+sub altname_mapping {
+    my ($ip_links) = @_;
+
+    $ip_links = ip_link_details() if !defined($ip_links);
+
+    my $altnames = {};
+
+    foreach my $iface_name (keys $ip_links->%*) {
+        my $iface = $ip_links->{$iface_name};
+
+        next if !$iface->{altnames};
+
+        foreach my $altname ($iface->{altnames}->@*) {
+            $altnames->{$altname} = $iface_name;
+        }
+    }
+
+    return $altnames;
+}
+
 1;
-- 
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-common v2 2/2] network: add nic prefix to physical nic regex

[Stefan Hanreich]

With the introduction of pveeth, users can now pin their NICs with
prefix nicX. In order for our stack to correctly pick up the pinned
interfaces, we need to add this prefix to the regex used for detecting
physical interfaces.

In the future we should abandon this method of detecting physical
interfaces altogether, either by using `ip link` or talking Netlink
directly. For now, we add this as a stop-gap so the pveeth tool
proof-of-concept works.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250709194526.560709-3-s.hanreich@proxmox.com
---
 src/PVE/Network.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
index ce87b93..b305105 100644
--- a/src/PVE/Network.pm
+++ b/src/PVE/Network.pm
@@ -17,7 +17,7 @@ use Socket qw(NI_NUMERICHOST NI_NUMERICSERV);
 
 # host network related utility functions
 
-our $PHYSICAL_NIC_RE = qr/(?:eth\d+|en[^:.]+|ib[^:.]+)/;
+our $PHYSICAL_NIC_RE = qr/(?:eth\d+|en[^:.]+|ib[^:.]+|nic\d+)/;
 
 our $ipv4_reverse_mask = [
     '0.0.0.0',
-- 
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 01/16] cli: add proxmox-network-interface-pinning tool

[Stefan Hanreich]

proxmox-network-interface-pinning is a tool for pinning network
interface names. It works by generating a link file in
/usr/local/lib/systemd/network and then updating the following files
by replacing the current name with the pinned name:

* /etc/network/interfaces
* /etc/pve/nodes/nodename/host.fw
* /etc/pve/sdn/controllers.cfg (IS-IS controllers)

In each case the tool creates a pending configuration file, that gets
applied on reboot (via pvenetcommit, pvesdncommit and
pvefirewallcommit respectively).

SDN and /e/n/i already have pending configuration files built in, so
the tool writes to them. For the host firewall we introduce a
host.fw.new file, that is currently only used by the
proxmox-network-interface-pinning tool, but could be used in the
future for creating pending configurations for the firewall stack.

There are still some places where interface names occur, where we do
not update the configuration:

* /etc/pve/firewall/cluster.fw - This is because we cannot update a
cluster-wide file with the locally-generated mappings. In this case a
warning is printed.

* In the node configuration there is a parameter for wakeonlan that
takes an interface as argument.

Otherwise all occurrences of interfaces or interface lists should be
included.

Example invocations of pveeth:

$ proxmox-network-interface-pinning generate --nic enp1s0

Generates a pinning for enp1s0 (if it doesn't exist already) and
updates the configuration file.

$ proxmox-network-interface-pinning generate

Generates a pinning for all physical interfaces, that do not yet have
one.

After rebooting, all pending changes made by the tool should get
automatically applied via the respective systemd one-shot services
(see the following commit).

Currently there is only support for a fixed prefix: 'nic'. This is
because we rely on PHYISCAL_NIC_RE for detecting physical network
interfaces across several places in our codebase. For now, nic has
been added as a valid prefix for NICs in pve-common, so that prefix is
used here.

In order to support custom prefixes, every place in the code relying
on PHYISCAL_NIC_RE (at least) would have to be reworked.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250716151815.348161-8-s.hanreich@proxmox.com
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 PVE/CLI/Makefile                             |   1 +
 PVE/CLI/proxmox_network_interface_pinning.pm | 396 +++++++++++++++++++
 bin/Makefile                                 |  17 +
 bin/proxmox-network-interface-pinning        |   8 +
 4 files changed, 422 insertions(+)
 create mode 100644 PVE/CLI/proxmox_network_interface_pinning.pm
 create mode 100644 bin/proxmox-network-interface-pinning

diff --git a/PVE/CLI/Makefile b/PVE/CLI/Makefile
index 9ff2aeb58..f85047d37 100644
--- a/PVE/CLI/Makefile
+++ b/PVE/CLI/Makefile
@@ -10,6 +10,7 @@ SOURCES = \
 	pvesh.pm \
 	pve7to8.pm \
 	pve8to9.pm \
+	proxmox_network_interface_pinning.pm \
 
 all:
 
diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
new file mode 100644
index 000000000..5dea9126a
--- /dev/null
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -0,0 +1,396 @@
+package PVE::CLI::proxmox_network_interface_pinning;
+
+use strict;
+use warnings;
+
+use File::Copy;
+use POSIX qw(:errno_h);
+use Storable qw(dclone);
+
+use PVE::Firewall;
+use PVE::INotify;
+use PVE::Network;
+use PVE::Network::SDN;
+use PVE::Network::SDN::Controllers;
+use PVE::RPCEnvironment;
+use PVE::SectionConfig;
+use PVE::Tools;
+
+use PVE::CLIHandler;
+use base qw(PVE::CLIHandler);
+
+my $PVEETH_LOCK = "/run/lock/proxmox-network-interface-pinning.lck";
+
+sub setup_environment {
+    PVE::RPCEnvironment->setup_default_cli_env();
+}
+
+my sub update_sdn_controllers {
+    my ($mapping) = @_;
+
+    print "Updating /etc/pve/sdn/controllers.cfg\n";
+
+    my $code = sub {
+        my $controllers = PVE::Network::SDN::Controllers::config();
+
+        my $local_node = PVE::INotify::nodename();
+
+        for my $controller (values $controllers->{ids}->%*) {
+            next
+                if $local_node ne $controller->{node}
+                || $controller->{type} ne 'isis';
+
+            $controller->{'isis-ifaces'} = $mapping->list($controller->{'isis-ifaces'});
+        }
+
+        PVE::Network::SDN::Controllers::write_config($controllers);
+    };
+
+    PVE::Network::SDN::lock_sdn_config($code);
+}
+
+my sub update_etc_network_interfaces {
+    my ($mapping, $existing_pins) = @_;
+
+    print "Updating /etc/network/interfaces.new\n";
+
+    my $code = sub {
+        my $config = dclone(PVE::INotify::read_file('interfaces'));
+
+        my $old_ifaces = $config->{ifaces};
+        my $new_ifaces = {};
+
+        for my $iface_name (keys $old_ifaces->%*) {
+            my $iface = $old_ifaces->{$iface_name};
+
+            if ($existing_pins->{$iface_name}) {
+                # reading the interfaces file adds active interfaces to the
+                # configuration - we do not want to include already pinned
+                # interfaces in the new configuration when writing the new
+                # interface file multiple times, so we skip the interface here
+                # if there already exists a pin for it.
+                next;
+            }
+
+            if ($iface->{type} =~ m/^(eth|OVSPort|alias)$/) {
+                $iface_name = $mapping->name($iface_name);
+            } elsif ($iface->{type} eq 'vlan') {
+                $iface_name = $mapping->name($iface_name);
+                $iface->{'vlan-raw-device'} = $mapping->name($iface->{'vlan-raw-device'});
+            } elsif ($iface->{type} eq 'bond') {
+                $iface->{'bond-primary'} = $mapping->name($iface->{'bond-primary'});
+                $iface->{slaves} = $mapping->list($iface->{slaves});
+            } elsif ($iface->{type} eq 'bridge') {
+                $iface->{bridge_ports} = $mapping->list($iface->{bridge_ports});
+            } elsif ($iface->{type} eq 'OVSBridge') {
+                $iface->{ovs_ports} = $mapping->list($iface->{ovs_ports});
+            } elsif ($iface->{type} eq 'OVSBond') {
+                $iface->{ovs_bonds} = $mapping->list($iface->{ovs_bonds});
+            }
+
+            $new_ifaces->{$iface_name} = $iface;
+        }
+
+        $config->{ifaces} = $new_ifaces;
+        PVE::INotify::write_file('interfaces', $config, 1);
+    };
+
+    PVE::Tools::lock_file("/etc/network/.pve-interfaces.lock", 10, $code);
+}
+
+my sub update_host_fw_config {
+    my ($mapping) = @_;
+
+    my $local_node = PVE::INotify::nodename();
+    print "Updating /etc/pve/nodes/$local_node/host.fw.new\n";
+
+    my $code = sub {
+        my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
+
+        my $temp_fw_file = "/etc/pve/nodes/$local_node/host.fw.new";
+
+        my $host_fw_file = (-e $temp_fw_file) ? $temp_fw_file : undef;
+        my $host_conf = PVE::Firewall::load_hostfw_conf($cluster_conf, $host_fw_file);
+
+        for my $rule ($cluster_conf->{rules}->@*) {
+            next if !$rule->{iface};
+
+            warn "found reference to iface $rule->{iface} in cluster config - not updating."
+                if $mapping->{ $rule->{iface} };
+        }
+
+        for my $rule ($host_conf->{rules}->@*) {
+            next if !$rule->{iface};
+            $rule->{iface} = $mapping->name($rule->{iface});
+        }
+
+        PVE::Firewall::save_hostfw_conf($host_conf, "/etc/pve/nodes/$local_node/host.fw.new");
+    };
+
+    PVE::Firewall::run_locked($code);
+}
+
+my sub parse_link_file {
+    my ($file_name) = @_;
+
+    my $content = PVE::Tools::file_get_contents($file_name);
+    my @lines = split(/\n/, $content);
+
+    my $section;
+    my $data = {};
+
+    for my $line (@lines) {
+        next if $line =~ m/^\s*$/;
+
+        if ($line =~ m/^\[(Match|Link)\]$/) {
+            $section = $1;
+            $data->{$section} = {};
+        } elsif ($line =~ m/^([a-zA-Z]+)=(.+)$/) {
+            die "key-value pair before section" if !$section;
+            $data->{$section}->{$1} = $2;
+        } else {
+            die "unrecognized line";
+        }
+    }
+
+    return $data;
+}
+
+my $LINK_DIRECTORY = "/usr/local/lib/systemd/network/";
+
+sub ensure_link_directory_exists {
+    mkdir '/usr/local/lib/systemd' if !-d '/usr/local/lib/systemd';
+    mkdir $LINK_DIRECTORY if !-d $LINK_DIRECTORY;
+}
+
+my sub get_pinned {
+    my $link_files = {};
+
+    ensure_link_directory_exists();
+
+    PVE::Tools::dir_glob_foreach(
+        $LINK_DIRECTORY,
+        qr/^50-pve-(.+)\.link$/,
+        sub {
+            my $parsed = parse_link_file($LINK_DIRECTORY . $_[0]);
+            $link_files->{ $parsed->{'Match'}->{'MACAddress'} } = $parsed->{'Link'}->{'Name'};
+        },
+    );
+
+    return $link_files;
+}
+
+my $LINK_FILE_TEMPLATE = <<EOF;
+[Match]
+MACAddress=%s
+Type=ether
+
+[Link]
+Name=%s
+EOF
+
+my sub link_file_name {
+    my ($iface_name) = @_;
+    return "50-pve-$iface_name.link";
+}
+
+my sub delete_link_files {
+    my ($pinned) = @_;
+
+    ensure_link_directory_exists();
+
+    for my $iface_name (values %$pinned) {
+        my $link_file = $LINK_DIRECTORY . link_file_name($iface_name);
+
+        if (!unlink $link_file) {
+            return if $! == ENOENT;
+            warn "failed to delete $link_file";
+        }
+    }
+}
+
+my sub generate_link_files {
+    my ($ip_links, $mapping) = @_;
+
+    print "Generating link files\n";
+
+    ensure_link_directory_exists();
+
+    for my $ip_link (values $ip_links->%*) {
+        my $mapped_name = $mapping->name($ip_link->{ifname});
+        my $link_file_content =
+            sprintf($LINK_FILE_TEMPLATE, get_ip_link_mac($ip_link), $mapped_name);
+
+        PVE::Tools::file_set_contents(
+            $LINK_DIRECTORY . link_file_name($mapped_name),
+            $link_file_content,
+        );
+    }
+}
+
+package PVE::CLI::proxmox_network_interface_pinning::InterfaceMapping {
+    use PVE::CLI::proxmox_network_interface_pinning;
+    use PVE::Tools;
+
+    sub generate {
+        my ($class, $ip_links, $pinned, $prefix) = @_;
+
+        my $index = 0;
+        my $mapping = {};
+
+        my %existing_names = map { $_ => 1 } values $pinned->%*;
+
+        for my $ifname (sort keys $ip_links->%*) {
+            my $ip_link = $ip_links->{$ifname};
+            my $generated_name;
+
+            do {
+                $generated_name = $prefix . $index++;
+            } while ($existing_names{$generated_name});
+
+            $mapping->{$ifname} = $generated_name;
+
+            for my $altname ($ip_link->{altnames}->@*) {
+                $mapping->{$altname} = $generated_name;
+            }
+        }
+
+        bless $mapping, $class;
+    }
+
+    sub name {
+        my ($self, $iface_name) = @_;
+
+        if ($iface_name =~ m/^([a-zA-Z0-9_]+)([:\.]\d+)$/) {
+            my $mapped_name = $self->{$1} // $1;
+            my $suffix = $2;
+
+            return "$mapped_name$suffix";
+        }
+
+        return $self->{$iface_name} // $iface_name;
+    }
+
+    sub list {
+        my ($self, $list) = @_;
+
+        my @mapped_list = map { $self->name($_) } PVE::Tools::split_list($list);
+        return join(' ', @mapped_list);
+    }
+}
+
+sub get_ip_link_mac {
+    my ($ip_link) = @_;
+
+    # members of bonds can have a different MAC than the physical interface, so
+    # we need to check if they're enslaved
+    return $ip_link->{link_info}->{info_slave_data}->{perm_hwaddr} // $ip_link->{address};
+}
+
+sub get_ip_links {
+    my $ip_links = PVE::Network::ip_link_details();
+
+    for my $iface_name (keys $ip_links->%*) {
+        delete $ip_links->{$iface_name}
+            if !PVE::Network::ip_link_is_physical($ip_links->{$iface_name});
+    }
+
+    return $ip_links;
+}
+
+sub resolve_pinned {
+    my ($ip_links, $pinned) = @_;
+
+    my %mac_lookup = map { get_ip_link_mac($_) => $_->{ifname} } values $ip_links->%*;
+
+    my $resolved = {};
+
+    for my $mac (keys $pinned->%*) {
+        if (!$mac_lookup{$mac}) {
+            warn "could not resolve $mac to an existing interface";
+            next;
+        }
+
+        $resolved->{ $mac_lookup{$mac} } = $pinned->{$mac};
+    }
+
+    return $resolved;
+}
+
+__PACKAGE__->register_method({
+    name => 'generate',
+    path => 'generate',
+    method => 'POST',
+    description => 'Generates the names of NICs via systemd.link files.',
+    parameters => {
+        additionalProperties => 0,
+        properties => {
+            nic => {
+                description => 'Only pin a specific NIC.',
+                type => 'string',
+                format => 'pve-iface',
+                optional => 1,
+            },
+        },
+    },
+    returns => {
+        type => 'null',
+    },
+    code => sub {
+        my ($params) = @_;
+
+        my $code = sub {
+            my $prefix = 'nic';
+
+            my $ip_links = get_ip_links();
+            my $pinned = get_pinned();
+            my $existing_pins = resolve_pinned($ip_links, $pinned);
+
+            if ($params->{nic}) {
+                die "Could not find link with name $params->{nic}"
+                    if !$ip_links->{ $params->{nic} };
+
+                die "There already exists a pin for NIC $params->{nic}"
+                    if $existing_pins->{ $params->{nic} };
+
+                $ip_links = { $params->{nic} => $ip_links->{ $params->{nic} } };
+            } else {
+                for my $iface_name (keys $existing_pins->%*) {
+                    delete $ip_links->{$iface_name};
+                }
+            }
+
+            my $mapping =
+                PVE::CLI::proxmox_network_interface_pinning::InterfaceMapping->generate(
+                    $ip_links,
+                    $pinned,
+                    $prefix,
+                );
+
+            for my $old_name (sort keys $mapping->%*) {
+                print "Renaming link '$old_name' to '$mapping->{$old_name}'\n";
+            }
+
+            generate_link_files($ip_links, $mapping);
+            print "Succesfully generated .link files\n";
+
+            update_host_fw_config($mapping);
+            update_etc_network_interfaces($mapping, $existing_pins);
+            update_sdn_controllers($mapping);
+
+            print "Successfully updated Proxmox VE configuration files\n";
+            print "Please reboot to apply the changes to your configuration\n";
+        };
+
+        PVE::Tools::lock_file($PVEETH_LOCK, 10, $code);
+        die $@ if $@;
+
+        return;
+    },
+});
+
+our $cmddef = {
+    generate => [__PACKAGE__, 'generate', [], {}],
+};
+
+1;
diff --git a/bin/Makefile b/bin/Makefile
index 51683596f..c36ac3398 100644
--- a/bin/Makefile
+++ b/bin/Makefile
@@ -14,6 +14,7 @@ CLITOOLS = \
 	pvesh \
 	pve7to8 \
 	pve8to9 \
+	proxmox-network-interface-pinning \
 
 
 SCRIPTS =  			\
@@ -70,6 +71,22 @@ pve7to8.1:
 	printf ".SH SYNOPSIS\npve7to8 [--full]\n" >> $@.tmp
 	mv $@.tmp $@
 
+proxmox-network-interface-pinning.1:
+	printf "proxmox-network-interface-pinning" > $@.tmp
+	mv $@.tmp $@
+
+proxmox-network-interface-pinning.api-verified:
+	perl ${PERL_DOC_INC} -T -e "use PVE::CLI::proxmox_network_interface_pinning; PVE::CLI::proxmox_network_interface_pinning->verify_api();"
+	touch 'proxmox-network-interface-pinning.service-api-verified'
+
+proxmox-network-interface-pinning.zsh-completion:
+	perl ${PERL_DOC_INC} -T -e "use PVE::CLI::proxmox_network_interface_pinning; PVE::CLI::proxmox_network_interface_pinning->generate_zsh_completions();" >$@.tmp
+	mv $@.tmp $@
+
+proxmox-network-interface-pinning.bash-completion:
+	perl ${PERL_DOC_INC} -T -e "use PVE::CLI::proxmox_network_interface_pinning; PVE::CLI::proxmox_network_interface_pinning->generate_bash_completions();" >$@.tmp
+	mv $@.tmp $@
+
 pve8to9.1:
 	printf ".TH PVE8TO9 1\n.SH NAME\npve8to9 \- Proxmox VE upgrade checker script for 8.4+ to current 9.x\n" > $@.tmp
 	printf ".SH DESCRIPTION\nThis tool will help you to detect common pitfalls and misconfguration\
diff --git a/bin/proxmox-network-interface-pinning b/bin/proxmox-network-interface-pinning
new file mode 100644
index 000000000..b6922fb46
--- /dev/null
+++ b/bin/proxmox-network-interface-pinning
@@ -0,0 +1,8 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use PVE::CLI::proxmox_network_interface_pinning;
+
+PVE::CLI::proxmox_network_interface_pinning->run_cli_handler();
-- 
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 02/16] services: add pvesdncommit and pvefirewallcommit

[Stefan Hanreich]

Changes to /etc/network/interfaces already get automatically applied
by pvenetcommit. In order to support automatically applying all
configuration files generated by proxmox-network-interface-pinning,
add two additional service that apply the SDN and the firewall
configuration respectively.

If the network configuration gets automatically applied, it makes
sense that the SDN configuration should also get re-applied, since it
relies on the current network configuration for some features (e.g.
SNAT ouput interface, IS-IS interface, ..).

For the firewall, the configuration file that gets automatically
applied is currently only generated by
proxmox-network-interface-pinning, so anyone not using that tool
should see no effect at all.

They are split into their own one-shot services, since pvenetcommit
needs to run before the network configuration gets loaded and applied
by ifupdown2, but pvesdncommit requires the new network configuration
to be already applied in order to work properly. pvefirewallcommit
requires at least pmxcfs to be up and running, since it reads / writes
configuration files there.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250716151815.348161-9-s.hanreich@proxmox.com
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 bin/Makefile                       |  4 +++-
 bin/pvefirewallcommit              | 14 ++++++++++++++
 bin/pvesdncommit                   | 14 ++++++++++++++
 debian/postinst                    |  2 +-
 services/Makefile                  |  4 +++-
 services/pvefirewallcommit.service | 13 +++++++++++++
 services/pvesdncommit.service      | 13 +++++++++++++
 7 files changed, 61 insertions(+), 3 deletions(-)
 create mode 100644 bin/pvefirewallcommit
 create mode 100644 bin/pvesdncommit
 create mode 100644 services/pvefirewallcommit.service
 create mode 100644 services/pvesdncommit.service

diff --git a/bin/Makefile b/bin/Makefile
index c36ac3398..2d5e6f3c5 100644
--- a/bin/Makefile
+++ b/bin/Makefile
@@ -29,7 +29,9 @@ SCRIPTS =  			\
 
 HELPERS =			\
 	pve-startall-delay	\
-	pve-init-ceph-crash
+	pve-init-ceph-crash \
+	pvefirewallcommit   \
+	pvesdncommit
 
 MIGRATIONS =			\
 	pve-lvm-disable-autoactivation
diff --git a/bin/pvefirewallcommit b/bin/pvefirewallcommit
new file mode 100644
index 000000000..ebcf9812d
--- /dev/null
+++ b/bin/pvefirewallcommit
@@ -0,0 +1,14 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use PVE::INotify;
+
+my $local_node = PVE::INotify::nodename();
+my $current_fw_config_file = "/etc/pve/nodes/$local_node/host.fw";
+my $new_fw_config_file = "/etc/pve/nodes/$local_node/host.fw.new";
+
+rename($new_fw_config_file, $current_fw_config_file) if -e $new_fw_config_file;
+
+exit 0;
diff --git a/bin/pvesdncommit b/bin/pvesdncommit
new file mode 100644
index 000000000..2654e17ed
--- /dev/null
+++ b/bin/pvesdncommit
@@ -0,0 +1,14 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use PVE::Network::SDN;
+
+PVE::Network::SDN::commit_config();
+
+PVE::Network::SDN::generate_zone_config();
+PVE::Network::SDN::generate_dhcp_config();
+PVE::Network::SDN::generate_controller_config(1);
+
+exit 0;
diff --git a/debian/postinst b/debian/postinst
index aba399045..dac40c3d8 100755
--- a/debian/postinst
+++ b/debian/postinst
@@ -170,7 +170,7 @@ case "$1" in
     # same as dh_systemd_enable (code copied)
 
     UNITS="pvedaemon.service pveproxy.service spiceproxy.service pvestatd.service pvebanner.service pvescheduler.service pve-daily-update.timer"
-    NO_RESTART_UNITS="pvenetcommit.service pve-guests.service"
+    NO_RESTART_UNITS="pvenetcommit.service pve-guests.service pvesdncommit.service pvefirewallcommit.service"
 
     for unit in ${UNITS} ${NO_RESTART_UNITS}; do
         deb-systemd-helper unmask "$unit" >/dev/null || true
diff --git a/services/Makefile b/services/Makefile
index 8a60fa9bb..b056c7c4d 100644
--- a/services/Makefile
+++ b/services/Makefile
@@ -13,7 +13,9 @@ SERVICES=			\
 	pve-storage.target	\
 	pve-daily-update.service\
 	pve-daily-update.timer	\
-	pvescheduler.service
+	pvescheduler.service    \
+	pvesdncommit.service    \
+	pvefirewallcommit.service
 
 .PHONY: install
 install: $(SERVICES)
diff --git a/services/pvefirewallcommit.service b/services/pvefirewallcommit.service
new file mode 100644
index 000000000..1c9a70e74
--- /dev/null
+++ b/services/pvefirewallcommit.service
@@ -0,0 +1,13 @@
+[Unit]
+Description=Commit Proxmox VE Firewall changes
+DefaultDependencies=no
+Wants=pve-cluster.service
+After=pve-cluster.service
+
+[Service]
+ExecStart=/usr/share/pve-manager/helpers/pvefirewallcommit
+Type=oneshot
+RemainAfterExit=yes
+
+[Install]
+WantedBy=multi-user.target
diff --git a/services/pvesdncommit.service b/services/pvesdncommit.service
new file mode 100644
index 000000000..b8b8c781f
--- /dev/null
+++ b/services/pvesdncommit.service
@@ -0,0 +1,13 @@
+[Unit]
+Description=Commit Proxmox VE SDN changes
+DefaultDependencies=no
+Wants=pve-cluster.service network.target
+After=frr.service network.target pve-cluster.service
+
+[Service]
+ExecStart=/usr/share/pve-manager/helpers/pvesdncommit
+Type=oneshot
+RemainAfterExit=yes
+
+[Install]
+WantedBy=multi-user.target
-- 
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 03/16] use kebab-case spelling for new SDN and firewall config-commit services

[Stefan Hanreich]

From: Thomas Lamprecht <t.lamprecht@proxmox.com>

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 bin/Makefile                                                | 6 +++---
 bin/{pvefirewallcommit => pve-firewall-commit}              | 0
 bin/{pvesdncommit => pve-sdn-commit}                        | 0
 debian/postinst                                             | 2 +-
 services/Makefile                                           | 6 +++---
 ...vefirewallcommit.service => pve-firewall-commit.service} | 2 +-
 services/{pvesdncommit.service => pve-sdn-commit.service}   | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)
 rename bin/{pvefirewallcommit => pve-firewall-commit} (100%)
 rename bin/{pvesdncommit => pve-sdn-commit} (100%)
 rename services/{pvefirewallcommit.service => pve-firewall-commit.service} (77%)
 rename services/{pvesdncommit.service => pve-sdn-commit.service} (81%)

diff --git a/bin/Makefile b/bin/Makefile
index 2d5e6f3c5..2891a663d 100644
--- a/bin/Makefile
+++ b/bin/Makefile
@@ -29,9 +29,9 @@ SCRIPTS =  			\
 
 HELPERS =			\
 	pve-startall-delay	\
-	pve-init-ceph-crash \
-	pvefirewallcommit   \
-	pvesdncommit
+	pve-init-ceph-crash	\
+	pve-firewall-commit	\
+	pve-sdn-commit
 
 MIGRATIONS =			\
 	pve-lvm-disable-autoactivation
diff --git a/bin/pvefirewallcommit b/bin/pve-firewall-commit
similarity index 100%
rename from bin/pvefirewallcommit
rename to bin/pve-firewall-commit
diff --git a/bin/pvesdncommit b/bin/pve-sdn-commit
similarity index 100%
rename from bin/pvesdncommit
rename to bin/pve-sdn-commit
diff --git a/debian/postinst b/debian/postinst
index dac40c3d8..823bedafd 100755
--- a/debian/postinst
+++ b/debian/postinst
@@ -170,7 +170,7 @@ case "$1" in
     # same as dh_systemd_enable (code copied)
 
     UNITS="pvedaemon.service pveproxy.service spiceproxy.service pvestatd.service pvebanner.service pvescheduler.service pve-daily-update.timer"
-    NO_RESTART_UNITS="pvenetcommit.service pve-guests.service pvesdncommit.service pvefirewallcommit.service"
+    NO_RESTART_UNITS="pvenetcommit.service pve-guests.service pve-sdn-commit.service pve-firewall-commit.service"
 
     for unit in ${UNITS} ${NO_RESTART_UNITS}; do
         deb-systemd-helper unmask "$unit" >/dev/null || true
diff --git a/services/Makefile b/services/Makefile
index b056c7c4d..347a6d119 100644
--- a/services/Makefile
+++ b/services/Makefile
@@ -13,9 +13,9 @@ SERVICES=			\
 	pve-storage.target	\
 	pve-daily-update.service\
 	pve-daily-update.timer	\
-	pvescheduler.service    \
-	pvesdncommit.service    \
-	pvefirewallcommit.service
+	pvescheduler.service	\
+	pve-sdn-commit.service	\
+	pve-firewall-commit.service
 
 .PHONY: install
 install: $(SERVICES)
diff --git a/services/pvefirewallcommit.service b/services/pve-firewall-commit.service
similarity index 77%
rename from services/pvefirewallcommit.service
rename to services/pve-firewall-commit.service
index 1c9a70e74..77ea095d7 100644
--- a/services/pvefirewallcommit.service
+++ b/services/pve-firewall-commit.service
@@ -5,7 +5,7 @@ Wants=pve-cluster.service
 After=pve-cluster.service
 
 [Service]
-ExecStart=/usr/share/pve-manager/helpers/pvefirewallcommit
+ExecStart=/usr/share/pve-manager/helpers/pve-firewall-commit
 Type=oneshot
 RemainAfterExit=yes
 
diff --git a/services/pvesdncommit.service b/services/pve-sdn-commit.service
similarity index 81%
rename from services/pvesdncommit.service
rename to services/pve-sdn-commit.service
index b8b8c781f..927d06c54 100644
--- a/services/pvesdncommit.service
+++ b/services/pve-sdn-commit.service
@@ -5,7 +5,7 @@ Wants=pve-cluster.service network.target
 After=frr.service network.target pve-cluster.service
 
 [Service]
-ExecStart=/usr/share/pve-manager/helpers/pvesdncommit
+ExecStart=/usr/share/pve-manager/helpers/pve-sdn-commit
 Type=oneshot
 RemainAfterExit=yes
 
-- 
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 04/16] firewall on-boot commit: report errors if rename fails

[Stefan Hanreich]

From: Thomas Lamprecht <t.lamprecht@proxmox.com>

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 bin/pve-firewall-commit | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/bin/pve-firewall-commit b/bin/pve-firewall-commit
index ebcf9812d..e0d4eb410 100644
--- a/bin/pve-firewall-commit
+++ b/bin/pve-firewall-commit
@@ -9,6 +9,9 @@ my $local_node = PVE::INotify::nodename();
 my $current_fw_config_file = "/etc/pve/nodes/$local_node/host.fw";
 my $new_fw_config_file = "/etc/pve/nodes/$local_node/host.fw.new";
 
-rename($new_fw_config_file, $current_fw_config_file) if -e $new_fw_config_file;
+if (-e $new_fw_config_file) {
+    rename($new_fw_config_file, $current_fw_config_file)
+        or die "failed to commit new local node firewall config '$new_fw_config_file' - $!\n";
+}
 
 exit 0;
-- 
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 05/16] nic pinning: prompt before continuing if connected to TTY

[Stefan Hanreich]

From: Thomas Lamprecht <t.lamprecht@proxmox.com>

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

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index 5dea9126a..fb45a1acd 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -1,7 +1,6 @@
 package PVE::CLI::proxmox_network_interface_pinning;
 
-use strict;
-use warnings;
+use v5.36;
 
 use File::Copy;
 use POSIX qw(:errno_h);
@@ -339,6 +338,18 @@ __PACKAGE__->register_method({
     code => sub {
         my ($params) = @_;
 
+        my $iface = $params->{interface}; # undef means all.
+
+        if (-t STDOUT) {
+            my $target = defined($iface) ? "the interface '$iface'" : 'all interfaces';
+            say "This will generate name pinning configuration for $target - continue (y/N)? ";
+
+            my $answer = <STDIN>;
+            my $continue = defined($answer) && $answer =~ m/^\s*y(?:es)?\s*$/i;
+
+            die "Aborting pinning as requested\n" if !$continue;
+        }
+
         my $code = sub {
             my $prefix = 'nic';
 
-- 
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 06/16] nic pinning: update description for generate command

[Stefan Hanreich]

From: Thomas Lamprecht <t.lamprecht@proxmox.com>

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 PVE/CLI/proxmox_network_interface_pinning.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index fb45a1acd..f9d94369a 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -320,7 +320,8 @@ __PACKAGE__->register_method({
     name => 'generate',
     path => 'generate',
     method => 'POST',
-    description => 'Generates the names of NICs via systemd.link files.',
+    description => 'Generate systemd.link files to pin the names of one or more network'
+        . ' interfaces and update all network-related configuration files.',
     parameters => {
         additionalProperties => 0,
         properties => {
-- 
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 07/16] nic pinning: rename 'nic' parameter to 'interface'

[Stefan Hanreich]

From: Thomas Lamprecht <t.lamprecht@proxmox.com>

Note that we do greedy matching for CLI parameters names and allow
using just a single minus too, so as of now one can stills use shorter
variants like `-i IFACE`.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 PVE/CLI/proxmox_network_interface_pinning.pm | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index f9d94369a..4b7f72404 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -325,10 +325,12 @@ __PACKAGE__->register_method({
     parameters => {
         additionalProperties => 0,
         properties => {
-            nic => {
-                description => 'Only pin a specific NIC.',
+            # TODO: support a target name or prefix once pve-common supports generic physical ifaces
+            interface => {
+                description => 'Only pin a specific interface.',
                 type => 'string',
                 format => 'pve-iface',
+                default => '<all>', # just for the docs.
                 optional => 1,
             },
         },
@@ -352,20 +354,19 @@ __PACKAGE__->register_method({
         }
 
         my $code = sub {
-            my $prefix = 'nic';
+            my $prefix = 'nic'; # TODO: make flexible once pve-common supports that.
 
             my $ip_links = get_ip_links();
             my $pinned = get_pinned();
             my $existing_pins = resolve_pinned($ip_links, $pinned);
 
-            if ($params->{nic}) {
-                die "Could not find link with name $params->{nic}"
-                    if !$ip_links->{ $params->{nic} };
+            if ($iface) {
+                die "Could not find link with name '$iface'\n" if !$ip_links->{$iface};
 
-                die "There already exists a pin for NIC $params->{nic}"
-                    if $existing_pins->{ $params->{nic} };
+                die "There already exists a pin for NIC '$iface' - aborting.\n"
+                    if $existing_pins->{$iface};
 
-                $ip_links = { $params->{nic} => $ip_links->{ $params->{nic} } };
+                $ip_links = { $iface => $ip_links->{$iface} };
             } else {
                 for my $iface_name (keys $existing_pins->%*) {
                     delete $ip_links->{$iface_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 08/16] nic pinning: improve some informational and error output wording/formatting

[Stefan Hanreich]

From: Thomas Lamprecht <t.lamprecht@proxmox.com>

Mainly add a trailing \n to die invocations to avoid getting a ugly
"at line XY" in the output.

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

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index 4b7f72404..ea98ccb5e 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -145,10 +145,10 @@ my sub parse_link_file {
             $section = $1;
             $data->{$section} = {};
         } elsif ($line =~ m/^([a-zA-Z]+)=(.+)$/) {
-            die "key-value pair before section" if !$section;
+            die "key-value pair before section at line: $line\n" if !$section;
             $data->{$section}->{$1} = $2;
         } else {
-            die "unrecognized line";
+            die "unrecognized line: $line\n";
         }
     }
 
@@ -381,18 +381,18 @@ __PACKAGE__->register_method({
                 );
 
             for my $old_name (sort keys $mapping->%*) {
-                print "Renaming link '$old_name' to '$mapping->{$old_name}'\n";
+                print "Name for link '$old_name' will change to '$mapping->{$old_name}'\n";
             }
 
             generate_link_files($ip_links, $mapping);
-            print "Succesfully generated .link files\n";
+            print "Successfully generated .link files in '/usr/local/lib/systemd/network/'\n";
 
             update_host_fw_config($mapping);
             update_etc_network_interfaces($mapping, $existing_pins);
             update_sdn_controllers($mapping);
 
-            print "Successfully updated Proxmox VE configuration files\n";
-            print "Please reboot to apply the changes to your configuration\n";
+            print "Successfully updated Proxmox VE configuration files.\n";
+            print "\nPlease reboot to apply the changes to your configuration\n\n";
         };
 
         PVE::Tools::lock_file($PVEETH_LOCK, 10, $code);
-- 
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 09/16] pve-sdn-commit: fix reloading logic

[Stefan Hanreich]

The service was also missing an ifreload, since the ifupdown2 config
gets regenerated by SDN and needs to be applied before generating the
FRR configuration in order for the FRR config generation to work
properly.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 bin/pve-sdn-commit | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/bin/pve-sdn-commit b/bin/pve-sdn-commit
index 2654e17ed..479056e25 100644
--- a/bin/pve-sdn-commit
+++ b/bin/pve-sdn-commit
@@ -9,6 +9,16 @@ PVE::Network::SDN::commit_config();
 
 PVE::Network::SDN::generate_zone_config();
 PVE::Network::SDN::generate_dhcp_config();
+
+my $err = sub {
+    my $line = shift;
+    if ($line =~ /(warning|error): (\S+):/) {
+        print "$2 : $line \n";
+    }
+};
+
+PVE::Tools::run_command(['ifreload', '-a'], errfunc => $err);
+
 PVE::Network::SDN::generate_controller_config(1);
 
 exit 0;
-- 
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 10/16] proxmox-network-interface-pinning: die on failing to write interfaces

[Stefan Hanreich]

lock_file sets the error variable in perl, but does not die if it
encounters an error in the callback. All other invocations of
lock_file already die, but it was missing for writing the interfaces
s file, which swallowed errors.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250717152841.397830-6-s.hanreich@proxmox.com
---
 PVE/CLI/proxmox_network_interface_pinning.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index ea98ccb5e..b45cc973a 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -95,6 +95,7 @@ my sub update_etc_network_interfaces {
     };
 
     PVE::Tools::lock_file("/etc/network/.pve-interfaces.lock", 10, $code);
+    die $@ if $@;
 }
 
 my sub update_host_fw_config {
-- 
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 11/16] proxmox-network-interface-pinning: fix pinning after reboot

[Stefan Hanreich]

We generate a list of existing pins as a reference throughout the
pinning tool. It works by reading the existing link files and looking
up interfaces with the corresponding MAC address. If pins have already
been applied, this would return a mapping of the pinned name to itself
(nic0 => nic0).

We use this list for filtering what we write to the pending
configuration, in order to avoid re-introducing already pinned names
to the pending configuration. This reflexive entry would cause the
interfaces file generation to filter all pinned network interfaces
after reboot, leading to invalid ifupdown2 configuration files. Fix
this by filtering entries in the existing-pins list who are reflexive.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250717152841.397830-7-s.hanreich@proxmox.com
---
 PVE/CLI/proxmox_network_interface_pinning.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index b45cc973a..9dbf91e73 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -311,7 +311,8 @@ sub resolve_pinned {
             next;
         }
 
-        $resolved->{ $mac_lookup{$mac} } = $pinned->{$mac};
+        $resolved->{ $mac_lookup{$mac} } = $pinned->{$mac}
+            if $mac_lookup{$mac} ne $pinned->{$mac};
     }
 
     return $resolved;
-- 
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 12/16] network-interface-pinning: avoid comparing undefined string

[Stefan Hanreich]

Controllers do not necessarily have a node defined, so check for
definedness before comparing the value to avoid ugly error messages.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250718123313.208460-2-s.hanreich@proxmox.com
---
 PVE/CLI/proxmox_network_interface_pinning.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index 9dbf91e73..42f53575c 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -36,7 +36,7 @@ my sub update_sdn_controllers {
 
         for my $controller (values $controllers->{ids}->%*) {
             next
-                if $local_node ne $controller->{node}
+                if ($controller->{node} && $local_node ne $controller->{node})
                 || $controller->{type} ne 'isis';
 
             $controller->{'isis-ifaces'} = $mapping->list($controller->{'isis-ifaces'});
-- 
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 13/16] {sdn, firewall}-commit: wait for quorum

[Stefan Hanreich]

Since both one-shot services need to wait for quorum, wait for it at
the beginning of the scripts, before proceeding with the actual logic.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250718123313.208460-3-s.hanreich@proxmox.com
---
 bin/pve-firewall-commit              | 10 ++++++++++
 bin/pve-sdn-commit                   | 10 ++++++++++
 services/pve-firewall-commit.service |  2 +-
 services/pve-sdn-commit.service      |  2 +-
 4 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/bin/pve-firewall-commit b/bin/pve-firewall-commit
index e0d4eb410..3d208f67b 100644
--- a/bin/pve-firewall-commit
+++ b/bin/pve-firewall-commit
@@ -3,8 +3,18 @@
 use strict;
 use warnings;
 
+use Time::HiRes qw(usleep);
+
+use PVE::Cluster;
 use PVE::INotify;
 
+for (my $i = 0; !PVE::Cluster::check_cfs_quorum(1); $i++) {
+    print "waiting for pmxcfs mount to appear and get quorate...\n"
+        if $i % 50 == 0;
+
+    usleep(100 * 1000);
+}
+
 my $local_node = PVE::INotify::nodename();
 my $current_fw_config_file = "/etc/pve/nodes/$local_node/host.fw";
 my $new_fw_config_file = "/etc/pve/nodes/$local_node/host.fw.new";
diff --git a/bin/pve-sdn-commit b/bin/pve-sdn-commit
index 479056e25..b174f46d9 100644
--- a/bin/pve-sdn-commit
+++ b/bin/pve-sdn-commit
@@ -3,8 +3,18 @@
 use strict;
 use warnings;
 
+use Time::HiRes qw(usleep);
+
+use PVE::Cluster;
 use PVE::Network::SDN;
 
+for (my $i = 0; !PVE::Cluster::check_cfs_quorum(1); $i++) {
+    print "waiting for pmxcfs mount to appear and get quorate...\n"
+        if $i % 50 == 0;
+
+    usleep(100 * 1000);
+}
+
 PVE::Network::SDN::commit_config();
 
 PVE::Network::SDN::generate_zone_config();
diff --git a/services/pve-firewall-commit.service b/services/pve-firewall-commit.service
index 77ea095d7..454ef6c2e 100644
--- a/services/pve-firewall-commit.service
+++ b/services/pve-firewall-commit.service
@@ -2,7 +2,7 @@
 Description=Commit Proxmox VE Firewall changes
 DefaultDependencies=no
 Wants=pve-cluster.service
-After=pve-cluster.service
+After=corosync.service
 
 [Service]
 ExecStart=/usr/share/pve-manager/helpers/pve-firewall-commit
diff --git a/services/pve-sdn-commit.service b/services/pve-sdn-commit.service
index 927d06c54..ff723725d 100644
--- a/services/pve-sdn-commit.service
+++ b/services/pve-sdn-commit.service
@@ -2,7 +2,7 @@
 Description=Commit Proxmox VE SDN changes
 DefaultDependencies=no
 Wants=pve-cluster.service network.target
-After=frr.service network.target pve-cluster.service
+After=frr.service network.target corosync.service
 
 [Service]
 ExecStart=/usr/share/pve-manager/helpers/pve-sdn-commit
-- 
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 14/16] sdn-commit: only reload ifupdown if sdn configuration changed

[Stefan Hanreich]

Check for any changes between the running config and the currently
applied config and guard against executing pve-sdn-commit if the
configuration is unchanged.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Link: https://lore.proxmox.com/20250718123313.208460-4-s.hanreich@proxmox.com
---
 bin/pve-sdn-commit | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/bin/pve-sdn-commit b/bin/pve-sdn-commit
index b174f46d9..cf3fd7480 100644
--- a/bin/pve-sdn-commit
+++ b/bin/pve-sdn-commit
@@ -7,6 +7,11 @@ use Time::HiRes qw(usleep);
 
 use PVE::Cluster;
 use PVE::Network::SDN;
+use PVE::Network::SDN::Zones;
+use PVE::Network::SDN::Vnets;
+use PVE::Network::SDN::Subnets;
+use PVE::Network::SDN::Controllers;
+use PVE::Tools;
 
 for (my $i = 0; !PVE::Cluster::check_cfs_quorum(1); $i++) {
     print "waiting for pmxcfs mount to appear and get quorate...\n"
@@ -15,6 +20,44 @@ for (my $i = 0; !PVE::Cluster::check_cfs_quorum(1); $i++) {
     usleep(100 * 1000);
 }
 
+sub has_pending_changes {
+    my ($pending_config) = @_;
+
+    for my $entity (values $pending_config->{ids}->%*) {
+        return 1 if $entity->{state};
+    }
+
+    return 0;
+}
+
+sub sdn_changed {
+    my $running_config = PVE::Network::SDN::running_config();
+
+    my $configs = {
+        zones => PVE::Network::SDN::Zones::config(),
+        vnets => PVE::Network::SDN::Vnets::config(),
+        subnets => PVE::Network::SDN::Subnets::config(),
+        controllers => PVE::Network::SDN::Controllers::config(),
+    };
+
+    for my $type (keys $configs->%*) {
+        my $pending_config = PVE::Network::SDN::pending_config(
+            $running_config,
+            $configs->{$type},
+            $type,
+        );
+
+        return 1 if has_pending_changes($pending_config);
+    }
+
+    return 0;
+}
+
+if (!sdn_changed()) {
+    print "No changes to SDN configuration detected, skipping reload\n";
+    exit 0;
+}
+
 PVE::Network::SDN::commit_config();
 
 PVE::Network::SDN::generate_zone_config();
-- 
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 15/16] network-interface-pinning: fix subsequent invocations

[Stefan Hanreich]

The fix introduced in 5b5db0e67 fixed the scenario where pins were
already applied, but broke the case where they weren't yet applied,
since now not-yet-applied pins would get overwritten again on
subsequent invocations of the pinning tool.

Move the check for the same name to the update_etc_network_interfaces
call, where it is actually required - instead of filtering already in
the resolve_pinned function, which is also used in the generate body
to filter eligible links for pinning.

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

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index 42f53575c..30815d3a1 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -62,7 +62,7 @@ my sub update_etc_network_interfaces {
         for my $iface_name (keys $old_ifaces->%*) {
             my $iface = $old_ifaces->{$iface_name};
 
-            if ($existing_pins->{$iface_name}) {
+            if ($existing_pins->{$iface_name} && $existing_pins->{$iface_name} ne $iface_name) {
                 # reading the interfaces file adds active interfaces to the
                 # configuration - we do not want to include already pinned
                 # interfaces in the new configuration when writing the new
@@ -311,8 +311,7 @@ sub resolve_pinned {
             next;
         }
 
-        $resolved->{ $mac_lookup{$mac} } = $pinned->{$mac}
-            if $mac_lookup{$mac} ne $pinned->{$mac};
+        $resolved->{ $mac_lookup{$mac} } = $pinned->{$mac};
     }
 
     return $resolved;
-- 
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 16/16] network-interface-pinning: early exit if nothing to do

[Stefan Hanreich]

We would regenerate all configuration files, even if no interface
would be mapped. While this shouldn't cause an issue, it's
unnecessary, has potential for creating bugs and leads to confusing
output for the users - so just abort in this case instead.

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

diff --git a/PVE/CLI/proxmox_network_interface_pinning.pm b/PVE/CLI/proxmox_network_interface_pinning.pm
index 30815d3a1..b37bd28e0 100644
--- a/PVE/CLI/proxmox_network_interface_pinning.pm
+++ b/PVE/CLI/proxmox_network_interface_pinning.pm
@@ -381,6 +381,11 @@ __PACKAGE__->register_method({
                     $prefix,
                 );
 
+            if (!$mapping->%*) {
+                print "Nothing to do, aborting.\n";
+                exit 0;
+            }
+
             for my $old_name (sort keys $mapping->%*) {
                 print "Name for link '$old_name' will change to '$mapping->{$old_name}'\n";
             }
-- 
2.39.5


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