From: Tony Finch Date: Tue, 14 Feb 2023 16:13:16 +0000 (+0000) Subject: Use a qp-trie for the zone table X-Git-Tag: v9.19.12~10^2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b171cacf4f0123ba96bef6eedfc92dfb608db6b7;p=thirdparty%2Fbind9.git Use a qp-trie for the zone table This change makes the zone table lock-free for reads. Previously, the zone table used a red-black tree, which is not thread safe, so the hot read path acquired both the per-view mutex and the per-zonetable rwlock. (The double locking was to fix to cleanup races on shutdown.) One visible difference is that zones are not necessarily shut down promptly: it depends on when the qp-trie garbage collector cleans up the zone table. The `catz` system test checks several times that zones have been deleted; the test now checks for zones to be removed from the server configuration, instead of being fully shut down. The catz test does not churn through enough zones to trigger a gc, so the zones are not fully detached until the server exits. After this change, it is still possible to improve the way we handle changes to the zone table, for instance, batching changes, or better compaction heuristics. --- diff --git a/CHANGES b/CHANGES index b1ce3a727b9..4634446aa1d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6146. [performance] Replace the zone table red-black tree and associated + locking with a lock-free qp-trie. [GL !7582] + 6145. [bug] Fix a possible use-after-free bug in the dns__catz_done_cb() function. [GL #3997] diff --git a/bin/delv/delv.c b/bin/delv/delv.c index a354a60e65d..91ebe623481 100644 --- a/bin/delv/delv.c +++ b/bin/delv/delv.c @@ -2134,7 +2134,8 @@ run_server(void *arg) { CHECK(ns_interfacemgr_create(mctx, sctx, loopmgr, netmgr, dispatchmgr, NULL, false, &interfacemgr)); - CHECK(dns_view_create(mctx, dns_rdataclass_in, "_default", &view)); + CHECK(dns_view_create(mctx, loopmgr, dns_rdataclass_in, "_default", + &view)); CHECK(dns_cache_create(loopmgr, dns_rdataclass_in, "", &cache)); dns_view_setcache(view, cache, false); dns_cache_detach(&cache); diff --git a/bin/named/server.c b/bin/named/server.c index 8bc4e32db99..8e3ba56205d 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2708,7 +2708,7 @@ catz_addmodzone_cb(void *arg) { goto cleanup; } - result = dns_zt_find(cz->view->zonetable, name, 0, NULL, &zone); + result = dns_view_findzone(cz->view, name, &zone); if (cz->mod) { dns_catz_zone_t *parentcatz; @@ -2780,19 +2780,8 @@ catz_addmodzone_cb(void *arg) { nameb); } goto cleanup; - } else if (result != ISC_R_NOTFOUND && - result != DNS_R_PARTIALMATCH) - { - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, - "catz: error \"%s\" while trying to " - "add zone '%s'", - isc_result_totext(result), nameb); - goto cleanup; - } else { /* this can happen in case of DNS_R_PARTIALMATCH */ - if (zone != NULL) { - dns_zone_detach(&zone); - } + } else { + RUNTIME_CHECK(result == ISC_R_NOTFOUND); } } RUNTIME_CHECK(zone == NULL); @@ -2845,7 +2834,7 @@ catz_addmodzone_cb(void *arg) { } /* Is it there yet? */ - CHECK(dns_zt_find(cz->view->zonetable, name, 0, NULL, &zone)); + CHECK(dns_view_findzone(cz->view, name, &zone)); /* * Load the zone from the master file. If this fails, we'll @@ -2901,8 +2890,8 @@ catz_delzone_cb(void *arg) { dns_name_format(dns_catz_entry_getname(cz->entry), cname, DNS_NAME_FORMATSIZE); - result = dns_zt_find(cz->view->zonetable, - dns_catz_entry_getname(cz->entry), 0, NULL, &zone); + result = dns_view_findzone(cz->view, dns_catz_entry_getname(cz->entry), + &zone); if (result != ISC_R_SUCCESS) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, @@ -6448,7 +6437,8 @@ create_view(const cfg_obj_t *vconfig, dns_viewlist_t *viewlist, } INSIST(view == NULL); - result = dns_view_create(named_g_mctx, viewclass, viewname, &view); + result = dns_view_create(named_g_mctx, named_g_loopmgr, viewclass, + viewname, &view); if (result != ISC_R_SUCCESS) { return (result); } @@ -9734,8 +9724,8 @@ cleanup_viewlist: if (result == ISC_R_SUCCESS && strcmp(view->name, "_bind") != 0) { dns_view_setviewrevert(view); - (void)dns_zt_apply(view->zonetable, isc_rwlocktype_read, - false, NULL, removed, view); + (void)dns_zt_apply(view->zonetable, false, NULL, + removed, view); } dns_view_detach(&view); } @@ -10564,8 +10554,7 @@ zone_from_args(named_server_t *server, isc_lex_t *lex, const char *zonetxt, result = ISC_R_NOTFOUND; } } else { - result = dns_zt_find(view->zonetable, name, 0, NULL, - zonep); + result = dns_view_findzone(view, name, zonep); } if (result != ISC_R_SUCCESS) { snprintf(problem, sizeof(problem), @@ -11304,8 +11293,8 @@ add_view_tolist(struct dumpcontext *dctx, dns_view_t *view) { ISC_LIST_INIT(vle->zonelist); ISC_LIST_APPEND(dctx->viewlist, vle, link); if (dctx->dumpzones) { - result = dns_zt_apply(view->zonetable, isc_rwlocktype_read, - true, NULL, add_zone_tolist, dctx); + result = dns_zt_apply(view->zonetable, true, NULL, + add_zone_tolist, dctx); } return (result); } @@ -12402,8 +12391,7 @@ named_server_sync(named_server_t *server, isc_lex_t *lex, isc_buffer_t **text) { for (view = ISC_LIST_HEAD(server->viewlist); view != NULL; view = ISC_LIST_NEXT(view, link)) { - result = dns_zt_apply(view->zonetable, - isc_rwlocktype_none, false, NULL, + result = dns_zt_apply(view->zonetable, false, NULL, synczone, &cleanup); if (result != ISC_R_SUCCESS && tresult == ISC_R_SUCCESS) { @@ -13430,19 +13418,15 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, /* Zone shouldn't already exist */ if (redirect) { - result = (view->redirect != NULL) ? ISC_R_SUCCESS - : ISC_R_NOTFOUND; + result = (view->redirect == NULL) ? ISC_R_NOTFOUND + : ISC_R_EXISTS; } else { - result = dns_zt_find(view->zonetable, name, 0, NULL, &zone); + result = dns_view_findzone(view, name, &zone); + if (result == ISC_R_SUCCESS) { + result = ISC_R_EXISTS; + } } - if (result == ISC_R_SUCCESS) { - result = ISC_R_EXISTS; - goto cleanup; - } else if (result == DNS_R_PARTIALMATCH) { - /* Create our sub-zone anyway */ - dns_zone_detach(&zone); - zone = NULL; - } else if (result != ISC_R_NOTFOUND) { + if (result != ISC_R_NOTFOUND) { goto cleanup; } @@ -13501,7 +13485,7 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, } dns_zone_attach(view->redirect, &zone); } else { - result = dns_zt_find(view->zonetable, name, 0, NULL, &zone); + result = dns_view_findzone(view, name, &zone); if (result != ISC_R_SUCCESS) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, @@ -13617,7 +13601,7 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, result = ISC_R_NOTFOUND; } } else { - result = dns_zt_find(view->zonetable, name, 0, NULL, &zone); + result = dns_view_findzone(view, name, &zone); } if (result != ISC_R_SUCCESS) { goto cleanup; @@ -13686,7 +13670,7 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, } dns_zone_attach(view->redirect, &zone); } else { - CHECK(dns_zt_find(view->zonetable, name, 0, NULL, &zone)); + CHECK(dns_view_findzone(view, name, &zone)); } #ifndef HAVE_LMDB diff --git a/bin/named/statschannel.c b/bin/named/statschannel.c index 7b595ec2678..0b9eb6b862a 100644 --- a/bin/named/statschannel.c +++ b/bin/named/statschannel.c @@ -1776,8 +1776,8 @@ generatexml(named_server_t *server, uint32_t flags, int *buflen, if ((flags & STATS_XML_ZONES) != 0) { TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "zones")); - CHECK(dns_zt_apply(view->zonetable, isc_rwlocktype_read, - true, NULL, zone_xmlrender, writer)); + CHECK(dns_zt_apply(view->zonetable, true, NULL, + zone_xmlrender, writer)); TRY0(xmlTextWriterEndElement(writer)); /* /zones */ } @@ -2490,9 +2490,8 @@ generatejson(named_server_t *server, size_t *msglen, const char **msg, CHECKMEM(za); if ((flags & STATS_JSON_ZONES) != 0) { - CHECK(dns_zt_apply(view->zonetable, - isc_rwlocktype_read, true, - NULL, zone_jsonrender, za)); + CHECK(dns_zt_apply(view->zonetable, true, NULL, + zone_jsonrender, za)); } if (json_object_array_length(za) != 0) { diff --git a/bin/tests/system/catz/tests.sh b/bin/tests/system/catz/tests.sh index ddab8f84f71..756edb9b180 100644 --- a/bin/tests/system/catz/tests.sh +++ b/bin/tests/system/catz/tests.sh @@ -294,7 +294,7 @@ status=$((status+ret)) n=$((n+1)) echo_i "waiting for secondary to sync up ($n)" ret=0 -wait_for_message ns2/named.run "zone_shutdown: zone dom1.example/IN/default: shutting down" || ret=1 +wait_for_message ns2/named.run "catz: catz_delzone_cb: zone 'dom1.example' deleted" || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -929,7 +929,7 @@ status=$((status+ret)) n=$((n+1)) echo_i "waiting for secondary to sync up ($n)" ret=0 -wait_for_message ns2/named.run "zone_shutdown: zone dom5.example/IN/default: shutting down" || ret=1 +wait_for_message ns2/named.run "catz: catz_delzone_cb: zone 'dom5.example' deleted" || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -987,7 +987,7 @@ status=$((status+ret)) n=$((n+1)) echo_i "waiting for secondary to sync up ($n)" ret=0 -wait_for_message ns2/named.run "zone_shutdown: zone dom6.example/IN/default: shutting down" || ret=1 +wait_for_message ns2/named.run "catz: catz_delzone_cb: zone 'dom6.example' deleted" || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -1483,7 +1483,7 @@ END n=$((n+1)) echo_i "waiting for secondary to sync up ($n)" ret=0 - wait_for_message ns2/named.run "zone_shutdown: zone ${special}/IN/default: shutting down" || ret=1 + wait_for_message ns2/named.run "catz: catz_delzone_cb: zone '${special}' deleted" || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -1621,7 +1621,7 @@ status=$((status+ret)) n=$((n+1)) echo_i "waiting for secondary to sync up ($n)" ret=0 -wait_for_message ns2/named.run "zone_shutdown: zone dom11.example/IN/default: shutting down" || ret=1 +wait_for_message ns2/named.run "catz: catz_delzone_cb: zone 'dom11.example' deleted" || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -1653,7 +1653,7 @@ status=$((status+ret)) n=$((n+1)) echo_i "waiting for secondary to sync up ($n)" ret=0 -wait_for_message ns2/named.run "zone_shutdown: zone subdomain.of.dom11.example/IN/default: shutting down" || ret=1 +wait_for_message ns2/named.run "catz: catz_delzone_cb: zone 'subdomain.of.dom11.example' deleted" || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -2424,10 +2424,8 @@ status=$((status+ret)) n=$((n+1)) echo_i "waiting for secondary to sync up ($n)" ret=0 -wait_for_message ns2/named.run "catz: deleting zone 'dom17.example' from catalog 'catalog1.example' - success" && -wait_for_message ns2/named.run "catz: deleting zone 'dom18.example' from catalog 'catalog1.example' - success" && -wait_for_message ns2/named.run "zone_shutdown: zone dom17.example/IN/default: shutting down" && -wait_for_message ns2/named.run "zone_shutdown: zone dom18.example/IN/default: shutting down" || ret=1 +wait_for_message ns2/named.run "catz: catz_delzone_cb: zone 'dom17.example' deleted" && +wait_for_message ns2/named.run "catz: catz_delzone_cb: zone 'dom18.example' deleted" && if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -2515,10 +2513,8 @@ status=$((status+ret)) n=$((n+1)) echo_i "waiting for secondary to sync up ($n)" ret=0 -wait_for_message ns2/named.run "catz: deleting zone 'dom17.example' from catalog 'catalog2.example' - success" && -wait_for_message ns2/named.run "catz: deleting zone 'dom18.example' from catalog 'catalog2.example' - success" && -wait_for_message ns2/named.run "zone_shutdown: zone dom17.example/IN/default: shutting down" && -wait_for_message ns2/named.run "zone_shutdown: zone dom18.example/IN/default: shutting down" || ret=1 +wait_for_message ns2/named.run "catz: catz_delzone_cb: zone 'dom17.example' deleted" && +wait_for_message ns2/named.run "catz: catz_delzone_cb: zone 'dom18.example' deleted" || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) diff --git a/bin/tests/system/dyndb/driver/syncptr.c b/bin/tests/system/dyndb/driver/syncptr.c index 15defbd2cc0..270c7acca6c 100644 --- a/bin/tests/system/dyndb/driver/syncptr.c +++ b/bin/tests/system/dyndb/driver/syncptr.c @@ -166,7 +166,7 @@ syncptr_find_zone(sample_instance_t *inst, dns_rdata_t *rdata, dns_name_t *name, } /* Find a zone containing owner name of the PTR record. */ - result = dns_zt_find(inst->view->zonetable, name, 0, NULL, zone); + result = dns_zt_find(inst->view->zonetable, name, 0, zone); if (result == DNS_R_PARTIALMATCH) { result = ISC_R_SUCCESS; } else if (result != ISC_R_SUCCESS) { diff --git a/bin/tests/system/pipelined/pipequeries.c b/bin/tests/system/pipelined/pipequeries.c index 805e3c53443..5569ad318d3 100644 --- a/bin/tests/system/pipelined/pipequeries.c +++ b/bin/tests/system/pipelined/pipequeries.c @@ -256,7 +256,7 @@ main(int argc, char *argv[]) { RUNCHECK(dns_requestmgr_create(mctx, dispatchmgr, dispatchv4, NULL, &requestmgr)); - RUNCHECK(dns_view_create(mctx, 0, "_test", &view)); + RUNCHECK(dns_view_create(mctx, loopmgr, 0, "_test", &view)); isc_loopmgr_setup(loopmgr, sendqueries, NULL); isc_loopmgr_run(loopmgr); diff --git a/bin/tools/mdig.c b/bin/tools/mdig.c index 98e72ffb2ea..4193f06912d 100644 --- a/bin/tools/mdig.c +++ b/bin/tools/mdig.c @@ -2141,7 +2141,7 @@ main(int argc, char *argv[]) { mctx, dispatchmgr, have_ipv4 ? dispatchvx : NULL, have_ipv6 ? dispatchvx : NULL, &requestmgr)); - RUNCHECK(dns_view_create(mctx, 0, "_test", &view)); + RUNCHECK(dns_view_create(mctx, loopmgr, 0, "_mdig", &view)); query = ISC_LIST_HEAD(queries); isc_loopmgr_setup(loopmgr, sendqueries, query); diff --git a/fuzz/dns_message_checksig.c b/fuzz/dns_message_checksig.c index 6679df60bd3..c27bfebe8bc 100644 --- a/fuzz/dns_message_checksig.c +++ b/fuzz/dns_message_checksig.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -85,6 +86,7 @@ static isc_mem_t *mctx = NULL; #define HMACSHA256 "\x0bhmac-sha256" static isc_stdtime_t fuzztime = 0x622acce1; +static isc_loopmgr_t *loopmgr = NULL; static dns_view_t *view = NULL; static dns_tsigkey_t *tsigkey = NULL; static dns_tsig_keyring_t *ring = NULL; @@ -232,7 +234,10 @@ LLVMFuzzerInitialize(int *argc ISC_ATTR_UNUSED, char ***argv ISC_ATTR_UNUSED) { } destroy_dst = true; - result = dns_view_create(mctx, dns_rdataclass_in, "view", &view); + isc_loopmgr_create(mctx, 1, &loopmgr); + + result = dns_view_create(mctx, loopmgr, dns_rdataclass_in, "view", + &view); if (result != ISC_R_SUCCESS) { fprintf(stderr, "dns_view_create failed: %s\n", isc_result_totext(result)); @@ -322,6 +327,7 @@ LLVMFuzzerInitialize(int *argc ISC_ATTR_UNUSED, char ***argv ISC_ATTR_UNUSED) { return (1); } + dns_zone_setview(zone, view); dns_view_freeze(view); dns_zone_detach(&zone); diff --git a/lib/dns/catz.c b/lib/dns/catz.c index 70b160469fd..f4b9c962fa8 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -526,7 +526,7 @@ dns__catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) { result = delcur ? isc_ht_iter_delcurrent_next(iter1) : isc_ht_iter_next(iter1)) { - isc_result_t zt_find_result; + isc_result_t find_result; dns_catz_zone_t *parentcatz = NULL; dns_catz_entry_t *nentry = NULL; dns_catz_entry_t *oentry = NULL; @@ -559,10 +559,10 @@ dns__catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) { &catz->zoneoptions, &nentry->opts); /* Try to find the zone in the view */ - zt_find_result = dns_zt_find(catz->catzs->view->zonetable, - dns_catz_entry_getname(nentry), 0, - NULL, &zone); - if (zt_find_result == ISC_R_SUCCESS) { + find_result = dns_view_findzone(catz->catzs->view, + dns_catz_entry_getname(nentry), + &zone); + if (find_result == ISC_R_SUCCESS) { dns_catz_coo_t *coo = NULL; char pczname[DNS_NAME_FORMATSIZE]; bool parentcatz_locked = false; @@ -606,10 +606,6 @@ dns__catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) { UNLOCK(&parentcatz->lock); LOCK(&catz->lock); } - } - if (zt_find_result == ISC_R_SUCCESS || - zt_find_result == DNS_R_PARTIALMATCH) - { dns_zone_detach(&zone); } @@ -617,8 +613,7 @@ dns__catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) { result = isc_ht_find(catz->entries, key, (uint32_t)keysize, (void **)&oentry); if (result != ISC_R_SUCCESS) { - if (zt_find_result == ISC_R_SUCCESS && - parentcatz == catz) + if (find_result == ISC_R_SUCCESS && parentcatz == catz) { /* * This means that the zone's unique label @@ -645,7 +640,7 @@ dns__catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) { continue; } - if (zt_find_result != ISC_R_SUCCESS) { + if (find_result != ISC_R_SUCCESS) { isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_DEBUG(3), "catz: zone '%s' was expected to exist " diff --git a/lib/dns/client.c b/lib/dns/client.c index 579b9c3aab1..5a3c0fadda8 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -206,7 +206,8 @@ createview(isc_mem_t *mctx, dns_rdataclass_t rdclass, isc_loopmgr_t *loopmgr, isc_result_t result; dns_view_t *view = NULL; - result = dns_view_create(mctx, rdclass, DNS_CLIENTVIEW_NAME, &view); + result = dns_view_create(mctx, loopmgr, rdclass, DNS_CLIENTVIEW_NAME, + &view); if (result != ISC_R_SUCCESS) { return (result); } diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index a552327bad1..b72a3dd44f1 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -259,8 +259,8 @@ struct dns_view { #endif /* HAVE_LMDB */ isc_result_t -dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, - dns_view_t **viewp); +dns_view_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, + dns_rdataclass_t rdclass, const char *name, dns_view_t **viewp); /*%< * Create a view. * @@ -364,24 +364,6 @@ dns_view_weakdetach(dns_view_t **targetp); *\li *viewp is NULL. */ -isc_result_t -dns_view_createzonetable(dns_view_t *view); -/*%< - * Create a zonetable for the view. - * - * Requires: - * - *\li 'view' is a valid, unfrozen view. - * - *\li 'view' does not have a zonetable already. - * - * Returns: - * - *\li #ISC_R_SUCCESS - * - *\li Any error that dns_zt_create() can return. - */ - isc_result_t dns_view_createresolver(dns_view_t *view, isc_loopmgr_t *loopmgr, unsigned int ndisp, isc_nm_t *netmgr, @@ -782,14 +764,13 @@ dns_view_findzone(dns_view_t *view, const dns_name_t *name, dns_zone_t **zonep); * Returns: *\li #ISC_R_SUCCESS A matching zone was found. *\li #ISC_R_NOTFOUND No matching zone was found. - *\li others An error occurred. */ isc_result_t dns_view_load(dns_view_t *view, bool stop, bool newonly); isc_result_t -dns_view_asyncload(dns_view_t *view, bool newonly, dns_zt_allloaded_t callback, +dns_view_asyncload(dns_view_t *view, bool newonly, dns_zt_callback_t *callback, void *arg); /*%< * Load zones attached to this view. dns_view_load() loads diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index 0c71cfceaa4..db67203797b 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -424,15 +424,14 @@ dns_zone_loadandthaw(dns_zone_t *zone); */ isc_result_t -dns_zone_asyncload(dns_zone_t *zone, bool newonly, dns_zt_zoneloaded_t done, +dns_zone_asyncload(dns_zone_t *zone, bool newonly, dns_zt_callback_t done, void *arg); /*%< * Cause the database to be loaded from its backing store asynchronously. * Other zone maintenance functions are suspended until this is complete. * When finished, 'done' is called to inform the caller, with 'arg' as - * its first argument and 'zone' as its second. (Normally, 'arg' is - * expected to point to the zone table but is left undefined for testing - * purposes.) + * its argument. (Normally, 'arg' is expected to point to the zone table + * but is left undefined for testing purposes.) * * Require: *\li 'zone' to be a valid zone. diff --git a/lib/dns/include/dns/zt.h b/lib/dns/include/dns/zt.h index afe13ead858..1118d064280 100644 --- a/lib/dns/include/dns/zt.h +++ b/lib/dns/include/dns/zt.h @@ -22,35 +22,37 @@ #include -#define DNS_ZTFIND_NOEXACT 0x01 -#define DNS_ZTFIND_MIRROR 0x02 - ISC_LANG_BEGINDECLS -typedef isc_result_t (*dns_zt_allloaded_t)(void *arg); -/*%< - * Method prototype: when all pending zone loads are complete, - * the zone table can inform the caller via a callback function with - * this signature. - */ +typedef enum dns_ztfind { + DNS_ZTFIND_EXACT = 1 << 0, + DNS_ZTFIND_NOEXACT = 1 << 1, + DNS_ZTFIND_MIRROR = 1 << 2, +} dns_ztfind_t; -typedef isc_result_t (*dns_zt_zoneloaded_t)(dns_zt_t *zt, dns_zone_t *zone); -/*%< - * Method prototype: when a zone finishes loading, the zt object - * can be informed via a callback function with this signature. - */ +typedef isc_result_t +dns_zt_callback_t(void *arg); -isc_result_t -dns_zt_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, dns_zt_t **zt); +void +dns_zt_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, dns_view_t *view, + dns_zt_t **ztp); /*%< - * Creates a new zone table. + * Creates a new zone table for a view. * * Requires: * \li 'mctx' to be initialized. + * \li 'view' is non-NULL + * \li 'ztp' is non-NULL + * \li '*ztp' is NULL + */ + +void +dns_zt_compact(dns_zt_t *zt); +/*%< + * Reclaim unused memory in the zone table * - * Returns: - * \li #ISC_R_SUCCESS on success. - * \li #ISC_R_NOMEMORY + * Requires: + * \li 'zt' to be valid */ isc_result_t @@ -65,8 +67,6 @@ dns_zt_mount(dns_zt_t *zt, dns_zone_t *zone); * Returns: * \li #ISC_R_SUCCESS * \li #ISC_R_EXISTS - * \li #ISC_R_NOSPACE - * \li #ISC_R_NOMEMORY */ isc_result_t @@ -75,37 +75,38 @@ dns_zt_unmount(dns_zt_t *zt, dns_zone_t *zone); * Unmount the given zone from the table. * * Requires: - * 'zt' to be valid + * 'zt' to be valid * \li 'zone' to be valid * * Returns: * \li #ISC_R_SUCCESS * \li #ISC_R_NOTFOUND - * \li #ISC_R_NOMEMORY */ isc_result_t -dns_zt_find(dns_zt_t *zt, const dns_name_t *name, unsigned int options, - dns_name_t *foundname, dns_zone_t **zone); +dns_zt_find(dns_zt_t *zt, const dns_name_t *name, dns_ztfind_t options, + dns_zone_t **zone); /*%< - * Find the best match for 'name' in 'zt'. If foundname is non NULL - * then the name of the zone found is returned. + * Find the best match for 'name' in 'zt'. * * Notes: - * \li If the DNS_ZTFIND_NOEXACT is set, the best partial match (if any) - * to 'name' will be returned. + * \li If the DNS_ZTFIND_EXACT option is set, only an exact match is + * returned. + * + * \li If the DNS_ZTFIND_NOEXACT option is set, the closest matching + * parent domain is returned, even when there is an exact match + * in the tree. * * Requires: * \li 'zt' to be valid * \li 'name' to be valid - * \li 'foundname' to be initialized and associated with a fixedname or NULL * \li 'zone' to be non NULL and '*zone' to be NULL + * \li DNS_ZTFIND_EXACT and DNS_ZTFIND_NOEXACT are not both set * * Returns: * \li #ISC_R_SUCCESS - * \li #DNS_R_PARTIALMATCH + * \li #DNS_R_PARTIALMATCH (if DNS_ZTFIND_EXACT is not set) * \li #ISC_R_NOTFOUND - * \li #ISC_R_NOSPACE */ void @@ -142,14 +143,14 @@ isc_result_t dns_zt_load(dns_zt_t *zt, bool stop, bool newonly); isc_result_t -dns_zt_asyncload(dns_zt_t *zt, bool newonly, dns_zt_allloaded_t alldone, +dns_zt_asyncload(dns_zt_t *zt, bool newonly, dns_zt_callback_t alldone, void *arg); /*%< - * Load all zones in the table. If 'stop' is true, - * stop on the first error and return it. If 'stop' - * is false, ignore errors. + * Load all zones in the table. If 'stop' is true, stop on the first + * error and return it. If 'stop' is false, ignore errors. + * + * If newonly is set only zones that were never loaded are loaded. * - * if newonly is set only zones that were never loaded are loaded. * dns_zt_asyncload() loads zones asynchronously; when all * zones in the zone table have finished loaded (or failed due * to errors), the caller is informed by calling 'alldone' @@ -168,7 +169,7 @@ dns_zt_freezezones(dns_zt_t *zt, dns_view_t *view, bool freeze); */ isc_result_t -dns_zt_apply(dns_zt_t *zt, isc_rwlocktype_t lock, bool stop, isc_result_t *sub, +dns_zt_apply(dns_zt_t *zt, bool stop, isc_result_t *sub, isc_result_t (*action)(dns_zone_t *, void *), void *uap); /*%< * Apply a given 'action' to all zone zones in the table. diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index dab6b404ede..194caf65394 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -6544,9 +6544,8 @@ static inline bool name_external(const dns_name_t *name, dns_rdatatype_t type, fetchctx_t *fctx) { isc_result_t result; dns_forwarders_t *forwarders = NULL; - dns_fixedname_t fixed, zfixed; + dns_fixedname_t fixed; dns_name_t *fname = dns_fixedname_initname(&fixed); - dns_name_t *zfname = dns_fixedname_initname(&zfixed); dns_name_t *apex = NULL; dns_name_t suffix; dns_zone_t *zone = NULL; @@ -6584,25 +6583,17 @@ name_external(const dns_name_t *name, dns_rdatatype_t type, fetchctx_t *fctx) { * If there is a locally served zone between 'apex' and 'name' * then don't cache. */ - LOCK(&fctx->res->view->lock); - if (fctx->res->view->zonetable != NULL) { - unsigned int options = DNS_ZTFIND_NOEXACT | DNS_ZTFIND_MIRROR; - result = dns_zt_find(fctx->res->view->zonetable, name, options, - zfname, &zone); - if (zone != NULL) { - dns_zone_detach(&zone); - } - if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { - if (dns_name_fullcompare(zfname, apex, &(int){ 0 }, - &(unsigned int){ 0U }) == - dns_namereln_subdomain) - { - UNLOCK(&fctx->res->view->lock); - return (true); - } + dns_ztfind_t options = DNS_ZTFIND_NOEXACT | DNS_ZTFIND_MIRROR; + result = dns_zt_find(fctx->res->view->zonetable, name, options, &zone); + if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { + dns_name_t *zname = dns_zone_getorigin(zone); + dns_namereln_t reln = dns_name_fullcompare( + zname, apex, &(int){ 0 }, &(unsigned int){ 0U }); + dns_zone_detach(&zone); + if (reln == dns_namereln_subdomain) { + return (true); } } - UNLOCK(&fctx->res->view->lock); /* * Look for a forward declaration below 'name'. diff --git a/lib/dns/view.c b/lib/dns/view.c index 133d6775bb2..5f581426277 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -79,7 +79,8 @@ #define DEFAULT_EDNS_BUFSIZE 1232 isc_result_t -dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, +dns_view_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, + dns_rdataclass_t rdclass, const char *name, dns_view_t **viewp) { dns_view_t *view = NULL; isc_result_t result; @@ -134,13 +135,7 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, isc_rwlock_init(&view->sfd_lock); view->zonetable = NULL; - result = dns_zt_create(mctx, rdclass, &view->zonetable); - if (result != ISC_R_SUCCESS) { - UNEXPECTED_ERROR("dns_zt_create() failed: %s", - isc_result_totext(result)); - result = ISC_R_UNEXPECTED; - goto cleanup_mutex; - } + dns_zt_create(mctx, loopmgr, view, &view->zonetable); result = dns_fwdtable_create(mctx, &view->fwdtable); if (result != ISC_R_SUCCESS) { @@ -214,11 +209,8 @@ cleanup_weakrefs: } cleanup_zt: - if (view->zonetable != NULL) { - dns_zt_detach(&view->zonetable); - } + dns_zt_detach(&view->zonetable); -cleanup_mutex: isc_rwlock_destroy(&view->sfd_lock); isc_mutex_destroy(&view->lock); @@ -572,8 +564,7 @@ dns_view_dialup(dns_view_t *view) { REQUIRE(DNS_VIEW_VALID(view)); REQUIRE(view->zonetable != NULL); - (void)dns_zt_apply(view->zonetable, isc_rwlocktype_read, false, NULL, - dialup, NULL); + (void)dns_zt_apply(view->zonetable, false, NULL, dialup, NULL); } void @@ -602,15 +593,6 @@ dns_view_weakdetach(dns_view_t **viewp) { } } -isc_result_t -dns_view_createzonetable(dns_view_t *view) { - REQUIRE(DNS_VIEW_VALID(view)); - REQUIRE(!view->frozen); - REQUIRE(view->zonetable == NULL); - - return (dns_zt_create(view->mctx, view->rdclass, &view->zonetable)); -} - isc_result_t dns_view_createresolver(dns_view_t *view, isc_loopmgr_t *loopmgr, unsigned int ndisp, isc_nm_t *netmgr, @@ -784,7 +766,6 @@ dns_view_addzone(dns_view_t *view, dns_zone_t *zone) { REQUIRE(DNS_VIEW_VALID(view)); REQUIRE(!view->frozen); - REQUIRE(view->zonetable != NULL); result = dns_zt_mount(view->zonetable, zone); @@ -794,23 +775,9 @@ dns_view_addzone(dns_view_t *view, dns_zone_t *zone) { isc_result_t dns_view_findzone(dns_view_t *view, const dns_name_t *name, dns_zone_t **zonep) { - isc_result_t result; - REQUIRE(DNS_VIEW_VALID(view)); - LOCK(&view->lock); - if (view->zonetable != NULL) { - result = dns_zt_find(view->zonetable, name, 0, NULL, zonep); - if (result == DNS_R_PARTIALMATCH) { - dns_zone_detach(zonep); - result = ISC_R_NOTFOUND; - } - } else { - result = ISC_R_NOTFOUND; - } - UNLOCK(&view->lock); - - return (result); + return (dns_zt_find(view->zonetable, name, DNS_ZTFIND_EXACT, zonep)); } isc_result_t @@ -820,11 +787,11 @@ dns_view_find(dns_view_t *view, const dns_name_t *name, dns_rdatatype_t type, dns_name_t *foundname, dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset) { isc_result_t result; - dns_db_t *db, *zdb; - dns_dbnode_t *node, *znode; + dns_db_t *db = NULL, *zdb = NULL; + dns_dbnode_t *node = NULL, *znode = NULL; bool is_cache, is_staticstub_zone; dns_rdataset_t zrdataset, zsigrdataset; - dns_zone_t *zone; + dns_zone_t *zone = NULL; /* * Find an rdataset whose owner name is 'name', and whose type is @@ -842,24 +809,12 @@ dns_view_find(dns_view_t *view, const dns_name_t *name, dns_rdatatype_t type, */ dns_rdataset_init(&zrdataset); dns_rdataset_init(&zsigrdataset); - zdb = NULL; - znode = NULL; /* * Find a database to answer the query. */ - db = NULL; - node = NULL; is_staticstub_zone = false; - zone = NULL; - LOCK(&view->lock); - if (view->zonetable != NULL) { - result = dns_zt_find(view->zonetable, name, DNS_ZTFIND_MIRROR, - NULL, &zone); - } else { - result = ISC_R_NOTFOUND; - } - UNLOCK(&view->lock); + result = dns_zt_find(view->zonetable, name, DNS_ZTFIND_MIRROR, &zone); if (zone != NULL && dns_zone_gettype(zone) == dns_zone_staticstub && !use_static_stub) { @@ -1109,10 +1064,10 @@ dns_view_findzonecut(dns_view_t *view, const dns_name_t *name, unsigned int options, bool use_hints, bool use_cache, dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset) { isc_result_t result; - dns_db_t *db; - bool is_cache, use_zone, try_hints; - dns_zone_t *zone; - dns_name_t *zfname; + dns_db_t *db = NULL; + bool is_cache, use_zone = false, try_hints = false; + dns_zone_t *zone = NULL; + dns_name_t *zfname = NULL; dns_rdataset_t zrdataset, zsigrdataset; dns_fixedname_t zfixedname; unsigned int ztoptions = DNS_ZTFIND_MIRROR; @@ -1120,11 +1075,6 @@ dns_view_findzonecut(dns_view_t *view, const dns_name_t *name, REQUIRE(DNS_VIEW_VALID(view)); REQUIRE(view->frozen); - db = NULL; - use_zone = false; - try_hints = false; - zfname = NULL; - /* * Initialize. */ @@ -1135,18 +1085,10 @@ dns_view_findzonecut(dns_view_t *view, const dns_name_t *name, /* * Find the right database. */ - zone = NULL; - LOCK(&view->lock); - if (view->zonetable != NULL) { - if ((options & DNS_DBFIND_NOEXACT) != 0) { - ztoptions |= DNS_ZTFIND_NOEXACT; - } - result = dns_zt_find(view->zonetable, name, ztoptions, NULL, - &zone); - } else { - result = ISC_R_NOTFOUND; + if ((options & DNS_DBFIND_NOEXACT) != 0) { + ztoptions |= DNS_ZTFIND_NOEXACT; } - UNLOCK(&view->lock); + result = dns_zt_find(view->zonetable, name, ztoptions, &zone); if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { result = dns_zone_getdb(zone, &db); } @@ -1339,7 +1281,6 @@ dns_viewlist_findzone(dns_viewlist_t *list, const dns_name_t *name, dns_view_t *view; isc_result_t result; dns_zone_t *zone1 = NULL, *zone2 = NULL; - dns_zone_t **zp = NULL; REQUIRE(list != NULL); REQUIRE(zonep != NULL && *zonep == NULL); @@ -1350,30 +1291,9 @@ dns_viewlist_findzone(dns_viewlist_t *list, const dns_name_t *name, if (!allclasses && view->rdclass != rdclass) { continue; } - - /* - * If the zone is defined in more than one view, - * treat it as not found. - */ - zp = (zone1 == NULL) ? &zone1 : &zone2; - LOCK(&view->lock); - if (view->zonetable != NULL) { - result = dns_zt_find(view->zonetable, name, 0, NULL, - zp); - } else { - result = ISC_R_NOTFOUND; - } - UNLOCK(&view->lock); - INSIST(result == ISC_R_SUCCESS || result == ISC_R_NOTFOUND || - result == DNS_R_PARTIALMATCH); - - /* Treat a partial match as no match */ - if (result == DNS_R_PARTIALMATCH) { - dns_zone_detach(zp); - result = ISC_R_NOTFOUND; - POST(result); - } - + result = dns_zt_find(view->zonetable, name, DNS_ZTFIND_EXACT, + (zone1 == NULL) ? &zone1 : &zone2); + INSIST(result == ISC_R_SUCCESS || result == ISC_R_NOTFOUND); if (zone2 != NULL) { dns_zone_detach(&zone1); dns_zone_detach(&zone2); @@ -1393,17 +1313,13 @@ dns_viewlist_findzone(dns_viewlist_t *list, const dns_name_t *name, isc_result_t dns_view_load(dns_view_t *view, bool stop, bool newonly) { REQUIRE(DNS_VIEW_VALID(view)); - REQUIRE(view->zonetable != NULL); - return (dns_zt_load(view->zonetable, stop, newonly)); } isc_result_t -dns_view_asyncload(dns_view_t *view, bool newonly, dns_zt_allloaded_t callback, +dns_view_asyncload(dns_view_t *view, bool newonly, dns_zt_callback_t *callback, void *arg) { REQUIRE(DNS_VIEW_VALID(view)); - REQUIRE(view->zonetable != NULL); - return (dns_zt_asyncload(view->zonetable, newonly, callback, arg)); } diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 2e31a829638..5dd59da6467 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -781,7 +781,7 @@ struct dns_nsfetch { struct dns_asyncload { dns_zone_t *zone; unsigned int flags; - dns_zt_zoneloaded_t loaded; + dns_zt_callback_t *loaded; void *loaded_arg; }; @@ -2371,7 +2371,7 @@ zone_asyncload(void *arg) { /* Inform the zone table we've finished loading */ if (asl->loaded != NULL) { - (asl->loaded)(asl->loaded_arg, zone); + asl->loaded(asl->loaded_arg); } isc_mem_put(zone->mctx, asl, sizeof(*asl)); @@ -2379,7 +2379,7 @@ zone_asyncload(void *arg) { } isc_result_t -dns_zone_asyncload(dns_zone_t *zone, bool newonly, dns_zt_zoneloaded_t done, +dns_zone_asyncload(dns_zone_t *zone, bool newonly, dns_zt_callback_t *done, void *arg) { dns_asyncload_t *asl = NULL; @@ -5617,15 +5617,7 @@ zone_destroy(dns_zone_t *zone) { * This zone is unmanaged; we're probably running in * named-checkzone or a unit test. There's no loop, so we * need to free it immediately. - * - * Unmanaged zones must not have null views; we have no way - * of detaching from the view here without causing deadlock - * because this code is called with the view already - * locked. */ - INSIST(isc_tid() == ISC_TID_UNKNOWN); - INSIST(zone->view == NULL); - zone_shutdown(zone); } else { /* diff --git a/lib/dns/zt.c b/lib/dns/zt.c index 9cfe366ba18..cf91f7285b0 100644 --- a/lib/dns/zt.c +++ b/lib/dns/zt.c @@ -22,38 +22,35 @@ #include #include #include +#include #include #include #include -#include +#include #include #include #include #include -struct zt_load_params { - dns_zt_zoneloaded_t dl; - bool newonly; -}; +#define ZTMAGIC ISC_MAGIC('Z', 'T', 'b', 'l') +#define VALID_ZT(zt) ISC_MAGIC_VALID(zt, ZTMAGIC) struct dns_zt { - /* Unlocked. */ unsigned int magic; isc_mem_t *mctx; - dns_rdataclass_t rdclass; - isc_rwlock_t rwlock; - dns_zt_allloaded_t loaddone; - void *loaddone_arg; - struct zt_load_params *loadparams; + dns_qpmulti_t *multi; - /* Atomic */ atomic_bool flush; isc_refcount_t references; isc_refcount_t loads_pending; +}; - /* Locked by lock. */ - dns_rbt_t *table; +struct zt_load_params { + dns_zt_t *zt; + dns_zt_callback_t *loaddone; + void *loaddone_arg; + bool newonly; }; struct zt_freeze_params { @@ -61,77 +58,93 @@ struct zt_freeze_params { bool freeze; }; -#define ZTMAGIC ISC_MAGIC('Z', 'T', 'b', 'l') -#define VALID_ZT(zt) ISC_MAGIC_VALID(zt, ZTMAGIC) - static void -auto_detach(void *, void *); +ztqpattach(void *uctx ISC_ATTR_UNUSED, void *pval, + uint32_t ival ISC_ATTR_UNUSED) { + dns_zone_t *zone = pval; + dns_zone_ref(zone); +} -static isc_result_t -load(dns_zone_t *zone, void *uap); +static void +ztqpdetach(void *uctx ISC_ATTR_UNUSED, void *pval, + uint32_t ival ISC_ATTR_UNUSED) { + dns_zone_t *zone = pval; + dns_zone_detach(&zone); +} -static isc_result_t -asyncload(dns_zone_t *zone, void *callback); +static size_t +ztqpmakekey(dns_qpkey_t key, void *uctx ISC_ATTR_UNUSED, void *pval, + uint32_t ival ISC_ATTR_UNUSED) { + dns_zone_t *zone = pval; + dns_name_t *name = dns_zone_getorigin(zone); + return (dns_qpkey_fromname(key, name)); +} -static isc_result_t -freezezones(dns_zone_t *zone, void *uap); +static void +ztqptriename(void *uctx, char *buf, size_t size) { + dns_view_t *view = uctx; + snprintf(buf, size, "view %s zone table", view->name); +} -static isc_result_t -doneloading(dns_zt_t *zt, dns_zone_t *zone); +static dns_qpmethods_t ztqpmethods = { + ztqpattach, + ztqpdetach, + ztqpmakekey, + ztqptriename, +}; -isc_result_t -dns_zt_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, dns_zt_t **ztp) { - dns_zt_t *zt; - isc_result_t result; +void +dns_zt_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, dns_view_t *view, + dns_zt_t **ztp) { + dns_qpmulti_t *multi = NULL; + dns_zt_t *zt = NULL; REQUIRE(ztp != NULL && *ztp == NULL); + REQUIRE(view != NULL); - zt = isc_mem_get(mctx, sizeof(*zt)); + dns_qpmulti_create(mctx, loopmgr, &ztqpmethods, view, &multi); - zt->table = NULL; - result = dns_rbt_create(mctx, auto_detach, zt, &zt->table); - if (result != ISC_R_SUCCESS) { - goto cleanup_zt; - } + zt = isc_mem_get(mctx, sizeof(*zt)); + *zt = (dns_zt_t){ + .magic = ZTMAGIC, + .multi = multi, + .references = 1, + }; - isc_rwlock_init(&zt->rwlock); - zt->mctx = NULL; isc_mem_attach(mctx, &zt->mctx); - isc_refcount_init(&zt->references, 1); - atomic_init(&zt->flush, false); - zt->rdclass = rdclass; - zt->magic = ZTMAGIC; - zt->loaddone = NULL; - zt->loaddone_arg = NULL; - zt->loadparams = NULL; - isc_refcount_init(&zt->loads_pending, 0); + *ztp = zt; +} - return (ISC_R_SUCCESS); +/* + * XXXFANF it isn't clear whether this function will be useful. There + * is only one zone table per view, so it is probably enough to let + * the qp-trie auto-GC do its thing. However it might be problematic + * if a very large zone is replaced, and its database memory is + * retained for a long time. + */ +void +dns_zt_compact(dns_zt_t *zt) { + dns_qp_t *qp = NULL; -cleanup_zt: - isc_mem_put(mctx, zt, sizeof(*zt)); + REQUIRE(VALID_ZT(zt)); - return (result); + dns_qpmulti_write(zt->multi, &qp); + dns_qp_compact(qp, DNS_QPGC_ALL); + dns_qpmulti_commit(zt->multi, &qp); } isc_result_t dns_zt_mount(dns_zt_t *zt, dns_zone_t *zone) { isc_result_t result; - dns_name_t *name = NULL; + dns_qp_t *qp = NULL; REQUIRE(VALID_ZT(zt)); - name = dns_zone_getorigin(zone); - - RWLOCK(&zt->rwlock, isc_rwlocktype_write); - - result = dns_rbt_addname(zt->table, name, zone); - if (result == ISC_R_SUCCESS) { - dns_zone_ref(zone); - } - - RWUNLOCK(&zt->rwlock, isc_rwlocktype_write); + dns_qpmulti_write(zt->multi, &qp); + result = dns_qp_insert(qp, zone, 0); + dns_qp_compact(qp, DNS_QPGC_MAYBE); + dns_qpmulti_commit(zt->multi, &qp); return (result); } @@ -139,39 +152,48 @@ dns_zt_mount(dns_zt_t *zt, dns_zone_t *zone) { isc_result_t dns_zt_unmount(dns_zt_t *zt, dns_zone_t *zone) { isc_result_t result; - dns_name_t *name; + dns_qp_t *qp = NULL; REQUIRE(VALID_ZT(zt)); - name = dns_zone_getorigin(zone); - - RWLOCK(&zt->rwlock, isc_rwlocktype_write); - - result = dns_rbt_deletename(zt->table, name, false); - - RWUNLOCK(&zt->rwlock, isc_rwlocktype_write); + dns_qpmulti_write(zt->multi, &qp); + result = dns_qp_deletename(qp, dns_zone_getorigin(zone)); + dns_qp_compact(qp, DNS_QPGC_MAYBE); + dns_qpmulti_commit(zt->multi, &qp); return (result); } isc_result_t -dns_zt_find(dns_zt_t *zt, const dns_name_t *name, unsigned int options, - dns_name_t *foundname, dns_zone_t **zonep) { +dns_zt_find(dns_zt_t *zt, const dns_name_t *name, dns_ztfind_t options, + dns_zone_t **zonep) { isc_result_t result; - dns_zone_t *dummy = NULL; - unsigned int rbtoptions = 0; + dns_qpread_t qpr; + void *pval = NULL; + uint32_t ival; + dns_ztfind_t exactmask = DNS_ZTFIND_NOEXACT | DNS_ZTFIND_EXACT; + dns_ztfind_t exactopts = options & exactmask; REQUIRE(VALID_ZT(zt)); + REQUIRE(exactopts != exactmask); - if ((options & DNS_ZTFIND_NOEXACT) != 0) { - rbtoptions |= DNS_RBTFIND_NOEXACT; + if (isc_tid() == ISC_TID_UNKNOWN) { + dns_qpmulti_lockedread(zt->multi, &qpr); + } else { + dns_qpmulti_query(zt->multi, &qpr); } + if (exactopts == DNS_ZTFIND_EXACT) { + result = dns_qp_getname(&qpr, name, &pval, &ival); + } else if (exactopts == DNS_ZTFIND_NOEXACT) { + result = dns_qp_findname_parent(&qpr, name, DNS_QPFIND_NOEXACT, + &pval, &ival); + } else { + result = dns_qp_findname_parent(&qpr, name, 0, &pval, &ival); + } + dns_qpread_destroy(zt->multi, &qpr); - RWLOCK(&zt->rwlock, isc_rwlocktype_read); - - result = dns_rbt_findname(zt->table, name, rbtoptions, foundname, - (void **)&dummy); if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { + dns_zone_t *zone = pval; /* * If DNS_ZTFIND_MIRROR is set and the zone which was * determined to be the deepest match for the supplied name is @@ -190,17 +212,15 @@ dns_zt_find(dns_zt_t *zt, const dns_name_t *name, unsigned int options, * arguably not worth the added complexity. */ if ((options & DNS_ZTFIND_MIRROR) != 0 && - dns_zone_gettype(dummy) == dns_zone_mirror && - !dns_zone_isloaded(dummy)) + dns_zone_gettype(zone) == dns_zone_mirror && + !dns_zone_isloaded(zone)) { result = ISC_R_NOTFOUND; } else { - dns_zone_attach(dummy, zonep); + dns_zone_attach(zone, zonep); } } - RWUNLOCK(&zt->rwlock, isc_rwlocktype_read); - return (result); } @@ -226,12 +246,10 @@ zt_destroy(dns_zt_t *zt) { isc_refcount_destroy(&zt->loads_pending); if (atomic_load_acquire(&zt->flush)) { - (void)dns_zt_apply(zt, isc_rwlocktype_none, false, NULL, flush, - NULL); + (void)dns_zt_apply(zt, false, NULL, flush, NULL); } - dns_rbt_destroy(&zt->table); - isc_rwlock_destroy(&zt->rwlock); + dns_qpmulti_destroy(&zt->multi); zt->magic = 0; isc_mem_putanddetach(&zt->mctx, zt, sizeof(*zt)); } @@ -256,22 +274,10 @@ dns_zt_flush(dns_zt_t *zt) { atomic_store_release(&zt->flush, true); } -isc_result_t -dns_zt_load(dns_zt_t *zt, bool stop, bool newonly) { - isc_result_t result; - struct zt_load_params params; - REQUIRE(VALID_ZT(zt)); - params.newonly = newonly; - result = dns_zt_apply(zt, isc_rwlocktype_read, stop, NULL, load, - ¶ms); - return (result); -} - static isc_result_t -load(dns_zone_t *zone, void *paramsv) { +load(dns_zone_t *zone, void *uap) { isc_result_t result; - struct zt_load_params *params = (struct zt_load_params *)paramsv; - result = dns_zone_load(zone, params->newonly); + result = dns_zone_load(zone, uap != NULL); if (result == DNS_R_CONTINUE || result == DNS_R_UPTODATE || result == DNS_R_DYNAMIC) { @@ -280,71 +286,41 @@ load(dns_zone_t *zone, void *paramsv) { return (result); } -static void -call_loaddone(dns_zt_t *zt) { - dns_zt_allloaded_t loaddone = zt->loaddone; - void *loaddone_arg = zt->loaddone_arg; - - /* - * Set zt->loaddone, zt->loaddone_arg and zt->loadparams to NULL - * before calling loaddone. - */ - zt->loaddone = NULL; - zt->loaddone_arg = NULL; - - isc_mem_put(zt->mctx, zt->loadparams, sizeof(struct zt_load_params)); - zt->loadparams = NULL; +isc_result_t +dns_zt_load(dns_zt_t *zt, bool stop, bool newonly) { + REQUIRE(VALID_ZT(zt)); + return (dns_zt_apply(zt, stop, NULL, load, newonly ? &newonly : NULL)); +} - /* - * Call the callback last. - */ - if (loaddone != NULL) { - loaddone(loaddone_arg); +static void +loaded_all(struct zt_load_params *params) { + if (params->loaddone != NULL) { + params->loaddone(params->loaddone_arg); } + isc_mem_put(params->zt->mctx, params, sizeof(*params)); } -isc_result_t -dns_zt_asyncload(dns_zt_t *zt, bool newonly, dns_zt_allloaded_t alldone, - void *arg) { - isc_result_t result; - uint_fast32_t loads_pending; +/* + * Decrement the loads_pending counter; when counter reaches + * zero, call the loaddone callback that was initially set by + * dns_zt_asyncload(). + */ +static isc_result_t +loaded_one(void *uap) { + struct zt_load_params *params = uap; + dns_zt_t *zt = params->zt; REQUIRE(VALID_ZT(zt)); - /* - * Obtain a reference to zt->loads_pending so that asyncload can - * safely decrement both zt->references and zt->loads_pending - * without going to zero. - */ - loads_pending = isc_refcount_increment0(&zt->loads_pending); - INSIST(loads_pending == 0); - - /* - * Only one dns_zt_asyncload call at a time should be active so - * these pointers should be NULL. They are set back to NULL - * before the zt->loaddone (alldone) is called in call_loaddone. - */ - INSIST(zt->loadparams == NULL); - INSIST(zt->loaddone == NULL); - INSIST(zt->loaddone_arg == NULL); - - zt->loadparams = isc_mem_get(zt->mctx, sizeof(struct zt_load_params)); - zt->loadparams->dl = doneloading; - zt->loadparams->newonly = newonly; - zt->loaddone = alldone; - zt->loaddone_arg = arg; - - result = dns_zt_apply(zt, isc_rwlocktype_read, false, NULL, asyncload, - zt); - - /* - * Have all the loads completed? - */ if (isc_refcount_decrement(&zt->loads_pending) == 1) { - call_loaddone(zt); + loaded_all(params); } - return (result); + if (isc_refcount_decrement(&zt->references) == 1) { + zt_destroy(zt); + } + + return (ISC_R_SUCCESS); } /* @@ -353,16 +329,18 @@ dns_zt_asyncload(dns_zt_t *zt, bool newonly, dns_zt_allloaded_t alldone, * the zone loading is complete. */ static isc_result_t -asyncload(dns_zone_t *zone, void *zt_) { +asyncload(dns_zone_t *zone, void *uap) { + struct zt_load_params *params = uap; + struct dns_zt *zt = params->zt; isc_result_t result; - struct dns_zt *zt = (dns_zt_t *)zt_; + + REQUIRE(VALID_ZT(zt)); REQUIRE(zone != NULL); isc_refcount_increment(&zt->references); isc_refcount_increment(&zt->loads_pending); - result = dns_zone_asyncload(zone, zt->loadparams->newonly, - *zt->loadparams->dl, zt); + result = dns_zone_asyncload(zone, params->newonly, loaded_one, params); if (result != ISC_R_SUCCESS) { /* * Caller is holding a reference to zt->loads_pending @@ -375,18 +353,40 @@ asyncload(dns_zone_t *zone, void *zt_) { } isc_result_t -dns_zt_freezezones(dns_zt_t *zt, dns_view_t *view, bool freeze) { - isc_result_t result, tresult; - struct zt_freeze_params params = { view, freeze }; +dns_zt_asyncload(dns_zt_t *zt, bool newonly, dns_zt_callback_t *loaddone, + void *arg) { + isc_result_t result; + uint_fast32_t loads_pending; + struct zt_load_params *params = NULL; REQUIRE(VALID_ZT(zt)); - result = dns_zt_apply(zt, isc_rwlocktype_read, false, &tresult, - freezezones, ¶ms); - if (tresult == ISC_R_NOTFOUND) { - tresult = ISC_R_SUCCESS; + /* + * Obtain a reference to zt->loads_pending so that asyncload can + * safely decrement both zt->references and zt->loads_pending + * without going to zero. + */ + loads_pending = isc_refcount_increment0(&zt->loads_pending); + INSIST(loads_pending == 0); + + params = isc_mem_get(zt->mctx, sizeof(*params)); + *params = (struct zt_load_params){ + .zt = zt, + .newonly = newonly, + .loaddone = loaddone, + .loaddone_arg = arg, + }; + + result = dns_zt_apply(zt, false, NULL, asyncload, params); + + /* + * Have all the loads completed? + */ + if (isc_refcount_decrement(&zt->loads_pending) == 1) { + loaded_all(params); } - return ((result == ISC_R_SUCCESS) ? tresult : result); + + return (result); } static isc_result_t @@ -447,8 +447,8 @@ freezezones(dns_zone_t *zone, void *uap) { } } view = dns_zone_getview(zone); - if (strcmp(view->name, "_bind") == 0 || strcmp(view->name, "_defaul" - "t") == 0) + if (strcmp(view->name, "_bind") == 0 || + strcmp(view->name, "_default") == 0) { vname = ""; sep = ""; @@ -470,143 +470,70 @@ freezezones(dns_zone_t *zone, void *uap) { return (result); } -void -dns_zt_setviewcommit(dns_zt_t *zt) { - dns_rbtnode_t *node; - dns_rbtnodechain_t chain; - isc_result_t result; +isc_result_t +dns_zt_freezezones(dns_zt_t *zt, dns_view_t *view, bool freeze) { + isc_result_t result, tresult; + struct zt_freeze_params params = { view, freeze }; REQUIRE(VALID_ZT(zt)); - RWLOCK(&zt->rwlock, isc_rwlocktype_read); - dns_rbtnodechain_init(&chain); + result = dns_zt_apply(zt, false, &tresult, freezezones, ¶ms); + if (tresult == ISC_R_NOTFOUND) { + tresult = ISC_R_SUCCESS; + } + return ((result == ISC_R_SUCCESS) ? tresult : result); +} - result = dns_rbtnodechain_first(&chain, zt->table, NULL, NULL); - while (result == DNS_R_NEWORIGIN || result == ISC_R_SUCCESS) { - result = dns_rbtnodechain_current(&chain, NULL, NULL, &node); - if (result == ISC_R_SUCCESS && node->data != NULL) { - dns_zone_setviewcommit(node->data); - } +typedef void +setview_cb(dns_zone_t *zone); - result = dns_rbtnodechain_next(&chain, NULL, NULL); - } +static isc_result_t +setview(dns_zone_t *zone, void *arg) { + setview_cb *cb = arg; + cb(zone); + return (ISC_R_SUCCESS); +} - dns_rbtnodechain_invalidate(&chain); - RWUNLOCK(&zt->rwlock, isc_rwlocktype_read); +void +dns_zt_setviewcommit(dns_zt_t *zt) { + dns_zt_apply(zt, false, NULL, setview, dns_zone_setviewcommit); } void dns_zt_setviewrevert(dns_zt_t *zt) { - dns_rbtnode_t *node; - dns_rbtnodechain_t chain; - isc_result_t result; - - REQUIRE(VALID_ZT(zt)); - - dns_rbtnodechain_init(&chain); - - result = dns_rbtnodechain_first(&chain, zt->table, NULL, NULL); - while (result == DNS_R_NEWORIGIN || result == ISC_R_SUCCESS) { - result = dns_rbtnodechain_current(&chain, NULL, NULL, &node); - if (result == ISC_R_SUCCESS && node->data != NULL) { - dns_zone_setviewrevert(node->data); - } - - result = dns_rbtnodechain_next(&chain, NULL, NULL); - } - - dns_rbtnodechain_invalidate(&chain); + dns_zt_apply(zt, false, NULL, setview, dns_zone_setviewrevert); } isc_result_t -dns_zt_apply(dns_zt_t *zt, isc_rwlocktype_t lock, bool stop, isc_result_t *sub, +dns_zt_apply(dns_zt_t *zt, bool stop, isc_result_t *sub, isc_result_t (*action)(dns_zone_t *, void *), void *uap) { - dns_rbtnode_t *node; - dns_rbtnodechain_t chain; - isc_result_t result, tresult = ISC_R_SUCCESS; - dns_zone_t *zone; + isc_result_t result = ISC_R_SUCCESS; + isc_result_t tresult = ISC_R_SUCCESS; + dns_qpiter_t qpi; + dns_qpread_t qpr; + void *zone = NULL; + uint32_t ival; REQUIRE(VALID_ZT(zt)); REQUIRE(action != NULL); - if (lock != isc_rwlocktype_none) { - RWLOCK(&zt->rwlock, lock); - } + dns_qpmulti_query(zt->multi, &qpr); + dns_qpiter_init(&qpr, &qpi); - dns_rbtnodechain_init(&chain); - result = dns_rbtnodechain_first(&chain, zt->table, NULL, NULL); - if (result == ISC_R_NOTFOUND) { - /* - * The tree is empty. - */ - tresult = result; - result = ISC_R_NOMORE; - } - while (result == DNS_R_NEWORIGIN || result == ISC_R_SUCCESS) { - result = dns_rbtnodechain_current(&chain, NULL, NULL, &node); - if (result == ISC_R_SUCCESS) { - zone = node->data; - if (zone != NULL) { - result = (action)(zone, uap); - } - if (result != ISC_R_SUCCESS && stop) { - tresult = result; - goto cleanup; /* don't break */ - } else if (result != ISC_R_SUCCESS && - tresult == ISC_R_SUCCESS) - { - tresult = result; - } + while (dns_qpiter_next(&qpi, &zone, &ival) == ISC_R_SUCCESS) { + result = action(zone, uap); + if (tresult == ISC_R_SUCCESS) { + tresult = result; + } + if (result != ISC_R_SUCCESS && stop) { + break; } - result = dns_rbtnodechain_next(&chain, NULL, NULL); - } - if (result == ISC_R_NOMORE) { - result = ISC_R_SUCCESS; } + dns_qpread_destroy(zt->multi, &qpr); -cleanup: - dns_rbtnodechain_invalidate(&chain); if (sub != NULL) { *sub = tresult; } - if (lock != isc_rwlocktype_none) { - RWUNLOCK(&zt->rwlock, lock); - } - return (result); } - -/* - * Decrement the loads_pending counter; when counter reaches - * zero, call the loaddone callback that was initially set by - * dns_zt_asyncload(). - */ -static isc_result_t -doneloading(dns_zt_t *zt, dns_zone_t *zone) { - REQUIRE(VALID_ZT(zt)); - - UNUSED(zone); - - if (isc_refcount_decrement(&zt->loads_pending) == 1) { - call_loaddone(zt); - } - - if (isc_refcount_decrement(&zt->references) == 1) { - zt_destroy(zt); - } - - return (ISC_R_SUCCESS); -} - -/*** - *** Private - ***/ - -static void -auto_detach(void *data, void *arg) { - dns_zone_t *zone = data; - - UNUSED(arg); - dns_zone_detach(&zone); -} diff --git a/lib/ns/notify.c b/lib/ns/notify.c index fd5b474c0c1..dd3a2882f5c 100644 --- a/lib/ns/notify.c +++ b/lib/ns/notify.c @@ -146,7 +146,7 @@ ns_notify_start(ns_client_t *client, isc_nmhandle_t *handle) { } dns_name_format(zonename, namebuf, sizeof(namebuf)); - result = dns_zt_find(client->view->zonetable, zonename, 0, NULL, &zone); + result = dns_view_findzone(client->view, zonename, &zone); if (result == ISC_R_SUCCESS) { dns_zonetype_t zonetype = dns_zone_gettype(zone); @@ -166,10 +166,10 @@ ns_notify_start(ns_client_t *client, isc_nmhandle_t *handle) { } } - notify_log(client, ISC_LOG_NOTICE, - "received notify for zone '%s'%s: not authoritative", - namebuf, tsigbuf); result = DNS_R_NOTAUTH; + notify_log(client, ISC_LOG_NOTICE, + "received notify for zone '%s'%s: %s", namebuf, tsigbuf, + isc_result_totext(result)); done: if (zone != NULL) { diff --git a/lib/ns/query.c b/lib/ns/query.c index a5fb82169aa..83413ec0ece 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -1116,8 +1116,7 @@ query_getzonedb(ns_client_t *client, const dns_name_t *name, ztoptions |= DNS_ZTFIND_NOEXACT; } - result = dns_zt_find(client->view->zonetable, name, ztoptions, NULL, - &zone); + result = dns_zt_find(client->view->zonetable, name, ztoptions, &zone); if (result == DNS_R_PARTIALMATCH) { partial = true; diff --git a/lib/ns/update.c b/lib/ns/update.c index 9b0e325a985..e99afc1b01d 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -1988,15 +1988,8 @@ ns_update_start(ns_client_t *client, isc_nmhandle_t *handle, "RRs"); } - result = dns_zt_find(client->view->zonetable, zonename, 0, NULL, &zone); + result = dns_view_findzone(client->view, zonename, &zone); if (result != ISC_R_SUCCESS) { - /* - * If we found a zone that is a parent of the update zonename, - * detach it so it isn't mentioned in log - it is irrelevant. - */ - if (zone != NULL) { - dns_zone_detach(&zone); - } FAILN(DNS_R_NOTAUTH, zonename, "not authoritative for update zone"); } diff --git a/lib/ns/xfrout.c b/lib/ns/xfrout.c index 23484caa5c5..b42c08cc6a7 100644 --- a/lib/ns/xfrout.c +++ b/lib/ns/xfrout.c @@ -797,9 +797,7 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) { FAILC(DNS_R_FORMERR, "multiple questions"); } - result = dns_zt_find(client->view->zonetable, question_name, 0, NULL, - &zone); - + result = dns_view_findzone(client->view, question_name, &zone); if (result != ISC_R_SUCCESS || dns_zone_gettype(zone) == dns_zone_dlz) { /* * The normal zone table does not have a match, or this is diff --git a/tests/dns/zt_test.c b/tests/dns/zt_test.c index 18a28488534..f3821abc2ef 100644 --- a/tests/dns/zt_test.c +++ b/tests/dns/zt_test.c @@ -64,8 +64,8 @@ ISC_LOOP_TEST_IMPL(apply) { assert_non_null(view->zonetable); assert_int_equal(nzones, 0); - result = dns_zt_apply(view->zonetable, isc_rwlocktype_read, false, NULL, - count_zone, &nzones); + result = dns_zt_apply(view->zonetable, false, NULL, count_zone, + &nzones); assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(nzones, 1); @@ -83,12 +83,10 @@ ISC_LOOP_TEST_IMPL(apply) { } static isc_result_t -load_done_last(dns_zt_t *zt, dns_zone_t *zone) { +load_done_last(void *uap) { + dns_zone_t *zone = uap; isc_result_t result; - UNUSED(zt); - UNUSED(zone); - /* The zone should now be loaded; test it */ result = dns_zone_getdb(zone, &db); assert_int_equal(result, ISC_R_SUCCESS); @@ -110,29 +108,25 @@ load_done_last(dns_zt_t *zt, dns_zone_t *zone) { } static isc_result_t -load_done_new_only(dns_zt_t *zt, dns_zone_t *zone) { +load_done_new_only(void *uap) { + dns_zone_t *zone = uap; isc_result_t result; - UNUSED(zt); - UNUSED(zone); - /* The zone should now be loaded; test it */ result = dns_zone_getdb(zone, &db); assert_int_equal(result, ISC_R_SUCCESS); dns_db_detach(&db); - dns_zone_asyncload(zone, true, load_done_last, NULL); + dns_zone_asyncload(zone, true, load_done_last, zone); return (ISC_R_SUCCESS); } static isc_result_t -load_done_first(dns_zt_t *zt, dns_zone_t *zone) { - atomic_bool *done = (atomic_bool *)zt; +load_done_first(void *uap) { + dns_zone_t *zone = uap; isc_result_t result; - UNUSED(zone); - /* The zone should now be loaded; test it */ result = dns_zone_getdb(zone, &db); assert_int_equal(result, ISC_R_SUCCESS); @@ -146,7 +140,7 @@ load_done_first(dns_zt_t *zt, dns_zone_t *zone) { fflush(zonefile); fclose(zonefile); - dns_zone_asyncload(zone, true, load_done_new_only, &done); + dns_zone_asyncload(zone, true, load_done_new_only, zone); return (ISC_R_SUCCESS); } @@ -157,9 +151,6 @@ ISC_LOOP_TEST_IMPL(asyncload_zone) { int n; dns_zone_t *zone = NULL; char buf[4096]; - atomic_bool done; - - atomic_init(&done, false); result = dns_test_makezone("foo", &zone, NULL, true); assert_int_equal(result, ISC_R_SUCCESS); @@ -172,7 +163,6 @@ ISC_LOOP_TEST_IMPL(asyncload_zone) { assert_non_null(view->zonetable); assert_false(dns__zone_loadpending(zone)); - assert_false(atomic_load(&done)); zonefile = fopen("./zone.data", "wb"); assert_non_null(zonefile); origfile = fopen(TESTS_DIR "/testdata/zt/zone1.db", "r+b"); @@ -185,7 +175,7 @@ ISC_LOOP_TEST_IMPL(asyncload_zone) { dns_zone_setfile(zone, "./zone.data", dns_masterformat_text, &dns_master_style_default); - dns_zone_asyncload(zone, false, load_done_first, &done); + dns_zone_asyncload(zone, false, load_done_first, zone); } dns_zone_t *zone1 = NULL, *zone2 = NULL, *zone3 = NULL; diff --git a/tests/libtest/dns.c b/tests/libtest/dns.c index d398c5c954f..b093b88c43a 100644 --- a/tests/libtest/dns.c +++ b/tests/libtest/dns.c @@ -64,7 +64,7 @@ dns_test_makeview(const char *name, bool with_cache, dns_view_t **viewp) { dns_view_t *view = NULL; dns_cache_t *cache = NULL; - result = dns_view_create(mctx, dns_rdataclass_in, name, &view); + result = dns_view_create(mctx, loopmgr, dns_rdataclass_in, name, &view); if (result != ISC_R_SUCCESS) { return (result); }