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/fuzz/dns_qp.c b/fuzz/dns_qp.c index a528019597..1bc19fa38f 100644 --- a/fuzz/dns_qp.c +++ b/fuzz/dns_qp.c @@ -57,19 +57,18 @@ static struct { dns_qpkey_t ascii; } item[256 * 256 / 4]; -static uint32_t +static void fuzz_attach(void *ctx, void *pval, uint32_t ival) { assert(ctx == NULL); assert(pval == &item[ival]); - return (item[ival].refcount++); + item[ival].refcount++; } -static uint32_t +static void fuzz_detach(void *ctx, void *pval, uint32_t ival) { assert(ctx == NULL); assert(pval == &item[ival]); - assert(item[ival].refcount > 0); - return (item[ival].refcount--); + item[ival].refcount--; } static size_t @@ -86,7 +85,7 @@ fuzz_triename(void *ctx, char *buf, size_t size) { strlcpy(buf, "fuzz", size); } -const struct dns_qpmethods fuzz_methods = { +const dns_qpmethods_t fuzz_methods = { fuzz_attach, fuzz_detach, fuzz_makekey, 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/qp.h b/lib/dns/include/dns/qp.h index 5e9a1f25a1..a425a4c257 100644 --- a/lib/dns/include/dns/qp.h +++ b/lib/dns/include/dns/qp.h @@ -19,7 +19,11 @@ * * Keys are `dns_qpkey_t`, which is a string-like thing, usually created * from a DNS name. You can use both relative and absolute DNS names as - * keys. + * keys, even in the same trie, except for one caveat: if a trie contains + * names relative to the zone apex, the natural way to represent the apex + * itself (spelled `@` in zone files) is a zero-length name; but a + * zero-length name has the same qpkey representation as the root zone + * (apart from its length), so they collide. * * Leaf values are a pair of a `void *` pointer and a `uint32_t` * (because that is what fits inside an internal qp-trie leaf node). @@ -49,6 +53,12 @@ * lifetime of a `dns_qpread_t`, instead of using locks. Readers are * not blocked by any write activity, and vice versa. * + * For read-only access outside the scope of a loop, such as from an + * isc_work callback, `dns_qpmulti_lockedread()`. This looks like a + * query transaction; the difference is that a locked read transaction + * takes the `dns_qpmulti_t` mutex. When you have finished with a + * `dns_qpread_t`, call `dns_qpread_destroy()` to release the mutex. + * * For reads that need a stable view of the trie for multiple cycles * of an isc_loop, or which can be used from any thread, call * `dns_qpmulti_snapshot()` to get a `dns_qpsnap_t`. A snapshot is for @@ -99,7 +109,9 @@ typedef struct dns_qpmulti dns_qpmulti_t; * Read-only parts of a qp-trie. * * A `dns_qpreader_t` is the common prefix of the `dns_qpreadable` - * types, containing just the fields neded for the hot path. + * types, containing just the fields neded for the hot path. The + * internals of a `dns_qpreader_t` are private; they are only exposed + * so that callers can allocate a `dns_qpread_t` on the stack. * * Ranty aside: annoyingly, C doesn't allow us to use a predeclared * structure type as an anonymous struct member, so we have to use a @@ -125,6 +137,9 @@ typedef struct dns_qpreader { * The caller provides space for it on the stack; it can be * used by only one thread. As well as the `DNS_QPREADER_FIELDS`, * it contains a thread ID to check for incorrect usage. + * + * The internals of a `dns_qpread_t` are private; they are only + * exposed so that callers can allocate an instance on the stack. */ typedef struct dns_qpread { DNS_QPREADER_FIELDS; @@ -154,10 +169,7 @@ typedef union dns_qpreadable { #define dns_qpreader(qpr) ((qpr).qp) /*% - * A trie lookup key is a small array, allocated on the stack during trie - * searches. Keys are usually created on demand from DNS names using - * `dns_qpkey_fromname()`, but in principle you can define your own - * functions to convert other types to trie lookup keys. + * The maximum size of a key is also the maximum depth of a trie. * * A domain name can be up to 255 bytes. When converted to a key, each * character in the name corresponds to one byte in the key if it is a @@ -165,7 +177,29 @@ typedef union dns_qpreadable { * using two bytes in the key. So we allow keys to be up to 512 bytes. * (The actual max is (255 - 5) * 2 + 6 == 506) */ -typedef uint8_t dns_qpkey_t[512]; +#define DNS_QP_MAXKEY 512 + +/*% + * A trie lookup key is a small array, allocated on the stack during trie + * searches. Keys are usually created on demand from DNS names using + * `dns_qpkey_fromname()`, but in principle you can define your own + * functions to convert other types to trie lookup keys. + */ +typedef uint8_t dns_qpkey_t[DNS_QP_MAXKEY]; + +/*% + * A trie iterator describes a path through the trie from the root to + * a leaf node, for use with `dns_qpiter_init()` and `dns_qpiter_next()`. + */ +typedef struct dns_qpiter { + unsigned int magic; + dns_qpreader_t *qp; + uint16_t sp; + struct __attribute__((__packed__)) { + uint32_t ref; + uint8_t more; + } stack[DNS_QP_MAXKEY]; +} dns_qpiter_t; /*% * These leaf methods allow the qp-trie code to call back to the code @@ -180,9 +214,7 @@ typedef uint8_t dns_qpkey_t[512]; * The `attach` and `detach` methods adjust reference counts on value * objects. They support copy-on-write and safe memory reclamation * needed for multi-version concurrency. The methods are only called - * when the `dns_qpmulti_t` mutex is held. For tracing purposes, they - * should return the same value as `isc_refcount_increment()` or - * `isc_refcount_decrement()`, respectively + * when the `dns_qpmulti_t` mutex is held. * * Note: When a value object reference count is greater than one, the * object is in use by concurrent readers so it must not be modified. A @@ -201,8 +233,8 @@ typedef uint8_t dns_qpkey_t[512]; * readable identifier into `buf` which has max length `size`. */ typedef struct dns_qpmethods { - uint32_t (*attach)(void *uctx, void *pval, uint32_t ival); - uint32_t (*detach)(void *uctx, void *pval, uint32_t ival); + void (*attach)(void *uctx, void *pval, uint32_t ival); + void (*detach)(void *uctx, void *pval, uint32_t ival); size_t (*makekey)(dns_qpkey_t key, void *uctx, void *pval, uint32_t ival); void (*triename)(void *uctx, char *buf, size_t size); @@ -240,6 +272,13 @@ typedef enum dns_qpgc { DNS_QPGC_ALL, } dns_qpgc_t; +/*% + * Options for fancy searches such as `dns_qp_findname_parent()` + */ +typedef enum dns_qpfind { + DNS_QPFIND_NOEXACT = 1 << 0, +} dns_qpfind_t; + /*********************************************************************** * * functions - create, destory, enquire @@ -376,16 +415,13 @@ dns_qpmulti_memusage(dns_qpmulti_t *multi); /* * XXXFANF todo, based on what we discover BIND needs * - * fancy searches: longest match, lexicographic predecessor, - * etc. + * more fancy searches: lexicographic predecessor (for NSEC), + * successor (for modification-safe iteration), etc. * * do we need specific lookup functions to find out if the * returned value is readonly or mutable? * * richer modification such as dns_qp_replace{key,name} - * - * iteration - probably best to put an explicit stack in the iterator, - * cf. rbtnodechain */ size_t @@ -441,6 +477,30 @@ dns_qp_getname(dns_qpreadable_t qpr, const dns_name_t *name, void **pval_r, * \li ISC_R_SUCCESS if the leaf was found */ +isc_result_t +dns_qp_findname_parent(dns_qpreadable_t qpr, const dns_name_t *name, + dns_qpfind_t options, void **pval_r, uint32_t *ival_r); +/*%< + * Find a leaf in a qp-trie that is a parent domain of or equal to the + * given DNS name. + * + * If the DNS_QPFIND_NOEXACT option is set, find a strict parent + * domain not equal to the search name. + * + * The leaf values are assigned to `*pval_r` and `*ival_r` + * + * Requires: + * \li `qpr` is a pointer to a readable qp-trie + * \li `name` is a pointer to a valid `dns_name_t` + * \li `pval_r != NULL` + * \li `ival_r != NULL` + * + * Returns: + * \li ISC_R_SUCCESS if an exact match was found + * \li ISC_R_PARTIALMATCH if a parent domain was found + * \li ISC_R_NOTFOUND if no match was found + */ + isc_result_t dns_qp_insert(dns_qp_t *qp, void *pval, uint32_t ival); /*%< @@ -484,6 +544,48 @@ dns_qp_deletename(dns_qp_t *qp, const dns_name_t *name); * \li ISC_R_SUCCESS if the leaf was deleted from the trie */ +void +dns_qpiter_init(dns_qpreadable_t qpr, dns_qpiter_t *qpi); +/*%< + * Initialize an iterator + * + * SAFETY NOTE: If `qpr` is a `dns_qp_t`, it is not safe to modify the + * trie during iteration. If `qpr` is a `dns_qpread_t` or `dns_qpsnap_t` + * then (like any other read-only access) modifications will not affect + * iteration. + * + * Requires: + * \li `qp` is a pointer to a valid qp-trie + * \li `qpi` is a pointer to a qp iterator + */ + +isc_result_t +dns_qpiter_next(dns_qpiter_t *qpi, void **pval_r, uint32_t *ival_r); +/*%< + * Get the next leaf object of a trie in lexicographic order of its keys. + * + * NOTE: see the safety note under `dns_qpiter_init()`. + * + * For example, + * + * dns_qpiter_t qpi; + * void *pval; + * uint32_t ival; + * dns_qpiter_init(qp, &qpi); + * while (dns_qpiter_next(&qpi, &pval, &ival)) { + * // do something with pval and ival + * } + * + * Requires: + * \li `qpi` is a pointer to a valid qp iterator + * \li `pval_r != NULL` + * \li `ival_r != NULL` + * + * Returns: + * \li ISC_R_SUCCESS if a leaf was found and pval_r and ival_r were set + * \li ISC_R_NOMORE otherwise + */ + /*********************************************************************** * * functions - transactions @@ -505,10 +607,27 @@ dns_qpmulti_query(dns_qpmulti_t *multi, dns_qpread_t *qpr); * \li `qpr` is a valid read-only qp-trie handle */ +void +dns_qpmulti_lockedread(dns_qpmulti_t *multi, dns_qpread_t *qpr); +/*%< + * Start a read-only transaction that takes the `dns_qpmulti_t` mutex. + * + * The `dns_qpmulti_lockedread()` function must NOT be called from an + * isc_loop thread. We keep query and read transactions separate to + * avoid accidentally taking or failing to take the mutex. + * + * Requires: + * \li `multi` is a pointer to a valid multi-threaded qp-trie + * \li `qpr != NULL` + * + * Returns: + * \li `qpr` is a valid read-only qp-trie handle + */ + void dns_qpread_destroy(dns_qpmulti_t *multi, dns_qpread_t *qpr); /*%< - * End a lightweight read transaction. + * End a lightweight query or read transaction. * * Requires: * \li `multi` is a pointer to a valid multi-threaded qp-trie 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/qp.c b/lib/dns/qp.c index e990a61e3f..60acf13ed5 100644 --- a/lib/dns/qp.c +++ b/lib/dns/qp.c @@ -278,6 +278,25 @@ qpkey_compare(const dns_qpkey_t key_a, const size_t keylen_a, return (QPKEY_EQUAL); } +/* + * Given a key constructed by dns_qpkey_fromname(), trim it down to the last + * label boundary before the `max` length. + * + * This is used when searching a trie for the best match for a name. + */ +static size_t +qpkey_trim_label(dns_qpkey_t key, size_t len, size_t max) { + size_t stop = 0; + for (size_t offset = 0; offset < max; offset++) { + if (qpkey_bit(key, len, offset) == SHIFT_NOBYTE && + qpkey_bit(key, len, offset + 1) != SHIFT_NOBYTE) + { + stop = offset + 1; + } + } + return (stop); +} + /*********************************************************************** * * allocator wrappers @@ -463,6 +482,7 @@ static inline qp_ref_t alloc_twigs(dns_qp_t *qp, qp_weight_t size) { qp_chunk_t chunk = qp->bump; qp_cell_t cell = qp->usage[chunk].used; + if (cell + size <= QP_CHUNK_SIZE) { qp->usage[chunk].used += size; qp->used_count += size; @@ -824,6 +844,7 @@ compact_recursive(dns_qp_t *qp, qp_node_t *parent) { qp_weight_t size = branch_twigs_size(parent); qp_ref_t twigs_ref = branch_twigs_ref(parent); qp_chunk_t chunk = ref_chunk(twigs_ref); + if (qp->compact_all || (chunk != qp->bump && chunk_usage(qp, chunk) < QP_MIN_USED)) { @@ -884,6 +905,7 @@ dns_qp_compact(dns_qp_t *qp, dns_qpgc_t mode) { return; } if (mode == DNS_QPGC_ALL) { + alloc_reset(qp); qp->compact_all = true; } compact(qp); @@ -1007,15 +1029,12 @@ dns_qp_gctime(isc_nanosecs_t *compact_p, isc_nanosecs_t *recycle_p, static dns_qp_t * transaction_open(dns_qpmulti_t *multi, dns_qp_t **qptp) { - dns_qp_t *qp; - REQUIRE(QPMULTI_VALID(multi)); REQUIRE(qptp != NULL && *qptp == NULL); LOCK(&multi->mutex); - qp = &multi->writer; - + dns_qp_t *qp = &multi->writer; INSIST(QP_VALID(qp)); /* @@ -1263,12 +1282,31 @@ dns_qpmulti_query(dns_qpmulti_t *multi, dns_qpread_t *qp) { REQUIRE(QPMULTI_VALID(multi)); REQUIRE(qp != NULL); - dns_qpmulti_t *whence = reader_open(multi, qp); - INSIST(whence == multi); - - /* we must be in an isc_loop thread */ + /* we MUST be in an isc_loop thread */ qp->tid = isc_tid(); REQUIRE(qp->tid != ISC_TID_UNKNOWN); + + dns_qpmulti_t *whence = reader_open(multi, qp); + INSIST(whence == multi); +} + +/* + * a locked read takes the mutex + */ + +void +dns_qpmulti_lockedread(dns_qpmulti_t *multi, dns_qpread_t *qp) { + REQUIRE(QPMULTI_VALID(multi)); + REQUIRE(qp != NULL); + + /* we MUST NOT be in an isc_loop thread */ + qp->tid = isc_tid(); + REQUIRE(qp->tid == ISC_TID_UNKNOWN); + + LOCK(&multi->mutex); + + dns_qpmulti_t *whence = reader_open(multi, qp); + INSIST(whence == multi); } void @@ -1276,6 +1314,9 @@ dns_qpread_destroy(dns_qpmulti_t *multi, dns_qpread_t *qp) { REQUIRE(QPMULTI_VALID(multi)); REQUIRE(QP_VALID(qp)); REQUIRE(qp->tid == isc_tid()); + if (qp->tid == ISC_TID_UNKNOWN) { + UNLOCK(&multi->mutex); + } *qp = (dns_qpread_t){}; } @@ -1354,11 +1395,9 @@ dns_qpsnap_destroy(dns_qpmulti_t *multi, dns_qpsnap_t **qpsp) { void dns_qp_create(isc_mem_t *mctx, const dns_qpmethods_t *methods, void *uctx, dns_qp_t **qptp) { - dns_qp_t *qp; - REQUIRE(qptp != NULL && *qptp == NULL); - qp = isc_mem_get(mctx, sizeof(*qp)); + dns_qp_t *qp = isc_mem_get(mctx, sizeof(*qp)); QP_INIT(qp, methods, uctx); isc_mem_attach(mctx, &qp->mctx); alloc_reset(qp); @@ -1370,12 +1409,9 @@ void dns_qpmulti_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, const dns_qpmethods_t *methods, void *uctx, dns_qpmulti_t **qpmp) { - dns_qpmulti_t *multi; - dns_qp_t *qp; - REQUIRE(qpmp != NULL && *qpmp == NULL); - multi = isc_mem_get(mctx, sizeof(*multi)); + dns_qpmulti_t *multi = isc_mem_get(mctx, sizeof(*multi)); *multi = (dns_qpmulti_t){ .magic = QPMULTI_MAGIC, .reader_ref = INVALID_REF, @@ -1390,7 +1426,7 @@ dns_qpmulti_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, * allocates; to ensure dns_qpmulti_write() does too, pretend the * previous transaction was an update */ - qp = &multi->writer; + dns_qp_t *qp = &multi->writer; QP_INIT(qp, methods, uctx); isc_mem_attach(mctx, &qp->mctx); qp->transaction_mode = QP_UPDATE; @@ -1418,12 +1454,10 @@ destroy_guts(dns_qp_t *qp) { void dns_qp_destroy(dns_qp_t **qptp) { - dns_qp_t *qp; - REQUIRE(qptp != NULL); REQUIRE(QP_VALID(*qptp)); - qp = *qptp; + dns_qp_t *qp = *qptp; *qptp = NULL; /* do not try to destroy part of a dns_qpmulti_t */ @@ -1436,14 +1470,11 @@ dns_qp_destroy(dns_qp_t **qptp) { void dns_qpmulti_destroy(dns_qpmulti_t **qpmp) { - dns_qp_t *qp = NULL; - dns_qpmulti_t *multi = NULL; - REQUIRE(qpmp != NULL); REQUIRE(QPMULTI_VALID(*qpmp)); - multi = *qpmp; - qp = &multi->writer; + dns_qpmulti_t *multi = *qpmp; + dns_qp_t *qp = &multi->writer; *qpmp = NULL; REQUIRE(QP_VALID(qp)); @@ -1471,7 +1502,7 @@ isc_result_t dns_qp_insert(dns_qp_t *qp, void *pval, uint32_t ival) { qp_ref_t new_ref, old_ref; qp_node_t new_leaf, old_node; - qp_node_t *new_twigs, *old_twigs; + qp_node_t *new_twigs = NULL, *old_twigs = NULL; qp_shift_t new_bit, old_bit; qp_weight_t old_size, new_size; dns_qpkey_t new_key, old_key; @@ -1480,7 +1511,7 @@ dns_qp_insert(dns_qp_t *qp, void *pval, uint32_t ival) { uint64_t index; qp_shift_t bit; qp_weight_t pos; - qp_node_t *n; + qp_node_t *n = NULL; REQUIRE(QP_VALID(qp)); @@ -1597,15 +1628,6 @@ growbranch: isc_result_t dns_qp_deletekey(dns_qp_t *qp, const dns_qpkey_t search_key, size_t search_keylen) { - dns_qpkey_t found_key; - size_t found_keylen; - qp_shift_t bit = 0; /* suppress warning */ - qp_weight_t pos, size; - qp_ref_t ref; - qp_node_t *twigs; - qp_node_t *parent; - qp_node_t *n; - REQUIRE(QP_VALID(qp)); REQUIRE(search_keylen < sizeof(dns_qpkey_t)); @@ -1613,8 +1635,9 @@ dns_qp_deletekey(dns_qp_t *qp, const dns_qpkey_t search_key, return (ISC_R_NOTFOUND); } - parent = NULL; - n = make_root_mutable(qp); + qp_shift_t bit = 0; /* suppress warning */ + qp_node_t *parent = NULL; + qp_node_t *n = make_root_mutable(qp); while (is_branch(n)) { prefetch_twigs(qp, n); bit = branch_keybit(n, search_key, search_keylen); @@ -1626,7 +1649,8 @@ dns_qp_deletekey(dns_qp_t *qp, const dns_qpkey_t search_key, n = branch_twig_ptr(qp, n, bit); } - found_keylen = leaf_qpkey(qp, n, found_key); + dns_qpkey_t found_key; + size_t found_keylen = leaf_qpkey(qp, n, found_key); if (qpkey_compare(search_key, search_keylen, found_key, found_keylen) != QPKEY_EQUAL) { @@ -1650,10 +1674,10 @@ dns_qp_deletekey(dns_qp_t *qp, const dns_qpkey_t search_key, parent = NULL; INSIST(bit != 0); - size = branch_twigs_size(n); - pos = branch_twig_pos(n, bit); - ref = branch_twigs_ref(n); - twigs = ref_ptr(qp, ref); + qp_weight_t size = branch_twigs_size(n); + qp_weight_t pos = branch_twig_pos(n, bit); + qp_ref_t ref = branch_twigs_ref(n); + qp_node_t *twigs = ref_ptr(qp, ref); if (size == 2) { /* @@ -1681,6 +1705,72 @@ dns_qp_deletename(dns_qp_t *qp, const dns_name_t *name) { return (dns_qp_deletekey(qp, key, keylen)); } +/*********************************************************************** + * + * iterate + */ + +void +dns_qpiter_init(dns_qpreadable_t qpr, dns_qpiter_t *qpi) { + dns_qpreader_t *qp = dns_qpreader(qpr); + REQUIRE(QP_VALID(qp)); + REQUIRE(qpi != NULL); + qpi->magic = QPITER_MAGIC; + qpi->qp = qp; + qpi->sp = 0; + qpi->stack[qpi->sp].ref = qp->root_ref; + qpi->stack[qpi->sp].more = 0; +} + +/* + * note: this function can go wrong when the iterator refers to + * a mutable view of the trie which is altered while iterating + */ +isc_result_t +dns_qpiter_next(dns_qpiter_t *qpi, void **pval_r, uint32_t *ival_r) { + REQUIRE(QPITER_VALID(qpi)); + REQUIRE(QP_VALID(qpi->qp)); + REQUIRE(pval_r != NULL); + REQUIRE(ival_r != NULL); + + dns_qpreader_t *qp = qpi->qp; + + if (qpi->stack[qpi->sp].ref == INVALID_REF) { + INSIST(qpi->sp == 0); + qpi->magic = 0; + return (ISC_R_NOMORE); + } + + /* push branch nodes onto the stack until we reach a leaf */ + for (;;) { + qp_node_t *n = ref_ptr(qp, qpi->stack[qpi->sp].ref); + if (node_tag(n) == LEAF_TAG) { + *pval_r = leaf_pval(n); + *ival_r = leaf_ival(n); + break; + } + qpi->sp++; + INSIST(qpi->sp < DNS_QP_MAXKEY); + qpi->stack[qpi->sp].ref = branch_twigs_ref(n); + qpi->stack[qpi->sp].more = branch_twigs_size(n) - 1; + } + + /* pop the stack until we find a twig with a successor */ + while (qpi->sp > 0 && qpi->stack[qpi->sp].more == 0) { + qpi->sp--; + } + + /* move across to the next twig */ + if (qpi->stack[qpi->sp].more > 0) { + qpi->stack[qpi->sp].more--; + qpi->stack[qpi->sp].ref++; + } else { + INSIST(qpi->sp == 0); + qpi->stack[qpi->sp].ref = INVALID_REF; + } + return (ISC_R_SUCCESS); +} + /*********************************************************************** * * search @@ -1693,7 +1783,7 @@ dns_qp_getkey(dns_qpreadable_t qpr, const dns_qpkey_t search_key, dns_qpkey_t found_key; size_t found_keylen; qp_shift_t bit; - qp_node_t *n; + qp_node_t *n = NULL; REQUIRE(QP_VALID(qp)); REQUIRE(pval_r != NULL); @@ -1734,4 +1824,101 @@ dns_qp_getname(dns_qpreadable_t qpr, const dns_name_t *name, void **pval_r, return (dns_qp_getkey(qpr, key, keylen, pval_r, ival_r)); } +isc_result_t +dns_qp_findname_parent(dns_qpreadable_t qpr, const dns_name_t *name, + dns_qpfind_t options, void **pval_r, uint32_t *ival_r) { + dns_qpreader_t *qp = dns_qpreader(qpr); + dns_qpkey_t search, found; + size_t searchlen, foundlen; + size_t offset; + qp_shift_t bit; + qp_node_t *n = NULL, *twigs = NULL; + isc_result_t result; + unsigned int labels = 0; + struct offref { + uint32_t off; + qp_ref_t ref; + } label[DNS_NAME_MAXLABELS]; + + REQUIRE(QP_VALID(qp)); + REQUIRE(pval_r != NULL); + REQUIRE(ival_r != NULL); + + searchlen = dns_qpkey_fromname(search, name); + if ((options & DNS_QPFIND_NOEXACT) != 0) { + searchlen = qpkey_trim_label(search, searchlen, searchlen); + result = DNS_R_PARTIALMATCH; + } else { + result = ISC_R_SUCCESS; + } + + n = get_root(qp); + if (n == NULL) { + return (ISC_R_NOTFOUND); + } + + /* + * Like `dns_qp_insert()`, we must find a leaf. However, we don't make a + * second pass: instead, we keep track of any leaves with shorter keys + * that we discover along the way. (In general, qp-trie searches can be + * one-pass, by recording their traversal, or two-pass, for less stack + * memory usage.) + * + * A shorter key that can be a parent domain always has a leaf node at + * SHIFT_NOBYTE (indicating end of its key) where our search key has a + * normal character immediately after a label separator. Note 1: It is + * OK if `offset - 1` underflows: it will become SIZE_MAX, which is + * greater than `searchlen`, so `qpkey_bit()` will return SHIFT_NOBYTE, + * which is what we want when `offset == 0`. Note 2: Any SHIFT_NOBYTE + * twig is always `twigs[0]`. + */ + while (is_branch(n)) { + prefetch_twigs(qp, n); + twigs = branch_twigs_vector(qp, n); + offset = branch_key_offset(n); + bit = qpkey_bit(search, searchlen, offset); + if (bit != SHIFT_NOBYTE && branch_has_twig(n, SHIFT_NOBYTE) && + qpkey_bit(search, searchlen, offset - 1) == SHIFT_NOBYTE && + !is_branch(&twigs[0])) + { + label[labels].off = offset; + label[labels].ref = branch_twigs_ref(n); + labels++; + INSIST(labels <= DNS_NAME_MAXLABELS); + } + if (branch_has_twig(n, bit)) { + n = branch_twig_ptr(qp, n, bit); + } else if (labels == 0) { + /* any twig will do */ + n = &twigs[0]; + } else { + n = ref_ptr(qp, label[labels - 1].ref); + break; + } + } + + /* do the keys differ, and if so, where? */ + foundlen = leaf_qpkey(qp, n, found); + offset = qpkey_compare(search, searchlen, found, foundlen); + + if (offset == QPKEY_EQUAL || offset == foundlen) { + *pval_r = leaf_pval(n); + *ival_r = leaf_ival(n); + if (offset == QPKEY_EQUAL) { + return (result); + } else { + return (DNS_R_PARTIALMATCH); + } + } + while (labels-- > 0) { + if (offset > label[labels].off) { + n = ref_ptr(qp, label[labels].ref); + *pval_r = leaf_pval(n); + *ival_r = leaf_ival(n); + return (DNS_R_PARTIALMATCH); + } + } + return (ISC_R_NOTFOUND); +} + /**********************************************************************/ diff --git a/lib/dns/qp_p.h b/lib/dns/qp_p.h index 183335e4e4..90d05a20d8 100644 --- a/lib/dns/qp_p.h +++ b/lib/dns/qp_p.h @@ -379,11 +379,13 @@ ref_ptr(dns_qpreadable_t qpr, qp_ref_t ref) { */ #define QP_MAGIC ISC_MAGIC('t', 'r', 'i', 'e') +#define QPITER_MAGIC ISC_MAGIC('q', 'p', 'i', 't') #define QPMULTI_MAGIC ISC_MAGIC('q', 'p', 'm', 'v') #define QPREADER_MAGIC ISC_MAGIC('q', 'p', 'r', 'x') -#define QP_VALID(qp) ISC_MAGIC_VALID(qp, QP_MAGIC) -#define QPMULTI_VALID(qp) ISC_MAGIC_VALID(qp, QPMULTI_MAGIC) +#define QP_VALID(p) ISC_MAGIC_VALID(p, QP_MAGIC) +#define QPITER_VALID(p) ISC_MAGIC_VALID(p, QPITER_MAGIC) +#define QPMULTI_VALID(p) ISC_MAGIC_VALID(p, QPMULTI_MAGIC) /* * Polymorphic initialization of the `dns_qpreader_t` prefix. @@ -892,34 +894,6 @@ unpack_reader(dns_qpreader_t *qp, qp_node_t *reader) { * method invocation helpers */ -#if 0 - -#define attach_leaf(qp, n) \ - do { \ - uint32_t iv = leaf_ival(n); \ - void *pv = leaf_pval(n); \ - uint32_t r = qp->methods->attach(qp->uctx, pv, iv); \ - fprintf(stderr, \ - "%s:%u:%s():t%u qp %p node %p leaf %p %u " \ - "(%u -> %u)\n", \ - __FILE__, __LINE__, __func__, isc_tid(), qp, n, pv, \ - iv, r, r + 1); \ - } while (0) - -#define detach_leaf(qp, n) \ - do { \ - uint32_t iv = leaf_ival(n); \ - void *pv = leaf_pval(n); \ - uint32_t r = qp->methods->detach(qp->uctx, pv, iv); \ - fprintf(stderr, \ - "%s:%u:%s():t%u qp %p node %p leaf %p %u " \ - "(%u -> %u)\n", \ - __FILE__, __LINE__, __func__, isc_tid(), qp, n, pv, \ - iv, r, r - 1); \ - } while (0) - -#else - static inline void attach_leaf(dns_qpreadable_t qpr, qp_node_t *n) { dns_qpreader_t *qp = dns_qpreader(qpr); @@ -932,8 +906,6 @@ detach_leaf(dns_qpreadable_t qpr, qp_node_t *n) { qp->methods->detach(qp->uctx, leaf_pval(n), leaf_ival(n)); } -#endif - static inline size_t leaf_qpkey(dns_qpreadable_t qpr, qp_node_t *n, dns_qpkey_t key) { dns_qpreader_t *qp = dns_qpreader(qpr); 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/bench/load-names.c b/tests/bench/load-names.c index a1473d8694..babdd37ef1 100644 --- a/tests/bench/load-names.c +++ b/tests/bench/load-names.c @@ -37,11 +37,10 @@ struct { dns_fixedname_t fixed; } item[1024 * 1024]; -static uint32_t +static void item_check(void *ctx, void *pval, uint32_t ival) { UNUSED(ctx); assert(pval == &item[ival]); - return (1); } static size_t @@ -57,7 +56,7 @@ testname(void *ctx, char *buf, size_t size) { strlcpy(buf, "test", size); } -const struct dns_qpmethods qpmethods = { +const dns_qpmethods_t qpmethods = { item_check, item_check, item_makekey, @@ -164,7 +163,7 @@ add_qp(void *qp, size_t count) { static void sqz_qp(void *qp) { - dns_qp_compact(qp, true); + dns_qp_compact(qp, DNS_QPGC_ALL); } static isc_result_t @@ -201,7 +200,7 @@ static struct fun { #define FILE_CHECK(check, msg) \ do { \ if (!(check)) { \ - fprintf(stderr, "%s:%zu: %s\n", filename, count, msg); \ + fprintf(stderr, "%s:%zu: %s\n", filename, lines, msg); \ exit(1); \ } \ } while (0) @@ -209,6 +208,12 @@ static struct fun { int main(int argc, char *argv[]) { isc_result_t result; + const char *filename = NULL; + char *filetext = NULL; + off_t fileoff; + FILE *fp = NULL; + size_t filesize, lines = 0, wirebytes = 0, labels = 0; + char *pos = NULL, *file_end = NULL; isc_mem_create(&mctx); @@ -217,18 +222,17 @@ main(int argc, char *argv[]) { exit(1); } - const char *filename = argv[1]; - off_t fileoff; + filename = argv[1]; result = isc_file_getsize(filename, &fileoff); if (result != ISC_R_SUCCESS) { fprintf(stderr, "stat(%s): %s\n", filename, isc_result_totext(result)); exit(1); } - size_t filesize = (size_t)fileoff; + filesize = (size_t)fileoff; - char *filetext = isc_mem_get(mctx, filesize + 1); - FILE *fp = fopen(filename, "r"); + filetext = isc_mem_get(mctx, filesize + 1); + fp = fopen(filename, "r"); if (fp == NULL || fread(filetext, 1, filesize, fp) < filesize) { fprintf(stderr, "read(%s): %s\n", filename, strerror(errno)); exit(1); @@ -236,29 +240,28 @@ main(int argc, char *argv[]) { fclose(fp); filetext[filesize] = '\0'; - size_t count = 0; - size_t wirebytes = 0; - size_t labels = 0; - - char *pos = filetext; - char *file_end = pos + filesize; + pos = filetext; + file_end = pos + filesize; while (pos < file_end) { - FILE_CHECK(count < ARRAY_SIZE(item), "too many lines"); + char *domain = NULL, *newline = NULL; + size_t len; + + FILE_CHECK(lines < ARRAY_SIZE(item), "too many lines"); pos += strspn(pos, "0123456789"); FILE_CHECK(*pos++ == ',', "missing comma"); - char *domain = pos; + domain = pos; pos += strcspn(pos, "\r\n"); FILE_CHECK(*pos != '\0', "missing newline"); - char *newline = pos; + newline = pos; pos += strspn(pos, "\r\n"); - size_t len = newline - domain; + len = newline - domain; - item[count].text = domain; + item[lines].text = domain; domain[len] = '\0'; - dns_name_t *name = dns_fixedname_initname(&item[count].fixed); + dns_name_t *name = dns_fixedname_initname(&item[lines].fixed); isc_buffer_t buffer; isc_buffer_init(&buffer, domain, len); isc_buffer_add(&buffer, len); @@ -268,41 +271,35 @@ main(int argc, char *argv[]) { wirebytes += name->length; labels += name->labels; - count++; + lines++; } printf("names %g MB labels %g MB\n", (double)wirebytes / 1048576.0, (double)labels / 1048576.0); - size_t lines = count; - for (struct fun *fun = fun_list; fun->name != NULL; fun++) { - isc_time_t t0; - t0 = isc_time_now_hires(); - isc_mem_t *mem = NULL; - isc_mem_create(&mem); - void *map = fun->new (mem); + void *map = NULL; - for (count = 0; count < lines; count++) { - result = fun->add(map, count); + isc_mem_create(&mem); + map = fun->new (mem); + + isc_time_t t0 = isc_time_now_hires(); + for (size_t n = 0; n < lines; n++) { + result = fun->add(map, n); CHECK(result); } fun->sqz(map); - isc_time_t t1; - t1 = isc_time_now_hires(); - - for (count = 0; count < lines; count++) { + isc_time_t t1 = isc_time_now_hires(); + for (size_t n = 0; n < lines; n++) { void *pval = NULL; - result = fun->get(map, count, &pval); + result = fun->get(map, n, &pval); CHECK(result); - assert(pval == &item[count]); + assert(pval == &item[n]); } - isc_time_t t2; - t2 = isc_time_now_hires(); - + isc_time_t t2 = isc_time_now_hires(); printf("%f sec to load %s\n", (double)isc_time_microdiff(&t1, &t0) / (1000.0 * 1000.0), fun->name); diff --git a/tests/bench/qp-dump.c b/tests/bench/qp-dump.c index 515048ea97..855bbbc3dc 100644 --- a/tests/bench/qp-dump.c +++ b/tests/bench/qp-dump.c @@ -95,19 +95,17 @@ qpkey_from_smallname(dns_qpkey_t key, void *ctx, void *pval, uint32_t ival) { return (dns_qpkey_fromname(key, &name)); } -static uint32_t +static void smallname_attach(void *ctx, void *pval, uint32_t ival) { UNUSED(ctx); - return (isc_refcount_increment0(smallname_refcount(pval, ival))); + isc_refcount_increment0(smallname_refcount(pval, ival)); } -static uint32_t +static void smallname_detach(void *ctx, void *pval, uint32_t ival) { - uint32_t refs = isc_refcount_decrement(smallname_refcount(pval, ival)); - if (refs == 1) { + if (isc_refcount_decrement(smallname_refcount(pval, ival)) == 1) { isc_mem_free(ctx, pval); } - return (refs); } static void @@ -116,7 +114,7 @@ testname(void *ctx, char *buf, size_t size) { strlcpy(buf, "test", size); } -const struct dns_qpmethods methods = { +const dns_qpmethods_t methods = { smallname_attach, smallname_detach, qpkey_from_smallname, @@ -126,15 +124,23 @@ const struct dns_qpmethods methods = { static void usage(void) { fprintf(stderr, - "usage: qp_dump [-drt] \n" + "usage: qp_dump [-dt] \n" " -d output in graphviz dot format\n" " -t output in ad-hoc indented text format\n"); } int main(int argc, char *argv[]) { - bool dumpdot = false; - bool dumptxt = false; + isc_result_t result; + dns_qp_t *qp = NULL; + const char *filename = NULL; + char *filetext = NULL; + size_t filesize; + off_t fileoff; + FILE *fp = NULL; + size_t wirebytes = 0, labels = 0, names = 0; + char *pos = NULL, *file_end = NULL; + bool dumpdot = false, dumptxt = false; int opt; while ((opt = isc_commandline_parse(argc, argv, "dt")) != -1) { @@ -162,18 +168,17 @@ main(int argc, char *argv[]) { isc_mem_create(&mctx); - const char *filename = argv[0]; - off_t fileoff; - isc_result_t result = isc_file_getsize(filename, &fileoff); + filename = argv[0]; + result = isc_file_getsize(filename, &fileoff); if (result != ISC_R_SUCCESS) { fprintf(stderr, "stat(%s): %s\n", filename, isc_result_totext(result)); exit(1); } - size_t filesize = (size_t)fileoff; - char *filetext = isc_mem_get(mctx, filesize + 1); - FILE *fp = fopen(filename, "r"); + filesize = (size_t)fileoff; + filetext = isc_mem_get(mctx, filesize + 1); + fp = fopen(filename, "r"); if (fp == NULL || fread(filetext, 1, filesize, fp) < filesize) { fprintf(stderr, "read(%s): %s\n", filename, strerror(errno)); exit(1); @@ -181,31 +186,30 @@ main(int argc, char *argv[]) { fclose(fp); filetext[filesize] = '\0'; - dns_qp_t *qp = NULL; dns_qp_create(mctx, &methods, NULL, &qp); - size_t wirebytes = 0; - size_t labels = 0; - size_t names = 0; - char *pos = filetext; - char *file_end = pos + filesize; + pos = filetext; + file_end = pos + filesize; while (pos < file_end) { - char *domain = pos; - pos += strcspn(pos, "\r\n"); - char *newline = pos; - pos += strspn(pos, "\r\n"); - size_t len = newline - domain; - domain[len] = '\0'; - + void *pval = NULL; + uint32_t ival = 0; dns_fixedname_t fixed; dns_name_t *name = dns_fixedname_initname(&fixed); isc_buffer_t buffer; + char *newline = NULL, *domain = pos; + size_t len; + + pos += strcspn(pos, "\r\n"); + newline = pos; + pos += strspn(pos, "\r\n"); + + len = newline - domain; + domain[len] = '\0'; + isc_buffer_init(&buffer, domain, len); isc_buffer_add(&buffer, len); result = dns_name_fromtext(name, &buffer, dns_rootname, 0, NULL); - void *pval = NULL; - uint32_t ival = 0; if (result == ISC_R_SUCCESS) { smallname_from_name(name, &pval, &ival); result = dns_qp_insert(qp, pval, ival); @@ -224,17 +228,18 @@ main(int argc, char *argv[]) { labels += name->labels; names += 1; } - dns_qp_compact(qp, true); - - size_t smallbytes = wirebytes + labels + names * sizeof(isc_refcount_t); - dns_qp_memusage_t memusage = dns_qp_memusage(qp); - uint64_t compaction_us, recovery_us, rollback_us; - dns_qp_gctime(&compaction_us, &recovery_us, &rollback_us); + dns_qp_compact(qp, DNS_QPGC_ALL); #define print_megabytes(label, value) \ printf("%6.2f MiB - " label "\n", (double)(value) / 1048576.0) if (!dumptxt && !dumpdot) { + size_t smallbytes = wirebytes + labels + + names * sizeof(isc_refcount_t); + dns_qp_memusage_t memusage = dns_qp_memusage(qp); + uint64_t compaction_us, recovery_us, rollback_us; + dns_qp_gctime(&compaction_us, &recovery_us, &rollback_us); + printf("leaves %zu\n" " nodes %zu\n" " used %zu\n" @@ -264,10 +269,12 @@ main(int argc, char *argv[]) { printf("%6zu - max key len\n", qp_test_maxkeylen(qp)); } - if (dumptxt) + if (dumptxt) { qp_test_dumptrie(qp); - if (dumpdot) + } + if (dumpdot) { qp_test_dumpdot(qp); + } return (0); } diff --git a/tests/bench/qpmulti.c b/tests/bench/qpmulti.c index 59c9a11a3e..7034ac983f 100644 --- a/tests/bench/qpmulti.c +++ b/tests/bench/qpmulti.c @@ -89,12 +89,11 @@ static struct { dns_qpkey_t key; } *item; -static uint32_t +static void item_refcount(void *ctx, void *pval, uint32_t ival) { UNUSED(ctx); UNUSED(pval); UNUSED(ival); - return (1); } static size_t @@ -111,7 +110,7 @@ benchname(void *ctx, char *buf, size_t size) { strlcpy(buf, "bench", size); } -const struct dns_qpmethods item_methods = { +const dns_qpmethods_t item_methods = { item_refcount, item_refcount, item_makekey, @@ -128,13 +127,12 @@ init_items(isc_mem_t *mctx) { void *pval = NULL; uint32_t ival = ~0U; dns_qp_t *qp = NULL; - size_t bytes = ITEM_COUNT * sizeof(*item); + uint64_t start; + start = isc_time_monotonic(); item = isc_mem_allocatex(mctx, bytes, ISC_MEM_ZERO); - uint64_t start = isc_time_monotonic(); - /* ensure there are no duplicate names */ dns_qp_create(mctx, &item_methods, NULL, &qp); for (size_t i = 0; i < ITEM_COUNT; i++) { @@ -224,6 +222,7 @@ first_loop(void *varg) { static void next_loop(struct thread_args *args, isc_nanosecs_t start) { isc_nanosecs_t stop = isc_time_monotonic(); + args->worked += stop - start; args->stop = stop; if (args->stop - args->start < RUNTIME) { @@ -238,6 +237,9 @@ next_loop(struct thread_args *args, isc_nanosecs_t start) { static void read_zipf(uv_idle_t *idle) { struct thread_args *args = idle->data; + isc_nanosecs_t start; + void *pval = NULL; + uint32_t ival; /* outside time because it is v slow */ uint32_t r[args->tx_per_loop][args->ops_per_tx]; @@ -247,10 +249,7 @@ read_zipf(uv_idle_t *idle) { } } - isc_nanosecs_t start = isc_time_monotonic(); - void *pval; - uint32_t ival; - + start = isc_time_monotonic(); for (uint32_t tx = 0; tx < args->tx_per_loop; tx++) { args->transactions++; dns_qpread_t qp; @@ -277,7 +276,7 @@ static void read_transactions(uv_idle_t *idle) { struct thread_args *args = idle->data; isc_nanosecs_t start = isc_time_monotonic(); - void *pval; + void *pval = NULL; uint32_t ival; for (uint32_t tx = 0; tx < args->tx_per_loop; tx++) { @@ -323,8 +322,12 @@ mutate_transactions(uv_idle_t *idle) { args->absent++; } } + /* + * We would normally use DNS_QPGC_MAYBE, but here we do the + * fragmented check ourself so we can count compactions + */ if (dns_qp_memusage(qp).fragmented) { - dns_qp_compact(qp, false); + dns_qp_compact(qp, DNS_QPGC_NOW); args->compactions++; } dns_qpmulti_commit(args->multi, &qp); @@ -375,13 +378,13 @@ static void load_multi(struct bench_state *bctx) { dns_qp_t *qp = NULL; size_t count = 0; - - uint64_t start = isc_time_monotonic(); + uint64_t start; dns_qpmulti_create(bctx->mctx, bctx->loopmgr, &item_methods, NULL, &bctx->multi); /* initial contents of the trie */ + start = isc_time_monotonic(); dns_qpmulti_update(bctx->multi, &qp); for (size_t i = 0; i < bctx->max_item; i++) { if (isc_random_uniform(2) == 0) { @@ -392,7 +395,7 @@ load_multi(struct bench_state *bctx) { item[i].present = true; count++; } - dns_qp_compact(qp, true); + dns_qp_compact(qp, DNS_QPGC_ALL); dns_qpmulti_commit(bctx->multi, &qp); bctx->load_time = isc_time_monotonic() - start; @@ -741,11 +744,18 @@ dispatch(struct bench_state *bctx) { static void collect(void *varg) { - TRACE(""); - struct thread_args *args = varg; struct bench_state *bctx = args->bctx; struct thread_args *thread = bctx->thread; + struct { + uint64_t worked, txns, ops, compactions; + } stats[2] = {}; + double load_time = bctx->load_time; + double elapsed = 0, mut_work, readers, read_work, elapsed_ms; + uint32_t nloops; + bool zipf; + + TRACE("collect"); bctx->waiting--; if (bctx->waiting > 0) { @@ -753,20 +763,15 @@ collect(void *varg) { } isc_barrier_destroy(&bctx->barrier); - struct { - uint64_t worked, txns, ops, compactions; - } stats[2] = {}; - - double load_time = bctx->load_time; load_time = load_time > 0 ? load_time / (double)NS_PER_SEC : NAN; - double elapsed = 0; - bool zipf = bctx->mutate == 0 && bctx->readers == 0; - uint32_t nloops = zipf ? bctx->nloops : bctx->readers + bctx->mutate; + zipf = bctx->mutate == 0 && bctx->readers == 0; + nloops = zipf ? bctx->nloops : bctx->readers + bctx->mutate; for (uint32_t t = 0; t < nloops; t++) { struct thread_args *tp = &thread[t]; elapsed = ISC_MAX(elapsed, (tp->stop - tp->start)); bool mut = t < bctx->mutate; + stats[mut].worked += tp->worked; stats[mut].txns += tp->transactions; stats[mut].ops += tp->transactions * tp->ops_per_tx; @@ -779,7 +784,7 @@ collect(void *varg) { printf("%7.2f\t", (double)bctx->qp_bytes / bctx->qp_items); printf("%7u\t", bctx->max_item); - double mut_work = stats[1].worked / (double)US_PER_MS; + mut_work = stats[1].worked / (double)US_PER_MS; printf("%7u\t", bctx->mutate); printf("%7u\t", bctx->mut_tx_per_loop); printf("%7u\t", bctx->mut_ops_per_tx); @@ -790,9 +795,9 @@ collect(void *varg) { printf("%7.2f\t", stats[1].txns / mut_work); printf("%7.2f\t", stats[1].ops / mut_work); - double readers = zipf ? bctx->nloops - bctx->mutate : bctx->readers; - double read_work = stats[0].worked / (double)US_PER_MS; - double elapsed_ms = elapsed / (double)US_PER_MS; + readers = zipf ? bctx->nloops - bctx->mutate : bctx->readers; + read_work = stats[0].worked / (double)US_PER_MS; + elapsed_ms = elapsed / (double)US_PER_MS; printf("%7u\t", bctx->readers); printf("%7u\t", bctx->read_tx_per_loop); printf("%7u\t", bctx->read_ops_per_tx); @@ -813,10 +818,8 @@ startup(void *arg) { isc_loop_t *loop = isc_loop_current(loopmgr); isc_mem_t *mctx = isc_loop_getmctx(loop); uint32_t nloops = isc_loopmgr_nloops(loopmgr); - size_t bytes = sizeof(struct bench_state) + sizeof(struct thread_args) * nloops; - struct bench_state *bctx = isc_mem_getx(mctx, bytes, ISC_MEM_ZERO); *bctx = (struct bench_state){ @@ -881,9 +884,9 @@ int main(void) { isc_loopmgr_t *loopmgr = NULL; isc_mem_t *mctx = NULL; - uint32_t nloops; const char *env_workers = getenv("ISC_TASK_WORKERS"); + if (env_workers != NULL) { nloops = atoi(env_workers); } else { diff --git a/tests/dns/qp_test.c b/tests/dns/qp_test.c index aceaa344f8..ee9b30da00 100644 --- a/tests/dns/qp_test.c +++ b/tests/dns/qp_test.c @@ -22,6 +22,9 @@ #define UNIT_TESTING #include +#include +#include +#include #include #include #include @@ -29,6 +32,8 @@ #include #include +#include "qp_p.h" + #include #include @@ -129,9 +134,230 @@ ISC_RUN_TEST_IMPL(qpkey_sort) { } } +#define ITER_ITEMS 100 + +static void +check_leaf(void *uctx, void *pval, uint32_t ival) { + uint32_t *items = uctx; + assert_in_range(ival, 1, ITER_ITEMS - 1); + assert_ptr_equal(items + ival, pval); +} + +static size_t +qpiter_makekey(dns_qpkey_t key, void *uctx, void *pval, uint32_t ival) { + check_leaf(uctx, pval, ival); + + char str[8]; + snprintf(str, sizeof(str), "%03u", ival); + + size_t i = 0; + while (str[i] != '\0') { + key[i] = str[i] - '0' + SHIFT_BITMAP; + i++; + } + key[i++] = SHIFT_NOBYTE; + + return (i); +} + +static void +getname(void *uctx, char *buf, size_t size) { + strlcpy(buf, "test", size); + UNUSED(uctx); + UNUSED(size); +} + +const dns_qpmethods_t qpiter_methods = { + check_leaf, + check_leaf, + qpiter_makekey, + getname, +}; + +ISC_RUN_TEST_IMPL(qpiter) { + dns_qp_t *qp = NULL; + uint32_t item[ITER_ITEMS] = { 0 }; + + dns_qp_create(mctx, &qpiter_methods, item, &qp); + for (size_t tests = 0; tests < 1234; tests++) { + uint32_t ival = isc_random_uniform(ITER_ITEMS - 1) + 1; + void *pval = &item[ival]; + item[ival] = ival; + + /* randomly insert or remove */ + dns_qpkey_t key; + size_t len = qpiter_makekey(key, item, pval, ival); + if (dns_qp_insert(qp, pval, ival) == ISC_R_EXISTS) { + dns_qp_deletekey(qp, key, len); + item[ival] = 0; + } + + /* check that we see only valid items in the correct order */ + uint32_t prev = 0; + dns_qpiter_t qpi; + dns_qpiter_init(qp, &qpi); + while (dns_qpiter_next(&qpi, &pval, &ival) == ISC_R_SUCCESS) { + assert_in_range(ival, prev + 1, ITER_ITEMS - 1); + assert_int_equal(ival, item[ival]); + assert_ptr_equal(pval, &item[ival]); + item[ival] = ~ival; + prev = ival; + } + + /* ensure we saw every item */ + for (ival = 0; ival < ITER_ITEMS; ival++) { + if (item[ival] != 0) { + assert_int_equal(item[ival], ~ival); + item[ival] = ival; + } + } + } + dns_qp_destroy(&qp); +} + +static void +no_op(void *uctx, void *pval, uint32_t ival) { + UNUSED(uctx); + UNUSED(pval); + UNUSED(ival); +} + +static size_t +qpkey_fromstring(dns_qpkey_t key, void *uctx, void *pval, uint32_t ival) { + dns_fixedname_t fixed; + + UNUSED(uctx); + UNUSED(ival); + if (*(char *)pval == '\0') { + return (0); + } + dns_test_namefromstring(pval, &fixed); + return (dns_qpkey_fromname(key, dns_fixedname_name(&fixed))); +} + +const dns_qpmethods_t string_methods = { + no_op, + no_op, + qpkey_fromstring, + getname, +}; + +struct check_partialmatch { + const char *query; + dns_qpfind_t options; + isc_result_t result; + const char *found; +}; + +static void +check_partialmatch(dns_qp_t *qp, struct check_partialmatch check[]) { + for (int i = 0; check[i].query != NULL; i++) { + isc_result_t result; + dns_fixedname_t fixed; + dns_name_t *name = dns_fixedname_name(&fixed); + void *pval = NULL; + uint32_t ival; + +#if 0 + fprintf(stderr, "%s %u %s %s\n", check[i].query, + check[i].options, isc_result_totext(check[i].result), + check[i].found); +#endif + dns_test_namefromstring(check[i].query, &fixed); + result = dns_qp_findname_parent(qp, name, check[i].options, + &pval, &ival); + assert_int_equal(result, check[i].result); + if (check[i].found == NULL) { + assert_null(pval); + } else { + assert_string_equal(pval, check[i].found); + } + } +} + +static void +insert_str(dns_qp_t *qp, const char *str) { + isc_result_t result; + uintptr_t pval = (uintptr_t)str; + INSIST((pval & 3) == 0); + result = dns_qp_insert(qp, (void *)pval, 0); + assert_int_equal(result, ISC_R_SUCCESS); +} + +ISC_RUN_TEST_IMPL(partialmatch) { + isc_result_t result; + dns_qp_t *qp = NULL; + + dns_qp_create(mctx, &string_methods, NULL, &qp); + + /* + * Fixed size strings [16] should ensure leaf-compatible alignment. + */ + const char insert[][16] = { + "a.b.", "b.", "fo.bar.", "foo.bar.", + "fooo.bar.", "web.foo.bar.", ".", "", + }; + + int i = 0; + while (insert[i][0] != '.') { + insert_str(qp, insert[i++]); + } + + static struct check_partialmatch check1[] = { + { "a.b.", 0, ISC_R_SUCCESS, "a.b." }, + { "a.b.", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH, "b." }, + { "b.c.", DNS_QPFIND_NOEXACT, ISC_R_NOTFOUND, NULL }, + { "bar.", 0, ISC_R_NOTFOUND, NULL }, + { "f.bar.", 0, ISC_R_NOTFOUND, NULL }, + { "foo.bar.", 0, ISC_R_SUCCESS, "foo.bar." }, + { "foo.bar.", DNS_QPFIND_NOEXACT, ISC_R_NOTFOUND, NULL }, + { "foooo.bar.", 0, ISC_R_NOTFOUND, NULL }, + { "w.foo.bar.", 0, DNS_R_PARTIALMATCH, "foo.bar." }, + { "www.foo.bar.", 0, DNS_R_PARTIALMATCH, "foo.bar." }, + { "web.foo.bar.", 0, ISC_R_SUCCESS, "web.foo.bar." }, + { "webby.foo.bar.", 0, DNS_R_PARTIALMATCH, "foo.bar." }, + { "my.web.foo.bar.", 0, DNS_R_PARTIALMATCH, "web.foo.bar." }, + { "web.foo.bar.", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH, + "foo.bar." }, + { "my.web.foo.bar.", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH, + "web.foo.bar." }, + { NULL, 0, 0, NULL }, + }; + check_partialmatch(qp, check1); + + /* what if the trie contains the root? */ + INSIST(insert[i][0] == '.'); + insert_str(qp, insert[i++]); + + static struct check_partialmatch check2[] = { + { "b.c.", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH, "." }, + { "bar.", 0, DNS_R_PARTIALMATCH, "." }, + { "foo.bar.", 0, ISC_R_SUCCESS, "foo.bar." }, + { "foo.bar.", DNS_QPFIND_NOEXACT, DNS_R_PARTIALMATCH, "." }, + { NULL, 0, 0, NULL }, + }; + check_partialmatch(qp, check2); + + /* what if entries in the trie are relative to the zone apex? */ + dns_qpkey_t rootkey = { SHIFT_NOBYTE }; + result = dns_qp_deletekey(qp, rootkey, 1); + assert_int_equal(result, ISC_R_SUCCESS); + INSIST(insert[i][0] == '\0'); + insert_str(qp, insert[i++]); + check_partialmatch(qp, (struct check_partialmatch[]){ + { "bar", 0, DNS_R_PARTIALMATCH, "" }, + { "bar.", 0, DNS_R_PARTIALMATCH, "" }, + { NULL, 0, 0, NULL }, + }); + + dns_qp_destroy(&qp); +} + ISC_TEST_LIST_START ISC_TEST_ENTRY(qpkey_name) ISC_TEST_ENTRY(qpkey_sort) +ISC_TEST_ENTRY(qpiter) +ISC_TEST_ENTRY(partialmatch) ISC_TEST_LIST_END ISC_TEST_MAIN diff --git a/tests/dns/qpmulti_test.c b/tests/dns/qpmulti_test.c index 28605080e0..16e5350dec 100644 --- a/tests/dns/qpmulti_test.c +++ b/tests/dns/qpmulti_test.c @@ -104,19 +104,19 @@ static struct { dns_qpkey_t ascii; } item[ITEM_COUNT]; -static uint32_t +static void item_attach(void *ctx, void *pval, uint32_t ival) { - assert_null(ctx); - assert_ptr_equal(pval, &item[ival]); - return (item[ival].refcount++); + INSIST(ctx == NULL); + INSIST(pval == &item[ival]); + item[ival].refcount++; } -static uint32_t +static void item_detach(void *ctx, void *pval, uint32_t ival) { assert_null(ctx); assert_ptr_equal(pval, &item[ival]); assert_int_not_equal(item[ival].refcount, 0); - return (item[ival].refcount--); + item[ival].refcount--; } static size_t @@ -144,7 +144,7 @@ testname(void *ctx, char *buf, size_t size) { strlcpy(buf, "test", size); } -const struct dns_qpmethods test_methods = { +const dns_qpmethods_t test_methods = { item_attach, item_detach, item_makekey, 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); }