From 6c3f1f09b2bbf8fb120be48e4f31c0a8c5a90a69 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. (cherry picked from commit 2cb5a6210f334374c601f408c6e7c7ec75ee0949) --- 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 46032c24b3..063ae1e7a9 100644 --- a/bin/delv/delv.c +++ b/bin/delv/delv.c @@ -2193,8 +2193,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 1f7e6baa78..d8a8194c7c 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -6572,8 +6572,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 e1b76cca4e..c7943be4a4 100644 --- a/bin/tests/system/pipelined/pipequeries.c +++ b/bin/tests/system/pipelined/pipequeries.c @@ -284,7 +284,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 ad7005a21f..063d779f4f 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 1ba6baf714..b2bd15fa8a 100644 --- a/fuzz/dns_message_checksig.c +++ b/fuzz/dns_message_checksig.c @@ -183,7 +183,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 02932e8a94..fab923f15e 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 746757cdc3..ab50ed8a69 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -10031,7 +10031,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); @@ -10744,12 +10744,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 a33dc7f504..dff81eddf9 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -86,9 +86,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]; @@ -150,7 +150,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 3d4f6c1f8d..a51b82b889 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -1073,7 +1073,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 34487e4242..1b1c711f8b 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 de7312ae5fabbf98a036eca70adf20168f2fac6a 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. (cherry picked from commit b61739836db9d72262e51596ea07ff8571a541f6) --- 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 d8a8194c7c..1ed94e9dbc 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -253,7 +253,6 @@ struct dumpcontext { bool dumpcache; bool dumpzones; bool dumpadb; - bool dumpbad; bool dumpexpired; bool dumpfail; FILE *fp; @@ -4936,9 +4935,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, cfg_obj_log(obj, named_g_lctx, 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. @@ -10834,7 +10831,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), @@ -11755,8 +11752,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); } @@ -11770,10 +11767,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); @@ -11870,7 +11863,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, }; @@ -11898,14 +11890,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); @@ -11918,7 +11908,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); } @@ -12599,9 +12588,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 90770fd8f0..897035672e 100644 --- a/lib/dns/include/dns/resolver.h +++ b/lib/dns/include/dns/resolver.h @@ -387,24 +387,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); @@ -523,58 +505,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 ab50ed8a69..549299df83 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -40,7 +40,6 @@ #include #include -#include #include #include #include @@ -253,9 +252,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 { @@ -596,8 +592,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; @@ -661,7 +655,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 @@ -1784,7 +1778,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: @@ -2872,7 +2866,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: @@ -2939,7 +2933,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); @@ -3414,7 +3408,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; @@ -3707,24 +3701,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; @@ -3983,7 +3965,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; @@ -4018,7 +4000,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; @@ -4289,7 +4271,7 @@ resume_qmin(void *arg) { fctx_cleanup(fctx); } - fctx_try(fctx, true, false); + fctx_try(fctx, true); cleanup: if (result != ISC_R_SUCCESS) { @@ -4415,7 +4397,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); @@ -5303,24 +5285,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: @@ -9514,7 +9482,7 @@ rctx_nextserver(respctx_t *rctx, dns_message_t *message, /* * Try again. */ - fctx_try(fctx, retrying, false); + fctx_try(fctx, retrying); } /* @@ -9937,7 +9905,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); @@ -10031,8 +9998,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); @@ -10691,18 +10656,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) { @@ -10726,45 +10679,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 982efc4c5f..8778a4df91 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -846,22 +846,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 dff81eddf9..19cc14ea10 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -1475,9 +1475,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); } @@ -1511,9 +1508,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); } @@ -1524,9 +1518,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); }