From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 6A0AE1FF185 for ; Mon, 4 Aug 2025 18:23:56 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5FE7F36B5D; Mon, 4 Aug 2025 18:25:27 +0200 (CEST) From: Stefan Hanreich To: pbs-devel@lists.proxmox.com Date: Mon, 4 Aug 2025 18:24:37 +0200 Message-ID: <20250804162448.607184-4-s.hanreich@proxmox.com> X-Mailer: git-send-email 2.47.2 In-Reply-To: <20250804162448.607184-1-s.hanreich@proxmox.com> References: <20250804162448.607184-1-s.hanreich@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.193 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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 Subject: [pbs-devel] [PATCH proxmox v5 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. Additionally we also add the information about the altnames of an interface, obtained from ip link, to the Interface struct. 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 Tested-by: Christian Ebner --- Cargo.toml | 1 + proxmox-network-api/Cargo.toml | 2 + proxmox-network-api/debian/control | 8 +- proxmox-network-api/src/api_types.rs | 12 ++ proxmox-network-api/src/config/helper.rs | 158 ++++++++++++++--------- proxmox-network-api/src/config/mod.rs | 4 +- proxmox-network-api/src/config/parser.rs | 39 ++++-- 7 files changed, 148 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/api_types.rs b/proxmox-network-api/src/api_types.rs index d0fbb0eb..8b261d73 100644 --- a/proxmox-network-api/src/api_types.rs +++ b/proxmox-network-api/src/api_types.rs @@ -225,6 +225,14 @@ pub const NETWORK_INTERFACE_LIST_SCHEMA: Schema = type: BondXmitHashPolicy, optional: true, }, + altnames: { + description: "List of altnames for this interface", + type: Array, + items: { + description: "altname", + type: String, + }, + }, } )] #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] @@ -291,6 +299,9 @@ pub struct Interface { pub bond_primary: Option, #[serde(skip_serializing_if = "Option::is_none")] pub bond_xmit_hash_policy: Option, + + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub altnames: Vec, } impl Interface { @@ -319,6 +330,7 @@ impl Interface { bond_mode: None, bond_primary: None, bond_xmit_hash_policy: None, + altnames: Vec::new(), } } diff --git a/proxmox-network-api/src/config/helper.rs b/proxmox-network-api/src/config/helper.rs index 9d817c43..7d5fe1e6 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, Eq, PartialEq, Ord, PartialOrd, Hash, serde::Deserialize)] +pub struct SlaveData { + perm_hw_addr: Option, +} + +#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, serde::Deserialize)] +pub struct LinkInfo { + info_slave_data: Option, + info_kind: Option, +} + +#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, 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..c90810ee 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,22 @@ 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) { + + interface.altnames = ip_link.altnames().cloned().collect(); + } 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 +595,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