From 2cb5a6210f334374c601f408c6e7c7ec75ee0949 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 14 Nov 2024 19:51:29 +0100 Subject: [PATCH] Improve the badcache cleaning by adding LRU and using RCU Instead of cleaning the dns_badcache opportunistically, add per-loop LRU, so each thread-loop can clean the expired entries. This also allows removal of the atomic operations as the badcache entries are now immutable, instead of updating the badcache entry in place, the old entry is now deleted from the hashtable and the LRU list, and the new entry is inserted in the LRU. --- bin/delv/delv.c | 4 +- bin/named/server.c | 5 +- bin/tests/system/pipelined/pipequeries.c | 2 +- bin/tools/mdig.c | 2 +- fuzz/dns_message_checksig.c | 3 +- lib/dns/badcache.c | 289 +++++++++++++---------- lib/dns/client.c | 6 +- lib/dns/include/dns/badcache.h | 14 +- lib/dns/include/dns/view.h | 5 +- lib/dns/resolver.c | 13 +- lib/dns/view.c | 8 +- lib/ns/client.c | 2 +- tests/dns/badcache_test.c | 104 +++++--- tests/dns/dnstap_test.c | 35 ++- tests/libtest/dns.c | 4 +- tests/ns/query_test.c | 3 +- 16 files changed, 291 insertions(+), 208 deletions(-) diff --git a/bin/delv/delv.c b/bin/delv/delv.c index e6371bc5fd..d1ba814be1 100644 --- a/bin/delv/delv.c +++ b/bin/delv/delv.c @@ -2189,8 +2189,8 @@ run_server(void *arg) { CHECK(ns_interfacemgr_create(mctx, sctx, loopmgr, netmgr, dispatchmgr, NULL, &interfacemgr)); - CHECK(dns_view_create(mctx, dispatchmgr, dns_rdataclass_in, "_default", - &view)); + CHECK(dns_view_create(mctx, loopmgr, dispatchmgr, dns_rdataclass_in, + "_default", &view)); CHECK(dns_cache_create(loopmgr, dns_rdataclass_in, "", mctx, &cache)); dns_view_setcache(view, cache, false); dns_cache_detach(&cache); diff --git a/bin/named/server.c b/bin/named/server.c index 72cc3669e8..ac05082489 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -6258,8 +6258,9 @@ create_view(const cfg_obj_t *vconfig, dns_viewlist_t *viewlist, } INSIST(view == NULL); - result = dns_view_create(named_g_mctx, named_g_dispatchmgr, viewclass, - viewname, &view); + result = dns_view_create(named_g_mctx, named_g_loopmgr, + named_g_dispatchmgr, viewclass, viewname, + &view); if (result != ISC_R_SUCCESS) { return result; } diff --git a/bin/tests/system/pipelined/pipequeries.c b/bin/tests/system/pipelined/pipequeries.c index 617890f9d5..f1474dd1e3 100644 --- a/bin/tests/system/pipelined/pipequeries.c +++ b/bin/tests/system/pipelined/pipequeries.c @@ -278,7 +278,7 @@ main(int argc, char *argv[]) { RUNCHECK(dns_requestmgr_create(mctx, loopmgr, dispatchmgr, dispatchv4, NULL, &requestmgr)); - RUNCHECK(dns_view_create(mctx, NULL, 0, "_test", &view)); + RUNCHECK(dns_view_create(mctx, loopmgr, NULL, 0, "_test", &view)); isc_loopmgr_setup(loopmgr, sendqueries, NULL); isc_loopmgr_teardown(loopmgr, teardown_view, view); diff --git a/bin/tools/mdig.c b/bin/tools/mdig.c index c9270fba25..600ab78be6 100644 --- a/bin/tools/mdig.c +++ b/bin/tools/mdig.c @@ -2103,7 +2103,7 @@ setup(void *arg ISC_ATTR_UNUSED) { mctx, loopmgr, dispatchmgr, have_ipv4 ? dispatchvx : NULL, have_ipv6 ? dispatchvx : NULL, &requestmgr)); - RUNCHECK(dns_view_create(mctx, NULL, 0, "_mdig", &view)); + RUNCHECK(dns_view_create(mctx, loopmgr, NULL, 0, "_mdig", &view)); } /*% Main processing routine for mdig */ diff --git a/fuzz/dns_message_checksig.c b/fuzz/dns_message_checksig.c index db9a41da55..99cdfe0e2a 100644 --- a/fuzz/dns_message_checksig.c +++ b/fuzz/dns_message_checksig.c @@ -173,7 +173,8 @@ LLVMFuzzerInitialize(int *argc ISC_ATTR_UNUSED, char ***argv ISC_ATTR_UNUSED) { isc_loopmgr_create(mctx, 1, &loopmgr); - result = dns_view_create(mctx, NULL, dns_rdataclass_in, "view", &view); + result = dns_view_create(mctx, loopmgr, NULL, dns_rdataclass_in, "view", + &view); if (result != ISC_R_SUCCESS) { fprintf(stderr, "dns_view_create failed: %s\n", isc_result_totext(result)); diff --git a/lib/dns/badcache.c b/lib/dns/badcache.c index 6bffa41ba5..427188b815 100644 --- a/lib/dns/badcache.c +++ b/lib/dns/badcache.c @@ -16,13 +16,13 @@ #include #include -#include +#include #include #include #include +#include #include #include -#include #include #include #include @@ -40,11 +40,17 @@ typedef struct dns_bcentry dns_bcentry_t; +typedef struct dns_bckey { + const dns_name_t *name; + dns_rdatatype_t type; +} dns__bckey_t; + struct dns_badcache { unsigned int magic; isc_mem_t *mctx; struct cds_lfht *ht; - atomic_bool purge_in_progress; + struct cds_list_head *lru; + uint32_t nloops; }; #define BADCACHE_MAGIC ISC_MAGIC('B', 'd', 'C', 'a') @@ -54,15 +60,16 @@ struct dns_badcache { #define BADCACHE_MIN_SIZE (1 << 8) /* Must be power of 2 */ struct dns_bcentry { - isc_mem_t *mctx; - dns_rdatatype_t type; - _Atomic(isc_stdtime_t) expire; - atomic_uint_fast32_t flags; - dns_fixedname_t fname; - dns_name_t *name; + isc_loop_t *loop; + isc_stdtime_t expire; + uint32_t flags; struct cds_lfht_node ht_node; struct rcu_head rcu_head; + struct cds_list_head lru_head; + + dns_name_t name; + dns_rdatatype_t type; }; static void @@ -71,19 +78,29 @@ bcentry_print(dns_bcentry_t *bad, isc_stdtime_t now, FILE *fp); static void bcentry_destroy(struct rcu_head *rcu_head); -dns_badcache_t * -dns_badcache_new(isc_mem_t *mctx) { - REQUIRE(mctx != NULL); +static bool +bcentry_alive(struct cds_lfht *ht, dns_bcentry_t *bad, isc_stdtime_t now); +dns_badcache_t * +dns_badcache_new(isc_mem_t *mctx, isc_loopmgr_t *loopmgr) { + REQUIRE(loopmgr != NULL); + + uint32_t nloops = isc_loopmgr_nloops(loopmgr); dns_badcache_t *bc = isc_mem_get(mctx, sizeof(*bc)); *bc = (dns_badcache_t){ .magic = BADCACHE_MAGIC, + .nloops = nloops, }; bc->ht = cds_lfht_new(BADCACHE_INIT_SIZE, BADCACHE_MIN_SIZE, 0, CDS_LFHT_AUTO_RESIZE | CDS_LFHT_ACCOUNTING, NULL); INSIST(bc->ht != NULL); + bc->lru = isc_mem_cget(mctx, bc->nloops, sizeof(bc->lru[0])); + for (size_t i = 0; i < bc->nloops; i++) { + CDS_INIT_LIST_HEAD(&bc->lru[i]); + } + isc_mem_attach(mctx, &bc->mctx); return bc; @@ -106,31 +123,55 @@ dns_badcache_destroy(dns_badcache_t **bcp) { } RUNTIME_CHECK(!cds_lfht_destroy(bc->ht, NULL)); + isc_mem_cput(bc->mctx, bc->lru, bc->nloops, sizeof(bc->lru[0])); + isc_mem_putanddetach(&bc->mctx, bc, sizeof(dns_badcache_t)); } static int -bcentry_match(struct cds_lfht_node *ht_node, const void *key) { - const dns_name_t *name = key; +bcentry_match(struct cds_lfht_node *ht_node, const void *key0) { + const dns__bckey_t *key = key0; dns_bcentry_t *bad = caa_container_of(ht_node, dns_bcentry_t, ht_node); - return dns_name_equal(bad->name, name); + return (bad->type == key->type) && + dns_name_equal(&bad->name, key->name); +} + +static uint32_t +bcentry_hash(const dns__bckey_t *key) { + isc_hash32_t state; + isc_hash32_init(&state); + isc_hash32_hash(&state, key->name->ndata, key->name->length, false); + isc_hash32_hash(&state, &key->type, sizeof(key->type), true); + return isc_hash32_finalize(&state); } static dns_bcentry_t * -bcentry_new(dns_badcache_t *bc, const dns_name_t *name, +bcentry_lookup(struct cds_lfht *ht, uint32_t hashval, dns__bckey_t *key) { + struct cds_lfht_iter iter; + + cds_lfht_lookup(ht, hashval, bcentry_match, key, &iter); + + return cds_lfht_entry(cds_lfht_iter_get_node(&iter), dns_bcentry_t, + ht_node); +} + +static dns_bcentry_t * +bcentry_new(isc_loop_t *loop, const dns_name_t *name, const dns_rdatatype_t type, const uint32_t flags, const isc_stdtime_t expire) { - dns_bcentry_t *bad = isc_mem_get(bc->mctx, sizeof(*bad)); + isc_mem_t *mctx = isc_loop_getmctx(loop); + dns_bcentry_t *bad = isc_mem_get(mctx, sizeof(*bad)); *bad = (dns_bcentry_t){ .type = type, .flags = flags, .expire = expire, + .loop = isc_loop_ref(loop), + .lru_head = CDS_LIST_HEAD_INIT(bad->lru_head), }; - isc_mem_attach(bc->mctx, &bad->mctx); - bad->name = dns_fixedname_initname(&bad->fname); - dns_name_copy(name, bad->name); + dns_name_init(&bad->name, NULL); + dns_name_dup(name, mctx, &bad->name); return bad; } @@ -139,25 +180,34 @@ static void bcentry_destroy(struct rcu_head *rcu_head) { dns_bcentry_t *bad = caa_container_of(rcu_head, dns_bcentry_t, rcu_head); + isc_loop_t *loop = bad->loop; + isc_mem_t *mctx = isc_loop_getmctx(loop); - isc_mem_putanddetach(&bad->mctx, bad, sizeof(*bad)); + dns_name_free(&bad->name, mctx); + isc_mem_put(mctx, bad, sizeof(*bad)); + + isc_loop_unref(loop); +} + +static void +bcentry_evict_async(void *arg) { + dns_bcentry_t *bad = arg; + + RUNTIME_CHECK(bad->loop == isc_loop()); + + cds_list_del(&bad->lru_head); + call_rcu(&bad->rcu_head, bcentry_destroy); } static void bcentry_evict(struct cds_lfht *ht, dns_bcentry_t *bad) { - /* - * The hashtable isn't locked in a traditional sense, so multiple - * threads can lookup and evict the same record at the same time. - * - * This is amplified by the bcentry_purge_next() that walks a few more - * records in the hashtable and evicts them if they are expired. - * - * We need to destroy the bcentry only once - from the thread that has - * deleted the entry from the hashtable, all other calls to this - * function were redundant. - */ if (!cds_lfht_del(ht, &bad->ht_node)) { - call_rcu(&bad->rcu_head, bcentry_destroy); + if (bad->loop == isc_loop()) { + bcentry_evict_async(bad); + return; + } + + isc_async_run(bad->loop, bcentry_evict_async, bad); } } @@ -165,7 +215,7 @@ static bool bcentry_alive(struct cds_lfht *ht, dns_bcentry_t *bad, isc_stdtime_t now) { if (cds_lfht_is_node_deleted(&bad->ht_node)) { return false; - } else if (atomic_load_relaxed(&bad->expire) < now) { + } else if (bad->expire < now) { bcentry_evict(ht, bad); return false; } @@ -183,13 +233,12 @@ bcentry_alive(struct cds_lfht *ht, dns_bcentry_t *bad, isc_stdtime_t now) { __typeof__(*(pos)), member)) static void -bcentry_purge_next(struct cds_lfht *ht, struct cds_lfht_iter *iter, - isc_stdtime_t now) { - /* Lazy-purge the table */ +bcentry_purge(struct cds_lfht *ht, struct cds_list_head *lru, + isc_stdtime_t now) { size_t count = 10; dns_bcentry_t *bad; - cds_lfht_for_each_entry_next(ht, iter, bad, ht_node) { - if (!bcentry_alive(ht, bad, now)) { + cds_list_for_each_entry_rcu(bad, lru, lru_head) { + if (bcentry_alive(ht, bad, now)) { break; } if (--count == 0) { @@ -200,11 +249,14 @@ bcentry_purge_next(struct cds_lfht *ht, struct cds_lfht_iter *iter, void dns_badcache_add(dns_badcache_t *bc, const dns_name_t *name, - dns_rdatatype_t type, bool update, uint32_t flags, - isc_stdtime_t expire) { + dns_rdatatype_t type, uint32_t flags, isc_stdtime_t expire) { REQUIRE(VALID_BADCACHE(bc)); REQUIRE(name != NULL); + isc_loop_t *loop = isc_loop(); + uint32_t tid = isc_tid(); + struct cds_list_head *lru = &bc->lru[tid]; + isc_stdtime_t now = isc_stdtime_now(); if (expire < now) { expire = now; @@ -214,36 +266,29 @@ dns_badcache_add(dns_badcache_t *bc, const dns_name_t *name, struct cds_lfht *ht = rcu_dereference(bc->ht); INSIST(ht != NULL); - dns_bcentry_t *bad = NULL; - uint32_t hashval = dns_name_hash(name); + dns__bckey_t key = { + .name = name, + .type = type, + }; + uint32_t hashval = bcentry_hash(&key); - struct cds_lfht_iter iter; - dns_bcentry_t *found = NULL; - cds_lfht_for_each_entry_duplicate(ht, hashval, bcentry_match, name, - &iter, bad, ht_node) { - if (bcentry_alive(ht, bad, now) && bad->type == type) { - found = bad; - /* - * We could bail-out on first match, but: - * 1. there could be duplicate .type entries - * 2. we want to check expire for all entries - */ + /* struct cds_lfht_iter iter; */ + dns_bcentry_t *bad = bcentry_new(loop, name, type, flags, expire); + struct cds_lfht_node *ht_node; + do { + ht_node = cds_lfht_add_unique(ht, hashval, bcentry_match, &key, + &bad->ht_node); + if (ht_node != &bad->ht_node) { + dns_bcentry_t *found = caa_container_of( + ht_node, dns_bcentry_t, ht_node); + bcentry_evict(ht, found); } - } + } while (ht_node != &bad->ht_node); - if (found == NULL) { - /* - * In theory, this could result in multiple entries for the same - * type, but we don't care much, as they are going to be roughly - * the same, and the last will always trump and the former - * entries will expire (see above). - */ - bad = bcentry_new(bc, name, type, flags, expire); - cds_lfht_add(ht, hashval, &bad->ht_node); - } else if (update) { - atomic_store_relaxed(&found->expire, expire); - atomic_store_relaxed(&found->flags, flags); - } + /* No locking, instead we are using per-thread lists */ + cds_list_add_tail_rcu(&bad->lru_head, lru); + + bcentry_purge(ht, lru, now); rcu_read_unlock(); } @@ -260,27 +305,25 @@ dns_badcache_find(dns_badcache_t *bc, const dns_name_t *name, struct cds_lfht *ht = rcu_dereference(bc->ht); INSIST(ht != NULL); - dns_bcentry_t *bad = NULL; - uint32_t hashval = dns_name_hash(name); + dns__bckey_t key = { + .name = name, + .type = type, + }; + uint32_t hashval = bcentry_hash(&key); - struct cds_lfht_iter iter; - dns_bcentry_t *found = NULL; - cds_lfht_for_each_entry_duplicate(ht, hashval, bcentry_match, name, - &iter, bad, ht_node) { - if (bad->type == type && bcentry_alive(ht, bad, now)) { - found = bad; - } - } + dns_bcentry_t *found = bcentry_lookup(ht, hashval, &key); - if (found) { + if (found != NULL && bcentry_alive(ht, found, now)) { result = ISC_R_SUCCESS; if (flagp != NULL) { - *flagp = atomic_load_relaxed(&found->flags); + *flagp = found->flags; } - - bcentry_purge_next(ht, &iter, now); } + uint32_t tid = isc_tid(); + struct cds_list_head *lru = &bc->lru[tid]; + bcentry_purge(ht, lru, now); + rcu_read_unlock(); return result; @@ -290,44 +333,14 @@ void dns_badcache_flush(dns_badcache_t *bc) { REQUIRE(VALID_BADCACHE(bc)); - struct cds_lfht *ht = - cds_lfht_new(BADCACHE_INIT_SIZE, BADCACHE_MIN_SIZE, 0, - CDS_LFHT_AUTO_RESIZE | CDS_LFHT_ACCOUNTING, NULL); - INSIST(ht != NULL); - - /* First swap the hashtables */ - rcu_read_lock(); - ht = rcu_xchg_pointer(&bc->ht, ht); - rcu_read_unlock(); - - /* Make sure nobody is using the old hash table */ - synchronize_rcu(); - - /* Flush the old hash table */ - dns_bcentry_t *bad = NULL; - struct cds_lfht_iter iter; - cds_lfht_for_each_entry(ht, &iter, bad, ht_node) { - INSIST(!cds_lfht_del(ht, &bad->ht_node)); - bcentry_destroy(&bad->rcu_head); - } - RUNTIME_CHECK(!cds_lfht_destroy(ht, NULL)); -} - -void -dns_badcache_flushname(dns_badcache_t *bc, const dns_name_t *name) { - REQUIRE(VALID_BADCACHE(bc)); - REQUIRE(name != NULL); - rcu_read_lock(); struct cds_lfht *ht = rcu_dereference(bc->ht); INSIST(ht != NULL); - dns_bcentry_t *bad = NULL; - uint32_t hashval = dns_name_hash(name); - + /* Flush the hash table */ + dns_bcentry_t *bad; struct cds_lfht_iter iter; - cds_lfht_for_each_entry_duplicate(ht, hashval, bcentry_match, name, - &iter, bad, ht_node) { + cds_lfht_for_each_entry(ht, &iter, bad, ht_node) { bcentry_evict(ht, bad); } @@ -335,24 +348,52 @@ dns_badcache_flushname(dns_badcache_t *bc, const dns_name_t *name) { } void -dns_badcache_flushtree(dns_badcache_t *bc, const dns_name_t *name) { - dns_bcentry_t *bad; - isc_stdtime_t now = isc_stdtime_now(); - +dns_badcache_flushname(dns_badcache_t *bc, const dns_name_t *name) { REQUIRE(VALID_BADCACHE(bc)); REQUIRE(name != NULL); + isc_stdtime_t now = isc_stdtime_now(); + rcu_read_lock(); struct cds_lfht *ht = rcu_dereference(bc->ht); INSIST(ht != NULL); + dns_bcentry_t *bad; struct cds_lfht_iter iter; cds_lfht_for_each_entry(ht, &iter, bad, ht_node) { - if (dns_name_issubdomain(bad->name, name)) { + if (dns_name_equal(&bad->name, name)) { bcentry_evict(ht, bad); - } else if (!bcentry_alive(ht, bad, now)) { - /* Flush all the expired entries */ + continue; } + + /* Flush all the expired entries */ + (void)bcentry_alive(ht, bad, now); + } + + rcu_read_unlock(); +} + +void +dns_badcache_flushtree(dns_badcache_t *bc, const dns_name_t *name) { + REQUIRE(VALID_BADCACHE(bc)); + REQUIRE(name != NULL); + + isc_stdtime_t now = isc_stdtime_now(); + + rcu_read_lock(); + struct cds_lfht *ht = rcu_dereference(bc->ht); + INSIST(ht != NULL); + + dns_bcentry_t *bad; + struct cds_lfht_iter iter; + cds_lfht_for_each_entry(ht, &iter, bad, ht_node) { + if (dns_name_issubdomain(&bad->name, name)) { + bcentry_evict(ht, bad); + continue; + } + + /* Flush all the expired entries */ + (void)bcentry_alive(ht, bad, now); } rcu_read_unlock(); @@ -363,10 +404,10 @@ bcentry_print(dns_bcentry_t *bad, isc_stdtime_t now, FILE *fp) { char namebuf[DNS_NAME_FORMATSIZE]; char typebuf[DNS_RDATATYPE_FORMATSIZE]; - dns_name_format(bad->name, namebuf, sizeof(namebuf)); + dns_name_format(&bad->name, namebuf, sizeof(namebuf)); dns_rdatatype_format(bad->type, typebuf, sizeof(typebuf)); fprintf(fp, "; %s/%s [ttl %" PRIu32 "]\n", namebuf, typebuf, - atomic_load_relaxed(&bad->expire) - now); + bad->expire - now); } void diff --git a/lib/dns/client.c b/lib/dns/client.c index 5e25b9b508..c22c4d4b66 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -199,13 +200,13 @@ getudpdispatch(int family, dns_dispatchmgr_t *dispatchmgr, static isc_result_t createview(isc_mem_t *mctx, dns_rdataclass_t rdclass, isc_nm_t *nm, - isc_tlsctx_cache_t *tlsctx_client_cache, + isc_tlsctx_cache_t *tlsctx_client_cache, isc_loopmgr_t *loopmgr, dns_dispatchmgr_t *dispatchmgr, dns_dispatch_t *dispatchv4, dns_dispatch_t *dispatchv6, dns_view_t **viewp) { isc_result_t result; dns_view_t *view = NULL; - result = dns_view_create(mctx, dispatchmgr, rdclass, + result = dns_view_create(mctx, loopmgr, dispatchmgr, rdclass, DNS_CLIENTVIEW_NAME, &view); if (result != ISC_R_SUCCESS) { return result; @@ -292,6 +293,7 @@ dns_client_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, isc_nm_t *nm, /* Create the default view for class IN */ result = createview(mctx, dns_rdataclass_in, nm, tlsctx_client_cache, + isc_loop_getloopmgr(client->loop), client->dispatchmgr, dispatchv4, dispatchv6, &view); if (result != ISC_R_SUCCESS) { goto cleanup_references; diff --git a/lib/dns/include/dns/badcache.h b/lib/dns/include/dns/badcache.h index 03eb2ec3b9..7552decef1 100644 --- a/lib/dns/include/dns/badcache.h +++ b/lib/dns/include/dns/badcache.h @@ -45,6 +45,7 @@ #include #include +#include #include #include @@ -57,7 +58,7 @@ ISC_LANG_BEGINDECLS ***/ dns_badcache_t * -dns_badcache_new(isc_mem_t *mctx); +dns_badcache_new(isc_mem_t *mctx, isc_loopmgr_t *loopmgr); /*% * Allocate and initialize a badcache and store it in '*bcp'. * @@ -78,18 +79,15 @@ dns_badcache_destroy(dns_badcache_t **bcp); void dns_badcache_add(dns_badcache_t *bc, const dns_name_t *name, - dns_rdatatype_t type, bool update, uint32_t flags, - isc_stdtime_t expire); + dns_rdatatype_t type, uint32_t flags, isc_stdtime_t expire); /*% - * Adds a badcache entry to the badcache 'bc' for name 'name' and - * type 'type'. If an entry already exists, then it will be updated if - * 'update' is true. The entry will be stored with flags 'flags' - * and expiration date 'expire'. + * Adds a badcache entry to the badcache 'bc' for name 'name' and type 'type'. + * If an entry already exists, then it will be updated. The entry will be + * stored with flags 'flags' and expiration date 'expire'. * * Requires: * \li bc to be a valid badcache. * \li name != NULL - * \li expire != NULL */ isc_result_t diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index 845531c0d8..c82708ca52 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -263,8 +263,9 @@ struct dns_view { #endif /* HAVE_LMDB */ isc_result_t -dns_view_create(isc_mem_t *mctx, dns_dispatchmgr_t *dispmgr, - dns_rdataclass_t rdclass, const char *name, dns_view_t **viewp); +dns_view_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, + dns_dispatchmgr_t *dispmgr, dns_rdataclass_t rdclass, + const char *name, dns_view_t **viewp); /*%< * Create a view. * diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 276bbea467..294d173a36 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -10027,7 +10027,7 @@ dns_resolver_create(dns_view_t *view, isc_loopmgr_t *loopmgr, isc_nm_t *nm, #endif isc_refcount_init(&res->references, 1); - res->badcache = dns_badcache_new(res->mctx); + res->badcache = dns_badcache_new(res->mctx, res->loopmgr); isc_hashmap_create(view->mctx, RES_DOMAIN_HASH_BITS, &res->fctxs); isc_rwlock_init(&res->fctxs_lock); @@ -10737,12 +10737,13 @@ void dns_resolver_addbadcache(dns_resolver_t *resolver, const dns_name_t *name, dns_rdatatype_t type, isc_time_t *expire) { #ifdef ENABLE_AFL - if (!dns_fuzzing_resolver) -#endif /* ifdef ENABLE_AFL */ - { - dns_badcache_add(resolver->badcache, name, type, false, 0, - isc_time_seconds(expire)); + if (dns_fuzzing_resolver) { + return; } +#endif /* ifdef ENABLE_AFL */ + + dns_badcache_add(resolver->badcache, name, type, 0, + isc_time_seconds(expire)); } isc_result_t diff --git a/lib/dns/view.c b/lib/dns/view.c index 3324108470..124dba3216 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -85,9 +85,9 @@ #define DEFAULT_EDNS_BUFSIZE 1232 isc_result_t -dns_view_create(isc_mem_t *mctx, dns_dispatchmgr_t *dispatchmgr, - dns_rdataclass_t rdclass, const char *name, - dns_view_t **viewp) { +dns_view_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, + dns_dispatchmgr_t *dispatchmgr, dns_rdataclass_t rdclass, + const char *name, dns_view_t **viewp) { dns_view_t *view = NULL; isc_result_t result; char buffer[1024]; @@ -149,7 +149,7 @@ dns_view_create(isc_mem_t *mctx, dns_dispatchmgr_t *dispatchmgr, dns_tsigkeyring_create(view->mctx, &view->dynamickeys); - view->failcache = dns_badcache_new(view->mctx); + view->failcache = dns_badcache_new(view->mctx, loopmgr); isc_mutex_init(&view->new_zone_lock); diff --git a/lib/ns/client.c b/lib/ns/client.c index 7249a0d57a..b1502dee48 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -1071,7 +1071,7 @@ ns_client_error(ns_client_t *client, isc_result_t result) { if (result == ISC_R_SUCCESS) { dns_badcache_add(client->view->failcache, client->query.qname, - client->query.qtype, true, flags, + client->query.qtype, flags, isc_time_seconds(&expire)); } } diff --git a/tests/dns/badcache_test.c b/tests/dns/badcache_test.c index 0c81c7ac02..2e2695b0b7 100644 --- a/tests/dns/badcache_test.c +++ b/tests/dns/badcache_test.c @@ -55,8 +55,8 @@ ISC_LOOP_TEST_IMPL(basic) { dns_name_fromstring(name, "example.com.", NULL, 0, NULL); - bc = dns_badcache_new(mctx); - dns_badcache_add(bc, name, dns_rdatatype_aaaa, false, flags, now + 60); + bc = dns_badcache_new(mctx, loopmgr); + dns_badcache_add(bc, name, dns_rdatatype_aaaa, flags, now + 60); flags = 0; result = dns_badcache_find(bc, name, dns_rdatatype_aaaa, &flags, now); @@ -83,9 +83,9 @@ ISC_LOOP_TEST_IMPL(expire) { dns_name_fromstring(name, "example.com.", NULL, 0, NULL); - bc = dns_badcache_new(mctx); - dns_badcache_add(bc, name, dns_rdatatype_aaaa, false, flags, now + 60); - dns_badcache_add(bc, name, dns_rdatatype_a, false, flags, now + 60); + bc = dns_badcache_new(mctx, loopmgr); + dns_badcache_add(bc, name, dns_rdatatype_aaaa, flags, now + 60); + dns_badcache_add(bc, name, dns_rdatatype_a, flags, now + 60); result = dns_badcache_find(bc, name, dns_rdatatype_aaaa, &flags, now); assert_int_equal(result, ISC_R_SUCCESS); @@ -99,10 +99,9 @@ ISC_LOOP_TEST_IMPL(expire) { assert_int_equal(result, ISC_R_NOTFOUND); result = dns_badcache_find(bc, name, dns_rdatatype_a, &flags, now); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(flags, BADCACHE_TEST_FLAG); + assert_int_equal(result, ISC_R_NOTFOUND); - dns_badcache_add(bc, name, dns_rdatatype_a, true, flags, now + 120); + dns_badcache_add(bc, name, dns_rdatatype_a, flags, now + 120); result = dns_badcache_find(bc, name, dns_rdatatype_a, &flags, now + 61); assert_int_equal(result, ISC_R_SUCCESS); @@ -125,16 +124,20 @@ ISC_LOOP_TEST_IMPL(print) { size_t len; char *pos; char *endptr; - const char *part1 = ";\n; badcache\n;\n; example.com/A [ttl "; - const char *part2 = "]\n; example.com/AAAA [ttl "; - const char *part3 = "]\n"; + const char *header_part = ";\n; badcache\n;\n"; + const char *bol_part = "; "; + const char *name_part = "example.com/"; + const char *ttl_part = " [ttl "; + const char *eol_part = "]\n"; + size_t num_a = 0; + bool seen_a = false, seen_aaaa = false; long ttl; dns_name_fromstring(name, "example.com.", NULL, 0, NULL); - bc = dns_badcache_new(mctx); - dns_badcache_add(bc, name, dns_rdatatype_a, false, flags, expire); - dns_badcache_add(bc, name, dns_rdatatype_aaaa, false, flags, expire); + bc = dns_badcache_new(mctx, loopmgr); + dns_badcache_add(bc, name, dns_rdatatype_a, flags, expire); + dns_badcache_add(bc, name, dns_rdatatype_aaaa, flags, expire); file = fopen("./badcache.out", "w"); dns_badcache_print(bc, "badcache", file); @@ -146,24 +149,47 @@ ISC_LOOP_TEST_IMPL(print) { fclose(file); pos = buf; - assert_memory_equal(pos, part1, strlen(part1)); - pos += strlen(part1); + assert_memory_equal(pos, header_part, strlen(header_part)); + pos += strlen(header_part); + +line: + /* There's no fixed order for A and AAAA types in the hash table */ + assert_memory_equal(pos, bol_part, strlen(bol_part)); + pos += strlen(bol_part); + + assert_memory_equal(pos, name_part, strlen(name_part)); + pos += strlen(name_part); + + num_a = 0; + while (*pos == 'A') { + num_a++; + pos++; + } + switch (num_a) { + case 1: + seen_a = true; + break; + case 4: + seen_aaaa = true; + break; + default: + assert_true(num_a == 1 || num_a == 4); + } + + assert_memory_equal(pos, ttl_part, strlen(ttl_part)); + pos += strlen(ttl_part); ttl = strtol(pos, &endptr, 0); assert_ptr_not_equal(pos, endptr); assert_true(ttl >= 0 && ttl <= 60); pos = endptr; - assert_memory_equal(pos, part2, strlen(part2)); - pos += strlen(part2); + assert_memory_equal(pos, eol_part, strlen(eol_part)); + pos += strlen(eol_part); - ttl = strtol(pos, &endptr, 0); - assert_ptr_not_equal(pos, endptr); - assert_true(ttl >= 0 && ttl <= 60); - pos = endptr; - - assert_memory_equal(pos, part3, strlen(part3)); - pos += strlen(part3); + if (!seen_a || !seen_aaaa) { + goto line; + } assert_int_equal(pos - buf, len); @@ -182,8 +208,8 @@ ISC_LOOP_TEST_IMPL(flush) { dns_name_fromstring(name, "example.com.", NULL, 0, NULL); - bc = dns_badcache_new(mctx); - dns_badcache_add(bc, name, dns_rdatatype_aaaa, false, flags, now + 60); + bc = dns_badcache_new(mctx, loopmgr); + dns_badcache_add(bc, name, dns_rdatatype_aaaa, flags, now + 60); result = dns_badcache_find(bc, name, dns_rdatatype_aaaa, &flags, now); assert_int_equal(result, ISC_R_SUCCESS); @@ -206,20 +232,20 @@ ISC_LOOP_TEST_IMPL(flushname) { isc_result_t result; uint32_t flags = BADCACHE_TEST_FLAG; - bc = dns_badcache_new(mctx); + bc = dns_badcache_new(mctx, loopmgr); dns_name_fromstring(name, "example.com.", NULL, 0, NULL); - dns_badcache_add(bc, name, dns_rdatatype_aaaa, false, flags, now + 60); + dns_badcache_add(bc, name, dns_rdatatype_aaaa, flags, now + 60); result = dns_badcache_find(bc, name, dns_rdatatype_aaaa, &flags, now); assert_int_equal(result, ISC_R_SUCCESS); dns_name_fromstring(name, "sub.example.com.", NULL, 0, NULL); - dns_badcache_add(bc, name, dns_rdatatype_aaaa, false, flags, now + 60); + dns_badcache_add(bc, name, dns_rdatatype_aaaa, flags, now + 60); result = dns_badcache_find(bc, name, dns_rdatatype_aaaa, &flags, now); assert_int_equal(result, ISC_R_SUCCESS); dns_name_fromstring(name, "sub.sub.example.com.", NULL, 0, NULL); - dns_badcache_add(bc, name, dns_rdatatype_aaaa, false, flags, now + 60); + dns_badcache_add(bc, name, dns_rdatatype_aaaa, flags, now + 60); result = dns_badcache_find(bc, name, dns_rdatatype_aaaa, &flags, now); assert_int_equal(result, ISC_R_SUCCESS); @@ -252,22 +278,22 @@ ISC_LOOP_TEST_IMPL(flushtree) { isc_result_t result; uint32_t flags = BADCACHE_TEST_FLAG; - bc = dns_badcache_new(mctx); + bc = dns_badcache_new(mctx, loopmgr); dns_name_fromstring(name, "example.com.", NULL, 0, NULL); - dns_badcache_add(bc, name, dns_rdatatype_aaaa, false, flags, now + 60); + dns_badcache_add(bc, name, dns_rdatatype_aaaa, flags, now + 60); result = dns_badcache_find(bc, name, dns_rdatatype_aaaa, &flags, now); assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(flags, BADCACHE_TEST_FLAG); dns_name_fromstring(name, "sub.example.com.", NULL, 0, NULL); - dns_badcache_add(bc, name, dns_rdatatype_aaaa, false, flags, now + 60); + dns_badcache_add(bc, name, dns_rdatatype_aaaa, flags, now + 60); result = dns_badcache_find(bc, name, dns_rdatatype_aaaa, &flags, now); assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(flags, BADCACHE_TEST_FLAG); dns_name_fromstring(name, "sub.sub.example.com.", NULL, 0, NULL); - dns_badcache_add(bc, name, dns_rdatatype_aaaa, false, flags, now + 60); + dns_badcache_add(bc, name, dns_rdatatype_aaaa, flags, now + 60); result = dns_badcache_find(bc, name, dns_rdatatype_aaaa, &flags, now); assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(flags, BADCACHE_TEST_FLAG); @@ -301,22 +327,22 @@ ISC_LOOP_TEST_IMPL(purge) { isc_result_t result; uint32_t flags = BADCACHE_TEST_FLAG; - bc = dns_badcache_new(mctx); + bc = dns_badcache_new(mctx, loopmgr); dns_name_fromstring(name, "example.com.", NULL, 0, NULL); - dns_badcache_add(bc, name, dns_rdatatype_aaaa, false, flags, now); + dns_badcache_add(bc, name, dns_rdatatype_aaaa, flags, now); result = dns_badcache_find(bc, name, dns_rdatatype_aaaa, &flags, now - 60); assert_int_equal(result, ISC_R_SUCCESS); dns_name_fromstring(name, "sub.example.com.", NULL, 0, NULL); - dns_badcache_add(bc, name, dns_rdatatype_aaaa, false, flags, now); + dns_badcache_add(bc, name, dns_rdatatype_aaaa, flags, now); result = dns_badcache_find(bc, name, dns_rdatatype_aaaa, &flags, now - 60); assert_int_equal(result, ISC_R_SUCCESS); dns_name_fromstring(name, "sub.sub.example.com.", NULL, 0, NULL); - dns_badcache_add(bc, name, dns_rdatatype_aaaa, false, flags, now); + dns_badcache_add(bc, name, dns_rdatatype_aaaa, flags, now); result = dns_badcache_find(bc, name, dns_rdatatype_aaaa, &flags, now - 60); assert_int_equal(result, ISC_R_SUCCESS); diff --git a/tests/dns/dnstap_test.c b/tests/dns/dnstap_test.c index 8e7b46894e..45bdf2005f 100644 --- a/tests/dns/dnstap_test.c +++ b/tests/dns/dnstap_test.c @@ -43,12 +43,10 @@ #define TAPSAVED TESTS_DIR "/testdata/dnstap/dnstap.saved" #define TAPTEXT TESTS_DIR "/testdata/dnstap/dnstap.text" -static int +static void cleanup(void **state ISC_ATTR_UNUSED) { (void)isc_file_remove(TAPFILE); (void)isc_file_remove(TAPSOCK); - - return 0; } static int @@ -63,11 +61,23 @@ setup(void **state) { * the testdata was originally generated. */ setenv("TZ", "PDT8", 1); + + setup_loopmgr(state); + + return 0; +} + +static int +teardown(void **state) { + cleanup(state); + + teardown_loopmgr(state); + return 0; } /* set up dnstap environment */ -ISC_RUN_TEST_IMPL(dns_dt_create) { +ISC_LOOP_TEST_IMPL(dns_dt_create) { isc_result_t result; dns_dtenv_t *dtenv = NULL; struct fstrm_iothr_options *fopt; @@ -118,10 +128,12 @@ ISC_RUN_TEST_IMPL(dns_dt_create) { if (fopt != NULL) { fstrm_iothr_options_destroy(&fopt); } + + isc_loopmgr_shutdown(loopmgr); } /* send dnstap messages */ -ISC_RUN_TEST_IMPL(dns_dt_send) { +ISC_LOOP_TEST_IMPL(dns_dt_send) { isc_result_t result; dns_dtenv_t *dtenv = NULL; dns_dthandle_t *handle = NULL; @@ -283,18 +295,18 @@ ISC_RUN_TEST_IMPL(dns_dt_send) { if (handle != NULL) { dns_dt_close(&handle); } + + isc_loopmgr_shutdown(loopmgr); } /* dnstap message to text */ -ISC_RUN_TEST_IMPL(dns_dt_totext) { +ISC_LOOP_TEST_IMPL(dns_dt_totext) { isc_result_t result; dns_dthandle_t *handle = NULL; uint8_t *data; size_t dsize; FILE *fp = NULL; - UNUSED(state); - result = dns_dt_open(TAPSAVED, dns_dtmode_file, mctx, &handle); assert_int_equal(result, ISC_R_SUCCESS); @@ -348,13 +360,14 @@ ISC_RUN_TEST_IMPL(dns_dt_totext) { if (handle != NULL) { dns_dt_close(&handle); } + isc_loopmgr_shutdown(loopmgr); } ISC_TEST_LIST_START -ISC_TEST_ENTRY_CUSTOM(dns_dt_create, setup, cleanup) -ISC_TEST_ENTRY_CUSTOM(dns_dt_send, setup, cleanup) -ISC_TEST_ENTRY_CUSTOM(dns_dt_totext, setup, cleanup) +ISC_TEST_ENTRY_CUSTOM(dns_dt_create, setup, teardown) +ISC_TEST_ENTRY_CUSTOM(dns_dt_send, setup, teardown) +ISC_TEST_ENTRY_CUSTOM(dns_dt_totext, setup, teardown) ISC_TEST_LIST_END diff --git a/tests/libtest/dns.c b/tests/libtest/dns.c index 4561340ffe..7d2df9efd9 100644 --- a/tests/libtest/dns.c +++ b/tests/libtest/dns.c @@ -72,8 +72,8 @@ dns_test_makeview(const char *name, bool with_dispatchmgr, bool with_cache, } } - result = dns_view_create(mctx, dispatchmgr, dns_rdataclass_in, name, - &view); + result = dns_view_create(mctx, loopmgr, dispatchmgr, dns_rdataclass_in, + name, &view); if (dispatchmgr != NULL) { dns_dispatchmgr_detach(&dispatchmgr); diff --git a/tests/ns/query_test.c b/tests/ns/query_test.c index b25ff25eea..3dcb1dd791 100644 --- a/tests/ns/query_test.c +++ b/tests/ns/query_test.c @@ -121,8 +121,7 @@ run_sfcache_test(const ns__query_sfcache_test_params_t *test) { assert_int_equal(result, ISC_R_SUCCESS); dns_badcache_add(qctx->client->view->failcache, dns_rootname, - dns_rdatatype_ns, true, - test->cache_entry_flags, + dns_rdatatype_ns, test->cache_entry_flags, isc_time_seconds(&expire)); }