]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
remove node lock for settrust and clearprefetch
authorEvan Hunt <each@isc.org>
Tue, 5 Aug 2025 04:26:01 +0000 (21:26 -0700)
committerEvan Hunt <each@isc.org>
Mon, 15 Sep 2025 16:11:50 +0000 (16:11 +0000)
dns_rdataset_clearprefetch() performs an atomic operation,
so it didn't actually need to lock the database node.

dns_rdataset_settrust() updates header->trust; this is also
now atomic, so it doesn't need to lock the node now either.

lib/dns/include/dns/rdataslab.h
lib/dns/qpcache.c
lib/dns/qpzone.c
lib/dns/rdataslab.c

index da60c8d469ef4b536ce3953df48729e00aee5708..76df3b78fe9c527e8761addc98aa47a46e398525 100644 (file)
@@ -83,13 +83,13 @@ struct dns_slabtop {
 };
 
 struct dns_slabheader {
-       _Atomic(uint16_t) attributes;
+       _Atomic(uint16_t)    attributes;
+       _Atomic(dns_trust_t) trust;
 
        /*%
         * Locked by the owning node's lock.
         */
-       dns_trust_t trust;
-       uint32_t    serial;
+       uint32_t serial;
        union {
                isc_stdtime_t expire;
                dns_ttl_t     ttl;
index 5d60c092d87f844597006f59704b1844871400c1..b6224c77ac36e66610e14c1c55cdbd03d341efc8 100644 (file)
@@ -1045,7 +1045,7 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header,
                rdataset->covers = DNS_TYPEPAIR_COVERS(header->typepair);
        }
        rdataset->ttl = !ZEROTTL(header) ? header->expire - now : 0;
-       rdataset->trust = header->trust;
+       rdataset->trust = atomic_load(&header->trust);
        rdataset->resign = 0;
 
        if (NEGATIVE(header)) {
@@ -1313,7 +1313,7 @@ check_zonecut(qpcnode_t *node, void *arg DNS__DB_FLARG) {
                }
        }
 
-       if (found != NULL && (!DNS_TRUST_PENDING(found->trust) ||
+       if (found != NULL && (!DNS_TRUST_PENDING(atomic_load(&found->trust)) ||
                              (search->options & DNS_DBFIND_PENDINGOK) != 0))
        {
                /*
@@ -1506,14 +1506,19 @@ find_coveringnsec(qpc_search_t *search, const dns_name_t *name,
        return result;
 }
 
-#define MISSING_ANSWER(found, options)                     \
-       ((found) == NULL ||                                \
-        (DNS_TRUST_ADDITIONAL((found)->trust) &&          \
-         (((options) & DNS_DBFIND_ADDITIONALOK) == 0)) || \
-        ((found)->trust == dns_trust_glue &&              \
-         (((options) & DNS_DBFIND_GLUEOK) == 0)) ||       \
-        (DNS_TRUST_PENDING((found)->trust) &&             \
-         (((options) & DNS_DBFIND_PENDINGOK) == 0)))
+static inline bool
+missing_answer(dns_slabheader_t *found, unsigned int options) {
+       if (found == NULL) {
+               return true;
+       }
+
+       dns_trust_t trust = atomic_load(&found->trust);
+       return (DNS_TRUST_ADDITIONAL(trust) &&
+               (options & DNS_DBFIND_ADDITIONALOK) == 0) ||
+              (DNS_TRUST_GLUE(trust) && (options & DNS_DBFIND_GLUEOK) == 0) ||
+              (DNS_TRUST_PENDING(trust) &&
+               (options & DNS_DBFIND_PENDINGOK) == 0);
+}
 
 static void
 qpc_search_init(qpc_search_t *search, qpcache_t *db, unsigned int options,
@@ -1698,7 +1703,7 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
                empty_node = false;
 
                if (header->noqname != NULL &&
-                   header->trust == dns_trust_secure)
+                   atomic_load(&header->trust) == dns_trust_secure)
                {
                        found_noqname = true;
                }
@@ -1710,15 +1715,14 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
                bool match = false;
                if (related_headers(header, typepair, sigpair, &found,
                                    &foundsig, &match) &&
-                   !MISSING_ANSWER(found, options))
+                   !missing_answer(found, options))
                {
                        /*
                         * We can't exit early until we have an answer with
-                        * sufficient trust level, see MISSING_ANSWER() macro
-                        * for details, because we might need NS or NSEC
+                        * sufficient trust level - see missing_answer()
+                        * for details - because we might need NS or NSEC
                         * records.
                         */
-
                        break;
                }
 
@@ -1808,7 +1812,7 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
        /*
         * If we didn't find what we were looking for...
         */
-       if (MISSING_ANSWER(found, options)) {
+       if (missing_answer(found, options)) {
                /*
                 * Return covering NODATA NSEC record.
                 */
@@ -2748,7 +2752,8 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
                 * data will supersede it below. Unclear what the best
                 * policy is here.
                 */
-               if (trust < oldheader->trust &&
+               dns_trust_t oldtrust = atomic_load(&oldheader->trust);
+               if (trust < oldtrust &&
                    (ACTIVE(oldheader, now) || !EXISTS(oldheader)))
                {
                        qpcache_hit(qpdb, oldheader);
@@ -2781,7 +2786,7 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
                if (ACTIVE(oldheader, now) &&
                    oldheader->typepair == DNS_TYPEPAIR(dns_rdatatype_ns) &&
                    EXISTS(oldheader) && EXISTS(newheader) &&
-                   oldheader->trust >= newheader->trust &&
+                   newheader->trust < oldtrust &&
                    oldheader->expire < newheader->expire &&
                    dns_rdataslab_equalx(
                            oldheader, newheader, qpdb->common.rdclass,
@@ -2815,14 +2820,14 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
                }
 
                /*
-                * If we will be replacing a NS RRset force its TTL
+                * If we will be replacing an NS RRset, force its TTL
                 * to be no more than the current NS RRset's TTL.  This
                 * ensures the delegations that are withdrawn are honoured.
                 */
                if (ACTIVE(oldheader, now) &&
                    oldheader->typepair == DNS_TYPEPAIR(dns_rdatatype_ns) &&
                    EXISTS(oldheader) && EXISTS(newheader) &&
-                   oldheader->trust <= newheader->trust)
+                   newheader->trust > oldtrust)
                {
                        if (newheader->expire > oldheader->expire) {
                                if (ZEROTTL(oldheader)) {
@@ -2841,7 +2846,7 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
                     oldheader->typepair ==
                             DNS_SIGTYPEPAIR(dns_rdatatype_ds)) &&
                    EXISTS(oldheader) && EXISTS(newheader) &&
-                   oldheader->trust >= newheader->trust &&
+                   newheader->trust < oldtrust &&
                    oldheader->expire < newheader->expire &&
                    dns_rdataslab_equal(oldheader, newheader))
                {
index 4642d04beecceabfc9edaa000e2fff70ba123064..e9dda2c36bb1119023276573596e5624ed56d293 100644 (file)
@@ -981,7 +981,7 @@ bindrdataset(qpzonedb_t *qpdb, qpznode_t *node, dns_slabheader_t *header,
        rdataset->type = DNS_TYPEPAIR_TYPE(header->typepair);
        rdataset->covers = DNS_TYPEPAIR_COVERS(header->typepair);
        rdataset->ttl = header->ttl;
-       rdataset->trust = header->trust;
+       rdataset->trust = atomic_load(&header->trust);
 
        if (OPTOUT(header)) {
                rdataset->attributes.optout = true;
@@ -2137,7 +2137,7 @@ loading_addrdataset(void *arg, const dns_name_t *name,
        dns_slabheader_reset(newheader, (dns_dbnode_t *)node);
 
        newheader->ttl = rdataset->ttl;
-       newheader->trust = rdataset->trust;
+       atomic_store(&newheader->trust, rdataset->trust);
        newheader->serial = 1;
 
        dns_slabheader_setownercase(newheader, name);
index f611da9fe4cd0c400f43f9c3ee51d34b4279b05d..3616b3aeadc00b89c39cc7f56dd4ebbe8b2e3fa6 100644 (file)
@@ -1123,9 +1123,8 @@ static void
 rdataset_settrust(dns_rdataset_t *rdataset, dns_trust_t trust) {
        dns_slabheader_t *header = dns_rdataset_getheader(rdataset);
 
-       dns_db_locknode(header->node, isc_rwlocktype_write);
-       header->trust = rdataset->trust = trust;
-       dns_db_unlocknode(header->node, isc_rwlocktype_write);
+       rdataset->trust = trust;
+       atomic_store(&header->trust, trust);
 }
 
 static void
@@ -1139,9 +1138,7 @@ static void
 rdataset_clearprefetch(dns_rdataset_t *rdataset) {
        dns_slabheader_t *header = dns_rdataset_getheader(rdataset);
 
-       dns_db_locknode(header->node, isc_rwlocktype_write);
        DNS_SLABHEADER_CLRATTR(header, DNS_SLABHEADERATTR_PREFETCH);
-       dns_db_unlocknode(header->node, isc_rwlocktype_write);
 }
 
 static void