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 331C21FF187 for ; Mon, 25 Aug 2025 10:11:30 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 77EF5BA7A; Mon, 25 Aug 2025 10:11:34 +0200 (CEST) Date: Mon, 25 Aug 2025 10:11:00 +0200 From: Wolfgang Bumiller To: Gabriel Goller Message-ID: References: <20250822090102.102949-1-g.goller@proxmox.com> <20250822090102.102949-5-g.goller@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20250822090102.102949-5-g.goller@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1756109456712 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.076 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH proxmox-perl-rs v2 2/4] fabrics: add function to get status of fabric X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Cc: pve-devel@lists.proxmox.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On Fri, Aug 22, 2025 at 11:00:36AM +0200, Gabriel Goller wrote: > Add a function to get the status of a fabric. This is the status which > will then be inserted into the pvestatd daemon and returned through the > resources api. In order the generate the HashMap of statuses for all > fabrics we need to read the fabric config and execute a vtysh (frr) > command to get the routes of the corresponding fabric. If there is at > least one route which is related to the fabric, the fabric is considered > "ok". > > Signed-off-by: Gabriel Goller > --- > pve-rs/src/bindings/sdn/fabrics.rs | 194 +++++++++++++++++++++++++++++ > 1 file changed, 194 insertions(+) > > diff --git a/pve-rs/src/bindings/sdn/fabrics.rs b/pve-rs/src/bindings/sdn/fabrics.rs > index 1dc8bf4320e6..3f70d421e582 100644 > --- a/pve-rs/src/bindings/sdn/fabrics.rs > +++ b/pve-rs/src/bindings/sdn/fabrics.rs > @@ -9,8 +9,10 @@ pub mod pve_rs_sdn_fabrics { > use std::fmt::Write; > use std::net::IpAddr; > use std::ops::Deref; > + use std::process::Command; > use std::sync::Mutex; > > + use anyhow::Context; > use anyhow::Error; > use openssl::hash::{MessageDigest, hash}; > use serde::{Deserialize, Serialize}; > @@ -578,4 +580,196 @@ pub mod pve_rs_sdn_fabrics { > > Ok(interfaces) > } > + > + /// This module contains status-related structs that represent Routes and Neighbors for all > + /// protocols > + pub mod status { ^ This seems to be a helper module which does not contain any perlmod/perl specifics. I'd argue it's time to start a `crate::sdn` module outside of the `bindings` submodule for this. The `bindings` module should become rather more lean in the future and focus mostly on the perl/rust interaction. > + use std::collections::{HashMap, HashSet}; > + > + use serde::Serialize; > + > + use proxmox_frr::de::{self}; > + use proxmox_ve_config::sdn::fabric::{ > + FabricConfig, > + section_config::{fabric::FabricId, node::Node as ConfigNode}, > + }; > + > + /// Protocol > + #[derive(Debug, Serialize, Clone, Copy)] > + pub enum Protocol { > + /// Openfabric > + Openfabric, > + /// OSPF > + Ospf, > + } > + > + /// The status of a fabric. > + #[derive(Debug, Serialize)] > + pub enum FabricStatus { > + /// The fabric exists and has a route > + #[serde(rename = "ok")] > + Ok, > + /// The fabric does not exist or doesn't distribute any routes > + #[serde(rename = "not ok")] > + NotOk, > + } > + > + /// Status of a fabric. > + /// > + /// Check if there are any routes, if yes, then the status is ok, otherwise not ok. ^ Not sure how this describes the *struct*, though ;-) > + #[derive(Debug, Serialize)] > + pub struct Status { > + #[serde(rename = "type")] > + ty: String, > + status: FabricStatus, > + protocol: Protocol, > + sdn: FabricId, > + sdn_type: String, > + } > + > + /// Parsed routes for all protocols > + /// > + /// These are the routes parsed from the json output of: > + /// `vtysh -c 'show ip route json'`. > + #[derive(Debug, Serialize)] > + pub struct RoutesParsed { > + /// All openfabric routes in FRR > + pub openfabric: de::Routes, > + /// All ospf routes in FRR > + pub ospf: de::Routes, > + } > + > + impl TryInto> for RoutesParsed { > + type Error = anyhow::Error; > + > + fn try_into(self) -> Result, Self::Error> { > + let hostname = proxmox_sys::nodename(); > + > + // to associate a route to a fabric, we get all the interfaces which are associated > + // with a fabric on this node and compare them with the interfaces on the route. > + let raw_config = std::fs::read_to_string("/etc/pve/sdn/fabrics.cfg")?; ^ I'm really not a fan of doing file I/O in a TryInto implementation. These are still supposed to be "simple"[1]. Better make this a method. [1] https://doc.rust-lang.org/std/convert/trait.TryFrom.html > + let config = FabricConfig::parse_section_config(&raw_config)?; > + > + let mut stats: HashMap = HashMap::new(); > + > + for (nodeid, node) in config.values().flat_map(|entry| { > + entry > + .nodes() > + .map(|(id, node)| (id.to_string(), node.clone())) ^ Unnecessary `.to_string()` and `.clone()` - the entire `.map()` line can be dropped if use `if nodeid.as_str() != hostname` below. > + }) { > + if nodeid != hostname { > + continue; > + } > + let fabric_id = node.id().fabric_id().clone(); > + > + let current_protocol = match &node { > + ConfigNode::Openfabric(_) => Protocol::Openfabric, > + ConfigNode::Ospf(_) => Protocol::Ospf, > + }; > + > + let mut all_routes = HashMap::new(); > + match &node { > + ConfigNode::Openfabric(_) => all_routes.extend(&self.openfabric.0), > + ConfigNode::Ospf(_) => all_routes.extend(&self.ospf.0), > + } > + > + // get interfaces > + let interface_names: HashSet = match node { This also doesn't need to allocate the string - use &str > + ConfigNode::Openfabric(n) => n > + .properties() > + .interfaces() > + .map(|i| i.name().to_string()) ^ with .as_str() here > + .collect(), > + ConfigNode::Ospf(n) => n > + .properties() > + .interfaces() > + .map(|i| i.name().to_string()) ^ and here > + .collect(), > + }; > + > + // determine status by checking if any routes exist for our interfaces > + let has_routes = all_routes.iter().any(|(_, v)| { ^ Could use .values().any(|v| { (no need to `_` the key) > + v.iter().any(|route| { > + route > + .nexthops > + .iter() > + .any(|nexthop| interface_names.contains(&nexthop.interface_name)) and on .interface_name here > + }) > + }); > + > + let fabric = Status { > + ty: "sdn".to_owned(), > + status: if has_routes { > + FabricStatus::Ok > + } else { > + FabricStatus::NotOk > + }, > + sdn_type: "fabric".to_string(), > + protocol: current_protocol, > + sdn: fabric_id.clone(), > + }; > + stats.insert(fabric_id, fabric); > + } > + > + Ok(stats) > + } > + } > + } > + > + /// Return the status of all fabrics on this node. > + /// > + /// Go through all fabrics in the config, then filter out the ones that exist on this node. > + /// Check if there are any routes in the routing table that use the interface specified in the > + /// config. If there are, show "ok" as status, otherwise "not ok". > + #[export] > + fn status() -> Result, Error> { > + let openfabric_ipv4_routes_string = String::from_utf8( > + Command::new("sh") > + .args(["-c", "vtysh -c 'show ip route openfabric json'"]) > + .output()? > + .stdout, > + )?; > + > + let openfabric_ipv6_routes_string = String::from_utf8( > + Command::new("sh") > + .args(["-c", "vtysh -c 'show ipv6 route openfabric json'"]) > + .output()? > + .stdout, > + )?; > + > + let ospf_routes_string = String::from_utf8( > + Command::new("sh") > + .args(["-c", "vtysh -c 'show ip route ospf json'"]) > + .output()? > + .stdout, > + )?; > + > + let mut openfabric_routes: proxmox_frr::de::Routes = > + if openfabric_ipv4_routes_string.is_empty() { > + proxmox_frr::de::Routes::default() > + } else { > + serde_json::from_str(&openfabric_ipv4_routes_string) > + .with_context(|| "error parsing openfabric ipv4 routes")? > + }; > + if !openfabric_ipv6_routes_string.is_empty() { > + let openfabric_ipv6_routes: proxmox_frr::de::Routes = > + serde_json::from_str(&openfabric_ipv6_routes_string) > + .with_context(|| "error parsing openfabric ipv6 routes")?; > + openfabric_routes.0.extend(openfabric_ipv6_routes.0); > + } > + > + let ospf_routes: proxmox_frr::de::Routes = if ospf_routes_string.is_empty() { > + proxmox_frr::de::Routes::default() > + } else { > + serde_json::from_str(&ospf_routes_string) > + .with_context(|| "error parsing ospf routes")? > + }; > + > + let route_status = status::RoutesParsed { > + openfabric: openfabric_routes, > + ospf: ospf_routes, > + }; > + > + route_status.try_into() > + } > } > -- > 2.47.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel