From: Vladimír Čunát Date: Mon, 28 Feb 2022 18:10:16 +0000 (+0100) Subject: lib/cache: tweak TTL computation for packets X-Git-Tag: v5.6.0~6^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=48ab4d3630914f0d98c664cd08949a83f0bbfe18;p=thirdparty%2Fknot-resolver.git lib/cache: tweak TTL computation for packets When a whole packet is cached (instead of individual RRs), let's simplify the way the packet's TTL gets computed. The previous mechanism came from commit 5b383a2bb7, probably a misunderstanding of: https://datatracker.ietf.org/doc/html/rfc2308#section-5 Anyway, I see no motivation to do it, and this way we should get rid of some weird cases where we might extend TTL of some records, except if they were below the cache.min_ttl() setting (5s default). --- diff --git a/daemon/http.c b/daemon/http.c index af387d818..ebc05f8f2 100644 --- a/daemon/http.c +++ b/daemon/http.c @@ -876,7 +876,6 @@ static int http_write_pkt(struct http_ctx *ctx, knot_pkt_t *pkt, int32_t stream_ { struct http_data *data; nghttp2_data_provider prov; - const bool is_negative = kr_response_classify(pkt) & (PKT_NODATA|PKT_NXDOMAIN); data = malloc(sizeof(struct http_data)); if (!data) @@ -887,7 +886,7 @@ static int http_write_pkt(struct http_ctx *ctx, knot_pkt_t *pkt, int32_t stream_ data->pos = 0; data->on_write = on_write; data->req = req; - data->ttl = packet_ttl(pkt, is_negative); + data->ttl = packet_ttl(pkt); prov.source.ptr = data; prov.read_callback = read_callback; diff --git a/daemon/lua/kres-gen-30.lua b/daemon/lua/kres-gen-30.lua index 76659b7f1..4353c5ce0 100644 --- a/daemon/lua/kres-gen-30.lua +++ b/daemon/lua/kres-gen-30.lua @@ -456,7 +456,7 @@ int kr_cache_insert_rr(struct kr_cache *, const knot_rrset_t *, const knot_rrset int kr_cache_remove(struct kr_cache *, const knot_dname_t *, uint16_t); int kr_cache_remove_subtree(struct kr_cache *, const knot_dname_t *, _Bool, int); int kr_cache_commit(struct kr_cache *); -uint32_t packet_ttl(const knot_pkt_t *, _Bool); +uint32_t packet_ttl(const knot_pkt_t *); typedef struct { int sock_type; _Bool tls; diff --git a/daemon/lua/kres-gen-31.lua b/daemon/lua/kres-gen-31.lua index b009868db..a68dd653f 100644 --- a/daemon/lua/kres-gen-31.lua +++ b/daemon/lua/kres-gen-31.lua @@ -456,7 +456,7 @@ int kr_cache_insert_rr(struct kr_cache *, const knot_rrset_t *, const knot_rrset int kr_cache_remove(struct kr_cache *, const knot_dname_t *, uint16_t); int kr_cache_remove_subtree(struct kr_cache *, const knot_dname_t *, _Bool, int); int kr_cache_commit(struct kr_cache *); -uint32_t packet_ttl(const knot_pkt_t *, _Bool); +uint32_t packet_ttl(const knot_pkt_t *); typedef struct { int sock_type; _Bool tls; diff --git a/daemon/lua/kres-gen-32.lua b/daemon/lua/kres-gen-32.lua index 7686419f1..222891e3f 100644 --- a/daemon/lua/kres-gen-32.lua +++ b/daemon/lua/kres-gen-32.lua @@ -457,7 +457,7 @@ int kr_cache_insert_rr(struct kr_cache *, const knot_rrset_t *, const knot_rrset int kr_cache_remove(struct kr_cache *, const knot_dname_t *, uint16_t); int kr_cache_remove_subtree(struct kr_cache *, const knot_dname_t *, _Bool, int); int kr_cache_commit(struct kr_cache *); -uint32_t packet_ttl(const knot_pkt_t *, _Bool); +uint32_t packet_ttl(const knot_pkt_t *); typedef struct { int sock_type; _Bool tls; diff --git a/lib/cache/entry_pkt.c b/lib/cache/entry_pkt.c index fa59380d3..778c0f4d8 100644 --- a/lib/cache/entry_pkt.c +++ b/lib/cache/entry_pkt.c @@ -13,33 +13,20 @@ #include "lib/cache/impl.h" -/** Compute TTL for a packet. Generally it's minimum TTL, with extra conditions. */ +/** Compute TTL for a packet. It's minimum TTL or zero. (You can apply limits.) */ KR_EXPORT -uint32_t packet_ttl(const knot_pkt_t *pkt, bool is_negative) +uint32_t packet_ttl(const knot_pkt_t *pkt) { bool has_ttl = false; uint32_t ttl = TTL_MAX_MAX; - /* Find minimum entry TTL in the packet or SOA minimum TTL. */ for (knot_section_t i = KNOT_ANSWER; i <= KNOT_ADDITIONAL; ++i) { const knot_pktsection_t *sec = knot_pkt_section(pkt, i); for (unsigned k = 0; k < sec->count; ++k) { const knot_rrset_t *rr = knot_pkt_rr(sec, k); - if (is_negative) { - /* Use SOA minimum TTL for negative answers. */ - if (rr->type == KNOT_RRTYPE_SOA) { - return MIN(rr->ttl, knot_soa_minimum(rr->rrs.rdata)); - } else { - continue; /* Use SOA only for negative answers. */ - } - } - if (knot_rrtype_is_metatype(rr->type)) { - continue; /* Skip metatypes. */ - } ttl = MIN(ttl, rr->ttl); has_ttl = true; } } - /* If no valid TTL present, go with zero (will get clamped to minimum). */ return has_ttl ? ttl : 0; } @@ -120,7 +107,7 @@ void stash_pkt(const knot_pkt_t *pkt, const struct kr_query *qry, struct entry_h *eh = val_new_entry.data; memset(eh, 0, offsetof(struct entry_h, data)); eh->time = qry->timestamp.tv_sec; - eh->ttl = MAX(MIN(packet_ttl(pkt, is_negative), cache->ttl_max), cache->ttl_min); + eh->ttl = MAX(MIN(packet_ttl(pkt), cache->ttl_max), cache->ttl_min); eh->rank = rank; eh->is_packet = true; eh->has_optout = qf->DNSSEC_OPTOUT; diff --git a/lib/cache/util.h b/lib/cache/util.h index 0a2f329c8..3f8183009 100644 --- a/lib/cache/util.h +++ b/lib/cache/util.h @@ -1,4 +1,4 @@ /* SPDX-License-Identifier: GPL-3.0-or-later */ #include -uint32_t packet_ttl(const knot_pkt_t *pkt, bool is_negative); +uint32_t packet_ttl(const knot_pkt_t *pkt); diff --git a/modules/http/http_doh.lua b/modules/http/http_doh.lua index 8625c12d9..33815f7b1 100644 --- a/modules/http/http_doh.lua +++ b/modules/http/http_doh.lua @@ -3,12 +3,6 @@ local basexx = require('basexx') local ffi = require('ffi') local condition = require('cqueues.condition') -local function get_http_ttl(pkt) - local an_records = pkt:section(kres.section.ANSWER) - local is_negative = #an_records <= 0 - return ffi.C.packet_ttl(pkt, is_negative) -end - -- Trace execution of DNS queries local function serve_doh(h, stream) local input @@ -68,7 +62,7 @@ local function serve_doh(h, stream) local cond = condition.new() local waiting, done = false, false local finish_cb = function (answer, _) - output_ttl = get_http_ttl(answer) + output_ttl = ffi.C.packet_ttl(answer) -- binary output output = ffi.string(answer.wire, answer.size) if waiting then