From b220993d65196174d8dd28a17bff97b1e9817ae0 Mon Sep 17 00:00:00 2001 From: crazyxy Date: Tue, 25 Jun 2019 00:48:15 -0700 Subject: [PATCH 1/7] Upgrade c-ares. Switch to ares_getaddrinfo Signed-off-by: crazyxy --- bazel/repository_locations.bzl | 10 +++-- source/common/network/dns_impl.cc | 73 +++++++++++++++++-------------- source/common/network/dns_impl.h | 12 ++--- tools/spelling_dictionary.txt | 1 + 4 files changed, 55 insertions(+), 41 deletions(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 9960796a9b88e..9541ccca159f6 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -27,9 +27,13 @@ REPOSITORY_LOCATIONS = dict( urls = ["https://files.pythonhosted.org/packages/c6/b4/510617906f8e0c5660e7d96fbc5585113f83ad547a3989b80297ac72a74c/thrift-0.11.0.tar.gz"], ), com_github_c_ares_c_ares = dict( - sha256 = "7deb7872cbd876c29036d5f37e30c4cbc3cc068d59d8b749ef85bb0736649f04", - strip_prefix = "c-ares-cares-1_15_0", - urls = ["https://github.com/c-ares/c-ares/archive/cares-1_15_0.tar.gz"], + sha256 = "96edccdb19d79f6bc48c2c0e5916346c8f0507efa72e76bd146a1b9d05f93c2a", + strip_prefix = "c-ares-5dd3629bc93449840c36dd635ea6cce606b8c366", + # 2019-06-19 + # 21 new commits from release-1.15.0. Upgrade for commit 7d3591ee8a1a63e7748e68e6d880bd1763a32885 "getaddrinfo enhancements" + # Use getaddrinfo to query DNS record and TTL. + # TODO(crazyxy): Update to release-1.16.0 when it is released. + urls = ["https://github.com/c-ares/c-ares/archive/5dd3629bc93449840c36dd635ea6cce606b8c366.tar.gz"], ), com_github_circonus_labs_libcircllhist = dict( sha256 = "8165aa25e529d7d4b9ae849d3bf30371255a99d6db0421516abcff23214cdc2c", diff --git a/source/common/network/dns_impl.cc b/source/common/network/dns_impl.cc index 3702210c50cef..28678b6f6f182 100644 --- a/source/common/network/dns_impl.cc +++ b/source/common/network/dns_impl.cc @@ -67,8 +67,8 @@ void DnsResolverImpl::initializeChannel(ares_options* options, int optmask) { ares_init_options(&channel_, options, optmask | ARES_OPT_SOCK_STATE_CB); } -void DnsResolverImpl::PendingResolution::onAresHostCallback(int status, int timeouts, - hostent* hostent) { +void DnsResolverImpl::PendingResolution::onAresGetAddrInfoCallback(int status, int timeouts, + ares_addrinfo* addrinfo) { // We receive ARES_EDESTRUCTION when destructing with pending queries. if (status == ARES_EDESTRUCTION) { ASSERT(owned_); @@ -81,32 +81,37 @@ void DnsResolverImpl::PendingResolution::onAresHostCallback(int status, int time std::list address_list; if (status == ARES_SUCCESS) { - if (hostent->h_addrtype == AF_INET) { - for (int i = 0; hostent->h_addr_list[i] != nullptr; ++i) { - ASSERT(hostent->h_length == sizeof(in_addr)); - sockaddr_in address; - memset(&address, 0, sizeof(address)); - address.sin_family = AF_INET; - address.sin_port = 0; - address.sin_addr = *reinterpret_cast(hostent->h_addr_list[i]); - address_list.emplace_back(new Address::Ipv4Instance(&address)); + if (addrinfo != nullptr && addrinfo->nodes != nullptr) { + if (addrinfo->nodes->ai_family == AF_INET) { + for (const ares_addrinfo_node* ai = addrinfo->nodes; ai != nullptr; ai = ai->ai_next) { + sockaddr_in address; + memset(&address, 0, sizeof(address)); + address.sin_family = AF_INET; + address.sin_port = 0; + address.sin_addr = reinterpret_cast(ai->ai_addr)->sin_addr; + address_list.emplace_back(new Address::Ipv4Instance(&address)); + } + } else if (addrinfo->nodes->ai_family == AF_INET6) { + for (const ares_addrinfo_node* ai = addrinfo->nodes; ai != nullptr; ai = ai->ai_next) { + sockaddr_in6 address; + memset(&address, 0, sizeof(address)); + address.sin6_family = AF_INET6; + address.sin6_port = 0; + address.sin6_addr = reinterpret_cast(ai->ai_addr)->sin6_addr; + address_list.emplace_back(new Address::Ipv6Instance(address)); + } } - } else if (hostent->h_addrtype == AF_INET6) { - for (int i = 0; hostent->h_addr_list[i] != nullptr; ++i) { - ASSERT(hostent->h_length == sizeof(in6_addr)); - sockaddr_in6 address; - memset(&address, 0, sizeof(address)); - address.sin6_family = AF_INET6; - address.sin6_port = 0; - address.sin6_addr = *reinterpret_cast(hostent->h_addr_list[i]); - address_list.emplace_back(new Address::Ipv6Instance(address)); - } - } - if (!address_list.empty()) { - completed_ = true; } } + if (!address_list.empty()) { + completed_ = true; + } + + if (addrinfo) { + ares_freeaddrinfo(addrinfo); + } + if (timeouts > 0) { ENVOY_LOG(debug, "DNS request timed out {} times", timeouts); } @@ -134,7 +139,7 @@ void DnsResolverImpl::PendingResolution::onAresHostCallback(int status, int time if (!completed_ && fallback_if_failed_) { fallback_if_failed_ = false; - getHostByName(AF_INET); + getAddrInfo(AF_INET); // Note: Nothing can follow this call to getHostByName due to deletion of this // object upon synchronous resolution. return; @@ -195,9 +200,9 @@ ActiveDnsQuery* DnsResolverImpl::resolve(const std::string& dns_name, } if (dns_lookup_family == DnsLookupFamily::V4Only) { - pending_resolution->getHostByName(AF_INET); + pending_resolution->getAddrInfo(AF_INET); } else { - pending_resolution->getHostByName(AF_INET6); + pending_resolution->getAddrInfo(AF_INET6); } if (pending_resolution->completed_) { @@ -215,11 +220,15 @@ ActiveDnsQuery* DnsResolverImpl::resolve(const std::string& dns_name, } } -void DnsResolverImpl::PendingResolution::getHostByName(int family) { - ares_gethostbyname( - channel_, dns_name_.c_str(), family, - [](void* arg, int status, int timeouts, hostent* hostent) { - static_cast(arg)->onAresHostCallback(status, timeouts, hostent); +void DnsResolverImpl::PendingResolution::getAddrInfo(int family) { + struct ares_addrinfo_hints hints = {}; + hints.ai_family = family; + hints.ai_flags = ARES_AI_CANONNAME | ARES_AI_ENVHOSTS | ARES_AI_NOSORT; + + ares_getaddrinfo( + channel_, dns_name_.c_str(), /* service */ nullptr, &hints, + [](void* arg, int status, int timeouts, ares_addrinfo* addrinfo) { + static_cast(arg)->onAresGetAddrInfoCallback(status, timeouts, addrinfo); }, this); } diff --git a/source/common/network/dns_impl.h b/source/common/network/dns_impl.h index 1dad26e11a017..096c2c71178f3 100644 --- a/source/common/network/dns_impl.h +++ b/source/common/network/dns_impl.h @@ -50,17 +50,17 @@ class DnsResolverImpl : public DnsResolver, protected Logger::Loggable Date: Tue, 25 Jun 2019 00:57:57 -0700 Subject: [PATCH 2/7] Update document Signed-off-by: crazyxy --- docs/root/intro/version_history.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 198cef464973a..a06cbbc2b44e6 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -16,6 +16,7 @@ Version history * build: releases are built with Clang and linked with LLD. * control-plane: management servers can respond with HTTP 304 to indicate that config is up to date for Envoy proxies polling a :ref:`REST API Config Type ` * csrf: added support for whitelisting additional source origins. +* dns: upgrade c-ares. Switch from ares_gethostbyname to ares_getaddrinfo. * dubbo_proxy: support the :ref:`Dubbo proxy filter `. * eds: added support to specify max time for which endpoints can be used :ref:`gRPC filter `. * event: added :ref:`loop duration and poll delay statistics `. From e77090835afef19e63c295916909e0a088306cfc Mon Sep 17 00:00:00 2001 From: crazyxy Date: Tue, 25 Jun 2019 16:47:11 -0700 Subject: [PATCH 3/7] Resolve comments Signed-off-by: crazyxy --- docs/root/intro/version_history.rst | 1 - source/common/network/dns_impl.cc | 19 +++++++++++++------ tools/spelling_dictionary.txt | 1 + 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index a06cbbc2b44e6..198cef464973a 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -16,7 +16,6 @@ Version history * build: releases are built with Clang and linked with LLD. * control-plane: management servers can respond with HTTP 304 to indicate that config is up to date for Envoy proxies polling a :ref:`REST API Config Type ` * csrf: added support for whitelisting additional source origins. -* dns: upgrade c-ares. Switch from ares_gethostbyname to ares_getaddrinfo. * dubbo_proxy: support the :ref:`Dubbo proxy filter `. * eds: added support to specify max time for which endpoints can be used :ref:`gRPC filter `. * event: added :ref:`loop duration and poll delay statistics `. diff --git a/source/common/network/dns_impl.cc b/source/common/network/dns_impl.cc index 28678b6f6f182..40252aa87a8c0 100644 --- a/source/common/network/dns_impl.cc +++ b/source/common/network/dns_impl.cc @@ -102,14 +102,14 @@ void DnsResolverImpl::PendingResolution::onAresGetAddrInfoCallback(int status, i } } } - } - if (!address_list.empty()) { - completed_ = true; - } + if (!address_list.empty()) { + completed_ = true; + } - if (addrinfo) { - ares_freeaddrinfo(addrinfo); + if (addrinfo) { + ares_freeaddrinfo(addrinfo); + } } if (timeouts > 0) { @@ -223,6 +223,13 @@ ActiveDnsQuery* DnsResolverImpl::resolve(const std::string& dns_name, void DnsResolverImpl::PendingResolution::getAddrInfo(int family) { struct ares_addrinfo_hints hints = {}; hints.ai_family = family; + + /** + * ARES_AI_CANONNAME the ares_addrinfo structure will return a canonical names list + * ARES_AI_ENVHOSTS read hosts file path from the environment variable + * ARES_AI_NOSORT result addresses will not be sorted and no connections to resolved addresses + * will be attempted + */ hints.ai_flags = ARES_AI_CANONNAME | ARES_AI_ENVHOSTS | ARES_AI_NOSORT; ares_getaddrinfo( diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index 86e3ab60e1db6..17da53d9c37c6 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -164,6 +164,7 @@ NOAUTH NOLINT NOLINTNEXTLINE NONCES +NOSORT NS NUL Nilsson From 44718e7123e2d9aa40aca0d63d306ee11f30fcde Mon Sep 17 00:00:00 2001 From: crazyxy Date: Tue, 25 Jun 2019 19:56:19 -0700 Subject: [PATCH 4/7] resolve comment Signed-off-by: crazyxy --- source/common/network/dns_impl.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source/common/network/dns_impl.cc b/source/common/network/dns_impl.cc index 40252aa87a8c0..9fb0048b0f45e 100644 --- a/source/common/network/dns_impl.cc +++ b/source/common/network/dns_impl.cc @@ -225,12 +225,11 @@ void DnsResolverImpl::PendingResolution::getAddrInfo(int family) { hints.ai_family = family; /** - * ARES_AI_CANONNAME the ares_addrinfo structure will return a canonical names list * ARES_AI_ENVHOSTS read hosts file path from the environment variable * ARES_AI_NOSORT result addresses will not be sorted and no connections to resolved addresses * will be attempted */ - hints.ai_flags = ARES_AI_CANONNAME | ARES_AI_ENVHOSTS | ARES_AI_NOSORT; + hints.ai_flags = ARES_AI_ENVHOSTS | ARES_AI_NOSORT; ares_getaddrinfo( channel_, dns_name_.c_str(), /* service */ nullptr, &hints, From 078dec414fd116c3a2367aeb061c999652ac20df Mon Sep 17 00:00:00 2001 From: crazyxy Date: Tue, 25 Jun 2019 20:16:09 -0700 Subject: [PATCH 5/7] Use Assert Signed-off-by: crazyxy --- source/common/network/dns_impl.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/source/common/network/dns_impl.cc b/source/common/network/dns_impl.cc index 9fb0048b0f45e..428e5d5bc2c11 100644 --- a/source/common/network/dns_impl.cc +++ b/source/common/network/dns_impl.cc @@ -107,9 +107,8 @@ void DnsResolverImpl::PendingResolution::onAresGetAddrInfoCallback(int status, i completed_ = true; } - if (addrinfo) { - ares_freeaddrinfo(addrinfo); - } + ASSERT(addrinfo != nullptr); + ares_freeaddrinfo(addrinfo); } if (timeouts > 0) { From c2b8ff976cf6e7b79000bd6db4f1739ccfd837c5 Mon Sep 17 00:00:00 2001 From: crazyxy Date: Wed, 26 Jun 2019 14:08:55 -0700 Subject: [PATCH 6/7] remove hints Signed-off-by: crazyxy --- source/common/network/dns_impl.cc | 7 ------- tools/spelling_dictionary.txt | 1 - 2 files changed, 8 deletions(-) diff --git a/source/common/network/dns_impl.cc b/source/common/network/dns_impl.cc index 428e5d5bc2c11..37839fe0ea562 100644 --- a/source/common/network/dns_impl.cc +++ b/source/common/network/dns_impl.cc @@ -223,13 +223,6 @@ void DnsResolverImpl::PendingResolution::getAddrInfo(int family) { struct ares_addrinfo_hints hints = {}; hints.ai_family = family; - /** - * ARES_AI_ENVHOSTS read hosts file path from the environment variable - * ARES_AI_NOSORT result addresses will not be sorted and no connections to resolved addresses - * will be attempted - */ - hints.ai_flags = ARES_AI_ENVHOSTS | ARES_AI_NOSORT; - ares_getaddrinfo( channel_, dns_name_.c_str(), /* service */ nullptr, &hints, [](void* arg, int status, int timeouts, ares_addrinfo* addrinfo) { diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index 17da53d9c37c6..86e3ab60e1db6 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -164,7 +164,6 @@ NOAUTH NOLINT NOLINTNEXTLINE NONCES -NOSORT NS NUL Nilsson From de191346bd36d8da5ec074011cb65187201abd78 Mon Sep 17 00:00:00 2001 From: crazyxy Date: Wed, 26 Jun 2019 15:02:54 -0700 Subject: [PATCH 7/7] add nosort Signed-off-by: crazyxy --- source/common/network/dns_impl.cc | 6 ++++++ tools/spelling_dictionary.txt | 1 + 2 files changed, 7 insertions(+) diff --git a/source/common/network/dns_impl.cc b/source/common/network/dns_impl.cc index 37839fe0ea562..29e2b48042fa6 100644 --- a/source/common/network/dns_impl.cc +++ b/source/common/network/dns_impl.cc @@ -223,6 +223,12 @@ void DnsResolverImpl::PendingResolution::getAddrInfo(int family) { struct ares_addrinfo_hints hints = {}; hints.ai_family = family; + /** + * ARES_AI_NOSORT result addresses will not be sorted and no connections to resolved addresses + * will be attempted + */ + hints.ai_flags = ARES_AI_NOSORT; + ares_getaddrinfo( channel_, dns_name_.c_str(), /* service */ nullptr, &hints, [](void* arg, int status, int timeouts, ares_addrinfo* addrinfo) { diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index 86e3ab60e1db6..17da53d9c37c6 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -164,6 +164,7 @@ NOAUTH NOLINT NOLINTNEXTLINE NONCES +NOSORT NS NUL Nilsson