Merge pull request #17851 from yuwata/network-address-compare-func

network: revert previous changes to address_compare_func()

6 files changed, 132 insertions, 71 deletions

diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c
index 9de4d82662..18eecf6c8a 100644
--- a/src/network/networkd-address.c
+++ b/src/network/networkd-address.c
@@ -153,26 +153,40 @@ static bool address_may_have_broadcast(const Address *a) {
         return a->family == AF_INET && in4_addr_is_null(&a->in_addr_peer.in) && a->prefixlen <= 30;
 }
 
+static uint32_t address_prefix(const Address *a) {
+        assert(a);
+
+        /* make sure we don't try to shift by 32.
+         * See ISO/IEC 9899:TC3 § 6.5.7.3. */
+        if (a->prefixlen == 0)
+                return 0;
+
+        if (a->in_addr_peer.in.s_addr != 0)
+                return be32toh(a->in_addr_peer.in.s_addr) >> (32 - a->prefixlen);
+        else
+                return be32toh(a->in_addr.in.s_addr) >> (32 - a->prefixlen);
+}
+
 void address_hash_func(const Address *a, struct siphash *state) {
         assert(a);
 
         siphash24_compress(&a->family, sizeof(a->family), state);
 
-        if (!IN_SET(a->family, AF_INET, AF_INET6))
-                /* treat non-IPv4 or IPv6 address family as AF_UNSPEC */
-                return;
-
-        if (a->family == AF_INET)
-                siphash24_compress_string(a->label, state);
+        switch (a->family) {
+        case AF_INET:
+                siphash24_compress(&a->prefixlen, sizeof(a->prefixlen), state);
 
-        siphash24_compress(&a->prefixlen, sizeof(a->prefixlen), state);
-        /* local address */
-        siphash24_compress(&a->in_addr, FAMILY_ADDRESS_SIZE(a->family), state);
-        /* peer address */
-        siphash24_compress(&a->in_addr_peer, FAMILY_ADDRESS_SIZE(a->family), state);
+                uint32_t prefix = address_prefix(a);
+                siphash24_compress(&prefix, sizeof(prefix), state);
 
-        if (address_may_have_broadcast(a))
-                siphash24_compress(&a->broadcast, sizeof(a->broadcast), state);
+                _fallthrough_;
+        case AF_INET6:
+                siphash24_compress(&a->in_addr, FAMILY_ADDRESS_SIZE(a->family), state);
+                break;
+        default:
+                /* treat any other address family as AF_UNSPEC */
+                break;
+        }
 }
 
 int address_compare_func(const Address *a1, const Address *a2) {
@@ -182,32 +196,25 @@ int address_compare_func(const Address *a1, const Address *a2) {
         if (r != 0)
                 return r;
 
-        if (!IN_SET(a1->family, AF_INET, AF_INET6))
-                /* treat non-IPv4 or IPv6 address family as AF_UNSPEC */
-                return 0;
-
-        if (a1->family == AF_INET) {
-                r = strcmp_ptr(a1->label, a2->label);
+        switch (a1->family) {
+        case AF_INET:
+                /* See kernel's find_matching_ifa() in net/ipv4/devinet.c */
+                r = CMP(a1->prefixlen, a2->prefixlen);
                 if (r != 0)
                         return r;
-        }
-
-        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;
-
-        if (address_may_have_broadcast(a1))
-                return CMP(a1->broadcast.s_addr, a2->broadcast.s_addr);
+                r = CMP(address_prefix(a1), address_prefix(a2));
+                if (r != 0)
+                        return r;
 
-        return 0;
+                _fallthrough_;
+        case AF_INET6:
+                /* See kernel's ipv6_get_ifaddr() in net/ipv6/addrconf.c */
+                return memcmp(&a1->in_addr, &a2->in_addr, FAMILY_ADDRESS_SIZE(a1->family));
+        default:
+                /* treat any other address family as AF_UNSPEC */
+                return 0;
+        }
 }
 
 DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(address_hash_ops, Address, address_hash_func, address_compare_func, address_free);
@@ -440,29 +447,23 @@ int address_get(Link *link, const Address *in, Address **ret) {
         return -ENOENT;
 }
 
-static bool address_exists_internal(Set *addresses, int family, const union in_addr_union *in_addr) {
-        Address *address;
+int link_has_ipv6_address(Link *link, const struct in6_addr *address) {
+        _cleanup_(address_freep) Address *a = NULL;
+        int r;
 
-        SET_FOREACH(address, addresses) {
-                if (address->family != family)
-                        continue;
-                if (in_addr_equal(address->family, &address->in_addr, in_addr))
-                        return true;
-        }
+        assert(link);
+        assert(address);
 
-        return false;
-}
+        r = address_new(&a);
+        if (r < 0)
+                return r;
 
-bool address_exists(Link *link, int family, const union in_addr_union *in_addr) {
-        assert(link);
-        assert(IN_SET(family, AF_INET, AF_INET6));
-        assert(in_addr);
+        /* 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;
 
-        if (address_exists_internal(link->addresses, family, in_addr))
-                return true;
-        if (address_exists_internal(link->addresses_foreign, family, in_addr))
-                return true;
-        return false;
+        return address_get(link, a, NULL) >= 0;
 }
 
 static int address_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h
index 036ac7a564..4764766996 100644
--- a/src/network/networkd-address.h
+++ b/src/network/networkd-address.h
@@ -49,7 +49,6 @@ typedef struct Address {
 int address_new(Address **ret);
 Address *address_free(Address *address);
 int address_get(Link *link, const Address *in, Address **ret);
-bool address_exists(Link *link, int family, const union in_addr_union *in_addr);
 int address_configure(const Address *address, Link *link, link_netlink_message_handler_t callback, bool update, Address **ret);
 int address_remove(const Address *address, Link *link, link_netlink_message_handler_t callback);
 bool address_equal(const Address *a1, const Address *a2);
@@ -63,6 +62,7 @@ int link_set_addresses(Link *link);
 int link_drop_addresses(Link *link);
 int link_drop_foreign_addresses(Link *link);
 bool link_address_is_dynamic(const Link *link, const Address *address);
+int link_has_ipv6_address(Link *link, const struct in6_addr *address);
 
 void ipv4_dad_unref(Link *link);
 int ipv4_dad_stop(Link *link);
diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c
index 6592cdbfe0..51a34693c2 100644
--- a/src/network/networkd-dhcp6.c
+++ b/src/network/networkd-dhcp6.c
@@ -969,6 +969,70 @@ 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, char **ret) {
+        char valid_buf[FORMAT_TIMESPAN_MAX], preferred_buf[FORMAT_TIMESPAN_MAX];
+        const char *valid_str = NULL, *preferred_str = NULL;
+        _cleanup_free_ char *buffer = NULL;
+        bool by_ndisc = false;
+        Address *existing;
+        NDiscAddress *na;
+        int log_level, r;
+
+        assert(link);
+        assert(address);
+
+        (void) in_addr_to_string(address->family, &address->in_addr, &buffer);
+        if (address->cinfo.ifa_valid != CACHE_INFO_INFINITY_LIFE_TIME)
+                valid_str = format_timespan(valid_buf, FORMAT_TIMESPAN_MAX,
+                                            address->cinfo.ifa_valid * USEC_PER_SEC,
+                                            USEC_PER_SEC);
+        if (address->cinfo.ifa_prefered != CACHE_INFO_INFINITY_LIFE_TIME)
+                preferred_str = format_timespan(preferred_buf, FORMAT_TIMESPAN_MAX,
+                                                address->cinfo.ifa_prefered * USEC_PER_SEC,
+                                                USEC_PER_SEC);
+
+        r = address_get(link, address, &existing);
+        if (r < 0) {
+                /* New address. */
+                log_level = LOG_INFO;
+                goto simple_log;
+        } else
+                log_level = LOG_DEBUG;
+
+        if (set_contains(link->dhcp6_addresses, address))
+                /* Already warned. */
+                goto simple_log;
+
+        if (address->prefixlen == existing->prefixlen)
+                /* Currently, only conflict in prefix length is reported. */
+                goto simple_log;
+
+        SET_FOREACH(na, link->ndisc_addresses)
+                if (address_compare_func(na->address, existing)) {
+                        by_ndisc = true;
+                        break;
+                }
+
+        log_link_warning(link, "DHCPv6 address %s/%u (valid %s%s, preferred %s%s) conflicts the existing address %s/%u%s.",
+                         strnull(buffer), address->prefixlen,
+                         valid_str ? "for " : "forever", strempty(valid_str),
+                         preferred_str ? "for " : "forever", strempty(preferred_str),
+                         strnull(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=" : "");
+        goto finalize;
+
+simple_log:
+        log_link_full(link, log_level, "DHCPv6 address %s/%u (valid %s%s, preferred %s%s)",
+                      strnull(buffer), address->prefixlen,
+                      valid_str ? "for " : "forever", strempty(valid_str),
+                      preferred_str ? "for " : "forever", strempty(preferred_str));
+
+finalize:
+        if (ret)
+                *ret = TAKE_PTR(buffer);
+}
+
 static int dhcp6_update_address(
                 Link *link,
                 const struct in6_addr *ip6_addr,
@@ -991,22 +1055,19 @@ static int dhcp6_update_address(
         addr->cinfo.ifa_prefered = lifetime_preferred;
         addr->cinfo.ifa_valid = lifetime_valid;
 
-        (void) in_addr_to_string(addr->family, &addr->in_addr, &buffer);
-        log_link_full(link, set_contains(link->dhcp6_addresses, addr) ? LOG_DEBUG : LOG_INFO,
-                      "DHCPv6 address %s/%u timeout preferred %d valid %d",
-                      strna(buffer), addr->prefixlen, lifetime_preferred, lifetime_valid);
+        log_dhcp6_address(link, addr, &buffer);
 
         r = address_configure(addr, link, dhcp6_address_handler, true, &ret);
         if (r < 0)
                 return log_link_error_errno(link, r, "Failed to set DHCPv6 address %s/%u: %m",
-                                            strna(buffer), addr->prefixlen);
+                                            strnull(buffer), addr->prefixlen);
 
         link->dhcp6_address_messages++;
 
         r = set_ensure_put(&link->dhcp6_addresses, &address_hash_ops, ret);
         if (r < 0)
                 return log_link_error_errno(link, r, "Failed to store DHCPv6 address %s/%u: %m",
-                                            strna(buffer), addr->prefixlen);
+                                            strnull(buffer), addr->prefixlen);
 
         (void) set_remove(link->dhcp6_addresses_old, ret);
 
diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
index a10860b63b..8af17b1194 100644
--- a/src/network/networkd-manager.c
+++ b/src/network/networkd-manager.c
@@ -888,13 +888,17 @@ void manager_free(Manager *m) {
         m->rules_foreign = set_free(m->rules_foreign);
         set_free(m->rules_saved);
 
-        m->routes = set_free(m->routes);
-        m->routes_foreign = set_free(m->routes_foreign);
-
         sd_netlink_unref(m->rtnl);
         sd_netlink_unref(m->genl);
         sd_resolve_unref(m->resolve);
 
+        /* reject (e.g. unreachable) type routes are managed by Manager, but may be referenced by a
+         * link. E.g., DHCP6 with prefix delegation creates unreachable routes, and they are referenced
+         * by the upstream link. And the links may be referenced by netlink slots. Hence, two
+         * set_free() must be called after the above sd_netlink_unref(). */
+        m->routes = set_free(m->routes);
+        m->routes_foreign = set_free(m->routes_foreign);
+
         sd_event_source_unref(m->speed_meter_event_source);
         sd_event_unref(m->event);
 
diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c
index d2aa3db175..903c995e74 100644
--- a/src/network/networkd-ndisc.c
+++ b/src/network/networkd-ndisc.c
@@ -483,7 +483,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 (address_exists(link, AF_INET6, &gateway)) {
+        if (link_has_ipv6_address(link, &gateway.in6) > 0) {
                 if (DEBUG_LOGGING) {
                         _cleanup_free_ char *buffer = NULL;
 
diff --git a/src/network/test-network.c b/src/network/test-network.c
index bb67c74e9b..03c94409fa 100644
--- a/src/network/test-network.c
+++ b/src/network/test-network.c
@@ -159,10 +159,8 @@ 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));
-        a2->in_addr_peer = a1->in_addr_peer;
         assert_se(address_equal(a1, a2));
         a1->prefixlen = 10;
         assert_se(!address_equal(a1, a2));
@@ -173,13 +171,10 @@ static void test_address_equality(void) {
         assert_se(!address_equal(a1, a2));
 
         a2->family = AF_INET6;
-        a1->in_addr_peer = a2->in_addr_peer = IN_ADDR_NULL;
         assert_se(in_addr_from_string(AF_INET6, "2001:4ca0:4f01::2", &a1->in_addr) >= 0);
         assert_se(in_addr_from_string(AF_INET6, "2001:4ca0:4f01::2", &a2->in_addr) >= 0);
         assert_se(address_equal(a1, a2));
 
-        a1->prefixlen = 8;
-        assert_se(!address_equal(a1, a2));
         a2->prefixlen = 8;
         assert_se(address_equal(a1, a2));