Leniency for target discovery when under load (for NRDelegation changes) (#764)

gthess · web-flow · commit f1d263a318af · 2022-10-04T22:21:08.000+02:00
* - Introduce leniency for target discovery when under load. * - Allow for easier testing (to be reverted). * - Happy compiler. * - Precheck access to target_fetch_policy. * - Do not mark a nameserver as resolved when one of A/AAAA is negative. * - Update fetch_glue.rpl test for (possible) outstanding queries. * - Update fetch_glue_cname.rpl test for possible outstanding queries. * - Better fix for fetch_glue_cname.rpl. * - Fix iter_emptydp_for_glue.rpl to match the referral. * - Disabled the nxns tests for now (to be reverted). * - Update iter_recurse.rpl for possible outstanding queries. * Revert "- Disabled the nxns tests for now (to be reverted)." This reverts commit 34a9c13. * Revert "- Allow for easier testing (to be reverted)." This reverts commit b6dfe35.
diff --git a/iterator/iterator.c b/iterator/iterator.c
@@ -255,9 +255,9 @@ error_supers(struct module_qstate* qstate, int id, struct module_qstate* super)
 				log_err("out of memory adding missing");
 		}
 		delegpt_mark_neg(dpns, qstate->qinfo.qtype);
-		dpns->resolved = 1; /* mark as failed */
 		if((dpns->got4 == 2 || !ie->supports_ipv4) &&
 			(dpns->got6 == 2 || !ie->supports_ipv6)) {
+			dpns->resolved = 1; /* mark as failed */
 			target_count_increase_nx(super_iq, 1);
 		}
 	}
@@ -2264,6 +2264,7 @@ processQueryTargets(struct module_qstate* qstate, struct iter_qstate* iq,
 	size_t qout_orig_len = 0;
 	int sq_check_ratelimit = 1;
 	int sq_was_ratelimited = 0;
+	int can_do_promisc = 0;
 
 	/* NOTE: a request will encounter this state for each target it
 	 * needs to send a query to. That is, at least one per referral,
@@ -2591,12 +2592,12 @@ processQueryTargets(struct module_qstate* qstate, struct iter_qstate* iq,
 	if(iq->depth < ie->max_dependency_depth
 		&& iq->num_target_queries == 0
 		&& (!iq->target_count || iq->target_count[TARGET_COUNT_NX]==0)
-		&& iq->sent_count < TARGET_FETCH_STOP
-		/* if the mesh query list is full, then do not waste cpu
-		 * and sockets to fetch promiscuous targets. They can be
-		 * looked up when needed. */
-		&& !mesh_jostle_exceeded(qstate->env->mesh)
-		) {
+		&& iq->sent_count < TARGET_FETCH_STOP) {
+		can_do_promisc = 1;
+	}
+	/* if the mesh query list is full, then do not waste cpu and sockets to
+	 * fetch promiscuous targets. They can be looked up when needed. */
+	if(can_do_promisc && !mesh_jostle_exceeded(qstate->env->mesh)) {
 		tf_policy = ie->target_fetch_policy[iq->depth];
 	}
 
@@ -2768,6 +2769,37 @@ processQueryTargets(struct module_qstate* qstate, struct iter_qstate* iq,
 		return 0;
 	}
 
+	/* We have a target. We could have created promiscuous target
+	 * queries but we are currently under pressure (mesh_jostle_exceeded).
+	 * If we are configured to allow promiscuous target queries and haven't
+	 * gone out to the network for a target query for this delegation, then
+	 * it is possible to slip in a promiscuous one with a 1/10 chance. */
+	if(can_do_promisc && tf_policy == 0 && iq->depth == 0
+		&& iq->depth < ie->max_dependency_depth
+		&& ie->target_fetch_policy[iq->depth] != 0
+		&& iq->dp_target_count == 0
+		&& !ub_random_max(qstate->env->rnd, 10)) {
+		int extra = 0;
+		verbose(VERB_ALGO, "available target exists in cache but "
+			"attempt to get extra 1 target");
+		(void)query_for_targets(qstate, iq, ie, id, 1, &extra);
+		/* errors ignored, these targets are not strictly necessary for
+		* this result, we do not have to reply with SERVFAIL */
+		if(extra > 0) {
+			iq->num_target_queries += extra;
+			target_count_increase(iq, extra);
+			check_waiting_queries(iq, qstate, id);
+			/* undo qname minimise step because we'll get back here
+			 * to do it again */
+			if(qout_orig && iq->minimise_count > 0) {
+				iq->minimise_count--;
+				iq->qinfo_out.qname = qout_orig;
+				iq->qinfo_out.qname_len = qout_orig_len;
+			}
+			return 0;
+		}
+	}
+
 	/* Do not check ratelimit for forwarding queries or if we already got a
 	 * pass. */
 	sq_check_ratelimit = (!(iq->chase_flags & BIT_RD) && !iq->ratelimit_ok);
@@ -3531,12 +3563,13 @@ processTargetResponse(struct module_qstate* qstate, int id,
 	} else {
 		verbose(VERB_ALGO, "iterator TargetResponse failed");
 		delegpt_mark_neg(dpns, qstate->qinfo.qtype);
-		dpns->resolved = 1; /* fail the target */
 		if((dpns->got4 == 2 || !ie->supports_ipv4) &&
-			(dpns->got6 == 2 || !ie->supports_ipv6) &&
+			(dpns->got6 == 2 || !ie->supports_ipv6)) {
+			dpns->resolved = 1; /* fail the target */
 			/* do not count cached answers */
-			(qstate->reply_origin && qstate->reply_origin->len != 0)) {
-			target_count_increase_nx(foriq, 1);
+			if(qstate->reply_origin && qstate->reply_origin->len != 0) {
+				target_count_increase_nx(foriq, 1);
+			}
 		}
 	}
 }
diff --git a/iterator/iterator.h b/iterator/iterator.h
@@ -324,7 +324,7 @@ struct iter_qstate {
 	/** the number of times this query has been restarted. */
 	int query_restart_count;
 
-	/** the number of times this query as followed a referral. */
+	/** the number of times this query has followed a referral. */
 	int referral_count;
 
 	/** number of queries fired off */
diff --git a/testdata/fetch_glue.rpl b/testdata/fetch_glue.rpl
@@ -176,36 +176,7 @@ SECTION ADDITIONAL
 ns.example.com.		IN 	A	1.2.3.4
 ENTRY_END
 
-; due to ordering of answer packets, this is still outstanding, remove it
-STEP 21 CHECK_OUT_QUERY
-ENTRY_BEGIN
-ADJUST copy_id
-MATCH qname qtype
-REPLY QR
-SECTION QUESTION
-ns.example.com IN AAAA
-ENTRY_END
-
-; some more recursion needed.
-; to finish the NS query
-STEP 40 QUERY
-ENTRY_BEGIN
-REPLY RD
-SECTION QUESTION
-. IN NS
-ENTRY_END
-
-STEP 41 CHECK_ANSWER
-ENTRY_BEGIN
-MATCH all
-REPLY QR RD RA NOERROR
-SECTION QUESTION
-. IN NS
-SECTION ANSWER
-. IN NS	K.ROOT-SERVERS.NET.
-SECTION AUTHORITY
-SECTION ADDITIONAL
-K.ROOT-SERVERS.NET.	IN	A	193.0.14.129
-ENTRY_END
+; let (possible) outstanding queries finish resolving
+STEP 21 TRAFFIC
 
 SCENARIO_END
diff --git a/testdata/fetch_glue_cname.rpl b/testdata/fetch_glue_cname.rpl
@@ -155,7 +155,7 @@ ENTRY_END
 ENTRY_BEGIN
 MATCH opcode qtype qname
 ADJUST copy_id
-REPLY QR NOERROR
+REPLY QR AA NOERROR
 SECTION QUESTION
 ns.example.com. IN AAAA
 SECTION AUTHORITY
@@ -188,36 +188,7 @@ SECTION ADDITIONAL
 ns.example.com.		IN 	A	1.2.3.4
 ENTRY_END
 
-; due to ordering of answer packets, this is still outstanding, remove it
-STEP 21 CHECK_OUT_QUERY
-ENTRY_BEGIN
-ADJUST copy_id
-MATCH qname qtype
-REPLY QR
-SECTION QUESTION
-ns.example.com IN AAAA
-ENTRY_END
-
-; some more recursion needed.
-; to finish the NS query
-STEP 40 QUERY
-ENTRY_BEGIN
-REPLY RD
-SECTION QUESTION
-. IN NS
-ENTRY_END
-
-STEP 41 CHECK_ANSWER
-ENTRY_BEGIN
-MATCH all
-REPLY QR RD RA NOERROR
-SECTION QUESTION
-. IN NS
-SECTION ANSWER
-. IN NS	K.ROOT-SERVERS.NET.
-SECTION AUTHORITY
-SECTION ADDITIONAL
-K.ROOT-SERVERS.NET.	IN	A	193.0.14.129
-ENTRY_END
+; let (possible) outstanding queries finish resolving
+STEP 21 TRAFFIC
 
 SCENARIO_END
diff --git a/testdata/iter_emptydp_for_glue.rpl b/testdata/iter_emptydp_for_glue.rpl
@@ -164,11 +164,11 @@ a.gtld-servers.net.	IN 	A	192.5.6.30
 ENTRY_END
 
 ENTRY_BEGIN
-MATCH opcode qname
+MATCH opcode subdomain
 ADJUST copy_id copy_query
 REPLY QR NOERROR
 SECTION QUESTION
-ns.example.org. IN A
+example.org. IN A
 SECTION AUTHORITY
 example.org. NS ns.example.net.
 example.org. NS ns.example.org.
diff --git a/testdata/iter_recurse.rpl b/testdata/iter_recurse.rpl
@@ -216,14 +216,7 @@ example.com.	IN NS	ns.example.net.
 ;ns.example.net 	IN A 	1.2.3.44
 ENTRY_END
 
-; due to ordering of answer packets, this is still outstanding, remove it
-STEP 21 CHECK_OUT_QUERY
-ENTRY_BEGIN
-ADJUST copy_id
-MATCH qname qtype
-REPLY QR
-SECTION QUESTION
-ns.example.net IN AAAA
-ENTRY_END
+; let (possible) outstanding queries finish resolving
+STEP 21 TRAFFIC
 
 SCENARIO_END
