Re: [pve-devel] [PATCH proxmox-ve-rs v4 06/22] frr: add openfabric types

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Wed, Jul 02, 2025 at 04:49:57PM +0200, Gabriel Goller wrote:
> Implement OpenFabric-specific variants of common enums that
> encapsulate protocol properties defined in proxmox-network-types. The
> primary addition is OpenFabricInterface, which stores
> protocol-specific timing parameters: HelloInterval (neighbor discovery
> frequency), CsnpInterval (database synchronization frequency), and
> HelloMultiplier (neighbor failure detection). Added `is_ipv6` flag to
> support FRR's command prefixing requirements during serialization for
> IPv6-specific commands (we need to add a 'ipv6' prefix to some
> commands).
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  proxmox-frr/debian/control    |   2 +
>  proxmox-frr/src/lib.rs        |   1 +
>  proxmox-frr/src/openfabric.rs | 114 ++++++++++++++++++++++++++++++++++
>  3 files changed, 117 insertions(+)
>  create mode 100644 proxmox-frr/src/openfabric.rs
> 
> diff --git a/proxmox-frr/debian/control b/proxmox-frr/debian/control
> index 07b4fbe87629..894bfeac8a65 100644
> --- a/proxmox-frr/debian/control
> +++ b/proxmox-frr/debian/control
> @@ -9,6 +9,7 @@ Build-Depends-Arch: cargo:native <!nocheck>,
>   librust-anyhow-1+default-dev <!nocheck>,
>   librust-itoa-1+default-dev (>= 1.0.9-~~) <!nocheck>,
>   librust-proxmox-network-types-0.1+default-dev <!nocheck>,
> + librust-proxmox-sdn-types-0.1+default-dev <!nocheck>,
>   librust-serde-1+default-dev <!nocheck>,
>   librust-serde-1+derive-dev <!nocheck>,
>   librust-serde-with-3+default-dev <!nocheck>,
> @@ -30,6 +31,7 @@ Depends:
>   librust-anyhow-1+default-dev,
>   librust-itoa-1+default-dev (>= 1.0.9-~~),
>   librust-proxmox-network-types-0.1+default-dev,
> + librust-proxmox-sdn-types-0.1+default-dev,
>   librust-serde-1+default-dev,
>   librust-serde-1+derive-dev,
>   librust-serde-with-3+default-dev,
> diff --git a/proxmox-frr/src/lib.rs b/proxmox-frr/src/lib.rs
> index 5e0b34602cf4..ba9eedfb4549 100644
> --- a/proxmox-frr/src/lib.rs
> +++ b/proxmox-frr/src/lib.rs
> @@ -1,3 +1,4 @@
> +pub mod openfabric;
>  use std::{fmt::Display, str::FromStr};
>  
>  use serde::{Deserialize, Serialize};
> diff --git a/proxmox-frr/src/openfabric.rs b/proxmox-frr/src/openfabric.rs
> new file mode 100644
> index 000000000000..91c18dc27254
> --- /dev/null
> +++ b/proxmox-frr/src/openfabric.rs
> @@ -0,0 +1,114 @@
> +use std::fmt::Debug;
> +use std::fmt::Display;
> +
> +use proxmox_sdn_types::net::Net;
> +use serde::{Deserialize, Serialize};
> +use serde_with::SerializeDisplay;
> +
> +use thiserror::Error;
> +
> +use crate::FrrWord;
> +use crate::FrrWordError;
> +
> +/// The name of a OpenFabric router. Is an FrrWord.
> +#[derive(Clone, Debug, PartialEq, Eq, Hash, Deserialize, SerializeDisplay, PartialOrd, Ord)]
> +pub struct OpenfabricRouterName(FrrWord);
> +
> +impl From<FrrWord> for OpenfabricRouterName {
> +    fn from(value: FrrWord) -> Self {
> +        Self(value)
> +    }
> +}
> +
> +impl OpenfabricRouterName {
> +    pub fn new(name: FrrWord) -> Self {
> +        Self(name)
> +    }
> +}
> +
> +impl Display for OpenfabricRouterName {
> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> +        write!(f, "openfabric {}", self.0)
> +    }
> +}
> +
> +/// All the properties a OpenFabric router can hold.
> +///
> +/// These can serialized with a " " space prefix as they are in the `router openfabric` block.
> +#[derive(Clone, Debug, PartialEq, Eq, Hash, Deserialize, Serialize, PartialOrd, Ord)]
> +pub struct OpenfabricRouter {
> +    /// The NET address
> +    pub net: Net,
> +}
> +
> +impl OpenfabricRouter {
> +    pub fn new(net: Net) -> Self {
> +        Self { net }
> +    }
> +
> +    pub fn net(&self) -> &Net {
> +        &self.net
> +    }
> +}
> +
> +/// The OpenFabric properties.
> +///
> +/// This struct holds all the OpenFabric interface properties. The most important one here is the
> +/// fabric_id, which ties the interface to a fabric. When serialized these properties all get
> +/// prefixed with a space (" ") as they are inside the interface block. They serialize roughly to:
> +///
> +/// ```text
> +/// interface ens20
> +///  ip router openfabric <fabric_id>
> +///  ipv6 router openfabric <fabric_id>
> +///  openfabric hello-interval <value>
> +///  openfabric hello-multiplier <value>
> +///  openfabric csnp-interval <value>
> +///  openfabric passive <value>
> +/// ```
> +///
> +/// The is_ipv4 and is_ipv6 properties decide if we need to add `ip router openfabric`, `ipv6
> +/// router openfabric`, or both. A interface can only be part of a single fabric.

An*

> +#[derive(Clone, Debug, PartialEq, Eq, Hash, Deserialize, Serialize, PartialOrd, Ord)]
> +pub struct OpenfabricInterface {
> +    // Note: an interface can only be a part of a single fabric (so no vec needed here)
> +    pub fabric_id: OpenfabricRouterName,
> +    pub passive: Option<bool>,

^ So in  `FrrSerializer` we do all this manually - but for the derived
`Serialize` implementation, shouldn't we have a
`#[serde(skip_serializing_if = "Option::is_none")` here?

(and probably also down below...)

> +    pub hello_interval: Option<proxmox_sdn_types::openfabric::HelloInterval>,
> +    pub csnp_interval: Option<proxmox_sdn_types::openfabric::CsnpInterval>,
> +    pub hello_multiplier: Option<proxmox_sdn_types::openfabric::HelloMultiplier>,
> +    pub is_ipv4: bool,
> +    pub is_ipv6: bool,
> +}
> +
> +impl OpenfabricInterface {

If the struct and its fields are all `pub` - why do we need/want getters?

> +    pub fn fabric_id(&self) -> &OpenfabricRouterName {
> +        &self.fabric_id
> +    }
> +    pub fn passive(&self) -> Option<bool> {
> +        self.passive
> +    }
> +    pub fn hello_interval(&self) -> Option<proxmox_sdn_types::openfabric::HelloInterval> {
> +        self.hello_interval
> +    }
> +    pub fn csnp_interval(&self) -> Option<proxmox_sdn_types::openfabric::CsnpInterval> {
> +        self.csnp_interval
> +    }
> +    pub fn hello_multiplier(&self) -> Option<proxmox_sdn_types::openfabric::HelloMultiplier> {
> +        self.hello_multiplier
> +    }
> +    pub fn set_hello_interval(
> +        &mut self,
> +        interval: impl Into<Option<proxmox_sdn_types::openfabric::HelloInterval>>,
> +    ) {
> +        self.hello_interval = interval.into();
> +    }
> +}
> +
> +#[derive(Error, Debug)]
> +pub enum OpenfabricInterfaceError {
> +    #[error("Unknown error converting to OpenFabricInterface")]
> +    UnknownError,
> +    #[error("Error parsing frr word")]
> +    FrrWordParse(#[from] FrrWordError),
> +}
> -- 
> 2.39.5


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