Merge pull request #1241 from NLnetLabs/bugfix/keep-probing-max-rtt

gthess · web-flow · commit edb20a2ad274 · 2025-02-19T13:44:40.000+01:00
Fix infra-keep-probing for low infra-cache-max-rtt values
diff --git a/Makefile.in b/Makefile.in
@@ -179,11 +179,11 @@ testcode/unitlruhash.c testcode/unitmain.c testcode/unitmsgparse.c \
 testcode/unitneg.c testcode/unitregional.c testcode/unitslabhash.c \
 testcode/unitverify.c testcode/readhex.c testcode/testpkts.c testcode/unitldns.c \
 testcode/unitecs.c testcode/unitauth.c testcode/unitzonemd.c \
-testcode/unittcpreuse.c testcode/unitdoq.c
+testcode/unittcpreuse.c testcode/unitdoq.c testcode/unitinfra.c
 UNITTEST_OBJ=unitanchor.lo unitdname.lo unitlruhash.lo unitmain.lo \
 unitmsgparse.lo unitneg.lo unitregional.lo unitslabhash.lo unitverify.lo \
 readhex.lo testpkts.lo unitldns.lo unitecs.lo unitauth.lo unitzonemd.lo \
-unittcpreuse.lo unitdoq.lo
+unittcpreuse.lo unitdoq.lo unitinfra.lo
 UNITTEST_OBJ_LINK=$(UNITTEST_OBJ) worker_cb.lo $(COMMON_OBJ) $(SLDNS_OBJ) \
 $(COMPAT_OBJ)
 DAEMON_SRC=daemon/acl_list.c daemon/cachedump.c daemon/daemon.c \
@@ -1261,6 +1261,7 @@ unitzonemd.lo unitzonemd.o: $(srcdir)/testcode/unitzonemd.c config.h $(srcdir)/u
  $(srcdir)/validator/val_anchor.h
 unittcpreuse.lo unittcpreuse.o: $(srcdir)/testcode/unittcpreuse.c config.h $(srcdir)/services/outside_network.h \
 $(srcdir)/util/random.h
+unitinfra.lo unitinfra.o: $(srcdir)/testcode/unitinfra.c config.h $(srcdir)/util/config_file.h $(srcdir)/util/net_help.h $(srcdir)/iterator/iterator.h
 acl_list.lo acl_list.o: $(srcdir)/daemon/acl_list.c config.h $(srcdir)/daemon/acl_list.h \
  $(srcdir)/util/storage/dnstree.h $(srcdir)/util/rbtree.h $(srcdir)/services/view.h $(srcdir)/util/locks.h \
  $(srcdir)/util/log.h $(srcdir)/util/regional.h $(srcdir)/util/config_file.h $(srcdir)/util/net_help.h \
diff --git a/iterator/iterator.c b/iterator/iterator.c
@@ -78,6 +78,8 @@ int UNKNOWN_SERVER_NICENESS = 376;
 int USEFUL_SERVER_TOP_TIMEOUT = 120000;
 /* Equals USEFUL_SERVER_TOP_TIMEOUT*4 */
 int BLACKLIST_PENALTY = (120000*4);
+/** Timeout when only a single probe query per IP is allowed. */
+int PROBE_MAXRTO = PROBE_MAXRTO_DEFAULT; /* in msec */
 
 static void target_count_increase_nx(struct iter_qstate* iq, int num);
 
diff --git a/services/cache/infra.c b/services/cache/infra.c
@@ -52,14 +52,6 @@
 #include "util/config_file.h"
 #include "iterator/iterator.h"
 
-/** Timeout when only a single probe query per IP is allowed. */
-#define PROBE_MAXRTO 12000 /* in msec */
-
-/** number of timeouts for a type when the domain can be blocked ;
- * even if another type has completely rtt maxed it, the different type
- * can do this number of packets (until those all timeout too) */
-#define TIMEOUT_COUNT_MAX 3
-
 /** ratelimit value for delegation point */
 int infra_dp_ratelimit = 0;
 
@@ -76,7 +68,8 @@ int infra_ip_ratelimit_cookie = 0;
  * blacklisted servers stay blacklisted if this is chosen.
  * If USEFUL_SERVER_TOP_TIMEOUT is below 1000 (configured via RTT_MAX_TIMEOUT,
  * infra-cache-max-rtt) change it to just above the RTT_BAND. */
-static int still_useful_timeout()
+int
+still_useful_timeout()
 {
 	return
 	USEFUL_SERVER_TOP_TIMEOUT < 1000 ||
diff --git a/services/cache/infra.h b/services/cache/infra.h
@@ -52,6 +52,19 @@
 struct slabhash;
 struct config_file;
 
+/** number of timeouts for a type when the domain can be blocked ;
+ * even if another type has completely rtt maxed it, the different type
+ * can do this number of packets (until those all timeout too) */
+#define TIMEOUT_COUNT_MAX 3
+
+
+/** Timeout when only a single probe query per IP is allowed.
+ *  Any RTO above this number is considered a probe.
+ *  It is synchronized (caped) with USEFUL_SERVER_TOP_TIMEOUT so that probing
+ *  keeps working even if that configurable number drops below the default
+ *  12000 ms of probing. */
+extern int PROBE_MAXRTO;
+
 /**
  * Host information kept for every server, per zone.
  */
@@ -502,4 +515,7 @@ void infra_wait_limit_inc(struct infra_cache* infra, struct comm_reply* rep,
 void infra_wait_limit_dec(struct infra_cache* infra, struct comm_reply* rep,
 	struct config_file* cfg);
 
+/** exported for unit test */
+int still_useful_timeout();
+
 #endif /* SERVICES_CACHE_INFRA_H */
diff --git a/testcode/unitinfra.c b/testcode/unitinfra.c
@@ -0,0 +1,209 @@
+/*
+ * testcode/unitinfra.c - unit test for infra cache.
+ *
+ * Copyright (c) 2025, NLnet Labs. All rights reserved.
+ *
+ * This software is open source.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * Neither the name of the NLNET LABS nor the names of its contributors may
+ * be used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
+ * TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+/**
+ * \file
+ * Tests the infra functionality.
+ */
+
+#include "config.h"
+#include "testcode/unitmain.h"
+#include "iterator/iterator.h"
+#include "services/cache/infra.h"
+#include "util/config_file.h"
+#include "util/net_help.h"
+
+/* lookup and get key and data structs easily */
+static struct infra_data* infra_lookup_host(struct infra_cache* infra,
+	struct sockaddr_storage* addr, socklen_t addrlen, uint8_t* zone,
+	size_t zonelen, int wr, time_t now, struct infra_key** k)
+{
+	struct infra_data* d;
+	struct lruhash_entry* e = infra_lookup_nottl(infra, addr, addrlen,
+		zone, zonelen, wr);
+	if(!e) return NULL;
+	d = (struct infra_data*)e->data;
+	if(d->ttl < now) {
+		lock_rw_unlock(&e->lock);
+		return NULL;
+	}
+	*k = (struct infra_key*)e->key;
+	return d;
+}
+
+static void test_keep_probing(struct infra_cache* slab,
+	struct config_file* cfg, struct sockaddr_storage one, socklen_t onelen,
+	uint8_t* zone, size_t zonelen, time_t *now, int keep_probing,
+	int rtt_max_timeout)
+{
+	uint8_t edns_lame;
+	int vs, to, lame, dnsseclame, reclame, probedelay;
+	struct infra_key* k;
+	struct infra_data* d;
+
+	/* configure */
+	cfg->infra_cache_max_rtt = rtt_max_timeout;
+	config_apply_max_rtt(rtt_max_timeout);
+	slab->infra_keep_probing = keep_probing;
+
+	/* expired previous entry */
+	*now += cfg->host_ttl + 10;
+	unit_assert( infra_host(slab, &one, onelen, zone, zonelen,
+			*now, &vs, &edns_lame, &to) );
+
+	/* simulate timeouts until the USEFUL_SERVER_TOP_TIMEOUT is reached */
+	while(to < USEFUL_SERVER_TOP_TIMEOUT) {
+		unit_assert( infra_rtt_update(slab, &one, onelen, zone, zonelen,
+			LDNS_RR_TYPE_A, -1, to, *now) );
+		unit_assert( infra_host(slab, &one, onelen, zone, zonelen,
+			*now, &vs, &edns_lame, &to) );
+		unit_assert( vs == 0 && to <= USEFUL_SERVER_TOP_TIMEOUT && edns_lame == 0 );
+	}
+	unit_assert( vs == 0 && to == USEFUL_SERVER_TOP_TIMEOUT && edns_lame == 0 );
+
+	/* don't let the record expire */
+	unit_assert( (d=infra_lookup_host(slab, &one, onelen, zone, zonelen, 0, *now, &k)) );
+	unit_assert( d->timeout_A >= TIMEOUT_COUNT_MAX );
+	unit_assert( d->probedelay > 0 );
+	probedelay = d->probedelay;
+	lock_rw_unlock(&k->entry.lock);
+	cfg->host_ttl = cfg->host_ttl + *now < probedelay
+		?cfg->host_ttl :probedelay + 10;
+
+	/* advance time and check that probing is as expected; we already had a
+	 * lot of A timeouts (checked above). */
+	*now = probedelay;
+	unit_assert( infra_get_lame_rtt(slab, &one, onelen, zone, zonelen,
+		LDNS_RR_TYPE_A, &lame, &dnsseclame, &reclame, &to, *now) );
+	unit_assert( lame == 0 && dnsseclame == 0 && reclame == 0
+		&& to == keep_probing ?still_useful_timeout() :USEFUL_SERVER_TOP_TIMEOUT);
+}
+
+/** test host cache */
+void infra_test(void)
+{
+	struct sockaddr_storage one;
+	socklen_t onelen;
+	uint8_t* zone = (uint8_t*)"\007example\003com\000";
+	size_t zonelen = 13;
+	struct infra_cache* slab;
+	struct config_file* cfg = config_create();
+	time_t now = 0;
+	uint8_t edns_lame;
+	int vs, to;
+	struct infra_key* k;
+	struct infra_data* d;
+	int init = UNKNOWN_SERVER_NICENESS;
+	int default_max_rtt = USEFUL_SERVER_TOP_TIMEOUT;
+
+	unit_show_feature("infra cache");
+	unit_assert(ipstrtoaddr("127.0.0.1", 53, &one, &onelen));
+
+	slab = infra_create(cfg);
+	/* insert new record */
+	unit_assert( infra_host(slab, &one, onelen, zone, zonelen, now,
+		&vs, &edns_lame, &to) );
+	unit_assert( vs == 0 && to == init && edns_lame == 0 );
+
+	/* simulate no answer */
+	unit_assert( infra_rtt_update(slab, &one, onelen, zone, zonelen, LDNS_RR_TYPE_A, -1, init, now) );
+	unit_assert( infra_host(slab, &one, onelen, zone, zonelen,
+			now, &vs, &edns_lame, &to) );
+	unit_assert( vs == 0 && to == init*2 && edns_lame == 0 );
+
+	/* simulate EDNS lame */
+	unit_assert( infra_edns_update(slab, &one, onelen, zone, zonelen, -1, now) );
+	unit_assert( infra_host(slab, &one, onelen, zone, zonelen,
+			now, &vs, &edns_lame, &to) );
+	unit_assert( vs == -1 && to == init*2  && edns_lame == 1);
+
+	/* simulate cache expiry */
+	now += cfg->host_ttl + 10;
+	unit_assert( infra_host(slab, &one, onelen, zone, zonelen,
+			now, &vs, &edns_lame, &to) );
+	unit_assert( vs == 0 && to == init && edns_lame == 0 );
+
+	/* simulate no lame answer */
+	unit_assert( infra_set_lame(slab, &one, onelen,
+		zone, zonelen,  now, 0, 0, LDNS_RR_TYPE_A) );
+	unit_assert( (d=infra_lookup_host(slab, &one, onelen, zone, zonelen, 0, now, &k)) );
+	unit_assert( d->ttl == now+cfg->host_ttl );
+	unit_assert( d->edns_version == 0 );
+	unit_assert(!d->isdnsseclame && !d->rec_lame && d->lame_type_A &&
+		!d->lame_other);
+	lock_rw_unlock(&k->entry.lock);
+
+	/* test merge of data */
+	unit_assert( infra_set_lame(slab, &one, onelen,
+		zone, zonelen,  now, 0, 0, LDNS_RR_TYPE_AAAA) );
+	unit_assert( (d=infra_lookup_host(slab, &one, onelen, zone, zonelen, 0, now, &k)) );
+	unit_assert(!d->isdnsseclame && !d->rec_lame && d->lame_type_A &&
+		d->lame_other);
+	lock_rw_unlock(&k->entry.lock);
+
+	/* test that noEDNS cannot overwrite known-yesEDNS */
+	now += cfg->host_ttl + 10;
+	unit_assert( infra_host(slab, &one, onelen, zone, zonelen,
+			now, &vs, &edns_lame, &to) );
+	unit_assert( vs == 0 && to == init && edns_lame == 0 );
+
+	unit_assert( infra_edns_update(slab, &one, onelen, zone, zonelen, 0, now) );
+	unit_assert( infra_host(slab, &one, onelen, zone, zonelen,
+			now, &vs, &edns_lame, &to) );
+	unit_assert( vs == 0 && to == init && edns_lame == 1 );
+
+	unit_assert( infra_edns_update(slab, &one, onelen, zone, zonelen, -1, now) );
+	unit_assert( infra_host(slab, &one, onelen, zone, zonelen,
+			now, &vs, &edns_lame, &to) );
+	unit_assert( vs == 0 && to == init && edns_lame == 1 );
+
+	unit_show_feature("infra cache probing (keep-probing off, default infra-cache-max-rtt)");
+	test_keep_probing(slab, cfg, one, onelen, zone, zonelen, &now, 0, default_max_rtt);
+
+	unit_show_feature("infra cache probing (keep-probing on,  default infra-cache-max-rtt)");
+	test_keep_probing(slab, cfg, one, onelen, zone, zonelen, &now, 1, default_max_rtt);
+
+	unit_show_feature("infra cache probing (keep-probing off, low infra-cache-max-rtt)");
+	test_keep_probing(slab, cfg, one, onelen, zone, zonelen, &now, 0, 3000);
+
+	unit_show_feature("infra cache probing (keep-probing on,  low infra-cache-max-rtt)");
+	test_keep_probing(slab, cfg, one, onelen, zone, zonelen, &now, 1, 3000);
+
+	/* Re-apply defaults for other unit tests that follow */
+	config_apply_max_rtt(default_max_rtt);
+
+	infra_delete(slab);
+	config_delete(cfg);
+}
diff --git a/testcode/unitmain.c b/testcode/unitmain.c
@@ -433,103 +433,6 @@ rtt_test(void)
 	unit_assert(UB_STATS_BUCKET_NUM == NUM_BUCKETS_HIST);
 }
 
-#include "services/cache/infra.h"
-
-/* lookup and get key and data structs easily */
-static struct infra_data* infra_lookup_host(struct infra_cache* infra,
-	struct sockaddr_storage* addr, socklen_t addrlen, uint8_t* zone,
-	size_t zonelen, int wr, time_t now, struct infra_key** k)
-{
-	struct infra_data* d;
-	struct lruhash_entry* e = infra_lookup_nottl(infra, addr, addrlen,
-		zone, zonelen, wr);
-	if(!e) return NULL;
-	d = (struct infra_data*)e->data;
-	if(d->ttl < now) {
-		lock_rw_unlock(&e->lock);
-		return NULL;
-	}
-	*k = (struct infra_key*)e->key;
-	return d;
-}
-
-/** test host cache */
-static void
-infra_test(void)
-{
-	struct sockaddr_storage one;
-	socklen_t onelen;
-	uint8_t* zone = (uint8_t*)"\007example\003com\000";
-	size_t zonelen = 13;
-	struct infra_cache* slab;
-	struct config_file* cfg = config_create();
-	time_t now = 0;
-	uint8_t edns_lame;
-	int vs, to;
-	struct infra_key* k;
-	struct infra_data* d;
-	int init = 376;
-
-	unit_show_feature("infra cache");
-	unit_assert(ipstrtoaddr("127.0.0.1", 53, &one, &onelen));
-
-	slab = infra_create(cfg);
-	unit_assert( infra_host(slab, &one, onelen, zone, zonelen, now,
-		&vs, &edns_lame, &to) );
-	unit_assert( vs == 0 && to == init && edns_lame == 0 );
-
-	unit_assert( infra_rtt_update(slab, &one, onelen, zone, zonelen, LDNS_RR_TYPE_A, -1, init, now) );
-	unit_assert( infra_host(slab, &one, onelen, zone, zonelen, 
-			now, &vs, &edns_lame, &to) );
-	unit_assert( vs == 0 && to == init*2 && edns_lame == 0 );
-
-	unit_assert( infra_edns_update(slab, &one, onelen, zone, zonelen, -1, now) );
-	unit_assert( infra_host(slab, &one, onelen, zone, zonelen, 
-			now, &vs, &edns_lame, &to) );
-	unit_assert( vs == -1 && to == init*2  && edns_lame == 1);
-
-	now += cfg->host_ttl + 10;
-	unit_assert( infra_host(slab, &one, onelen, zone, zonelen, 
-			now, &vs, &edns_lame, &to) );
-	unit_assert( vs == 0 && to == init && edns_lame == 0 );
-	
-	unit_assert( infra_set_lame(slab, &one, onelen,
-		zone, zonelen,  now, 0, 0, LDNS_RR_TYPE_A) );
-	unit_assert( (d=infra_lookup_host(slab, &one, onelen, zone, zonelen, 0, now, &k)) );
-	unit_assert( d->ttl == now+cfg->host_ttl );
-	unit_assert( d->edns_version == 0 );
-	unit_assert(!d->isdnsseclame && !d->rec_lame && d->lame_type_A &&
-		!d->lame_other);
-	lock_rw_unlock(&k->entry.lock);
-
-	/* test merge of data */
-	unit_assert( infra_set_lame(slab, &one, onelen,
-		zone, zonelen,  now, 0, 0, LDNS_RR_TYPE_AAAA) );
-	unit_assert( (d=infra_lookup_host(slab, &one, onelen, zone, zonelen, 0, now, &k)) );
-	unit_assert(!d->isdnsseclame && !d->rec_lame && d->lame_type_A &&
-		d->lame_other);
-	lock_rw_unlock(&k->entry.lock);
-
-	/* test that noEDNS cannot overwrite known-yesEDNS */
-	now += cfg->host_ttl + 10;
-	unit_assert( infra_host(slab, &one, onelen, zone, zonelen, 
-			now, &vs, &edns_lame, &to) );
-	unit_assert( vs == 0 && to == init && edns_lame == 0 );
-
-	unit_assert( infra_edns_update(slab, &one, onelen, zone, zonelen, 0, now) );
-	unit_assert( infra_host(slab, &one, onelen, zone, zonelen, 
-			now, &vs, &edns_lame, &to) );
-	unit_assert( vs == 0 && to == init && edns_lame == 1 );
-
-	unit_assert( infra_edns_update(slab, &one, onelen, zone, zonelen, -1, now) );
-	unit_assert( infra_host(slab, &one, onelen, zone, zonelen, 
-			now, &vs, &edns_lame, &to) );
-	unit_assert( vs == 0 && to == init && edns_lame == 1 );
-
-	infra_delete(slab);
-	config_delete(cfg);
-}
-
 #include "util/edns.h"
 /* Complete version-invalid client cookie; needs a new one.
  * Based on edns_cookie_rfc9018_a2 */
diff --git a/testcode/unitmain.h b/testcode/unitmain.h
diff --git a/util/config_file.c b/util/config_file.c
diff --git a/util/config_file.h b/util/config_file.h
