Merge pull request #1074 from maxslarsson/set-ipv4-src-addr

Improve IPv4 source address selection for multi-subnet interfaces

Author: thvdveld

src/iface/interface/ipv4.rs

@@ -38,17 +38,27 @@ impl InterfaceInner {
 
     /// Get an IPv4 source address based on a destination address.
     ///
-    /// **NOTE**: unlike for IPv6, no specific selection algorithm is implemented. The first IPv4
-    /// address from the interface is returned.
+    /// This function tries to find the first IPv4 address from the interface
+    /// that is in the same subnet as the destination address. If no such
+    /// address is found, the first IPv4 address from the interface is returned.
     #[allow(unused)]
-    pub(crate) fn get_source_address_ipv4(&self, _dst_addr: &Ipv4Address) -> Option<Ipv4Address> {
+    pub(crate) fn get_source_address_ipv4(&self, dst_addr: &Ipv4Address) -> Option<Ipv4Address> {
+        let mut first_ipv4 = None;
         for cidr in self.ip_addrs.iter() {
             #[allow(irrefutable_let_patterns)] // if only ipv4 is enabled
             if let IpCidr::Ipv4(cidr) = cidr {
-                return Some(cidr.address());
+                // Return immediately if we find an address in the same subnet
+                if cidr.contains_addr(dst_addr) {
+                    return Some(cidr.address());
+                }
+
+                // Remember the first IPv4 address as fallback
+                if first_ipv4.is_none() {
+                    first_ipv4 = Some(cidr.address());
+                }
             }
         }
-        None
+        first_ipv4
     }
 
     /// Checks if an address is broadcast, taking into account ipv4 subnet-local

src/iface/interface/mod.rs

@@ -333,15 +333,18 @@ impl Interface {
         self.inner.ipv6_addr()
     }
 
-    /// Get an address from the interface that could be used as source address. For IPv4, this is
-    /// the first IPv4 address from the list of addresses. For IPv6, the address is based on the
-    /// destination address and uses RFC6724 for selecting the source address.
+    /// Get an address from the interface that could be used as source address.
+    /// For IPv4, this function tries to find a registered IPv4 address in the same
+    /// subnet as the destination, falling back to the first IPv4 address if none is
+    /// found. For IPv6, the selection is based on RFC6724.
     pub fn get_source_address(&self, dst_addr: &IpAddress) -> Option<IpAddress> {
         self.inner.get_source_address(dst_addr)
     }
 
-    /// Get an address from the interface that could be used as source address. This is the first
-    /// IPv4 address from the list of addresses in the interface.
+    /// Get an IPv4 source address based on a destination address. This function tries
+    /// to find the first IPv4 address from the interface that is in the same subnet as
+    /// the destination address. If no such address is found, the first IPv4 address
+    /// from the interface is returned.
     #[cfg(feature = "proto-ipv4")]
     pub fn get_source_address_ipv4(&self, dst_addr: &Ipv4Address) -> Option<Ipv4Address> {
         self.inner.get_source_address_ipv4(dst_addr)

src/iface/interface/tests/ipv4.rs

@@ -1208,3 +1208,83 @@ fn test_icmp_reply_size(#[case] medium: Medium) {
         ))
     );
 }
+
+#[rstest]
+#[case(Medium::Ip)]
+#[cfg(feature = "medium-ip")]
+#[case(Medium::Ethernet)]
+#[cfg(feature = "medium-ethernet")]
+fn get_source_address(#[case] medium: Medium) {
+    let (mut iface, _, _) = setup(medium);
+
+    const OWN_UNIQUE_LOCAL_ADDR1: Ipv4Address = Ipv4Address::new(172, 18, 1, 2);
+    const OWN_UNIQUE_LOCAL_ADDR2: Ipv4Address = Ipv4Address::new(172, 24, 24, 14);
+
+    // List of addresses of the interface:
+    //   172.18.1.2/24
+    //   172.24.24.14/24
+    iface.update_ip_addrs(|addrs| {
+        addrs.clear();
+
+        addrs
+            .push(IpCidr::Ipv4(Ipv4Cidr::new(OWN_UNIQUE_LOCAL_ADDR1, 24)))
+            .unwrap();
+        addrs
+            .push(IpCidr::Ipv4(Ipv4Cidr::new(OWN_UNIQUE_LOCAL_ADDR2, 24)))
+            .unwrap();
+    });
+
+    // List of addresses we test:
+    //   172.18.1.254 -> 172.18.1.2
+    //   172.24.24.12 -> 172.24.24.14
+    //   172.24.23.254 -> 172.18.1.2
+    const UNIQUE_LOCAL_ADDR1: Ipv4Address = Ipv4Address::new(172, 18, 1, 254);
+    const UNIQUE_LOCAL_ADDR2: Ipv4Address = Ipv4Address::new(172, 24, 24, 12);
+    const UNIQUE_LOCAL_ADDR3: Ipv4Address = Ipv4Address::new(172, 24, 23, 254);
+
+    assert_eq!(
+        iface.inner.get_source_address_ipv4(&UNIQUE_LOCAL_ADDR1),
+        Some(OWN_UNIQUE_LOCAL_ADDR1)
+    );
+
+    assert_eq!(
+        iface.inner.get_source_address_ipv4(&UNIQUE_LOCAL_ADDR2),
+        Some(OWN_UNIQUE_LOCAL_ADDR2)
+    );
+    assert_eq!(
+        iface.inner.get_source_address_ipv4(&UNIQUE_LOCAL_ADDR3),
+        Some(OWN_UNIQUE_LOCAL_ADDR1)
+    );
+}
+
+#[rstest]
+#[case(Medium::Ip)]
+#[cfg(feature = "medium-ip")]
+#[case(Medium::Ethernet)]
+#[cfg(feature = "medium-ethernet")]
+fn get_source_address_empty_interface(#[case] medium: Medium) {
+    let (mut iface, _, _) = setup(medium);
+
+    iface.update_ip_addrs(|ips| ips.clear());
+
+    // List of addresses we test:
+    //   172.18.1.254 -> None
+    //   172.24.24.12 -> None
+    //   172.24.23.254 -> None
+    const UNIQUE_LOCAL_ADDR1: Ipv4Address = Ipv4Address::new(172, 18, 1, 254);
+    const UNIQUE_LOCAL_ADDR2: Ipv4Address = Ipv4Address::new(172, 24, 24, 12);
+    const UNIQUE_LOCAL_ADDR3: Ipv4Address = Ipv4Address::new(172, 24, 23, 254);
+
+    assert_eq!(
+        iface.inner.get_source_address_ipv4(&UNIQUE_LOCAL_ADDR1),
+        None
+    );
+    assert_eq!(
+        iface.inner.get_source_address_ipv4(&UNIQUE_LOCAL_ADDR2),
+        None
+    );
+    assert_eq!(
+        iface.inner.get_source_address_ipv4(&UNIQUE_LOCAL_ADDR3),
+        None
+    );
+}

src/iface/interface/tests/ipv6.rs

@@ -997,13 +997,15 @@ fn get_source_address() {
     //   fd00::201:1:1:1:1 -> fd00::201:1:1:1:2
     //   fd01::201:1:1:1:1 -> fd01::201:1:1:1:2
     //   fd02::201:1:1:1:1 -> fd00::201:1:1:1:2 (because first added in the list)
+    //   fd01::201:1:1:1:3 -> fd01::201:1:1:1:2 (because in same subnet)
     //   ff02::1           -> fe80::1 (same scope)
     //   2001:db8:3::2     -> 2001:db8:3::1
     //   2001:db9:3::2     -> 2001:db8:3::1
     const LINK_LOCAL_ADDR: Ipv6Address = Ipv6Address::new(0xfe80, 0, 0, 0, 0, 0, 0, 42);
     const UNIQUE_LOCAL_ADDR1: Ipv6Address = Ipv6Address::new(0xfd00, 0, 0, 201, 1, 1, 1, 1);
     const UNIQUE_LOCAL_ADDR2: Ipv6Address = Ipv6Address::new(0xfd01, 0, 0, 201, 1, 1, 1, 1);
     const UNIQUE_LOCAL_ADDR3: Ipv6Address = Ipv6Address::new(0xfd02, 0, 0, 201, 1, 1, 1, 1);
+    const UNIQUE_LOCAL_ADDR4: Ipv6Address = Ipv6Address::new(0xfd01, 0, 0, 201, 1, 1, 1, 3);
     const GLOBAL_UNICAST_ADDR1: Ipv6Address =
         Ipv6Address::new(0x2001, 0x0db8, 0x0003, 0, 0, 0, 0, 2);
     const GLOBAL_UNICAST_ADDR2: Ipv6Address =
@@ -1030,6 +1032,10 @@ fn get_source_address() {
         iface.inner.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR3),
         OWN_UNIQUE_LOCAL_ADDR1
     );
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR4),
+        OWN_UNIQUE_LOCAL_ADDR2
+    );
     assert_eq!(
         iface
             .inner