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

[Gabriel Goller <g.goller@proxmox.com>]

On 07.07.2025 13:28, Wolfgang Bumiller wrote:
>On Wed, Jul 02, 2025 at 04:49:58PM +0200, Gabriel Goller wrote:
>> [snip]
>> +/// The NetworkType of the interface.
>> +///
>> +/// The most important options here are Broadcast (which is the default) and PointToPoint.
>> +/// When PointToPoint is set, then the interface has to have a /32 address and will be treated as
>> +/// unnumbered.
>> +///
>> +/// This roughly serializes to:
>> +/// ```text
>> +/// ip ospf network point-to-point
>> +/// ! or
>> +/// ip ospf network broadcast
>> +/// ```
>> +#[derive(Clone, Debug, PartialEq, Eq, Hash, Deserialize, Serialize, PartialOrd, Ord)]
>> +pub enum NetworkType {
>> +    Broadcast,
>> +    NonBroadcast,
>> +    /// If the interface is unnumbered (i.e. the router-id /32 ip-address is set on the interface).
>> +    ///
>> +    /// If OSPF is used in an unnumbered way, you don't need to configure peer-to-peer (e.g. /31)
>> +    /// addresses at every interface, but you just need to set the router-id at the interface
>> +    /// (/32). You also need to configure the `ip ospf network point-to-point` FRR option.
>> +    PointToPoint,
>> +    PointToMultipoint,
>
>Will there be larger entries? Could consider `Copy` here maybe.

Nope, will add Copy.

>> +}
>> +
>> +impl Display for NetworkType {
>> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
>> +        match self {
>> +            NetworkType::Broadcast => write!(f, "broadcast"),
>> +            NetworkType::NonBroadcast => write!(f, "non-broadcast"),
>> +            NetworkType::PointToPoint => write!(f, "point-to-point"),
>
>(I see a `t` where some people don't use a `t` 🤪 /hj)

Thanks I hate it -_-

>> +            NetworkType::PointToMultipoint => write!(f, "point-to-multicast"),
>> +        }
>> +    }
>> +}
>> +
>> +/// The OSPF interface properties.
>> +///
>> +/// The interface gets tied to its fabric by the area property and the FRR `ip ospf area <area>`
>> +/// command.
>> +///
>> +/// This serializes to:
>> +///
>> +/// ```text
>> +/// router ospf
>> +///  ip ospf area <area>
>> +///  ip ospf passive <value>
>> +///  ip ospf network <value>
>> +/// ```
>> +#[derive(Clone, Debug, PartialEq, Eq, Hash, Deserialize, Serialize, PartialOrd, Ord)]
>> +pub struct OspfInterface {
>> +    // Note: an interface can only be a part of a single area(so no vec needed here)
>> +    pub area: Area,
>> +    pub passive: Option<bool>,
>> +    pub network_type: Option<NetworkType>,
>> +}
>> +
>> +impl OspfInterface {
>> +    pub fn area(&self) -> &Area {
>> +        &self.area
>> +    }
>> +    pub fn passive(&self) -> &Option<bool> {
>> +        &self.passive
>> +    }
>> +    pub fn network_type(&self) -> &Option<NetworkType> {
>> +        &self.network_type
>> +    }
>
>^ like in the previous patch - pub fields vs getters

Yep, removed it.

Thanks for the review!


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