From: Evan Hunt Date: Wed, 26 May 2021 22:35:18 +0000 (-0700) Subject: clean up query correctly if already answered by serve-stale X-Git-Tag: v9.17.14~18^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8bd8e995f185729eec13f8d3bdef3d65c64895a1;p=thirdparty%2Fbind9.git clean up query correctly if already answered by serve-stale when a serve-stale answer has been sent, the client continues waiting for a proper answer. if a final completion event for the client does arrive, it can just be cleaned up without sending a response, similar to a canceled fetch. --- diff --git a/lib/ns/include/ns/query.h b/lib/ns/include/ns/query.h index 74a95c9b944..cee2e1796c4 100644 --- a/lib/ns/include/ns/query.h +++ b/lib/ns/include/ns/query.h @@ -100,25 +100,26 @@ struct ns_query { bool root_key_sentinel_not_ta; }; -#define NS_QUERYATTR_RECURSIONOK 0x00001 -#define NS_QUERYATTR_CACHEOK 0x00002 -#define NS_QUERYATTR_PARTIALANSWER 0x00004 -#define NS_QUERYATTR_NAMEBUFUSED 0x00008 -#define NS_QUERYATTR_RECURSING 0x00010 -#define NS_QUERYATTR_QUERYOKVALID 0x00040 -#define NS_QUERYATTR_QUERYOK 0x00080 -#define NS_QUERYATTR_WANTRECURSION 0x00100 -#define NS_QUERYATTR_SECURE 0x00200 -#define NS_QUERYATTR_NOAUTHORITY 0x00400 -#define NS_QUERYATTR_NOADDITIONAL 0x00800 -#define NS_QUERYATTR_CACHEACLOKVALID 0x01000 -#define NS_QUERYATTR_CACHEACLOK 0x02000 -#define NS_QUERYATTR_DNS64 0x04000 -#define NS_QUERYATTR_DNS64EXCLUDE 0x08000 -#define NS_QUERYATTR_RRL_CHECKED 0x10000 -#define NS_QUERYATTR_REDIRECT 0x20000 -#define NS_QUERYATTR_ANSWERED 0x40000 -#define NS_QUERYATTR_STALEOK 0x80000 +#define NS_QUERYATTR_RECURSIONOK 0x000001 +#define NS_QUERYATTR_CACHEOK 0x000002 +#define NS_QUERYATTR_PARTIALANSWER 0x000004 +#define NS_QUERYATTR_NAMEBUFUSED 0x000008 +#define NS_QUERYATTR_RECURSING 0x000010 +#define NS_QUERYATTR_QUERYOKVALID 0x000040 +#define NS_QUERYATTR_QUERYOK 0x000080 +#define NS_QUERYATTR_WANTRECURSION 0x000100 +#define NS_QUERYATTR_SECURE 0x000200 +#define NS_QUERYATTR_NOAUTHORITY 0x000400 +#define NS_QUERYATTR_NOADDITIONAL 0x000800 +#define NS_QUERYATTR_CACHEACLOKVALID 0x001000 +#define NS_QUERYATTR_CACHEACLOK 0x002000 +#define NS_QUERYATTR_DNS64 0x004000 +#define NS_QUERYATTR_DNS64EXCLUDE 0x008000 +#define NS_QUERYATTR_RRL_CHECKED 0x010000 +#define NS_QUERYATTR_REDIRECT 0x020000 +#define NS_QUERYATTR_ANSWERED 0x040000 +#define NS_QUERYATTR_STALEOK 0x080000 +#define NS_QUERYATTR_STALEPENDING 0x100000 typedef struct query_ctx query_ctx_t; diff --git a/lib/ns/query.c b/lib/ns/query.c index 2ef0d67c91b..d3546f9d5cc 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -134,9 +134,13 @@ #define REDIRECT(c) (((c)->query.attributes & NS_QUERYATTR_REDIRECT) != 0) -/*% Was the query already answered due to stale-answer-client-timeout? */ +/*% Was the client already sent a response? */ #define QUERY_ANSWERED(q) (((q)->attributes & NS_QUERYATTR_ANSWERED) != 0) +/*% Have we already processed an answer via stale-answer-client-timeout? */ +#define QUERY_STALEPENDING(q) \ + (((q)->attributes & NS_QUERYATTR_STALEPENDING) != 0) + /*% Does the query allow stale data in the response? */ #define QUERY_STALEOK(q) (((q)->attributes & NS_QUERYATTR_STALEOK) != 0) @@ -5926,7 +5930,7 @@ query_lookup(query_ctx_t *qctx) { dns_resolver_destroyfetch( &qctx->client->query.fetch); } - return query_lookup(qctx); + return (query_lookup(qctx)); } else { /* * Immediately return the stale answer, start a @@ -5955,6 +5959,13 @@ query_lookup(query_ctx_t *qctx) { if (!stale_found) { return (result); } + + /* + * There still might be real answer later. Mark the + * query so we'll know we can skip answering. + */ + qctx->client->query.attributes |= + NS_QUERYATTR_STALEPENDING; } } @@ -5967,6 +5978,7 @@ query_lookup(query_ctx_t *qctx) { qctx->client->query.attributes |= NS_QUERYATTR_STALEOK; qctx->rdataset->attributes |= DNS_RDATASETATTR_STALE_ADDED; } + result = query_gotanswer(qctx, result); if (refresh_rrset) { @@ -6072,10 +6084,12 @@ static void fetch_callback(isc_task_t *task, isc_event_t *event) { dns_fetchevent_t *devent = (dns_fetchevent_t *)event; dns_fetch_t *fetch = NULL; - ns_client_t *client; - bool fetch_canceled, client_shuttingdown; - isc_result_t result; + ns_client_t *client = NULL; + bool fetch_canceled = false; + bool fetch_answered = false; + bool client_shuttingdown = false; isc_logcategory_t *logcategory = NS_LOGCATEGORY_QUERY_ERRORS; + isc_result_t result; int errorloglevel; query_ctx_t qctx; @@ -6083,7 +6097,9 @@ fetch_callback(isc_task_t *task, isc_event_t *event) { REQUIRE(event->ev_type == DNS_EVENT_FETCHDONE || event->ev_type == DNS_EVENT_TRYSTALE); + client = devent->ev_arg; + REQUIRE(NS_CLIENT_VALID(client)); REQUIRE(task == client->task); REQUIRE(RECURSING(client)); @@ -6095,7 +6111,6 @@ fetch_callback(isc_task_t *task, isc_event_t *event) { isc_event_free(ISC_EVENT_PTR(&event)); return; } - /* * We are resuming from recursion. Reset any attributes, options * that a lookup due to stale-answer-client-timeout may have set. @@ -6107,13 +6122,23 @@ fetch_callback(isc_task_t *task, isc_event_t *event) { client->nodetach = false; LOCK(&client->query.fetchlock); - if (client->query.fetch != NULL) { + INSIST(client->query.fetch == devent->fetch || + client->query.fetch == NULL); + if (QUERY_STALEPENDING(&client->query)) { + /* + * We've gotten an authoritative answer to a query that + * was left pending after a stale timeout. We don't need + * to do anything with it; free all the data and go home. + */ + client->query.fetch = NULL; + fetch_answered = true; + } else if (client->query.fetch != NULL) { /* * This is the fetch we've been waiting for. */ INSIST(devent->fetch == client->query.fetch); client->query.fetch = NULL; - fetch_canceled = false; + /* * Update client->now. */ @@ -6126,7 +6151,6 @@ fetch_callback(isc_task_t *task, isc_event_t *event) { fetch_canceled = true; } UNLOCK(&client->query.fetchlock); - INSIST(client->query.fetch == NULL); SAVE(fetch, devent->fetch); @@ -6169,7 +6193,7 @@ fetch_callback(isc_task_t *task, isc_event_t *event) { qctx_init(client, &devent, 0, &qctx); client_shuttingdown = ns_client_shuttingdown(client); - if (fetch_canceled || client_shuttingdown) { + if (fetch_canceled || fetch_answered || client_shuttingdown) { /* * We've timed out or are shutting down. We can now * free the event and other resources held by qctx, but