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

[Gabriel Goller]

On 28.02.2025 14:57, Thomas Lamprecht wrote:
>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.

I just spoke again with Stefan and there were some doubts and are
unsure if this is actually useful or just unnecessary abstraction. Some
feedback would be very appreciated!

We planned this intermediate config as a layer between the SectionConfig
(including Vecs, PropertyStrings, etc.) and the FrrConfig, which would
hold Frr-specific stuff such as routers, interfaces, etc..

The two outer layers, so SectionConfig and FrrConfig, are very specific
to their respective config files, so they don't look that nice, nor are
easy to work with.

The intermediate layer acts as a layer above the SectionConfig types
that:
  * Correct hierarchial representation (e.g.: nodes are stored in fabrics)
  * Enforces invariants and allows to include runtime-checks (e.g.:
    BTreeMap doesn't allow duplicate nodes in fabrics, check that
    router-id is unique).
  * Doesn't use section-config-specific types such as `PropertyString`.
  * Allows us to (eventually) switch config file format bit easier and
    makes proxmox-frr easier to isolate (as an independent lib).
  * Would allow us to eventually separate section-config (stored config)
    and running-config. (Intermediate Config could be parsed out of the
    running-config.)

The Intermediate layer is generally written per-protocol as there are
protocol-specific attributes that are difficult to generalize and we
want to use the protocol-specific terminology as well. Nevertheless we
have common types (mostly the simple ones such as Hostname, Net,
RouterId), that are stored in the common proxmox-network-types crate and
get used by all of the layers and protocols.

                         ┌───────────────┐
                         │ SectionConfig │
                         └───────┬───────┘
                                 │
           ┌─────────────────────┼─────────────────────┐
           ▼                     ▼                     ▼
┌──────────────────────────────────────────────────────────┐
│                                                          │ SectionConfig-specific types
│  OspfSectionConfig    OpenFabricSectionConfig      ...   │ (proxmox-ve-config::sdn::fabric
│         │                     │                     │    │  ::openfabric::OpenFabricSectionConfig)
└─────────┼─────────────────────┼─────────────────────┼────┘
           │                     │                     │
           │                     │                     │
┌─────────┼─────────────────────┼─────────────────────┼────┐
│         ▼                     ▼                     ▼    │ Intermediate Representation
│  OspfConfig           OpenFabricConfig             ...   │ (proxmox-ve-config::sdn::fabric
│         │                     │                     │    │  ::openfabric::internal::OpenFabricConfig)
└─────────┼─────────────────────┼─────────────────────┼────┘
           │                     │                     │
           │                     │                     │
┌─────────┼─────────────────────┼─────────────────────┼───┐
│         ▼                     ▼                     ▼   │
│  OspfRouter           OpenFabricRouter             ...  │ Frr-representation
│  OspfInterface        OpenFabricInterface               │ (proxmox-frr::openfabric::OpenFabricRouter)
│                                                         │
└─────────────────────────────────────────────────────────┘


The other, simpler option would be to parse directly from the
SectionConfig to the FrrConfig. This would greatly reduce the amount of
code needed and deduplicate lots of options/parameters. Downside is that
semantic checking would be hard to do (not that we do a lot there, but
anyway) and proxmox-frr would be kinda weird including SectionConfig
types (gated by feature-flags, but still).