[pve-devel] [RFC common/docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v3 0/9] NIC renaming mitigations

[pve-devel] [RFC common/docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v3 0/9] NIC renaming mitigations

[Stefan Hanreich]

This patch series contains the following features:
* transparent altname support for {pve, proxmox}-firewall and pve-network
* proxmox-network-interface-pinning tool for pinning NIC names

Both are improvements aimed at mitigating the fallout caused from changing
network interface names.

For more information on the pinning tool, see the respective commit that
explains the usage - a short tl;dr:

* Pin network interfaces via proxmox-network-interface-pinning generate
* reboot to apply the changes to the network configuration

I also introduce two new one-shot services, that are executed on boot, for
applying pending network configuration changes.

Changes from RFC v2:
* add documentation patch

Changes from RFC:
* renamed pin -> generate
* removed unpinning feature (will be implemented in a follow-up)
* configuration now gets generated into pending files, instead of directly
rewriting configuration
* created SDN and FW pendants to pvenetcommit for auto-comitting changes on
reboot

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(-)


proxmox-ve-rs:

Stefan Hanreich (1):
  config: ip link struct

 proxmox-ve-config/src/host/mod.rs     |  1 +
 proxmox-ve-config/src/host/network.rs | 35 +++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)
 create mode 100644 proxmox-ve-config/src/host/network.rs


proxmox-firewall:

Stefan Hanreich (1):
  firewall: add altname support for firewall rules

 proxmox-firewall/src/config.rs              | 29 +++++++++++++++++++++
 proxmox-firewall/src/rule.rs                |  6 ++++-
 proxmox-firewall/tests/integration_tests.rs |  7 +++++
 3 files changed, 41 insertions(+), 1 deletion(-)


pve-firewall:

Stefan Hanreich (1):
  firewall: add altname support

 src/PVE/Firewall.pm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)


pve-network:

Stefan Hanreich (1):
  controllers: isis: add altname support

 src/PVE/Network/SDN/Controllers/IsisPlugin.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)


pve-manager:

Stefan Hanreich (2):
  cli: add proxmox-network-interface-pinning tool
  services: add pvesdncommit and pvefirewallcommit

 PVE/CLI/Makefile                             |   1 +
 PVE/CLI/proxmox_network_interface_pinning.pm | 396 +++++++++++++++++++
 bin/Makefile                                 |  21 +-
 bin/proxmox-network-interface-pinning        |   8 +
 bin/pvefirewallcommit                        |  14 +
 bin/pvesdncommit                             |  14 +
 debian/postinst                              |   2 +-
 services/Makefile                            |   4 +-
 services/pvefirewallcommit.service           |  13 +
 services/pvesdncommit.service                |  13 +
 10 files changed, 483 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/pvefirewallcommit
 create mode 100644 bin/pvesdncommit
 create mode 100644 services/pvefirewallcommit.service
 create mode 100644 services/pvesdncommit.service


pve-docs:

Stefan Hanreich (1):
  network: add documentation for proxmox-network-interface-pinning

 pve-network.adoc | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)


Summary over all repositories:
  19 files changed, 657 insertions(+), 8 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 v3 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
proxmox-network-interface-pinning tool which can be used for pinning
interface names.

Signed-off-by: Stefan Hanreich <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 v3 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>
---
 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 proxmox-ve-rs v3 1/1] config: ip link struct

[Stefan Hanreich]

Add a bare-bones struct for parsing the output of `ip -details -json
link show`. Currently we only require the name of the interfaces as
well as its altnames for transparently supporting altnames in the
firewall.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---

Notes:
    There is probably a better place than proxmox-ve-config for this. I've
    put it here for now, since we have some network-related stuff in this
    repository already. Mid to short-term it would be better to extract
    all this stuff into either a separate crate or proxmox-sys. For this
    POC its fine.

 proxmox-ve-config/src/host/mod.rs     |  1 +
 proxmox-ve-config/src/host/network.rs | 35 +++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)
 create mode 100644 proxmox-ve-config/src/host/network.rs

diff --git a/proxmox-ve-config/src/host/mod.rs b/proxmox-ve-config/src/host/mod.rs
index b4ab6a6..a9da919 100644
--- a/proxmox-ve-config/src/host/mod.rs
+++ b/proxmox-ve-config/src/host/mod.rs
@@ -1,2 +1,3 @@
+pub mod network;
 pub mod types;
 pub mod utils;
diff --git a/proxmox-ve-config/src/host/network.rs b/proxmox-ve-config/src/host/network.rs
new file mode 100644
index 0000000..09f4fb1
--- /dev/null
+++ b/proxmox-ve-config/src/host/network.rs
@@ -0,0 +1,35 @@
+use std::collections::HashMap;
+
+#[derive(Debug, Clone, serde::Deserialize)]
+pub struct IpLink {
+    ifname: String,
+    #[serde(default)]
+    altnames: Vec<String>,
+}
+
+#[derive(Debug, Clone, serde::Deserialize)]
+pub struct InterfaceMapping {
+    mapping: HashMap<String, String>,
+}
+
+impl std::ops::Deref for InterfaceMapping {
+    type Target = HashMap<String, String>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.mapping
+    }
+}
+
+impl FromIterator<IpLink> for InterfaceMapping {
+    fn from_iter<T: IntoIterator<Item = IpLink>>(iter: T) -> Self {
+        let mut mapping = HashMap::new();
+
+        for iface in iter.into_iter() {
+            for altname in iface.altnames {
+                mapping.insert(altname, iface.ifname.clone());
+            }
+        }
+
+        Self { 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 proxmox-firewall v3 1/1] firewall: add altname support for firewall rules

[Stefan Hanreich]

This works by reading all the currently configured altnames and then
replacing any occurences of altnames when creating the firewall rules.
We handle it this way because nftables has no support for matching on
the altnames of interfaces.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-firewall/src/config.rs              | 29 +++++++++++++++++++++
 proxmox-firewall/src/rule.rs                |  6 ++++-
 proxmox-firewall/tests/integration_tests.rs |  7 +++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/proxmox-firewall/src/config.rs b/proxmox-firewall/src/config.rs
index ec9849e..f07fb1e 100644
--- a/proxmox-firewall/src/config.rs
+++ b/proxmox-firewall/src/config.rs
@@ -13,6 +13,8 @@ use proxmox_ve_config::firewall::types::alias::{Alias, AliasName, AliasScope};
 
 use proxmox_ve_config::guest::types::Vmid;
 use proxmox_ve_config::guest::{GuestEntry, GuestMap};
+use proxmox_ve_config::host::network::InterfaceMapping;
+use proxmox_ve_config::host::network::IpLink;
 use proxmox_ve_config::host::types::BridgeName;
 
 use proxmox_nftables::command::{CommandOutput, Commands, List, ListOutput};
@@ -40,6 +42,7 @@ pub trait FirewallConfigLoader {
         &self,
         bridge_name: &BridgeName,
     ) -> Result<Option<Box<dyn io::BufRead>>, Error>;
+    fn interface_mapping(&self) -> Result<InterfaceMapping, Error>;
 }
 
 #[derive(Default)]
@@ -221,6 +224,26 @@ impl FirewallConfigLoader for PveFirewallConfigLoader {
 
         Ok(None)
     }
+
+    fn interface_mapping(&self) -> Result<InterfaceMapping, Error> {
+        let output = std::process::Command::new("ip")
+            .arg("-details")
+            .arg("-json")
+            .arg("link")
+            .arg("show")
+            .stdout(std::process::Stdio::piped())
+            .output()
+            .with_context(|| "could not obtain ip link output")?;
+
+        if !output.status.success() {
+            bail!("ip link returned non-zero exit code")
+        }
+
+        Ok(serde_json::from_slice::<Vec<IpLink>>(&output.stdout)
+            .with_context(|| "could not deserialize ip link output")?
+            .into_iter()
+            .collect())
+    }
 }
 
 pub trait NftConfigLoader {
@@ -255,6 +278,7 @@ pub struct FirewallConfig {
     nft_config: BTreeMap<String, ListChain>,
     sdn_config: Option<SdnConfig>,
     ipam_config: Option<Ipam>,
+    interface_mapping: InterfaceMapping,
 }
 
 impl FirewallConfig {
@@ -380,6 +404,7 @@ impl FirewallConfig {
             sdn_config: Self::parse_sdn(firewall_loader)?,
             ipam_config: Self::parse_ipam(firewall_loader)?,
             nft_config: Self::parse_nft(nft_loader)?,
+            interface_mapping: firewall_loader.interface_mapping()?,
         })
     }
 
@@ -415,6 +440,10 @@ impl FirewallConfig {
         self.cluster().is_enabled() && self.host().nftables()
     }
 
+    pub fn interface_mapping(&self, iface_name: &str) -> Option<&str> {
+        self.interface_mapping.get(iface_name).map(|x| x.as_str())
+    }
+
     pub fn alias(&self, name: &AliasName, vmid: Option<Vmid>) -> Option<&Alias> {
         log::trace!("getting alias {name:?}");
 
diff --git a/proxmox-firewall/src/rule.rs b/proxmox-firewall/src/rule.rs
index 14ee544..c4975a9 100644
--- a/proxmox-firewall/src/rule.rs
+++ b/proxmox-firewall/src/rule.rs
@@ -135,7 +135,11 @@ impl NftRuleEnv<'_> {
 
                 rule_iface.to_string()
             }
-            None => rule_iface.to_string(),
+            None => self
+                .firewall_config
+                .interface_mapping(rule_iface)
+                .map(|iface_name| iface_name.to_string())
+                .unwrap_or_else(|| rule_iface.to_string()),
         }
     }
 
diff --git a/proxmox-firewall/tests/integration_tests.rs b/proxmox-firewall/tests/integration_tests.rs
index 1c014ad..69f9cc2 100644
--- a/proxmox-firewall/tests/integration_tests.rs
+++ b/proxmox-firewall/tests/integration_tests.rs
@@ -1,4 +1,5 @@
 use anyhow::{Context, Error};
+use proxmox_ve_config::host::network::InterfaceMapping;
 use std::collections::HashMap;
 
 use proxmox_firewall::config::{FirewallConfig, FirewallConfigLoader, NftConfigLoader};
@@ -91,6 +92,12 @@ impl FirewallConfigLoader for MockFirewallConfigLoader {
     ) -> Result<Option<Box<dyn std::io::BufRead>>, Error> {
         Ok(None)
     }
+
+    fn interface_mapping(
+        &self,
+    ) -> Result<proxmox_ve_config::host::network::InterfaceMapping, Error> {
+        Ok(InterfaceMapping::from_iter(vec![]))
+    }
 }
 
 struct MockNftConfigLoader {}
-- 
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-firewall v3 1/1] firewall: add altname support

[Stefan Hanreich]

Add support for altnames by transparently mapping them with the
information from 'ip link' when generating the ruleset. The firewall
will now replace any altname in the ruleset with the actual, physical,
name from the interface. We handle it this way, because iptables
cannot match on the altnames on interfaces, only the 'real' name.

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

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 173ce98..e3d21f6 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -2861,6 +2861,8 @@ sub enable_host_firewall {
     my $rules = $hostfw_conf->{rules};
     my $cluster_rules = $cluster_conf->{rules};
 
+    my $interface_mapping = PVE::Network::altname_mapping();
+
     # corosync preparation
     my $corosync_rule = "-p udp --dport 5404:5405";
     my $corosync_local_addresses = {};
@@ -2908,7 +2910,7 @@ sub enable_host_firewall {
         next if !$rule->{enable} || $rule->{errors};
         next if $rule->{ipversion} && ($rule->{ipversion} != $ipversion);
 
-        $rule->{iface_in} = $rule->{iface} if $rule->{iface};
+        $rule->{iface_in} = ($interface_mapping->{$rule->{iface}} // $rule->{iface}) if $rule->{iface};
 
         eval {
             $rule->{logmsg} = "$rule->{action}: ";
@@ -2994,7 +2996,8 @@ sub enable_host_firewall {
         next if !$rule->{enable} || $rule->{errors};
         next if $rule->{ipversion} && ($rule->{ipversion} != $ipversion);
 
-        $rule->{iface_out} = $rule->{iface} if $rule->{iface};
+        $rule->{iface_out} = ($interface_mapping->{$rule->{iface}} // $rule->{iface}) if $rule->{iface};
+
         eval {
             $rule->{logmsg} = "$rule->{action}: ";
             if ($rule->{type} eq 'group') {
-- 
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-network v3 1/1] controllers: isis: add altname support

[Stefan Hanreich]

Since this only has an effect on applying the configuration, users
will still need to reapply the configuration when an interface changes
names / altnames. In order to add full altname support for IS-IS,
altname support would need to be implemented in FRR.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---

Notes:
    Gabriel is currently working on altname support in FRR [1]
    
    [1] https://github.com/FRRouting/frr/pull/19156

 src/PVE/Network/SDN/Controllers/IsisPlugin.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Network/SDN/Controllers/IsisPlugin.pm b/src/PVE/Network/SDN/Controllers/IsisPlugin.pm
index 6e3574d..0ef9fb9 100644
--- a/src/PVE/Network/SDN/Controllers/IsisPlugin.pm
+++ b/src/PVE/Network/SDN/Controllers/IsisPlugin.pm
@@ -80,9 +80,12 @@ sub generate_controller_config {
 
     my @iface_config = ("ip router isis $isis_domain");
 
+    my $altnames = PVE::Network::altname_mapping();
+
     my @ifaces = PVE::Tools::split_list($isis_ifaces);
     for my $iface (sort @ifaces) {
-        push(@{ $config->{frr_interfaces}->{$iface} }, @iface_config);
+        my $iface_name = $altnames->{$iface} // $iface;
+        push(@{ $config->{frr_interfaces}->{$iface_name} }, @iface_config);
     }
 
     return $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 v3 1/2] 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>
---

Notes:
    The Makefile changes might warrant a closer look by someone who is
    more familiar with the way we're building our CLI tools. I had to do
    some overrides in order to get the name with hyphens work properly
    with the API verificiation and building the completions.

 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 3931804b1..fcace77bd 100644
--- a/bin/Makefile
+++ b/bin/Makefile
@@ -14,6 +14,7 @@ CLITOOLS = \
 	pvesh \
 	pve7to8 \
 	pve8to9 \
+	proxmox-network-interface-pinning \
 
 
 SCRIPTS =  			\
@@ -67,6 +68,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 v3 2/2] 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>
---

Notes:
    We could alternatively only enable those services (or at least, the FW
    one) in the pinning tool and disable them once they ran successfully
    if we want to refrain from making this the default behavior for the
    SDN configuration.

 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 fcace77bd..c59f9ec0c 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
 
 SERVICE_MANS = $(addsuffix .8, $(SERVICES))
 
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-docs v3 1/1] network: add documentation for proxmox-network-interface-pinning

[Stefan Hanreich]

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 pve-network.adoc | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/pve-network.adoc b/pve-network.adoc
index 55c796b..48f3d5b 100644
--- a/pve-network.adoc
+++ b/pve-network.adoc
@@ -158,6 +158,48 @@ command line. You need to reboot for the changes to take effect.
 Overriding network device names
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+Using the proxmox-network-interface-pinning tool
+++++++++++++++++++++++++++++++++++++++++++++++++
+
+Proxmox VE provides a tool for automatically generating .link files for
+overriding the name of network devices. It also automatically replaces the
+occurences of the old interface name in the following files:
+
+* `/etc/network/interfaces`
+* `/etc/pve/nodes/<nodename>/host.fw`
+* `/etc/pve/sdn/controllers.cfg`
+
+NOTE: Since the generated mapping is local to the node it is generated on,
+interface names contained in the Firewall Datacenter configuration
+(`/etc/pve/firewall/cluster.fw`) are **not** automatically updated.
+
+The generated link files are stored in `/usr/local/lib/systemd/network`.
+
+It is currently only possible to generate names with the prefix `nic`, since
+otherwise the interfaces would not get recognized as a physical interface. When
+auto-generating new interface names, the tool will use `nic0`, `nic1`, ...
+
+The following command will generate a .link file for all physical network
+interfaces that do not yet have a .link file and update selected Proxmox VE
+configuration files (see above):
+
+----
+proxmox-network-interface-pinning generate
+----
+
+It is also possible to pin only a specific interface:
+
+----
+proxmox-network-interface-pinning generate --nic enp1s0
+----
+
+In order to apply the changes made by `proxmox-network-interface-pinning` to the
+network configuration, the node needs to be rebooted.
+
+
+Manual method
++++++++++++++
+
 You can manually assign a name to a particular network device using a custom
 https://manpages.debian.org/stable/udev/systemd.link.5.en.html[systemd.link
 file]. This overrides the name that would be assigned according to the latest
-- 
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-series: [RFC common/docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v3 0/9] NIC renaming mitigations

[Thomas Lamprecht]

Am 16.07.25 um 18:39 schrieb Stefan Hanreich:
> This patch series contains the following features:
> * transparent altname support for {pve, proxmox}-firewall and pve-network
> * proxmox-network-interface-pinning tool for pinning NIC names
> 
> Both are improvements aimed at mitigating the fallout caused from changing
> network interface names.
> 
> For more information on the pinning tool, see the respective commit that
> explains the usage - a short tl;dr:
> 
> * Pin network interfaces via proxmox-network-interface-pinning generate
> * reboot to apply the changes to the network configuration
> 
> I also introduce two new one-shot services, that are executed on boot, for
> applying pending network configuration changes.

applied series, thanks!

FWIW, I made some follow-up commits in pve-manager, would be great if you
can take another look to verify I did not mess anything up.


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