public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stefan Hanreich <s.hanreich@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: Re: [PATCH proxmox-perl-rs v4 3/7] sdn: fabrics: add BGP status endpoints
Date: Wed, 13 May 2026 14:33:33 +0200	[thread overview]
Message-ID: <03e0b16c-79df-44cc-bf18-bf2e89c4982d@proxmox.com> (raw)
In-Reply-To: <20260512141305.199664-4-h.laimer@proxmox.com>

On 5/12/26 4:12 PM, Hannes Laimer wrote:
> Expose BGP fabric status through the existing fabric status API.
> Routes are fetched for both IPv4 and IPv6, and neighbor/interface
> state is derived from BGP session info.
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  pve-rs/src/bindings/sdn/fabrics.rs |  97 +++++++++++++++++++++++++-
>  pve-rs/src/sdn/status.rs           | 106 ++++++++++++++++++++++++++++-
>  2 files changed, 198 insertions(+), 5 deletions(-)
> 
> diff --git a/pve-rs/src/bindings/sdn/fabrics.rs b/pve-rs/src/bindings/sdn/fabrics.rs
> index f914094..0189ecd 100644
> --- a/pve-rs/src/bindings/sdn/fabrics.rs
> +++ b/pve-rs/src/bindings/sdn/fabrics.rs
> @@ -12,8 +12,8 @@ pub mod pve_rs_sdn_fabrics {
>      use std::process::Command;
>      use std::sync::Mutex;
>  
> -    use anyhow::{Context, Error, format_err};
> -    use openssl::hash::{MessageDigest, hash};
> +    use anyhow::{format_err, Context, Error};
> +    use openssl::hash::{hash, MessageDigest};
>      use proxmox_ve_config::sdn::fabric::section_config::node::api::{Node, NodeUpdater};
>      use serde::{Deserialize, Serialize};
>  
> @@ -31,8 +31,8 @@ pub mod pve_rs_sdn_fabrics {
>      };
>      use proxmox_ve_config::sdn::fabric::section_config::interface::InterfaceName;
>      use proxmox_ve_config::sdn::fabric::section_config::node::{Node as ConfigNode, NodeId};
> -    use proxmox_ve_config::sdn::fabric::section_config::Section;
>      use proxmox_ve_config::sdn::fabric::section_config::protocol::bgp::BgpNode;
> +    use proxmox_ve_config::sdn::fabric::section_config::Section;
>      use proxmox_ve_config::sdn::fabric::{FabricConfig, FabricEntry};
>      use proxmox_ve_config::sdn::wireguard::WireGuardConfigBuilder;
>  
> @@ -840,6 +840,35 @@ pub mod pve_rs_sdn_fabrics {
>                  status::get_routes(fabric_id, config, ospf_routes, proxmox_sys::nodename())
>              }
>              FabricEntry::WireGuard(_) => Ok(Vec::new()),
> +            FabricEntry::Bgp(_) => {
> +                let bgp_ipv4_routes_string = String::from_utf8(
> +                    Command::new("sh")
> +                        .args(["-c", "vtysh -c 'show ip route bgp json'"])
> +                        .output()?
> +                        .stdout,
> +                )?;
> +
> +                let bgp_ipv6_routes_string = String::from_utf8(
> +                    Command::new("sh")
> +                        .args(["-c", "vtysh -c 'show ipv6 route bgp json'"])
> +                        .output()?
> +                        .stdout,
> +                )?;

pre-existing but something that really irks me is that we pollute the
vtysh history with our pvestatd invocations. It's possible to set
VTYSH_HISTFILE=/dev/null to avoid writing the commands to history - so
we could do that here for all vtysh invocations?

I can prepare a patch for OSPF / Openfabric.

> +                let mut bgp_routes: proxmox_frr::de::Routes = if bgp_ipv4_routes_string.is_empty() {
> +                    proxmox_frr::de::Routes::default()
> +                } else {
> +                    serde_json::from_str(&bgp_ipv4_routes_string)
> +                        .with_context(|| "error parsing bgp ipv4 routes")?
> +                };
> +                if !bgp_ipv6_routes_string.is_empty() {
> +                    let bgp_ipv6_routes: proxmox_frr::de::Routes =
> +                        serde_json::from_str(&bgp_ipv6_routes_string)
> +                            .with_context(|| "error parsing bgp ipv6 routes")?;
> +                    bgp_routes.0.extend(bgp_ipv6_routes.0);
> +                }
> +                status::get_routes(fabric_id, config, bgp_routes, proxmox_sys::nodename())
> +            }
>          }
>      }
>  
> @@ -899,6 +928,23 @@ pub mod pve_rs_sdn_fabrics {
>                  .map(|v| v.into())
>              }
>              FabricEntry::WireGuard(_) => Ok(status::NeighborStatus::WireGuard(Vec::new())),
> +            FabricEntry::Bgp(_) => {
> +                let bgp_neighbors_string = String::from_utf8(
> +                    Command::new("sh")
> +                        .args(["-c", "vtysh -c 'show bgp neighbors json'"])

here as well

> +                        .output()?
> +                        .stdout,
> +                )?;
> +                let bgp_neighbors: std::collections::BTreeMap<String, status::BgpNeighborInfo> =
> +                    if bgp_neighbors_string.is_empty() {
> +                        std::collections::BTreeMap::new()
> +                    } else {
> +                        serde_json::from_str(&bgp_neighbors_string)
> +                            .with_context(|| "error parsing bgp neighbors")?
> +                    };
> +
> +                status::get_neighbors_bgp(fabric_id, bgp_neighbors).map(|v| v.into())
> +            }
>          }
>      }
>  
> @@ -959,6 +1005,23 @@ pub mod pve_rs_sdn_fabrics {
>                  .map(|v| v.into())
>              }
>              FabricEntry::WireGuard(_) => Ok(status::InterfaceStatus::WireGuard(Vec::new())),
> +            FabricEntry::Bgp(_) => {
> +                let bgp_neighbors_string = String::from_utf8(
> +                    Command::new("sh")
> +                        .args(["-c", "vtysh -c 'show bgp neighbors json'"])

here as well

> +                        .output()?
> +                        .stdout,
> +                )?;
> +                let bgp_neighbors: std::collections::BTreeMap<String, status::BgpNeighborInfo> =
> +                    if bgp_neighbors_string.is_empty() {
> +                        std::collections::BTreeMap::new()
> +                    } else {
> +                        serde_json::from_str(&bgp_neighbors_string)
> +                            .with_context(|| "error parsing bgp neighbors")?
> +                    };
> +
> +                status::get_interfaces_bgp(fabric_id, bgp_neighbors).map(|v| v.into())
> +            }
>          }
>      }
>  
> @@ -1019,9 +1082,37 @@ pub mod pve_rs_sdn_fabrics {
>                  .with_context(|| "error parsing ospf routes")?
>          };
>  
> +        let bgp_ipv4_routes_string = String::from_utf8(
> +            Command::new("sh")
> +                .args(["-c", "vtysh -c 'show ip route bgp json'"])

here as well

> +                .output()?
> +                .stdout,
> +        )?;
> +
> +        let bgp_ipv6_routes_string = String::from_utf8(
> +            Command::new("sh")
> +                .args(["-c", "vtysh -c 'show ipv6 route bgp json'"])

here as well

> +                .output()?
> +                .stdout,
> +        )?;
> +
> +        let mut bgp_routes: proxmox_frr::de::Routes = if bgp_ipv4_routes_string.is_empty() {
> +            proxmox_frr::de::Routes::default()
> +        } else {
> +            serde_json::from_str(&bgp_ipv4_routes_string)
> +                .with_context(|| "error parsing bgp ipv4 routes")?
> +        };
> +        if !bgp_ipv6_routes_string.is_empty() {
> +            let bgp_ipv6_routes: proxmox_frr::de::Routes =
> +                serde_json::from_str(&bgp_ipv6_routes_string)
> +                    .with_context(|| "error parsing bgp ipv6 routes")?;
> +            bgp_routes.0.extend(bgp_ipv6_routes.0);
> +        }
> +
>          let route_status = status::RoutesParsed {
>              openfabric: openfabric_routes,
>              ospf: ospf_routes,
> +            bgp: bgp_routes,
>          };
>  
>          status::get_status(config, route_status, proxmox_sys::nodename())
> diff --git a/pve-rs/src/sdn/status.rs b/pve-rs/src/sdn/status.rs
> index 132a0f4..f661d74 100644
> --- a/pve-rs/src/sdn/status.rs
> +++ b/pve-rs/src/sdn/status.rs
> @@ -6,14 +6,15 @@ use proxmox_network_types::mac_address::MacAddress;
>  use serde::{Deserialize, Serialize};
>  
>  use proxmox_frr::de::{self};
> +use proxmox_ve_config::sdn::fabric::section_config::protocol::bgp::BgpNode;
>  use proxmox_ve_config::sdn::fabric::section_config::protocol::ospf::{
>      OspfNodeProperties, OspfProperties,
>  };
>  use proxmox_ve_config::{
>      common::valid::Valid,
>      sdn::fabric::{
> +        section_config::{fabric::FabricId, node::Node as ConfigNode, node::NodeId, Section},
>          Entry, FabricConfig,
> -        section_config::{Section, fabric::FabricId, node::Node as ConfigNode, node::NodeId},
>      },
>  };
>  
> @@ -90,12 +91,33 @@ mod wireguard {
>  }
>  
>  /// Common NeighborStatus that contains either OSPF or Openfabric neighbors
> +mod bgp {
> +    use serde::Serialize;
> +
> +    /// The status of a BGP neighbor.
> +    #[derive(Debug, Serialize, PartialEq, Eq)]
> +    pub struct NeighborStatus {
> +        pub neighbor: String,
> +        pub status: String,
> +        pub uptime: String,
> +    }
> +
> +    /// The status of a BGP fabric interface.
> +    #[derive(Debug, Serialize, PartialEq, Eq)]
> +    pub struct InterfaceStatus {
> +        pub name: String,
> +        pub state: super::InterfaceState,
> +    }
> +}
> +
> +/// Common NeighborStatus that contains either OSPF, Openfabric, or BGP neighbors
>  #[derive(Debug, Serialize)]
>  #[serde(untagged)]
>  pub enum NeighborStatus {
>      Openfabric(Vec<openfabric::NeighborStatus>),
>      Ospf(Vec<ospf::NeighborStatus>),
>      WireGuard(Vec<wireguard::NeighborStatus>),
> +    Bgp(Vec<bgp::NeighborStatus>),
>  }
>  
>  impl From<Vec<openfabric::NeighborStatus>> for NeighborStatus {
> @@ -108,14 +130,20 @@ impl From<Vec<ospf::NeighborStatus>> for NeighborStatus {
>          NeighborStatus::Ospf(value)
>      }
>  }
> +impl From<Vec<bgp::NeighborStatus>> for NeighborStatus {
> +    fn from(value: Vec<bgp::NeighborStatus>) -> Self {
> +        NeighborStatus::Bgp(value)
> +    }
> +}
>  
> -/// Common InterfaceStatus that contains either OSPF or Openfabric interfaces
> +/// Common InterfaceStatus that contains either OSPF, Openfabric, or BGP interfaces
>  #[derive(Debug, Serialize)]
>  #[serde(untagged)]
>  pub enum InterfaceStatus {
>      Openfabric(Vec<openfabric::InterfaceStatus>),
>      Ospf(Vec<ospf::InterfaceStatus>),
>      WireGuard(Vec<wireguard::InterfaceStatus>),
> +    Bgp(Vec<bgp::InterfaceStatus>),
>  }
>  
>  impl From<Vec<openfabric::InterfaceStatus>> for InterfaceStatus {
> @@ -128,6 +156,11 @@ impl From<Vec<ospf::InterfaceStatus>> for InterfaceStatus {
>          InterfaceStatus::Ospf(value)
>      }
>  }
> +impl From<Vec<bgp::InterfaceStatus>> for InterfaceStatus {
> +    fn from(value: Vec<bgp::InterfaceStatus>) -> Self {
> +        InterfaceStatus::Bgp(value)
> +    }
> +}
>  
>  /// The status of a route.
>  ///
> @@ -148,6 +181,8 @@ pub enum Protocol {
>      Ospf,
>      /// WireGuard
>      WireGuard,
> +    /// BGP
> +    Bgp,
>  }
>  
>  /// The status of a fabric.
> @@ -186,6 +221,8 @@ pub struct RoutesParsed {
>      pub openfabric: de::Routes,
>      /// All ospf routes in FRR
>      pub ospf: de::Routes,
> +    /// All bgp routes in FRR
> +    pub bgp: de::Routes,
>  }
>  
>  /// Config used to parse the fabric part of the running-config
> @@ -231,6 +268,10 @@ pub fn get_routes(
>                  .map(|i| i.name().as_str())
>                  .collect(),
>              ConfigNode::WireGuard(_) => HashSet::new(),
> +            ConfigNode::Bgp(n) => match n.properties() {
> +                BgpNode::Internal(props) => props.interfaces().map(|i| i.name().as_str()).collect(),
> +                BgpNode::External(_) => HashSet::new(),
> +            },
>          };
>  
>          let dummy_interface = format!("dummy_{}", fabric_id.as_str());
> @@ -422,6 +463,62 @@ pub fn get_interfaces_ospf(
>      Ok(stats)
>  }
>  
> +/// Convert the `show bgp neighbors json` output into a list of [`bgp::NeighborStatus`].
> +///
> +/// BGP neighbors are filtered by the fabric's peer-group name (which matches the fabric ID).
> +pub fn get_neighbors_bgp(
> +    fabric_id: FabricId,
> +    neighbors: BTreeMap<String, BgpNeighborInfo>,
> +) -> Result<Vec<bgp::NeighborStatus>, anyhow::Error> {
> +    let mut stats = Vec::new();
> +
> +    for (peer_name, info) in &neighbors {
> +        if info.peer_group.as_deref() == Some(fabric_id.as_str()) {
> +            stats.push(bgp::NeighborStatus {
> +                neighbor: peer_name.clone(),
> +                status: info.bgp_state.clone(),
> +                uptime: info.bgp_timer_up_string.clone().unwrap_or_default(),
> +            });
> +        }
> +    }
> +
> +    Ok(stats)
> +}
> +
> +/// Convert the `show bgp neighbors json` output into a list of [`bgp::InterfaceStatus`].
> +///
> +/// For BGP unnumbered, each interface peer maps to a fabric interface.
> +pub fn get_interfaces_bgp(
> +    fabric_id: FabricId,
> +    neighbors: BTreeMap<String, BgpNeighborInfo>,
> +) -> Result<Vec<bgp::InterfaceStatus>, anyhow::Error> {
> +    let mut stats = Vec::new();
> +
> +    for (peer_name, info) in &neighbors {
> +        if info.peer_group.as_deref() == Some(fabric_id.as_str()) {
> +            stats.push(bgp::InterfaceStatus {
> +                name: peer_name.clone(),
> +                state: if info.bgp_state == "Established" {
> +                    InterfaceState::Up
> +                } else {
> +                    InterfaceState::Down
> +                },
> +            });
> +        }
> +    }
> +
> +    Ok(stats)
> +}
> +
> +/// Minimal BGP neighbor info from `show bgp neighbors json`
> +#[derive(Debug, Deserialize)]
> +#[serde(rename_all = "camelCase")]
> +pub struct BgpNeighborInfo {
> +    pub bgp_state: String,
> +    pub peer_group: Option<String>,
> +    pub bgp_timer_up_string: Option<String>,
> +}
> +
>  /// Get the status for each fabric using the parsed routes from frr
>  ///
>  /// Using the parsed routes we get from frr, filter and map them to a HashMap mapping every
> @@ -444,6 +541,7 @@ pub fn get_status(
>              ConfigNode::Openfabric(_) => (Protocol::Openfabric, &routes.openfabric.0),
>              ConfigNode::Ospf(_) => (Protocol::Ospf, &routes.ospf.0),
>              ConfigNode::WireGuard(_) => (Protocol::WireGuard, &BTreeMap::new()),
> +            ConfigNode::Bgp(_) => (Protocol::Bgp, &routes.bgp.0),
>          };
>  
>          // get interfaces
> @@ -459,6 +557,10 @@ pub fn get_status(
>                  .map(|i| i.name().as_str())
>                  .collect(),
>              ConfigNode::WireGuard(_n) => HashSet::new(),
> +            ConfigNode::Bgp(n) => match n.properties() {
> +                BgpNode::Internal(props) => props.interfaces().map(|i| i.name().as_str()).collect(),
> +                BgpNode::External(_) => HashSet::new(),
> +            },
>          };
>  
>          // determine status by checking if any routes exist for our interfaces





  reply	other threads:[~2026-05-13 12:33 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-12 14:12 [PATCH docs/manager/network/proxmox{-ve-rs,-perl-rs} v4 0/7] sdn: add BGP fabric Hannes Laimer
2026-05-12 14:12 ` [PATCH proxmox-ve-rs v4 1/7] sdn: fabric: add BGP protocol support Hannes Laimer
2026-05-13 12:29   ` Stefan Hanreich
2026-05-13 13:00     ` Hannes Laimer
2026-05-12 14:13 ` [PATCH proxmox-perl-rs v4 2/7] sdn: fabrics: add BGP config generation Hannes Laimer
2026-05-12 14:13 ` [PATCH proxmox-perl-rs v4 3/7] sdn: fabrics: add BGP status endpoints Hannes Laimer
2026-05-13 12:33   ` Stefan Hanreich [this message]
2026-05-13 13:02     ` Hannes Laimer
2026-05-12 14:13 ` [PATCH pve-network v4 4/7] sdn: fabrics: register bgp as a fabric protocol type Hannes Laimer
2026-05-12 14:13 ` [PATCH pve-network v4 5/7] test: evpn: add integration test for EVPN over BGP fabric Hannes Laimer
2026-05-12 14:13 ` [PATCH pve-manager v4 6/7] ui: sdn: add BGP fabric support Hannes Laimer
2026-05-13 12:38   ` Stefan Hanreich
2026-05-12 14:13 ` [PATCH pve-docs v4 7/7] sdn: add bgp fabric section Hannes Laimer
2026-05-13 12:39 ` [PATCH docs/manager/network/proxmox{-ve-rs,-perl-rs} v4 0/7] sdn: add BGP fabric Stefan Hanreich
2026-05-13 18:43 ` superseded: " Hannes Laimer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=03e0b16c-79df-44cc-bf18-bf2e89c4982d@proxmox.com \
    --to=s.hanreich@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal