network: manage addresses in the way the kernel does

This effectively reverts 5d0030310c134a016321ad8cf0b4ede8b1800d84.

With the commit 5d0030310c134a016321ad8cf0b4ede8b1800d84, networkd manages
addresses with the detailed hash and compare functions. But that causes
networkd cannot detect address update by the kernel or an external tool.
See issue
https://github.com/systemd/systemd/issues/481#issuecomment-1328132401.

With this commit, networkd (again) manages addresses in the way that the
kernel does. Hence, we can correctly detect address update.

4 files changed, 73 insertions, 34 deletions

diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c
index eae374323e..5b3b7d128a 100644
--- a/src/network/networkd-address.c
+++ b/src/network/networkd-address.c
@@ -309,6 +309,14 @@ DEFINE_PRIVATE_HASH_OPS(
         address_kernel_hash_func,
         address_kernel_compare_func);
 
+DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(
+        address_kernel_hash_ops_free,
+        Address,
+        address_kernel_hash_func,
+        address_kernel_compare_func,
+        address_free);
+
+/* The functions below are mainly used by managing Request. */
 static void address_hash_func(const Address *a, struct siphash *state) {
         assert(a);
 
@@ -367,12 +375,37 @@ int address_compare_func(const Address *a1, const Address *a2) {
         return 0;
 }
 
-DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(
-        address_hash_ops_free,
-        Address,
-        address_hash_func,
-        address_compare_func,
-        address_free);
+int address_equal(const Address *a1, const Address *a2) {
+        if (a1 == a2)
+                return true;
+
+        if (!a1 || !a2)
+                return false;
+
+        return address_compare_func(a1, a2) == 0;
+}
+
+static int address_equalify(Address *address, const Address *src) {
+        int r;
+
+        assert(address);
+        assert(src);
+
+        if (address_kernel_compare_func(address, src) != 0)
+                return -EINVAL;
+
+        if (address->family == AF_INET) {
+                address->broadcast = src->broadcast;
+                r = free_and_strdup(&address->label, src->label);
+                if (r < 0)
+                        return r;
+        } else {
+                address->prefixlen = src->prefixlen;
+                address->in_addr_peer = src->in_addr_peer;
+        }
+
+        return 0;
+}
 
 int address_dup(const Address *src, Address **ret) {
         _cleanup_(address_freep) Address *dest = NULL;
@@ -451,7 +484,7 @@ static int address_add(Link *link, Address *address) {
         assert(link);
         assert(address);
 
-        r = set_ensure_put(&link->addresses, &address_hash_ops_free, address);
+        r = set_ensure_put(&link->addresses, &address_kernel_hash_ops_free, address);
         if (r < 0)
                 return r;
         if (r == 0)
@@ -542,10 +575,10 @@ int link_get_address(Link *link, int family, const union in_addr_union *address,
          * 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) {
+        if (family == AF_INET6 || prefixlen != 0) {
                 _cleanup_(address_freep) Address *tmp = NULL;
 
-                /* If prefixlen is set, then we can use address_get(). */
+                /* In this case, we can use address_get(). */
 
                 r = address_new(&tmp);
                 if (r < 0)
@@ -554,20 +587,24 @@ int link_get_address(Link *link, int family, const union in_addr_union *address,
                 tmp->family = family;
                 tmp->in_addr = *address;
                 tmp->prefixlen = prefixlen;
-                address_set_broadcast(tmp, link);
 
-                if (address_get(link, tmp, &a) >= 0) {
-                        if (ret)
-                                *ret = a;
+                r = address_get(link, tmp, &a);
+                if (r < 0)
+                        return r;
 
-                        return 0;
+                if (family == AF_INET6) {
+                        /* IPv6 addresses are managed without peer address and prefix length. Hence, we need
+                         * to check them explicitly. */
+                        if (in_addr_is_set(family, &a->in_addr_peer))
+                                return -ENOENT;
+                        if (prefixlen != 0 && a->prefixlen != prefixlen)
+                                return -ENOENT;
                 }
 
-                if (family == AF_INET6)
-                        return -ENOENT;
+                if (ret)
+                        *ret = a;
 
-                /* IPv4 addresses may have label and/or non-default broadcast address.
-                 * Hence, we need to always fallback below. */
+                return 0;
         }
 
         SET_FOREACH(a, link->addresses) {
@@ -925,7 +962,11 @@ int link_drop_foreign_addresses(Link *link) {
         ORDERED_HASHMAP_FOREACH(address, link->network->addresses_by_section) {
                 Address *existing;
 
-                if (address_get(link, address, &existing) >= 0)
+                /* On update, the kernel ignores the address label and broadcast address. Hence we need to
+                 * distinguish addresses with different labels or broadcast addresses. Thus, we need to check
+                 * the existing address with address_equal(). Otherwise, the label or broadcast address
+                 * change will not be applied when we reconfigure the interface. */
+                if (address_get(link, address, &existing) >= 0 && address_equal(address, existing))
                         address_unmark(existing);
         }
 
@@ -1199,6 +1240,9 @@ int link_request_address(
 
                 existing = TAKE_PTR(tmp);
         } else {
+                r = address_equalify(existing, address);
+                if (r < 0)
+                        return r;
                 existing->source = address->source;
                 existing->provider = address->provider;
                 existing->duplicate_address_detection = address->duplicate_address_detection;
@@ -1468,6 +1512,12 @@ int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message,
         case RTM_NEWADDR:
                 if (address) {
                         /* update flags and etc. */
+                        r = address_equalify(address, tmp);
+                        if (r < 0) {
+                                log_link_warning_errno(link, r, "Failed to update properties of address %s, ignoring: %m",
+                                                       IN_ADDR_PREFIX_TO_STRING(address->family, &address->in_addr, address->prefixlen));
+                                return 0;
+                        }
                         address->flags = tmp->flags;
                         address->scope = tmp->scope;
                         address_set_lifetime(m, address, &cinfo);
@@ -2085,9 +2135,8 @@ int network_drop_invalid_addresses(Network *network) {
                         address_free(dup);
                 }
 
-                /* 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. */
+                /* Use address_kernel_hash_ops, instead of address_kernel_hash_ops_free. Otherwise, the
+                 * Address objects will be freed. */
                 r = set_ensure_put(&addresses, &address_kernel_hash_ops, address);
                 if (r < 0)
                         return log_oom();
diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h
index 7a1e44632d..89b9621791 100644
--- a/src/network/networkd-address.h
+++ b/src/network/networkd-address.h
@@ -118,6 +118,7 @@ int manager_rtnl_process_address(sd_netlink *nl, sd_netlink_message *message, Ma
 int network_drop_invalid_addresses(Network *network);
 
 int address_compare_func(const Address *a1, const Address *a2);
+int address_equal(const Address *a1, const Address *a2);
 
 DEFINE_NETWORK_CONFIG_STATE_FUNCTIONS(Address, address);
 
diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c
index d6ec233351..c44c37f3aa 100644
--- a/src/network/networkd-dhcp6.c
+++ b/src/network/networkd-dhcp6.c
@@ -163,8 +163,7 @@ static int verify_dhcp6_address(Link *link, const Address *address) {
 
         const char *pretty = IN6_ADDR_TO_STRING(&address->in_addr.in6);
 
-        if (address_get(link, address, &existing) < 0 &&
-            link_get_address(link, AF_INET6, &address->in_addr, 0, &existing) < 0) {
+        if (address_get(link, address, &existing) < 0) {
                 /* New address. */
                 log_level = LOG_INFO;
                 goto simple_log;
diff --git a/src/network/test-network.c b/src/network/test-network.c
index 0145c8b7c7..250ab9eff4 100644
--- a/src/network/test-network.c
+++ b/src/network/test-network.c
@@ -174,16 +174,6 @@ static int test_load_config(Manager *manager) {
         return 0;
 }
 
-static bool address_equal(const Address *a1, const Address *a2) {
-        if (a1 == a2)
-                return true;
-
-        if (!a1 || !a2)
-                return false;
-
-        return address_compare_func(a1, a2) == 0;
-}
-
 static void test_address_equality(void) {
         _cleanup_(address_freep) Address *a1 = NULL, *a2 = NULL;