On Mon, Aug 25, 2025 at 10:11:01AM +0200, Wolfgang Bumiller wrote:
> 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 <g.gol...@proxmox.com>
> > ---
> > 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,
^ If we already have serialization helpers, this could be an enum to
avoid some more string allocations (or use `&'static str`, but an enum
is safer...)
> > + }
> > +
> > + /// Parsed routes for all protocols
> > + ///
> > + /// These are the routes parsed from the json output of:
> > + /// `vtysh -c 'show ip route <protocol> 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<HashMap<FabricId, Status>> for RoutesParsed {
> > + type Error = anyhow::Error;
> > +
> > + fn try_into(self) -> Result<HashMap<FabricId, Status>,
> > 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<FabricId, Status> = 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();
^ same as in patch 3
> > +
> > + let current_protocol = match &node {
> > + ConfigNode::Openfabric(_) => Protocol::Openfabric,
> > + ConfigNode::Ospf(_) => Protocol::Ospf,
> > + };
> > +
> > + let mut all_routes = HashMap::new();
^ same as in patch 3
> > + match &node {
> > + ConfigNode::Openfabric(_) =>
> > all_routes.extend(&self.openfabric.0),
> > + ConfigNode::Ospf(_) =>
> > all_routes.extend(&self.ospf.0),
> > + }
> > +
> > + // get interfaces
> > + let interface_names: HashSet<String> = 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<HashMap<FabricId, status::Status>, 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
