From 70c8054b841340f0cb27558043b96c77eee84951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 15 Sep 2025 18:02:08 +0200 Subject: [PATCH 1/5] Remove CacheNSECNodes statistics counter There is no auxiliary NSEC tree, so we can't count the NSEC nodes separately. Remove the CacheNSECNodes statistics counter as it would be always zero. --- bin/tests/system/synthfromdnssec/tests.sh | 44 ----------------------- lib/dns/cache.c | 7 ---- 2 files changed, 51 deletions(-) diff --git a/bin/tests/system/synthfromdnssec/tests.sh b/bin/tests/system/synthfromdnssec/tests.sh index b2427ec09d..0ce3046f9f 100644 --- a/bin/tests/system/synthfromdnssec/tests.sh +++ b/bin/tests/system/synthfromdnssec/tests.sh @@ -683,21 +683,6 @@ for ns in 2 4 5 6; do if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) - echo_i "check 'rndc stats' output for 'cache NSEC auxiliary database nodes' (synth-from-dnssec ${description};) ($n)" - ret=0 - # 2 views, _bind should always be '0 cache NSEC auxiliary database nodes' - count=$(grep "cache NSEC auxiliary database nodes" ns${ns}/named.stats | wc -l) - test $count = 2 || ret=1 - zero=$(grep "0 cache NSEC auxiliary database nodes" ns${ns}/named.stats | wc -l) - if [ ${ad} = yes ]; then - test $zero = 1 || ret=1 - else - test $zero = 2 || ret=1 - fi - n=$((n + 1)) - if [ $ret != 0 ]; then echo_i "failed"; fi - status=$((status + ret)) - for synthesized in NXDOMAIN no-data wildcard; do case $synthesized in NXDOMAIN) count=2 ;; @@ -740,21 +725,6 @@ for ns in 2 4 5 6; do if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) - echo_i "check XML for 'CacheNSECNodes' with (synth-from-dnssec ${description};) ($n)" - ret=0 - counter=$(sed -n 's;.*.*\([0-9]*\).*0<" | wc -l) - if [ ${ad} = yes ]; then - test $zero = 0 || ret=1 - else - test $zero = 1 || ret=1 - fi - n=$((n + 1)) - if [ $ret != 0 ]; then echo_i "failed"; fi - status=$((status + ret)) - for synthesized in SynthNXDOMAIN SynthNODATA SynthWILDCARD; do case $synthesized in SynthNXDOMAIN) count=2 ;; @@ -800,20 +770,6 @@ for ns in 2 4 5 6; do if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) - echo_i "check JSON for 'CacheNSECNodes' with (synth-from-dnssec ${description};) ($n)" - ret=0 - count=$(grep '"CacheNSECNodes":' $json | wc -l) - test $count = 2 || ret=1 - zero=$(grep '"CacheNSECNodes":0' $json | wc -l) - if [ ${ad} = yes ]; then - test $zero = 1 || ret=1 - else - test $zero = 2 || ret=1 - fi - n=$((n + 1)) - if [ $ret != 0 ]; then echo_i "failed"; fi - status=$((status + ret)) - for synthesized in SynthNXDOMAIN SynthNODATA SynthWILDCARD; do case $synthesized in SynthNXDOMAIN) count=2 ;; diff --git a/lib/dns/cache.c b/lib/dns/cache.c index 69267ea4d4..777de10438 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -679,8 +679,6 @@ dns_cache_renderxml(dns_cache_t *cache, void *writer0) { TRY0(renderstat("CacheNodes", dns_db_nodecount(cache->db, dns_dbtree_main), writer)); - TRY0(renderstat("CacheNSECNodes", - dns_db_nodecount(cache->db, dns_dbtree_nsec), writer)); TRY0(renderstat("TreeMemInUse", isc_mem_inuse(cache->tmctx), writer)); @@ -745,11 +743,6 @@ dns_cache_renderjson(dns_cache_t *cache, void *cstats0) { CHECKMEM(obj); json_object_object_add(cstats, "CacheNodes", obj); - obj = json_object_new_int64( - dns_db_nodecount(cache->db, dns_dbtree_nsec)); - CHECKMEM(obj); - json_object_object_add(cstats, "CacheNSECNodes", obj); - obj = json_object_new_int64(isc_mem_inuse(cache->tmctx)); CHECKMEM(obj); json_object_object_add(cstats, "TreeMemInUse", obj); From ec0a5f3a9dcb1597c1895d769a5f96619b19e3b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 16 Sep 2025 11:45:51 +0200 Subject: [PATCH 2/5] Remove the dbiterator_{last,prev} from the qpcache The dbiterator_{last,prev} functions are not used in the cache, and the implementation would get quite complicated when we squash the main and nsec trees together. It's easier to just not implement these. --- lib/dns/qpcache.c | 66 +++-------------------------------------------- 1 file changed, 4 insertions(+), 62 deletions(-) diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index 06d9a91b04..17caa4a39a 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -3604,39 +3604,8 @@ dbiterator_first(dns_dbiterator_t *iterator DNS__DB_FLARG) { } static isc_result_t -dbiterator_last(dns_dbiterator_t *iterator DNS__DB_FLARG) { - isc_result_t result; - qpc_dbit_t *qpdbiter = (qpc_dbit_t *)iterator; - qpcache_t *qpdb = (qpcache_t *)iterator->db; - - if (qpdbiter->result != ISC_R_SUCCESS && - qpdbiter->result != ISC_R_NOTFOUND && - qpdbiter->result != DNS_R_PARTIALMATCH && - qpdbiter->result != ISC_R_NOMORE) - { - return qpdbiter->result; - } - - if (qpdbiter->paused) { - resume_iteration(qpdbiter, false); - } - - dereference_iter_node(qpdbiter DNS__DB_FLARG_PASS); - - 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. */ - qpdbiter->node = NULL; - } - - qpdbiter->result = result; - return result; +dbiterator_last(dns_dbiterator_t *iterator ISC_ATTR_UNUSED DNS__DB_FLARG) { + return ISC_R_NOTIMPLEMENTED; } static isc_result_t @@ -3677,35 +3646,8 @@ dbiterator_seek(dns_dbiterator_t *iterator, } static isc_result_t -dbiterator_prev(dns_dbiterator_t *iterator DNS__DB_FLARG) { - isc_result_t result; - qpc_dbit_t *qpdbiter = (qpc_dbit_t *)iterator; - - REQUIRE(qpdbiter->node != NULL); - - if (qpdbiter->result != ISC_R_SUCCESS) { - return qpdbiter->result; - } - - if (qpdbiter->paused) { - resume_iteration(qpdbiter, true); - } - - dereference_iter_node(qpdbiter DNS__DB_FLARG_PASS); - - 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); - qpdbiter->node = NULL; - } - - qpdbiter->result = result; - return result; +dbiterator_prev(dns_dbiterator_t *iterator ISC_ATTR_UNUSED DNS__DB_FLARG) { + return ISC_R_NOTIMPLEMENTED; } static isc_result_t From a3e96f2d49d44f971b9feac0574f41d4ea7f12bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 15 Sep 2025 17:46:39 +0200 Subject: [PATCH 3/5] 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. --- lib/dns/qpcache.c | 82 +++++++++++++++++++---------------------------- lib/dns/qpzone.c | 51 +++++++++++++++++------------ 2 files changed, 63 insertions(+), 70 deletions(-) diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index 17caa4a39a..fb551e1c8b 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 e081f15c26..82374dd2fa 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) { From 9e2d5d94bd983bffb3c5042087509a16f6174e69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 15 Sep 2025 18:07:07 +0200 Subject: [PATCH 4/5] Remove dns_dbtree_t and its usage As we removed the ability to count nodes in the auxiliary trees (because there are no auxiliary trees), we can also cleanup the API and associated enum type (dns_dbtree_t). --- bin/dnssec/dnssec-signzone.c | 3 +-- bin/named/server.c | 2 +- bin/tests/system/dyndb/driver/db.c | 4 ++-- lib/dns/cache.c | 10 +++------- lib/dns/db.c | 4 ++-- lib/dns/include/dns/db.h | 6 +++--- lib/dns/include/dns/types.h | 6 ------ lib/dns/qpcache.c | 2 +- lib/dns/qpzone.c | 6 ++---- lib/dns/zone.c | 3 +-- 10 files changed, 16 insertions(+), 30 deletions(-) diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 025f5703ff..496cfbfeee 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -3826,8 +3826,7 @@ main(int argc, char *argv[]) { bool answer; hash_length = dns_nsec3_hashlength(dns_hash_sha1); - hashlist_init(&hashlist, - dns_db_nodecount(gdb, dns_dbtree_main) * 2, + hashlist_init(&hashlist, dns_db_nodecount(gdb) * 2, hash_length); result = dns_nsec_nseconly(gdb, gversion, NULL, &answer); if (result == ISC_R_NOTFOUND) { diff --git a/bin/named/server.c b/bin/named/server.c index 8448b3d715..3ae0d826ed 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -14824,7 +14824,7 @@ named_server_zonestatus(named_server_t *server, isc_lex_t *lex, } /* Database node count */ - nodes = dns_db_nodecount(hasraw ? rawdb : db, dns_dbtree_main); + nodes = dns_db_nodecount(hasraw ? rawdb : db); snprintf(nodebuf, sizeof(nodebuf), "%u", nodes); /* Security */ diff --git a/bin/tests/system/dyndb/driver/db.c b/bin/tests/system/dyndb/driver/db.c index f23e0793c8..c108bc3183 100644 --- a/bin/tests/system/dyndb/driver/db.c +++ b/bin/tests/system/dyndb/driver/db.c @@ -249,12 +249,12 @@ issecure(dns_db_t *db) { } static unsigned int -nodecount(dns_db_t *db, dns_dbtree_t tree) { +nodecount(dns_db_t *db) { sampledb_t *sampledb = (sampledb_t *)db; REQUIRE(VALID_SAMPLEDB(sampledb)); - return dns_db_nodecount(sampledb->db, tree); + return dns_db_nodecount(sampledb->db); } static isc_result_t diff --git a/lib/dns/cache.c b/lib/dns/cache.c index 777de10438..0fbce6c372 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -618,10 +618,8 @@ dns_cache_dumpstats(dns_cache_t *cache, FILE *fp) { fprintf(fp, "%20" PRIu64 " %s\n", values[dns_cachestatscounter_coveringnsec], "covering nsec returned"); - fprintf(fp, "%20u %s\n", dns_db_nodecount(cache->db, dns_dbtree_main), + fprintf(fp, "%20u %s\n", dns_db_nodecount(cache->db), "cache database nodes"); - fprintf(fp, "%20u %s\n", dns_db_nodecount(cache->db, dns_dbtree_nsec), - "cache NSEC auxiliary database nodes"); fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_inuse(cache->tmctx), "cache tree memory in use"); @@ -677,8 +675,7 @@ dns_cache_renderxml(dns_cache_t *cache, void *writer0) { TRY0(renderstat("CoveringNSEC", values[dns_cachestatscounter_coveringnsec], writer)); - TRY0(renderstat("CacheNodes", - dns_db_nodecount(cache->db, dns_dbtree_main), writer)); + TRY0(renderstat("CacheNodes", dns_db_nodecount(cache->db), writer)); TRY0(renderstat("TreeMemInUse", isc_mem_inuse(cache->tmctx), writer)); @@ -738,8 +735,7 @@ dns_cache_renderjson(dns_cache_t *cache, void *cstats0) { CHECKMEM(obj); json_object_object_add(cstats, "CoveringNSEC", obj); - obj = json_object_new_int64( - dns_db_nodecount(cache->db, dns_dbtree_main)); + obj = json_object_new_int64(dns_db_nodecount(cache->db)); CHECKMEM(obj); json_object_object_add(cstats, "CacheNodes", obj); diff --git a/lib/dns/db.c b/lib/dns/db.c index 9200352024..2ab84644c9 100644 --- a/lib/dns/db.c +++ b/lib/dns/db.c @@ -734,11 +734,11 @@ freenode: } unsigned int -dns_db_nodecount(dns_db_t *db, dns_dbtree_t tree) { +dns_db_nodecount(dns_db_t *db) { REQUIRE(DNS_DB_VALID(db)); if (db->methods->nodecount != NULL) { - return (db->methods->nodecount)(db, tree); + return (db->methods->nodecount)(db); } return 0; } diff --git a/lib/dns/include/dns/db.h b/lib/dns/include/dns/db.h index 7ebeeb2253..8f0de40c5c 100644 --- a/lib/dns/include/dns/db.h +++ b/lib/dns/include/dns/db.h @@ -127,7 +127,7 @@ typedef struct dns_db_methods { dns_rdatatype_t type, dns_rdatatype_t covers DNS__DB_FLARG); bool (*issecure)(dns_db_t *db); - unsigned int (*nodecount)(dns_db_t *db, dns_dbtree_t); + unsigned int (*nodecount)(dns_db_t *db); isc_result_t (*getoriginnode)(dns_db_t *db, dns_dbnode_t **nodep DNS__DB_FLARG); isc_result_t (*getnsec3parameters)(dns_db_t *db, @@ -1366,9 +1366,9 @@ dns_db_getsoaserial(dns_db_t *db, dns_dbversion_t *ver, uint32_t *serialp); */ unsigned int -dns_db_nodecount(dns_db_t *db, dns_dbtree_t tree); +dns_db_nodecount(dns_db_t *db); /*%< - * Count the number of nodes in 'db' or its auxiliary trees. + * Count the number of nodes in 'db'. * * Requires: * diff --git a/lib/dns/include/dns/types.h b/lib/dns/include/dns/types.h index 931910f9a8..ba1fd98a3d 100644 --- a/lib/dns/include/dns/types.h +++ b/lib/dns/include/dns/types.h @@ -202,12 +202,6 @@ typedef enum { dns_dbtype_stub = 3 } dns_dbtype_t; -typedef enum { - dns_dbtree_main = 0, - dns_dbtree_nsec = 1, - dns_dbtree_nsec3 = 2 -} dns_dbtree_t; - typedef enum { dns_checkdstype_no = 0, dns_checkdstype_yes = 1, diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index fb551e1c8b..efc987c12c 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -3218,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 ISC_ATTR_UNUSED) { +nodecount(dns_db_t *db) { qpcache_t *qpdb = (qpcache_t *)db; dns_qp_memusage_t mu; isc_rwlocktype_t tlocktype = isc_rwlocktype_none; diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index 82374dd2fa..9eaf4f28e7 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -3867,12 +3867,10 @@ qpzone_detachnode(dns_dbnode_t **nodep DNS__DB_FLARG) { } static unsigned int -nodecount(dns_db_t *db, dns_dbtree_t tree ISC_ATTR_UNUSED) { - qpzonedb_t *qpdb = NULL; +nodecount(dns_db_t *db) { + qpzonedb_t *qpdb = qpdb = (qpzonedb_t *)db; dns_qp_memusage_t mu; - qpdb = (qpzonedb_t *)db; - REQUIRE(VALID_QPZONE(qpdb)); mu = dns_qpmulti_memusage(qpdb->tree); diff --git a/lib/dns/zone.c b/lib/dns/zone.c index e12ecae892..68685dcad2 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -5413,8 +5413,7 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime, } dns_zone_logc(zone, DNS_LOGCATEGORY_ZONELOAD, ISC_LOG_DEBUG(2), - "number of nodes in database: %u", - dns_db_nodecount(db, dns_dbtree_main)); + "number of nodes in database: %u", dns_db_nodecount(db)); if (result == DNS_R_SEENINCLUDE) { DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_HASINCLUDE); From cdc6950d04f994617a0b522b580d3b8b35c68cc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 16 Sep 2025 12:01:44 +0200 Subject: [PATCH 5/5] Add more unit tests for dns_qp unit Add basic unit tests and add missing DbC checks for mandatory dns_qp_create() arguments. --- lib/dns/qp.c | 2 ++ tests/dns/qp_test.c | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/lib/dns/qp.c b/lib/dns/qp.c index d27726d74f..625e4675b3 100644 --- a/lib/dns/qp.c +++ b/lib/dns/qp.c @@ -1494,6 +1494,8 @@ dns_qpsnap_destroy(dns_qpmulti_t *multi, dns_qpsnap_t **qpsp) { void dns_qp_create(isc_mem_t *mctx, const dns_qpmethods_t *methods, void *uctx, dns_qp_t **qptp) { + REQUIRE(mctx != NULL); + REQUIRE(methods != NULL); REQUIRE(qptp != NULL && *qptp == NULL); dns_qp_t *qp = isc_mem_get(mctx, sizeof(*qp)); diff --git a/tests/dns/qp_test.c b/tests/dns/qp_test.c index e93d20040d..e6729056e9 100644 --- a/tests/dns/qp_test.c +++ b/tests/dns/qp_test.c @@ -40,6 +40,10 @@ #include #include +#define DONT_REORDER + +#include "../qp.c" + bool verbose = false; ISC_RUN_TEST_IMPL(qpkey_name) { @@ -2059,7 +2063,39 @@ ISC_RUN_TEST_IMPL(qpkey_delete) { dns_qp_destroy(&qp); } +ISC_RUN_TEST_IMPL(qp_basics) { + dns_qp_t *qp = NULL; + expect_assert_failure( + dns_qp_create(isc_g_mctx, &string_methods, NULL, NULL)); + expect_assert_failure(dns_qp_create(NULL, &string_methods, NULL, &qp)); + expect_assert_failure(dns_qp_create(isc_g_mctx, NULL, NULL, &qp)); + + qp = NULL; + dns_qp_create(isc_g_mctx, &string_methods, NULL, &qp); + assert_non_null(qp); + + dns_qp_destroy(&qp); + assert_null(qp); +} + +ISC_RUN_TEST_IMPL(qp_memusage) { + dns_qp_t *qp = NULL; + dns_qp_memusage_t mu; + + dns_qp_create(isc_g_mctx, &string_methods, NULL, &qp); + assert_non_null(qp); + + mu = dns_qp_memusage(qp); + assert_int_equal(mu.leaves, 0); + assert_int_equal(mu.used, 0); + + dns_qp_destroy(&qp); + assert_null(qp); +} + ISC_TEST_LIST_START +ISC_TEST_ENTRY(qp_basics) +ISC_TEST_ENTRY(qp_memusage) ISC_TEST_ENTRY(qpkey_name) ISC_TEST_ENTRY(qpkey_sort) ISC_TEST_ENTRY(qpiter)