diff --git a/CHANGES b/CHANGES index 47a2198dfd..f0469c6d83 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +3333. [bug] Setting resolver-query-timeout too low can cause + named to not recover if it looses connectivity. + [RT #29623] + 3332. [bug] Re-use cached DS rrsets if possible. [RT 29446] 3331. [security] dns_rdataslab_fromrdataset could produce bad diff --git a/bin/named/config.c b/bin/named/config.c index eb1eef3504..79fc539d1f 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -89,7 +89,7 @@ options {\n\ #endif "\ recursive-clients 1000;\n\ - resolver-query-timeout 30;\n\ + resolver-query-timeout 10;\n\ rrset-order { order random; };\n\ serial-queries 20;\n\ serial-query-rate 20;\n\ diff --git a/bin/tests/resolver/t_resolver.c b/bin/tests/resolver/t_resolver.c index 3d611b60cb..27a18ac714 100644 --- a/bin/tests/resolver/t_resolver.c +++ b/bin/tests/resolver/t_resolver.c @@ -154,11 +154,11 @@ test_dns_resolver_settimeout(void) { t_info("The default timeout is %d second%s\n", default_timeout, (default_timeout == 1 ? "" : "s")); - dns_resolver_settimeout(resolver, default_timeout - 1); + dns_resolver_settimeout(resolver, default_timeout + 1); timeout = dns_resolver_gettimeout(resolver); t_info("The new timeout is %d second%s\n", timeout, (timeout == 1 ? "" : "s")); - test_result = (timeout == default_timeout - 1) ? T_PASS : T_FAIL; + test_result = (timeout == default_timeout + 1) ? T_PASS : T_FAIL; destroy_resolver(&resolver); teardown(); diff --git a/doc/arm/Bv9ARM-book.xml b/doc/arm/Bv9ARM-book.xml index 252f18bb02..48119cb5d2 100644 --- a/doc/arm/Bv9ARM-book.xml +++ b/doc/arm/Bv9ARM-book.xml @@ -7474,8 +7474,8 @@ options { The amount of time the resolver will spend attempting - to resolve a recursive query before failing. The - default is 10 and the maximum is + to resolve a recursive query before failing. The default + and minimum is 10 and the maximum is 30. Setting it to 0 will result in the default being used. diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 48dcedf5a0..f6ce93bdd4 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -106,8 +106,21 @@ #define QTRACE(m) #endif +#define US_PER_SEC 1000000U +/* + * The maximum time we will wait for a single query. + */ +#define MAX_SINGLE_QUERY_TIMEOUT 9U +#define MAX_SINGLE_QUERY_TIMEOUT_US (MAX_SINGLE_QUERY_TIMEOUT*US_PER_SEC) + +/* + * We need to allow a individual query time to complete / timeout. + */ +#define MINIMUM_QUERY_TIMEOUT (MAX_SINGLE_QUERY_TIMEOUT + 1U) + +/* The default time in seconds for the whole query to live. */ #ifndef DEFAULT_QUERY_TIMEOUT -#define DEFAULT_QUERY_TIMEOUT 10 /* The default time in seconds for the whole query to live. */ +#define DEFAULT_QUERY_TIMEOUT MINIMUM_QUERY_TIMEOUT #endif #ifndef MAXIMUM_QUERY_TIMEOUT @@ -827,8 +840,8 @@ fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp, */ INSIST(no_response); rtt = query->addrinfo->srtt + 200000; - if (rtt > 10000000) - rtt = 10000000; + if (rtt > MAX_SINGLE_QUERY_TIMEOUT_US) + rtt = MAX_SINGLE_QUERY_TIMEOUT_US; /* * Replace the current RTT with our value. */ @@ -1343,12 +1356,18 @@ fctx_setretryinterval(fetchctx_t *fctx, unsigned int rtt) { us = (800000 << (fctx->restarts - 2)); /* - * Double the round-trip time. + * Add a fudge factor to the expected rtt based on the current + * estimate. */ - rtt *= 2; + if (rtt < 50000) + rtt += 50000; + else if (rtt < 100000) + rtt += 100000; + else + rtt += 200000; /* - * Always wait for at least the doubled round-trip time. + * Always wait for at least the expected rtt. */ if (us < rtt) us = rtt; @@ -1356,11 +1375,11 @@ fctx_setretryinterval(fetchctx_t *fctx, unsigned int rtt) { /* * But don't ever wait for more than 10 seconds. */ - if (us > 10000000) - us = 10000000; + if (us > MAX_SINGLE_QUERY_TIMEOUT_US) + us = MAX_SINGLE_QUERY_TIMEOUT_US; - seconds = us / 1000000; - us -= seconds * 1000000; + seconds = us / US_PER_SEC; + us -= seconds * US_PER_SEC; isc_interval_set(&fctx->interval, seconds, us * 1000); } @@ -1382,6 +1401,11 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, task = res->buckets[fctx->bucketnum].task; srtt = addrinfo->srtt; + + /* + * A forwarder needs to make multiple queries. Give it at least + * a second to do these in. + */ if (ISFORWARDER(addrinfo) && srtt < 1000000) srtt = 1000000; @@ -8191,8 +8215,8 @@ dns_resolver_logfetch(dns_fetch_t *fetch, isc_log_t *lctx, "timeout:%u,lame:%u,neterr:%u,badresp:%u," "adberr:%u,findfail:%u,valfail:%u]", __FILE__, fctx->exitline, fctx->info, - fctx->duration / 1000000, - fctx->duration % 1000000, + fctx->duration / US_PER_SEC, + fctx->duration % US_PER_SEC, isc_result_totext(fctx->result), isc_result_totext(fctx->vresult), domainbuf, fctx->referrals, fctx->restarts, @@ -8797,6 +8821,8 @@ dns_resolver_settimeout(dns_resolver_t *resolver, unsigned int seconds) { seconds = DEFAULT_QUERY_TIMEOUT; if (seconds > MAXIMUM_QUERY_TIMEOUT) seconds = MAXIMUM_QUERY_TIMEOUT; + if (seconds < MINIMUM_QUERY_TIMEOUT) + seconds = MINIMUM_QUERY_TIMEOUT; resolver->query_timeout = seconds; }