network: compare addresses more strictly

This re-introduce the full hash and compre functions for Address,
which was reverted 1d30fc5cb64ecba2f03fe42aa0d8c65c3decad82 (#17851).

The issue #17831, which is fixed by #17851, is handled in a different way;
simply ignore to configure the conflicted DHCPv6 address. Previously, we
warn about the conflict, but continue to configure the address anyway, and
the kernel ignores the request. So, it is not necessary to request the
conflicted address in networkd side.

This fixes the following issues:
- when a link has an address, and corresponding .network file has the
  address with different prefix length, then the prefix length specified
  in the .network file will not be applied,
- we cannot specify multiple IPv4 addresses with different prefix
  length, e.g.
  ----
  Address=10.10.10.10/24
  Address=10.10.10.10/16
  ----
  This is spurious setup, but the kernel allows it.

Fixes #22515.

src/network/networkd-address.c

@@ -271,7 +271,7 @@ static uint32_t address_prefix(const Address *a) {
                 return be32toh(a->in_addr.in.s_addr) >> (32 - a->prefixlen);
 }
 
-void address_hash_func(const Address *a, struct siphash *state) {
+static void address_kernel_hash_func(const Address *a, struct siphash *state) {
         assert(a);
 
         siphash24_compress(&a->family, sizeof(a->family), state);
@@ -293,7 +293,7 @@ void address_hash_func(const Address *a, struct siphash *state) {
         }
 }
 
-int address_compare_func(const Address *a1, const Address *a2) {
+static int address_kernel_compare_func(const Address *a1, const Address *a2) {
         int r;
 
         r = CMP(a1->family, a2->family);
@@ -322,10 +322,49 @@ int address_compare_func(const Address *a1, const Address *a2) {
 }
 
 DEFINE_PRIVATE_HASH_OPS(
-        address_hash_ops,
+        address_kernel_hash_ops,
         Address,
-        address_hash_func,
-        address_compare_func);
+        address_kernel_hash_func,
+        address_kernel_compare_func);
+
+void address_hash_func(const Address *a, struct siphash *state) {
+        assert(a);
+
+        siphash24_compress(&a->family, sizeof(a->family), state);
+
+        /* treat any other address family as AF_UNSPEC */
+        if (!IN_SET(a->family, AF_INET, AF_INET6))
+                return;
+
+        siphash24_compress(&a->prefixlen, sizeof(a->prefixlen), state);
+        siphash24_compress(&a->in_addr, FAMILY_ADDRESS_SIZE(a->family), state);
+        siphash24_compress(&a->in_addr_peer, FAMILY_ADDRESS_SIZE(a->family), state);
+}
+
+int address_compare_func(const Address *a1, const Address *a2) {
+        int r;
+
+        r = CMP(a1->family, a2->family);
+        if (r != 0)
+                return r;
+
+        if (!IN_SET(a1->family, AF_INET, AF_INET6))
+                return 0;
+
+        r = CMP(a1->prefixlen, a2->prefixlen);
+        if (r != 0)
+                return r;
+
+        r = memcmp(&a1->in_addr, &a2->in_addr, FAMILY_ADDRESS_SIZE(a1->family));
+        if (r != 0)
+                return r;
+
+        r = memcmp(&a1->in_addr_peer, &a2->in_addr_peer, FAMILY_ADDRESS_SIZE(a1->family));
+        if (r != 0)
+                return r;
+
+        return 0;
+}
 
 DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(
         address_hash_ops_free,
@@ -496,30 +535,16 @@ int address_get(Link *link, const Address *in, Address **ret) {
         return 0;
 }
 
-int link_get_ipv6_address(Link *link, const struct in6_addr *address, Address **ret) {
-        _cleanup_(address_freep) Address *a = NULL;
+int link_get_address(Link *link, int family, const union in_addr_union *address, unsigned char prefixlen, Address **ret) {
         int r;
 
         assert(link);
+        assert(IN_SET(family, AF_INET, AF_INET6));
         assert(address);
 
-        r = address_new(&a);
-        if (r < 0)
-                return r;
-
-        /* address_compare_func() only compares the local address for IPv6 case. So, it is enough to
-         * set only family and the address. */
-        a->family = AF_INET6;
-        a->in_addr.in6 = *address;
-
-        return address_get(link, a, ret);
-}
-
-int link_get_ipv4_address(Link *link, const struct in_addr *address, unsigned char prefixlen, Address **ret) {
-        int r;
-
-        assert(link);
-        assert(address);
+        /* This find an Address object on the link which matches the given address and prefix length
+         * and does not have peer address. When the prefixlen is zero, then an Address object with an
+         * arbitrary prefixlen will be returned. */
 
         if (prefixlen != 0) {
                 _cleanup_(address_freep) Address *a = NULL;
@@ -530,8 +555,8 @@ int link_get_ipv4_address(Link *link, const struct in_addr *address, unsigned ch
                 if (r < 0)
                         return r;
 
-                a->family = AF_INET;
-                a->in_addr.in = *address;
+                a->family = family;
+                a->in_addr = *address;
                 a->prefixlen = prefixlen;
 
                 return address_get(link, a, ret);
@@ -539,10 +564,13 @@ int link_get_ipv4_address(Link *link, const struct in_addr *address, unsigned ch
                 Address *a;
 
                 SET_FOREACH(a, link->addresses) {
-                        if (a->family != AF_INET)
+                        if (a->family != family)
                                 continue;
 
-                        if (!in4_addr_equal(&a->in_addr.in, address))
+                        if (!in_addr_equal(family, &a->in_addr, address))
+                                continue;
+
+                        if (in_addr_is_set(family, &a->in_addr_peer))
                                 continue;
 
                         if (ret)
@@ -558,30 +586,14 @@ int link_get_ipv4_address(Link *link, const struct in_addr *address, unsigned ch
 int manager_has_address(Manager *manager, int family, const union in_addr_union *address, bool check_ready) {
         Address *a;
         Link *link;
-        int r;
 
         assert(manager);
         assert(IN_SET(family, AF_INET, AF_INET6));
         assert(address);
 
-        if (family == AF_INET) {
-                HASHMAP_FOREACH(link, manager->links_by_index)
-                        if (link_get_ipv4_address(link, &address->in, 0, &a) >= 0)
-                                return check_ready ? address_is_ready(a) : address_exists(a);
-        } else {
-                _cleanup_(address_freep) Address *tmp = NULL;
-
-                r = address_new(&tmp);
-                if (r < 0)
-                        return r;
-
-                tmp->family = family;
-                tmp->in_addr = *address;
-
-                HASHMAP_FOREACH(link, manager->links_by_index)
-                        if (address_get(link, tmp, &a) >= 0)
-                                return check_ready ? address_is_ready(a) : address_exists(a);
-        }
+        HASHMAP_FOREACH(link, manager->links_by_index)
+                if (link_get_address(link, family, address, 0, &a) >= 0)
+                        return check_ready ? address_is_ready(a) : address_exists(a);
 
         return false;
 }
@@ -1980,8 +1992,10 @@ int network_drop_invalid_addresses(Network *network) {
                         address_free(dup);
                 }
 
-                /* Do not use address_hash_ops_free here. Otherwise, all address settings will be freed. */
-                r = set_ensure_put(&addresses, &address_hash_ops, address);
+                /* Use address_kernel_hash_ops here. The function address_kernel_compare_func() matches
+                 * how kernel compares addresses, and is more lenient than address_compare_func().
+                 * Hence, the logic of dedup here is stricter than when address_hash_ops is used. */
+                r = set_ensure_put(&addresses, &address_kernel_hash_ops, address);
                 if (r < 0)
                         return log_oom();
                 assert(r > 0);

src/network/networkd-address.h

@@ -79,8 +79,15 @@ int link_drop_foreign_addresses(Link *link);
 int link_drop_ipv6ll_addresses(Link *link);
 void link_foreignize_addresses(Link *link);
 bool link_address_is_dynamic(const Link *link, const Address *address);
-int link_get_ipv6_address(Link *link, const struct in6_addr *address, Address **ret);
-int link_get_ipv4_address(Link *link, const struct in_addr *address, unsigned char prefixlen, Address **ret);
+int link_get_address(Link *link, int family, const union in_addr_union *address, unsigned char prefixlen, Address **ret);
+static inline int link_get_ipv6_address(Link *link, const struct in6_addr *address, unsigned char prefixlen, Address **ret) {
+        assert(address);
+        return link_get_address(link, AF_INET6, &(union in_addr_union) { .in6 = *address }, prefixlen, ret);
+}
+static inline int link_get_ipv4_address(Link *link, const struct in_addr *address, unsigned char prefixlen, Address **ret) {
+        assert(address);
+        return link_get_address(link, AF_INET, &(union in_addr_union) { .in = *address }, prefixlen, ret);
+}
 int manager_has_address(Manager *manager, int family, const union in_addr_union *address, bool check_ready);
 
 void address_cancel_request(Address *address);

src/network/networkd-dhcp6.c

@@ -155,7 +155,7 @@ static int dhcp6_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *
         return 1;
 }
 
-static void log_dhcp6_address(Link *link, const Address *address) {
+static int verify_dhcp6_address(Link *link, const Address *address) {
         _cleanup_free_ char *buffer = NULL;
         bool by_ndisc = false;
         Address *existing;
@@ -167,7 +167,8 @@ static void log_dhcp6_address(Link *link, const Address *address) {
 
         (void) in6_addr_to_string(&address->in_addr.in6, &buffer);
 
-        if (address_get(link, address, &existing) < 0) {
+        if (address_get(link, address, &existing) < 0 &&
+            link_get_address(link, AF_INET6, &address->in_addr, 0, &existing) < 0) {
                 /* New address. */
                 log_level = LOG_INFO;
                 goto simple_log;
@@ -181,20 +182,23 @@ static void log_dhcp6_address(Link *link, const Address *address) {
         if (existing->source == NETWORK_CONFIG_SOURCE_NDISC)
                 by_ndisc = true;
 
-        log_link_warning(link, "DHCPv6 address %s/%u (valid %s, preferred %s) conflicts the address %s/%u%s.",
+        log_link_warning(link, "Ignoring DHCPv6 address %s/%u (valid %s, preferred %s) which conflicts with %s/%u%s.",
                          strna(buffer), address->prefixlen,
                          FORMAT_LIFETIME(address->lifetime_valid_usec),
                          FORMAT_LIFETIME(address->lifetime_preferred_usec),
                          strna(buffer), existing->prefixlen,
-                         by_ndisc ? " assigned by NDisc. Please try to use or update IPv6Token= setting "
-                         "to change the address generated by NDISC, or disable UseAutonomousPrefix=" : "");
-        return;
+                         by_ndisc ? " assigned by NDisc" : "");
+        if (by_ndisc)
+                log_link_warning(link, "Hint: use IPv6Token= setting to change the address generated by NDisc or set UseAutonomousPrefix=no.");
+
+        return -EEXIST;
 
 simple_log:
         log_link_full(link, log_level, "DHCPv6 address %s/%u (valid %s, preferred %s)",
                       strna(buffer), address->prefixlen,
                       FORMAT_LIFETIME(address->lifetime_valid_usec),
                       FORMAT_LIFETIME(address->lifetime_preferred_usec));
+        return 0;
 }
 
 static int dhcp6_request_address(
@@ -221,7 +225,8 @@ static int dhcp6_request_address(
         addr->lifetime_preferred_usec = lifetime_preferred_usec;
         addr->lifetime_valid_usec = lifetime_valid_usec;
 
-        log_dhcp6_address(link, addr);
+        if (verify_dhcp6_address(link, addr) < 0)
+                return 0;
 
         if (address_get(link, addr, &existing) < 0)
                 link->dhcp6_configured = false;

src/network/networkd-ndisc.c

@@ -332,7 +332,7 @@ static int ndisc_router_process_default(Link *link, sd_ndisc_router *rt) {
         if (r < 0)
                 return log_link_error_errno(link, r, "Failed to get gateway address from RA: %m");
 
-        if (link_get_ipv6_address(link, &gateway, NULL) >= 0) {
+        if (link_get_ipv6_address(link, &gateway, 0, NULL) >= 0) {
                 if (DEBUG_LOGGING) {
                         _cleanup_free_ char *buffer = NULL;
 
@@ -644,7 +644,7 @@ static int ndisc_router_process_route(Link *link, sd_ndisc_router *rt) {
         if (r < 0)
                 return log_link_error_errno(link, r, "Failed to get gateway address from RA: %m");
 
-        if (link_get_ipv6_address(link, &gateway, NULL) >= 0) {
+        if (link_get_ipv6_address(link, &gateway, 0, NULL) >= 0) {
                 if (DEBUG_LOGGING) {
                         _cleanup_free_ char *buf = NULL;
 

src/network/test-network.c

@@ -208,8 +208,10 @@ static void test_address_equality(void) {
         assert_se(in_addr_from_string(AF_INET, "192.168.3.9", &a2->in_addr) >= 0);
         assert_se(address_equal(a1, a2));
         assert_se(in_addr_from_string(AF_INET, "192.168.3.10", &a1->in_addr_peer) >= 0);
-        assert_se(address_equal(a1, a2));
+        assert_se(!address_equal(a1, a2));
         assert_se(in_addr_from_string(AF_INET, "192.168.3.11", &a2->in_addr_peer) >= 0);
+        assert_se(!address_equal(a1, a2));
+        assert_se(in_addr_from_string(AF_INET, "192.168.3.10", &a2->in_addr_peer) >= 0);
         assert_se(address_equal(a1, a2));
         a1->prefixlen = 10;
         assert_se(!address_equal(a1, a2));
@@ -225,8 +227,9 @@ static void test_address_equality(void) {
         assert_se(address_equal(a1, a2));
 
         a2->prefixlen = 8;
-        assert_se(address_equal(a1, a2));
+        assert_se(!address_equal(a1, a2));
 
+        a2->prefixlen = 10;
         assert_se(in_addr_from_string(AF_INET6, "2001:4ca0:4f01::1", &a2->in_addr) >= 0);
         assert_se(!address_equal(a1, a2));
 }