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

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

On Wed, Jul 02, 2025 at 04:49:56PM +0200, Gabriel Goller wrote:
> Add common FRR configuration types such as FrrWord,
> CommonInterfaceName, etc. These are some common types that are used by
> both openfabric and ospf and the generic types that span the two
> protocols. The FrrWord is a simple primitive in FRR, which is a
> ascii-string that doesn't contain whitespaces.
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  proxmox-frr/src/lib.rs | 118 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 118 insertions(+)
> 
> diff --git a/proxmox-frr/src/lib.rs b/proxmox-frr/src/lib.rs
> index e69de29bb2d1..5e0b34602cf4 100644
> --- a/proxmox-frr/src/lib.rs
> +++ b/proxmox-frr/src/lib.rs
> @@ -0,0 +1,118 @@
> +use std::{fmt::Display, str::FromStr};
> +
> +use serde::{Deserialize, Serialize};
> +use serde_with::{DeserializeFromStr, SerializeDisplay};
> +use thiserror::Error;
> +
> +#[derive(Error, Debug)]
> +pub enum RouterNameError {
> +    #[error("invalid name")]
> +    InvalidName,
> +    #[error("invalid frr word")]
> +    FrrWordError(#[from] FrrWordError),
> +}
> +
> +/// The interface name is the same on ospf and openfabric, but it is an enum so we can have two
> +/// different entries in the hashmap. This allows us to have an interface in an ospf and openfabric
> +/// fabric.
> +#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, Hash, PartialOrd, Ord)]
> +pub enum InterfaceName {
> +    Openfabric(CommonInterfaceName),
> +    Ospf(CommonInterfaceName),
> +}
> +
> +impl Display for InterfaceName {
> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> +        match self {
> +            InterfaceName::Openfabric(frr_word) => frr_word.fmt(f),
> +            InterfaceName::Ospf(frr_word) => frr_word.fmt(f),
> +        }
> +    }
> +}
> +
> +#[derive(Error, Debug)]
> +pub enum FrrWordError {
> +    #[error("word is empty")]
> +    IsEmpty,
> +    #[error("word contains invalid character")]
> +    InvalidCharacter,
> +}
> +
> +/// A simple FRR Word.
> +///
> +/// Every string argument or value in FRR is an FrrWord. FrrWords must only contain ascii
> +/// characters and must not have a whitespace.
> +#[derive(
> +    Clone, Debug, PartialEq, Eq, Hash, DeserializeFromStr, SerializeDisplay, PartialOrd, Ord,
> +)]
> +pub struct FrrWord(String);
> +
> +impl FrrWord {
> +    pub fn new(name: String) -> Result<Self, FrrWordError> {
> +        if name.is_empty() {
> +            return Err(FrrWordError::IsEmpty);
> +        }
> +
> +        if name
> +            .as_bytes()
> +            .iter()
> +            .any(|c| !c.is_ascii() || c.is_ascii_whitespace())
> +        {
> +            eprintln!("invalid char in: \"{name}\"");
> +            return Err(FrrWordError::InvalidCharacter);
> +        }
> +
> +        Ok(Self(name))
> +    }
> +}
> +
> +impl FromStr for FrrWord {
> +    type Err = FrrWordError;
> +
> +    fn from_str(s: &str) -> Result<Self, Self::Err> {
> +        FrrWord::new(s.to_string())

^ Let's try to avoid allocating before error-checking. We could move the
check out of `new()` into a helper, call that, then just build
`Self(s.to_string())`.

> +    }
> +}
> +
> +impl Display for FrrWord {
> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> +        self.0.fmt(f)
> +    }
> +}
> +
> +impl AsRef<str> for FrrWord {
> +    fn as_ref(&self) -> &str {
> +        &self.0
> +    }
> +}
> +
> +#[derive(Error, Debug)]
> +pub enum CommonInterfaceNameError {
> +    #[error("interface name too long")]
> +    TooLong,
> +}
> +
> +/// Name of a interface, which is common between all protocols.
> +///
> +/// FRR itself doesn't enforce any limits, but the kernel does. Linux only allows interface names
> +/// to be a maximum of 16 bytes. This is enforced by this struct.
> +#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, Hash, PartialOrd, Ord)]
> +pub struct CommonInterfaceName(String);
> +
> +impl FromStr for CommonInterfaceName {

^ `FromStr` is not in the prelude.
Via prelude this only provides `.parse()`, but we don't really "parse
it".
IMO this could also be a `new()`, and  have `TryFrom<String>` and `TryFrom<&str>`.
We can make `fn new<T: AsRef<str> + Into<String>>(T)` to be able to
run the check before any potential allocation...

> +    type Err = CommonInterfaceNameError;
> +
> +    fn from_str(s: &str) -> Result<Self, Self::Err> {
> +        if s.len() <= 15 {
> +            Ok(Self(s.to_owned()))
> +        } else {
> +            Err(CommonInterfaceNameError::TooLong)
> +        }
> +    }
> +}
> +
> +impl Display for CommonInterfaceName {
> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> +        self.0.fmt(f)
> +    }
> +}
> -- 
> 2.39.5


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