[pve-devel] [PATCH proxmox-ve-rs 03/11] add intermediate fabric representation

[Thomas Lamprecht]

Am 14.02.25 um 14:39 schrieb Gabriel Goller:
> This adds the intermediate, type-checked fabrics config. This one is
> parsed from the SectionConfig and can be converted into the
> Frr-Representation.

The short description of the patch is good, but I would like to see more
rationale here about choosing this way, like benefits and trade-offs to other
options that got evaluated, if this can/will be generic for all fabrics planned,
..., and definitively some more rust-documentation for public types and modules.

One thing I noticed below, I did not managed to do a thorough review besides
of that yet though.

> 
> Co-authored-by: Stefan Hanreich <s.hanreich at proxmox.com>
> Signed-off-by: Gabriel Goller <g.goller at proxmox.com>
> ---
>  proxmox-ve-config/Cargo.toml                  |  10 +-
>  proxmox-ve-config/debian/control              |   4 +-
>  proxmox-ve-config/src/sdn/fabric/common.rs    |  90 ++++
>  proxmox-ve-config/src/sdn/fabric/mod.rs       |  68 +++
>  .../src/sdn/fabric/openfabric.rs              | 494 ++++++++++++++++++
>  proxmox-ve-config/src/sdn/fabric/ospf.rs      | 375 +++++++++++++
>  proxmox-ve-config/src/sdn/mod.rs              |   1 +
>  7 files changed, 1036 insertions(+), 6 deletions(-)
>  create mode 100644 proxmox-ve-config/src/sdn/fabric/common.rs
>  create mode 100644 proxmox-ve-config/src/sdn/fabric/mod.rs
>  create mode 100644 proxmox-ve-config/src/sdn/fabric/openfabric.rs
>  create mode 100644 proxmox-ve-config/src/sdn/fabric/ospf.rs
> 

> diff --git a/proxmox-ve-config/src/sdn/fabric/common.rs b/proxmox-ve-config/src/sdn/fabric/common.rs
> new file mode 100644
> index 000000000000..400f5b6d6b12
> --- /dev/null
> +++ b/proxmox-ve-config/src/sdn/fabric/common.rs
> @@ -0,0 +1,90 @@
> +use serde::{Deserialize, Serialize};
> +use std::fmt::Display;
> +use thiserror::Error;
> +
> +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Error)]
> +pub enum ConfigError {
> +    #[error("node id has invalid format")]
> +    InvalidNodeId,
> +}
> +
> +#[derive(Debug, Deserialize, Serialize, Clone, Eq, Hash, PartialOrd, Ord, PartialEq)]
> +pub struct Hostname(String);
> +
> +impl From<String> for Hostname {
> +    fn from(value: String) -> Self {
> +        Hostname::new(value)
> +    }
> +}
> +
> +impl AsRef<str> for Hostname {
> +    fn as_ref(&self) -> &str {
> +        &self.0
> +    }
> +}
> +
> +impl Display for Hostname {
> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> +        self.0.fmt(f)
> +    }
> +}
> +
> +impl Hostname {
> +    pub fn new(name: impl Into<String>) -> Hostname {
> +        Self(name.into())
> +    }
> +}
> +
> +// parses a bool from a string OR bool
> +pub mod serde_option_bool {

might be maybe something to put in proxmox-serde?