On 25.08.2025 10:11, 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.

Umm do I understand you correctly that you want to have something like
this:

    src/
    ├─ bindings/
    │  ├─ sdn/
    │  │  ├─ fabrics.rs
    ├─ sdn/
    │  ├─ status.rs

?

IMO we could move all the status stuff out to
crate::bindings::sdn::status. But I don't know about separating all the
types, conversion methods and actual perl methods -- I'd rather keep all
the perl-facing stuff in the same file.

+        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 ;-)

Oops, this slipped through, should have been somewhere else.

+        #[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 <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

Yup, I agree, changed all the TryInto impls to functions `get_routes`,
`get_neighbors` and `get_status`.

Also fixed all the other stuff below.

Thanks for the review!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to