From b171cacf4f0123ba96bef6eedfc92dfb608db6b7 Mon Sep 17 00:00:00 2001 From: Tony Finch Date: Tue, 14 Feb 2023 16:13:16 +0000 Subject: [PATCH] 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. --- CHANGES | 3 + bin/delv/delv.c | 3 +- bin/named/server.c | 64 ++- bin/named/statschannel.c | 9 +- bin/tests/system/catz/tests.sh | 24 +- bin/tests/system/dyndb/driver/syncptr.c | 2 +- bin/tests/system/pipelined/pipequeries.c | 2 +- bin/tools/mdig.c | 2 +- fuzz/dns_message_checksig.c | 8 +- lib/dns/catz.c | 19 +- lib/dns/client.c | 3 +- lib/dns/include/dns/view.h | 25 +- lib/dns/include/dns/zone.h | 7 +- lib/dns/include/dns/zt.h | 79 ++-- lib/dns/resolver.c | 29 +- lib/dns/view.c | 126 +----- lib/dns/zone.c | 14 +- lib/dns/zt.c | 499 ++++++++++------------- lib/ns/notify.c | 8 +- lib/ns/query.c | 3 +- lib/ns/update.c | 9 +- lib/ns/xfrout.c | 4 +- tests/dns/zt_test.c | 32 +- tests/libtest/dns.c | 2 +- 24 files changed, 374 insertions(+), 602 deletions(-) diff --git a/CHANGES b/CHANGES index b1ce3a727b..4634446aa1 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 a354a60e65..91ebe62348 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 8bc4e32db9..8e3ba56205 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 7b595ec267..0b9eb6b862 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 ddab8f84f7..756edb9b18 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 15defbd2cc..270c7acca6 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 805e3c5344..5569ad318d 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 98e72ffb2e..4193f06912 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 6679df60bd..c27bfebe8b 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 70b160469f..f4b9c962fa 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 579b9c3aab..5a3c0fadda 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 a552327bad..b72a3dd44f 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 0c71cfceaa..db67203797 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 afe13ead85..1118d06428 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 dab6b404ed..194caf6539 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 133d6775bb..5f58142627 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 2e31a82963..5dd59da646 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 9cfe366ba1..cf91f7285b 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 +ztqpattach(void *uctx ISC_ATTR_UNUSED, void *pval, + uint32_t ival ISC_ATTR_UNUSED) { + dns_zone_t *zone = pval; + dns_zone_ref(zone); +} static void -auto_detach(void *, 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 -load(dns_zone_t *zone, void *uap); +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 -asyncload(dns_zone_t *zone, void *callback); +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 -freezezones(dns_zone_t *zone, void *uap); +static dns_qpmethods_t ztqpmethods = { + ztqpattach, + ztqpdetach, + ztqpmakekey, + ztqptriename, +}; -static isc_result_t -doneloading(dns_zt_t *zt, dns_zone_t *zone); - -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); + + dns_qpmulti_create(mctx, loopmgr, &ztqpmethods, view, &multi); zt = isc_mem_get(mctx, sizeof(*zt)); + *zt = (dns_zt_t){ + .magic = ZTMAGIC, + .multi = multi, + .references = 1, + }; - zt->table = NULL; - result = dns_rbt_create(mctx, auto_detach, zt, &zt->table); - if (result != ISC_R_SUCCESS) { - goto cleanup_zt; - } - - 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; - - /* - * Call the callback last. - */ - if (loaddone != NULL) { - loaddone(loaddone_arg); - } +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)); } -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; +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)); +} + +/* + * 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_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); - } - - result = dns_rbtnodechain_next(&chain, NULL, NULL); + 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); +} - dns_rbtnodechain_invalidate(&chain); - RWUNLOCK(&zt->rwlock, isc_rwlocktype_read); +typedef void +setview_cb(dns_zone_t *zone); + +static isc_result_t +setview(dns_zone_t *zone, void *arg) { + setview_cb *cb = arg; + cb(zone); + return (ISC_R_SUCCESS); +} + +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 fd5b474c0c..dd3a2882f5 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 a5fb82169a..83413ec0ec 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 9b0e325a98..e99afc1b01 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 23484caa5c..b42c08cc6a 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 18a2848853..f3821abc2e 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 d398c5c954..b093b88c43 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); }