From: Ondřej Surý <ondrej@isc.org>
Date: Fri, 22 Nov 2024 14:10:26 +0000 (+0100)
Subject: Remove dns_badcache usage in the resolver (lame-ttl)
X-Git-Tag: v9.21.3~7^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b61739836db9d72262e51596ea07ff8571a541f6;p=thirdparty%2Fbind9.git

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.
---

diff --git a/bin/named/server.c b/bin/named/server.c
index ac05082489a..4b624d7ebf7 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 75f92b63f7b..daa7ef2cff6 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 <name,type> 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
- * <name,type>.
- *
- * 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 294d173a369..cd5a780ad8c 100644
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -40,7 +40,6 @@
 
 #include <dns/acl.h>
 #include <dns/adb.h>
-#include <dns/badcache.h>
 #include <dns/cache.h>
 #include <dns/db.h>
 #include <dns/dispatch.h>
@@ -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(&copy, 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(&copy, 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 a1934169d32..aafe5147d06 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 124dba3216d..5251f7a4051 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);
 		}