From: Gabriel Goller <g.goller@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>
Cc: Wolfgang Bumiller <w.bumiller@proxmox.com>,
Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH proxmox-ve-rs 03/11] add intermediate fabric representation
Date: Tue, 4 Mar 2025 18:30:50 +0100 [thread overview]
Message-ID: <5cvfps7jicwcvawb3hgp75276ku3rrj7uiqnlan42cwtppg4qn@j5pbctkkpos6> (raw)
In-Reply-To: <e289a4aa-5948-466a-8d49-1e6ea9280843@proxmox.com>
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).
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply other threads:[~2025-03-04 17:31 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-14 13:39 [pve-devel] [RFC cluster/manager/network/proxmox{-ve-rs, -perl-rs} 00/11] Add SDN Fabrics Gabriel Goller
2025-02-14 13:39 ` [pve-devel] [PATCH proxmox-ve-rs 01/11] add crate with common network types Gabriel Goller
2025-03-03 15:08 ` Stefan Hanreich
2025-03-05 8:28 ` Gabriel Goller
2025-02-14 13:39 ` [pve-devel] [PATCH proxmox-ve-rs 02/11] add proxmox-frr crate with frr types Gabriel Goller
2025-03-03 16:29 ` Stefan Hanreich
2025-03-04 16:28 ` Gabriel Goller
2025-02-14 13:39 ` [pve-devel] [PATCH proxmox-ve-rs 03/11] add intermediate fabric representation Gabriel Goller
2025-02-28 13:57 ` Thomas Lamprecht
2025-02-28 16:19 ` Gabriel Goller
2025-03-04 17:30 ` Gabriel Goller [this message]
2025-03-05 9:03 ` Wolfgang Bumiller
2025-03-04 8:45 ` Stefan Hanreich
2025-03-05 9:09 ` Gabriel Goller
2025-02-14 13:39 ` [pve-devel] [PATCH proxmox-perl-rs 04/11] fabrics: add CRUD and generate fabrics methods Gabriel Goller
2025-03-04 9:28 ` Stefan Hanreich
2025-03-05 10:20 ` Gabriel Goller
2025-02-14 13:39 ` [pve-devel] [PATCH pve-cluster 05/11] cluster: add sdn fabrics config files Gabriel Goller
2025-02-28 12:19 ` Thomas Lamprecht
2025-02-28 12:52 ` Gabriel Goller
2025-02-14 13:39 ` [pve-devel] [PATCH pve-network 06/11] add config file and common read/write methods Gabriel Goller
2025-02-14 13:39 ` [pve-devel] [PATCH pve-network 07/11] merge the frr config with the fabrics frr config on apply Gabriel Goller
2025-02-14 13:39 ` [pve-devel] [PATCH pve-network 08/11] add api endpoints for fabrics Gabriel Goller
2025-03-04 9:51 ` Stefan Hanreich
2025-02-14 13:39 ` [pve-devel] [PATCH pve-manager 09/11] sdn: add Fabrics view Gabriel Goller
2025-03-04 9:57 ` Stefan Hanreich
2025-03-07 15:57 ` Gabriel Goller
2025-02-14 13:39 ` [pve-devel] [PATCH pve-manager 10/11] sdn: add fabric edit/delete forms Gabriel Goller
2025-03-04 10:07 ` Stefan Hanreich
2025-03-07 16:04 ` Gabriel Goller
2025-02-14 13:39 ` [pve-devel] [PATCH pve-manager 11/11] network: return loopback interface on network endpoint Gabriel Goller
2025-03-03 16:58 ` [pve-devel] [RFC cluster/manager/network/proxmox{-ve-rs, -perl-rs} 00/11] Add SDN Fabrics Stefan Hanreich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5cvfps7jicwcvawb3hgp75276ku3rrj7uiqnlan42cwtppg4qn@j5pbctkkpos6 \
--to=g.goller@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=t.lamprecht@proxmox.com \
--cc=w.bumiller@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal