From: Matthias-Christian Ott Date: Mon, 11 Jun 2018 18:07:36 +0000 (+0200) Subject: resolve: do not derive query timeout from RTT X-Git-Tag: v239~84 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=dbc4661a2c527ead70445bfd8f1fa47e2c4971a8;p=platform%2Fupstream%2Fsystemd.git resolve: do not derive query timeout from RTT DNS queries need timeout values to detect whether a DNS server is unresponsive or, if the query is sent over UDP, whether a DNS message was lost and has to be resent. The total time that it takes to answer a query to arrive is t + RTT, where t is the maximum time that the DNS server that is being queried needs to answer the query. An authoritative server stores a copy of the zone that it serves in main memory or secondary storage, so t is very small and therefore the time that it takes to answer a query is almost entirely determined by the RTT. Modern authoritative server software keeps its zones in main memory and, for example, Knot DNS and NSD are able to answer in less than 100 µs [1]. So iterative resolvers continuously measure the RTT to optimize their query timeouts and to resend queries more quickly if they are lost. systemd-resolved is a stub resolver: it forwards DNS queries to an upstream resolver and waits for an answer. So the time that it takes for systemd-resolved to answer a query is determined by the RTT and the time that it takes the upstream resolver to answer the query. It seems common for iterative resolver software to set a total timeout for the query. Such total timeout subsumes the timeout of all queries that the iterative has to make to answer a query. For example, BIND seems to use a default timeout of 10 s. At the moment systemd-resolved derives its query timeout entirely from the RTT and does not consider the query timeout of the upstream resolver. Therefore it often mistakenly degrades the feature set of its upstream resolvers if it takes them longer than usual to answer a query. It has been reported to be a considerable problem in practice, in particular if DNSSEC=yes. So the query timeout systemd-resolved should be derived from the timeout of the upstream resolved and the RTT to the upstream resolver. At the moment systemd-resolved measures the RTT as the time that it takes the upstream resolver to answer a query. This clearly leads to incorrect measurements. In order to correctly measure the RTT systemd-resolved would have to measure RTT separately and continuously, for example with a query with an empty question section or a query for the SOA RR of the root zone so that the upstream resolver would be able to answer to query without querying another server. However, this requires significant changes to systemd-resolved. So it seems best to postpone them until other issues have been addressed and to set the resend timeout to a fixed value for now. As mentioned, BIND seems to use a timeout of 10 s, so perhaps 12 s is a reasonable value that also accounts for common RTT values. If we assume that the we are going to retry, it could be less. So it should be enough to set the resend timeout to DNS_TIMEOUT_MAX_USEC as DNS_SERVER_FEATURE_RETRY_ATTEMPTS * DNS_TIMEOUT_MAX_USEC = 15 s. However, this will not solve the incorrect feature set degradation and should be seen as a temporary change until systemd-resolved does probe the feature set of an upstream resolver independently from the actual queries. [1] https://www.knot-dns.cz/benchmark/ --- diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index 164f460..133d26d 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -15,10 +15,6 @@ #include "string-table.h" #include "string-util.h" -/* After how much time to repeat classic DNS requests */ -#define DNS_TIMEOUT_MIN_USEC (750 * USEC_PER_MSEC) -#define DNS_TIMEOUT_MAX_USEC (SD_RESOLVED_QUERY_TIMEOUT_USEC / DNS_TRANSACTION_ATTEMPTS_MAX) - /* The amount of time to wait before retrying with a full feature set */ #define DNS_SERVER_FEATURE_GRACE_PERIOD_MAX_USEC (6 * USEC_PER_HOUR) #define DNS_SERVER_FEATURE_GRACE_PERIOD_MIN_USEC (5 * USEC_PER_MINUTE) @@ -265,7 +261,7 @@ static void dns_server_reset_counters(DnsServer *s) { * incomplete. */ } -void dns_server_packet_received(DnsServer *s, int protocol, DnsServerFeatureLevel level, usec_t rtt, size_t size) { +void dns_server_packet_received(DnsServer *s, int protocol, DnsServerFeatureLevel level, size_t size) { assert(s); if (protocol == IPPROTO_UDP) { @@ -304,14 +300,6 @@ void dns_server_packet_received(DnsServer *s, int protocol, DnsServerFeatureLeve this size. */ if (protocol == IPPROTO_UDP && s->received_udp_packet_max < size) s->received_udp_packet_max = size; - - if (s->max_rtt < rtt) { - s->max_rtt = rtt; - s->resend_timeout = CLAMP(s->max_rtt * 2, DNS_TIMEOUT_MIN_USEC, DNS_TIMEOUT_MAX_USEC); - } else if (s->resend_timeout > rtt) - /* If we received the packet faster than the resend_timeout, bias - * the resend_timeout back to the rtt. */ - s->resend_timeout = CLAMP((2 * s->resend_timeout + rtt) / 3, DNS_TIMEOUT_MIN_USEC, DNS_TIMEOUT_MAX_USEC); } void dns_server_packet_lost(DnsServer *s, int protocol, DnsServerFeatureLevel level, usec_t usec) { @@ -328,11 +316,6 @@ void dns_server_packet_lost(DnsServer *s, int protocol, DnsServerFeatureLevel le s->n_failed_tcp++; } } - - if (s->resend_timeout > usec) - return; - - s->resend_timeout = MIN(s->resend_timeout * 2, DNS_TIMEOUT_MAX_USEC); } void dns_server_packet_truncated(DnsServer *s, DnsServerFeatureLevel level) { @@ -859,9 +842,6 @@ void dns_server_flush_cache(DnsServer *s) { void dns_server_reset_features(DnsServer *s) { assert(s); - s->max_rtt = 0; - s->resend_timeout = DNS_TIMEOUT_MIN_USEC; - s->verified_feature_level = _DNS_SERVER_FEATURE_LEVEL_INVALID; s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_BEST; diff --git a/src/resolve/resolved-dns-server.h b/src/resolve/resolved-dns-server.h index a546eb3..6b62e44 100644 --- a/src/resolve/resolved-dns-server.h +++ b/src/resolve/resolved-dns-server.h @@ -67,9 +67,6 @@ struct DnsServer { gnutls_datum_t tls_session_data; #endif - usec_t resend_timeout; - usec_t max_rtt; - DnsServerFeatureLevel verified_feature_level; DnsServerFeatureLevel possible_feature_level; @@ -112,7 +109,7 @@ DnsServer* dns_server_unref(DnsServer *s); void dns_server_unlink(DnsServer *s); void dns_server_move_back_and_unmark(DnsServer *s); -void dns_server_packet_received(DnsServer *s, int protocol, DnsServerFeatureLevel level, usec_t rtt, size_t size); +void dns_server_packet_received(DnsServer *s, int protocol, DnsServerFeatureLevel level, size_t size); void dns_server_packet_lost(DnsServer *s, int protocol, DnsServerFeatureLevel level, usec_t usec); void dns_server_packet_truncated(DnsServer *s, DnsServerFeatureLevel level); void dns_server_packet_rrsig_missing(DnsServer *s, DnsServerFeatureLevel level); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 99c7496..7a512a5 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -25,6 +25,9 @@ #define TRANSACTIONS_MAX 4096 #define TRANSACTION_TCP_TIMEOUT_USEC (10U*USEC_PER_SEC) +/* After how much time to repeat classic DNS requests */ +#define DNS_TIMEOUT_USEC (SD_RESOLVED_QUERY_TIMEOUT_USEC / DNS_TRANSACTION_ATTEMPTS_MAX) + static void dns_transaction_reset_answer(DnsTransaction *t) { assert(t); @@ -1137,7 +1140,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { dns_server_packet_bad_opt(t->server, t->current_feature_level); /* Report that we successfully received a packet */ - dns_server_packet_received(t->server, p->ipproto, t->current_feature_level, ts - t->start_usec, p->size); + dns_server_packet_received(t->server, p->ipproto, t->current_feature_level, p->size); } /* See if we know things we didn't know before that indicate we better restart the lookup immediately. */ @@ -1355,8 +1358,7 @@ static usec_t transaction_get_resend_timeout(DnsTransaction *t) { if (t->stream) return TRANSACTION_TCP_TIMEOUT_USEC; - assert(t->server); - return t->server->resend_timeout; + return DNS_TIMEOUT_USEC; case DNS_PROTOCOL_MDNS: assert(t->n_attempts > 0);