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 b61739836d)

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));
 	}
 

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

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>
@@ -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(&copy, 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(&copy, 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) {

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,

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);
 		}