From 74a74b5f29b0c4208ad4c55f2f704ee6879d3960 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Thu, 15 Jan 2026 10:42:13 +0100 Subject: [PATCH 1/7] resolver: Defer cloning of fetch responses until events are sent Instead of cloning fetch responses immediately after writing to the head of the fetch response list, defer cloning until the events are actually sent. This removes the need for the `fctx->cloned` state. However, a new fetch state value, fetchstate_sentevents, is introduced and occurs after fetchstate_done. To prevent new fetch responses from being prepended after the head is written but before cloning occurs, fetchstate_done is now set at all call sites that previously invoked `clone_results()`. --- lib/dns/resolver.c | 128 ++++++++++++++++++++++----------------------- 1 file changed, 62 insertions(+), 66 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 919e518ac9..4fe66e4b6e 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -299,8 +299,9 @@ struct tried { #define RESQUERY_SENDING(q) ((q)->sends > 0) typedef enum { - fetchstate_active, - fetchstate_done /*%< Fetch completion events posted. */ + fetchstate_active = 0, + fetchstate_done, + fetchstate_sendevents /*%< Fetch completion events posted. */ } fetchstate_t; typedef enum { @@ -350,7 +351,6 @@ struct fetchctx { /*% Locked by lock. */ isc_mutex_t lock; fetchstate_t state; - bool cloned; bool spilled; uint_fast32_t allowed; uint_fast32_t dropped; @@ -496,7 +496,7 @@ struct fetchctx { ((atomic_load_acquire(&(f)->attributes) & FCTX_ATTR_GLUING) != 0) #define ADDRWAIT(f) \ ((atomic_load_acquire(&(f)->attributes) & FCTX_ATTR_ADDRWAIT) != 0) -#define SHUTTINGDOWN(f) ((f)->state == fetchstate_done) +#define SHUTTINGDOWN(f) ((f)->state >= fetchstate_done) #define WANTCACHE(f) \ ((atomic_load_acquire(&(f)->attributes) & FCTX_ATTR_WANTCACHE) != 0) #define WANTNCACHE(f) \ @@ -806,9 +806,6 @@ fctx__done(fetchctx_t *fctx, isc_result_t result, const char *func, static void resume_qmin(void *arg); -static void -clone_results(fetchctx_t *fctx); - static isc_result_t get_attached_fctx(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, dns_rdatatype_t type, const dns_name_t *domain, @@ -1615,6 +1612,48 @@ fcount_decr(fetchctx_t *fctx) { static void spillattimer_countdown(void *arg); +static void +clone_results(fetchctx_t *fctx) { + dns_fetchresponse_t *hresp = NULL; + + REQUIRE(!ISC_LIST_EMPTY(fctx->resps)); + + /* + * Set up any other resps to have the same data as the first. + * Caller must be holding the appropriate lock. + */ + + FCTXTRACE("clone_results"); + + ISC_LIST_FOREACH(fctx->resps, resp, link) { + /* This is the head resp; keep a pointer and move on */ + if (hresp == NULL) { + hresp = ISC_LIST_HEAD(fctx->resps); + FCTXTRACEN("clone_results", hresp->foundname, + hresp->result); + continue; + } + + resp->result = hresp->result; + dns_name_copy(hresp->foundname, resp->foundname); + dns_db_attach(hresp->db, &resp->db); + dns_db_attachnode(hresp->node, &resp->node); + + INSIST(hresp->rdataset != NULL && resp->rdataset != NULL); + if (dns_rdataset_isassociated(hresp->rdataset)) { + dns_rdataset_clone(hresp->rdataset, resp->rdataset); + } + + INSIST(hresp->sigrdataset != NULL || resp->sigrdataset == NULL); + if (resp->sigrdataset != NULL && hresp->sigrdataset != NULL && + dns_rdataset_isassociated(hresp->sigrdataset)) + { + dns_rdataset_clone(hresp->sigrdataset, + resp->sigrdataset); + } + } +} + static void fctx_sendevents(fetchctx_t *fctx, isc_result_t result) { unsigned int count = 0; @@ -1626,8 +1665,11 @@ fctx_sendevents(fetchctx_t *fctx, isc_result_t result) { LOCK(&fctx->lock); - REQUIRE(fctx->state == fetchstate_done); + REQUIRE(fctx->state == fetchstate_sendevents); + if (result == ISC_R_SUCCESS) { + clone_results(fctx); + } FCTXTRACE("sendevents"); /* @@ -1760,11 +1802,11 @@ fctx__done(fetchctx_t *fctx, isc_result_t result, const char *func, LOCK(&fctx->lock); } /* We need to do this under the lock for intra-thread synchronization */ - if (fctx->state == fetchstate_done) { + if (fctx->state == fetchstate_sendevents) { UNLOCK(&fctx->lock); return false; } - fctx->state = fetchstate_done; + fctx->state = fetchstate_sendevents; FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); /* The fctx will get deleted either here or in get_attached_fctx() */ @@ -4390,7 +4432,7 @@ resume_qmin(void *arg) { dns_db_attachnode(node, &resp->node); } dns_name_copy(fname, resp->foundname); - clone_results(fctx); + fctx->state = fetchstate_done; UNLOCK(&fctx->lock); goto cleanup; } @@ -4457,7 +4499,7 @@ resume_qmin(void *arg) { dns_db_attachnode(node, &resp->node); } dns_name_copy(fname, resp->foundname); - clone_results(fctx); + fctx->state = fetchstate_done; if (result == DNS_R_CNAME && dns_rdataset_isassociated(&rdataset) && fctx->type == dns_rdatatype_cname) @@ -5243,50 +5285,6 @@ same_question(fetchctx_t *fctx, dns_message_t *message) { return ISC_R_SUCCESS; } -static void -clone_results(fetchctx_t *fctx) { - dns_fetchresponse_t *hresp = NULL; - - REQUIRE(!ISC_LIST_EMPTY(fctx->resps)); - - /* - * Set up any other resps to have the same data as the first. - * Caller must be holding the appropriate lock. - */ - - FCTXTRACE("clone_results"); - - ISC_LIST_FOREACH(fctx->resps, resp, link) { - /* This is the head resp; keep a pointer and move on */ - if (hresp == NULL) { - hresp = ISC_LIST_HEAD(fctx->resps); - FCTXTRACEN("clone_results", hresp->foundname, - hresp->result); - continue; - } - - resp->result = hresp->result; - dns_name_copy(hresp->foundname, resp->foundname); - dns_db_attach(hresp->db, &resp->db); - dns_db_attachnode(hresp->node, &resp->node); - - INSIST(hresp->rdataset != NULL && resp->rdataset != NULL); - if (dns_rdataset_isassociated(hresp->rdataset)) { - dns_rdataset_clone(hresp->rdataset, resp->rdataset); - } - - INSIST(hresp->sigrdataset != NULL || resp->sigrdataset == NULL); - if (resp->sigrdataset != NULL && hresp->sigrdataset != NULL && - dns_rdataset_isassociated(hresp->sigrdataset)) - { - dns_rdataset_clone(hresp->sigrdataset, - resp->sigrdataset); - } - } - - fctx->cloned = true; -} - #define CACHE(r) (((r)->attributes.cache)) #define ANSWER(r) (((r)->attributes.answer)) #define ANSWERSIG(r) (((r)->attributes.answersig)) @@ -5794,7 +5792,7 @@ answer_response: dns_name_copy(val->name, resp->foundname); dns_db_attach(fctx->cache, &resp->db); dns_db_transfernode(fctx->cache, &node, &resp->node); - clone_results(fctx); + fctx->state = fetchstate_done; } done = true; @@ -6250,8 +6248,7 @@ rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) { dns_name_copy(name, resp->foundname); dns_db_attach(fctx->cache, &resp->db); dns_db_transfernode(fctx->cache, &node, &resp->node); - clone_results(fctx); - + fctx->state = fetchstate_done; FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); } } @@ -6442,7 +6439,7 @@ rctx_ncache(respctx_t *rctx) { dns_name_copy(name, resp->foundname); dns_db_attach(fctx->cache, &resp->db); dns_db_transfernode(fctx->cache, &node, &resp->node); - clone_results(fctx); + fctx->state = fetchstate_done; } unlock: @@ -10164,7 +10161,7 @@ get_attached_fctx(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, goto create; } - if (SHUTTINGDOWN(fctx) || fctx->cloned) { + if (SHUTTINGDOWN(fctx)) { /* The fctx will get deleted either here or in fctx__done() */ cds_lfht_del(res->fctxs_ht, &fctx->ht_node); @@ -10419,7 +10416,7 @@ dns_resolver_destroyfetch(dns_fetch_t **fetchp) { * Sanity check: the caller should have gotten its event before * trying to destroy the fetch. */ - if (fctx->state != fetchstate_done) { + if (!SHUTTINGDOWN(fctx)) { ISC_LIST_FOREACH(fctx->resps, resp, link) { RUNTIME_CHECK(resp->fetch != fetch); } @@ -10721,6 +10718,8 @@ dns_resolver_dumpfetches(dns_resolver_t *res, isc_statsformat_t format, char typebuf[DNS_RDATATYPE_FORMATSIZE]; char timebuf[1024]; unsigned int resp_count = 0, query_count = 0; + const char *fetchstatestr[] = { "active", "done", + "sendevents" }; LOCK(&fctx->lock); dns_name_print(fctx->name, fp); @@ -10731,10 +10730,7 @@ dns_resolver_dumpfetches(dns_resolver_t *res, isc_statsformat_t format, dns_rdatatype_format(fctx->type, typebuf, sizeof(typebuf)); fprintf(fp, "/%s (%s), 0x%x: started %s, ", typebuf, - fctx->state == fetchstate_done ? "done" - : fctx->cloned ? "cloned" - : "active", - fctx->options, timebuf); + fetchstatestr[fctx->state], fctx->options, timebuf); ISC_LIST_FOREACH(fctx->resps, resp, link) { resp_count++; From b764d432032cc81d1a508dc5041bfaf869572901 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Thu, 15 Jan 2026 14:47:46 +0100 Subject: [PATCH 2/7] resolver: temporarily store query answer in fetch context Query answers are now stored in dedicated fetch context properties, instead of using `ISC_LIST_HEAD(fctx->resps)`. This reduces lock critical section usage in some places, and enables further simplifications. (In particular, it removes the need for special logic to prepend a fetch response to the list when it contains a sigrdataset.) --- lib/dns/resolver.c | 316 ++++++++++++++++++++++----------------------- 1 file changed, 154 insertions(+), 162 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 4fe66e4b6e..f8dcd24808 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -406,6 +406,18 @@ struct fetchctx { dns_fixedname_t fwdfname; dns_name_t *fwdname; + /*% + * Results from the query needing to be copied to the fetch response + * objects (dns_fetchresponse_t). (Single-thread access thanks to + * loop serialization.) + */ + isc_result_t resp_result; + dns_fixedname_t resp_foundname; + dns_db_t *resp_db; + dns_dbnode_t *resp_node; + dns_rdataset_t resp_rdataset; + dns_rdataset_t resp_sigrdataset; + /*% * Used to track started ADB finds with event. */ @@ -1614,8 +1626,6 @@ spillattimer_countdown(void *arg); static void clone_results(fetchctx_t *fctx) { - dns_fetchresponse_t *hresp = NULL; - REQUIRE(!ISC_LIST_EMPTY(fctx->resps)); /* @@ -1626,29 +1636,21 @@ clone_results(fetchctx_t *fctx) { FCTXTRACE("clone_results"); ISC_LIST_FOREACH(fctx->resps, resp, link) { - /* This is the head resp; keep a pointer and move on */ - if (hresp == NULL) { - hresp = ISC_LIST_HEAD(fctx->resps); - FCTXTRACEN("clone_results", hresp->foundname, - hresp->result); - continue; + resp->result = fctx->resp_result; + dns_name_copy(dns_fixedname_name(&fctx->resp_foundname), + resp->foundname); + dns_db_attach(fctx->resp_db, &resp->db); + dns_db_attachnode(fctx->resp_node, &resp->node); + + if (dns_rdataset_isassociated(&fctx->resp_rdataset)) { + dns_rdataset_clone(&fctx->resp_rdataset, + resp->rdataset); } - resp->result = hresp->result; - dns_name_copy(hresp->foundname, resp->foundname); - dns_db_attach(hresp->db, &resp->db); - dns_db_attachnode(hresp->node, &resp->node); - - INSIST(hresp->rdataset != NULL && resp->rdataset != NULL); - if (dns_rdataset_isassociated(hresp->rdataset)) { - dns_rdataset_clone(hresp->rdataset, resp->rdataset); - } - - INSIST(hresp->sigrdataset != NULL || resp->sigrdataset == NULL); - if (resp->sigrdataset != NULL && hresp->sigrdataset != NULL && - dns_rdataset_isassociated(hresp->sigrdataset)) + if (resp->sigrdataset != NULL && + dns_rdataset_isassociated(&fctx->resp_sigrdataset)) { - dns_rdataset_clone(hresp->sigrdataset, + dns_rdataset_clone(&fctx->resp_sigrdataset, resp->sigrdataset); } } @@ -4412,31 +4414,26 @@ resume_qmin(void *arg) { if (result == DNS_R_NXDOMAIN && fctx->qmin_labels == dns_name_countlabels(fctx->name)) { - LOCK(&fctx->lock); - resp = ISC_LIST_HEAD(fctx->resps); - if (resp != NULL) { - if (dns_rdataset_isassociated(&rdataset)) { - dns_rdataset_clone(&rdataset, - resp->rdataset); - } - if (dns_rdataset_isassociated(&sigrdataset) && - resp->sigrdataset != NULL) - { - dns_rdataset_clone(&sigrdataset, - resp->sigrdataset); - } - if (db != NULL) { - dns_db_attach(db, &resp->db); - } - if (node != NULL) { - dns_db_attachnode(node, &resp->node); - } - dns_name_copy(fname, resp->foundname); - fctx->state = fetchstate_done; - UNLOCK(&fctx->lock); - goto cleanup; + if (dns_rdataset_isassociated(&rdataset)) { + dns_rdataset_clone(&rdataset, + &fctx->resp_rdataset); } + if (dns_rdataset_isassociated(&sigrdataset)) { + dns_rdataset_clone(&sigrdataset, + &fctx->resp_sigrdataset); + } + if (db != NULL) { + dns_db_attach(db, &fctx->resp_db); + } + if (node != NULL) { + dns_db_attachnode(node, &fctx->resp_node); + } + dns_name_copy(fname, dns_fixedname_name( + &fctx->resp_foundname)); + LOCK(&fctx->lock); + fctx->state = fetchstate_done; UNLOCK(&fctx->lock); + goto cleanup; } /* ...or disable minimization in relaxed mode */ @@ -4479,39 +4476,42 @@ resume_qmin(void *arg) { fctx->type != dns_rdatatype_sig && fctx->type != dns_rdatatype_rrsig) { - LOCK(&fctx->lock); - resp = ISC_LIST_HEAD(fctx->resps); - if (resp != NULL) { - if (dns_rdataset_isassociated(&rdataset)) { - dns_rdataset_clone(&rdataset, - resp->rdataset); - } - if (dns_rdataset_isassociated(&sigrdataset) && - resp->sigrdataset != NULL) - { - dns_rdataset_clone(&sigrdataset, - resp->sigrdataset); - } - if (db != NULL) { - dns_db_attach(db, &resp->db); - } - if (node != NULL) { - dns_db_attachnode(node, &resp->node); - } - dns_name_copy(fname, resp->foundname); - fctx->state = fetchstate_done; - if (result == DNS_R_CNAME && - dns_rdataset_isassociated(&rdataset) && - fctx->type == dns_rdatatype_cname) - { - fctx_success_unref(fctx); - result = ISC_R_SUCCESS; - } else { - UNLOCK(&fctx->lock); - } - goto cleanup; + if (dns_rdataset_isassociated(&rdataset)) { + dns_rdataset_clone(&rdataset, + &fctx->resp_rdataset); } - UNLOCK(&fctx->lock); + if (dns_rdataset_isassociated(&sigrdataset)) { + dns_rdataset_clone(&sigrdataset, + &fctx->resp_sigrdataset); + } + if (db != NULL) { + dns_db_attach(db, &fctx->resp_db); + } + if (node != NULL) { + dns_db_attachnode(node, &fctx->resp_node); + } + dns_name_copy(fname, dns_fixedname_name( + &fctx->resp_foundname)); + + LOCK(&fctx->lock); + fctx->state = fetchstate_done; + if (result == DNS_R_CNAME && + dns_rdataset_isassociated(&rdataset) && + fctx->type == dns_rdatatype_cname) + { + fctx_success_unref(fctx); + result = ISC_R_SUCCESS; + /* + * Yes, we're not releasing the lock, because + * `fctx_success_unref()` directly goes into the + * `fctx__done()` flow which expect fctx to be + * locked in case of success. Spicy... + */ + } else { + UNLOCK(&fctx->lock); + } + + goto cleanup; } /* @@ -4665,6 +4665,15 @@ fctx__destroy(fetchctx_t *fctx, const char *func, const char *file, dns_ede_invalidate(&fctx->edectx); + if (fctx->resp_db != NULL) { + dns_db_detach(&fctx->resp_db); + } + if (fctx->resp_node != NULL) { + dns_db_detachnode(&fctx->resp_node); + } + dns_rdataset_cleanup(&fctx->resp_rdataset); + dns_rdataset_cleanup(&fctx->resp_sigrdataset); + isc_mem_free(fctx->mctx, fctx->info); call_rcu(&fctx->rcu_head, fctx_destroy_rcu); @@ -4831,31 +4840,32 @@ fctx__create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, REQUIRE(fctxp != NULL && *fctxp == NULL); fctx = isc_mem_get(mctx, sizeof(*fctx)); - *fctx = (fetchctx_t){ - .type = type, - .qmintype = type, - .options = options, - .tid = isc_tid(), - .state = fetchstate_active, - .depth = depth, - .qmin_labels = 1, - .fwdpolicy = dns_fwdpolicy_none, - .result = ISC_R_FAILURE, - .loop = loop, - .queries = ISC_LIST_INITIALIZER, - .finds = ISC_LIST_INITIALIZER, - .altfinds = ISC_LIST_INITIALIZER, - .forwaddrs = ISC_LIST_INITIALIZER, - .altaddrs = ISC_LIST_INITIALIZER, - .forwarders = ISC_LIST_INITIALIZER, - .bad = ISC_LIST_INITIALIZER, - .edns = ISC_LIST_INITIALIZER, - .validators = ISC_LIST_INITIALIZER, - .nameservers = DNS_RDATASET_INIT, - .qminrrset = DNS_RDATASET_INIT, - .qminsigrrset = DNS_RDATASET_INIT, - .nsrrset = DNS_RDATASET_INIT, - }; + *fctx = (fetchctx_t){ .type = type, + .qmintype = type, + .options = options, + .tid = isc_tid(), + .state = fetchstate_active, + .depth = depth, + .qmin_labels = 1, + .fwdpolicy = dns_fwdpolicy_none, + .result = ISC_R_FAILURE, + .loop = loop, + .queries = ISC_LIST_INITIALIZER, + .finds = ISC_LIST_INITIALIZER, + .altfinds = ISC_LIST_INITIALIZER, + .forwaddrs = ISC_LIST_INITIALIZER, + .altaddrs = ISC_LIST_INITIALIZER, + .forwarders = ISC_LIST_INITIALIZER, + .bad = ISC_LIST_INITIALIZER, + .edns = ISC_LIST_INITIALIZER, + .validators = ISC_LIST_INITIALIZER, + .nameservers = DNS_RDATASET_INIT, + .qminrrset = DNS_RDATASET_INIT, + .qminsigrrset = DNS_RDATASET_INIT, + .nsrrset = DNS_RDATASET_INIT, + .resp_result = DNS_R_SERVFAIL, + .resp_rdataset = DNS_RDATASET_INIT, + .resp_sigrdataset = DNS_RDATASET_INIT }; isc_mem_attach(mctx, &fctx->mctx); dns_resolver_attach(res, &fctx->res); @@ -4870,6 +4880,7 @@ fctx__create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, fctx->qminname = dns_fixedname_initname(&fctx->qminfname); fctx->qmindcname = dns_fixedname_initname(&fctx->qmindcfname); fctx->fwdname = dns_fixedname_initname(&fctx->fwdfname); + dns_fixedname_init(&fctx->resp_foundname); dns_name_copy(name, fctx->name); dns_name_copy(name, fctx->qminname); @@ -5380,27 +5391,28 @@ has_000_label(dns_rdataset_t *nsecset) { return false; } -static isc_result_t -fctx_setresult(fetchctx_t *fctx, dns_rdataset_t *rdataset) { - isc_result_t eresult = ISC_R_SUCCESS; +static void +fctx_setresult(fetchctx_t *fctx) { + isc_result_t result = ISC_R_SUCCESS; + dns_rdataset_t *rdataset = &fctx->resp_rdataset; if (NEGATIVE(rdataset)) { - eresult = NXDOMAIN(rdataset) ? DNS_R_NCACHENXDOMAIN - : DNS_R_NCACHENXRRSET; - } else if (eresult == ISC_R_SUCCESS && rdataset->type != fctx->type) { + result = NXDOMAIN(rdataset) ? DNS_R_NCACHENXDOMAIN + : DNS_R_NCACHENXRRSET; + } else if (result == ISC_R_SUCCESS && rdataset->type != fctx->type) { switch (rdataset->type) { case dns_rdatatype_cname: - eresult = DNS_R_CNAME; + result = DNS_R_CNAME; break; case dns_rdatatype_dname: - eresult = DNS_R_DNAME; + result = DNS_R_DNAME; break; default: break; } } - return eresult; + fctx->resp_result = result; } static inline dns_trust_t @@ -5615,7 +5627,6 @@ validated(void *arg) { dns_validator_t *nextval = NULL; dns_adbaddrinfo_t *addrinfo = NULL; dns_dbnode_t *node = NULL; - dns_fetchresponse_t *resp = NULL; dns_rdataset_t *ardataset = NULL, *asigrdataset = NULL; dns_message_t *message = NULL; fetchctx_t *fctx = NULL; @@ -5696,12 +5707,9 @@ validated(void *arg) { * we pass an rdataset back to the caller. Otherwise the caller * iterates the node. */ - resp = ISC_LIST_HEAD(fctx->resps); - if (resp != NULL && - (negative || chaining || !dns_rdatatype_ismulti(fctx->type))) - { - ardataset = resp->rdataset; - asigrdataset = resp->sigrdataset; + if (negative || chaining || !dns_rdatatype_ismulti(fctx->type)) { + ardataset = &fctx->resp_rdataset; + asigrdataset = &fctx->resp_sigrdataset; } /* @@ -5785,15 +5793,13 @@ answer_response: * We're responding with an answer, positive or negative, * not an error. */ - if (resp != NULL) { - FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); + FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); - resp->result = fctx_setresult(fctx, resp->rdataset); - dns_name_copy(val->name, resp->foundname); - dns_db_attach(fctx->cache, &resp->db); - dns_db_transfernode(fctx->cache, &node, &resp->node); - fctx->state = fetchstate_done; - } + fctx_setresult(fctx); + dns_name_copy(val->name, dns_fixedname_name(&fctx->resp_foundname)); + dns_db_attach(fctx->cache, &fctx->resp_db); + dns_db_transfernode(fctx->cache, &node, &fctx->resp_node); + fctx->state = fetchstate_done; done = true; @@ -6019,7 +6025,6 @@ rctx_cache_secure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, fetchctx_t *fctx = rctx->fctx; resquery_t *query = rctx->query; dns_rdataset_t *ardataset = NULL, *asigset = NULL; - dns_fetchresponse_t *resp = ISC_LIST_HEAD(fctx->resps); /* * RRSIGs are validated as part of validating the type they cover. @@ -6084,12 +6089,11 @@ rctx_cache_secure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, * but we got a CNAME/DNAME - then we need to * set up rdatasets to send back to the caller. */ - if (resp != NULL && - (!dns_rdatatype_ismulti(fctx->type) || - CHAINING(rdataset))) + if (!dns_rdatatype_ismulti(fctx->type) || + CHAINING(rdataset)) { - ardataset = resp->rdataset; - asigset = resp->sigrdataset; + ardataset = &fctx->resp_rdataset; + asigset = &fctx->resp_sigrdataset; } } @@ -6112,7 +6116,6 @@ rctx_cache_insecure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, dns_rdataset_t *sigrdataset) { isc_result_t result; fetchctx_t *fctx = rctx->fctx; - dns_fetchresponse_t *resp = ISC_LIST_HEAD(fctx->resps); dns_rdataset_t *added = NULL; /* @@ -6120,13 +6123,11 @@ rctx_cache_insecure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, * CNAME/DNAME - then we need to set up an rdataset to send * back to the caller. */ - if (resp != NULL && - (!dns_rdatatype_ismulti(fctx->type) || CHAINING(rdataset))) - { + if (!dns_rdatatype_ismulti(fctx->type) || CHAINING(rdataset)) { if (ANSWER(rdataset)) { - added = resp->rdataset; + added = &fctx->resp_rdataset; } else if (ANSWERSIG(rdataset)) { - added = resp->sigrdataset; + added = &fctx->resp_sigrdataset; } } @@ -6236,21 +6237,16 @@ rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) { * it back to the caller. */ if (!need_validation && name->attributes.answer && !HAVE_ANSWER(fctx)) { - dns_fetchresponse_t *resp = ISC_LIST_HEAD(fctx->resps); - if (resp != NULL) { - isc_result_t eresult = ISC_R_SUCCESS; + fctx->resp_result = ISC_R_SUCCESS; - if (dns_rdataset_isassociated(resp->rdataset)) { - eresult = fctx_setresult(fctx, resp->rdataset); - } - - resp->result = eresult; - dns_name_copy(name, resp->foundname); - dns_db_attach(fctx->cache, &resp->db); - dns_db_transfernode(fctx->cache, &node, &resp->node); - fctx->state = fetchstate_done; - FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); + if (dns_rdataset_isassociated(&fctx->resp_rdataset)) { + fctx_setresult(fctx); } + dns_name_copy(name, dns_fixedname_name(&fctx->resp_foundname)); + dns_db_attach(fctx->cache, &fctx->resp_db); + dns_db_transfernode(fctx->cache, &node, &fctx->resp_node); + fctx->state = fetchstate_done; + FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); } cleanup: @@ -6369,7 +6365,6 @@ rctx_ncache(respctx_t *rctx) { dns_message_t *message = rctx->query->rmessage; dns_adbaddrinfo_t *addrinfo = rctx->query->addrinfo; dns_dbnode_t *node = NULL; - dns_fetchresponse_t *resp = NULL; dns_rdataset_t *added = NULL; FCTXTRACE("rctx_ncache"); @@ -6422,9 +6417,8 @@ rctx_ncache(respctx_t *rctx) { * Cache the negative answer, and copy it into the fetch response. */ LOCK(&fctx->lock); - resp = ISC_LIST_HEAD(fctx->resps); - if (!HAVE_ANSWER(fctx) && resp != NULL) { - added = resp->rdataset; + if (!HAVE_ANSWER(fctx)) { + added = &fctx->resp_rdataset; } result = negcache(message, fctx, name, rctx->now, false, false, added, @@ -6434,13 +6428,11 @@ rctx_ncache(respctx_t *rctx) { } FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); - if (resp != NULL) { - resp->result = fctx_setresult(fctx, resp->rdataset); - dns_name_copy(name, resp->foundname); - dns_db_attach(fctx->cache, &resp->db); - dns_db_transfernode(fctx->cache, &node, &resp->node); - fctx->state = fetchstate_done; - } + fctx_setresult(fctx); + dns_name_copy(name, dns_fixedname_name(&fctx->resp_foundname)); + dns_db_attach(fctx->cache, &fctx->resp_db); + dns_db_transfernode(fctx->cache, &node, &fctx->resp_node); + fctx->state = fetchstate_done; unlock: UNLOCK(&fctx->lock); From a5b2a8c931caaa60ab292968152b4c2fb31fd058 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Thu, 15 Jan 2026 16:30:59 +0100 Subject: [PATCH 3/7] resolver: simplify fetch response handling There is no longer a need to decide whether a fetch response should be prepended or appended to the fetch response list. As query response data is stored directly in the fetch context object, responses containing a sigrdataset no longer need to be ordered first. Remove the code implementing this logic. Additionally, the distinction between `fetchstate_done` and `fetchstate_sendevents` is no longer needed. New clients `dns_fetchresponse_t` can be attached any time to the fetch context until `fctx__done()` is called, since there is no dependency on the first fetch response in the list. This simplifies the code and reduces (just a bit) locking usage. --- lib/dns/resolver.c | 43 +++++++++++-------------------------------- 1 file changed, 11 insertions(+), 32 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index f8dcd24808..aae71053f0 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -299,9 +299,8 @@ struct tried { #define RESQUERY_SENDING(q) ((q)->sends > 0) typedef enum { - fetchstate_active = 0, - fetchstate_done, - fetchstate_sendevents /*%< Fetch completion events posted. */ + fetchstate_active, + fetchstate_done /*%< Fetch completion events posted. */ } fetchstate_t; typedef enum { @@ -508,7 +507,7 @@ struct fetchctx { ((atomic_load_acquire(&(f)->attributes) & FCTX_ATTR_GLUING) != 0) #define ADDRWAIT(f) \ ((atomic_load_acquire(&(f)->attributes) & FCTX_ATTR_ADDRWAIT) != 0) -#define SHUTTINGDOWN(f) ((f)->state >= fetchstate_done) +#define SHUTTINGDOWN(f) ((f)->state == fetchstate_done) #define WANTCACHE(f) \ ((atomic_load_acquire(&(f)->attributes) & FCTX_ATTR_WANTCACHE) != 0) #define WANTNCACHE(f) \ @@ -1667,7 +1666,7 @@ fctx_sendevents(fetchctx_t *fctx, isc_result_t result) { LOCK(&fctx->lock); - REQUIRE(fctx->state == fetchstate_sendevents); + REQUIRE(fctx->state == fetchstate_done); if (result == ISC_R_SUCCESS) { clone_results(fctx); @@ -1804,11 +1803,11 @@ fctx__done(fetchctx_t *fctx, isc_result_t result, const char *func, LOCK(&fctx->lock); } /* We need to do this under the lock for intra-thread synchronization */ - if (fctx->state == fetchstate_sendevents) { + if (fctx->state == fetchstate_done) { UNLOCK(&fctx->lock); return false; } - fctx->state = fetchstate_sendevents; + fctx->state = fetchstate_done; FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); /* The fctx will get deleted either here or in get_attached_fctx() */ @@ -4430,9 +4429,6 @@ resume_qmin(void *arg) { } dns_name_copy(fname, dns_fixedname_name( &fctx->resp_foundname)); - LOCK(&fctx->lock); - fctx->state = fetchstate_done; - UNLOCK(&fctx->lock); goto cleanup; } @@ -4493,22 +4489,18 @@ resume_qmin(void *arg) { dns_name_copy(fname, dns_fixedname_name( &fctx->resp_foundname)); - LOCK(&fctx->lock); - fctx->state = fetchstate_done; if (result == DNS_R_CNAME && dns_rdataset_isassociated(&rdataset) && fctx->type == dns_rdatatype_cname) { + LOCK(&fctx->lock); fctx_success_unref(fctx); result = ISC_R_SUCCESS; /* - * Yes, we're not releasing the lock, because * `fctx_success_unref()` directly goes into the * `fctx__done()` flow which expect fctx to be - * locked in case of success. Spicy... + * locked in case of success. */ - } else { - UNLOCK(&fctx->lock); } goto cleanup; @@ -4770,16 +4762,7 @@ fctx_add_event(fetchctx_t *fctx, isc_loop_t *loop, const isc_sockaddr_t *client, isc_mem_attach(fctx->mctx, &resp->mctx); resp->foundname = dns_fixedname_initname(&resp->fname); - - /* - * Store the sigrdataset in the first resp in case it is needed - * by any of the events. - */ - if (resp->sigrdataset != NULL) { - ISC_LIST_PREPEND(fctx->resps, resp, link); - } else { - ISC_LIST_APPEND(fctx->resps, resp, link); - } + ISC_LIST_APPEND(fctx->resps, resp, link); } static void @@ -5799,7 +5782,6 @@ answer_response: dns_name_copy(val->name, dns_fixedname_name(&fctx->resp_foundname)); dns_db_attach(fctx->cache, &fctx->resp_db); dns_db_transfernode(fctx->cache, &node, &fctx->resp_node); - fctx->state = fetchstate_done; done = true; @@ -6245,7 +6227,6 @@ rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) { dns_name_copy(name, dns_fixedname_name(&fctx->resp_foundname)); dns_db_attach(fctx->cache, &fctx->resp_db); dns_db_transfernode(fctx->cache, &node, &fctx->resp_node); - fctx->state = fetchstate_done; FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); } @@ -6432,7 +6413,6 @@ rctx_ncache(respctx_t *rctx) { dns_name_copy(name, dns_fixedname_name(&fctx->resp_foundname)); dns_db_attach(fctx->cache, &fctx->resp_db); dns_db_transfernode(fctx->cache, &node, &fctx->resp_node); - fctx->state = fetchstate_done; unlock: UNLOCK(&fctx->lock); @@ -10710,8 +10690,6 @@ dns_resolver_dumpfetches(dns_resolver_t *res, isc_statsformat_t format, char typebuf[DNS_RDATATYPE_FORMATSIZE]; char timebuf[1024]; unsigned int resp_count = 0, query_count = 0; - const char *fetchstatestr[] = { "active", "done", - "sendevents" }; LOCK(&fctx->lock); dns_name_print(fctx->name, fp); @@ -10722,7 +10700,8 @@ dns_resolver_dumpfetches(dns_resolver_t *res, isc_statsformat_t format, dns_rdatatype_format(fctx->type, typebuf, sizeof(typebuf)); fprintf(fp, "/%s (%s), 0x%x: started %s, ", typebuf, - fetchstatestr[fctx->state], fctx->options, timebuf); + fctx->state == fetchstate_done ? "done" : "active", + fctx->options, timebuf); ISC_LIST_FOREACH(fctx->resps, resp, link) { resp_count++; From 5972ee2cd599036d09f171a94701a6187fc6e668 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Thu, 15 Jan 2026 17:36:50 +0100 Subject: [PATCH 4/7] resolver: copy fetch responses and send events in one go Instead of first copying query response data into each fetch response and then iterating again to send the response to the caller, perform both operations in one go. Also removed some duplicate code. --- lib/dns/resolver.c | 159 +++++++++++++++++---------------------------- 1 file changed, 61 insertions(+), 98 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index aae71053f0..dbc3a3bb8d 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -1624,35 +1624,41 @@ static void spillattimer_countdown(void *arg); static void -clone_results(fetchctx_t *fctx) { - REQUIRE(!ISC_LIST_EMPTY(fctx->resps)); +copy_to_resp(fetchctx_t *fctx, dns_fetchresponse_t *resp) { + resp->result = fctx->resp_result; - /* - * Set up any other resps to have the same data as the first. - * Caller must be holding the appropriate lock. - */ + dns_name_copy(dns_fixedname_name(&fctx->resp_foundname), + resp->foundname); - FCTXTRACE("clone_results"); + dns_db_attach(fctx->resp_db, &resp->db); + dns_db_attachnode(fctx->resp_node, &resp->node); - ISC_LIST_FOREACH(fctx->resps, resp, link) { - resp->result = fctx->resp_result; - dns_name_copy(dns_fixedname_name(&fctx->resp_foundname), - resp->foundname); - dns_db_attach(fctx->resp_db, &resp->db); - dns_db_attachnode(fctx->resp_node, &resp->node); - - if (dns_rdataset_isassociated(&fctx->resp_rdataset)) { - dns_rdataset_clone(&fctx->resp_rdataset, - resp->rdataset); - } - - if (resp->sigrdataset != NULL && - dns_rdataset_isassociated(&fctx->resp_sigrdataset)) - { - dns_rdataset_clone(&fctx->resp_sigrdataset, - resp->sigrdataset); - } + if (dns_rdataset_isassociated(&fctx->resp_rdataset)) { + dns_rdataset_clone(&fctx->resp_rdataset, resp->rdataset); } + if (resp->sigrdataset != NULL && + dns_rdataset_isassociated(&fctx->resp_sigrdataset)) + { + dns_rdataset_clone(&fctx->resp_sigrdataset, resp->sigrdataset); + } +} + +static void +pull_from_resp(dns_fetchresponse_t *resp, fetchctx_t *fctx) { + if (dns_rdataset_isassociated(resp->rdataset)) { + dns_rdataset_clone(resp->rdataset, &fctx->resp_rdataset); + } + if (dns_rdataset_isassociated(resp->sigrdataset)) { + dns_rdataset_clone(resp->sigrdataset, &fctx->resp_sigrdataset); + } + if (resp->db != NULL) { + dns_db_attach(resp->db, &fctx->resp_db); + } + if (resp->node != NULL) { + dns_db_attachnode(resp->node, &fctx->resp_node); + } + dns_name_copy(resp->foundname, + dns_fixedname_name(&fctx->resp_foundname)); } static void @@ -1668,9 +1674,6 @@ fctx_sendevents(fetchctx_t *fctx, isc_result_t result) { REQUIRE(fctx->state == fetchstate_done); - if (result == ISC_R_SUCCESS) { - clone_results(fctx); - } FCTXTRACE("sendevents"); /* @@ -1683,6 +1686,9 @@ fctx_sendevents(fetchctx_t *fctx, isc_result_t result) { ISC_LIST_FOREACH(fctx->resps, resp, link) { ISC_LIST_UNLINK(fctx->resps, resp, link); + if (result == ISC_R_SUCCESS) { + copy_to_resp(fctx, resp); + } count++; resp->vresult = fctx->vresult; @@ -4331,6 +4337,26 @@ done: } } +static void +clear_resp(dns_fetchresponse_t **respp) { + dns_fetchresponse_t *resp = *respp; + + if (resp == NULL) { + return; + } + + if (resp->node != NULL) { + dns_db_detachnode(&resp->node); + } + if (resp->db != NULL) { + dns_db_detach(&resp->db); + } + dns_rdataset_cleanup(resp->rdataset); + dns_rdataset_cleanup(resp->sigrdataset); + + dns_resolver_freefresp(respp); +} + static void resume_qmin(void *arg) { dns_fetchresponse_t *resp = (dns_fetchresponse_t *)arg; @@ -4340,10 +4366,6 @@ resume_qmin(void *arg) { unsigned int findoptions = 0; dns_name_t *fname = NULL, *dcname = NULL; dns_fixedname_t ffixed, dcfixed; - dns_rdataset_t rdataset; - dns_rdataset_t sigrdataset; - dns_db_t *db = NULL; - dns_dbnode_t *node = NULL; REQUIRE(VALID_FCTX(fctx)); @@ -4356,32 +4378,8 @@ resume_qmin(void *arg) { fname = dns_fixedname_initname(&ffixed); dcname = dns_fixedname_initname(&dcfixed); - dns_rdataset_init(&rdataset); - dns_rdataset_init(&sigrdataset); - - if (resp->node != NULL) { - dns_db_attachnode(resp->node, &node); - dns_db_detachnode(&resp->node); - } - if (resp->db != NULL) { - dns_db_attach(resp->db, &db); - dns_db_detach(&resp->db); - } - - if (dns_rdataset_isassociated(resp->rdataset)) { - dns_rdataset_clone(resp->rdataset, &rdataset); - dns_rdataset_disassociate(resp->rdataset); - } - if (dns_rdataset_isassociated(resp->sigrdataset)) { - dns_rdataset_clone(resp->sigrdataset, &sigrdataset); - dns_rdataset_disassociate(resp->sigrdataset); - } - dns_name_copy(resp->foundname, fname); - result = resp->result; - dns_resolver_freefresp(&resp); - LOCK(&fctx->lock); if (SHUTTINGDOWN(fctx)) { result = ISC_R_SHUTTINGDOWN; @@ -4413,22 +4411,7 @@ resume_qmin(void *arg) { if (result == DNS_R_NXDOMAIN && fctx->qmin_labels == dns_name_countlabels(fctx->name)) { - if (dns_rdataset_isassociated(&rdataset)) { - dns_rdataset_clone(&rdataset, - &fctx->resp_rdataset); - } - if (dns_rdataset_isassociated(&sigrdataset)) { - dns_rdataset_clone(&sigrdataset, - &fctx->resp_sigrdataset); - } - if (db != NULL) { - dns_db_attach(db, &fctx->resp_db); - } - if (node != NULL) { - dns_db_attachnode(node, &fctx->resp_node); - } - dns_name_copy(fname, dns_fixedname_name( - &fctx->resp_foundname)); + pull_from_resp(resp, fctx); goto cleanup; } @@ -4472,25 +4455,10 @@ resume_qmin(void *arg) { fctx->type != dns_rdatatype_sig && fctx->type != dns_rdatatype_rrsig) { - if (dns_rdataset_isassociated(&rdataset)) { - dns_rdataset_clone(&rdataset, - &fctx->resp_rdataset); - } - if (dns_rdataset_isassociated(&sigrdataset)) { - dns_rdataset_clone(&sigrdataset, - &fctx->resp_sigrdataset); - } - if (db != NULL) { - dns_db_attach(db, &fctx->resp_db); - } - if (node != NULL) { - dns_db_attachnode(node, &fctx->resp_node); - } - dns_name_copy(fname, dns_fixedname_name( - &fctx->resp_foundname)); + pull_from_resp(resp, fctx); if (result == DNS_R_CNAME && - dns_rdataset_isassociated(&rdataset) && + dns_rdataset_isassociated(resp->rdataset) && fctx->type == dns_rdatatype_cname) { LOCK(&fctx->lock); @@ -4525,6 +4493,7 @@ resume_qmin(void *arg) { break; } + clear_resp(&resp); dns_rdataset_cleanup(&fctx->nameservers); if (dns_rdatatype_atparent(fctx->type)) { @@ -4564,14 +4533,8 @@ resume_qmin(void *arg) { fctx_try(fctx, true); cleanup: - if (node != NULL) { - dns_db_detachnode(&node); - } - if (db != NULL) { - dns_db_detach(&db); - } - dns_rdataset_cleanup(&rdataset); - dns_rdataset_cleanup(&sigrdataset); + clear_resp(&resp); + if (result != ISC_R_SUCCESS) { /* An error occurred, tear down whole fctx */ fctx_failure_unref(fctx, result); @@ -10859,7 +10822,7 @@ dns_resolver_freefresp(dns_fetchresponse_t **frespp) { } dns_fetchresponse_t *fresp = *frespp; - *frespp = NULL; + isc_mem_putanddetach(&fresp->mctx, fresp, sizeof(*fresp)); } From fd526c0ad0e3251035a25677511bd0daa733173f Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Fri, 16 Jan 2026 12:14:58 +0100 Subject: [PATCH 5/7] resolver: remove `qminrrset`, `qminsigrrset` from fctx Two rdataset property `qminrrset` and `qminsigrrset` are removed from the fetch context. They only are used as temporary storage for the query result of the qmin query, and are immediately detached from `resume_qmin` once the query is over. As an alternative, use `resp_rdataset` and `resp_sigrdataset` instead; those are not needed for storing the response data until after qmin_resume() is over. --- lib/dns/resolver.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index dbc3a3bb8d..92e18c223d 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -398,8 +398,6 @@ struct fetchctx { dns_name_t *qminname; dns_rdatatype_t qmintype; dns_fetch_t *qminfetch; - dns_rdataset_t qminrrset; - dns_rdataset_t qminsigrrset; dns_fixedname_t qmindcfname; dns_name_t *qmindcname; dns_fixedname_t fwdfname; @@ -4284,8 +4282,8 @@ fctx_try(fetchctx_t *fctx, bool retrying) { &fctx->nameservers, NULL, NULL, 0, options | DNS_FETCHOPT_QMINFETCH, 0, fctx->qc, fctx->gqc, fctx, fctx->loop, resume_qmin, fctx, - &fctx->edectx, &fctx->qminrrset, &fctx->qminsigrrset, - &fctx->qminfetch); + &fctx->edectx, &fctx->resp_rdataset, + &fctx->resp_sigrdataset, &fctx->qminfetch); if (result != ISC_R_SUCCESS) { fetchctx_unref(fctx); goto done; @@ -4806,8 +4804,6 @@ fctx__create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, .edns = ISC_LIST_INITIALIZER, .validators = ISC_LIST_INITIALIZER, .nameservers = DNS_RDATASET_INIT, - .qminrrset = DNS_RDATASET_INIT, - .qminsigrrset = DNS_RDATASET_INIT, .nsrrset = DNS_RDATASET_INIT, .resp_result = DNS_R_SERVFAIL, .resp_rdataset = DNS_RDATASET_INIT, From feed0fb43c6fa4bdd43e95b82e72181f7b77d31f Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 16 Jan 2026 20:59:08 -0800 Subject: [PATCH 6/7] use a union for resp and qmin data It's potentially confusing to use "resp_rdataset" for QNAME minimization, but we can make it a union and have resp.rdataset and qmin.rdataset using the same memory. We can save even more space by using the same union to combine qminname and resp_foundname and access them as qmin.name and resp.foundname. --- lib/dns/resolver.c | 143 ++++++++++++++++++++++++--------------------- 1 file changed, 76 insertions(+), 67 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 92e18c223d..56313bc2b4 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -389,31 +389,49 @@ struct fetchctx { isc_counter_t *qc; isc_counter_t *gqc; bool minimized; + dns_fixedname_t fwdfname; + dns_name_t *fwdname; + bool forwarding; + + /*% + * These hold state information regarding QNAME minimization. + */ unsigned int qmin_labels; isc_result_t qmin_warning; bool force_qmin_warning; bool ip6arpaskip; - bool forwarding; - dns_fixedname_t qminfname; - dns_name_t *qminname; dns_rdatatype_t qmintype; dns_fetch_t *qminfetch; - dns_fixedname_t qmindcfname; - dns_name_t *qmindcname; - dns_fixedname_t fwdfname; - dns_name_t *fwdname; /*% - * Results from the query needing to be copied to the fetch response - * objects (dns_fetchresponse_t). (Single-thread access thanks to - * loop serialization.) + * These are results from the query that need to be copied to the + * response objects (dns_fetchresponse_t). */ isc_result_t resp_result; - dns_fixedname_t resp_foundname; - dns_db_t *resp_db; dns_dbnode_t *resp_node; - dns_rdataset_t resp_rdataset; - dns_rdataset_t resp_sigrdataset; + + /*% + * These are used both during the QNAME minimization process, and + * also to store final query results to be copied into the fetch + * response. These uses never occur at the same time, so we can + * save space in the fetchctx object by making them a union. + */ + union { + struct { + dns_rdataset_t rdataset; + dns_rdataset_t sigrdataset; + dns_fixedname_t fname; + dns_name_t *name; + dns_fixedname_t dcfname; + dns_name_t *dcname; + } qmin; + struct { + dns_rdataset_t rdataset; + dns_rdataset_t sigrdataset; + dns_fixedname_t fname; + dns_name_t *foundname; + } resp; + }; /*% * Used to track started ADB finds with event. @@ -1625,38 +1643,36 @@ static void copy_to_resp(fetchctx_t *fctx, dns_fetchresponse_t *resp) { resp->result = fctx->resp_result; - dns_name_copy(dns_fixedname_name(&fctx->resp_foundname), - resp->foundname); + dns_name_copy(fctx->resp.foundname, resp->foundname); - dns_db_attach(fctx->resp_db, &resp->db); + dns_db_attach(fctx->cache, &resp->db); dns_db_attachnode(fctx->resp_node, &resp->node); - if (dns_rdataset_isassociated(&fctx->resp_rdataset)) { - dns_rdataset_clone(&fctx->resp_rdataset, resp->rdataset); + if (dns_rdataset_isassociated(&fctx->resp.rdataset)) { + dns_rdataset_clone(&fctx->resp.rdataset, resp->rdataset); } if (resp->sigrdataset != NULL && - dns_rdataset_isassociated(&fctx->resp_sigrdataset)) + dns_rdataset_isassociated(&fctx->resp.sigrdataset)) { - dns_rdataset_clone(&fctx->resp_sigrdataset, resp->sigrdataset); + dns_rdataset_clone(&fctx->resp.sigrdataset, resp->sigrdataset); } } static void pull_from_resp(dns_fetchresponse_t *resp, fetchctx_t *fctx) { if (dns_rdataset_isassociated(resp->rdataset)) { - dns_rdataset_clone(resp->rdataset, &fctx->resp_rdataset); + dns_rdataset_clone(resp->rdataset, &fctx->resp.rdataset); } if (dns_rdataset_isassociated(resp->sigrdataset)) { - dns_rdataset_clone(resp->sigrdataset, &fctx->resp_sigrdataset); + dns_rdataset_clone(resp->sigrdataset, &fctx->resp.sigrdataset); } if (resp->db != NULL) { - dns_db_attach(resp->db, &fctx->resp_db); + INSIST(resp->db == fctx->cache); } if (resp->node != NULL) { dns_db_attachnode(resp->node, &fctx->resp_node); } - dns_name_copy(resp->foundname, - dns_fixedname_name(&fctx->resp_foundname)); + dns_name_copy(resp->foundname, fctx->resp.foundname); } static void @@ -4247,7 +4263,7 @@ fctx_try(fetchctx_t *fctx, bool retrying) { char namebuf[DNS_NAME_FORMATSIZE]; char typebuf[DNS_RDATATYPE_FORMATSIZE]; - dns_name_format(fctx->qminname, namebuf, + dns_name_format(fctx->qmin.name, namebuf, sizeof(namebuf)); dns_rdatatype_format(fctx->qmintype, typebuf, sizeof(typebuf)); @@ -4278,12 +4294,12 @@ fctx_try(fetchctx_t *fctx, bool retrying) { fetchctx_ref(fctx); result = dns_resolver_createfetch( - fctx->res, fctx->qminname, fctx->qmintype, fctx->domain, - &fctx->nameservers, NULL, NULL, 0, + fctx->res, fctx->qmin.name, fctx->qmintype, + fctx->domain, &fctx->nameservers, NULL, NULL, 0, options | DNS_FETCHOPT_QMINFETCH, 0, fctx->qc, fctx->gqc, fctx, fctx->loop, resume_qmin, fctx, - &fctx->edectx, &fctx->resp_rdataset, - &fctx->resp_sigrdataset, &fctx->qminfetch); + &fctx->edectx, &fctx->qmin.rdataset, + &fctx->qmin.sigrdataset, &fctx->qminfetch); if (result != ISC_R_SUCCESS) { fetchctx_unref(fctx); goto done; @@ -4511,7 +4527,7 @@ resume_qmin(void *arg) { CHECK(fcount_incr(fctx, false)); - dns_name_copy(dcname, fctx->qmindcname); + dns_name_copy(dcname, fctx->qmin.dcname); fctx->ns_ttl = fctx->nameservers.ttl; fctx->ns_ttl_ok = true; @@ -4618,14 +4634,11 @@ fctx__destroy(fetchctx_t *fctx, const char *func, const char *file, dns_ede_invalidate(&fctx->edectx); - if (fctx->resp_db != NULL) { - dns_db_detach(&fctx->resp_db); - } if (fctx->resp_node != NULL) { dns_db_detachnode(&fctx->resp_node); } - dns_rdataset_cleanup(&fctx->resp_rdataset); - dns_rdataset_cleanup(&fctx->resp_sigrdataset); + dns_rdataset_cleanup(&fctx->resp.rdataset); + dns_rdataset_cleanup(&fctx->resp.sigrdataset); isc_mem_free(fctx->mctx, fctx->info); @@ -4806,8 +4819,8 @@ fctx__create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, .nameservers = DNS_RDATASET_INIT, .nsrrset = DNS_RDATASET_INIT, .resp_result = DNS_R_SERVFAIL, - .resp_rdataset = DNS_RDATASET_INIT, - .resp_sigrdataset = DNS_RDATASET_INIT }; + .qmin.rdataset = DNS_RDATASET_INIT, + .qmin.sigrdataset = DNS_RDATASET_INIT }; isc_mem_attach(mctx, &fctx->mctx); dns_resolver_attach(res, &fctx->res); @@ -4819,13 +4832,12 @@ fctx__create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, fctx->name = dns_fixedname_initname(&fctx->fname); fctx->nsname = dns_fixedname_initname(&fctx->nsfname); fctx->domain = dns_fixedname_initname(&fctx->dfname); - fctx->qminname = dns_fixedname_initname(&fctx->qminfname); - fctx->qmindcname = dns_fixedname_initname(&fctx->qmindcfname); + fctx->qmin.name = dns_fixedname_initname(&fctx->qmin.fname); + fctx->qmin.dcname = dns_fixedname_initname(&fctx->qmin.dcfname); fctx->fwdname = dns_fixedname_initname(&fctx->fwdfname); - dns_fixedname_init(&fctx->resp_foundname); dns_name_copy(name, fctx->name); - dns_name_copy(name, fctx->qminname); + dns_name_copy(name, fctx->qmin.name); fctx->start = isc_time_now(); fctx->now = (isc_stdtime_t)fctx->start.seconds; @@ -4921,7 +4933,7 @@ fctx__create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, * domain. */ dns_name_copy(fctx->fwdname, fctx->domain); - dns_name_copy(fctx->fwdname, fctx->qmindcname); + dns_name_copy(fctx->fwdname, fctx->qmin.dcname); /* * Disable query minimization */ @@ -4947,14 +4959,14 @@ fctx__create(dns_resolver_t *res, isc_loop_t *loop, const dns_name_t *name, } dns_name_copy(fctx->fwdname, fctx->domain); - dns_name_copy(dcname, fctx->qmindcname); + dns_name_copy(dcname, fctx->qmin.dcname); fctx->ns_ttl = fctx->nameservers.ttl; fctx->ns_ttl_ok = true; } } else { dns_rdataset_clone(nameservers, &fctx->nameservers); dns_name_copy(domain, fctx->domain); - dns_name_copy(domain, fctx->qmindcname); + dns_name_copy(domain, fctx->qmin.dcname); fctx->ns_ttl = fctx->nameservers.ttl; fctx->ns_ttl_ok = true; } @@ -5336,7 +5348,7 @@ has_000_label(dns_rdataset_t *nsecset) { static void fctx_setresult(fetchctx_t *fctx) { isc_result_t result = ISC_R_SUCCESS; - dns_rdataset_t *rdataset = &fctx->resp_rdataset; + dns_rdataset_t *rdataset = &fctx->resp.rdataset; if (NEGATIVE(rdataset)) { result = NXDOMAIN(rdataset) ? DNS_R_NCACHENXDOMAIN @@ -5650,8 +5662,8 @@ validated(void *arg) { * iterates the node. */ if (negative || chaining || !dns_rdatatype_ismulti(fctx->type)) { - ardataset = &fctx->resp_rdataset; - asigrdataset = &fctx->resp_sigrdataset; + ardataset = &fctx->resp.rdataset; + asigrdataset = &fctx->resp.sigrdataset; } /* @@ -5738,8 +5750,7 @@ answer_response: FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); fctx_setresult(fctx); - dns_name_copy(val->name, dns_fixedname_name(&fctx->resp_foundname)); - dns_db_attach(fctx->cache, &fctx->resp_db); + dns_name_copy(val->name, fctx->resp.foundname); dns_db_transfernode(fctx->cache, &node, &fctx->resp_node); done = true; @@ -6033,8 +6044,8 @@ rctx_cache_secure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, if (!dns_rdatatype_ismulti(fctx->type) || CHAINING(rdataset)) { - ardataset = &fctx->resp_rdataset; - asigset = &fctx->resp_sigrdataset; + ardataset = &fctx->resp.rdataset; + asigset = &fctx->resp.sigrdataset; } } @@ -6066,9 +6077,9 @@ rctx_cache_insecure(respctx_t *rctx, dns_message_t *message, dns_name_t *name, */ if (!dns_rdatatype_ismulti(fctx->type) || CHAINING(rdataset)) { if (ANSWER(rdataset)) { - added = &fctx->resp_rdataset; + added = &fctx->resp.rdataset; } else if (ANSWERSIG(rdataset)) { - added = &fctx->resp_sigrdataset; + added = &fctx->resp.sigrdataset; } } @@ -6180,11 +6191,10 @@ rctx_cachename(respctx_t *rctx, dns_message_t *message, dns_name_t *name) { if (!need_validation && name->attributes.answer && !HAVE_ANSWER(fctx)) { fctx->resp_result = ISC_R_SUCCESS; - if (dns_rdataset_isassociated(&fctx->resp_rdataset)) { + if (dns_rdataset_isassociated(&fctx->resp.rdataset)) { fctx_setresult(fctx); } - dns_name_copy(name, dns_fixedname_name(&fctx->resp_foundname)); - dns_db_attach(fctx->cache, &fctx->resp_db); + dns_name_copy(name, fctx->resp.foundname); dns_db_transfernode(fctx->cache, &node, &fctx->resp_node); FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); } @@ -6358,7 +6368,7 @@ rctx_ncache(respctx_t *rctx) { */ LOCK(&fctx->lock); if (!HAVE_ANSWER(fctx)) { - added = &fctx->resp_rdataset; + added = &fctx->resp.rdataset; } result = negcache(message, fctx, name, rctx->now, false, false, added, @@ -6369,8 +6379,7 @@ rctx_ncache(respctx_t *rctx) { FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); fctx_setresult(fctx); - dns_name_copy(name, dns_fixedname_name(&fctx->resp_foundname)); - dns_db_attach(fctx->cache, &fctx->resp_db); + dns_name_copy(name, fctx->resp.foundname); dns_db_transfernode(fctx->cache, &node, &fctx->resp_node); unlock: @@ -9031,7 +9040,7 @@ rctx_referral(respctx_t *rctx) { dns_name_copy(rctx->ns_name, fctx->domain); if ((fctx->options & DNS_FETCHOPT_QMINIMIZE) != 0) { - dns_name_copy(rctx->ns_name, fctx->qmindcname); + dns_name_copy(rctx->ns_name, fctx->qmin.dcname); fctx_minimize_qname(fctx); } @@ -9171,7 +9180,7 @@ rctx_nextserver(respctx_t *rctx, dns_message_t *message, fcount_decr(fctx); dns_name_copy(fname, fctx->domain); - dns_name_copy(dcname, fctx->qmindcname); + dns_name_copy(dcname, fctx->qmin.dcname); result = fcount_incr(fctx, true); if (result != ISC_R_SUCCESS) { @@ -9928,7 +9937,7 @@ fctx_minimize_qname(fetchctx_t *fctx) { dns_name_init(&name); - dlabels = dns_name_countlabels(fctx->qmindcname); + dlabels = dns_name_countlabels(fctx->qmin.dcname); nlabels = dns_name_countlabels(fctx->name); if (dlabels > fctx->qmin_labels) { @@ -10001,18 +10010,18 @@ fctx_minimize_qname(fetchctx_t *fctx) { } if (fctx->qmin_labels < nlabels) { - dns_name_copy(&name, fctx->qminname); + dns_name_copy(&name, fctx->qmin.name); fctx->qmintype = dns_rdatatype_ns; fctx->minimized = true; } else { /* Minimization is done, we'll ask for whole qname */ - dns_name_copy(fctx->name, fctx->qminname); + dns_name_copy(fctx->name, fctx->qmin.name); fctx->qmintype = fctx->type; fctx->minimized = false; } char domainbuf[DNS_NAME_FORMATSIZE]; - dns_name_format(fctx->qminname, domainbuf, sizeof(domainbuf)); + dns_name_format(fctx->qmin.name, domainbuf, sizeof(domainbuf)); isc_log_write(DNS_LOGCATEGORY_RESOLVER, DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(5), "QNAME minimization - %s minimized, qmintype %d " From e62cafd3c76a9bb6c37f796dce507dd9cb47d47f Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Sat, 24 Jan 2026 17:57:34 +0100 Subject: [PATCH 7/7] rename fetch response `db` field to `cache` As the `dns_fetchresponse_t` `db` field can only be attached to the resolver cache database, rename it into `cache` to avoid ambiguities. --- bin/named/server.c | 4 ++-- lib/dns/adb.c | 4 ++-- lib/dns/client.c | 2 +- lib/dns/include/dns/resolver.h | 2 +- lib/dns/nta.c | 4 ++-- lib/dns/resolver.c | 18 +++++++++--------- lib/dns/validator.c | 8 ++++---- lib/dns/zonefetch.c | 4 ++-- lib/ns/query.c | 12 ++++++------ 9 files changed, 29 insertions(+), 29 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 9b3f170025..006b90c33c 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -6461,8 +6461,8 @@ tat_done(void *arg) { if (resp->node != NULL) { dns_db_detachnode(&resp->node); } - if (resp->db != NULL) { - dns_db_detach(&resp->db); + if (resp->cache != NULL) { + dns_db_detach(&resp->cache); } dns_resolver_freefresp(&resp); diff --git a/lib/dns/adb.c b/lib/dns/adb.c index f5cfa7e58e..06256be588 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -2586,8 +2586,8 @@ fetch_callback(void *arg) { if (resp->node != NULL) { dns_db_detachnode(&resp->node); } - if (resp->db != NULL) { - dns_db_detach(&resp->db); + if (resp->cache != NULL) { + dns_db_detach(&resp->cache); } if (atomic_load(&adb->shuttingdown)) { diff --git a/lib/dns/client.c b/lib/dns/client.c index 0e2d12f1aa..2097fa3ec5 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -531,7 +531,7 @@ client_resfind(resctx_t *rctx, dns_fetchresponse_t *resp) { INSIST(resp != NULL); INSIST(resp->fetch == rctx->fetch); dns_resolver_destroyfetch(&rctx->fetch); - db = resp->db; + db = resp->cache; node = resp->node; result = resp->result; vresult = resp->vresult; diff --git a/lib/dns/include/dns/resolver.h b/lib/dns/include/dns/resolver.h index 406e7d47d7..5c56d5d86d 100644 --- a/lib/dns/include/dns/resolver.h +++ b/lib/dns/include/dns/resolver.h @@ -82,7 +82,7 @@ struct dns_fetchresponse { isc_result_t vresult; dns_edectx_t *edectx; dns_rdatatype_t qtype; - dns_db_t *db; + dns_db_t *cache; dns_dbnode_t *node; dns_rdataset_t *rdataset; dns_rdataset_t *sigrdataset; diff --git a/lib/dns/nta.c b/lib/dns/nta.c index 17345e2613..aba39e95c6 100644 --- a/lib/dns/nta.c +++ b/lib/dns/nta.c @@ -163,8 +163,8 @@ fetch_done(void *arg) { if (resp->node != NULL) { dns_db_detachnode(&resp->node); } - if (resp->db != NULL) { - dns_db_detach(&resp->db); + if (resp->cache != NULL) { + dns_db_detach(&resp->cache); } dns_resolver_freefresp(&resp); diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 56313bc2b4..f475af5ffd 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -1645,7 +1645,7 @@ copy_to_resp(fetchctx_t *fctx, dns_fetchresponse_t *resp) { dns_name_copy(fctx->resp.foundname, resp->foundname); - dns_db_attach(fctx->cache, &resp->db); + dns_db_attach(fctx->cache, &resp->cache); dns_db_attachnode(fctx->resp_node, &resp->node); if (dns_rdataset_isassociated(&fctx->resp.rdataset)) { @@ -1666,8 +1666,8 @@ pull_from_resp(dns_fetchresponse_t *resp, fetchctx_t *fctx) { if (dns_rdataset_isassociated(resp->sigrdataset)) { dns_rdataset_clone(resp->sigrdataset, &fctx->resp.sigrdataset); } - if (resp->db != NULL) { - INSIST(resp->db == fctx->cache); + if (resp->cache != NULL) { + INSIST(resp->cache == fctx->cache); } if (resp->node != NULL) { dns_db_attachnode(resp->node, &fctx->resp_node); @@ -4362,8 +4362,8 @@ clear_resp(dns_fetchresponse_t **respp) { if (resp->node != NULL) { dns_db_detachnode(&resp->node); } - if (resp->db != NULL) { - dns_db_detach(&resp->db); + if (resp->cache != NULL) { + dns_db_detach(&resp->cache); } dns_rdataset_cleanup(resp->rdataset); dns_rdataset_cleanup(resp->sigrdataset); @@ -6827,8 +6827,8 @@ resume_dslookup(void *arg) { if (resp->node != NULL) { dns_db_detachnode(&resp->node); } - if (resp->db != NULL) { - dns_db_detach(&resp->db); + if (resp->cache != NULL) { + dns_db_detach(&resp->cache); } /* Preserve data from resp before freeing it. */ @@ -9804,8 +9804,8 @@ prime_done(void *arg) { if (resp->node != NULL) { dns_db_detachnode(&resp->node); } - if (resp->db != NULL) { - dns_db_detach(&resp->db); + if (resp->cache != NULL) { + dns_db_detach(&resp->cache); } dns_rdataset_cleanup(resp->rdataset); INSIST(resp->sigrdataset == NULL); diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 5316e9a666..2e731a7576 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -427,8 +427,8 @@ fetch_callback_dnskey(void *arg) { if (resp->node != NULL) { dns_db_detachnode(&resp->node); } - if (resp->db != NULL) { - dns_db_detach(&resp->db); + if (resp->cache != NULL) { + dns_db_detach(&resp->cache); } dns_rdataset_cleanup(&val->fsigrdataset); @@ -520,8 +520,8 @@ fetch_callback_ds(void *arg) { if (resp->node != NULL) { dns_db_detachnode(&resp->node); } - if (resp->db != NULL) { - dns_db_detach(&resp->db); + if (resp->cache != NULL) { + dns_db_detach(&resp->cache); } dns_rdataset_cleanup(&val->fsigrdataset); diff --git a/lib/dns/zonefetch.c b/lib/dns/zonefetch.c index d8a5df9b18..d73c6df6ef 100644 --- a/lib/dns/zonefetch.c +++ b/lib/dns/zonefetch.c @@ -148,8 +148,8 @@ dns_zonefetch_done(void *arg) { if (resp->node != NULL) { dns_db_detachnode(&resp->node); } - if (resp->db != NULL) { - dns_db_detach(&resp->db); + if (resp->cache != NULL) { + dns_db_detach(&resp->cache); } dns_resolver_destroyfetch(&fetch->fetch); diff --git a/lib/ns/query.c b/lib/ns/query.c index b23db80903..59165a2537 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -2591,8 +2591,8 @@ free_fresp(ns_client_t *client, dns_fetchresponse_t **frespp) { if (fresp->node != NULL) { dns_db_detachnode(&fresp->node); } - if (fresp->db != NULL) { - dns_db_detach(&fresp->db); + if (fresp->cache != NULL) { + dns_db_detach(&fresp->cache); } if (fresp->rdataset != NULL) { ns_client_putrdataset(client, &fresp->rdataset); @@ -6385,7 +6385,7 @@ query_resume(query_ctx_t *qctx) { if (qctx->fresp->node != NULL) { dns_db_detachnode(&qctx->fresp->node); } - SAVE(qctx->rpz_st->r.db, qctx->fresp->db); + SAVE(qctx->rpz_st->r.db, qctx->fresp->cache); qctx->rpz_st->r.r_type = qctx->fresp->qtype; SAVE(qctx->rpz_st->r.r_rdataset, qctx->fresp->rdataset); ns_client_putrdataset(qctx->client, &qctx->fresp->sigrdataset); @@ -6423,15 +6423,15 @@ query_resume(query_ctx_t *qctx) { if (qctx->fresp->node != NULL) { dns_db_detachnode(&qctx->fresp->node); } - if (qctx->fresp->db != NULL) { - dns_db_detach(&qctx->fresp->db); + if (qctx->fresp->cache != NULL) { + dns_db_detach(&qctx->fresp->cache); } } else { CCTRACE(ISC_LOG_DEBUG(3), "resume from normal recursion"); qctx->authoritative = false; qctx->qtype = qctx->fresp->qtype; - SAVE(qctx->db, qctx->fresp->db); + SAVE(qctx->db, qctx->fresp->cache); SAVE(qctx->node, qctx->fresp->node); SAVE(qctx->rdataset, qctx->fresp->rdataset); SAVE(qctx->sigrdataset, qctx->fresp->sigrdataset);