From c53b95e1343ca02fbe8c3ef4a0b3f02c62ee45dc Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Wed, 10 Jan 2024 16:29:57 +0100 Subject: [PATCH] Replace rbt_addnode with qp_insert Replace dns_rbt_addnode calls with dns_qp_insert. With QP, it sometimes makes more sense to first lookup the name and see if there is an existing node (rather than create new data, insert, find out a node already exists, and destroy the data again). This is done with dns_qp_getname(), which is more lightweight than dns_qp_lookup(), and we are only interested in if there is already a leaf node for this name or not. --- lib/dns/qp-zonedb.c | 95 +++++++++++++++++++++++---------------------- lib/dns/qpdb.c | 70 +++++++++++++++------------------ 2 files changed, 79 insertions(+), 86 deletions(-) diff --git a/lib/dns/qp-zonedb.c b/lib/dns/qp-zonedb.c index 63476caf47..a73820e16b 100644 --- a/lib/dns/qp-zonedb.c +++ b/lib/dns/qp-zonedb.c @@ -1608,14 +1608,17 @@ delegating_type(dns_qpdb_t *qpdb, dns_rbtnode_t *node, dns_typepair_t type) { static isc_result_t loadnode(dns_qpdb_t *qpdb, const dns_name_t *name, dns_rbtnode_t **nodep, bool hasnsec) { - isc_result_t noderesult, nsecresult, tmpresult; + isc_result_t noderesult, nsecresult; dns_rbtnode_t *nsecnode = NULL, *node = NULL; - noderesult = dns_rbt_addnode(qpdb->tree, name, &node); - if (!hasnsec) { - goto done; - } - if (noderesult == ISC_R_EXISTS) { + noderesult = dns_qp_getname(qpdb->tree, name, (void **)&node, NULL); + if (noderesult != ISC_R_SUCCESS) { + INSIST(node == NULL); + node = dns_qpdata_create(qpdb, name); + noderesult = dns_qp_insert(qpdb->tree, node, 0); + INSIST(noderesult == ISC_R_SUCCESS); + dns_qpdata_unref(node); + } else if (noderesult == ISC_R_SUCCESS) { /* * Add a node to the auxiliary NSEC tree for an old node * just now getting an NSEC record. @@ -1623,7 +1626,11 @@ loadnode(dns_qpdb_t *qpdb, const dns_name_t *name, dns_rbtnode_t **nodep, if (node->nsec == DNS_DB_NSEC_HAS_NSEC) { goto done; } - } else if (noderesult != ISC_R_SUCCESS) { + } else { + goto done; + } + + if (!hasnsec) { goto done; } @@ -1635,14 +1642,8 @@ loadnode(dns_qpdb_t *qpdb, const dns_name_t *name, dns_rbtnode_t **nodep, * Add nodes to the auxiliary tree after corresponding nodes have * been added to the main tree. */ - nsecresult = dns_rbt_addnode(qpdb->nsec, name, &nsecnode); + nsecresult = dns_qp_getname(qpdb->nsec, name, (void **)&nsecnode, NULL); if (nsecresult == ISC_R_SUCCESS) { - nsecnode->nsec = DNS_DB_NSEC_NSEC; - node->nsec = DNS_DB_NSEC_HAS_NSEC; - goto done; - } - - if (nsecresult == ISC_R_EXISTS) { #if 1 /* 0 */ isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_CACHE, ISC_LOG_WARNING, @@ -1650,28 +1651,14 @@ loadnode(dns_qpdb_t *qpdb, const dns_name_t *name, dns_rbtnode_t **nodep, #endif /* if 1 */ node->nsec = DNS_DB_NSEC_HAS_NSEC; goto done; + } else { + INSIST(nsecnode == NULL); + nsecnode = dns_qpdata_create(qpdb, name); + nsecresult = dns_qp_insert(qpdb->nsec, nsecnode, 0); + INSIST(nsecresult == ISC_R_SUCCESS); } - - if (noderesult == ISC_R_SUCCESS) { - /* - * Remove the node we just added above. - */ - tmpresult = dns_rbt_deletenode(qpdb->tree, node, false); - if (tmpresult != ISC_R_SUCCESS) { - isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE, - DNS_LOGMODULE_CACHE, ISC_LOG_WARNING, - "loading_addrdataset: " - "dns_rbt_deletenode: %s after " - "dns_rbt_addnode(NSEC): %s", - isc_result_totext(tmpresult), - isc_result_totext(noderesult)); - } - } - - /* - * Set the error condition to be returned. - */ - noderesult = nsecresult; + nsecnode->nsec = DNS_DB_NSEC_NSEC; + node->nsec = DNS_DB_NSEC_HAS_NSEC; done: if (noderesult == ISC_R_SUCCESS || noderesult == ISC_R_EXISTS) { @@ -1731,10 +1718,15 @@ loading_addrdataset(void *arg, const dns_name_t *name, if (rdataset->type == dns_rdatatype_nsec3 || rdataset->covers == dns_rdatatype_nsec3) { - result = dns_rbt_addnode(qpdb->nsec3, name, &node); - if (result == ISC_R_SUCCESS) { - node->nsec = DNS_DB_NSEC_NSEC3; + result = dns_qp_getname(qpdb->nsec3, name, (void **)&node, + NULL); + if (result != ISC_R_SUCCESS) { + INSIST(node == NULL); + node = dns_qpdata_create(qpdb, name); + result = dns_qp_insert(qpdb->nsec3, node, 0); + INSIST(result == ISC_R_SUCCESS); } + node->nsec = DNS_DB_NSEC_NSEC3; } else if (rdataset->type == dns_rdatatype_nsec) { result = loadnode(qpdb, name, &node, true); } else { @@ -2462,13 +2454,17 @@ dns__qpzone_wildcardmagic(dns_qpdb_t *qpdb, const dns_name_t *name, bool lock) { INSIST(n >= 2); n--; dns_name_getlabelsequence(name, 1, n, &foundname); - result = dns_rbt_addnode(qpdb->tree, &foundname, &node); - if (result != ISC_R_SUCCESS && result != ISC_R_EXISTS) { - return (result); - } - if (result == ISC_R_SUCCESS) { - node->nsec = DNS_DB_NSEC_NORMAL; + + result = dns_qp_getname(qpdb->tree, &foundname, (void **)&node, NULL); + if (result != ISC_R_SUCCESS) { + INSIST(node == NULL); + node = dns_qpdata_create(qpdb, &foundname); + result = dns_qp_insert(qpdb->tree, node, 0); + INSIST(result == ISC_R_SUCCESS); } + + INSIST(result == ISC_R_SUCCESS); + node->nsec = DNS_DB_NSEC_NORMAL; node->find_callback = 1; if (lock) { NODE_WRLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); @@ -2500,9 +2496,14 @@ dns__qpzone_addwildcards(dns_qpdb_t *qpdb, const dns_name_t *name, bool lock) { if (result != ISC_R_SUCCESS) { return (result); } - result = dns_rbt_addnode(qpdb->tree, &foundname, &node); - if (result != ISC_R_SUCCESS && result != ISC_R_EXISTS) { - return (result); + + result = dns_qp_getname(qpdb->tree, name, + (void **)&node, NULL); + if (result != ISC_R_SUCCESS) { + INSIST(node == NULL); + node = dns_qpdata_create(qpdb, name); + result = dns_qp_insert(qpdb->tree, node, 0); + INSIST(result == ISC_R_SUCCESS); } if (result == ISC_R_SUCCESS) { node->nsec = DNS_DB_NSEC_NORMAL; diff --git a/lib/dns/qpdb.c b/lib/dns/qpdb.c index 96ce0341e1..af9e0efeb8 100644 --- a/lib/dns/qpdb.c +++ b/lib/dns/qpdb.c @@ -2013,29 +2013,23 @@ dns__qpdb_findnodeintree(dns_qpdb_t *qpdb, dns_qp_t *tree, * Try to upgrade the lock and if that fails unlock then relock. */ TREE_FORCEUPGRADE(&qpdb->tree_lock, &tlocktype); - node = NULL; - result = dns_rbt_addnode(tree, name, &node); - if (result == ISC_R_SUCCESS) { - dns_rbt_namefromnode(node, &nodename); - node->locknum = node->hashval % qpdb->node_lock_count; - if (tree == qpdb->tree) { - dns__qpzone_addwildcards(qpdb, name, true); + node = dns_qpdata_create(qpdb, name); + result = dns_qp_insert(tree, node, 0); + INSIST(result == ISC_R_SUCCESS); - if (dns_name_iswildcard(name)) { - result = dns__qpzone_wildcardmagic( - qpdb, name, true); - if (result != ISC_R_SUCCESS) { - goto unlock; - } + if (tree == qpdb->tree) { + dns__qpzone_addwildcards(qpdb, name, true); + + if (dns_name_iswildcard(name)) { + result = dns__qpzone_wildcardmagic(qpdb, name, + true); + if (result != ISC_R_SUCCESS) { + goto unlock; } } - if (tree == qpdb->nsec3) { - node->nsec = DNS_DB_NSEC_NSEC3; - } - } else if (result == ISC_R_EXISTS) { - result = ISC_R_SUCCESS; - } else { - goto unlock; + } + if (tree == qpdb->nsec3) { + node->nsec = DNS_DB_NSEC_NSEC3; } } @@ -3337,14 +3331,18 @@ dns__qpdb_addrdataset(dns_db_t *db, dns_dbnode_t *node, if (newnsec) { dns_rbtnode_t *nsecnode = NULL; - result = dns_rbt_addnode(qpdb->nsec, name, &nsecnode); + result = dns_qp_getname(qpdb->nsec, name, (void **)&nsecnode, + NULL); if (result == ISC_R_SUCCESS) { - nsecnode->nsec = DNS_DB_NSEC_NSEC; - rbtnode->nsec = DNS_DB_NSEC_HAS_NSEC; - } else if (result == ISC_R_EXISTS) { - rbtnode->nsec = DNS_DB_NSEC_HAS_NSEC; result = ISC_R_SUCCESS; + } else { + INSIST(nsecnode == NULL); + nsecnode = dns_qpdata_create(qpdb, name); + nsecnode->nsec = DNS_DB_NSEC_NSEC; + result = dns_qp_insert(qpdb->nsec, nsecnode, 0); + INSIST(result == ISC_R_SUCCESS); } + rbtnode->nsec = DNS_DB_NSEC_HAS_NSEC; } if (result == ISC_R_SUCCESS) { @@ -3889,27 +3887,21 @@ dns__qpdb_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type, * change. */ if (!IS_CACHE(qpdb)) { - result = dns_rbt_addnode(qpdb->tree, &qpdb->common.origin, - &qpdb->origin_node); - if (result != ISC_R_SUCCESS) { - INSIST(result != ISC_R_EXISTS); - free_qpdb(qpdb, false); - return (result); - } + qpdb->origin_node = dns_qpdata_create(qpdb, + &qpdb->common.origin); + result = dns_qp_insert(qpdb->tree, qpdb->origin_node, 0); + INSIST(result == ISC_R_SUCCESS); INSIST(qpdb->origin_node != NULL); qpdb->origin_node->nsec = DNS_DB_NSEC_NORMAL; + /* * Add an apex node to the NSEC3 tree so that NSEC3 searches * return partial matches when there is only a single NSEC3 * record in the tree. */ - result = dns_rbt_addnode(qpdb->nsec3, &qpdb->common.origin, - &qpdb->nsec3_origin_node); - if (result != ISC_R_SUCCESS) { - INSIST(result != ISC_R_EXISTS); - free_qpdb(qpdb, false); - return (result); - } + qpdb->nsec3_origin_node = + dns_qpdata_create(qpdb, &qpdb->common.origin); + result = dns_qp_insert(qpdb->nsec3, qpdb->nsec3_origin_node, 0); INSIST(result == ISC_R_SUCCESS); INSIST(qpdb->nsec3_origin_node != NULL); qpdb->nsec3_origin_node->nsec = DNS_DB_NSEC_NSEC3;