From: Ondřej Surý Date: Mon, 15 Sep 2025 15:46:39 +0000 (+0200) Subject: Squash the qpcache tree and nsec trees X-Git-Tag: v9.21.14~37^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a3e96f2d49d44f971b9feac0574f41d4ea7f12bb;p=thirdparty%2Fbind9.git Squash the qpcache tree and nsec trees The dns_qpcache already had all the namespace changes needed to put the normal data and auxiliary NSEC data into a single tree. Remove the extra nsec QP trie and use the single QP trie for all the cache data. --- diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index 17caa4a39a0..fb551e1c8b1 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -252,7 +252,6 @@ struct qpcache { /* Locked by tree_lock. */ dns_qp_t *tree; - dns_qp_t *nsec; isc_mem_t *hmctx; /* Memory context for the heaps */ @@ -407,12 +406,11 @@ static dns_dbiteratormethods_t dbiterator_methods = { }; /* - * 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. + * In the cache, NSEC3 records are currently stored in the NORMAL + * namespace. If we ever implement synth-from-dnssec using NSEC3 records, + * they'll need be moved into the NSEC3 namespace for efficiency, and + * the iterator implementation will need to be more complex, as in + * qpzone. */ typedef struct qpc_dbit { dns_dbiterator_t common; @@ -629,7 +627,7 @@ delete_node(qpcache_t *qpdb, qpcnode_t *node) { * Delete the corresponding node from the auxiliary NSEC * tree before deleting from the main tree. */ - result = dns_qp_deletename(qpdb->nsec, &node->name, + result = dns_qp_deletename(qpdb->tree, &node->name, DNS_DBNAMESPACE_NSEC, NULL, NULL); if (result != ISC_R_SUCCESS) { @@ -645,7 +643,7 @@ delete_node(qpcache_t *qpdb, qpcnode_t *node) { node->nspace, NULL, NULL); break; case DNS_DBNAMESPACE_NSEC: - result = dns_qp_deletename(qpdb->nsec, &node->name, + result = dns_qp_deletename(qpdb->tree, &node->name, node->nspace, NULL, NULL); break; } @@ -1427,9 +1425,9 @@ find_coveringnsec(qpc_search_t *search, const dns_name_t *name, dns_slabheader_t *found = NULL, *foundsig = NULL; /* - * Look for the node in the auxilary tree. + * Look for the node in the auxiliary NSEC namespace. */ - result = dns_qp_lookup(search->qpdb->nsec, name, DNS_DBNAMESPACE_NSEC, + result = dns_qp_lookup(search->qpdb->tree, name, DNS_DBNAMESPACE_NSEC, NULL, &iter, NULL, (void **)&node, NULL); /* * When DNS_R_PARTIALMATCH or ISC_R_NOTFOUND is returned from @@ -1453,7 +1451,7 @@ find_coveringnsec(qpc_search_t *search, const dns_name_t *name, } /* - * Lookup the predecessor in the main tree. + * Lookup the predecessor in the normal namespace. */ node = NULL; result = dns_qp_getname(search->qpdb->tree, predecessor, @@ -2269,23 +2267,8 @@ static void qpcache__destroy(qpcache_t *qpdb) { unsigned int i; char buf[DNS_NAME_FORMATSIZE]; - dns_qp_t **treep = NULL; - - for (;;) { - /* - * pick the next tree to (start to) destroy - */ - treep = &qpdb->tree; - if (*treep == NULL) { - treep = &qpdb->nsec; - if (*treep == NULL) { - break; - } - } - dns_qp_destroy(treep); - INSIST(*treep == NULL); - } + dns_qp_destroy(&qpdb->tree); if (dns_name_dynamic(&qpdb->common.origin)) { dns_name_format(&qpdb->common.origin, buf, sizeof(buf)); @@ -3147,13 +3130,13 @@ qpcache_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, if (newnsec && !qpnode->havensec) { qpcnode_t *nsecnode = NULL; - result = dns_qp_getname(qpdb->nsec, name, DNS_DBNAMESPACE_NSEC, + result = dns_qp_getname(qpdb->tree, name, DNS_DBNAMESPACE_NSEC, (void **)&nsecnode, NULL); if (result != ISC_R_SUCCESS) { INSIST(nsecnode == NULL); nsecnode = new_qpcnode(qpdb, name, DNS_DBNAMESPACE_NSEC); - result = dns_qp_insert(qpdb->nsec, nsecnode, 0); + result = dns_qp_insert(qpdb->tree, nsecnode, 0); INSIST(result == ISC_R_SUCCESS); qpcnode_detach(&nsecnode); } @@ -3235,7 +3218,7 @@ qpcache_deleterdataset(dns_db_t *db, dns_dbnode_t *node, } static unsigned int -nodecount(dns_db_t *db, dns_dbtree_t tree) { +nodecount(dns_db_t *db, dns_dbtree_t tree ISC_ATTR_UNUSED) { qpcache_t *qpdb = (qpcache_t *)db; dns_qp_memusage_t mu; isc_rwlocktype_t tlocktype = isc_rwlocktype_none; @@ -3243,16 +3226,7 @@ nodecount(dns_db_t *db, dns_dbtree_t tree) { REQUIRE(VALID_QPDB(qpdb)); TREE_RDLOCK(&qpdb->tree_lock, &tlocktype); - switch (tree) { - case dns_dbtree_main: - mu = dns_qp_memusage(qpdb->tree); - break; - case dns_dbtree_nsec: - mu = dns_qp_memusage(qpdb->nsec); - break; - default: - UNREACHABLE(); - } + mu = dns_qp_memusage(qpdb->tree); TREE_UNLOCK(&qpdb->tree_lock, &tlocktype); return mu.leaves; @@ -3324,10 +3298,9 @@ dns__qpcache_create(isc_mem_t *mctx, const dns_name_t *origin, dns_name_dup(origin, mctx, &qpdb->common.origin); /* - * Make the qp tries. + * Make the qp trie. */ dns_qp_create(mctx, &qpmethods, qpdb, &qpdb->tree); - dns_qp_create(mctx, &qpmethods, qpdb, &qpdb->nsec); qpdb->common.magic = DNS_DB_MAGIC; qpdb->common.impmagic = QPDB_MAGIC; @@ -3520,9 +3493,9 @@ resume_iteration(qpc_dbit_t *qpdbiter, bool continuing) { TREE_RDLOCK(&qpdb->tree_lock, &qpdbiter->tree_locked); /* - * If we're being called from dbiterator_next or _prev, - * then we may need to reinitialize the iterator to the current - * name. The tree could have changed while it was unlocked, + * If we're being called from dbiterator_next, we may need + * to reinitialize the iterator to the current name. The + * tree could have changed while it was unlocked, which * would make the iterator traversal inconsistent. * * As long as the iterator is holding a reference to @@ -3586,11 +3559,17 @@ dbiterator_first(dns_dbiterator_t *iterator DNS__DB_FLARG) { result = dns_qpiter_next(&qpdbiter->iter, NULL, (void **)&qpdbiter->node, NULL); - if (result == ISC_R_SUCCESS) { + if (result == ISC_R_SUCCESS && + qpdbiter->node->nspace == DNS_DBNAMESPACE_NORMAL) + { dns_name_copy(&qpdbiter->node->name, qpdbiter->name); reference_iter_node(qpdbiter DNS__DB_FLARG_PASS); + } else if (result == ISC_R_SUCCESS) { + result = ISC_R_NOMORE; + qpdbiter->node = NULL; } else { - INSIST(result == ISC_R_NOMORE); /* The tree is empty. */ + /* The tree is empty. */ + INSIST(result == ISC_R_NOMORE); qpdbiter->node = NULL; } @@ -3670,9 +3649,14 @@ dbiterator_next(dns_dbiterator_t *iterator DNS__DB_FLARG) { result = dns_qpiter_next(&qpdbiter->iter, NULL, (void **)&qpdbiter->node, NULL); - if (result == ISC_R_SUCCESS) { + if (result == ISC_R_SUCCESS && + qpdbiter->node->nspace == DNS_DBNAMESPACE_NORMAL) + { dns_name_copy(&qpdbiter->node->name, qpdbiter->name); reference_iter_node(qpdbiter DNS__DB_FLARG_PASS); + } else if (result == ISC_R_SUCCESS) { + result = ISC_R_NOMORE; + qpdbiter->node = NULL; } else { INSIST(result == ISC_R_NOMORE); qpdbiter->node = NULL; diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index e081f15c261..82374dd2fa8 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -2880,7 +2880,7 @@ find_wildcard(qpz_search_t *search, qpznode_t **nodep, const dns_name_t *qname, } /* - * Find node of the NSEC/NSEC3 record that is 'name'. + * Find node of the NSEC/NSEC3 record preceding 'name'. */ static isc_result_t previous_closest_nsec(dns_rdatatype_t type, qpz_search_t *search, @@ -2903,8 +2903,8 @@ previous_closest_nsec(dns_rdatatype_t type, qpz_search_t *search, for (;;) { if (*firstp) { /* - * Construct the name of the second node to check. - * It is the first node sought in the NSEC tree. + * This is the first attempt to find 'name' in the + * NSEC namespace. */ *firstp = false; result = dns_qp_lookup(&qpr, name, DNS_DBNAMESPACE_NSEC, @@ -2912,27 +2912,31 @@ previous_closest_nsec(dns_rdatatype_t type, qpz_search_t *search, INSIST(result != ISC_R_NOTFOUND); if (result == ISC_R_SUCCESS) { /* - * Since this was the first loop, finding the - * name in the NSEC tree implies that the first - * node checked in the main tree had an - * unacceptable NSEC record. - * Try the previous node in the NSEC tree. + * If we find an exact match in the NSEC + * namespace on our first attempt, it + * implies that the corresponding node in + * the normal namespace had an unacceptable + * NSEC record; we want the previous node + * in the NSEC tree. */ result = dns_qpiter_prev(nit, name, NULL, NULL); } else if (result == DNS_R_PARTIALMATCH) { /* - * The iterator is already where we want it. + * This was a partial match, so the + * iterator is already at the previous + * node in the NSEC namespace, which is + * what we want. */ dns_qpiter_current(nit, name, NULL, NULL); result = ISC_R_SUCCESS; } } else { /* - * This is a second or later trip through the auxiliary - * tree for the name of a third or earlier NSEC node in - * the main tree. Previous trips through the NSEC tree - * must have found nodes in the main tree with NSEC - * records. Perhaps they lacked signature records. + * We've taken at least two steps back through the + * NSEC namespace. The previous steps must have + * found nodes with NSEC records, but they didn't + * work; perhaps they lacked signature records. + * Keep searching. */ result = dns_qpiter_prev(nit, name, NULL, NULL); } @@ -2949,9 +2953,10 @@ previous_closest_nsec(dns_rdatatype_t type, qpz_search_t *search, } /* - * There should always be a node in the main tree with the - * same name as the node in the auxiliary NSEC tree, except for - * nodes in the auxiliary tree that are awaiting deletion. + * There should always be a node in the normal namespace + * with the same name as the node in the NSEC namespace, + * except when nodes in the NSEC namespace are awaiting + * deletion. */ if (result != DNS_R_PARTIALMATCH && result != ISC_R_NOTFOUND) { isc_log_write(DNS_LOGCATEGORY_DATABASE, @@ -2968,8 +2973,8 @@ previous_closest_nsec(dns_rdatatype_t type, qpz_search_t *search, } /* - * Find the NSEC/NSEC3 which is or before the current point on the - * search chain. For NSEC3 records only NSEC3 records that match the + * Find the NSEC/NSEC3 which is at or before the name being sought. + * For NSEC3 records only NSEC3 records that match the * current NSEC3PARAM record are considered. */ static isc_result_t @@ -2992,8 +2997,12 @@ find_closest_nsec(qpz_search_t *search, dns_dbnode_t **nodep, bool need_sig = secure; /* - * Use the auxiliary tree only starting with the second node in the - * hope that the original node will be right much of the time. + * When a lookup is unsuccessful, the QP iterator will already + * be pointing at the node preceding the searched-for name in + * the normal namespace. We'll check there first, assuming it will + * be right much of the time. If we don't find an NSEC there, + * then we start using the auxiliary NSEC namespace to find + * the next predecessor. */ result = dns_qpiter_current(&search->iter, name, (void **)&node, NULL); if (result != ISC_R_SUCCESS) {