]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Convert rwlock in dns_acl to RCU
authorOndřej Surý <ondrej@isc.org>
Fri, 13 Oct 2023 06:59:41 +0000 (08:59 +0200)
committerOndřej Surý <ondrej@isc.org>
Fri, 13 Oct 2023 12:44:40 +0000 (14:44 +0200)
The dns_aclenv_t contains two dns_acl_t - localhost and localnets that
can be swapped with a different ACLs as we configure BIND 9.  Instead of
protecting those two pointers with heavyweight read-write lock, use RCU
mechanism to dereference and swap the pointers.

lib/dns/acl.c
lib/dns/include/dns/acl.h
lib/dns/ssu.c
lib/ns/sortlist.c

index 85c3cdbd2bd573216e1cb2f9ff76c2320b48378b..95a14efe44ed2784b80eea62bb4afa90993ea5e3 100644 (file)
@@ -19,6 +19,7 @@
 #include <isc/mem.h>
 #include <isc/once.h>
 #include <isc/string.h>
+#include <isc/urcu.h>
 #include <isc/util.h>
 
 #include <dns/acl.h>
@@ -47,6 +48,7 @@ dns_acl_create(isc_mem_t *mctx, int n, dns_acl_t **target) {
        };
 
        isc_mem_attach(mctx, &acl->mctx);
+       dns_iptable_create(acl->mctx, &acl->iptable);
 
        *target = acl;
 }
@@ -401,26 +403,18 @@ dns_aclelement_match(const isc_netaddr_t *reqaddr, const dns_name_t *reqsigner,
                if (env == NULL) {
                        return (false);
                }
-               RWLOCK(&env->rwlock, isc_rwlocktype_read);
-               if (env->localhost == NULL) {
-                       RWUNLOCK(&env->rwlock, isc_rwlocktype_read);
-                       return (false);
-               }
-               dns_acl_attach(env->localhost, &inner);
-               RWUNLOCK(&env->rwlock, isc_rwlocktype_read);
+               rcu_read_lock();
+               dns_acl_attach(rcu_dereference(env->localhost), &inner);
+               rcu_read_unlock();
                break;
 
        case dns_aclelementtype_localnets:
                if (env == NULL) {
                        return (false);
                }
-               RWLOCK(&env->rwlock, isc_rwlocktype_read);
-               if (env->localnets == NULL) {
-                       RWUNLOCK(&env->rwlock, isc_rwlocktype_read);
-                       return (false);
-               }
-               dns_acl_attach(env->localnets, &inner);
-               RWUNLOCK(&env->rwlock, isc_rwlocktype_read);
+               rcu_read_lock();
+               dns_acl_attach(rcu_dereference(env->localnets), &inner);
+               rcu_read_unlock();
                break;
 
 #if defined(HAVE_GEOIP2)
@@ -652,7 +646,6 @@ dns_aclenv_create(isc_mem_t *mctx, dns_aclenv_t **envp) {
 
        isc_mem_attach(mctx, &env->mctx);
        isc_refcount_init(&env->references, 1);
-       isc_rwlock_init(&env->rwlock);
 
        dns_acl_create(mctx, 0, &env->localhost);
        dns_acl_create(mctx, 0, &env->localnets);
@@ -663,34 +656,45 @@ dns_aclenv_create(isc_mem_t *mctx, dns_aclenv_t **envp) {
 void
 dns_aclenv_set(dns_aclenv_t *env, dns_acl_t *localhost, dns_acl_t *localnets) {
        REQUIRE(VALID_ACLENV(env));
+       REQUIRE(DNS_ACL_VALID(localhost));
+       REQUIRE(DNS_ACL_VALID(localnets));
 
-       RWLOCK(&env->rwlock, isc_rwlocktype_write);
-       dns_acl_detach(&env->localhost);
-       dns_acl_attach(localhost, &env->localhost);
-       dns_acl_detach(&env->localnets);
-       dns_acl_attach(localnets, &env->localnets);
-       RWUNLOCK(&env->rwlock, isc_rwlocktype_write);
+       rcu_read_lock();
+       localhost = rcu_xchg_pointer(&env->localhost, dns_acl_ref(localhost));
+       localnets = rcu_xchg_pointer(&env->localnets, dns_acl_ref(localnets));
+       rcu_read_unlock();
+
+       dns_acl_detach(&localhost);
+       dns_acl_detach(&localnets);
 }
 
 void
-dns_aclenv_copy(dns_aclenv_t *t, dns_aclenv_t *s) {
-       REQUIRE(VALID_ACLENV(s));
-       REQUIRE(VALID_ACLENV(t));
-
-       RWLOCK(&t->rwlock, isc_rwlocktype_write);
-       RWLOCK(&s->rwlock, isc_rwlocktype_read);
-       dns_acl_detach(&t->localhost);
-       dns_acl_attach(s->localhost, &t->localhost);
-       dns_acl_detach(&t->localnets);
-       dns_acl_attach(s->localnets, &t->localnets);
-
-       t->match_mapped = s->match_mapped;
+dns_aclenv_copy(dns_aclenv_t *target, dns_aclenv_t *source) {
+       REQUIRE(VALID_ACLENV(source));
+       REQUIRE(VALID_ACLENV(target));
+
+       rcu_read_lock();
+
+       dns_acl_t *localhost = rcu_dereference(source->localhost);
+       INSIST(DNS_ACL_VALID(localhost));
+
+       dns_acl_t *localnets = rcu_dereference(source->localnets);
+       INSIST(DNS_ACL_VALID(localnets));
+
+       localhost = rcu_xchg_pointer(&target->localhost,
+                                    dns_acl_ref(localhost));
+       localnets = rcu_xchg_pointer(&target->localnets,
+                                    dns_acl_ref(localnets));
+
+       target->match_mapped = source->match_mapped;
 #if defined(HAVE_GEOIP2)
-       t->geoip = s->geoip;
+       target->geoip = source->geoip;
 #endif /* if defined(HAVE_GEOIP2) */
 
-       RWUNLOCK(&s->rwlock, isc_rwlocktype_read);
-       RWUNLOCK(&t->rwlock, isc_rwlocktype_write);
+       rcu_read_unlock();
+
+       dns_acl_detach(&localhost);
+       dns_acl_detach(&localnets);
 }
 
 static void
@@ -699,10 +703,17 @@ dns__aclenv_destroy(dns_aclenv_t *aclenv) {
 
        aclenv->magic = 0;
 
-       isc_refcount_destroy(&aclenv->references);
-       dns_acl_detach(&aclenv->localhost);
-       dns_acl_detach(&aclenv->localnets);
-       isc_rwlock_destroy(&aclenv->rwlock);
+       rcu_read_lock();
+       dns_acl_t *localhost = rcu_xchg_pointer(&aclenv->localhost, NULL);
+       INSIST(DNS_ACL_VALID(localhost));
+
+       dns_acl_t *localnets = rcu_xchg_pointer(&aclenv->localnets, NULL);
+       INSIST(DNS_ACL_VALID(localnets));
+
+       rcu_read_unlock();
+
+       dns_acl_detach(&localhost);
+       dns_acl_detach(&localnets);
 
        isc_mem_putanddetach(&aclenv->mctx, aclenv, sizeof(*aclenv));
 }
index d1be3a3d4304edc1fee9435a26c0f96f35f8e3fd..00579664959ab8fe1cdd8e1461598865ce28c957 100644 (file)
@@ -34,7 +34,6 @@
 #include <isc/magic.h>
 #include <isc/netaddr.h>
 #include <isc/refcount.h>
-#include <isc/rwlock.h>
 
 #include <dns/geoip.h>
 #include <dns/iptable.h>
@@ -103,9 +102,8 @@ struct dns_aclenv {
        isc_mem_t     *mctx;
        isc_refcount_t references;
 
-       isc_rwlock_t rwlock; /*%< Locks localhost and localnets */
-       dns_acl_t   *localhost;
-       dns_acl_t   *localnets;
+       dns_acl_t *localhost;
+       dns_acl_t *localnets;
 
        bool match_mapped;
 #if defined(HAVE_GEOIP2)
index 2f43c9e3f1653a87a819fdc9187a58a1142eb0f5..b54157ecc16b7746f5074c2fd31436f715565ed2 100644 (file)
@@ -367,10 +367,11 @@ dns_ssutable_checkrules(dns_ssutable_t *table, const dns_name_t *signer,
                        if (!dns_name_issubdomain(name, rule->name)) {
                                continue;
                        }
-                       RWLOCK(&env->rwlock, isc_rwlocktype_read);
-                       dns_acl_match(addr, NULL, env->localhost, NULL, &match,
+                       rcu_read_lock();
+                       dns_acl_t *localhost = rcu_dereference(env->localhost);
+                       dns_acl_match(addr, NULL, localhost, NULL, &match,
                                      NULL);
-                       RWUNLOCK(&env->rwlock, isc_rwlocktype_read);
+                       rcu_read_unlock();
                        if (match == 0) {
                                if (signer != NULL) {
                                        isc_log_write(dns_lctx,
index 971edb60d2b07d6a8ddb590d722d0b61ad282a1b..bebc2c8c9afe8c5d999843d9f45cc88554d1c953 100644 (file)
@@ -84,30 +84,25 @@ ns_sortlist_setup(dns_acl_t *acl, dns_aclenv_t *env, isc_netaddr_t *clientaddr,
                }
 
                if (order_elt->type == dns_aclelementtype_localhost) {
-                       dns_acl_t *inner = NULL;
-                       RWLOCK(&env->rwlock, isc_rwlocktype_read);
-                       if (env->localhost != NULL) {
-                               dns_acl_attach(env->localhost, &inner);
-                       }
-                       RWUNLOCK(&env->rwlock, isc_rwlocktype_read);
-
+                       rcu_read_lock();
+                       dns_acl_t *inner = rcu_dereference(env->localhost);
                        if (inner != NULL) {
-                               *argp = inner;
+                               *argp = dns_acl_ref(inner);
+                               rcu_read_unlock();
                                return (NS_SORTLISTTYPE_2ELEMENT);
                        }
+                       rcu_read_unlock();
                }
 
                if (order_elt->type == dns_aclelementtype_localnets) {
-                       dns_acl_t *inner = NULL;
-                       RWLOCK(&env->rwlock, isc_rwlocktype_read);
-                       if (env->localnets != NULL) {
-                               dns_acl_attach(env->localnets, &inner);
-                       }
-                       RWUNLOCK(&env->rwlock, isc_rwlocktype_read);
+                       rcu_read_lock();
+                       dns_acl_t *inner = rcu_dereference(env->localhost);
                        if (inner != NULL) {
-                               *argp = inner;
+                               *argp = dns_acl_ref(inner);
+                               rcu_read_unlock();
                                return (NS_SORTLISTTYPE_2ELEMENT);
                        }
+                       rcu_read_unlock();
                }
 
                /*