From bb655fdee67e4381ead06793137efdea2e206994 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20=C4=8Cun=C3=A1t?= <vladimir.cunat@nic.cz>
Date: Tue, 22 Dec 2020 11:29:39 +0100
Subject: [PATCH] lib/selection: minor refactorings and comments

Small things I've noticed while reading it all.
- line breaks: I believe <90 is OK, as usually the attempts to reduce
  lengths impair readability
- avoid unnecessary casts; usually the type was visible
  on the same line anyway
- avoid `|` on booleans
- one block gets de-indented (often badly shown in diffs)
- no need for UNRECOVERABLE_ERRORS in a header (and a weird one, too)
- recoverability from failed assertions (in case they're turned off)
---
 lib/layer/iterate.c     |   6 +-
 lib/selection.c         |  71 ++++++++++++---------
 lib/selection.h         |  21 ++++--
 lib/selection_forward.c |  15 ++---
 lib/selection_iter.c    | 138 +++++++++++++++++-----------------------
 lib/selection_iter.h    |  24 +------
 6 files changed, 124 insertions(+), 151 deletions(-)

diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c
index 791a6849c..aedaa411a 100644
--- a/lib/layer/iterate.c
+++ b/lib/layer/iterate.c
@@ -1131,10 +1131,8 @@ static int resolve(kr_layer_t *ctx, knot_pkt_t *pkt)
 		break;
 	}
 
-	if (query->server_selection.initialized) {
-		if (selection_error != -1) {
-			query->server_selection.error(query, req->upstream.transport, selection_error);
-		}
+	if (query->server_selection.initialized && selection_error != -1) {
+		query->server_selection.error(query, req->upstream.transport, selection_error);
 	}
 
 	if (ret) {
diff --git a/lib/selection.c b/lib/selection.c
index e6c39a259..8031fb74d 100644
--- a/lib/selection.c
+++ b/lib/selection.c
@@ -31,6 +31,27 @@
 #define EPSILON_NOMIN 1
 #define EPSILON_DENOM 20
 
+/**
+ * If one of the errors set to true is encountered,
+ * there is no point in asking this server again.
+ */
+static const bool UNRECOVERABLE_ERRORS[] = {
+	[KR_SELECTION_QUERY_TIMEOUT] = false,
+	[KR_SELECTION_TLS_HANDSHAKE_FAILED] = false,
+	[KR_SELECTION_TCP_CONNECT_FAILED] = false,
+	[KR_SELECTION_TCP_CONNECT_TIMEOUT] = false,
+	[KR_SELECTION_REFUSED] = true,
+	[KR_SELECTION_SERVFAIL] = true,
+	[KR_SELECTION_FORMERROR] = false,
+	[KR_SELECTION_NOTIMPL] = true,
+	[KR_SELECTION_OTHER_RCODE] = true,
+	[KR_SELECTION_TRUNCATED] = false,
+	[KR_SELECTION_DNSSEC_ERROR] = true,
+	[KR_SELECTION_LAME_DELEGATION] = true,
+	[KR_SELECTION_BAD_CNAME] = true,
+};
+
+
 /* Simple cache interface follows */
 
 static knot_db_val_t cache_key(const uint8_t *ip, size_t len)
@@ -51,8 +72,7 @@ static knot_db_val_t cache_key(const uint8_t *ip, size_t len)
 /* First value of timeout will be calculated as SRTT+4*DEFAULT_TIMEOUT
  * by calc_timeout(), so it'll be equal to DEFAULT_TIMEOUT. */
 static const struct rtt_state default_rtt_state = { .srtt = 0,
-						    .variance =
-							    DEFAULT_TIMEOUT / 4,
+						    .variance = DEFAULT_TIMEOUT / 4,
 						    .consecutive_timeouts = 0,
 						    .dead_since = 0 };
 
@@ -143,8 +163,7 @@ static unsigned back_off_timeout(uint32_t to, int pow)
  * RFC6298, sec. 2. */
 static unsigned calc_timeout(struct rtt_state state)
 {
-	int32_t timeout =
-		state.srtt + MAX(4 * state.variance, MINIMAL_TIMEOUT_ADDITION);
+	int32_t timeout = state.srtt + MAX(4 * state.variance, MINIMAL_TIMEOUT_ADDITION);
 	return back_off_timeout(timeout, state.consecutive_timeouts);
 }
 
@@ -255,8 +274,8 @@ void update_address_state(struct address_state *state, uint8_t *address,
 
 static int cmp_choices(const void *a, const void *b)
 {
-	struct choice *a_ = (struct choice *)a;
-	struct choice *b_ = (struct choice *)b;
+	const struct choice *a_ = a;
+	const struct choice *b_ = b;
 
 	int diff;
 	/* Address with no RTT information is better than address
@@ -302,8 +321,7 @@ struct kr_transport *select_transport(struct choice choices[], int choices_len,
 		return NULL;
 	}
 
-	struct kr_transport *transport =
-		mm_alloc(mempool, sizeof(struct kr_transport));
+	struct kr_transport *transport = mm_alloc(mempool, sizeof(struct kr_transport));
 	memset(transport, 0, sizeof(struct kr_transport));
 
 	int choice = 0;
@@ -370,12 +388,11 @@ struct kr_transport *select_transport(struct choice choices[], int choices_len,
 		.ns_name = chosen->address_state->ns_name,
 		.protocol = protocol,
 		.timeout = timeout,
-		.safe_mode =
-			chosen->address_state->errors[KR_SELECTION_FORMERROR],
+		.safe_mode = chosen->address_state->errors[KR_SELECTION_FORMERROR],
 	};
 
-	int port;
-	if (!(port = chosen->port)) {
+	int port = chosen->port;
+	if (!port) {
 		switch (transport->protocol) {
 		case KR_TRANSPORT_TLS:
 			port = KR_DNS_TLS_PORT;
@@ -410,8 +427,7 @@ void update_rtt(struct kr_query *qry, struct address_state *addr_state,
 
 	struct kr_cache *cache = &qry->request->ctx->cache;
 
-	uint8_t *address =
-		ip_to_bytes(&transport->address, transport->address_len);
+	uint8_t *address = ip_to_bytes(&transport->address, transport->address_len);
 	/* This construct is a bit racy since the global state may change
 	 * between calls to `get_rtt_state` and `put_rtt_state`  but we don't
 	 * care that much since it is rare and we only risk slightly suboptimal
@@ -429,7 +445,8 @@ void update_rtt(struct kr_query *qry, struct address_state *addr_state,
 
 	VERBOSE_MSG(
 		qry,
-		"=> id: '%05u' updating: '%s'@'%s' zone cut: '%s' with rtt %u to srtt: %d and variance: %d \n",
+		"=> id: '%05u' updating: '%s'@'%s' zone cut: '%s'"
+		" with rtt %u to srtt: %d and variance: %d \n",
 		qry->id, ns_name, ns_str ? ns_str : "", zonecut_str,
 		rtt, new_rtt_state.srtt, new_rtt_state.variance);
 	}
@@ -444,8 +461,7 @@ static void cache_timeout(const struct kr_transport *transport,
 		return;
 	}
 
-	uint8_t *address =
-		ip_to_bytes(&transport->address, transport->address_len);
+	uint8_t *address = ip_to_bytes(&transport->address, transport->address_len);
 	struct rtt_state old_state = addr_state->rtt_state;
 	struct rtt_state cur_state =
 		get_rtt_state(address, transport->address_len, cache);
@@ -457,8 +473,7 @@ static void cache_timeout(const struct kr_transport *transport,
 		    KR_NS_TIMEOUT_ROW_DEAD) {
 			cur_state.dead_since = kr_now();
 		}
-		put_rtt_state(address, transport->address_len, cur_state,
-			      cache);
+		put_rtt_state(address, transport->address_len, cur_state, cache);
 	} else {
 		/* `get_rtt_state` opens a cache transaction, we have to end it. */
 		kr_cache_commit(cache);
@@ -469,15 +484,12 @@ void error(struct kr_query *qry, struct address_state *addr_state,
 	   const struct kr_transport *transport,
 	   enum kr_selection_error sel_error)
 {
+	assert(sel_error >= KR_SELECTION_OK && sel_error < KR_SELECTION_NUMBER_OF_ERRORS);
 	if (!transport || !addr_state) {
 		/* Answers from cache have NULL transport, ignore them. */
 		return;
 	}
 
-	if (sel_error >= KR_SELECTION_NUMBER_OF_ERRORS) {
-		assert(0);
-	}
-
 	if (sel_error == KR_SELECTION_QUERY_TIMEOUT) {
 		qry->server_selection.local_state->timeouts++;
 		// Make sure the query was chosen by this query
@@ -526,17 +538,17 @@ void error(struct kr_query *qry, struct address_state *addr_state,
 void kr_server_selection_init(struct kr_query *qry)
 {
 	struct knot_mm *mempool = &qry->request->pool;
+	struct local_state *local_state = mm_alloc(mempool, sizeof(struct local_state));
+	memset(local_state, 0, sizeof(struct local_state));
+
 	if (qry->flags.FORWARD || qry->flags.STUB) {
 		qry->server_selection = (struct kr_server_selection){
 			.initialized = true,
 			.choose_transport = forward_choose_transport,
 			.update_rtt = forward_update_rtt,
 			.error = forward_error,
-			.local_state =
-				mm_alloc(mempool, sizeof(struct local_state)),
+			.local_state = local_state,
 		};
-		memset(qry->server_selection.local_state, 0,
-		       sizeof(struct local_state));
 		forward_local_state_alloc(
 			mempool, &qry->server_selection.local_state->private,
 			qry->request);
@@ -546,11 +558,8 @@ void kr_server_selection_init(struct kr_query *qry)
 			.choose_transport = iter_choose_transport,
 			.update_rtt = iter_update_rtt,
 			.error = iter_error,
-			.local_state =
-				mm_alloc(mempool, sizeof(struct local_state)),
+			.local_state = local_state,
 		};
-		memset(qry->server_selection.local_state, 0,
-		       sizeof(struct local_state));
 		iter_local_state_alloc(
 			mempool, &qry->server_selection.local_state->private);
 	}
diff --git a/lib/selection.h b/lib/selection.h
index a5acf88bc..113a468bf 100644
--- a/lib/selection.h
+++ b/lib/selection.h
@@ -6,7 +6,8 @@
 
 /**
  * @file selection.h
- * Provides server selection API (see `kr_server_selection`) and functions common to both implementations.
+ * Provides server selection API (see `kr_server_selection`)
+ * and functions common to both implementations.
  */
 
 #include "lib/cache/api.h"
@@ -43,7 +44,7 @@ enum kr_selection_error {
 	KR_SELECTION_BAD_CNAME,
 
 	/** Leave this last, as it is used as array size. */
-	KR_SELECTION_NUMBER_OF_ERRORS 
+	KR_SELECTION_NUMBER_OF_ERRORS
 };
 
 enum kr_transport_protocol {
@@ -123,12 +124,18 @@ void kr_server_selection_init(struct kr_query *qry);
 KR_EXPORT
 int kr_forward_add_target(struct kr_request *req, const struct sockaddr *sock);
 
+
+
+
+
+/* Below are internal parts shared by ./selection_{forward,iter}.c */
+
 /**
  * To be held per IP address in the global LMDB cache
  */
 struct rtt_state {
-	int32_t srtt;
-	int32_t variance;
+	int32_t srtt; /**< Smoothed RTT, i.e. an estimate of round-trip time. */
+	int32_t variance; /**< An estimate of RTT's standard derivation (not variance). */
 	int32_t consecutive_timeouts;
 	/** Timestamp of pronouncing this IP bad based on KR_NS_TIMEOUT_ROW_DEAD */
 	uint64_t dead_since;
@@ -138,7 +145,7 @@ struct rtt_state {
  * @brief To be held per IP address and locally "inside" query.
  */
 struct address_state {
-	/** Used to distinguish old and valid records in local_state. */
+	/** Used to distinguish old and valid records in local_state; -1 means unusable IP. */
 	unsigned int generation;
 	struct rtt_state rtt_state;
 	knot_dname_t *ns_name;
@@ -185,8 +192,8 @@ struct to_resolve {
  * @param timeouts Number of timeouts that occured in this query (used for exponential backoff)
  * @param mempool Memory context of current request
  * @param tcp Force TCP as transport protocol
- * @param[out] choice_index Optinally index of the chosen transport in the @p choices array is stored here.
- * @return Chosen transport or NULL when no choice is viable
+ * @param[out] choice_index Optionally index of the chosen transport in the @p choices array.
+ * @return Chosen transport (on mempool) or NULL when no choice is viable
  */
 struct kr_transport *select_transport(struct choice choices[], int choices_len,
 				      struct to_resolve unresolved[],
diff --git a/lib/selection_forward.c b/lib/selection_forward.c
index 2f85bcd80..d74141006 100644
--- a/lib/selection_forward.c
+++ b/lib/selection_forward.c
@@ -23,14 +23,12 @@ void forward_local_state_alloc(struct knot_mm *mm, void **local_state,
 	*local_state = mm_alloc(mm, sizeof(struct forward_local_state));
 	memset(*local_state, 0, sizeof(struct forward_local_state));
 
-	struct forward_local_state *forward_state =
-		(struct forward_local_state *)*local_state;
+	struct forward_local_state *forward_state = *local_state;
 	forward_state->targets = &req->selection_context.forwarding_targets;
 
-	forward_state->addr_states = mm_alloc(
-		mm, sizeof(struct address_state) * forward_state->targets->len);
-	memset(forward_state->addr_states, 0,
-	       sizeof(struct address_state) * forward_state->targets->len);
+	size_t as_bytes = sizeof(struct address_state) * forward_state->targets->len;
+	forward_state->addr_states = mm_alloc(mm, as_bytes);
+	memset(forward_state->addr_states, 0, as_bytes);
 }
 
 void forward_choose_transport(struct kr_query *qry,
@@ -77,8 +75,7 @@ void forward_choose_transport(struct kr_query *qry,
 		};
 	}
 
-	bool tcp =
-		qry->flags.TCP | qry->server_selection.local_state->truncated;
+	bool tcp = qry->flags.TCP || qry->server_selection.local_state->truncated;
 	*transport =
 		select_transport(choices, valid, NULL, 0,
 				 qry->server_selection.local_state->timeouts,
@@ -126,4 +123,4 @@ void forward_update_rtt(struct kr_query *qry,
 		&local_state->addr_states[local_state->last_choice_index];
 
 	update_rtt(qry, addr_state, transport, rtt);
-}
\ No newline at end of file
+}
diff --git a/lib/selection_iter.c b/lib/selection_iter.c
index 24fd3ce9b..4e8642d7a 100644
--- a/lib/selection_iter.c
+++ b/lib/selection_iter.c
@@ -12,10 +12,10 @@
 
 #define VERBOSE_MSG(qry, ...) QRVERBOSE((qry), "slct", __VA_ARGS__)
 
-// To be held per query and locally
+/// To be held per query and locally.  Allocations are in the kr_request's mempool.
 struct iter_local_state {
-	trie_t *names;
-	trie_t *addresses;
+	trie_t *names; /// knot_dname_t -> struct iter_name_state *
+	trie_t *addresses; /// IP address -> struct address_state *
 	knot_dname_t *zonecut;
 	/** Used to distinguish old and valid records in tries. */
 	unsigned int generation;
@@ -45,27 +45,15 @@ static struct address_state *get_address_state(struct iter_local_state *local_st
 		return NULL;
 	}
 
-	trie_t *addresses = local_state->addresses;
-	uint8_t *address =
-		ip_to_bytes(&transport->address, transport->address_len);
-
-	trie_val_t *address_state = trie_get_try(addresses, (char *)address,
+	uint8_t *address = ip_to_bytes(&transport->address, transport->address_len);
+	trie_val_t *address_state = trie_get_try(local_state->addresses, (char *)address,
 						 transport->address_len);
-
 	if (!address_state) {
-		if (transport->deduplicated) {
-			/* Transport was chosen by a different query. */
-			return NULL;
-		}
-
-		assert(0);
+		assert(transport->deduplicated);
+		/* Transport was chosen by a different query. */
+		return NULL;
 	}
-	return (struct address_state *)*address_state;
-}
-
-static bool zonecut_changed(knot_dname_t *new, knot_dname_t *old)
-{
-	return knot_dname_cmp(old, new);
+	return *address_state;
 }
 
 static void unpack_state_from_zonecut(struct iter_local_state *local_state,
@@ -81,7 +69,7 @@ static void unpack_state_from_zonecut(struct iter_local_state *local_state,
 		local_state->names = trie_create(mm);
 		local_state->addresses = trie_create(mm);
 	} else {
-		zcut_changed = zonecut_changed(zonecut->name, local_state->zonecut);
+		zcut_changed = !knot_dname_is_equal(zonecut->name, local_state->zonecut);
 	}
 	local_state->zonecut = zonecut->name;
 	local_state->generation++;
@@ -91,11 +79,11 @@ static void unpack_state_from_zonecut(struct iter_local_state *local_state,
 	}
 
 	trie_it_t *it;
-	unsigned int current_generation = local_state->generation;
+	const unsigned int current_generation = local_state->generation;
 
 	for (it = trie_it_begin(zonecut->nsset); !trie_it_finished(it); trie_it_next(it)) {
 		knot_dname_t *dname = (knot_dname_t *)trie_it_key(it, NULL);
-		pack_t *addresses = (pack_t *)*trie_it_val(it);
+		pack_t *addresses = *trie_it_val(it);
 
 		trie_val_t *val = trie_get_ins(local_state->names, (char *)dname,
 					       knot_dname_size(dname));
@@ -104,7 +92,7 @@ static void unpack_state_from_zonecut(struct iter_local_state *local_state,
 			*val = mm_alloc(mm, sizeof(struct iter_name_state));
 			memset(*val, 0, sizeof(struct iter_name_state));
 		}
-		struct iter_name_state *name_state = *(struct iter_name_state **)val;
+		struct iter_name_state *name_state = *val;
 		name_state->generation = current_generation;
 
 		if (zcut_changed) {
@@ -114,11 +102,7 @@ static void unpack_state_from_zonecut(struct iter_local_state *local_state,
 			name_state->aaaa_state = RECORD_UNKNOWN;
 		}
 		
-		if (addresses->len == 0) {
-			continue;
-		}
-
-		/* We have some addresses to work with, let's iterate over them. */
+		/* Iterate over all addresses of this NS (if any). */
 		for (uint8_t *obj = pack_head(*addresses); obj != pack_tail(*addresses);
 		     obj = pack_obj_next(obj)) {
 			uint8_t *address = pack_obj_val(obj);
@@ -131,7 +115,7 @@ static void unpack_state_from_zonecut(struct iter_local_state *local_state,
 				*tval = mm_alloc(mm, sizeof(struct address_state));
 				memset(*tval, 0, sizeof(struct address_state));
 			}
-			struct address_state *address_state = (*(struct address_state **)tval);
+			struct address_state *address_state = *tval;
 			address_state->generation = current_generation;
 			address_state->ns_name = dname;
 
@@ -155,8 +139,7 @@ static int get_valid_addresses(struct iter_local_state *local_state,
 	     trie_it_next(it)) {
 		size_t address_len;
 		uint8_t *address = (uint8_t *)trie_it_key(it, &address_len);
-		struct address_state *address_state =
-			(struct address_state *)*trie_it_val(it);
+		struct address_state *address_state = *trie_it_val(it);
 		if (address_state->generation == local_state->generation &&
 		    !address_state->unrecoverable_errors) {
 			choices[count] = (struct choice){
@@ -184,45 +167,42 @@ static int get_resolvable_names(struct iter_local_state *local_state,
 	trie_it_t *it;
 	for (it = trie_it_begin(local_state->names); !trie_it_finished(it);
 	     trie_it_next(it)) {
-		struct iter_name_state *name_state =
-			*(struct iter_name_state **)trie_it_val(it);
-		if (name_state->generation == local_state->generation) {
-			knot_dname_t *name = (knot_dname_t *)trie_it_key(it, NULL);
-			if (qry->stype == KNOT_RRTYPE_DNSKEY &&
-			    knot_dname_in_bailiwick(name, qry->sname) > 0) {
-				/* Resolving `domain. DNSKEY` can't trigger the
-				 * resolution of `sub.domain. A/AAAA` since it
-				 * will cause a cycle. */
-				continue;
-			}
+		struct iter_name_state *name_state = *trie_it_val(it);
+		if (name_state->generation != local_state->generation)
+			continue;
 
-			/* FIXME: kr_rplan_satisfies(qry,…) should have been here, but this leads to failures on 
-			 * iter_ns_badip.rpl, this is because the test requires the resolver to switch to parent
-			 * side after a record in cache expires. Only way to do this in the current zonecut setup is
-			 * to requery the same query twice in the row. So we have to allow that and only check the 
-			 * rplan from parent upwards.
-			 */
-			bool a_in_rplan = kr_rplan_satisfies(qry->parent, name,
-							     KNOT_CLASS_IN,
-							     KNOT_RRTYPE_A);
-			bool aaaa_in_rplan =
-				kr_rplan_satisfies(qry->parent, name,
-						   KNOT_CLASS_IN,
-						   KNOT_RRTYPE_AAAA);
-
-			if (name_state->a_state == RECORD_UNKNOWN &&
-			    !qry->flags.NO_IPV4 && !a_in_rplan) {
-				resolvable[count++] = (struct to_resolve){
-					name, KR_TRANSPORT_RESOLVE_A
-				};
-			}
+		knot_dname_t *name = (knot_dname_t *)trie_it_key(it, NULL);
+		if (qry->stype == KNOT_RRTYPE_DNSKEY &&
+		    knot_dname_in_bailiwick(name, qry->sname) > 0) {
+			/* Resolving `domain. DNSKEY` can't trigger the
+			 * resolution of `sub.domain. A/AAAA` since it
+			 * will cause a cycle. */
+			continue;
+		}
 
-			if (name_state->aaaa_state == RECORD_UNKNOWN &&
-			    !qry->flags.NO_IPV6 && !aaaa_in_rplan) {
-				resolvable[count++] = (struct to_resolve){
-					name, KR_TRANSPORT_RESOLVE_AAAA
-				};
-			}
+		/* FIXME: kr_rplan_satisfies(qry,…) should have been here, but this leads to failures on
+		 * iter_ns_badip.rpl, this is because the test requires the resolver to switch to parent
+		 * side after a record in cache expires. Only way to do this in the current zonecut setup is
+		 * to requery the same query twice in the row. So we have to allow that and only check the
+		 * rplan from parent upwards.
+		 */
+		bool a_in_rplan = kr_rplan_satisfies(qry->parent, name,
+						     KNOT_CLASS_IN, KNOT_RRTYPE_A);
+		bool aaaa_in_rplan = kr_rplan_satisfies(qry->parent, name,
+							KNOT_CLASS_IN, KNOT_RRTYPE_AAAA);
+
+		if (name_state->a_state == RECORD_UNKNOWN &&
+		    !qry->flags.NO_IPV4 && !a_in_rplan) {
+			resolvable[count++] = (struct to_resolve){
+				name, KR_TRANSPORT_RESOLVE_A
+			};
+		}
+
+		if (name_state->aaaa_state == RECORD_UNKNOWN &&
+		    !qry->flags.NO_IPV6 && !aaaa_in_rplan) {
+			resolvable[count++] = (struct to_resolve){
+				name, KR_TRANSPORT_RESOLVE_AAAA
+			};
 		}
 	}
 	trie_it_free(it);
@@ -252,8 +232,7 @@ static void update_name_state(knot_dname_t *name, enum kr_transport_protocol typ
 	}
 }
 
-void iter_choose_transport(struct kr_query *qry,
-			   struct kr_transport **transport)
+void iter_choose_transport(struct kr_query *qry, struct kr_transport **transport)
 {
 	struct knot_mm *mempool = &qry->request->pool;
 	struct iter_local_state *local_state =
@@ -272,8 +251,7 @@ void iter_choose_transport(struct kr_query *qry,
 	int resolvable_len = get_resolvable_names(local_state, resolvable, qry);
 
 	if (choices_len || resolvable_len) {
-		bool tcp = qry->flags.TCP |
-			   qry->server_selection.local_state->truncated;
+		bool tcp = qry->flags.TCP || qry->server_selection.local_state->truncated;
 		*transport = select_transport(
 			choices, choices_len, resolvable, resolvable_len,
 			qry->server_selection.local_state->timeouts, mempool,
@@ -308,7 +286,7 @@ void iter_choose_transport(struct kr_query *qry,
 	}
 
 	bool nxnsattack_mitigation = false;
-	enum kr_transport_protocol proto =
+	const enum kr_transport_protocol proto =
 		*transport ? (*transport)->protocol : -1;
 	if (proto == KR_TRANSPORT_RESOLVE_A || proto == KR_TRANSPORT_RESOLVE_AAAA) {
 		if (++local_state->no_ns_addr_count > KR_COUNT_NO_NSADDR_LIMIT) {
@@ -333,14 +311,18 @@ void iter_choose_transport(struct kr_query *qry,
 				    qry->id, ip_version, ns_name, zonecut_str);
 			break;
 		default:
-			VERBOSE_MSG(qry, "=> id: '%05u' choosing: '%s'@'%s' with timeout %u ms zone cut: '%s'%s\n",
-				    qry->id, ns_name, ns_str ? ns_str : "", (*transport)->timeout, zonecut_str,
+			VERBOSE_MSG(qry, "=> id: '%05u' choosing: '%s'@'%s'"
+				    " with timeout %u ms zone cut: '%s'%s\n",
+				    qry->id, ns_name, ns_str ? ns_str : "",
+				    (*transport)->timeout, zonecut_str,
 				    (*transport)->safe_mode ? " SAFEMODE" : "");
 			break;
 		}
 	} else {
+		const char *nxns_msg = nxnsattack_mitigation
+			? " (stopped due to mitigation for NXNSAttack CVE-2020-12667)" : "";
 		VERBOSE_MSG(qry, "=> id: '%05u' no suitable transport, zone cut: '%s'%s\n",
-			qry->id, zonecut_str, nxnsattack_mitigation ? " (stopped due to mitigation for NXNSAttack CVE-2020-12667)" : "");
+			    qry->id, zonecut_str, nxns_msg );
 	}
 	}
 }
diff --git a/lib/selection_iter.h b/lib/selection_iter.h
index f1c798bfc..ed8444763 100644
--- a/lib/selection_iter.h
+++ b/lib/selection_iter.h
@@ -6,29 +6,9 @@
 
 #include "lib/selection.h"
 
-/**
- * If one of the errors set to true is encountered, there is no point in asking this server again.
- */
-static const bool UNRECOVERABLE_ERRORS[] = {
-	[KR_SELECTION_QUERY_TIMEOUT] = false,
-	[KR_SELECTION_TLS_HANDSHAKE_FAILED] = false,
-	[KR_SELECTION_TCP_CONNECT_FAILED] = false,
-	[KR_SELECTION_TCP_CONNECT_TIMEOUT] = false,
-	[KR_SELECTION_REFUSED] = true,
-	[KR_SELECTION_SERVFAIL] = true,
-	[KR_SELECTION_FORMERROR] = false,
-	[KR_SELECTION_NOTIMPL] = true,
-	[KR_SELECTION_OTHER_RCODE] = true,
-	[KR_SELECTION_TRUNCATED] = false,
-	[KR_SELECTION_DNSSEC_ERROR] = true,
-	[KR_SELECTION_LAME_DELEGATION] = true,
-	[KR_SELECTION_BAD_CNAME] = true,
-};
-
 void iter_local_state_alloc(struct knot_mm *mm, void **local_state);
-void iter_choose_transport(struct kr_query *qry,
-			   struct kr_transport **transport);
+void iter_choose_transport(struct kr_query *qry, struct kr_transport **transport);
 void iter_error(struct kr_query *qry, const struct kr_transport *transport,
 		enum kr_selection_error sel_error);
 void iter_update_rtt(struct kr_query *qry, const struct kr_transport *transport,
-		     unsigned rtt);
\ No newline at end of file
+		     unsigned rtt);
-- 
GitLab