]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
simplify qpcache iterators
authorEvan Hunt <each@isc.org>
Wed, 13 Mar 2024 05:19:47 +0000 (22:19 -0700)
committerEvan Hunt <each@isc.org>
Tue, 30 Apr 2024 19:50:01 +0000 (12:50 -0700)
in a cache database, unlike zones, NSEC3 records are stored in
the main tree. it is not necessary to maintain a separate 'nsec3'
tree, nor to have code in the dbiterator implementation to traverse
from one tree to another.

(if we ever implement synth-from-dnssec using NSEC3 records, we'll
need to revert this change. in the meantime, simpler code is better.)

lib/dns/include/dns/db.h
lib/dns/qpcache.c

index c8089b66c67914561c36e49cb0fa7ca11a9eb4bc..90e8ba072e124d020f37eefe9fa772b3464c5949 100644 (file)
@@ -1077,15 +1077,19 @@ isc_result_t
 dns_db_createiterator(dns_db_t *db, unsigned int options,
                      dns_dbiterator_t **iteratorp);
 /*%<
- * Create an iterator for version 'version' of 'db'.
+ * Create an iterator for 'db'.
  *
  * Notes:
  *
- * \li One or more of the following options can be set.
+ * \li One or more of the following options can be set:
+ *
  *     #DNS_DB_RELATIVENAMES
  *     #DNS_DB_NSEC3ONLY
  *     #DNS_DB_NONSEC3
  *
+ *     (Note that it is not mandatory to implement these flags;
+ *     some databases will ignore them.)
+ *
  * Requires:
  *
  * \li 'db' is a valid database.
index 5cd54b0fd3345f95f414c917a30655b60828c5e5..529e637ddf804e9fc94f5156fc97fa6c7d82b086 100644 (file)
@@ -47,7 +47,6 @@
 #include <dns/log.h>
 #include <dns/masterdump.h>
 #include <dns/nsec.h>
-#include <dns/nsec3.h>
 #include <dns/qp.h>
 #include <dns/rbt.h>
 #include <dns/rdata.h>
 
 #define KEEPSTALE(qpdb) ((qpdb)->common.serve_stale_ttl > 0)
 
-#define QPDBITER_NSEC3_ORIGIN_NODE(qpdb, iterator)        \
-       ((iterator)->current == &(iterator)->nsec3iter && \
-        (iterator)->node == (qpdb)->nsec3_origin_node)
-
 /*%
  * Note that "impmagic" is not the first four bytes of the struct, so
  * ISC_MAGIC_VALID cannot be used.
@@ -243,7 +238,6 @@ struct qpcache {
        unsigned int node_lock_count;
        db_nodelock_t *node_locks;
        qpcnode_t *origin_node;
-       qpcnode_t *nsec3_origin_node;
        dns_stats_t *rrsetstats;     /* cache DB only */
        isc_stats_t *cachestats;     /* cache DB only */
        isc_stats_t *gluecachestats; /* zone DB only */
@@ -294,7 +288,6 @@ struct qpcache {
        /* Locked by tree_lock. */
        dns_qp_t *tree;
        dns_qp_t *nsec;
-       dns_qp_t *nsec3;
 };
 
 /*%
@@ -387,15 +380,6 @@ typedef struct qpdb_rdatasetiter {
        dns_slabheader_t *current;
 } qpdb_rdatasetiter_t;
 
-/*
- * Note that these iterators, unless created with either DNS_DB_NSEC3ONLY or
- * DNS_DB_NONSEC3, will transparently move between the last node of the
- * "regular" QP ("iter" field) and the root node of the NSEC3 QP
- * ("nsec3iter" field) of the database in question, as if the latter was a
- * successor to the former in lexical order.  The "current" field always holds
- * the address of either "iter" or "nsec3iter", depending on which QP is
- * being traversed at given time.
- */
 static void
 dbiterator_destroy(dns_dbiterator_t **iteratorp DNS__DB_FLARG);
 static isc_result_t
@@ -424,7 +408,12 @@ static dns_dbiteratormethods_t dbiterator_methods = {
 };
 
 /*
- * If 'paused' is true, then the tree lock is not being held.
+ * Note that the QP cache database only needs a single QP iterator, because
+ * unlike the QP zone database, NSEC3 records are cached in the main tree.
+ *
+ * If we ever implement synth-from-dnssec using NSEC3 records, we'll need
+ * to have a separate tree for NSEC3 records, and to copy in the more complex
+ * iterator implementation from qpzone.c.
  */
 typedef struct qpdb_dbiterator {
        dns_dbiterator_t common;
@@ -434,10 +423,7 @@ typedef struct qpdb_dbiterator {
        dns_fixedname_t fixed;
        dns_name_t *name;
        dns_qpiter_t iter;
-       dns_qpiter_t nsec3iter;
-       dns_qpiter_t *current;
        qpcnode_t *node;
-       enum { full, nonsec3, nsec3only } nsec3mode;
 } qpdb_dbiterator_t;
 
 static void
@@ -643,10 +629,6 @@ delete_node(qpcache_t *qpdb, qpcnode_t *node) {
        case DNS_DB_NSEC_NSEC:
                result = dns_qp_deletename(qpdb->nsec, &node->name, NULL, NULL);
                break;
-       case DNS_DB_NSEC_NSEC3:
-               result = dns_qp_deletename(qpdb->nsec3, &node->name, NULL,
-                                          NULL);
-               break;
        }
        if (result != ISC_R_SUCCESS) {
                isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE,
@@ -738,9 +720,7 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep,
 
        nodelock = &qpdb->node_locks[bucket];
 
-#define KEEP_NODE(n, r)                                  \
-       ((n)->data != NULL || (n) == (r)->origin_node || \
-        (n) == (r)->nsec3_origin_node)
+#define KEEP_NODE(n, r) ((n)->data != NULL || (n) == (r)->origin_node)
 
        /* Handle easy and typical case first. */
        if (!node->dirty && KEEP_NODE(node, qpdb)) {
@@ -2540,13 +2520,7 @@ free_qpdb(qpcache_t *qpdb, bool log) {
                if (*treep == NULL) {
                        treep = &qpdb->nsec;
                        if (*treep == NULL) {
-                               treep = &qpdb->nsec3;
-                               /*
-                                * we're finished after clear cutting
-                                */
-                               if (*treep == NULL) {
-                                       break;
-                               }
+                               break;
                        }
                }
 
@@ -2640,9 +2614,6 @@ qpdb_destroy(dns_db_t *arg) {
        if (qpdb->origin_node != NULL) {
                qpcnode_detach(&qpdb->origin_node);
        }
-       if (qpdb->nsec3_origin_node != NULL) {
-               qpcnode_detach(&qpdb->nsec3_origin_node);
-       }
 
        /*
         * Even though there are no external direct references, there still
@@ -2910,44 +2881,23 @@ detachnode(dns_db_t *db, dns_dbnode_t **targetp DNS__DB_FLARG) {
 }
 
 static isc_result_t
-createiterator(dns_db_t *db, unsigned int options,
+createiterator(dns_db_t *db, unsigned int options ISC_ATTR_UNUSED,
               dns_dbiterator_t **iteratorp) {
        qpcache_t *qpdb = (qpcache_t *)db;
        qpdb_dbiterator_t *qpdbiter = NULL;
 
        REQUIRE(VALID_QPDB(qpdb));
-       REQUIRE((options & (DNS_DB_NSEC3ONLY | DNS_DB_NONSEC3)) !=
-               (DNS_DB_NSEC3ONLY | DNS_DB_NONSEC3));
 
        qpdbiter = isc_mem_get(qpdb->common.mctx, sizeof(*qpdbiter));
+       *qpdbiter = (qpdb_dbiterator_t){
+               .common.methods = &dbiterator_methods,
+               .common.magic = DNS_DBITERATOR_MAGIC,
+               .paused = true,
+       };
 
-       qpdbiter->common.methods = &dbiterator_methods;
-       qpdbiter->common.db = NULL;
-       dns_db_attach(db, &qpdbiter->common.db);
-       qpdbiter->common.relative_names = 0; /* no special logic for relative
-                                                names */
-       qpdbiter->common.magic = DNS_DBITERATOR_MAGIC;
-       qpdbiter->paused = true;
-       qpdbiter->tree_locked = isc_rwlocktype_none;
-       qpdbiter->result = ISC_R_SUCCESS;
-       dns_fixedname_init(&qpdbiter->fixed);
        qpdbiter->name = dns_fixedname_initname(&qpdbiter->fixed);
-       qpdbiter->node = NULL;
-
-       if ((options & DNS_DB_NSEC3ONLY) != 0) {
-               qpdbiter->nsec3mode = nsec3only;
-       } else if ((options & DNS_DB_NONSEC3) != 0) {
-               qpdbiter->nsec3mode = nonsec3;
-       } else {
-               qpdbiter->nsec3mode = full;
-       }
+       dns_db_attach(db, &qpdbiter->common.db);
        dns_qpiter_init(qpdb->tree, &qpdbiter->iter);
-       dns_qpiter_init(qpdb->nsec3, &qpdbiter->nsec3iter);
-       if (qpdbiter->nsec3mode == nsec3only) {
-               qpdbiter->current = &qpdbiter->nsec3iter;
-       } else {
-               qpdbiter->current = &qpdbiter->iter;
-       }
 
        *iteratorp = (dns_dbiterator_t *)qpdbiter;
        return (ISC_R_SUCCESS);
@@ -3709,9 +3659,6 @@ nodecount(dns_db_t *db, dns_dbtree_t tree) {
        case dns_dbtree_nsec:
                mu = dns_qp_memusage(qpdb->nsec);
                break;
-       case dns_dbtree_nsec3:
-               mu = dns_qp_memusage(qpdb->nsec3);
-               break;
        default:
                UNREACHABLE();
        }
@@ -3862,7 +3809,6 @@ dns__qpcache_create(isc_mem_t *mctx, const dns_name_t *origin,
         */
        dns_qp_create(mctx, &qpmethods, qpdb, &qpdb->tree);
        dns_qp_create(mctx, &qpmethods, qpdb, &qpdb->nsec);
-       dns_qp_create(mctx, &qpmethods, qpdb, &qpdb->nsec3);
 
        qpdb->common.magic = DNS_DB_MAGIC;
        qpdb->common.impmagic = QPDB_MAGIC;
@@ -4135,13 +4081,8 @@ resume_iteration(qpdb_dbiterator_t *qpdbiter, bool continuing) {
         */
        if (continuing && qpdbiter->node != NULL) {
                isc_result_t result;
-               dns_qp_t *tree = qpdb->tree;
-
-               if (qpdbiter->current == &qpdbiter->nsec3iter) {
-                       tree = qpdb->nsec3;
-               }
-               result = dns_qp_lookup(tree, qpdbiter->name, NULL,
-                                      qpdbiter->current, NULL, NULL, NULL);
+               result = dns_qp_lookup(qpdb->tree, qpdbiter->name, NULL,
+                                      &qpdbiter->iter, NULL, NULL, NULL);
                INSIST(result == ISC_R_SUCCESS);
        }
 
@@ -4190,45 +4131,12 @@ dbiterator_first(dns_dbiterator_t *iterator DNS__DB_FLARG) {
 
        dereference_iter_node(qpdbiter DNS__DB_FLARG_PASS);
 
-       switch (qpdbiter->nsec3mode) {
-       case nsec3only:
-               qpdbiter->current = &qpdbiter->nsec3iter;
-               dns_qpiter_init(qpdb->nsec3, qpdbiter->current);
-               result = dns_qpiter_next(qpdbiter->current, qpdbiter->name,
-                                        (void **)&qpdbiter->node, NULL);
-               if (result == ISC_R_SUCCESS) {
-                       /* If we're in the NSEC3 tree, skip the origin */
-                       if (QPDBITER_NSEC3_ORIGIN_NODE(qpdb, qpdbiter)) {
-                               result = dns_qpiter_next(
-                                       qpdbiter->current, qpdbiter->name,
-                                       (void **)&qpdbiter->node, NULL);
-                       }
-               }
-               break;
-       case nonsec3:
-               qpdbiter->current = &qpdbiter->iter;
-               dns_qpiter_init(qpdb->tree, qpdbiter->current);
-               result = dns_qpiter_next(qpdbiter->current, qpdbiter->name,
-                                        (void **)&qpdbiter->node, NULL);
-               break;
-       case full:
-               qpdbiter->current = &qpdbiter->iter;
-               dns_qpiter_init(qpdb->tree, qpdbiter->current);
-               result = dns_qpiter_next(qpdbiter->current, qpdbiter->name,
-                                        (void **)&qpdbiter->node, NULL);
-               if (result == ISC_R_NOMORE) {
-                       qpdbiter->current = &qpdbiter->nsec3iter;
-                       dns_qpiter_init(qpdb->nsec3, qpdbiter->current);
-                       result = dns_qpiter_next(
-                               qpdbiter->current, qpdbiter->name,
-                               (void **)&qpdbiter->node, NULL);
-               }
-               break;
-       default:
-               UNREACHABLE();
-       }
+       dns_qpiter_init(qpdb->tree, &qpdbiter->iter);
+       result = dns_qpiter_next(&qpdbiter->iter, NULL,
+                                (void **)&qpdbiter->node, NULL);
 
        if (result == ISC_R_SUCCESS) {
+               dns_name_copy(&qpdbiter->node->name, qpdbiter->name);
                reference_iter_node(qpdbiter DNS__DB_FLARG_PASS);
        } else {
                INSIST(result == ISC_R_NOMORE); /* The tree is empty. */
@@ -4264,55 +4172,12 @@ dbiterator_last(dns_dbiterator_t *iterator DNS__DB_FLARG) {
 
        dereference_iter_node(qpdbiter DNS__DB_FLARG_PASS);
 
-       switch (qpdbiter->nsec3mode) {
-       case nsec3only:
-               qpdbiter->current = &qpdbiter->nsec3iter;
-               dns_qpiter_init(qpdb->nsec3, qpdbiter->current);
-               result = dns_qpiter_prev(qpdbiter->current, qpdbiter->name,
-                                        (void **)&qpdbiter->node, NULL);
-               if (result == ISC_R_SUCCESS &&
-                   QPDBITER_NSEC3_ORIGIN_NODE(qpdb, qpdbiter))
-               {
-                       /*
-                        * NSEC3 tree only has an origin node.
-                        */
-                       qpdbiter->node = NULL;
-                       result = ISC_R_NOMORE;
-               }
-               break;
-       case nonsec3:
-               qpdbiter->current = &qpdbiter->iter;
-               dns_qpiter_init(qpdb->tree, qpdbiter->current);
-               result = dns_qpiter_prev(qpdbiter->current, qpdbiter->name,
-                                        (void **)&qpdbiter->node, NULL);
-               break;
-       case full:
-               qpdbiter->current = &qpdbiter->nsec3iter;
-               dns_qpiter_init(qpdb->nsec3, qpdbiter->current);
-               result = dns_qpiter_prev(qpdbiter->current, qpdbiter->name,
-                                        (void **)&qpdbiter->node, NULL);
-               if (result == ISC_R_SUCCESS &&
-                   QPDBITER_NSEC3_ORIGIN_NODE(qpdb, qpdbiter))
-               {
-                       /*
-                        * NSEC3 tree only has an origin node.
-                        */
-                       qpdbiter->node = NULL;
-                       result = ISC_R_NOMORE;
-               }
-               if (result == ISC_R_NOMORE) {
-                       qpdbiter->current = &qpdbiter->iter;
-                       dns_qpiter_init(qpdb->tree, qpdbiter->current);
-                       result = dns_qpiter_prev(
-                               qpdbiter->current, qpdbiter->name,
-                               (void **)&qpdbiter->node, NULL);
-               }
-               break;
-       default:
-               UNREACHABLE();
-       }
+       dns_qpiter_init(qpdb->tree, &qpdbiter->iter);
+       result = dns_qpiter_prev(&qpdbiter->iter, NULL,
+                                (void **)&qpdbiter->node, NULL);
 
        if (result == ISC_R_SUCCESS) {
+               dns_name_copy(&qpdbiter->node->name, qpdbiter->name);
                reference_iter_node(qpdbiter DNS__DB_FLARG_PASS);
        } else {
                INSIST(result == ISC_R_NOMORE); /* The tree is empty. */
@@ -4320,14 +4185,13 @@ dbiterator_last(dns_dbiterator_t *iterator DNS__DB_FLARG) {
        }
 
        qpdbiter->result = result;
-
        return (result);
 }
 
 static isc_result_t
 dbiterator_seek(dns_dbiterator_t *iterator,
                const dns_name_t *name DNS__DB_FLARG) {
-       isc_result_t result, tresult;
+       isc_result_t result;
        qpdb_dbiterator_t *qpdbiter = (qpdb_dbiterator_t *)iterator;
        qpcache_t *qpdb = (qpcache_t *)iterator->db;
 
@@ -4345,43 +4209,8 @@ dbiterator_seek(dns_dbiterator_t *iterator,
 
        dereference_iter_node(qpdbiter DNS__DB_FLARG_PASS);
 
-       switch (qpdbiter->nsec3mode) {
-       case nsec3only:
-               qpdbiter->current = &qpdbiter->nsec3iter;
-               result = dns_qp_lookup(qpdb->nsec3, name, NULL,
-                                      qpdbiter->current, NULL,
-                                      (void **)&qpdbiter->node, NULL);
-               break;
-       case nonsec3:
-               qpdbiter->current = &qpdbiter->iter;
-               result = dns_qp_lookup(qpdb->tree, name, NULL,
-                                      qpdbiter->current, NULL,
-                                      (void **)&qpdbiter->node, NULL);
-               break;
-       case full:
-               /*
-                * Stay on main chain if not found on
-                * either iterator.
-                */
-               qpdbiter->current = &qpdbiter->iter;
-               result = dns_qp_lookup(qpdb->tree, name, NULL,
-                                      qpdbiter->current, NULL,
-                                      (void **)&qpdbiter->node, NULL);
-               if (result == DNS_R_PARTIALMATCH) {
-                       qpcnode_t *node = NULL;
-                       tresult = dns_qp_lookup(qpdb->nsec3, name, NULL,
-                                               &qpdbiter->nsec3iter, NULL,
-                                               (void **)&node, NULL);
-                       if (tresult == ISC_R_SUCCESS) {
-                               qpdbiter->node = node;
-                               qpdbiter->current = &qpdbiter->nsec3iter;
-                               result = tresult;
-                       }
-               }
-               break;
-       default:
-               UNREACHABLE();
-       }
+       result = dns_qp_lookup(qpdb->tree, name, NULL, &qpdbiter->iter, NULL,
+                              (void **)&qpdbiter->node, NULL);
 
        if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) {
                dns_name_copy(&qpdbiter->node->name, qpdbiter->name);
@@ -4392,7 +4221,6 @@ dbiterator_seek(dns_dbiterator_t *iterator,
 
        qpdbiter->result = (result == DNS_R_PARTIALMATCH) ? ISC_R_SUCCESS
                                                          : result;
-
        return (result);
 }
 
@@ -4400,7 +4228,6 @@ static isc_result_t
 dbiterator_prev(dns_dbiterator_t *iterator DNS__DB_FLARG) {
        isc_result_t result;
        qpdb_dbiterator_t *qpdbiter = (qpdb_dbiterator_t *)iterator;
-       qpcache_t *qpdb = (qpcache_t *)iterator->db;
 
        REQUIRE(qpdbiter->node != NULL);
 
@@ -4414,31 +4241,11 @@ dbiterator_prev(dns_dbiterator_t *iterator DNS__DB_FLARG) {
 
        dereference_iter_node(qpdbiter DNS__DB_FLARG_PASS);
 
-       result = dns_qpiter_prev(qpdbiter->current, qpdbiter->name,
+       result = dns_qpiter_prev(&qpdbiter->iter, NULL,
                                 (void **)&qpdbiter->node, NULL);
 
-       if (qpdbiter->current == &qpdbiter->nsec3iter) {
-               if (result == ISC_R_SUCCESS) {
-                       /*
-                        * If we're in the NSEC3 tree, it's empty or
-                        * we've reached the origin, then we're done
-                        * with it.
-                        */
-                       if (QPDBITER_NSEC3_ORIGIN_NODE(qpdb, qpdbiter)) {
-                               qpdbiter->node = NULL;
-                               result = ISC_R_NOMORE;
-                       }
-               }
-               if (result == ISC_R_NOMORE && qpdbiter->nsec3mode == full) {
-                       qpdbiter->current = &qpdbiter->iter;
-                       dns_qpiter_init(qpdb->tree, qpdbiter->current);
-                       result = dns_qpiter_prev(
-                               qpdbiter->current, qpdbiter->name,
-                               (void **)&qpdbiter->node, NULL);
-               }
-       }
-
        if (result == ISC_R_SUCCESS) {
+               dns_name_copy(&qpdbiter->node->name, qpdbiter->name);
                reference_iter_node(qpdbiter DNS__DB_FLARG_PASS);
        } else {
                INSIST(result == ISC_R_NOMORE);
@@ -4446,7 +4253,6 @@ dbiterator_prev(dns_dbiterator_t *iterator DNS__DB_FLARG) {
        }
 
        qpdbiter->result = result;
-
        return (result);
 }
 
@@ -4454,7 +4260,6 @@ static isc_result_t
 dbiterator_next(dns_dbiterator_t *iterator DNS__DB_FLARG) {
        isc_result_t result;
        qpdb_dbiterator_t *qpdbiter = (qpdb_dbiterator_t *)iterator;
-       qpcache_t *qpdb = (qpcache_t *)iterator->db;
 
        REQUIRE(qpdbiter->node != NULL);
 
@@ -4468,42 +4273,11 @@ dbiterator_next(dns_dbiterator_t *iterator DNS__DB_FLARG) {
 
        dereference_iter_node(qpdbiter DNS__DB_FLARG_PASS);
 
-       result = dns_qpiter_next(qpdbiter->current, qpdbiter->name,
+       result = dns_qpiter_next(&qpdbiter->iter, NULL,
                                 (void **)&qpdbiter->node, NULL);
 
-       if (result == ISC_R_NOMORE && qpdbiter->nsec3mode == full &&
-           qpdbiter->current == &qpdbiter->iter)
-       {
-               qpdbiter->current = &qpdbiter->nsec3iter;
-               dns_qpiter_init(qpdb->nsec3, qpdbiter->current);
-               result = dns_qpiter_next(qpdbiter->current, qpdbiter->name,
-                                        (void **)&qpdbiter->node, NULL);
-       }
-
-       if (result == ISC_R_SUCCESS) {
-               /*
-                * If we've just started the NSEC3 tree,
-                * skip over the origin.
-                */
-               if (QPDBITER_NSEC3_ORIGIN_NODE(qpdb, qpdbiter)) {
-                       switch (qpdbiter->nsec3mode) {
-                       case nsec3only:
-                       case full:
-                               result = dns_qpiter_next(
-                                       qpdbiter->current, qpdbiter->name,
-                                       (void **)&qpdbiter->node, NULL);
-                               break;
-                       case nonsec3:
-                               result = ISC_R_NOMORE;
-                               qpdbiter->node = NULL;
-                               break;
-                       default:
-                               UNREACHABLE();
-                       }
-               }
-       }
-
        if (result == ISC_R_SUCCESS) {
+               dns_name_copy(&qpdbiter->node->name, qpdbiter->name);
                reference_iter_node(qpdbiter DNS__DB_FLARG_PASS);
        } else {
                INSIST(result == ISC_R_NOMORE);
@@ -4511,7 +4285,6 @@ dbiterator_next(dns_dbiterator_t *iterator DNS__DB_FLARG) {
        }
 
        qpdbiter->result = result;
-
        return (result);
 }
 
@@ -4523,21 +4296,20 @@ dbiterator_current(dns_dbiterator_t *iterator, dns_dbnode_t **nodep,
        qpcnode_t *node = qpdbiter->node;
 
        REQUIRE(qpdbiter->result == ISC_R_SUCCESS);
-       REQUIRE(qpdbiter->node != NULL);
+       REQUIRE(node != NULL);
 
        if (qpdbiter->paused) {
                resume_iteration(qpdbiter, false);
        }
 
        if (name != NULL) {
-               dns_name_copy(&qpdbiter->node->name, name);
+               dns_name_copy(&node->name, name);
        }
 
        newref(qpdb, node, isc_rwlocktype_none,
               qpdbiter->tree_locked DNS__DB_FLARG_PASS);
 
        *nodep = qpdbiter->node;
-
        return (ISC_R_SUCCESS);
 }