From 26050b0d3205f4004a54b0572acd01d8b1dd9dcc Mon Sep 17 00:00:00 2001
From: =?utf8?q?Vladim=C3=ADr=20=C4=8Cun=C3=A1t?= <vladimir.cunat@nic.cz>
Date: Thu, 7 Mar 2019 15:39:11 +0100
Subject: [PATCH] resolve, iterate: fix fallback when upstream SERVFAILs

or REFUSEs.  We typically ended up retrying with the same server,
which is almost never good.  Now we remove the server from the set.

Nitpick: a couple kr_query fields are reordered for better packing.
---
 NEWS                    |  1 +
 daemon/lua/kres-gen.lua |  5 ++---
 lib/layer/iterate.c     | 11 +----------
 lib/resolve.c           |  7 ++++++-
 lib/rplan.h             |  5 ++---
 tests/deckard           |  2 +-
 6 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/NEWS b/NEWS
index 3d6960229..ac2229ff9 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,7 @@ Bugfixes
 - fix startup on macOS+BSD when </dev/null and cqueues installed
 - policy.RPZ: log problems from zone-file level of parser as well (#453)
 - fix flushing of messages to logs in some cases (!781)
+- fix fallback when SERVFAIL or REFUSED is received from upstream (!784)
 
 Improvements
 ------------
diff --git a/daemon/lua/kres-gen.lua b/daemon/lua/kres-gen.lua
index 727d62461..0651f07d3 100644
--- a/daemon/lua/kres-gen.lua
+++ b/daemon/lua/kres-gen.lua
@@ -244,17 +244,16 @@ struct kr_query {
 	uint16_t stype;
 	uint16_t sclass;
 	uint16_t id;
+	uint16_t reorder;
 	struct kr_qflags flags;
 	struct kr_qflags forward_flags;
 	uint32_t secret;
-	uint16_t fails;
-	uint16_t reorder;
+	uint32_t uid;
 	uint64_t creation_time_mono;
 	uint64_t timestamp_mono;
 	struct timeval timestamp;
 	struct kr_zonecut zone_cut;
 	struct kr_layer_pickle *deferred;
-	uint32_t uid;
 	struct kr_query *cname_parent;
 	struct kr_request *request;
 	kr_stale_cb stale_cb;
diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c
index cf57cc54c..d5448af03 100644
--- a/lib/layer/iterate.c
+++ b/lib/layer/iterate.c
@@ -1073,16 +1073,7 @@ static int resolve(kr_layer_t *ctx, knot_pkt_t *pkt)
 			break;
 		}
 		VERBOSE_MSG("<= rcode: %s\n", rcode ? rcode->name : "??");
-		query->fails += 1;
-		if (query->fails >= KR_QUERY_NSRETRY_LIMIT) {
-			query->fails = 0; /* Reset per-query counter. */
-			return resolve_error(pkt, req);
-		} else {
-			if (!query->flags.FORWARD) {
-				query->flags.NO_MINIMIZE = true; /* Drop minimisation as a safe-guard. */
-			}
-			return KR_STATE_CONSUME;
-		}
+		return KR_STATE_FAIL;
 	}
 	case KNOT_RCODE_FORMERR:
 		VERBOSE_MSG("<= rcode: %s\n", rcode ? rcode->name : "??");
diff --git a/lib/resolve.c b/lib/resolve.c
index ce21edf99..a83de468e 100644
--- a/lib/resolve.c
+++ b/lib/resolve.c
@@ -162,7 +162,12 @@ static int invalidate_ns(struct kr_rplan *rplan, struct kr_query *qry)
 	if (qry->ns.addr[0].ip.sa_family != AF_UNSPEC) {
 		const char *addr = kr_inaddr(&qry->ns.addr[0].ip);
 		int addr_len = kr_inaddr_len(&qry->ns.addr[0].ip);
-		return kr_zonecut_del(&qry->zone_cut, qry->ns.name, addr, addr_len);
+		int ret = kr_zonecut_del(&qry->zone_cut, qry->ns.name, addr, addr_len);
+		/* Also remove it from the qry->ns.addr array.
+		 * That's useful at least for STUB and FORWARD modes. */
+		memmove(qry->ns.addr, qry->ns.addr + 1,
+			sizeof(qry->ns.addr[0]) * (KR_NSREP_MAXADDR - 1));
+		return ret;
 	} else {
 		return kr_zonecut_del_all(&qry->zone_cut, qry->ns.name);
 	}
diff --git a/lib/rplan.h b/lib/rplan.h
index b5ac1f91c..6e93afc71 100644
--- a/lib/rplan.h
+++ b/lib/rplan.h
@@ -90,10 +90,10 @@ struct kr_query {
 	uint16_t stype;
 	uint16_t sclass;
 	uint16_t id;
+	uint16_t reorder; /**< Seed to reorder (cached) RRs in answer or zero. */
 	struct kr_qflags flags, forward_flags;
 	uint32_t secret;
-	uint16_t fails;
-	uint16_t reorder; /**< Seed to reorder (cached) RRs in answer or zero. */
+	uint32_t uid; /**< Query iteration number, unique within the kr_rplan. */
 	uint64_t creation_time_mono; /* The time of query's creation (milliseconds).
 				 * Or time of creation of an oldest
 				 * ancestor if it is a subquery. */
@@ -102,7 +102,6 @@ struct kr_query {
 	struct timeval timestamp; /**< Real time for TTL+DNSSEC checks (.tv_sec only). */
 	struct kr_zonecut zone_cut;
 	struct kr_layer_pickle *deferred;
-	uint32_t uid; /**< Query iteration number, unique within the kr_rplan. */
 	/** Pointer to the query that originated this one because of following a CNAME (or NULL). */
 	struct kr_query *cname_parent;
 	struct kr_request *request; /**< Parent resolution request. */
diff --git a/tests/deckard b/tests/deckard
index 7ec9d93e0..2a9c7de53 160000
--- a/tests/deckard
+++ b/tests/deckard
@@ -1 +1 @@
-Subproject commit 7ec9d93e05cf40fdeee58bc6f8bbdd3ddb216115
+Subproject commit 2a9c7de5377ed62f2569dcdb250fee44035776f4
-- 
2.47.2