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 917EF1FF13A for ; Wed, 13 May 2026 14:33:44 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 609B0C2AE; Wed, 13 May 2026 14:33:42 +0200 (CEST) Message-ID: <03e0b16c-79df-44cc-bf18-bf2e89c4982d@proxmox.com> Date: Wed, 13 May 2026 14:33:33 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox-perl-rs v4 3/7] sdn: fabrics: add BGP status endpoints To: pve-devel@lists.proxmox.com References: <20260512141305.199664-1-h.laimer@proxmox.com> <20260512141305.199664-4-h.laimer@proxmox.com> Content-Language: en-US From: Stefan Hanreich In-Reply-To: <20260512141305.199664-4-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.611 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 Message-ID-Hash: JZBFDEVB7CHQXX7W7YG7DAVISV7QUXBN X-Message-ID-Hash: JZBFDEVB7CHQXX7W7YG7DAVISV7QUXBN X-MailFrom: s.hanreich@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 > --- > 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 = > + 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 = > + 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), > Ospf(Vec), > WireGuard(Vec), > + Bgp(Vec), > } > > impl From> for NeighborStatus { > @@ -108,14 +130,20 @@ impl From> for NeighborStatus { > NeighborStatus::Ospf(value) > } > } > +impl From> for NeighborStatus { > + fn from(value: Vec) -> 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), > Ospf(Vec), > WireGuard(Vec), > + Bgp(Vec), > } > > impl From> for InterfaceStatus { > @@ -128,6 +156,11 @@ impl From> for InterfaceStatus { > InterfaceStatus::Ospf(value) > } > } > +impl From> for InterfaceStatus { > + fn from(value: Vec) -> 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, > +) -> Result, 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, > +) -> Result, 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, > + pub bgp_timer_up_string: Option, > +} > + > /// 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