all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal