public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH proxmox 1/1] network-types: add method to canonicalize IPv4 and IPv6 CIDRs
@ 2025-07-23 14:31 Gabriel Goller
  2025-07-23 14:32 ` [pve-devel] [PATCH proxmox-ve-rs 1/1] ve-config: fabrics: force ip-prefix to be canonical Gabriel Goller
  2025-07-30 12:00 ` [pve-devel] applied: [PATCH proxmox 1/1] network-types: add method to canonicalize IPv4 and IPv6 CIDRs Thomas Lamprecht
  0 siblings, 2 replies; 3+ messages in thread
From: Gabriel Goller @ 2025-07-23 14:31 UTC (permalink / raw)
  To: pve-devel

When a cidr address in a FRR access-list is not canonicalized (i.e. is
not a network address) then we get a warning in the journal and
frr-reload.py will even fail. So we need to convert the address entered
by the user into a network address. Factor out the already existing
helper and add a new method to do this. Also add some unit tests.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-network-types/src/ip_address.rs | 209 ++++++++++++++++++++++--
 1 file changed, 195 insertions(+), 14 deletions(-)

diff --git a/proxmox-network-types/src/ip_address.rs b/proxmox-network-types/src/ip_address.rs
index b91ede445070..1c72197534ce 100644
--- a/proxmox-network-types/src/ip_address.rs
+++ b/proxmox-network-types/src/ip_address.rs
@@ -322,6 +322,15 @@ impl Ipv4Cidr {
         self.mask
     }
 
+    /// Get the canonical representation of a IPv4 CIDR address.
+    ///
+    /// This normalizes the address, so we get the first address of a CIDR subnet (e.g.
+    /// 2.2.2.200/24 -> 2.2.2.0) we do this by using a bitwise AND operation over the address and
+    /// the u32::MAX (all ones) shifted by the mask.
+    fn normalize(addr: u32, mask: u8) -> u32 {
+        addr & u32::MAX.checked_shl((32 - mask).into()).unwrap_or(0)
+    }
+
     /// Checks if the two CIDRs overlap.
     ///
     /// CIDRs are always disjoint so we only need to check if one CIDR contains
@@ -329,14 +338,20 @@ impl Ipv4Cidr {
     pub fn overlaps(&self, other: &Ipv4Cidr) -> bool {
         // we normalize by the smallest mask, so the larger of the two subnets.
         let min_mask = self.mask().min(other.mask());
-        // this normalizes the address, so we get the first address of a CIDR
-        // (e.g. 2.2.2.200/24 -> 2.2.2.0) we do this by using a bitwise AND
-        // operation over the address and the u32::MAX (all ones) shifted by
-        // the mask.
-        let normalize =
-            |addr: u32| addr & u32::MAX.checked_shl((32 - min_mask).into()).unwrap_or(0);
         // if the prefix is the same we have an overlap
-        normalize(self.address().to_bits()) == normalize(other.address().to_bits())
+        Self::normalize(self.address().to_bits(), min_mask)
+            == Self::normalize(other.address().to_bits(), min_mask)
+    }
+
+    /// Get the canonical version of the CIDR.
+    ///
+    /// A canonicalized CIDR is a the normalized address, so the first address in the subnet
+    /// (sometimes also called "network address"). E.g. 2.2.2.5/24 -> 2.2.2.0/24
+    pub fn canonical(&self) -> Self {
+        Self {
+            addr: Ipv4Addr::from_bits(Self::normalize(self.addr.to_bits(), self.mask())),
+            mask: self.mask(),
+        }
     }
 }
 
@@ -424,6 +439,15 @@ impl Ipv6Cidr {
         self.mask
     }
 
+    /// Get the canonical representation of a IPv6 CIDR address.
+    ///
+    /// This normalizes the address, so we get the first address of a CIDR subnet (e.g.
+    /// 2001:db8::4/64 -> 2001:db8::0/64) we do this by using a bitwise AND operation over the address and
+    /// the u128::MAX (all ones) shifted by the mask.
+    fn normalize(addr: u128, mask: u8) -> u128 {
+        addr & u128::MAX.checked_shl((128 - mask).into()).unwrap_or(0)
+    }
+
     /// Checks if the two CIDRs overlap.
     ///
     /// CIDRs are always disjoint so we only need to check if one CIDR contains
@@ -431,14 +455,20 @@ impl Ipv6Cidr {
     pub fn overlaps(&self, other: &Ipv6Cidr) -> bool {
         // we normalize by the smallest mask, so the larger of the two subnets.
         let min_mask = self.mask().min(other.mask());
-        // this normalizes the address, so we get the first address of a CIDR
-        // (e.g. 2001:db8::200/64 -> 2001:db8::0) we do this by using a bitwise AND
-        // operation over the address and the u128::MAX (all ones) shifted by
-        // the mask.
-        let normalize =
-            |addr: u128| addr & u128::MAX.checked_shl((128 - min_mask).into()).unwrap_or(0);
         // if the prefix is the same we have an overlap
-        normalize(self.address().to_bits()) == normalize(other.address().to_bits())
+        Self::normalize(self.address().to_bits(), min_mask)
+            == Self::normalize(other.address().to_bits(), min_mask)
+    }
+
+    /// Get the canonical version of the CIDR.
+    ///
+    /// A canonicalized CIDR is a the normalized address, so the first address in the subnet
+    /// (sometimes also called "network address"). E.g. 2001:db8::5/64 -> 2001:db8::0/64
+    pub fn canonical(&self) -> Self {
+        Self {
+            addr: Ipv6Addr::from_bits(Self::normalize(self.addr.to_bits(), self.mask())),
+            mask: self.mask(),
+        }
     }
 }
 
@@ -1855,4 +1885,155 @@ mod tests {
                 )
         );
     }
+
+    #[test]
+    fn test_ipv4_canonical() {
+        let cidr = Ipv4Cidr::new("192.168.1.100".parse::<Ipv4Addr>().unwrap(), 24).unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(canonical.addr, Ipv4Addr::new(192, 168, 1, 0));
+        assert_eq!(canonical.mask, 24);
+
+        let cidr = Ipv4Cidr::new("10.50.75.200".parse::<Ipv4Addr>().unwrap(), 16).unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(canonical.addr, Ipv4Addr::new(10, 50, 0, 0));
+        assert_eq!(canonical.mask, 16);
+
+        let cidr = Ipv4Cidr::new("172.16.100.50".parse::<Ipv4Addr>().unwrap(), 8).unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(canonical.addr, Ipv4Addr::new(172, 0, 0, 0));
+        assert_eq!(canonical.mask, 8);
+
+        let cidr = Ipv4Cidr::new("192.168.1.1".parse::<Ipv4Addr>().unwrap(), 32).unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(canonical.addr, Ipv4Addr::new(192, 168, 1, 1));
+        assert_eq!(canonical.mask, 32);
+
+        let cidr = Ipv4Cidr::new("255.255.255.255".parse::<Ipv4Addr>().unwrap(), 0).unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(canonical.addr, Ipv4Addr::new(0, 0, 0, 0));
+        assert_eq!(canonical.mask, 0);
+
+        let cidr = Ipv4Cidr::new("192.168.1.103".parse::<Ipv4Addr>().unwrap(), 30).unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(canonical.addr, Ipv4Addr::new(192, 168, 1, 100));
+        assert_eq!(canonical.mask, 30);
+
+        let cidr = Ipv4Cidr::new("10.10.15.128".parse::<Ipv4Addr>().unwrap(), 23).unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(canonical.addr, Ipv4Addr::new(10, 10, 14, 0));
+        assert_eq!(canonical.mask, 23);
+
+        let cidr = Ipv4Cidr::new("203.0.113.99".parse::<Ipv4Addr>().unwrap(), 25).unwrap();
+        let canonical1 = cidr.canonical();
+        let canonical2 = canonical1.canonical();
+        assert_eq!(canonical1.addr, canonical2.addr);
+        assert_eq!(canonical1.mask, canonical2.mask);
+    }
+
+    #[test]
+    fn test_ipv6_canonical() {
+        let cidr = Ipv6Cidr::new(
+            "2001:db8:85a3::8a2e:370:7334".parse::<Ipv6Addr>().unwrap(),
+            64,
+        )
+        .unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(
+            canonical.addr,
+            Ipv6Addr::new(0x2001, 0xdb8, 0x85a3, 0, 0, 0, 0, 0)
+        );
+        assert_eq!(canonical.mask, 64);
+
+        let cidr = Ipv6Cidr::new(
+            "2001:db8:1234:5678:9abc:def0:1234:5678"
+                .parse::<Ipv6Addr>()
+                .unwrap(),
+            48,
+        )
+        .unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(
+            canonical.addr,
+            Ipv6Addr::new(0x2001, 0xdb8, 0x1234, 0, 0, 0, 0, 0)
+        );
+        assert_eq!(canonical.mask, 48);
+
+        let cidr = Ipv6Cidr::new(
+            "2001:db8:abcd:ef01:2345:6789:abcd:ef01"
+                .parse::<Ipv6Addr>()
+                .unwrap(),
+            32,
+        )
+        .unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(
+            canonical.addr,
+            Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 0)
+        );
+        assert_eq!(canonical.mask, 32);
+
+        let cidr = Ipv6Cidr::new("2001:db8::1".parse::<Ipv6Addr>().unwrap(), 128).unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(
+            canonical.addr,
+            Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1)
+        );
+        assert_eq!(canonical.mask, 128);
+
+        let cidr = Ipv6Cidr::new(
+            "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"
+                .parse::<Ipv6Addr>()
+                .unwrap(),
+            0,
+        )
+        .unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(canonical.addr, Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0));
+        assert_eq!(canonical.mask, 0);
+
+        let cidr = Ipv6Cidr::new(
+            "2001:db8:1234:5600:ffff:ffff:ffff:ffff"
+                .parse::<Ipv6Addr>()
+                .unwrap(),
+            56,
+        )
+        .unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(
+            canonical.addr,
+            Ipv6Addr::new(0x2001, 0xdb8, 0x1234, 0x5600, 0, 0, 0, 0)
+        );
+        assert_eq!(canonical.mask, 56);
+
+        let cidr = Ipv6Cidr::new(
+            "2001:db8:1234:5678:9abc:def0:ffff:ffff"
+                .parse::<Ipv6Addr>()
+                .unwrap(),
+            96,
+        )
+        .unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(
+            canonical.addr,
+            Ipv6Addr::new(0x2001, 0xdb8, 0x1234, 0x5678, 0x9abc, 0xdef0, 0, 0)
+        );
+        assert_eq!(canonical.mask, 96);
+
+        let cidr = Ipv6Cidr::new(
+            "2001:db8:cafe:face:dead:beef:1234:5678"
+                .parse::<Ipv6Addr>()
+                .unwrap(),
+            80,
+        )
+        .unwrap();
+        let canonical1 = cidr.canonical();
+        let canonical2 = canonical1.canonical();
+        assert_eq!(canonical1.addr, canonical2.addr);
+        assert_eq!(canonical1.mask, canonical2.mask);
+
+        let cidr = Ipv6Cidr::new("fe80::1".parse::<Ipv6Addr>().unwrap(), 64).unwrap();
+        let canonical = cidr.canonical();
+        assert_eq!(canonical.addr, Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 0));
+        assert_eq!(canonical.mask, 64);
+    }
 }
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pve-devel] [PATCH proxmox-ve-rs 1/1] ve-config: fabrics: force ip-prefix to be canonical
  2025-07-23 14:31 [pve-devel] [PATCH proxmox 1/1] network-types: add method to canonicalize IPv4 and IPv6 CIDRs Gabriel Goller
@ 2025-07-23 14:32 ` Gabriel Goller
  2025-07-30 12:00 ` [pve-devel] applied: [PATCH proxmox 1/1] network-types: add method to canonicalize IPv4 and IPv6 CIDRs Thomas Lamprecht
  1 sibling, 0 replies; 3+ messages in thread
From: Gabriel Goller @ 2025-07-23 14:32 UTC (permalink / raw)
  To: pve-devel

FRR spits out a warning if the address of the access-list is not
canonicalized (i.e. not a network address). So when the user creates a new
fabric, just store the canonicalized version of the passed ip-prefix.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-ve-config/src/sdn/fabric/mod.rs       |  9 ++++++++-
 .../src/sdn/fabric/section_config/fabric.rs   | 20 +++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/proxmox-ve-config/src/sdn/fabric/mod.rs b/proxmox-ve-config/src/sdn/fabric/mod.rs
index 9ec2f615c8f1..58a06f9423cb 100644
--- a/proxmox-ve-config/src/sdn/fabric/mod.rs
+++ b/proxmox-ve-config/src/sdn/fabric/mod.rs
@@ -587,11 +587,18 @@ impl FabricConfig {
     /// Add a fabric to the [`FabricConfig`].
     ///
     /// Returns an error if a fabric with the same name exists.
-    pub fn add_fabric(&mut self, fabric: Fabric) -> Result<(), FabricConfigError> {
+    pub fn add_fabric(&mut self, mut fabric: Fabric) -> Result<(), FabricConfigError> {
         if self.fabrics.contains_key(fabric.id()) {
             return Err(FabricConfigError::DuplicateFabric(fabric.id().to_string()));
         }
 
+        if let Some(prefix) = fabric.ip_prefix() {
+            fabric.set_ip_prefix(prefix.canonical());
+        }
+        if let Some(prefix) = fabric.ip6_prefix() {
+            fabric.set_ip6_prefix(prefix.canonical());
+        }
+
         self.fabrics.insert(fabric.id().clone(), fabric.into());
 
         Ok(())
diff --git a/proxmox-ve-config/src/sdn/fabric/section_config/fabric.rs b/proxmox-ve-config/src/sdn/fabric/section_config/fabric.rs
index d41c6d82fd88..38911a624740 100644
--- a/proxmox-ve-config/src/sdn/fabric/section_config/fabric.rs
+++ b/proxmox-ve-config/src/sdn/fabric/section_config/fabric.rs
@@ -186,6 +186,16 @@ impl Fabric {
         }
     }
 
+    /// Set the ip-prefix (IPv4 CIDR) of the [`Fabric`].
+    ///
+    /// This is a common property for all protocols.
+    pub fn set_ip_prefix(&mut self, ipv4_cidr: Ipv4Cidr) {
+        match self {
+            Fabric::Openfabric(fabric_section) => fabric_section.ip_prefix = Some(ipv4_cidr),
+            Fabric::Ospf(fabric_section) => fabric_section.ip_prefix = Some(ipv4_cidr),
+        }
+    }
+
     /// Get the ip6-prefix (IPv6 CIDR) of the [`Fabric`].
     ///
     /// This is a common property for all protocols.
@@ -195,6 +205,16 @@ impl Fabric {
             Fabric::Ospf(fabric_section) => fabric_section.ip6_prefix(),
         }
     }
+
+    /// Set the ip6-prefix (IPv6 CIDR) of the [`Fabric`].
+    ///
+    /// This is a common property for all protocols.
+    pub fn set_ip6_prefix(&mut self, ipv6_cidr: Ipv6Cidr) {
+        match self {
+            Fabric::Openfabric(fabric_section) => fabric_section.ip6_prefix = Some(ipv6_cidr),
+            Fabric::Ospf(fabric_section) => fabric_section.ip6_prefix = Some(ipv6_cidr),
+        }
+    }
 }
 
 impl Validatable for Fabric {
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pve-devel] applied: [PATCH proxmox 1/1] network-types: add method to canonicalize IPv4 and IPv6 CIDRs
  2025-07-23 14:31 [pve-devel] [PATCH proxmox 1/1] network-types: add method to canonicalize IPv4 and IPv6 CIDRs Gabriel Goller
  2025-07-23 14:32 ` [pve-devel] [PATCH proxmox-ve-rs 1/1] ve-config: fabrics: force ip-prefix to be canonical Gabriel Goller
@ 2025-07-30 12:00 ` Thomas Lamprecht
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2025-07-30 12:00 UTC (permalink / raw)
  To: pve-devel, Gabriel Goller

On Wed, 23 Jul 2025 16:31:59 +0200, Gabriel Goller wrote:
> When a cidr address in a FRR access-list is not canonicalized (i.e. is
> not a network address) then we get a warning in the journal and
> frr-reload.py will even fail. So we need to convert the address entered
> by the user into a network address. Factor out the already existing
> helper and add a new method to do this. Also add some unit tests.
> 
> 
> [...]

Applied, thanks!

[1/1] network-types: add method to canonicalize IPv4 and IPv6 CIDRs
      commit: c4afa43396a0a7dc7d9ae684bc583c04bce3e675


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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-30 11:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-23 14:31 [pve-devel] [PATCH proxmox 1/1] network-types: add method to canonicalize IPv4 and IPv6 CIDRs Gabriel Goller
2025-07-23 14:32 ` [pve-devel] [PATCH proxmox-ve-rs 1/1] ve-config: fabrics: force ip-prefix to be canonical Gabriel Goller
2025-07-30 12:00 ` [pve-devel] applied: [PATCH proxmox 1/1] network-types: add method to canonicalize IPv4 and IPv6 CIDRs Thomas Lamprecht

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