From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Mon, 7 Jul 2025 12:00:27 +0000 (+0200)
Subject: lib/cache: simplify logic for qry->flags.CACHE_TRIED
X-Git-Tag: v6.0.15~3^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=726a3bdc098a43d469b6864143d70e4872367b97;p=thirdparty%2Fknot-resolver.git

lib/cache: simplify logic for qry->flags.CACHE_TRIED

As a side effect, this solves an issue which could be seen
during resolver startup where the trust anchor update would fail.
[taupd ] active refresh failed for . with rcode: 2

The cause is that for queries started with .flags.NO_CACHE,
we'd skip the section setting .flags.CACHE_TRIED, and consequently
kr_rule_local_data_answer() would get run more often than expected.

The new logic should be also much simpler to follow.
We always apply cache (and policy) just once per kr_query.
---

diff --git a/lib/cache/api.c b/lib/cache/api.c
index 046dae203..d542060c9 100644
--- a/lib/cache/api.c
+++ b/lib/cache/api.c
@@ -314,32 +314,31 @@ int cache_peek(kr_layer_t *ctx, knot_pkt_t *pkt)
 	struct kr_request *req = ctx->req;
 	struct kr_query *qry = req->current_query;
 
-	/* TODO: review when to run this?  We want to process rules here
-	 * even when some of the cache exit-conditions happen.  NO_CACHE in particular. */
-	if (!req->options.PASSTHRU_LEGACY && !qry->flags.CACHE_TRIED) {
+	if (ctx->state & (KR_STATE_FAIL|KR_STATE_DONE))
+		return ctx->state;
+	/* ATM cache only peeks for qry->sname and that would be useless
+	 * to repeat on every iteration, so disable it from now on.
+	 * Note that xNAME causes a followup kr_query, so cache will get re-tried.
+	 * LATER(optim.): assist with more precise QNAME minimization. */
+	if (qry->flags.CACHE_TRIED && !qry->stale_cb)
+		return ctx->state;
+	qry->flags.CACHE_TRIED = true;
+
+	if (!req->options.PASSTHRU_LEGACY) {
 		int ret = kr_rule_local_data_answer(qry, pkt);
 		if (ret < 0)
 			ctx->state = KR_STATE_FAIL;
-		if (ret != 0) {
-			qry->flags.CACHE_TRIED = true;
+		if (ret != 0)
 			return ctx->state;
-		}
 	}
 
 	/* We first check various exit-conditions and then call the _real function. */
-
 	if (!kr_cache_is_open(&req->ctx->cache)
-	    || ctx->state & (KR_STATE_FAIL|KR_STATE_DONE) || qry->flags.NO_CACHE
-	    || (qry->flags.CACHE_TRIED && !qry->stale_cb)
+	    || qry->flags.NO_CACHE
 	    || !check_rrtype(qry->stype, qry) /* LATER: some other behavior for some of these? */
 	    || qry->sclass != KNOT_CLASS_IN) {
 		return ctx->state; /* Already resolved/failed or already tried, etc. */
 	}
-	/* ATM cache only peeks for qry->sname and that would be useless
-	 * to repeat on every iteration, so disable it from now on.
-	 * Note that xNAME causes a followup kr_query, so cache will get re-tried.
-	 * LATER(optim.): assist with more precise QNAME minimization. */
-	qry->flags.CACHE_TRIED = true;
 
 	if (qry->stype == KNOT_RRTYPE_NSEC) {
 		VERBOSE_MSG(qry, "=> skipping stype NSEC\n");