From f4b4f030c453a9152e2e31f43067dd9d88778248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Sat, 31 Jan 2026 07:24:49 +0100 Subject: [PATCH 1/2] Cleanup the duplicate logic and comments around add into NSEC tree After merging the NORMAL, NSEC and NSEC3 tree into single QP tree, there were some comments still speaking about auxiliary NSEC tree. These were cleaned up and the logic when we pass the qp tree (write transaction) to qpzone_addrdataset_inner() was changed to be more obvious that this is needed only when we are adding NSEC records. --- lib/dns/qpzone.c | 49 ++++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index a9d8f56854..8d012d2f25 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -2462,7 +2462,7 @@ findnodeintree(qpzonedb_t *qpdb, dns_qp_t *qp, const dns_name_t *name, INSIST(node->nspace == DNS_DBNAMESPACE_NSEC3 || !nsec3); } else if (result != ISC_R_SUCCESS && create) { /* - * ... if the lookup is unsuccesful, but the caller asked us to + * ... if the lookup is unsuccessful, but the caller asked us to * create a new node, create one and insert it into the tree. */ node = new_qpznode(qpdb, name, nspace); @@ -4683,14 +4683,11 @@ qpzone_createiterator(dns_db_t *db, unsigned int options, * nodes, only rdatasets. */ static isc_result_t -qpzone_addrdataset_inner(dns_db_t *db, dns_dbnode_t *dbnode, - dns_dbversion_t *dbversion, - isc_stdtime_t now ISC_ATTR_UNUSED, - dns_rdataset_t *rdataset, unsigned int options, - dns_rdataset_t *addedrdataset, dns_qp_t *nsec) { +qpzone_addrdataset_inner(qpzonedb_t *qpdb, qpznode_t *node, + dns_dbversion_t *dbversion, dns_rdataset_t *rdataset, + unsigned int options, dns_rdataset_t *addedrdataset, + dns_qp_t *nsec) { isc_result_t result; - qpzonedb_t *qpdb = (qpzonedb_t *)db; - qpznode_t *node = (qpznode_t *)dbnode; qpz_version_t *version = (qpz_version_t *)dbversion; isc_region_t region; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; @@ -4749,13 +4746,7 @@ qpzone_addrdataset_inner(dns_db_t *db, dns_dbnode_t *dbnode, } /* - * Add to the auxiliary NSEC tree if we're adding an NSEC record. - */ - bool is_nsec = !node->havensec && rdataset->type == dns_rdatatype_nsec; - REQUIRE(!is_nsec || nsec != NULL); - - /* - * If we're adding a delegation type or adding to the auxiliary NSEC + * If we're adding a delegation type or adding NSEC records * tree hold an exclusive lock on the tree. In the latter case the * lock does not necessarily have to be acquired but it will help * purge ancient entries more effectively. @@ -4768,7 +4759,7 @@ qpzone_addrdataset_inner(dns_db_t *db, dns_dbnode_t *dbnode, NODE_WRLOCK(nlock, &nlocktype); result = ISC_R_SUCCESS; - if (is_nsec) { + if (nsec != NULL) { node->havensec = true; /* @@ -4819,15 +4810,14 @@ qpzone_addrdataset(dns_db_t *db, dns_dbnode_t *dbnode, REQUIRE(VALID_QPZONE(qpdb)); /* - * Add to the auxiliary NSEC tree if we're adding an NSEC record. + * Open a new write transaction if we're adding an NSEC record. */ if (!node->havensec && rdataset->type == dns_rdatatype_nsec) { dns_qpmulti_write(qpdb->tree, &nsec); } - isc_result_t result = qpzone_addrdataset_inner(db, dbnode, dbversion, - now, rdataset, options, - addedrdataset, nsec); + isc_result_t result = qpzone_addrdataset_inner( + qpdb, node, dbversion, rdataset, options, addedrdataset, nsec); if (nsec != NULL) { dns_qpmulti_commit(qpdb->tree, &nsec); @@ -5391,7 +5381,8 @@ qpzone_update_rdataset(qpzonedb_t *qpdb, qpz_version_t *version, dns_qp_t *qp, isc_result_t result; unsigned int options; dns_rdataset_t ardataset; - dns_dbnode_t *node = NULL; + qpznode_t *node = NULL; + dns_qp_t *nsec = NULL; bool is_nsec3; dns_rdataset_init(&ardataset); @@ -5400,7 +5391,7 @@ qpzone_update_rdataset(qpzonedb_t *qpdb, qpz_version_t *version, dns_qp_t *qp, rds->covers == dns_rdatatype_nsec3); result = findnodeintree(qpdb, qp, name, true, is_nsec3, - &node DNS__DB_FLARG_PASS); + (dns_dbnode_t **)&node DNS__DB_FLARG_PASS); if (result != ISC_R_SUCCESS) { goto failure; @@ -5415,16 +5406,20 @@ qpzone_update_rdataset(qpzonedb_t *qpdb, qpz_version_t *version, dns_qp_t *qp, */ options = DNS_DBADD_MERGE | DNS_DBADD_EXACT | DNS_DBADD_EXACTTTL; + if (!node->havensec && ardataset.type == dns_rdatatype_nsec) { + nsec = qp; + } result = qpzone_addrdataset_inner( - (dns_db_t *)qpdb, node, (dns_dbversion_t *)version, 0, - rds, options, &ardataset, qp DNS__DB_FLARG_PASS); + qpdb, node, (dns_dbversion_t *)version, rds, options, + &ardataset, nsec DNS__DB_FLARG_PASS); break; case DNS_DIFFOP_DEL: case DNS_DIFFOP_DELRESIGN: options = DNS_DBSUB_EXACT | DNS_DBSUB_WANTOLD; result = qpzone_subtractrdataset( - (dns_db_t *)qpdb, node, (dns_dbversion_t *)version, rds, - options, &ardataset DNS__DB_FLARG_PASS); + (dns_db_t *)qpdb, (dns_dbnode_t *)node, + (dns_dbversion_t *)version, rds, options, + &ardataset DNS__DB_FLARG_PASS); break; default: UNREACHABLE(); @@ -5442,7 +5437,7 @@ qpzone_update_rdataset(qpzonedb_t *qpdb, qpz_version_t *version, dns_qp_t *qp, failure: if (node != NULL) { - dns_db_detachnode(&node); + dns_db_detachnode((dns_dbnode_t **)&node); } dns_rdataset_cleanup(&ardataset); return result; From 6e286beaa6edf3085ad17dd07db7b99d7d985169 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Sat, 31 Jan 2026 07:32:08 +0100 Subject: [PATCH 2/2] Cleanup weird syntax defining struct dns_ixfr The struct dns_ixfr was defined as part of struct dns_xfrin, probably because at some point it was an anonymous struct and then it was changed to named struct with typedef at the top. Move the definition from struct dns_xfrin into and fold into the typedef ... dns_ixfr_t. --- lib/dns/qpzone.c | 12 ++++++++---- lib/dns/xfrin.c | 16 ++++++++-------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index 8d012d2f25..2993d833c0 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -4679,6 +4679,9 @@ qpzone_createiterator(dns_db_t *db, unsigned int options, * The reason this is split from qpzone_addrdataset is to allow the reuse of * the same qp transaction for multiple adds. * + * If the rdataset is of type NSEC, 'nsec' must point to the qp trie for the + * zone, otherwise it must be NULL. + * * qpzone_subtractrdataset doesn't have the same problem since it cannot delete * nodes, only rdatasets. */ @@ -4746,9 +4749,9 @@ qpzone_addrdataset_inner(qpzonedb_t *qpdb, qpznode_t *node, } /* - * If we're adding a delegation type or adding NSEC records - * tree hold an exclusive lock on the tree. In the latter case the - * lock does not necessarily have to be acquired but it will help + * If we're adding a delegation type or adding to the auxiliary NSEC + * namespace, hold an exclusive lock on the tree. In the latter case + * the lock does not necessarily have to be acquired but it will help * purge ancient entries more effectively. * * (Note: node lock must be acquired after starting @@ -4810,7 +4813,8 @@ qpzone_addrdataset(dns_db_t *db, dns_dbnode_t *dbnode, REQUIRE(VALID_QPZONE(qpdb)); /* - * Open a new write transaction if we're adding an NSEC record. + * Open a new write transaction if we're adding to the auxiliary + * NSEC namespace. */ if (!node->havensec && rdataset->type == dns_rdatatype_nsec) { dns_qpmulti_write(qpdb->tree, &nsec); diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c index 902abc34d9..3ecac75281 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -80,7 +80,13 @@ typedef enum { * Incoming zone transfer context. */ -typedef struct dns_ixfr dns_ixfr_t; +typedef struct dns_ixfr { + uint32_t diffs; + uint32_t maxdiffs; + uint32_t request_serial; + uint32_t current_serial; + dns_journal_t *journal; +} dns_ixfr_t; struct dns_xfrin { unsigned int magic; @@ -174,13 +180,7 @@ struct dns_xfrin { */ dns_rdatacallbacks_t axfr; - struct dns_ixfr { - uint32_t diffs; - uint32_t maxdiffs; - uint32_t request_serial; - uint32_t current_serial; - dns_journal_t *journal; - } ixfr; + dns_ixfr_t ixfr; dns_rdata_t firstsoa; unsigned char *firstsoa_data;