network: introduce request_hash_ops to dedup requests

If KeepConfiguration= or ConfigureWithoutCarrier= is set, then the same
requests may be queued.

7 files changed, 127 insertions, 15 deletions

diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c
index 5192bbcc00..06c5c39f1c 100644
--- a/src/network/networkd-address.c
+++ b/src/network/networkd-address.c
@@ -1148,6 +1148,8 @@ int link_request_static_addresses(Link *link) {
                                          static_address_handler, &req);
                 if (r < 0)
                         return r;
+                if (r == 0)
+                        continue;
 
                 req->after_configure = static_address_after_configure;
         }
@@ -1178,6 +1180,8 @@ int link_request_static_addresses(Link *link) {
                                          static_address_handler, &req);
                 if (r < 0)
                         return r;
+                if (r == 0)
+                        continue;
 
                 req->after_configure = static_address_after_configure;
         }
@@ -1203,8 +1207,8 @@ int link_request_static_addresses(Link *link) {
                                                  static_address_handler, &req);
                         if (r < 0)
                                 return r;
-
-                        req->after_configure = static_address_after_configure;
+                        if (r > 0)
+                                req->after_configure = static_address_after_configure;
                 }
         }
 
diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c
index e9e6fe72c4..934ee05002 100644
--- a/src/network/networkd-dhcp4.c
+++ b/src/network/networkd-dhcp4.c
@@ -208,7 +208,7 @@ static int dhcp4_request_route(Route *in, Link *link) {
 
         r = link_request_route(link, TAKE_PTR(route), true, &link->dhcp4_messages,
                                dhcp4_route_handler, &req);
-        if (r < 0)
+        if (r <= 0)
                 return r;
 
         req->after_configure = dhcp4_after_route_configure;
@@ -1064,6 +1064,8 @@ static int dhcp4_request_address(Link *link, bool announce) {
                                  dhcp4_address_handler, &req);
         if (r < 0)
                 return log_link_error_errno(link, r, "Failed to request DHCPv4 address: %m");
+        if (r == 0)
+                return 0;
 
         req->after_configure = dhcp4_after_address_configure;
 
diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c
index 7a2e7c106c..488f903329 100644
--- a/src/network/networkd-dhcp6.c
+++ b/src/network/networkd-dhcp6.c
@@ -312,6 +312,8 @@ static int dhcp6_pd_request_route(Link *link, const struct in6_addr *prefix, con
                                dhcp6_pd_route_handler, &req);
         if (r < 0)
                 return log_link_error_errno(link, r, "Failed to request DHCPv6 prefix route: %m");
+        if (r == 0)
+                return 0;
 
         req->after_configure = dhcp6_pd_after_route_configure;
 
@@ -470,6 +472,8 @@ static int dhcp6_pd_request_address(
                                  dhcp6_pd_address_handler, &req);
         if (r < 0)
                 return log_link_error_errno(link, r, "Failed to request DHCPv6 delegated prefix address: %m");
+        if (r == 0)
+                return 0;
 
         req->after_configure = dhcp6_pd_after_address_configure;
 
@@ -897,6 +901,8 @@ static int dhcp6_request_unreachable_route(Link *link, const struct in6_addr *ad
         if (r < 0)
                 return log_link_error_errno(link, r, "Failed to request unreachable route for DHCPv6 delegated subnet %s: %m",
                                             strna(buf));
+        if (r == 0)
+                return 0;
 
         req->after_configure = dhcp6_after_route_configure;
 
@@ -1173,6 +1179,8 @@ static int dhcp6_request_address(
                                  dhcp6_address_handler, &req);
         if (r < 0)
                 return log_link_error_errno(link, r, "Failed to request DHCPv6 address %s: %m", strna(buffer));
+        if (r == 0)
+                return 0;
 
         req->after_configure = dhcp6_after_address_configure;
 
diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
index 60e6d6f369..9406f8c630 100644
--- a/src/network/networkd-manager.c
+++ b/src/network/networkd-manager.c
@@ -452,7 +452,7 @@ Manager* manager_free(Manager *m) {
         HASHMAP_FOREACH(link, m->links)
                 (void) link_stop_engines(link, true);
 
-        m->request_queue = ordered_set_free_with_destructor(m->request_queue, request_free);
+        m->request_queue = ordered_set_free(m->request_queue);
 
         m->dhcp6_prefixes = hashmap_free_with_destructor(m->dhcp6_prefixes, dhcp6_pd_free);
         m->dhcp6_pd_prefixes = set_free_with_destructor(m->dhcp6_pd_prefixes, dhcp6_pd_free);
diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c
index 9ce579bfe5..80cb80247a 100644
--- a/src/network/networkd-ndisc.c
+++ b/src/network/networkd-ndisc.c
@@ -395,7 +395,7 @@ static int ndisc_request_route(Route *in, Link *link, sd_ndisc_router *rt) {
 
         r = link_request_route(link, TAKE_PTR(route), true, &link->ndisc_routes_messages,
                                ndisc_route_handler, &req);
-        if (r < 0)
+        if (r <= 0)
                 return r;
 
         req->userdata = sd_ndisc_router_ref(rt);
@@ -507,7 +507,7 @@ static int ndisc_request_address(Address *in, Link *link, sd_ndisc_router *rt) {
 
         r = link_request_address(link, TAKE_PTR(address), true, &link->ndisc_addresses_messages,
                                  ndisc_address_handler, &req);
-        if (r < 0)
+        if (r <= 0)
                 return r;
 
         req->userdata = sd_ndisc_router_ref(rt);
diff --git a/src/network/networkd-queue.c b/src/network/networkd-queue.c
index ccc1b37b40..01636468c6 100644
--- a/src/network/networkd-queue.c
+++ b/src/network/networkd-queue.c
@@ -49,16 +49,19 @@ static void request_free_object(RequestType type, void *object) {
         }
 }
 
-Request *request_free(Request *req) {
+static Request *request_free(Request *req) {
         if (!req)
                 return NULL;
 
+        if (req->link && req->link->manager)
+                /* To prevent from triggering assertions in hash functions, remove this request before
+                 * freeing object below. */
+                ordered_set_remove(req->link->manager->request_queue, req);
         if (req->on_free)
+                /* on_free() may use object. So, let's call this earlier. */
                 req->on_free(req);
         if (req->consume_object)
                 request_free_object(req->type, req->object);
-        if (req->link && req->link->manager)
-                ordered_set_remove(req->link->manager->request_queue, req);
         link_unref(req->link);
 
         return mfree(req);
@@ -73,6 +76,95 @@ void request_drop(Request *req) {
         request_free(req);
 }
 
+static void request_hash_func(const Request *req, struct siphash *state) {
+        assert(req);
+        assert(req->link);
+        assert(state);
+
+        siphash24_compress(&req->link->ifindex, sizeof(req->link->ifindex), state);
+        siphash24_compress(&req->type, sizeof(req->type), state);
+
+        switch(req->type) {
+        case REQUEST_TYPE_ADDRESS:
+                address_hash_func(req->address, state);
+                break;
+        case REQUEST_TYPE_ADDRESS_LABEL:
+        case REQUEST_TYPE_BRIDGE_FDB:
+        case REQUEST_TYPE_BRIDGE_MDB:
+                /* TODO: Currently, these types do not have any specific hash and compare functions.
+                 * Fortunately, all these objects are 'static', thus we can use the trivial functions. */
+                trivial_hash_func(req->object, state);
+                break;
+        case REQUEST_TYPE_DHCP_SERVER:
+                /* This type does not have object. */
+                break;
+        case REQUEST_TYPE_IPV6_PROXY_NDP:
+                in6_addr_hash_func(req->ipv6_proxy_ndp, state);
+                break;
+        case REQUEST_TYPE_NEIGHBOR:
+                neighbor_hash_func(req->neighbor, state);
+                break;
+        case REQUEST_TYPE_NEXTHOP:
+                nexthop_hash_func(req->nexthop, state);
+                break;
+        case REQUEST_TYPE_ROUTE:
+                route_hash_func(req->route, state);
+                break;
+        case REQUEST_TYPE_ROUTING_POLICY_RULE:
+                routing_policy_rule_hash_func(req->rule, state);
+                break;
+        default:
+                assert_not_reached("invalid request type.");
+        }
+}
+
+static int request_compare_func(const struct Request *a, const struct Request *b) {
+        int r;
+
+        assert(a);
+        assert(b);
+        assert(a->link);
+        assert(b->link);
+
+        r = CMP(a->link->ifindex, b->link->ifindex);
+        if (r != 0)
+                return r;
+
+        r = CMP(a->type, b->type);
+        if (r != 0)
+                return r;
+
+        switch (a->type) {
+        case REQUEST_TYPE_ADDRESS:
+                return address_compare_func(a->address, b->address);
+        case REQUEST_TYPE_ADDRESS_LABEL:
+        case REQUEST_TYPE_BRIDGE_FDB:
+        case REQUEST_TYPE_BRIDGE_MDB:
+                return trivial_compare_func(a->object, b->object);
+        case REQUEST_TYPE_DHCP_SERVER:
+                return 0;
+        case REQUEST_TYPE_IPV6_PROXY_NDP:
+                return in6_addr_compare_func(a->ipv6_proxy_ndp, b->ipv6_proxy_ndp);
+        case REQUEST_TYPE_NEIGHBOR:
+                return neighbor_compare_func(a->neighbor, b->neighbor);
+        case REQUEST_TYPE_NEXTHOP:
+                return nexthop_compare_func(a->nexthop, b->nexthop);
+        case REQUEST_TYPE_ROUTE:
+                return route_compare_func(a->route, b->route);
+        case REQUEST_TYPE_ROUTING_POLICY_RULE:
+                return routing_policy_rule_compare_func(a->rule, b->rule);
+        default:
+                assert_not_reached("invalid request type.");
+        }
+}
+
+DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(
+                request_hash_ops,
+                Request,
+                request_hash_func,
+                request_compare_func,
+                request_free);
+
 int link_queue_request(
                 Link *link,
                 RequestType type,
@@ -101,7 +193,7 @@ int link_queue_request(
         }
 
         *req = (Request) {
-                .link = link,
+                .link = link_ref(link),
                 .type = type,
                 .object = object,
                 .consume_object = consume_object,
@@ -109,11 +201,18 @@ int link_queue_request(
                 .netlink_handler = netlink_handler,
         };
 
-        link_ref(link);
+        r = ordered_set_ensure_put(&link->manager->request_queue, &request_hash_ops, req);
+        if (r < 0) {
+                /* To prevent from removing duplicated request. */
+                req->link = link_unref(req->link);
 
-        r = ordered_set_ensure_put(&link->manager->request_queue, NULL, req);
-        if (r < 0)
+                if (r == -EEXIST) {
+                        if (ret)
+                                *ret = NULL;
+                        return 0;
+                }
                 return r;
+        }
 
         if (req->message_counter)
                 (*req->message_counter)++;
@@ -122,7 +221,7 @@ int link_queue_request(
                 *ret = req;
 
         TAKE_PTR(req);
-        return 0;
+        return 1;
 }
 
 int manager_process_requests(sd_event_source *s, void *userdata) {
diff --git a/src/network/networkd-queue.h b/src/network/networkd-queue.h
index 813ef260b0..9fe954aa83 100644
--- a/src/network/networkd-queue.h
+++ b/src/network/networkd-queue.h
@@ -57,7 +57,6 @@ typedef struct Request {
         request_on_free_handler_t on_free;
 } Request;
 
-Request *request_free(Request *req);
 void request_drop(Request *req);
 
 int link_queue_request(