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.

diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c
index 35142093ddbc3e26fe792f7dd4dd950a48f29229..7d84b0df50989496509d05ea0c027f47b0308c60 100644 (file)
--- a/src/network/networkd-address.c
+++ b/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 (!in_addr_equal(family, &a->in_addr, address))
                                 continue;
 
-                        if (!in4_addr_equal(&a->in_addr.in, address))
+                        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);

diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h
index 682f7e80e4fe980333701947c76018e0961ecdf6..b135c66a09f46e8a62dd405c0cf01af04002f155 100644 (file)
--- a/src/network/networkd-address.h
+++ b/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);

diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c
index 1293a4d913eebfe16943f157d6167d8ff8c97334..244c3a2a4d928bbcac0f22fe990bd97d9cb73fa6 100644 (file)
--- a/src/network/networkd-dhcp6.c
+++ b/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;

diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c
index 06c4366b704854f50f2fd69792d09906898e8861..7f2366ba7056903fb28c0d635bc102ae8e8e32c2 100644 (file)
--- a/src/network/networkd-ndisc.c
+++ b/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;
 

diff --git a/src/network/test-network.c b/src/network/test-network.c
index 4409a47cfba7bda3373dcdeb034f59bb540182fa..aef7459b31dd74a01b80c5f860ee2de0d56a1aed 100644 (file)
--- a/src/network/test-network.c
+++ b/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));
 }