[pbs-devel] [PATCH proxmox 2/3] proxmox-network-api: use ip link for querying interface information

[Stefan Hanreich <s.hanreich@proxmox.com>]

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 <s.hanreich@proxmox.com>
---
 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 <!nocheck>,
  libstd-rust-dev <!nocheck>,
  librust-anyhow-1+default-dev <!nocheck>,
  librust-const-format-0.2+default-dev <!nocheck>,
+ librust-proxmox-network-types-0.1+default-dev <!nocheck>,
  librust-proxmox-schema-4+api-macro-dev (>= 4.1.0-~~) <!nocheck>,
  librust-proxmox-schema-4+api-types-dev (>= 4.1.0-~~) <!nocheck>,
  librust-proxmox-schema-4+default-dev (>= 4.1.0-~~) <!nocheck>,
  librust-regex-1+default-dev (>= 1.5-~~) <!nocheck>,
  librust-serde-1+default-dev <!nocheck>,
- librust-serde-1+derive-dev <!nocheck>
+ librust-serde-1+derive-dev <!nocheck>,
+ librust-serde-json-1+default-dev <!nocheck>
 Maintainer: Proxmox Support Team <support@proxmox.com>
 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<u8>, b
     }
 }
 
-pub(crate) fn get_network_interfaces() -> Result<HashMap<String, bool>, Error> {
-    const PROC_NET_DEV: &str = "/proc/net/dev";
+#[derive(Debug, Clone, serde::Deserialize)]
+pub struct SlaveData {
+    perm_hw_addr: Option<MacAddress>,
+}
+
+#[derive(Debug, Clone, serde::Deserialize)]
+pub struct LinkInfo {
+    info_slave_data: Option<SlaveData>,
+    info_kind: Option<String>,
+}
+
+#[derive(Debug, Clone, serde::Deserialize)]
+pub struct IpLink {
+    ifname: String,
+    #[serde(default)]
+    altnames: Vec<String>,
+    ifindex: i64,
+    link_type: String,
+    address: MacAddress,
+    linkinfo: Option<LinkInfo>,
+    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<Regex> =
-        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<Item = &String> {
+        self.altnames.iter()
+    }
+
+    pub fn active(&self) -> bool {
+        self.operstate == "UP"
+    }
+}
+
+#[derive(Debug, Clone, serde::Deserialize)]
+pub struct AltnameMapping {
+    mapping: HashMap<String, String>,
+}
+
+impl std::ops::Deref for AltnameMapping {
+    type Target = HashMap<String, String>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.mapping
+    }
+}
+
+impl FromIterator<IpLink> for AltnameMapping {
+    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());
             }
-            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<HashMap<String, IpLink>, 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::<Vec<IpLink>>(&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<String, Error> {
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<R: BufRead> NetworkParser<R> {
 
     pub fn parse_interfaces(
         &mut self,
-        existing_interfaces: Option<&HashMap<String, bool>>,
+        existing_interfaces: Option<&HashMap<String, IpLink>>,
     ) -> Result<NetworkConfig, Error> {
         self.do_parse_interfaces(existing_interfaces)
             .map_err(|err| format_err!("line {}: {}", self.line_nr, err))
@@ -510,7 +510,7 @@ impl<R: BufRead> NetworkParser<R> {
 
     fn do_parse_interfaces(
         &mut self,
-        existing_interfaces: Option<&HashMap<String, bool>>,
+        existing_interfaces: Option<&HashMap<String, IpLink>>,
     ) -> Result<NetworkConfig, Error> {
         let mut config = NetworkConfig::new();
 
@@ -555,20 +555,20 @@ impl<R: BufRead> NetworkParser<R> {
             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<R: BufRead> NetworkParser<R> {
                 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