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 1/2] 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)); } From b61739836db9d72262e51596ea07ff8571a541f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 22 Nov 2024 15:10:26 +0100 Subject: [PATCH 2/2] Remove dns_badcache usage in the resolver (lame-ttl) The lame-ttl processing was overriden to be disabled in the config, but the code related to the lame-ttl was still kept in the resolver code. More importantly, the DNS_RESOLVER_BADCACHETTL() macro would cause the entries in the resolver badcache to be always cached for at least 30 seconds even if the lame-ttl would be set to 0. Remove the dns_badcache code from the dns_resolver unit, so we save some processing time and memory in the resolver code. --- bin/named/server.c | 22 ++----- lib/dns/include/dns/resolver.h | 70 --------------------- lib/dns/resolver.c | 110 ++++----------------------------- lib/dns/validator.c | 13 ---- lib/dns/view.c | 9 --- 5 files changed, 17 insertions(+), 207 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index ac05082489..4b624d7ebf 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -251,7 +251,6 @@ struct dumpcontext { bool dumpcache; bool dumpzones; bool dumpadb; - bool dumpbad; bool dumpexpired; bool dumpfail; FILE *fp; @@ -4628,9 +4627,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, cfg_obj_log(obj, ISC_LOG_WARNING, "disabling lame cache despite lame-ttl > 0 as it " "may cause performance issues"); - lame_ttl = 0; } - dns_resolver_setlamettl(view->resolver, lame_ttl); /* * Set the resolver's query timeout. @@ -10344,7 +10341,7 @@ zone_from_args(named_server_t *server, isc_lex_t *lex, const char *zonetxt, } } else { result = dns_viewlist_findzone(&server->viewlist, name, - (classtxt == NULL), + classtxt == NULL, rdclass, zonep); if (result == ISC_R_NOTFOUND) { snprintf(problem, sizeof(problem), @@ -11260,8 +11257,8 @@ resume: } } - if ((dctx->dumpadb || dctx->dumpbad || dctx->dumpfail) && - dctx->cache == NULL && dctx->view->view->cachedb != NULL) + if ((dctx->dumpadb || dctx->dumpfail) && dctx->cache == NULL && + dctx->view->view->cachedb != NULL) { dns_db_attach(dctx->view->view->cachedb, &dctx->cache); } @@ -11275,10 +11272,6 @@ resume: dns_adb_detach(&adb); } } - if (dctx->dumpbad) { - dns_resolver_printbadcache(dctx->view->view->resolver, - dctx->fp); - } if (dctx->dumpfail) { dns_badcache_print(dctx->view->view->failcache, "SERVFAIL cache", dctx->fp); @@ -11374,7 +11367,6 @@ named_server_dumpdb(named_server_t *server, isc_lex_t *lex, .mctx = server->mctx, .dumpcache = true, .dumpadb = true, - .dumpbad = true, .dumpfail = true, .viewlist = ISC_LIST_INITIALIZER, }; @@ -11402,14 +11394,12 @@ named_server_dumpdb(named_server_t *server, isc_lex_t *lex, } else if (ptr != NULL && strcmp(ptr, "-zones") == 0) { /* only dump zones, suppress caches */ dctx->dumpadb = false; - dctx->dumpbad = false; dctx->dumpcache = false; dctx->dumpfail = false; dctx->dumpzones = true; ptr = next_token(lex, NULL); } else if (ptr != NULL && strcmp(ptr, "-adb") == 0) { /* only dump adb, suppress other caches */ - dctx->dumpbad = false; dctx->dumpcache = false; dctx->dumpfail = false; ptr = next_token(lex, NULL); @@ -11422,7 +11412,6 @@ named_server_dumpdb(named_server_t *server, isc_lex_t *lex, } else if (ptr != NULL && strcmp(ptr, "-fail") == 0) { /* only dump servfail cache, suppress other caches */ dctx->dumpadb = false; - dctx->dumpbad = false; dctx->dumpcache = false; ptr = next_token(lex, NULL); } @@ -12099,9 +12088,8 @@ named_server_status(named_server_t *server, isc_buffer_t **text) { reload_status = atomic_load(&server->reload_status); if (reload_status != NAMED_RELOAD_DONE) { snprintf(line, sizeof(line), "reload/reconfig %s\n", - (reload_status == NAMED_RELOAD_FAILED - ? "failed" - : "in progress")); + reload_status == NAMED_RELOAD_FAILED ? "failed" + : "in progress"); CHECK(putstr(text, line)); } diff --git a/lib/dns/include/dns/resolver.h b/lib/dns/include/dns/resolver.h index 75f92b63f7..daa7ef2cff 100644 --- a/lib/dns/include/dns/resolver.h +++ b/lib/dns/include/dns/resolver.h @@ -386,24 +386,6 @@ dns_resolver_dispatchv4(dns_resolver_t *resolver); dns_dispatch_t * dns_resolver_dispatchv6(dns_resolver_t *resolver); -uint32_t -dns_resolver_getlamettl(dns_resolver_t *resolver); -/*%< - * Get the resolver's lame-ttl. zero => no lame processing. - * - * Requires: - *\li 'resolver' to be valid. - */ - -void -dns_resolver_setlamettl(dns_resolver_t *resolver, uint32_t lame_ttl); -/*%< - * Set the resolver's lame-ttl. zero => no lame processing. - * - * Requires: - *\li 'resolver' to be valid. - */ - void dns_resolver_addalternate(dns_resolver_t *resolver, const isc_sockaddr_t *alt, const dns_name_t *name, in_port_t port); @@ -522,58 +504,6 @@ dns_resolver_getoptions(dns_resolver_t *resolver); * \li resolver to be valid. */ -void -dns_resolver_addbadcache(dns_resolver_t *resolver, const dns_name_t *name, - dns_rdatatype_t type, isc_time_t *expire); -/*%< - * Add a entry to the bad cache for that will expire at 'expire'. - * - * Requires: - * \li resolver to be valid. - * \li name to be valid. - */ - -isc_result_t -dns_resolver_getbadcache(dns_resolver_t *resolver, const dns_name_t *name, - dns_rdatatype_t type, isc_time_t *now); -/*%< - * Check to see if there is a unexpired entry in the bad cache for - * . - * - * Requires: - * \li resolver to be valid. - * \li name to be valid. - */ - -void -dns_resolver_flushbadcache(dns_resolver_t *resolver, const dns_name_t *name); -/*%< - * Flush the bad cache of all entries at 'name' if 'name' is non NULL. - * Flush the entire bad cache if 'name' is NULL. - * - * Requires: - * \li resolver to be valid. - */ - -void -dns_resolver_flushbadnames(dns_resolver_t *resolver, const dns_name_t *name); -/*%< - * Flush the bad cache of all entries at or below 'name'. - * - * Requires: - * \li resolver to be valid. - * \li name != NULL - */ - -void -dns_resolver_printbadcache(dns_resolver_t *resolver, FILE *fp); -/*% - * Print out the contents of the bad cache to 'fp'. - * - * Requires: - * \li resolver to be valid. - */ - void dns_resolver_setmaxvalidations(dns_resolver_t *resolver, uint32_t max); void diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 294d173a36..cd5a780ad8 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -40,7 +40,6 @@ #include #include -#include #include #include #include @@ -246,9 +245,6 @@ STATIC_ASSERT(NS_PROCESSING_LIMIT > NS_RR_LIMIT, */ #define MAX_EDNS0_TIMEOUTS 3 -#define DNS_RESOLVER_BADCACHETTL(fctx) \ - (((fctx)->res->lame_ttl > 30) ? (fctx)->res->lame_ttl : 30) - typedef struct fetchctx fetchctx_t; typedef struct query { @@ -589,8 +585,6 @@ struct dns_resolver { /* Locked by lock. */ unsigned int spillat; /* clients-per-query */ - dns_badcache_t *badcache; /* Bad cache. */ - /* Locked by primelock. */ dns_fetch_t *primefetch; @@ -654,7 +648,7 @@ resquery_response_continue(void *arg, isc_result_t result); static void resquery_connected(isc_result_t eresult, isc_region_t *region, void *arg); static void -fctx_try(fetchctx_t *fctx, bool retrying, bool badcache); +fctx_try(fetchctx_t *fctx, bool retrying); static void fctx_shutdown(void *arg); static void @@ -1777,7 +1771,7 @@ resquery_senddone(isc_result_t eresult, isc_region_t *region, void *arg) { badns_unreachable); fctx_cancelquery(©, NULL, true, false); FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); - fctx_try(fctx, true, false); + fctx_try(fctx, true); break; default: @@ -2865,7 +2859,7 @@ resquery_connected(isc_result_t eresult, isc_region_t *region, void *arg) { fctx_cancelquery(©, NULL, true, false); FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); - fctx_try(fctx, true, false); + fctx_try(fctx, true); break; default: @@ -2932,7 +2926,7 @@ fctx_finddone(void *arg) { fctx_done_unref(fctx, ISC_R_FAILURE); } else if (want_try) { - fctx_try(fctx, true, false); + fctx_try(fctx, true); } fetchctx_detach(&fctx); @@ -3407,7 +3401,7 @@ isstrictsubdomain(const dns_name_t *name1, const dns_name_t *name2) { } static isc_result_t -fctx_getaddresses(fetchctx_t *fctx, bool badcache) { +fctx_getaddresses(fetchctx_t *fctx) { dns_rdata_t rdata = DNS_RDATA_INIT; isc_result_t result; dns_resolver_t *res; @@ -3700,24 +3694,12 @@ out: */ result = DNS_R_WAIT; } else { - isc_time_t expire; - isc_interval_t i; /* * We've lost completely. We don't know any * addresses, and the ADB has told us it can't * get them. */ FCTXTRACE("no addresses"); - isc_interval_set(&i, DNS_RESOLVER_BADCACHETTL(fctx), 0); - result = isc_time_nowplusinterval(&expire, &i); - if (badcache && - (fctx->type == dns_rdatatype_dnskey || - fctx->type == dns_rdatatype_ds) && - result == ISC_R_SUCCESS) - { - dns_resolver_addbadcache(res, fctx->name, - fctx->type, &expire); - } result = ISC_R_FAILURE; @@ -3986,7 +3968,7 @@ fctx_nextaddress(fetchctx_t *fctx) { } static void -fctx_try(fetchctx_t *fctx, bool retrying, bool badcache) { +fctx_try(fetchctx_t *fctx, bool retrying) { isc_result_t result; dns_adbaddrinfo_t *addrinfo = NULL; dns_resolver_t *res = NULL; @@ -4021,7 +4003,7 @@ fctx_try(fetchctx_t *fctx, bool retrying, bool badcache) { /* We have no more addresses. Start over. */ fctx_cancelqueries(fctx, true, false); fctx_cleanup(fctx); - result = fctx_getaddresses(fctx, badcache); + result = fctx_getaddresses(fctx); switch (result) { case ISC_R_SUCCESS: break; @@ -4292,7 +4274,7 @@ resume_qmin(void *arg) { fctx_cleanup(fctx); } - fctx_try(fctx, true, false); + fctx_try(fctx, true); cleanup: if (result != ISC_R_SUCCESS) { @@ -4418,7 +4400,7 @@ fctx_start(void *arg) { * while a response is being processed normally.) */ fctx_starttimer(fctx); - fctx_try(fctx, false, false); + fctx_try(fctx, false); detach: fetchctx_detach(&fctx); @@ -5305,24 +5287,10 @@ validated(void *arg) { done = true; goto cleanup_fetchctx; } else if (result == DNS_R_BROKENCHAIN) { - isc_result_t tresult; - isc_time_t expire; - isc_interval_t i; - - isc_interval_set(&i, DNS_RESOLVER_BADCACHETTL(fctx), 0); - tresult = isc_time_nowplusinterval(&expire, &i); - if (negative && - (fctx->type == dns_rdatatype_dnskey || - fctx->type == dns_rdatatype_ds) && - tresult == ISC_R_SUCCESS) - { - dns_resolver_addbadcache(res, fctx->name, - fctx->type, &expire); - } done = true; goto cleanup_fetchctx; } else { - fctx_try(fctx, true, true); + fctx_try(fctx, true); goto cleanup_fetchctx; } UNREACHABLE(); @@ -7102,7 +7070,7 @@ resume_dslookup(void *arg) { } /* Try again. */ - fctx_try(fctx, true, false); + fctx_try(fctx, true); break; case ISC_R_SHUTTINGDOWN: @@ -9510,7 +9478,7 @@ rctx_nextserver(respctx_t *rctx, dns_message_t *message, /* * Try again. */ - fctx_try(fctx, retrying, false); + fctx_try(fctx, retrying); } /* @@ -9933,7 +9901,6 @@ dns_resolver__destroy(dns_resolver_t *res) { } isc_mem_put(res->mctx, a, sizeof(*a)); } - dns_badcache_destroy(&res->badcache); dns_view_weakdetach(&res->view); @@ -10027,8 +9994,6 @@ 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->loopmgr); - isc_hashmap_create(view->mctx, RES_DOMAIN_HASH_BITS, &res->fctxs); isc_rwlock_init(&res->fctxs_lock); @@ -10684,18 +10649,6 @@ dns_resolver_dispatchv6(dns_resolver_t *resolver) { return dns_dispatchset_get(resolver->dispatches6); } -uint32_t -dns_resolver_getlamettl(dns_resolver_t *resolver) { - REQUIRE(VALID_RESOLVER(resolver)); - return resolver->lame_ttl; -} - -void -dns_resolver_setlamettl(dns_resolver_t *resolver, uint32_t lame_ttl) { - REQUIRE(VALID_RESOLVER(resolver)); - resolver->lame_ttl = lame_ttl; -} - void dns_resolver_addalternate(dns_resolver_t *res, const isc_sockaddr_t *alt, const dns_name_t *name, in_port_t port) { @@ -10719,45 +10672,6 @@ dns_resolver_addalternate(dns_resolver_t *res, const isc_sockaddr_t *alt, ISC_LIST_APPEND(res->alternates, a, link); } -void -dns_resolver_flushbadcache(dns_resolver_t *resolver, const dns_name_t *name) { - if (name != NULL) { - dns_badcache_flushname(resolver->badcache, name); - } else { - dns_badcache_flush(resolver->badcache); - } -} - -void -dns_resolver_flushbadnames(dns_resolver_t *resolver, const dns_name_t *name) { - dns_badcache_flushtree(resolver->badcache, name); -} - -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) { - return; - } -#endif /* ifdef ENABLE_AFL */ - - dns_badcache_add(resolver->badcache, name, type, 0, - isc_time_seconds(expire)); -} - -isc_result_t -dns_resolver_getbadcache(dns_resolver_t *resolver, const dns_name_t *name, - dns_rdatatype_t type, isc_time_t *now) { - return dns_badcache_find(resolver->badcache, name, type, NULL, - isc_time_seconds(now)); -} - -void -dns_resolver_printbadcache(dns_resolver_t *resolver, FILE *fp) { - (void)dns_badcache_print(resolver->badcache, "Bad cache", fp); -} - isc_result_t dns_resolver_disable_algorithm(dns_resolver_t *resolver, const dns_name_t *name, unsigned int alg) { diff --git a/lib/dns/validator.c b/lib/dns/validator.c index a1934169d3..aafe5147d0 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -852,22 +852,9 @@ view_find(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type) { dns_name_t *foundname; isc_result_t result; unsigned int options; - isc_time_t now = isc_time_now(); - char namebuf[DNS_NAME_FORMATSIZE]; - char typebuf[DNS_RDATATYPE_FORMATSIZE]; disassociate_rdatasets(val); - result = dns_resolver_getbadcache(val->view->resolver, name, type, - &now); - if (result == ISC_R_SUCCESS) { - dns_name_format(name, namebuf, sizeof(namebuf)); - dns_rdatatype_format(type, typebuf, sizeof(typebuf)); - validator_log(val, ISC_LOG_INFO, "bad cache hit (%s/%s)", - namebuf, typebuf); - return DNS_R_BROKENCHAIN; - } - options = DNS_DBFIND_PENDINGOK; foundname = dns_fixedname_initname(&fixedname); result = dns_view_find(val->view, name, type, 0, options, false, false, diff --git a/lib/dns/view.c b/lib/dns/view.c index 124dba3216..5251f7a405 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -1453,9 +1453,6 @@ dns_view_flushcache(dns_view_t *view, bool fixuponly) { } dns_db_detach(&view->cachedb); dns_cache_attachdb(view->cache, &view->cachedb); - if (view->resolver != NULL) { - dns_resolver_flushbadcache(view->resolver, NULL); - } if (view->failcache != NULL) { dns_badcache_flush(view->failcache); } @@ -1489,9 +1486,6 @@ dns_view_flushnode(dns_view_t *view, const dns_name_t *name, bool tree) { dns_adb_flushnames(adb, name); } rcu_read_unlock(); - if (view->resolver != NULL) { - dns_resolver_flushbadnames(view->resolver, name); - } if (view->failcache != NULL) { dns_badcache_flushtree(view->failcache, name); } @@ -1502,9 +1496,6 @@ dns_view_flushnode(dns_view_t *view, const dns_name_t *name, bool tree) { dns_adb_flushname(adb, name); } rcu_read_unlock(); - if (view->resolver != NULL) { - dns_resolver_flushbadcache(view->resolver, name); - } if (view->failcache != NULL) { dns_badcache_flushname(view->failcache, name); }