From 9eaddf2e4f85cee3c26a216f098c7a158200ef0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Tue, 14 Jun 2022 13:13:32 +0200 Subject: [PATCH] Separate prefetch handling from RPZ fetch handling Both prefetch code and RPZ code ignore recursion results (caching the response notwithstanding). RPZ code has been (ab)using that fact since commit 08e36aa5a5c7697a839f83831fccf8fb3f792848 by employing prefetch_done() as the fetch completion callback. This is only seemingly a simplification as it makes the code harder to follow ("why is prefetch code used for handling RPZ-triggered recursion?"). Turn prefetch_done() into a new function whose name clearly conveys its purpose. Add a parameter to its prototype in order to allow callers to specify which slot in the 'recursions' array it should use. Reintroduce prefetch_done() as a wrapper for that function. Add rpzfetch_done(), an RPZ-exclusive wrapper for that function (using a distinct recursion type). Since each slot in the 'recursions' array needs to be initialized before getting cleaned up when recursion completes, rework fetch_and_forget() so that it takes recursion type rather than extra fetch options as the last parameter and make it use the requested slot in the 'recursions' array rather than a fixed slot (RECTYPE_PREFETCH) for all callers. This makes fetch_and_forget() a logical complement of cleanup_after_fetch(). Collectively, these changes make prefetch and RPZ code logically separate (except for reusing client->recursionquota, which will be refactored later). --- lib/ns/query.c | 87 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 59 insertions(+), 28 deletions(-) diff --git a/lib/ns/query.c b/lib/ns/query.c index 6a7ebee0f0..0696eb35a2 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -2511,8 +2511,11 @@ recursionquota_detach(ns_client_t *client) { } static void -prefetch_done(isc_task_t *task, isc_event_t *event) { +cleanup_after_fetch(isc_task_t *task, isc_event_t *event, + ns_query_rectype_t recursion_type) { dns_fetchevent_t *devent = (dns_fetchevent_t *)event; + isc_nmhandle_t **handlep; + dns_fetch_t **fetchp; ns_client_t *client; UNUSED(task); @@ -2524,37 +2527,50 @@ prefetch_done(isc_task_t *task, isc_event_t *event) { CTRACE(ISC_LOG_DEBUG(3), "prefetch_done"); + handlep = &client->query.recursions[recursion_type].handle; + fetchp = &client->query.recursions[recursion_type].fetch; + LOCK(&client->query.fetchlock); - if (FETCH_RECTYPE_PREFETCH(client) != NULL) { - INSIST(devent->fetch == FETCH_RECTYPE_PREFETCH(client)); - FETCH_RECTYPE_PREFETCH(client) = NULL; + if (*fetchp != NULL) { + INSIST(devent->fetch == *fetchp); + *fetchp = NULL; } UNLOCK(&client->query.fetchlock); - /* - * We're done prefetching, detach from quota. - */ recursionquota_detach(client); free_devent(client, &event, &devent); - isc_nmhandle_detach(&HANDLE_RECTYPE_PREFETCH(client)); + isc_nmhandle_detach(handlep); +} + +static void +prefetch_done(isc_task_t *task, isc_event_t *event) { + cleanup_after_fetch(task, event, RECTYPE_PREFETCH); +} + +static void +rpzfetch_done(isc_task_t *task, isc_event_t *event) { + cleanup_after_fetch(task, event, RECTYPE_RPZ); } /* - * Try initiating a fetch for the given 'qname' and 'qtype' (with any requested - * 'extra_fetch_options') that will be associated with 'client'. If the - * recursive clients quota (or even soft quota) is reached or some other error - * occurs, just return without starting the fetch. If a fetch is successfully - * created, its results will be cached upon successful completion, but no - * further actions will be taken afterwards. + * Try initiating a fetch for the given 'qname' and 'qtype' (using the slot in + * the 'recursions' array indicated by 'recursion_type') that will be + * associated with 'client'. If the recursive clients quota (or even soft + * quota) is reached or some other error occurs, just return without starting + * the fetch. If a fetch is successfully created, its results will be cached + * upon successful completion, but no further actions will be taken afterwards. */ static void fetch_and_forget(ns_client_t *client, dns_name_t *qname, dns_rdatatype_t qtype, - unsigned int extra_fetch_options) { - isc_result_t result; - isc_sockaddr_t *peeraddr; + ns_query_rectype_t recursion_type) { dns_rdataset_t *tmprdataset; + isc_sockaddr_t *peeraddr; unsigned int options; + isc_taskaction_t action; + isc_nmhandle_t **handlep; + dns_fetch_t **fetchp; + isc_result_t result; result = recursionquota_attach_hard(client); if (result != ISC_R_SUCCESS) { @@ -2569,16 +2585,31 @@ fetch_and_forget(ns_client_t *client, dns_name_t *qname, dns_rdatatype_t qtype, peeraddr = NULL; } - isc_nmhandle_attach(client->handle, &HANDLE_RECTYPE_PREFETCH(client)); - options = client->query.fetchoptions | extra_fetch_options; - result = dns_resolver_createfetch( - client->view->resolver, qname, qtype, NULL, NULL, NULL, - peeraddr, client->message->id, options, 0, NULL, - client->manager->task, prefetch_done, client, tmprdataset, NULL, - &FETCH_RECTYPE_PREFETCH(client)); + switch (recursion_type) { + case RECTYPE_PREFETCH: + options = client->query.fetchoptions | DNS_FETCHOPT_PREFETCH; + action = prefetch_done; + break; + case RECTYPE_RPZ: + options = client->query.fetchoptions; + action = rpzfetch_done; + break; + default: + UNREACHABLE(); + } + + handlep = &client->query.recursions[recursion_type].handle; + fetchp = &client->query.recursions[recursion_type].fetch; + + isc_nmhandle_attach(client->handle, handlep); + result = dns_resolver_createfetch(client->view->resolver, qname, qtype, + NULL, NULL, NULL, peeraddr, + client->message->id, options, 0, NULL, + client->manager->task, action, client, + tmprdataset, NULL, fetchp); if (result != ISC_R_SUCCESS) { ns_client_putrdataset(client, &tmprdataset); - isc_nmhandle_detach(&HANDLE_RECTYPE_PREFETCH(client)); + isc_nmhandle_detach(handlep); } } @@ -2595,7 +2626,7 @@ query_prefetch(ns_client_t *client, dns_name_t *qname, return; } - fetch_and_forget(client, qname, rdataset->type, DNS_FETCHOPT_PREFETCH); + fetch_and_forget(client, qname, rdataset->type, RECTYPE_PREFETCH); dns_rdataset_clearprefetch(rdataset); ns_stats_increment(client->manager->sctx->nsstats, @@ -2762,11 +2793,11 @@ static void query_rpzfetch(ns_client_t *client, dns_name_t *qname, dns_rdatatype_t type) { CTRACE(ISC_LOG_DEBUG(3), "query_rpzfetch"); - if (FETCH_RECTYPE_PREFETCH(client) != NULL) { + if (FETCH_RECTYPE_RPZ(client) != NULL) { return; } - fetch_and_forget(client, qname, type, 0); + fetch_and_forget(client, qname, type, RECTYPE_RPZ); } /*