Merge pull request #18667 from poettering/resolved-change-notification

resolved: optimize dbus change notification signals a bit

src/basic/hashmap.c

@@ -2036,3 +2036,35 @@ int set_strjoin(Set *s, const char *separator, bool wrap_with_separator, char **
         *ret = TAKE_PTR(str);
         return 0;
 }
+
+bool set_equal(Set *a, Set *b) {
+        void *p;
+
+        /* Checks whether each entry of 'a' is also in 'b' and vice versa, i.e. the two sets contain the same
+         * entries */
+
+        if (a == b)
+                return true;
+
+        if (set_isempty(a) && set_isempty(b))
+                return true;
+
+        if (set_size(a) != set_size(b)) /* Cheap check that hopefully catches a lot of inequality cases
+                                         * already */
+                return false;
+
+        SET_FOREACH(p, a)
+                if (!set_contains(b, p))
+                        return false;
+
+        /* If we have the same hashops, then we don't need to check things backwards given we compared the
+         * size and that all of a is in b. */
+        if (a->b.hash_ops == b->b.hash_ops)
+                return true;
+
+        SET_FOREACH(p, b)
+                if (!set_contains(a, p))
+                        return false;
+
+        return true;
+}

src/basic/set.h

@@ -152,3 +152,5 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Set*, set_free_free);
 #define _cleanup_set_free_free_ _cleanup_(set_free_freep)
 
 int set_strjoin(Set *s, const char *separator, bool wrap_with_separator, char **ret);
+
+bool set_equal(Set *a, Set *b);

src/resolve/resolved-dns-search-domain.c

@@ -151,18 +151,22 @@ void dns_search_domain_unlink_all(DnsSearchDomain *first) {
         dns_search_domain_unlink_all(next);
 }
 
-void dns_search_domain_unlink_marked(DnsSearchDomain *first) {
+bool dns_search_domain_unlink_marked(DnsSearchDomain *first) {
         DnsSearchDomain *next;
+        bool changed;
 
         if (!first)
-                return;
+                return false;
 
         next = first->domains_next;
 
-        if (first->marked)
+        if (first->marked) {
                 dns_search_domain_unlink(first);
+                changed = true;
+        } else
+                changed = false;
 
-        dns_search_domain_unlink_marked(next);
+        return changed || dns_search_domain_unlink_marked(next);
 }
 
 void dns_search_domain_mark_all(DnsSearchDomain *first) {

src/resolve/resolved-dns-search-domain.h

@@ -44,7 +44,7 @@ void dns_search_domain_unlink(DnsSearchDomain *d);
 void dns_search_domain_move_back_and_unmark(DnsSearchDomain *d);
 
 void dns_search_domain_unlink_all(DnsSearchDomain *first);
-void dns_search_domain_unlink_marked(DnsSearchDomain *first);
+bool dns_search_domain_unlink_marked(DnsSearchDomain *first);
 void dns_search_domain_mark_all(DnsSearchDomain *first);
 
 int dns_search_domain_find(DnsSearchDomain *first, const char *name, DnsSearchDomain **ret);

src/resolve/resolved-dns-server.c

@@ -746,18 +746,22 @@ void dns_server_unlink_all(DnsServer *first) {
         dns_server_unlink_all(next);
 }
 
-void dns_server_unlink_marked(DnsServer *first) {
+bool dns_server_unlink_marked(DnsServer *first) {
         DnsServer *next;
+        bool changed;
 
         if (!first)
-                return;
+                return false;
 
         next = first->servers_next;
 
-        if (first->marked)
+        if (first->marked) {
+                changed = true;
                 dns_server_unlink(first);
+        } else
+                changed = false;
 
-        dns_server_unlink_marked(next);
+        return changed || dns_server_unlink_marked(next);
 }
 
 void dns_server_mark_all(DnsServer *first) {

src/resolve/resolved-dns-server.h

@@ -143,7 +143,7 @@ void dns_server_warn_downgrade(DnsServer *server);
 DnsServer *dns_server_find(DnsServer *first, int family, const union in_addr_union *in_addr, uint16_t port, int ifindex, const char *name);
 
 void dns_server_unlink_all(DnsServer *first);
-void dns_server_unlink_marked(DnsServer *first);
+bool dns_server_unlink_marked(DnsServer *first);
 void dns_server_mark_all(DnsServer *first);
 
 DnsServer *manager_get_first_dns_server(Manager *m, DnsServerType t);

src/resolve/resolved-link-bus.c

@@ -255,6 +255,7 @@ static int verify_unmanaged_link(Link *l, sd_bus_error *error) {
 static int bus_link_method_set_dns_servers_internal(sd_bus_message *message, void *userdata, sd_bus_error *error, bool extended) {
         _cleanup_free_ char *j = NULL;
         struct in_addr_full **dns;
+        bool changed = false;
         Link *l = userdata;
         size_t n;
         int r;
@@ -312,21 +313,26 @@ static int bus_link_method_set_dns_servers_internal(sd_bus_message *message, voi
                                 dns_server_unlink_all(l->dns_servers);
                                 goto finalize;
                         }
+
+                        changed = true;
                 }
 
         }
 
-        dns_server_unlink_marked(l->dns_servers);
-        link_allocate_scopes(l);
+        changed = dns_server_unlink_marked(l->dns_servers) || changed;
 
-        (void) link_save_user(l);
-        (void) manager_write_resolv_conf(l->manager);
-        (void) manager_send_changed(l->manager, "DNS");
+        if (changed) {
+                link_allocate_scopes(l);
 
-        if (j)
-                log_link_info(l, "Bus client set DNS server list to: %s", j);
-        else
-                log_link_info(l, "Bus client reset DNS server list.");
+                (void) link_save_user(l);
+                (void) manager_write_resolv_conf(l->manager);
+                (void) manager_send_changed(l->manager, "DNS");
+
+                if (j)
+                        log_link_info(l, "Bus client set DNS server list to: %s", j);
+                else
+                        log_link_info(l, "Bus client reset DNS server list.");
+        }
 
         r = sd_bus_reply_method_return(message, NULL);
 
@@ -349,6 +355,7 @@ int bus_link_method_set_dns_servers_ex(sd_bus_message *message, void *userdata,
 int bus_link_method_set_domains(sd_bus_message *message, void *userdata, sd_bus_error *error) {
         _cleanup_free_ char *j = NULL;
         Link *l = userdata;
+        bool changed = false;
         int r;
 
         assert(message);
@@ -431,6 +438,8 @@ int bus_link_method_set_domains(sd_bus_message *message, void *userdata, sd_bus_
                         r = dns_search_domain_new(l->manager, &d, DNS_SEARCH_DOMAIN_LINK, l, name);
                         if (r < 0)
                                 goto clear;
+
+                        changed = true;
                 }
 
                 d->route_only = route_only;
@@ -440,15 +449,17 @@ int bus_link_method_set_domains(sd_bus_message *message, void *userdata, sd_bus_
         if (r < 0)
                 goto clear;
 
-        dns_search_domain_unlink_marked(l->search_domains);
+        changed = dns_search_domain_unlink_marked(l->search_domains) || changed;
 
-        (void) link_save_user(l);
-        (void) manager_write_resolv_conf(l->manager);
+        if (changed) {
+                (void) link_save_user(l);
+                (void) manager_write_resolv_conf(l->manager);
 
-        if (j)
-                log_link_info(l, "Bus client set search domain list to: %s", j);
-        else
-                log_link_info(l, "Bus client reset search domain list.");
+                if (j)
+                        log_link_info(l, "Bus client set search domain list to: %s", j);
+                else
+                        log_link_info(l, "Bus client reset search domain list.");
+        }
 
         return sd_bus_reply_method_return(message, NULL);
 
@@ -488,9 +499,9 @@ int bus_link_method_set_default_route(sd_bus_message *message, void *userdata, s
 
                 (void) link_save_user(l);
                 (void) manager_write_resolv_conf(l->manager);
-        }
 
-        log_link_info(l, "Bus client set default route setting: %s", yes_no(b));
+                log_link_info(l, "Bus client set default route setting: %s", yes_no(b));
+        }
 
         return sd_bus_reply_method_return(message, NULL);
 }
@@ -531,13 +542,15 @@ int bus_link_method_set_llmnr(sd_bus_message *message, void *userdata, sd_bus_er
 
         bus_client_log(message, "LLMNR change");
 
-        l->llmnr_support = mode;
-        link_allocate_scopes(l);
-        link_add_rrs(l, false);
+        if (l->llmnr_support != mode) {
+                l->llmnr_support = mode;
+                link_allocate_scopes(l);
+                link_add_rrs(l, false);
 
-        (void) link_save_user(l);
+                (void) link_save_user(l);
 
-        log_link_info(l, "Bus client set LLMNR setting: %s", resolve_support_to_string(mode));
+                log_link_info(l, "Bus client set LLMNR setting: %s", resolve_support_to_string(mode));
+        }
 
         return sd_bus_reply_method_return(message, NULL);
 }
@@ -578,13 +591,15 @@ int bus_link_method_set_mdns(sd_bus_message *message, void *userdata, sd_bus_err
 
         bus_client_log(message, "mDNS change");
 
-        l->mdns_support = mode;
-        link_allocate_scopes(l);
-        link_add_rrs(l, false);
+        if (l->mdns_support != mode) {
+                l->mdns_support = mode;
+                link_allocate_scopes(l);
+                link_add_rrs(l, false);
 
-        (void) link_save_user(l);
+                (void) link_save_user(l);
 
-        log_link_info(l, "Bus client set MulticastDNS setting: %s", resolve_support_to_string(mode));
+                log_link_info(l, "Bus client set MulticastDNS setting: %s", resolve_support_to_string(mode));
+        }
 
         return sd_bus_reply_method_return(message, NULL);
 }
@@ -625,12 +640,14 @@ int bus_link_method_set_dns_over_tls(sd_bus_message *message, void *userdata, sd
 
         bus_client_log(message, "D-o-T change");
 
-        link_set_dns_over_tls_mode(l, mode);
+        if (l->dns_over_tls_mode != mode) {
+                link_set_dns_over_tls_mode(l, mode);
 
-        (void) link_save_user(l);
+                (void) link_save_user(l);
 
-        log_link_info(l, "Bus client set DNSOverTLS setting: %s",
-                      mode < 0 ? "default" : dns_over_tls_mode_to_string(mode));
+                log_link_info(l, "Bus client set DNSOverTLS setting: %s",
+                              mode < 0 ? "default" : dns_over_tls_mode_to_string(mode));
+        }
 
         return sd_bus_reply_method_return(message, NULL);
 }
@@ -671,12 +688,14 @@ int bus_link_method_set_dnssec(sd_bus_message *message, void *userdata, sd_bus_e
 
         bus_client_log(message, "DNSSEC change");
 
-        link_set_dnssec_mode(l, mode);
+        if (l->dnssec_mode != mode) {
+                link_set_dnssec_mode(l, mode);
 
-        (void) link_save_user(l);
+                (void) link_save_user(l);
 
-        log_link_info(l, "Bus client set DNSSEC setting: %s",
-                      mode < 0 ? "default" : dnssec_mode_to_string(mode));
+                log_link_info(l, "Bus client set DNSSEC setting: %s",
+                              mode < 0 ? "default" : dnssec_mode_to_string(mode));
+        }
 
         return sd_bus_reply_method_return(message, NULL);
 }
@@ -731,15 +750,17 @@ int bus_link_method_set_dnssec_negative_trust_anchors(sd_bus_message *message, v
 
         bus_client_log(message, "DNSSEC NTA change");
 
-        set_free_free(l->dnssec_negative_trust_anchors);
-        l->dnssec_negative_trust_anchors = TAKE_PTR(ns);
+        if (!set_equal(ns, l->dnssec_negative_trust_anchors)) {
+                set_free_free(l->dnssec_negative_trust_anchors);
+                l->dnssec_negative_trust_anchors = TAKE_PTR(ns);
 
-        (void) link_save_user(l);
+                (void) link_save_user(l);
 
-        if (j)
-                log_link_info(l, "Bus client set NTA list to: %s", j);
-        else
-                log_link_info(l, "Bus client reset NTA list.");
+                if (j)
+                        log_link_info(l, "Bus client set NTA list to: %s", j);
+                else
+                        log_link_info(l, "Bus client reset NTA list.");
+        }
 
         return sd_bus_reply_method_return(message, NULL);
 }

src/test/test-set.c

@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
+#include "random-util.h"
 #include "set.h"
 #include "strv.h"
 
@@ -227,6 +228,77 @@ static void test_set_strjoin(void) {
         assert_se(STR_IN_SET(joined, "xxxaaaxxxbbbxxx", "xxxbbbxxxaaaxxx"));
 }
 
+static void test_set_equal(void) {
+        _cleanup_set_free_ Set *a = NULL, *b = NULL;
+        void *p;
+        int r;
+
+        assert_se(a = set_new(NULL));
+        assert_se(b = set_new(NULL));
+
+        assert_se(set_equal(a, a));
+        assert_se(set_equal(b, b));
+        assert_se(set_equal(a, b));
+        assert_se(set_equal(b, a));
+        assert_se(set_equal(NULL, a));
+        assert_se(set_equal(NULL, b));
+        assert_se(set_equal(a, NULL));
+        assert_se(set_equal(b, NULL));
+        assert_se(set_equal(NULL, NULL));
+
+        for (unsigned i = 0; i < 333; i++) {
+                p = INT32_TO_PTR(1 + (random_u32() & 0xFFFU));
+
+                r = set_put(a, p);
+                assert_se(r >= 0 || r == -EEXIST);
+        }
+
+        assert_se(set_put(a, INT32_TO_PTR(0x1000U)) >= 0);
+
+        assert_se(set_size(a) >= 2);
+        assert_se(set_size(a) <= 334);
+
+        assert_se(!set_equal(a, b));
+        assert_se(!set_equal(b, a));
+        assert_se(!set_equal(a, NULL));
+
+        SET_FOREACH(p, a)
+                assert_se(set_put(b, p) >= 0);
+
+        assert_se(set_equal(a, b));
+        assert_se(set_equal(b, a));
+
+        assert_se(set_remove(a, INT32_TO_PTR(0x1000U)) == INT32_TO_PTR(0x1000U));
+
+        assert_se(!set_equal(a, b));
+        assert_se(!set_equal(b, a));
+
+        assert_se(set_remove(b, INT32_TO_PTR(0x1000U)) == INT32_TO_PTR(0x1000U));
+
+        assert_se(set_equal(a, b));
+        assert_se(set_equal(b, a));
+
+        assert_se(set_put(b, INT32_TO_PTR(0x1001U)) >= 0);
+
+        assert_se(!set_equal(a, b));
+        assert_se(!set_equal(b, a));
+
+        assert_se(set_put(a, INT32_TO_PTR(0x1001U)) >= 0);
+
+        assert_se(set_equal(a, b));
+        assert_se(set_equal(b, a));
+
+        set_clear(a);
+
+        assert_se(!set_equal(a, b));
+        assert_se(!set_equal(b, a));
+
+        set_clear(b);
+
+        assert_se(set_equal(a, b));
+        assert_se(set_equal(b, a));
+}
+
 int main(int argc, const char *argv[]) {
         test_set_steal_first();
         test_set_free_with_destructor();
@@ -238,6 +310,7 @@ int main(int argc, const char *argv[]) {
         test_set_ensure_put();
         test_set_ensure_consume();
         test_set_strjoin();
+        test_set_equal();
 
         return 0;
 }