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.

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

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
 

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

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