From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 2A9921FF16B for ; Tue, 29 Jul 2025 18:56:08 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id BC3C418CCF; Tue, 29 Jul 2025 18:57:31 +0200 (CEST) From: Stefan Hanreich To: pbs-devel@lists.proxmox.com Date: Tue, 29 Jul 2025 18:56:48 +0200 Message-ID: <20250729165655.681368-4-s.hanreich@proxmox.com> X-Mailer: git-send-email 2.47.2 In-Reply-To: <20250729165655.681368-1-s.hanreich@proxmox.com> References: <20250729165655.681368-1-s.hanreich@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.195 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [helper.rs, parser.rs, mod.rs, proxmox.com] Subject: [pbs-devel] [PATCH proxmox 2/3] proxmox-network-api: use ip link for querying interface information X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" We only obtained information about whether an interface is active or not when querying the interfaces of a host. This is not sufficient anymore with the introduction of proxmox-network-interface-pinning. We need additional information (MAC address, type of interface, altnames) to pin network interfaces and to detect them in the rest of the PBS network stack. Use 'ip link', analogous to Proxmox VE, instead which provides all the required information. In the future this could be adapted to query the kernel via netlink directly, avoiding spawning an additional process. For this reason, the struct does not expose any internals, so we can easily switch to netlink in a transparent way for all call sites. Part of this has been copied over from proxmox-ve-rs, which already had versions of the IpLink and AltnameMapping struct. Signed-off-by: Stefan Hanreich --- Cargo.toml | 1 + proxmox-network-api/Cargo.toml | 2 + proxmox-network-api/debian/control | 8 +- proxmox-network-api/src/config/helper.rs | 158 ++++++++++++++--------- proxmox-network-api/src/config/mod.rs | 4 +- proxmox-network-api/src/config/parser.rs | 37 ++++-- 6 files changed, 134 insertions(+), 76 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fd7eba63..a3d185c6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -148,6 +148,7 @@ proxmox-io = { version = "1.2.0", path = "proxmox-io" } proxmox-lang = { version = "1.5", path = "proxmox-lang" } proxmox-log = { version = "1.0.0", path = "proxmox-log" } proxmox-login = { version = "1.0.0", path = "proxmox-login" } +proxmox-network-types = { version = "0.1.0", path = "proxmox-network-types" } proxmox-product-config = { version = "1.0.0", path = "proxmox-product-config" } proxmox-config-digest = { version = "1.0.0", path = "proxmox-config-digest" } proxmox-rest-server = { version = "1.0.0", path = "proxmox-rest-server" } diff --git a/proxmox-network-api/Cargo.toml b/proxmox-network-api/Cargo.toml index add3018b..b4b7db9e 100644 --- a/proxmox-network-api/Cargo.toml +++ b/proxmox-network-api/Cargo.toml @@ -17,6 +17,7 @@ const_format.workspace = true regex.workspace = true serde = { workspace = true, features = ["derive"] } +serde_json.workspace = true nix = { workspace = true, optional = true } libc = { workspace = true, optional = true } @@ -24,6 +25,7 @@ proxmox-sys = { workspace = true, optional = true } proxmox-schema = { workspace = true, features = ["api-macro", "api-types"] } proxmox-config-digest = { workspace = true, optional = true } proxmox-product-config = { workspace = true, optional = true } +proxmox-network-types.workspace = true [features] default = [] diff --git a/proxmox-network-api/debian/control b/proxmox-network-api/debian/control index dcdad532..581db711 100644 --- a/proxmox-network-api/debian/control +++ b/proxmox-network-api/debian/control @@ -8,12 +8,14 @@ Build-Depends-Arch: cargo:native , libstd-rust-dev , librust-anyhow-1+default-dev , librust-const-format-0.2+default-dev , + librust-proxmox-network-types-0.1+default-dev , librust-proxmox-schema-4+api-macro-dev (>= 4.1.0-~~) , librust-proxmox-schema-4+api-types-dev (>= 4.1.0-~~) , librust-proxmox-schema-4+default-dev (>= 4.1.0-~~) , librust-regex-1+default-dev (>= 1.5-~~) , librust-serde-1+default-dev , - librust-serde-1+derive-dev + librust-serde-1+derive-dev , + librust-serde-json-1+default-dev Maintainer: Proxmox Support Team Standards-Version: 4.7.0 Vcs-Git: git://git.proxmox.com/git/proxmox.git @@ -29,12 +31,14 @@ Depends: ${misc:Depends}, librust-anyhow-1+default-dev, librust-const-format-0.2+default-dev, + librust-proxmox-network-types-0.1+default-dev, librust-proxmox-schema-4+api-macro-dev (>= 4.1.0-~~), librust-proxmox-schema-4+api-types-dev (>= 4.1.0-~~), librust-proxmox-schema-4+default-dev (>= 4.1.0-~~), librust-regex-1+default-dev (>= 1.5-~~), librust-serde-1+default-dev, - librust-serde-1+derive-dev + librust-serde-1+derive-dev, + librust-serde-json-1+default-dev Suggests: librust-proxmox-network-api+impl-dev (= ${binary:Version}) Provides: diff --git a/proxmox-network-api/src/config/helper.rs b/proxmox-network-api/src/config/helper.rs index 9d817c43..fa8a64de 100644 --- a/proxmox-network-api/src/config/helper.rs +++ b/proxmox-network-api/src/config/helper.rs @@ -1,15 +1,13 @@ use std::collections::HashMap; -use std::os::unix::io::AsRawFd; use std::path::Path; use std::process::Command; use std::sync::LazyLock; -use anyhow::{bail, format_err, Error}; +use anyhow::{bail, format_err, Context, Error}; use const_format::concatcp; -use nix::ioctl_read_bad; -use nix::sys::socket::{socket, AddressFamily, SockFlag, SockType}; use regex::Regex; +use proxmox_network_types::mac_address::MacAddress; use proxmox_schema::api_types::IPV4RE_STR; use proxmox_schema::api_types::IPV6RE_STR; @@ -119,72 +117,110 @@ pub(crate) fn parse_address_or_cidr(cidr: &str) -> Result<(String, Option, b } } -pub(crate) fn get_network_interfaces() -> Result, Error> { - const PROC_NET_DEV: &str = "/proc/net/dev"; +#[derive(Debug, Clone, serde::Deserialize)] +pub struct SlaveData { + perm_hw_addr: Option, +} + +#[derive(Debug, Clone, serde::Deserialize)] +pub struct LinkInfo { + info_slave_data: Option, + info_kind: Option, +} + +#[derive(Debug, Clone, serde::Deserialize)] +pub struct IpLink { + ifname: String, + #[serde(default)] + altnames: Vec, + ifindex: i64, + link_type: String, + address: MacAddress, + linkinfo: Option, + operstate: String, +} + +impl IpLink { + pub fn index(&self) -> i64 { + self.ifindex + } + + pub fn is_physical(&self) -> bool { + self.link_type == "ether" + && (self.linkinfo.is_none() || self.linkinfo.as_ref().unwrap().info_kind.is_none()) + } - #[repr(C)] - pub struct ifreq { - ifr_name: [libc::c_uchar; libc::IFNAMSIZ], - ifru_flags: libc::c_short, + pub fn name(&self) -> &str { + &self.ifname } - ioctl_read_bad!(get_interface_flags, libc::SIOCGIFFLAGS, ifreq); - - static IFACE_LINE_REGEX: LazyLock = - LazyLock::new(|| Regex::new(r"^\s*([^:\s]+):").unwrap()); - let raw = std::fs::read_to_string(PROC_NET_DEV) - .map_err(|err| format_err!("unable to read {} - {}", PROC_NET_DEV, err))?; - - let lines = raw.lines(); - - let sock = socket( - AddressFamily::Inet, - SockType::Datagram, - SockFlag::empty(), - None, - ) - .or_else(|_| { - socket( - AddressFamily::Inet6, - SockType::Datagram, - SockFlag::empty(), - None, - ) - })?; - - let mut interface_list = HashMap::new(); - - for line in lines { - if let Some(cap) = IFACE_LINE_REGEX.captures(line) { - let ifname = &cap[1]; - - let mut req = ifreq { - ifr_name: *b"0000000000000000", - ifru_flags: 0, - }; - for (i, b) in std::ffi::CString::new(ifname)? - .as_bytes_with_nul() - .iter() - .enumerate() - { - if i < (libc::IFNAMSIZ - 1) { - req.ifr_name[i] = *b as libc::c_uchar; + pub fn permanent_mac(&self) -> MacAddress { + if let Some(link_info) = &self.linkinfo { + if let Some(info_slave_data) = &link_info.info_slave_data { + if let Some(perm_hw_addr) = info_slave_data.perm_hw_addr { + return perm_hw_addr; } } - let res = unsafe { get_interface_flags(sock.as_raw_fd(), &mut req)? }; - if res != 0 { - bail!( - "ioctl get_interface_flags for '{}' failed ({})", - ifname, - res - ); + } + + self.address + } + + pub fn altnames(&self) -> impl Iterator { + self.altnames.iter() + } + + pub fn active(&self) -> bool { + self.operstate == "UP" + } +} + +#[derive(Debug, Clone, serde::Deserialize)] +pub struct AltnameMapping { + mapping: HashMap, +} + +impl std::ops::Deref for AltnameMapping { + type Target = HashMap; + + fn deref(&self) -> &Self::Target { + &self.mapping + } +} + +impl FromIterator for AltnameMapping { + fn from_iter>(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()); } - let is_up = (req.ifru_flags & (libc::IFF_UP as libc::c_short)) != 0; - interface_list.insert(ifname.to_string(), is_up); } + + Self { mapping } + } +} + +pub fn get_network_interfaces() -> Result, 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(interface_list) + Ok(serde_json::from_slice::>(&output.stdout) + .with_context(|| "could not deserialize ip link output")? + .into_iter() + .map(|ip_link| (ip_link.ifname.clone(), ip_link)) + .collect()) } pub(crate) fn compute_file_diff(filename: &str, shadow: &str) -> Result { diff --git a/proxmox-network-api/src/config/mod.rs b/proxmox-network-api/src/config/mod.rs index 054f53c8..e8cb81d1 100644 --- a/proxmox-network-api/src/config/mod.rs +++ b/proxmox-network-api/src/config/mod.rs @@ -2,7 +2,7 @@ mod helper; mod lexer; mod parser; -pub use helper::{assert_ifupdown2_installed, network_reload, parse_cidr}; +pub use helper::{assert_ifupdown2_installed, network_reload, parse_cidr, AltnameMapping, IpLink}; use std::collections::{BTreeMap, HashMap, HashSet}; use std::io::Write; @@ -17,7 +17,7 @@ use super::{ }; use helper::compute_file_diff; -use helper::get_network_interfaces; +pub use helper::get_network_interfaces; use parser::NetworkParser; use proxmox_config_digest::ConfigDigest; diff --git a/proxmox-network-api/src/config/parser.rs b/proxmox-network-api/src/config/parser.rs index d05a67b0..500ae8ec 100644 --- a/proxmox-network-api/src/config/parser.rs +++ b/proxmox-network-api/src/config/parser.rs @@ -1,4 +1,4 @@ -use crate::VLAN_INTERFACE_REGEX; +use crate::{PHYSICAL_NIC_REGEX, VLAN_INTERFACE_REGEX}; use std::collections::{HashMap, HashSet}; use std::io::BufRead; @@ -502,7 +502,7 @@ impl NetworkParser { pub fn parse_interfaces( &mut self, - existing_interfaces: Option<&HashMap>, + existing_interfaces: Option<&HashMap>, ) -> Result { self.do_parse_interfaces(existing_interfaces) .map_err(|err| format_err!("line {}: {}", self.line_nr, err)) @@ -510,7 +510,7 @@ impl NetworkParser { fn do_parse_interfaces( &mut self, - existing_interfaces: Option<&HashMap>, + existing_interfaces: Option<&HashMap>, ) -> Result { let mut config = NetworkConfig::new(); @@ -555,20 +555,20 @@ impl NetworkParser { LazyLock::new(|| Regex::new(r"^\S+:\d+$").unwrap()); if let Some(existing_interfaces) = existing_interfaces { - for (iface, active) in existing_interfaces.iter() { + for (iface, ip_link) in existing_interfaces.iter() { if let Some(interface) = config.interfaces.get_mut(iface) { - interface.active = *active; + interface.active = ip_link.active(); if interface.interface_type == NetworkInterfaceType::Unknown - && super::is_physical_nic(iface) + && ip_link.is_physical() { interface.interface_type = NetworkInterfaceType::Eth; } - } else if super::is_physical_nic(iface) { + } else if ip_link.is_physical() { // also add all physical NICs let mut interface = Interface::new(iface.clone()); set_method_v4(&mut interface, NetworkConfigMethod::Manual)?; interface.interface_type = NetworkInterfaceType::Eth; - interface.active = *active; + interface.active = ip_link.active(); config.interfaces.insert(interface.name.clone(), interface); config .order @@ -593,9 +593,24 @@ impl NetworkParser { interface.interface_type = NetworkInterfaceType::Vlan; continue; } - if super::is_physical_nic(name) { - interface.interface_type = NetworkInterfaceType::Eth; - continue; + + match existing_interfaces { + Some(existing_interfaces) => { + let is_physical = existing_interfaces + .get(name) + .map(|iface| iface.is_physical()) + .unwrap_or(false); + + if is_physical { + interface.interface_type = NetworkInterfaceType::Eth; + continue; + } + } + None if PHYSICAL_NIC_REGEX.is_match(name) => { + interface.interface_type = NetworkInterfaceType::Eth; + continue; + } + _ => {} } } -- 2.47.2 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel