129 lines
4.3 KiB
Diff
129 lines
4.3 KiB
Diff
From 321805f39940169072dd0d9246fdd923274a6427 Mon Sep 17 00:00:00 2001
|
|
From: Colin Vidal <colin@isc.org>
|
|
Date: Tue, 7 Apr 2026 22:18:10 +0200
|
|
Subject: [PATCH] Refactor incrementing query counters
|
|
|
|
Move the logic incrementing the query counter and the global query
|
|
counter into a dedicated helper function.
|
|
|
|
(cherry picked from commit 05d6da2de54c093689e675e81ae898ee41220666)
|
|
(cherry picked from commit 9ebfca2af824a60ea072292ffb1ab01ff87c7fa7)
|
|
|
|
rctx_resend() increment query counters
|
|
|
|
Calls to `rctx_resend()` are done internally within the resolver, in
|
|
flow which are not supposed to happens more than once. For instance,
|
|
if some query fails, and a specific flag "F" wasn't set, then set the
|
|
flag and try again. This wouldn't occur more than once because if the
|
|
query fails the next attempt, the flag "F" would be set already, so the
|
|
resolver would move to the next server (or give up).
|
|
|
|
However, a subtle bug missing checking a flag, for instance, could lead
|
|
to an unbounded loop re-trying to query the same server. This is now
|
|
impossible as `rctx_resend()` also increment the query counters (so if
|
|
such case occurs, it would stop once the maximum limit is reached).
|
|
|
|
The dns_resstatscounter_retry are also only incremented if the
|
|
`fctx_query()` succeeds, similar to as is done in `fctx_try()`.
|
|
|
|
(cherry picked from commit f3e74304889a2e8b69c8e88fc9a383589decda32)
|
|
(cherry picked from commit d3ba533080e31de98986f3beff70d7c330ba9f89)
|
|
---
|
|
lib/dns/resolver.c | 49 ++++++++++++++++++++++++++++++++++------------
|
|
1 file changed, 36 insertions(+), 13 deletions(-)
|
|
|
|
diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
|
|
index d56a439..7d70992 100644
|
|
--- a/lib/dns/resolver.c
|
|
+++ b/lib/dns/resolver.c
|
|
@@ -68,6 +68,13 @@
|
|
#include <dns/view.h>
|
|
#include <dns/zone.h>
|
|
|
|
+#define CHECK(op) \
|
|
+ do { \
|
|
+ result = (op); \
|
|
+ if (result != ISC_R_SUCCESS) \
|
|
+ goto cleanup; \
|
|
+ } while (0)
|
|
+
|
|
/* Detailed logging of fctx attach/detach */
|
|
#ifndef FCTX_TRACE
|
|
#undef FCTX_TRACE
|
|
@@ -4121,6 +4128,22 @@ fctx_nextaddress(fetchctx_t *fctx) {
|
|
return (addrinfo);
|
|
}
|
|
|
|
+/* RH: modified compared to bind 9.18.33 function, no fctx->gqc increment */
|
|
+static isc_result_t
|
|
+incr_query_counters(fetchctx_t *fctx) {
|
|
+ isc_result_t result;
|
|
+
|
|
+ result = isc_counter_increment(fctx->qc);
|
|
+ if (result != ISC_R_SUCCESS) {
|
|
+ isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER,
|
|
+ DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(3),
|
|
+ "exceeded max queries resolving '%s'"
|
|
+ "(max-recursion-queries, querycount=%u)",
|
|
+ fctx->info, isc_counter_used(fctx->qc));
|
|
+ }
|
|
+ return result;
|
|
+}
|
|
+
|
|
static void
|
|
fctx_try(fetchctx_t *fctx, bool retrying, bool badcache) {
|
|
isc_result_t result;
|
|
@@ -4258,17 +4281,10 @@ fctx_try(fetchctx_t *fctx, bool retrying, bool badcache) {
|
|
return;
|
|
}
|
|
|
|
- result = isc_counter_increment(fctx->qc);
|
|
- if (result != ISC_R_SUCCESS) {
|
|
- isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER,
|
|
- DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(3),
|
|
- "exceeded max queries resolving '%s'",
|
|
- fctx->info);
|
|
- fctx_done_detach(&fctx, DNS_R_SERVFAIL);
|
|
- return;
|
|
- }
|
|
+ CHECK(incr_query_counters(fctx));
|
|
|
|
result = fctx_query(fctx, addrinfo, fctx->options);
|
|
+cleanup:
|
|
if (result != ISC_R_SUCCESS) {
|
|
fctx_done_detach(&fctx, result);
|
|
} else if (retrying) {
|
|
@@ -10015,9 +10031,9 @@ rctx_nextserver(respctx_t *rctx, dns_message_t *message,
|
|
* rctx_resend():
|
|
*
|
|
* Resend the query, probably with the options changed. Calls
|
|
- * fctx_query(), passing rctx->retryopts (which is based on
|
|
- * query->options, but may have been updated since the last time
|
|
- * fctx_query() was called).
|
|
+ * fctx_query(), unless query counter limits are hit, passing
|
|
+ * rctx->retryopts (which is based on query->options, but may have
|
|
+ * been updated since the last time fctx_query() was called).
|
|
*/
|
|
static void
|
|
rctx_resend(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo) {
|
|
@@ -10025,8 +10041,15 @@ rctx_resend(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo) {
|
|
isc_result_t result;
|
|
|
|
FCTXTRACE("resend");
|
|
- inc_stats(fctx->res, dns_resstatscounter_retry);
|
|
+
|
|
+ CHECK(incr_query_counters(fctx));
|
|
+
|
|
result = fctx_query(fctx, addrinfo, rctx->retryopts);
|
|
+ if (result == ISC_R_SUCCESS) {
|
|
+ inc_stats(fctx->res, dns_resstatscounter_retry);
|
|
+ }
|
|
+
|
|
+cleanup:
|
|
if (result != ISC_R_SUCCESS) {
|
|
fctx_done_detach(&rctx->fctx, result);
|
|
}
|
|
--
|
|
2.55.0
|
|
|