diff --git a/CHANGES b/CHANGES index 25924a89fe..c3b3cb3e21 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5417. [cleanup] The code determining the advertised UDP buffer size in + outgoing EDNS queries has been refactored to improve its + clarity. [GL #1868] + 5416. [bug] Fix a lock order inversion in unix/socket.c. [GL #1859] 5415. [test] Address race in dnssec system test that led to diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst index 0ea798f90b..42b234fa75 100644 --- a/doc/arm/reference.rst +++ b/doc/arm/reference.rst @@ -3401,16 +3401,26 @@ Tuning buffer size of 512, as this has the greatest chance of success on the first try. - If the initial query is successful with EDNS advertising a buffer size of - 512, then ``named`` will advertise progressively larger buffer sizes on - successive queries, until responses begin timing out or ``edns-udp-size`` is - reached. + If the initial query is successful with EDNS advertising a buffer + size of 512, then ``named`` will switch to advertising a buffer size + of 4096 bytes (unless ``edns-udp-size`` is lower, in which case the + latter will be used). - The default buffer sizes used by ``named`` are 512, 1232, 1432, and - 4096, but never exceeding ``edns-udp-size``. (The values 1232 and - 1432 are chosen to allow for an IPv4/IPv6 encapsulated UDP message to - be sent without fragmentation at the minimum MTU sizes for Ethernet - and IPv6 networks.) + Query timeouts observed for any given server will affect the buffer + size advertised in queries sent to that server. Depending on + observed packet dropping patterns, the advertised buffer size will be + lowered to 1432 bytes, 1232 bytes, 512 bytes, or the size of the + largest UDP response ever received from a given server, and then + clamped to the ``<512, edns-udp-size>`` range. Per-server EDNS + statistics are only retained in memory for the lifetime of a given + server's ADB entry. + + (The values 1232 and 1432 are chosen to allow for an IPv4/IPv6 + encapsulated UDP message to be sent without fragmentation at the + minimum MTU sizes for Ethernet and IPv6 networks.) + + Any server-specific ``edns-udp-size`` setting has precedence over all + the above rules. ``max-udp-size`` Sets the maximum EDNS UDP message size ``named`` will send in bytes. diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index a518a1f613..e97bc41b5d 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -369,12 +369,6 @@ struct fetchctx { */ unsigned int nqueries; /* Bucket lock. */ - /*% - * The reason to print when logging a successful - * response to a query. - */ - const char *reason; - /*% * Random numbers to use for mixing up server addresses. */ @@ -1748,25 +1742,6 @@ fctx_sendevents(fetchctx_t *fctx, isc_result_t result, int line) { } } -static inline void -log_edns(fetchctx_t *fctx) { - char domainbuf[DNS_NAME_FORMATSIZE]; - - if (fctx->reason == NULL) { - return; - } - - /* - * We do not know if fctx->domain is the actual domain the record - * lives in or a parent domain so we have a '?' after it. - */ - dns_name_format(&fctx->domain, domainbuf, sizeof(domainbuf)); - isc_log_write(dns_lctx, DNS_LOGCATEGORY_EDNS_DISABLED, - DNS_LOGMODULE_RESOLVER, ISC_LOG_INFO, - "success resolving '%s' (in '%s'?) after %s", fctx->info, - domainbuf, fctx->reason); -} - static void fctx_done(fetchctx_t *fctx, isc_result_t result, int line) { dns_resolver_t *res; @@ -1780,10 +1755,6 @@ fctx_done(fetchctx_t *fctx, isc_result_t result, int line) { res = fctx->res; if (result == ISC_R_SUCCESS) { - /*% - * Log any deferred EDNS timeout messages. - */ - log_edns(fctx); no_response = true; if (fctx->qmin_warning != ISC_R_SUCCESS) { isc_log_write(dns_lctx, DNS_LOGCATEGORY_LAME_SERVERS, @@ -1799,7 +1770,6 @@ fctx_done(fetchctx_t *fctx, isc_result_t result, int line) { } fctx->qmin_warning = ISC_R_SUCCESS; - fctx->reason = NULL; fctx_stopqueries(fctx, no_response, age_untried); @@ -2618,14 +2588,21 @@ resquery_send(resquery_t *query) { isc_sockaddr_t *sockaddr = &query->addrinfo->sockaddr; struct tried *tried; + /* + * If this is the first timeout for this server in this fetch + * context, try setting EDNS UDP buffer size to the largest UDP + * response size we have seen from this server so far. + * + * If this server has already timed out twice or more in this + * fetch context, force setting the advertised UDP buffer size + * to 512 bytes. + */ if ((tried = triededns(fctx, sockaddr)) != NULL) { if (tried->count == 1U) { hint = dns_adb_getudpsize(fctx->adb, query->addrinfo); } else if (tried->count >= 2U) { query->options |= DNS_FETCHOPT_EDNS512; - fctx->reason = "reducing the advertised EDNS " - "UDP packet size to 512 octets"; } } } @@ -2637,6 +2614,7 @@ resquery_send(resquery_t *query) { */ if ((query->options & DNS_FETCHOPT_NOEDNS0) == 0) { if ((query->addrinfo->flags & DNS_FETCHOPT_NOEDNS0) == 0) { + uint16_t peerudpsize = 0; unsigned int version = DNS_EDNS_VERSION; unsigned int flags = query->addrinfo->flags; bool reqnsid = res->view->requestnsid; @@ -2645,9 +2623,17 @@ resquery_send(resquery_t *query) { unsigned char cookie[64]; uint16_t padding = 0; - if ((flags & FCTX_ADDRINFO_EDNSOK) != 0 && - (query->options & DNS_FETCHOPT_EDNS512) == 0) - { + /* + * If we ever received an EDNS response from this + * server, initialize 'udpsize' with a value between + * 512 and 4096, based on any potential EDNS timeouts + * observed for this particular server in the past and + * the total number of query timeouts observed for this + * fetch context so far. Clamp 'udpsize' to the global + * 'edns-udp-size' value (if unset, the latter defaults + * to 4096 bytes). + */ + if ((flags & FCTX_ADDRINFO_EDNSOK) != 0) { udpsize = dns_adb_probesize(fctx->adb, query->addrinfo, fctx->timeouts); @@ -2656,36 +2642,40 @@ resquery_send(resquery_t *query) { } } - if (peer != NULL) { - (void)dns_peer_getudpsize(peer, &udpsize); - } - - if (udpsize == 0U && res->udpsize == 512U) { - udpsize = 512; - } - /* - * Was the size forced to 512 in the configuration? - */ - if (udpsize == 512U) { - query->options |= DNS_FETCHOPT_EDNS512; - } - - /* - * We have talked to this server before. + * This server timed out for the first time in this + * fetch context and we received a response from it + * before (either in this fetch context or in a + * different one). Set 'udpsize' to the size of the + * largest UDP response we have received from this + * server so far. */ if (hint != 0U) { udpsize = hint; } /* - * We know nothing about the peer's capabilities - * so start with minimal EDNS UDP size. + * If we have not received any responses from this + * server before or if this server has already timed + * out twice or more in this fetch context, use an EDNS + * UDP buffer size of 512 bytes. */ - if (udpsize == 0U) { + if (udpsize == 0U || + (query->options & DNS_FETCHOPT_EDNS512) != 0) { udpsize = 512; } + /* + * If a fixed EDNS UDP buffer size is configured for + * this server, make sure we obey that. + */ + if (peer != NULL) { + (void)dns_peer_getudpsize(peer, &peerudpsize); + if (peerudpsize != 0) { + udpsize = peerudpsize; + } + } + if ((flags & DNS_FETCHOPT_EDNSVERSIONSET) != 0) { version = flags & DNS_FETCHOPT_EDNSVERSIONMASK; version >>= DNS_FETCHOPT_EDNSVERSIONSHIFT; @@ -4583,7 +4573,6 @@ fctx_timeout(isc_task_t *task, isc_event_t *event) { inc_stats(fctx->res, dns_resstatscounter_querytimeout); if (event->ev_type == ISC_TIMEREVENT_LIFE) { - fctx->reason = NULL; fctx_done(fctx, ISC_R_TIMEDOUT, __LINE__); } else { isc_result_t result; @@ -4996,7 +4985,6 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, atomic_init(&fctx->attributes, 0); fctx->spilled = false; fctx->nqueries = 0; - fctx->reason = NULL; fctx->rand_buf = 0; fctx->rand_bits = 0; fctx->timeout = false;