[pbs-devel] [PATCH proxmox-network-interface-pinning v4 1/1] initial commit

Stefan Hanreich s.hanreich at proxmox.com
Mon Aug 4 16:46:13 CEST 2025


On 8/4/25 3:47 PM, Wolfgang Bumiller wrote:
> On Thu, Jul 31, 2025 at 04:08:53PM +0200, Stefan Hanreich wrote:
>> Introduce proxmox-network-interface-pinning, which is a
>> reimplementation of the tool from pve-manager. It should function
>> identically to the PVE version, except for virtual function support.
>> It also uses the ifupdown2 configuration parser from Rust, instead of
>> the perl implementation, which might have some subtle differences in
>> their handling of ifupdown2 configuration files.
>>
>> In order to support hosts that have both Proxmox VE and Proxmox Backup
>> Server installed, this tool tries to detect the existence of the
>> Proxmox VE tool and executes the Proxmox VE tool instead, if it is
>> present on the host.
>>
>> Signed-off-by: Stefan Hanreich <s.hanreich at proxmox.com>
>> Tested-by: Christian Ebner <c.ebner at proxmox.com>
>> ---
>>  .cargo/config.toml   |   5 +
>>  .gitignore           |   8 +
>>  Cargo.toml           |  24 ++
>>  Makefile             |  83 ++++++
>>  debian/changelog     |   5 +
>>  debian/control       |  36 +++
>>  debian/copyright     |  17 ++
>>  debian/debcargo.toml |   8 +
>>  debian/rules         |  31 ++
>>  src/main.rs          | 667 +++++++++++++++++++++++++++++++++++++++++++
>>  10 files changed, 884 insertions(+)
>>  create mode 100644 .cargo/config.toml
>>  create mode 100644 .gitignore
>>  create mode 100644 Cargo.toml
>>  create mode 100644 Makefile
>>  create mode 100644 debian/changelog
>>  create mode 100644 debian/control
>>  create mode 100644 debian/copyright
>>  create mode 100644 debian/debcargo.toml
>>  create mode 100755 debian/rules
>>  create mode 100644 src/main.rs
>>
>> diff --git a/.cargo/config.toml b/.cargo/config.toml
>> new file mode 100644
>> index 0000000..3b5b6e4
>> --- /dev/null
>> +++ b/.cargo/config.toml
>> @@ -0,0 +1,5 @@
>> +[source]
>> +[source.debian-packages]
>> +directory = "/usr/share/cargo/registry"
>> +[source.crates-io]
>> +replace-with = "debian-packages"
>> diff --git a/.gitignore b/.gitignore
>> new file mode 100644
>> index 0000000..26ba170
>> --- /dev/null
>> +++ b/.gitignore
>> @@ -0,0 +1,8 @@
>> +/target
>> +/proxmox-network-interface-pinning-[0-9]*/
>> +*.build
>> +*.buildinfo
>> +*.changes
>> +*.deb
>> +*.dsc
>> +*.tar*
>> diff --git a/Cargo.toml b/Cargo.toml
>> new file mode 100644
>> index 0000000..ded4d7a
>> --- /dev/null
>> +++ b/Cargo.toml
>> @@ -0,0 +1,24 @@
>> +[package]
>> +name = "proxmox-network-interface-pinning"
>> +version = "0.1.0"
>> +authors = ["Stefan Hanreich <s.hanreich at proxmox.com>"]
>> +edition = "2024"
>> +license = "AGPL-3"
>> +description = "Tool for pinning the name of network interfaces."
>> +
>> +exclude = ["debian"]
>> +
>> +[dependencies]
>> +anyhow = "1.0.95"
>> +nix = "0.29"
>> +serde = "1.0.217"
>> +serde_json = "1.0.139"
>> +walkdir = "2.5.0"
>> +
>> +proxmox-async = "0.5.0"
>> +proxmox-log = "1.0.0"
>> +proxmox-network-api = { version = "1.0.0", features = [ "impl" ] }
>> +proxmox-network-types = "0.1.1"
>> +proxmox-product-config = "1"
>> +proxmox-router = "3.2.2"
>> +proxmox-schema = { version = "4.1.1", features = [ "api-macro" ] }
>> diff --git a/Makefile b/Makefile
>> new file mode 100644
>> index 0000000..7477cd4
>> --- /dev/null
>> +++ b/Makefile
>> @@ -0,0 +1,83 @@
>> +include /usr/share/dpkg/default.mk
>> +
>> +PACKAGE=proxmox-network-interface-pinning
>> +CRATENAME=proxmox-network-interface-pinning
>> +
>> +BUILDDIR ?= $(PACKAGE)-$(DEB_VERSION_UPSTREAM)
>> +ORIG_SRC_TAR=$(PACKAGE)_$(DEB_VERSION_UPSTREAM).orig.tar.gz
>> +
>> +DEB=$(PACKAGE)_$(DEB_VERSION)_$(DEB_HOST_ARCH).deb
>> +DBG_DEB=$(PACKAGE)-dbgsym_$(DEB_VERSION)_$(DEB_HOST_ARCH).deb
>> +DSC=$(PACKAGE)_$(DEB_VERSION).dsc
>> +
>> +CARGO ?= cargo
>> +ifeq ($(BUILD_MODE), release)
>> +CARGO_BUILD_ARGS += --release
>> +COMPILEDIR := target/release
>> +else
>> +COMPILEDIR := target/debug
>> +endif
>> +
>> +INSTALLDIR = /usr/bin
>> +PROXMOX_NETWORK_INTERFACE_PINNING_BIN := $(addprefix $(COMPILEDIR)/,proxmox-network-interface-pinning)
>> +
>> +all:
>> +
>> +install: $(PROXMOX_NETWORK_INTERFACE_PINNING_BIN)
>> +	install -dm755 $(DESTDIR)$(INSTALLDIR)
>> +	install -m755 $(PROXMOX_NETWORK_INTERFACE_PINNING_BIN) $(DESTDIR)$(INSTALLDIR)/
>> +
>> +$(PROXMOX_NETWORK_INTERFACE_PINNING_BIN): .do-cargo-build
>> +.do-cargo-build:
>> +	$(CARGO) build $(CARGO_BUILD_ARGS)
>> +	touch .do-cargo-build
>> +
>> +
>> +.PHONY: cargo-build
>> +cargo-build: .do-cargo-build
>> +
>> +$(BUILDDIR):
>> +	rm -rf $@ $@.tmp
>> +	mkdir $@.tmp
>> +	cp -a debian/ src/ Makefile Cargo.toml $@.tmp
>> +	mv $@.tmp $@
>> +
>> +
>> +$(ORIG_SRC_TAR): $(BUILDDIR)
>> +	tar czf $(ORIG_SRC_TAR) --exclude="$(BUILDDIR)/debian" $(BUILDDIR)
>> +
>> +.PHONY: deb
>> +deb: $(DEB)
>> +$(DEB) $(DBG_DEB) &: $(BUILDDIR)
>> +	cd $(BUILDDIR); dpkg-buildpackage -b -uc -us
>> +	lintian $(DEB)
>> +	@echo $(DEB)
>> +
>> +.PHONY: dsc
>> +dsc:
>> +	rm -rf $(DSC) $(BUILDDIR)
>> +	$(MAKE) $(DSC)
>> +	lintian $(DSC)
>> +
>> +$(DSC): $(BUILDDIR) $(ORIG_SRC_TAR)
>> +	cd $(BUILDDIR); dpkg-buildpackage -S -us -uc -d
>> +
>> +sbuild: $(DSC)
>> +	sbuild $(DSC)
>> +
>> +.PHONY: upload
>> +upload: UPLOAD_DIST ?= $(DEB_DISTRIBUTION)
>> +upload: $(DEB) $(DBG_DEB)
>> +	tar cf - $(DEB) $(DBG_DEB) |ssh -X repoman at repo.proxmox.com -- upload --product pbs --dist $(UPLOAD_DIST) --arch $(DEB_HOST_ARCH)
>> +
>> +.PHONY: clean distclean
>> +distclean: clean
>> +clean:
>> +	$(CARGO) clean
>> +	rm -rf $(PACKAGE)-[0-9]*/ build/
>> +	rm -f *.deb *.changes *.dsc *.tar.* *.buildinfo *.build .do-cargo-build
>> +
>> +.PHONY: dinstall
>> +dinstall: deb
>> +	dpkg -i $(DEB)
>> +
>> diff --git a/debian/changelog b/debian/changelog
>> new file mode 100644
>> index 0000000..56422e9
>> --- /dev/null
>> +++ b/debian/changelog
>> @@ -0,0 +1,5 @@
>> +proxmox-network-interface-pinning (0.1.0) trixie; urgency=medium
>> +
>> +  * Initial release.
>> +
>> + -- Proxmox Support Team <support at proxmox.com>  Tue, 29 Jul 2025 14:39:57 +0200
>> diff --git a/debian/control b/debian/control
>> new file mode 100644
>> index 0000000..9c025b6
>> --- /dev/null
>> +++ b/debian/control
>> @@ -0,0 +1,36 @@
>> +Source: proxmox-network-interface-pinning
>> +Section: admin
>> +Priority: optional
>> +Build-Depends: debhelper-compat (= 13),
>> +               dh-sequence-cargo,
>> +               cargo:native,
>> +               rustc:native,
>> +               libstd-rust-dev,
>> +               librust-anyhow-1+default-dev (>= 1.0.95-~~),
>> +               librust-nix-0.29+default-dev,
>> +               librust-proxmox-async-0.5+default-dev,
>> +               librust-proxmox-log-1+default-dev,
>> +               librust-proxmox-network-api-1+default-dev,
>> +               librust-proxmox-network-api-1+impl-dev,
>> +               librust-proxmox-network-types-0.1+default-dev,
>> +               librust-proxmox-product-config-1+default-dev,
>> +               librust-proxmox-router-3+default-dev (>= 3.2.2-~~),
>> +               librust-proxmox-schema-4+api-macro-dev (>= 4.1.1-~~),
>> +               librust-proxmox-schema-4+default-dev (>= 4.1.1-~~),
>> +               librust-serde-1+default-dev (>= 1.0.217-~~),
>> +               librust-serde-json-1+default-dev (>= 1.0.139-~~),
>> +               librust-walkdir-2+default-dev (>= 2.5.0-~~)
>> +Maintainer: Proxmox Support Team <support at proxmox.com>
>> +Standards-Version: 4.7.0
>> +Vcs-Git: 
>> +Vcs-Browser: 
>> +Rules-Requires-Root: no
>> +
>> +Package: proxmox-network-interface-pinning
>> +Architecture: any
>> +Multi-Arch: allowed
>> +Depends: ${misc:Depends}, ${shlibs:Depends},
>> +Description: Pinning the name of network interfaces
>> + This package contains the following binaries built from the Rust crate
>> + "proxmox-network-interface-pinning":
>> +  - proxmox-network-interface-pinning
>> diff --git a/debian/copyright b/debian/copyright
>> new file mode 100644
>> index 0000000..61c573d
>> --- /dev/null
>> +++ b/debian/copyright
>> @@ -0,0 +1,17 @@
>> +Copyright (C) 2025 Proxmox Server Solutions GmbH
>> +
>> +This software is written by Proxmox Server Solutions GmbH <support at proxmox.com>
>> +
>> +This program is free software: you can redistribute it and/or modify
>> +it under the terms of the GNU Affero General Public License as published by
>> +the Free Software Foundation, either version 3 of the License, or
>> +(at your option) any later version.
>> +
>> +This program is distributed in the hope that it will be useful,
>> +but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +GNU Affero General Public License for more details.
>> +
>> +You should have received a copy of the GNU Affero General Public License
>> +along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> diff --git a/debian/debcargo.toml b/debian/debcargo.toml
>> new file mode 100644
>> index 0000000..703440f
>> --- /dev/null
>> +++ b/debian/debcargo.toml
>> @@ -0,0 +1,8 @@
>> +overlay = "."
>> +crate_src_path = ".."
>> +maintainer = "Proxmox Support Team <support at proxmox.com>"
>> +
>> +[source]
>> +# TODO: update once public
>> +vcs_git = ""
>> +vcs_browser = ""
>> diff --git a/debian/rules b/debian/rules
>> new file mode 100755
>> index 0000000..e157e13
>> --- /dev/null
>> +++ b/debian/rules
>> @@ -0,0 +1,31 @@
>> +#!/usr/bin/make -f
>> +# See debhelper(7) (uncomment to enable)
>> +# output every command that modifies files on the build system.
>> +DH_VERBOSE = 1
>> +
>> +include /usr/share/dpkg/pkg-info.mk
>> +include /usr/share/rustc/architecture.mk
>> +
>> +export BUILD_MODE=release
>> +
>> +CARGO=/usr/share/cargo/bin/cargo
>> +
>> +export CFLAGS CXXFLAGS CPPFLAGS LDFLAGS
>> +export DEB_HOST_RUST_TYPE DEB_HOST_GNU_TYPE
>> +export CARGO_HOME = $(CURDIR)/debian/cargo_home
>> +
>> +export DEB_CARGO_CRATE=proxmox-network-interface-pinning_$(DEB_VERSION_UPSTREAM)
>> +export DEB_CARGO_PACKAGE=proxmox-network-interface-pinning
>> +
>> +%:
>> +	dh $@
>> +
>> +override_dh_auto_configure:
>> +	@perl -ne 'if (/^version\s*=\s*"(\d+(?:\.\d+)+)"/) { my $$v_cargo = $$1; my $$v_deb = "$(DEB_VERSION_UPSTREAM)"; \
>> +	    die "ERROR: d/changelog <-> Cargo.toml version mismatch: $$v_cargo != $$v_deb\n" if $$v_cargo ne $$v_deb; exit(0); }' Cargo.toml
>> +	$(CARGO) prepare-debian $(CURDIR)/debian/cargo_registry --link-from-system
>> +	dh_auto_configure
>> +
>> +override_dh_missing:
>> +	dh_missing --fail-missing
>> +
>> diff --git a/src/main.rs b/src/main.rs
>> new file mode 100644
>> index 0000000..4c83d17
>> --- /dev/null
>> +++ b/src/main.rs
>> @@ -0,0 +1,667 @@
>> +use std::collections::{HashMap, HashSet};
>> +use std::fmt::{Display, Formatter};
>> +use std::ops::{Deref, DerefMut};
>> +use std::os::unix::process::CommandExt;
>> +use std::process::Command;
>> +use std::process::Stdio;
>> +
>> +use anyhow::{anyhow, bail, format_err, Error};
>> +use nix::unistd::{Uid, User};
>> +use serde::de::{value::MapDeserializer, Deserialize};
>> +
>> +use proxmox_log::{debug, LevelFilter};
>> +use proxmox_network_types::mac_address::MacAddress;
>> +use proxmox_router::cli::{
>> +    run_cli_command, CliCommand, CliCommandMap, CliEnvironment, Confirmation,
>> +};
>> +use proxmox_schema::api;
>> +use walkdir::WalkDir;
>> +
>> +const SYSTEMD_LINK_FILE_PATH: &str = "/usr/local/lib/systemd/network";
>> +
>> +/// A mapping of interface names.
>> +///
>> +/// The containing HashMap uses the old name as key and the designated new name as value.
>> +#[derive(Debug, Clone, serde::Deserialize, Default)]
>> +pub struct InterfaceMapping {
>> +    mapping: HashMap<String, String>,
>> +}
>> +
>> +impl Deref for InterfaceMapping {
>> +    type Target = HashMap<String, String>;
>> +
>> +    fn deref(&self) -> &Self::Target {
>> +        &self.mapping
>> +    }
>> +}
>> +
>> +impl DerefMut for InterfaceMapping {
>> +    fn deref_mut(&mut self) -> &mut Self::Target {
>> +        &mut self.mapping
>> +    }
>> +}
>> +
>> +impl InterfaceMapping {
>> +    /// writes an interface mapping to `/usr/local/lib/systemd/network/`.
> 
> We probably never generate rustdocs here so might as well just say
> `SYSTEMD_LINK_FILE_PATH` here.

done

>> +    ///
>> +    /// It uses [`ip_links`] to determine the MAC address of the interfaces that should be pinned,
>> +    /// since we pin based on MAC addresses.
>> +    pub fn write(
>> +        &self,
>> +        ip_links: HashMap<String, proxmox_network_api::IpLink>,
>> +    ) -> Result<(), Error> {
>> +        if self.mapping.is_empty() {
>> +            return Ok(());
>> +        }
>> +
>> +        println!("Generating link files");
>> +
>> +        std::fs::create_dir_all(SYSTEMD_LINK_FILE_PATH)?;
>> +
>> +        let mut sorted_links: Vec<&proxmox_network_api::IpLink> = ip_links.values().collect();
>> +        sorted_links.sort_by_key(|a| a.index());
>> +
>> +        for ip_link in sorted_links {
>> +            if let Some(new_name) = self.mapping.get(ip_link.name()) {
>> +                let link_file = LinkFile::new_ether(ip_link.permanent_mac(), new_name.to_string());
>> +
>> +                std::fs::write(
>> +                    format!("{}/{}", SYSTEMD_LINK_FILE_PATH, link_file.file_name()),
>> +                    link_file.to_string().as_bytes(),
>> +                )?;
>> +            }
>> +        }
>> +
>> +        println!("Successfully generated .link files in '/usr/local/lib/systemd/network/'");
> 
> Replace the path with `{SYSTEMD_LINK_FILE_PATH}`

done

>> +        Ok(())
>> +    }
>> +}
>> +
>> +/// A struct holding information about existing pinned network interfaces.
>> +///
>> +/// Can be constructed from an iterator over [`LinkFile`] via [`PinnedInterfaces::from_iter`].
>> +struct PinnedInterfaces {
>> +    mapping: HashMap<MacAddress, String>,
>> +}
>> +
>> +impl PinnedInterfaces {
>> +    pub fn get(&self, mac_address: &MacAddress) -> Option<&String> {
>> +        self.mapping.get(mac_address)
>> +    }
>> +
>> +    pub fn values(&self) -> impl Iterator<Item = &String> {
>> +        self.mapping.values()
>> +    }
>> +
>> +    pub fn contains_key(&self, mac_address: &MacAddress) -> bool {
>> +        self.mapping.contains_key(mac_address)
>> +    }
>> +}
>> +
>> +impl FromIterator<LinkFile> for PinnedInterfaces {
>> +    fn from_iter<T: IntoIterator<Item = LinkFile>>(iter: T) -> Self {
>> +        Self {
>> +            mapping: iter
>> +                .into_iter()
>> +                .map(|link_file| {
>> +                    (
>> +                        link_file.match_section.mac_address,
>> +                        link_file.link_section.name,
>> +                    )
>> +                })
>> +                .collect(),
>> +        }
>> +    }
>> +}
>> +
>> +/// The Match section of a systemd .link file, as created by this tool.
>> +///
>> +/// This struct only models the properties that are contained in .link files managed by this tool.
>> +/// It cannot be used to model / parse arbitrary .link files.
>> +#[derive(Debug, Clone, serde::Deserialize)]
>> +struct MatchSection {
>> +    #[serde(rename = "MACAddress")]
>> +    mac_address: MacAddress,
>> +    #[serde(rename = "Type")]
>> +    ty: String,
>> +}
>> +
>> +impl Display for MatchSection {
>> +    fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
>> +        writeln!(f, "MACAddress={}", self.mac_address)?;
>> +        writeln!(f, "Type={}", self.ty)?;
>> +
>> +        Ok(())
>> +    }
>> +}
>> +
>> +/// The Link section of a systemd .link file, as created by this tool.
>> +///
>> +/// This struct only models the properties that are contained in .link files managed by this tool.
>> +/// It cannot be used to model / parse arbitrary .link files.
>> +#[derive(Debug, Clone, serde::Deserialize)]
>> +struct LinkSection {
>> +    #[serde(rename = "Name")]
>> +    name: String,
>> +}
>> +
>> +impl Display for LinkSection {
>> +    fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
>> +        writeln!(f, "Name={}", self.name)
>> +    }
>> +}
>> +
>> +/// Section types contained in .link files managed by this tool.
>> +#[derive(Debug, Clone, serde::Deserialize, Hash, Eq, PartialEq)]
> 
> ^ Should be Copy, too.

done

>> +pub enum LinkFileSection {
>> +    Match,
>> +    Link,
>> +}
>> +
>> +impl std::str::FromStr for LinkFileSection {
>> +    type Err = Error;
>> +
>> +    fn from_str(value: &str) -> Result<Self, Self::Err> {
>> +        Ok(match value {
>> +            "[Match]" => LinkFileSection::Match,
>> +            "[Link]" => LinkFileSection::Link,
>> +            _ => bail!("invalid section type: {value}"),
>> +        })
>> +    }
>> +}
>> +
>> +impl Display for LinkFileSection {
>> +    fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
>> +        writeln!(
>> +            f,
>> +            "{}",
>> +            match self {
>> +                LinkFileSection::Link => "[Link]",
>> +                LinkFileSection::Match => "[Match]",
>> +            }
>> +        )
>> +    }
>> +}
>> +
>> +/// A systemd .link file, as created by this tool.
>> +///
>> +/// This struct only models the sections that are contained in .link files managed by this tool.
>> +/// It cannot be used to model / parse arbitrary .link files.
>> +#[derive(Debug, Clone, serde::Deserialize)]
>> +struct LinkFile {
>> +    match_section: MatchSection,
>> +    link_section: LinkSection,
>> +}
>> +
>> +impl LinkFile {
>> +    /// creates a new [`LinkFile`] to pin a network interface.
>> +    ///
>> +    /// `mac_address` is the MAC address of the interface that should be pinned and `name` is the
>> +    /// name that the interface should get pinned to.
>> +    pub fn new_ether(mac_address: MacAddress, name: String) -> Self {
>> +        Self {
>> +            match_section: MatchSection {
>> +                mac_address,
>> +                ty: "ether".to_string(),
>> +            },
>> +            link_section: LinkSection { name },
>> +        }
>> +    }
>> +
>> +    /// returns the file name, that this link file should be saved as when writing it to disk.
>> +    pub fn file_name(&self) -> String {
>> +        format!("50-pve-{}.link", self.link_section.name)
>> +    }
>> +}
>> +
>> +impl std::str::FromStr for LinkFile {
>> +    type Err = Error;
>> +
>> +    fn from_str(value: &str) -> Result<Self, Self::Err> {
> 
> Note: This could probably go into a micro crate `proxmox-systemd-config`
> as a generic "systemd config file to hashmap parser" (with the sections
> as strings obviously).
> 
> The crate could later be extended to *optionally* also take into
> account overriding files existing in paths which take precedence, as
> well as taking drop-in `.d` into account.
> 
> If from that crate the *line* parsing functions are reusable,
> proxmox-section-config could reuse that as well. There we also have a
> systemd-format parser...

Ah, I didn't know we had one there - I searched the repositories but
didn't find this one.

>> +        let mut sections: HashMap<LinkFileSection, HashMap<String, String>> = HashMap::new();
>> +        let mut current_section = None;
>> +
>> +        for line in value.lines() {
> 
> Technically systemd.syntax also states that lines ending with `\` are
> concatenated with the next *non-comment* line with the `\` replaced by a space.

Yes, I think it is fine for now though since this parser is currently
only intended for files that are generated by this tool. Should
definitely be considered when we split this out and generalize this.

>> +            let line = line.trim();
>> +
>> +            if line.is_empty() || line.starts_with(['#', ';']) {
>> +                continue;
>> +            }
>> +
>> +            if line.starts_with('[') {
>> +                current_section = Some(line.parse()?);
>> +                sections.insert(current_section.as_ref().cloned().unwrap(), HashMap::new());
> 
> .as_ref().cloned() -> .clone()
> and to get rid of the .unwrap(), use Option::Insert:
> 
>     let current_section = *current_section.insert(line.parse()?);
>     sections.insert(current_section, HashMap::new());
> 
> (Note the `*` - since it's Copy now)

done

>> +            } else {
>> +                if current_section.is_none() {
>> +                    bail!("config line without section")
>> +                }
> 
> ^ to get rid of the `.expect()` further down, assign this:
> 
>     let current_section = current_section.context("config line without section")?;
> 
>> +
>> +                let Some((key, value)) = line.split_once("=") else {
>> +                    bail!("could not find key, value pair in link config");
>> +                };
> 
> Note that while systemd in some place I cannot remember tells you that
> you're not supposed to put spaces around the `=`, systemd.syntax(7) also
> explicitly states that whitespace *immediately* before and afterwards is
> ignored.

added a trim_end / trim_start to key / value respectively

>> +
>> +                if key.is_empty() || value.is_empty() {
>> +                    bail!("could not find key, value pair in link config");
>> +                }
>> +
>> +                sections
>> +                    .get_mut(current_section.as_ref().expect("current section is some"))
> 
> ^ Then drop the .as_ref().expect()
> And add an &` instead.

done

>> +                    .expect("section has been inserted")
>> +                    .insert(key.to_string(), value.to_string());
>> +            }
>> +        }
>> +
>> +        let link_data = sections
>> +            .remove(&LinkFileSection::Link)
>> +            .ok_or_else(|| anyhow!("no link section in link file"))?;
>> +
>> +        let link_section = LinkSection::deserialize(MapDeserializer::<
>> +            std::collections::hash_map::IntoIter<String, String>,
>> +            SerdeStringError,
>> +        >::new(link_data.into_iter()))?;
>> +
>> +        let match_data = sections
>> +            .remove(&LinkFileSection::Match)
>> +            .ok_or_else(|| anyhow!("no match section in link file"))?;
>> +
>> +        let match_section = MatchSection::deserialize(MapDeserializer::<
>> +            std::collections::hash_map::IntoIter<String, String>,
> 
> ^ You don't need to name this type, you can just pass `_`.

done

>> +            SerdeStringError,
>> +        >::new(match_data.into_iter()))?;
>> +
>> +        Ok(Self {
>> +            match_section,
>> +            link_section,
>> +        })
>> +    }
>> +}
>> +
>> +/// Custom serde Error type for parsing dynamic key-value pairs via [`MapDeserializer`]
>> +#[derive(Debug)]
>> +pub struct SerdeStringError(String);
>> +
>> +impl std::error::Error for SerdeStringError {}
>> +
>> +impl Display for SerdeStringError {
>> +    fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
>> +        f.write_str(&self.0)
>> +    }
>> +}
>> +
>> +impl serde::de::Error for SerdeStringError {
>> +    fn custom<T: Display>(msg: T) -> Self {
>> +        Self(msg.to_string())
>> +    }
>> +}
>> +
>> +impl Display for LinkFile {
>> +    fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
>> +        write!(f, "{}", LinkFileSection::Match)?;
>> +        writeln!(f, "{}", self.match_section)?;
>> +
>> +        write!(f, "{}", LinkFileSection::Link)?;
>> +        writeln!(f, "{}", self.link_section)?;
>> +
>> +        Ok(())
>> +    }
>> +}
>> +
>> +#[repr(transparent)]
>> +#[derive(Debug, Clone, Eq, PartialEq, Hash)]
>> +/// A wrapper struct for [`proxmox_network_api::IpLink`], that implements Ord by comparing
>> +/// ifindexes.
>> +struct IpLink(proxmox_network_api::IpLink);
>> +
>> +impl Deref for IpLink {
>> +    type Target = proxmox_network_api::IpLink;
>> +
>> +    fn deref(&self) -> &Self::Target {
>> +        &self.0
>> +    }
>> +}
>> +
>> +impl PartialOrd for IpLink {
>> +    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
>> +        Some(self.cmp(other))
>> +    }
>> +}
>> +
>> +impl Ord for IpLink {
>> +    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
>> +        self.0.index().cmp(&other.0.index())
>> +    }
>> +}
>> +
>> +impl From<proxmox_network_api::IpLink> for IpLink {
>> +    fn from(value: proxmox_network_api::IpLink) -> Self {
>> +        Self(value)
>> +    }
>> +}
>> +
>> +/// Container that provides the functionality for network interface pinning.
>> +///
>> +/// It holds all information required for generating
>> +pub struct PinningTool {
>> +    ip_links: HashMap<String, proxmox_network_api::IpLink>,
>> +    pinned_interfaces: PinnedInterfaces,
>> +    existing_names: HashSet<String>,
>> +}
>> +
>> +impl PinningTool {
>> +    fn read_link_files() -> Result<Vec<LinkFile>, Error> {
>> +        debug!("reading link data");
>> +
>> +        let link_files = WalkDir::new(SYSTEMD_LINK_FILE_PATH)
>> +            .max_depth(1)
>> +            .into_iter()
>> +            .filter_map(|entry| entry.ok())
>> +            .filter(|entry| {
>> +                if let Some(file_name) = entry.path().file_name() {
>> +                    let name = file_name.to_str().unwrap();
>> +                    return name.starts_with("50-pve-") && name.ends_with(".link");
>> +                }
>> +
>> +                false
>> +            });
>> +
>> +        let mut ip_links = Vec::new();
>> +
>> +        for link_file in link_files {
>> +            debug!("reading file {}", link_file.path().display());
>> +
>> +            let file_content = std::fs::read(link_file.path())?;
>> +            let ip_link = std::str::from_utf8(&file_content)?;
>> +
>> +            ip_links.push(ip_link.parse()?);
>> +        }
>> +
>> +        Ok(ip_links)
>> +    }
>> +
>> +    /// Constructs a new instance of the pinning tool.
>> +    pub fn new() -> Result<Self, Error> {
>> +        let ip_links = proxmox_network_api::get_network_interfaces()?;
>> +        let pinned_interfaces: PinnedInterfaces = Self::read_link_files()?.into_iter().collect();
>> +
>> +        let mut existing_names = HashSet::new();
>> +
>> +        for name in ip_links.keys() {
>> +            existing_names.insert(name.clone());
>> +        }
> 
> Could just be
>     existing_names.extend(ip_link.keys().cloned());

done

>> +
>> +        for pinned_interface in pinned_interfaces.values() {
>> +            existing_names.insert(pinned_interface.clone());
>> +        }
> 
> Could just be
>     existing_names.extend(pinned_interfaces.values().cloned());

done

>> +
>> +        Ok(Self {
>> +            ip_links,
>> +            pinned_interfaces,
>> +            existing_names,
>> +        })
>> +    }
>> +
>> +    /// Pins a specific interface by writing a .link file.
>> +    ///
>> +    /// If `prefix` is given, auto-generates the name for the interface with the given prefix,
>> +    /// otherwise `nic` is used.
>> +    /// If `target_name` is given, the interface is pinned to the specific name.
>> +    pub fn pin_interface(
>> +        self,
>> +        interface_name: &str,
>> +        target_name: Option<String>,
>> +        prefix: Option<String>,
>> +    ) -> Result<InterfaceMapping, Error> {
>> +        let ip_link = self
>> +            .ip_links
>> +            .get(interface_name)
>> +            .ok_or_else(|| anyhow!("cannot find interface with name {interface_name}"))?;
>> +
>> +        if self
>> +            .pinned_interfaces
>> +            .contains_key(&ip_link.permanent_mac())
>> +        {
>> +            bail!("pin already exists for interface {interface_name}");
>> +        }
>> +
>> +        let mut mapping = InterfaceMapping::default();
>> +
>> +        if let Some(target_name) = target_name {
>> +            if self.existing_names.contains(&target_name) {
>> +                bail!("target name already exists");
>> +            }
>> +
>> +            let mut current_altnames = Vec::new();
>> +
>> +            mapping.insert(ip_link.name().to_string(), target_name.to_string());
>> +
>> +            for altname in ip_link.altnames() {
>> +                current_altnames.push(altname.as_str());
>> +                mapping.insert(altname.to_string(), target_name.to_string());
>> +            }
>> +
>> +            println!(
>> +                "Name for {} ({}) will change to {target_name}",
>> +                ip_link.name(),
>> +                current_altnames.join(", ")
>> +            );
>> +        } else if let Some(prefix) = prefix {
>> +            let mut idx = 0;
>> +
>> +            loop {
>> +                let target_name = format!("{prefix}{idx}");
>> +
>> +                if !self.existing_names.contains(&target_name) {
>> +                    let mut current_altnames = Vec::new();
>> +
>> +                    mapping.insert(ip_link.name().to_string(), target_name.to_string());
>> +
>> +                    for altname in ip_link.altnames() {
>> +                        current_altnames.push(altname.as_str());
>> +                        mapping.insert(altname.to_string(), target_name.to_string());
>> +                    }
>> +
>> +                    println!(
>> +                        "Name for {} ({}) will change to {target_name}",
>> +                        ip_link.name(),
>> +                        current_altnames.join(", ")
>> +                    );
>> +
>> +                    break;
>> +                }
>> +
>> +                idx += 1;
>> +            }
>> +        } else {
>> +            return Err(anyhow!(
> bail! ? :)

done

>> +                "neither target-name nor prefix provided for interface"
>> +            ));
>> +        }
>> +
>> +        mapping.write(self.ip_links)?;
>> +        Ok(mapping)
>> +    }
>> +
>> +    /// Pins all physical interfaces available on the host by writing respective .link files.
>> +    ///
>> +    /// Names are generated according to the given `prefix`. Interfaces are iterated in ascending
>> +    /// order, sorted by their ifindex.
>> +    pub fn pin_all(mut self, prefix: &str) -> Result<InterfaceMapping, Error> {
>> +        let mut mapping = InterfaceMapping::default();
>> +
>> +        let mut idx = 0;
>> +
>> +        let mut eligible_links: Vec<IpLink> = self
>> +            .ip_links
>> +            .values()
>> +            .filter(|ip_link| {
>> +                ip_link.is_physical()
>> +                    && self
>> +                        .pinned_interfaces
>> +                        .get(&ip_link.permanent_mac())
>> +                        .is_none()
>> +            })
>> +            .cloned()
>> +            .map(IpLink::from)
>> +            .collect();
>> +
>> +        eligible_links.sort();
>> +
>> +        for ip_link in eligible_links {
>> +            loop {
>> +                let target_name = format!("{prefix}{idx}");
>> +
>> +                if !self.existing_names.contains(&target_name) {
>> +                    let mut current_altnames = Vec::new();
>> +
>> +                    mapping.insert(ip_link.name().to_string(), target_name.to_string());
>> +
>> +                    for altname in ip_link.altnames() {
>> +                        current_altnames.push(altname.as_str());
>> +                        mapping.insert(altname.to_string(), target_name.to_string());
>> +                    }
>> +
>> +                    println!(
>> +                        "Name for {} ({}) will change to {target_name}",
>> +                        ip_link.name(),
>> +                        current_altnames.join(", ")
>> +                    );
>> +
>> +                    self.existing_names.insert(target_name);
>> +
>> +                    break;
>> +                }
>> +
>> +                idx += 1;
>> +            }
>> +        }
>> +
>> +        mapping.write(self.ip_links)?;
>> +        Ok(mapping)
>> +    }
>> +}
>> +
>> +#[api(
>> +    input: {
>> +        properties: {
>> +            interface: {
>> +                type: String,
>> +                optional: true,
>> +                description: "Only pin a specific interface.",
>> +            },
>> +            prefix: {
>> +                type: String,
>> +                optional: true,
>> +                description: "Use a specific prefix for automatically choosing the pinned name.",
>> +            },
>> +            "target-name": {
>> +                type: String,
>> +                optional: true,
>> +                description: "Pin the interface to a specific name.",
>> +            },
>> +        }
>> +    }
>> +)]
>> +/// Generates link files to pin the names of network interfaces (based on MAC address).
>> +fn generate_mapping(
>> +    interface: Option<String>,
>> +    prefix: Option<String>,
>> +    target_name: Option<String>,
>> +) -> Result<(), Error> {
>> +    let pinning_tool = PinningTool::new()?;
>> +
>> +    let target = if let Some(ref interface) = interface {
>> +        interface.as_str()
>> +    } else {
>> +        "all interfaces"
>> +    };
>> +
>> +    let confirmation = Confirmation::query_with_default(
>> +        format!("This will generate name pinning configuration for {target} - continue (y/N)?")
>> +            .as_str(),
>> +        Confirmation::No,
>> +    )?;
>> +
>> +    if confirmation.is_no() {
>> +        return Ok(());
>> +    }
>> +
>> +    let mapping = if let Some(interface) = interface {
>> +        pinning_tool.pin_interface(&interface, target_name, prefix)?
>> +    } else {
>> +        let prefix = prefix.unwrap_or("nic".to_string());
>> +        pinning_tool.pin_all(&prefix)?
>> +    };
>> +
>> +    if mapping.is_empty() {
>> +        println!("Nothing to do. Aborting.");
>> +        return Ok(());
>> +    }
>> +
>> +    println!("Updating /etc/network/interfaces.new");
>> +
>> +    proxmox_network_api::lock_config()?;
>> +
>> +    let (mut config, _) = proxmox_network_api::config()?;
>> +    config.rename_interfaces(mapping.deref())?;
>> +
>> +    proxmox_network_api::save_config(&config)?;
>> +
>> +    println!("Successfully updated network configuration files.");
>> +
>> +    println!("\nPlease reboot to apply the changes to your configuration\n");
>> +
>> +    Ok(())
>> +}
>> +
>> +// TODO: make this load the unprivileged user dynamically, depending on product, default to backup
>> +// for now since we only ship the tool with PBS currently
>> +pub fn unprivileged_user() -> Result<nix::unistd::User, Error> {
>> +    if cfg!(test) {
>> +        Ok(User::from_uid(Uid::current())?.expect("current user does not exist"))
>> +    } else {
>> +        User::from_name("backup")?.ok_or_else(|| format_err!("Unable to lookup 'backup' user."))
>> +    }
>> +}
>> +
>> +pub fn privileged_user() -> Result<nix::unistd::User, Error> {
>> +    if cfg!(test) {
>> +        Ok(User::from_uid(Uid::current())?.expect("current user does not exist"))
>> +    } else {
>> +        User::from_name("root")?.ok_or_else(|| format_err!("Unable to lookup superuser."))
>> +    }
>> +}
>> +
>> +const PVE_NETWORK_INTERFACE_PINNING_BIN: &str =
>> +    "/usr/libexec/proxmox/pve-network-interface-pinning";
>> +
>> +fn main() -> Result<(), Error> {
>> +    // This is run on a PVE host, so we use the PVE-specific pinning tool instead with the
>> +    // parameters supplied.
>> +    if std::fs::exists(PVE_NETWORK_INTERFACE_PINNING_BIN)? {
>> +        let args = std::env::args().skip(1);
>> +
>> +        return Err(Command::new(PVE_NETWORK_INTERFACE_PINNING_BIN)
>> +            .args(args)
>> +            .stdout(Stdio::inherit())
>> +            .stderr(Stdio::inherit())
>> +            .exec()
>> +            .into());
>> +    }
>> +
>> +    proxmox_log::Logger::from_env("PBS_LOG", LevelFilter::INFO)
>> +        .stderr()
>> +        .init()
>> +        .expect("failed to initiate logger");
>> +
>> +    // required for locking the network config
>> +    proxmox_product_config::init(unprivileged_user()?, privileged_user()?);
>> +
>> +    let generate_command = CliCommand::new(&API_METHOD_GENERATE_MAPPING);
>> +    let commands = CliCommandMap::new().insert("generate", generate_command);
>> +
>> +    let rpcenv = CliEnvironment::new();
>> +
>> +    run_cli_command(commands, rpcenv, None);
>> +
>> +    Ok(())
>> +}
>> -- 
>> 2.47.2





More information about the pbs-devel mailing list