On 07.07.2025 13:25, Wolfgang Bumiller wrote:
On Wed, Jul 02, 2025 at 04:49:57PM +0200, Gabriel Goller wrote:[snip] +/// 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*
writing skills of a 4th-grader -_- thanks!
+#[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...)
Umm actually we don't need serde (and Serialize, Deserialize) in proxmox-frr at all. My first tests where with a serde serializer, which didn't really work out and I forgot to remove serde :( Removed serde and all the Serialize/Deserialize derives.
+ 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?
No reason. Removed all the setters/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(); + } +} + [snip]
Thanks for the review! _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
