- Fix cachedb for serve-expired with serve-expired-reply-ttl.

2024-09-21 06:37:08 +00:00 · 2024-04-10 17:01:57 +02:00 · 2024-04-10 17:01:57 +02:00 · d47849a26e
commit d47849a26e
parent 63ee97d0fd
7 changed files with 325 additions and 18 deletions
--- a/cachedb/cachedb.c
+++ b/cachedb/cachedb.c
@ -267,10 +267,6 @@ cachedb_init(struct module_env* env, int id)
 		return 0;
 	}
 	cachedb_env->enabled = 1;
-	if(env->cfg->serve_expired && env->cfg->serve_expired_reply_ttl)
-		log_warn(
-			"cachedb: serve-expired-reply-ttl is set but not working for data "
-			"originating from the external cache; 0 TTL is used for those.");
 	if(env->cfg->serve_expired && env->cfg->serve_expired_client_timeout)
 		log_warn(
 			"cachedb: serve-expired-client-timeout is set but not working for "
@ -513,9 +509,38 @@ adjust_msg_ttl(struct dns_msg* msg, time_t adjust)
 	}
 }

+/* Set the TTL of the given RRset to fixed value. */
+static void
+packed_rrset_ttl_set(struct packed_rrset_data* data, time_t ttl)
+{
+	size_t i;
+	size_t total = data->count + data->rrsig_count;
+	data->ttl = ttl;
+	for(i=0; i<total; i++) {
+		data->rr_ttl[i] = ttl;
+	}
+	data->ttl_add = 0;
+}
+
+/* Set the TTL of a DNS message and its RRs by to a fixed value. */
+static void
+set_msg_ttl(struct dns_msg* msg, time_t ttl)
+{
+	size_t i;
+	msg->rep->ttl = ttl;
+	msg->rep->prefetch_ttl = PREFETCH_TTL_CALC(msg->rep->ttl);
+	msg->rep->serve_expired_ttl = msg->rep->ttl + SERVE_EXPIRED_TTL;
+
+	for(i=0; i<msg->rep->rrset_count; i++) {
+		packed_rrset_ttl_set((struct packed_rrset_data*)msg->
+			rep->rrsets[i]->entry.data, ttl);
+	}
+}
+
 /** convert dns message in buffer to return_msg */
 static int
-parse_data(struct module_qstate* qstate, struct sldns_buffer* buf)
+parse_data(struct module_qstate* qstate, struct sldns_buffer* buf,
+	int* msg_expired)
 {
 	struct msg_parse* prs;
 	struct edns_data edns;
@ -585,6 +610,7 @@ parse_data(struct module_qstate* qstate, struct sldns_buffer* buf)
 	adjust = *qstate->env->now - (time_t)timestamp;
 	if(qstate->return_msg->rep->ttl < adjust) {
 		verbose(VERB_ALGO, "cachedb msg expired");
+		*msg_expired = 1;
 		/* If serve-expired is enabled, we still use an expired message
 		 * setting the TTL to 0. */
 		if(!qstate->env->cfg->serve_expired ||
@ -619,7 +645,8 @@ parse_data(struct module_qstate* qstate, struct sldns_buffer* buf)
 * return true if lookup was successful.
 */
 static int
-cachedb_extcache_lookup(struct module_qstate* qstate, struct cachedb_env* ie)
+cachedb_extcache_lookup(struct module_qstate* qstate, struct cachedb_env* ie,
+	int* msg_expired)
 {
 	char key[(CACHEDB_HASHSIZE/8)*2+1];
 	calc_hash(qstate, key, sizeof(key));
@ -636,7 +663,7 @@ cachedb_extcache_lookup(struct module_qstate* qstate, struct cachedb_env* ie)
 	}

 	/* parse dns message into return_msg */
-	if( !parse_data(qstate, qstate->env->scratch_buffer) ) {
+	if( !parse_data(qstate, qstate->env->scratch_buffer, msg_expired) ) {
 		return 0;
 	}
 	return 1;
@ -707,7 +734,7 @@ cachedb_intcache_lookup(struct module_qstate* qstate, struct cachedb_env* cde)
 * Store query into the internal cache of unbound.
 */
 static void
-cachedb_intcache_store(struct module_qstate* qstate)
+cachedb_intcache_store(struct module_qstate* qstate, int msg_expired)
 {
 	uint32_t store_flags = qstate->query_flags;

@ -715,9 +742,23 @@ cachedb_intcache_store(struct module_qstate* qstate)
 		store_flags |= DNSCACHE_STORE_ZEROTTL;
 	if(!qstate->return_msg)
 		return;
+	if(msg_expired) {
+		/* Set TTLs to a value such that value + *env->now is
+		 * going to be now-3 seconds. Making it expired
+		 * in the cache. */
+		set_msg_ttl(qstate->return_msg, (time_t)-3);
+	}
 	(void)dns_cache_store(qstate->env, &qstate->qinfo,
 		qstate->return_msg->rep, 0, qstate->prefetch_leeway, 0,
 		qstate->region, store_flags, qstate->qstarttime);
+	if(msg_expired) {
+		/* set TTLs to zero again */
+		adjust_msg_ttl(qstate->return_msg, -1);
+		/* Send serve expired responses based on the cachedb
+		 * returned message, that was just stored in the cache.
+		 * It can then continue to work on this query. */
+		mesh_respond_serve_expired(qstate->mesh_info);
+	}
 }

 /**
@ -733,6 +774,7 @@ cachedb_handle_query(struct module_qstate* qstate,
 	struct cachedb_qstate* ATTR_UNUSED(iq),
 	struct cachedb_env* ie, int id)
 {
+	int msg_expired = 0;
 	qstate->is_cachedb_answer = 0;
 	/* check if we are enabled, and skip if so */
 	if(!ie->enabled) {
@ -767,13 +809,13 @@ cachedb_handle_query(struct module_qstate* qstate,
 	}

 	/* ask backend cache to see if we have data */
-	if(cachedb_extcache_lookup(qstate, ie)) {
+	if(cachedb_extcache_lookup(qstate, ie, &msg_expired)) {
 		if(verbosity >= VERB_ALGO)
 			log_dns_msg(ie->backend->name,
 				&qstate->return_msg->qinfo,
 				qstate->return_msg->rep);
 		/* store this result in internal cache */
-		cachedb_intcache_store(qstate);
+		cachedb_intcache_store(qstate, msg_expired);
 		/* In case we have expired data but there is a client timer for expired
 		 * answers, pass execution to next module in order to try updating the
 		 * data first.
--- a/doc/Changelog
+++ b/doc/Changelog
@ -6,6 +6,7 @@
 	- Add test for cachedb serve expired.
 	- Extended test for cachedb serve expired.
 	- Fix makefile dependencies for fake_event.c.
+	- Fix cachedb for serve-expired with serve-expired-reply-ttl.

 9 April 2024: Yorgos
 	- Merge #1043 from xiaoxiaoafeifei: Add loongarch support; updates
--- a/doc/unbound.conf.5.in
+++ b/doc/unbound.conf.5.in
@ -2641,10 +2641,10 @@ query as usual, and stores the answer in the backend.
 .P
 This module interacts with the \fBserve\-expired\-*\fR options and will reply
 with expired data if Unbound is configured for that.  Currently the use
-of \fBserve\-expired\-client\-timeout:\fR and
-\fBserve\-expired\-reply\-ttl:\fR is not consistent for data originating from
-the external cache as these will result in a reply with 0 TTL without trying to
-update the data first, ignoring the configured values.
+of \fBserve\-expired\-client\-timeout:\fR
+is not consistent for data originating from
+the external cache as it will result in a reply without trying to
+update the data first, ignoring the configured value.
 .P
 If Unbound was built with
 \fB\-\-with\-libhiredis\fR
@ -2703,7 +2703,7 @@ is retrieved. The default is no.
 .TP
 .B cachedb-check-when-serve-expired: \fI<yes or no>\fR
 If enabled, the cachedb is checked before an expired response is returned.
-When serve-expired is enabled, without serve-expired-client-timeout, it then
+When \fBserve\-expired\fR is enabled, without \fBserve\-expired\-client\-timeout\fR, it then
 does not immediately respond with an expired response from cache, but instead
 first checks the cachedb for valid contents, and if so returns it. If the
 cachedb also has no valid contents, the serve expired response is sent.
--- a/services/cache/rrset.c
+++ b/services/cache/rrset.c
@ -127,6 +127,9 @@ need_to_update_rrset(void* nd, void* cd, time_t timenow, int equal, int ns)
 {
 	struct packed_rrset_data* newd = (struct packed_rrset_data*)nd;
 	struct packed_rrset_data* cached = (struct packed_rrset_data*)cd;
+	/*	o if new data is expired, current data is better */
+	if( newd->ttl < timenow && cached->ttl >= timenow)
+		return 0;
 	/* 	o store if rrset has been validated 
 	 *  		everything better than bogus data 
 	 *  		secure is preferred */
--- a/services/mesh.c
+++ b/services/mesh.c
@ -2264,6 +2264,8 @@ mesh_serve_expired_callback(void* arg)
 void
 mesh_respond_serve_expired(struct mesh_state* mstate)
 {
+	if(!mstate->s.serve_expired_data)
+		mesh_serve_expired_init(mstate, -1);
 	mesh_serve_expired_callback(mstate);
 }

--- a/testdata/cachedb_expired.crpl
+++ b/testdata/cachedb_expired.crpl
@ -150,7 +150,7 @@ REPLY QR RD RA NOERROR
 SECTION QUESTION
 www2.example.com. IN A
 SECTION ANSWER
-www2.example.com. 0 IN A 1.2.3.5
+www2.example.com. 30 IN A 1.2.3.5
 ENTRY_END

 ; cache is expired, cachedb has no answer
@ -232,7 +232,7 @@ REPLY QR RD RA NOERROR
 SECTION QUESTION
 www.example.com. IN A
 SECTION ANSWER
-www.example.com. 0 IN A 1.2.3.4
+www.example.com. 30 IN A 1.2.3.4
 ENTRY_END

 STEP 190 TRAFFIC
@ -297,7 +297,7 @@ REPLY QR RD RA NOERROR
 SECTION QUESTION
 www.example.com. IN A
 SECTION ANSWER
-www.example.com. 0 IN A 1.2.3.4
+www.example.com. 30 IN A 1.2.3.4
 ENTRY_END

 STEP 290 TRAFFIC
--- a/testdata/cachedb_expired_reply_ttl.crpl
+++ b/testdata/cachedb_expired_reply_ttl.crpl
@ -0,0 +1,259 @@
+; config options
+server:
+	target-fetch-policy: "0 0 0 0 0"
+	qname-minimisation: no
+	minimal-responses: no
+	serve-expired: yes
+	serve-expired-reply-ttl: 30
+	module-config: "cachedb iterator"
+
+cachedb:
+	backend: "testframe"
+	secret-seed: "testvalue"
+	cachedb-check-when-serve-expired: yes
+
+stub-zone:
+	name: "."
+	stub-addr: 193.0.14.129
+CONFIG_END
+
+SCENARIO_BEGIN Test cachedb and serve-expired-reply-ttl.
+
+; K.ROOT-SERVERS.NET.
+RANGE_BEGIN 0 400
+	ADDRESS 193.0.14.129
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+. IN NS
+SECTION ANSWER
+. IN NS K.ROOT-SERVERS.NET.
+SECTION ADDITIONAL
+K.ROOT-SERVERS.NET.     IN      A       193.0.14.129
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode subdomain
+ADJUST copy_id copy_query
+REPLY QR NOERROR
+SECTION QUESTION
+com. IN NS
+SECTION AUTHORITY
+com. IN NS a.gtld-servers.net.
+SECTION ADDITIONAL
+a.gtld-servers.net.	IN	A	192.5.6.30
+ENTRY_END
+RANGE_END
+
+; a.gtld-servers.net.
+RANGE_BEGIN 0 400
+	ADDRESS 192.5.6.30
+ENTRY_BEGIN
+MATCH opcode subdomain
+ADJUST copy_id copy_query
+REPLY QR NOERROR
+SECTION QUESTION
+example.com. IN NS
+SECTION AUTHORITY
+example.com. IN NS ns2.example.com.
+SECTION ADDITIONAL
+ns2.example.com.	IN	A	1.2.3.5
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode subdomain
+ADJUST copy_id copy_query
+REPLY QR NOERROR
+SECTION QUESTION
+foo.com. IN NS
+SECTION AUTHORITY
+foo.com. IN NS ns.example.com.
+ENTRY_END
+RANGE_END
+
+; ns2.example.com.
+RANGE_BEGIN 0 400
+	ADDRESS 1.2.3.5
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN A 1.2.3.4
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qname qtype
+REPLY QR AA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.5
+ENTRY_END
+RANGE_END
+
+; make time not 0
+STEP 2 TIME_PASSES ELAPSE 212
+
+; Get an entry in cache, to make it expired.
+STEP 4 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+; get the answer for it
+STEP 10 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN A 1.2.3.4
+ENTRY_END
+
+; Get another query in cache to make it expired.
+STEP 20 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www2.example.com. IN A
+ENTRY_END
+
+; get the answer for it
+STEP 30 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.5
+ENTRY_END
+
+; it is now expired
+STEP 40 TIME_PASSES ELAPSE 20
+
+; cache is expired, and cachedb is expired.
+STEP 50 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www2.example.com. IN A
+ENTRY_END
+
+STEP 60 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 30 IN A 1.2.3.5
+ENTRY_END
+
+; got an answer from upstream
+STEP 61 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www2.example.com. IN A
+ENTRY_END
+
+STEP 62 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www2.example.com. IN A
+SECTION ANSWER
+www2.example.com. 10 IN A 1.2.3.5
+ENTRY_END
+
+; cache is expired, cachedb has no answer
+STEP 70 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+STEP 80 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 30 IN A 1.2.3.4
+ENTRY_END
+
+STEP 90 TRAFFIC
+; the entry should be refreshed in cache now.
+; cache is valid and cachedb is valid.
+STEP 100 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+STEP 110 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN A 1.2.3.4
+ENTRY_END
+
+; make both cache and cachedb expired.
+STEP 120 TIME_PASSES ELAPSE 20
+STEP 130 FLUSH_MESSAGE www.example.com. IN A
+
+; cache has no entry and cachedb is expired.
+STEP 140 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+STEP 150 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 30 IN A 1.2.3.4
+ENTRY_END
+
+; the name is resolved
+STEP 160 TRAFFIC
+
+; the resolve name has been updated.
+STEP 170 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+STEP 180 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN A 1.2.3.4
+ENTRY_END
+
+SCENARIO_END