From 7c4538cb53631a907c215657fc7f29ae734a46a0 Mon Sep 17 00:00:00 2001 From: Ryan Hamilton Date: Thu, 4 Nov 2021 23:48:47 +0000 Subject: [PATCH 1/9] upstream: Implement Happy Eyeballs address sorting Signed-off-by: Ryan Hamilton --- .../network/happy_eyeballs_connection_impl.cc | 38 ++++++++++++- .../network/happy_eyeballs_connection_impl.h | 9 +++- .../happy_eyeballs_connection_impl_test.cc | 54 +++++++++++++++++-- 3 files changed, 94 insertions(+), 7 deletions(-) diff --git a/source/common/network/happy_eyeballs_connection_impl.cc b/source/common/network/happy_eyeballs_connection_impl.cc index 40802f0919fb3..168f7d5bc1a7e 100644 --- a/source/common/network/happy_eyeballs_connection_impl.cc +++ b/source/common/network/happy_eyeballs_connection_impl.cc @@ -10,7 +10,8 @@ HappyEyeballsConnectionImpl::HappyEyeballsConnectionImpl( Address::InstanceConstSharedPtr source_address, TransportSocketFactory& socket_factory, TransportSocketOptionsConstSharedPtr transport_socket_options, const ConnectionSocket::OptionsSharedPtr options) - : id_(ConnectionImpl::next_global_id_++), dispatcher_(dispatcher), address_list_(address_list), + : id_(ConnectionImpl::next_global_id_++), dispatcher_(dispatcher), + address_list_(sortAddresses(address_list)), connection_construction_state_( {source_address, socket_factory, transport_socket_options, options}), next_attempt_timer_(dispatcher_.createTimer([this]() -> void { tryAnotherConnection(); })) { @@ -379,6 +380,41 @@ void HappyEyeballsConnectionImpl::dumpState(std::ostream& os, int indent_level) } } +std::vector +HappyEyeballsConnectionImpl::sortAddresses(const std::vector& in) { + std::vector address_list = in; + for (size_t current = 1; current < address_list.size(); ++current) { + // If the current address has a different family than the previous address then + // it is in the correct position. + if (address_list[current]->type() != Address::Type::Ip || + address_list[current - 1]->type() != Address::Type::Ip || + address_list[current]->ip()->version() != address_list[current - 1]->ip()->version()) { + continue; + } + // Find the first address with a different address family than the current address. + size_t next = current + 1; + while (next < address_list.size()) { + if (address_list[next]->type() != Address::Type::Ip || + address_list[current]->ip()->version() != address_list[next]->ip()->version()) { + break; + } + ++next; + } + // There are no more addresses with different families so sorting is finished. + if (next >= address_list.size()) { + break; + } + + // Bubble the address up to the current position. + for (size_t i = next; i > current; i--) { + using std::swap; + swap(address_list[i], address_list[i - 1]); + } + current = next - 1; + } + return address_list; +} + ClientConnectionPtr HappyEyeballsConnectionImpl::createNextConnection() { ASSERT(next_address_ < address_list_.size()); auto connection = dispatcher_.createClientConnection( diff --git a/source/common/network/happy_eyeballs_connection_impl.h b/source/common/network/happy_eyeballs_connection_impl.h index 4a64bc420d676..b071e652bea05 100644 --- a/source/common/network/happy_eyeballs_connection_impl.h +++ b/source/common/network/happy_eyeballs_connection_impl.h @@ -98,6 +98,13 @@ class HappyEyeballsConnectionImpl : public ClientConnection, void hashKey(std::vector& hash_key) const override; void dumpState(std::ostream& os, int indent_level) const override; + // Returns a new vector containing the contents of |address_list| sorted + // with address families interleaved, as per Section 4 of RFC 8305, Happy + // Eyeballs v2. It is assumed that The list must already sorted as per + // Section 6 of RFC6724, which happens in ares_getaddrinfo(). + static std::vector + sortAddresses(const std::vector& address_list); + private: // ConnectionCallbacks which will be set on an ClientConnection which // sends connection events back to the HappyEyeballsConnectionImpl. @@ -196,7 +203,7 @@ class HappyEyeballsConnectionImpl : public ClientConnection, Event::Dispatcher& dispatcher_; // List of addresses to attempt to connect to. - const std::vector& address_list_; + const std::vector address_list_; // Index of the next address to use. size_t next_address_ = 0; diff --git a/test/common/network/happy_eyeballs_connection_impl_test.cc b/test/common/network/happy_eyeballs_connection_impl_test.cc index faa25b383a5b8..8f147f316e61a 100644 --- a/test/common/network/happy_eyeballs_connection_impl_test.cc +++ b/test/common/network/happy_eyeballs_connection_impl_test.cc @@ -21,9 +21,10 @@ class HappyEyeballsConnectionImplTest : public testing::Test { : failover_timer_(new testing::StrictMock(&dispatcher_)), transport_socket_options_(std::make_shared()), options_(std::make_shared()), - address_list_({std::make_shared("127.0.0.1"), - std::make_shared("127.0.0.2"), - std::make_shared("127.0.0.3")}) { + raw_address_list_({std::make_shared("127.0.0.1"), + std::make_shared("127.0.0.2"), + std::make_shared("ff02::1", 0)}), + address_list_({raw_address_list_[0], raw_address_list_[2], raw_address_list_[1]}) { EXPECT_CALL(transport_socket_factory_, createTransportSocket(_)); EXPECT_CALL(dispatcher_, createClientConnection_(address_list_[0], _, _, _)) .WillOnce(testing::InvokeWithoutArgs( @@ -31,8 +32,8 @@ class HappyEyeballsConnectionImplTest : public testing::Test { next_connections_.push_back(std::make_unique>()); impl_ = std::make_unique( - dispatcher_, address_list_, Address::InstanceConstSharedPtr(), transport_socket_factory_, - transport_socket_options_, options_); + dispatcher_, raw_address_list_, Address::InstanceConstSharedPtr(), + transport_socket_factory_, transport_socket_options_, options_); } // Called by the dispatcher to return a MockClientConnection. In order to allow expectations to @@ -94,6 +95,7 @@ class HappyEyeballsConnectionImplTest : public testing::Test { MockTransportSocketFactory transport_socket_factory_; TransportSocketOptionsConstSharedPtr transport_socket_options_; const ConnectionSocket::OptionsSharedPtr options_; + const std::vector raw_address_list_; const std::vector address_list_; std::vector*> created_connections_; std::vector connection_callbacks_; @@ -1049,5 +1051,47 @@ TEST_F(HappyEyeballsConnectionImplTest, LastRoundTripTime) { EXPECT_EQ(rtt, impl_->lastRoundTripTime()); } +TEST_F(HappyEyeballsConnectionImplTest, SortAddresses) { + auto ip_v4_1 = std::make_shared("127.0.0.1"); + auto ip_v4_2 = std::make_shared("127.0.0.2"); + auto ip_v4_3 = std::make_shared("127.0.0.3"); + auto ip_v4_4 = std::make_shared("127.0.0.4"); + + auto ip_v6_1 = std::make_shared("ff02::1", 0); + auto ip_v6_2 = std::make_shared("ff02::2", 0); + auto ip_v6_3 = std::make_shared("ff02::3", 0); + auto ip_v6_4 = std::make_shared("ff02::4", 0); + + // All v4 address so unchanged. + std::vector v4_list = {ip_v4_1, ip_v4_2, ip_v4_3, ip_v4_4}; + EXPECT_EQ(v4_list, HappyEyeballsConnectionImpl::sortAddresses(v4_list)); + + // All v6 address so unchanged. + std::vector v6_list = {ip_v6_1, ip_v6_2, ip_v6_3, ip_v6_4}; + EXPECT_EQ(v6_list, HappyEyeballsConnectionImpl::sortAddresses(v6_list)); + + std::vector v6_then_v4 = {ip_v6_1, ip_v6_2, ip_v4_1, ip_v4_2}; + std::vector interleaved = {ip_v6_1, ip_v4_1, ip_v6_2, ip_v4_2}; + EXPECT_EQ(interleaved, HappyEyeballsConnectionImpl::sortAddresses(v6_then_v4)); + + std::vector v6_then_single_v4 = {ip_v6_1, ip_v6_2, ip_v6_3, + ip_v4_1}; + std::vector interleaved2 = {ip_v6_1, ip_v4_1, ip_v6_2, ip_v6_3}; + EXPECT_EQ(interleaved2, HappyEyeballsConnectionImpl::sortAddresses(v6_then_single_v4)); + + std::vector mixed = {ip_v6_1, ip_v6_2, ip_v6_3, ip_v4_1, + ip_v4_2, ip_v4_3, ip_v4_4, ip_v6_4}; + std::vector interleaved3 = {ip_v6_1, ip_v4_1, ip_v6_2, ip_v4_2, + ip_v6_3, ip_v4_3, ip_v6_4, ip_v4_4}; + EXPECT_EQ(interleaved3, HappyEyeballsConnectionImpl::sortAddresses(mixed)); + for (size_t i = 0; i < interleaved3.size(); ++i) { + std::cout << i << " " << interleaved3[i]->asString() << "\n"; + } + interleaved3 = HappyEyeballsConnectionImpl::sortAddresses(mixed); + for (size_t i = 0; i < interleaved3.size(); ++i) { + std::cout << i << " " << interleaved3[i]->asString() << "\n"; + } +} + } // namespace Network } // namespace Envoy From e03104a1b5cf4d240b75d075f6a59e118f104197 Mon Sep 17 00:00:00 2001 From: Ryan Hamilton Date: Fri, 5 Nov 2021 00:03:11 +0000 Subject: [PATCH 2/9] no logging Signed-off-by: Ryan Hamilton --- test/common/network/happy_eyeballs_connection_impl_test.cc | 7 ------- 1 file changed, 7 deletions(-) diff --git a/test/common/network/happy_eyeballs_connection_impl_test.cc b/test/common/network/happy_eyeballs_connection_impl_test.cc index 8f147f316e61a..2588af571f164 100644 --- a/test/common/network/happy_eyeballs_connection_impl_test.cc +++ b/test/common/network/happy_eyeballs_connection_impl_test.cc @@ -1084,13 +1084,6 @@ TEST_F(HappyEyeballsConnectionImplTest, SortAddresses) { std::vector interleaved3 = {ip_v6_1, ip_v4_1, ip_v6_2, ip_v4_2, ip_v6_3, ip_v4_3, ip_v6_4, ip_v4_4}; EXPECT_EQ(interleaved3, HappyEyeballsConnectionImpl::sortAddresses(mixed)); - for (size_t i = 0; i < interleaved3.size(); ++i) { - std::cout << i << " " << interleaved3[i]->asString() << "\n"; - } - interleaved3 = HappyEyeballsConnectionImpl::sortAddresses(mixed); - for (size_t i = 0; i < interleaved3.size(); ++i) { - std::cout << i << " " << interleaved3[i]->asString() << "\n"; - } } } // namespace Network From ef769a6bebb9d9ad2cff21d5fc69380dde763e86 Mon Sep 17 00:00:00 2001 From: Ryan Hamilton Date: Tue, 9 Nov 2021 20:01:08 +0000 Subject: [PATCH 3/9] WIP Signed-off-by: Ryan Hamilton --- .../network/happy_eyeballs_connection_impl.cc | 43 +++++++++++++++++++ .../happy_eyeballs_connection_impl_test.cc | 25 +++++++++-- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/source/common/network/happy_eyeballs_connection_impl.cc b/source/common/network/happy_eyeballs_connection_impl.cc index 168f7d5bc1a7e..3b8d086268308 100644 --- a/source/common/network/happy_eyeballs_connection_impl.cc +++ b/source/common/network/happy_eyeballs_connection_impl.cc @@ -380,9 +380,52 @@ void HappyEyeballsConnectionImpl::dumpState(std::ostream& os, int indent_level) } } +bool hasSameAddressFamily(const Address::InstanceConstSharedPtr& a, + const Address::InstanceConstSharedPtr& b) { + return (a->type() == Address::Type::Ip && + b->type() == Address::Type::Ip && + a->ip()->version() == b->ip()->version()); +} + std::vector HappyEyeballsConnectionImpl::sortAddresses(const std::vector& in) { std::vector address_list = in; + auto current = address_list.begin(); + std::cerr << "CURRENT: " << (*current)->asString()<<"\n"; + while (current != address_list.end()) { + std::cerr << "current: " << (*current)->asString()<<"\n"; + auto it = std::find_if(current, address_list.end(), + [&](const auto& val){ return !hasSameAddressFamily(*current, val); } ); + if (it == address_list.end()) { + break; + } + std::cerr << "it: " << (*it)->asString()<<"\n"; + std::cerr << "---\n"; + for (auto i = current; i!=address_list.end(); ++i) { + std::cerr << ": " << (*i)->asString()<<"\n"; + } + std::cerr << "===\n"; + + std::cerr << "it: " << (*it)->asString()<<"\n"; + auto start = std::make_reverse_iterator(it+1); + std::cerr << "start: " << (*start)->asString()<<"\n"; + std::cerr << "it: " << (*it)->asString()<<"\n"; + + auto end = std::make_reverse_iterator(current+1); + std::cerr << "start: " << (*start)->asString()<<"\n"; + std::cerr << "start+1: " << (*(start+1))->asString()<<"\n"; + std::cerr << "end: " << (*end)->asString()<<"\n"; + std::rotate(start, start + 1, end); + std::cerr << "---\n"; + for (auto i = current; i!=address_list.end(); ++i) { + std::cerr << ": " << (*i)->asString()<<"\n"; + } + std::cerr << "===\n"; + current++; + } + if (0==0) return address_list; + + for (size_t current = 1; current < address_list.size(); ++current) { // If the current address has a different family than the previous address then // it is in the correct position. diff --git a/test/common/network/happy_eyeballs_connection_impl_test.cc b/test/common/network/happy_eyeballs_connection_impl_test.cc index 2588af571f164..404634db6e704 100644 --- a/test/common/network/happy_eyeballs_connection_impl_test.cc +++ b/test/common/network/happy_eyeballs_connection_impl_test.cc @@ -1063,16 +1063,33 @@ TEST_F(HappyEyeballsConnectionImplTest, SortAddresses) { auto ip_v6_4 = std::make_shared("ff02::4", 0); // All v4 address so unchanged. - std::vector v4_list = {ip_v4_1, ip_v4_2, ip_v4_3, ip_v4_4}; - EXPECT_EQ(v4_list, HappyEyeballsConnectionImpl::sortAddresses(v4_list)); + // std::vector v4_list = {ip_v4_1, ip_v4_2, ip_v4_3, ip_v4_4}; + // EXPECT_EQ(v4_list, HappyEyeballsConnectionImpl::sortAddresses(v4_list)); // All v6 address so unchanged. - std::vector v6_list = {ip_v6_1, ip_v6_2, ip_v6_3, ip_v6_4}; - EXPECT_EQ(v6_list, HappyEyeballsConnectionImpl::sortAddresses(v6_list)); + // std::vector v6_list = {ip_v6_1, ip_v6_2, ip_v6_3, ip_v6_4}; + // EXPECT_EQ(v6_list, HappyEyeballsConnectionImpl::sortAddresses(v6_list)); + { + std::vector v6_then_v4 = {ip_v6_1, ip_v6_2, ip_v6_3, ip_v6_4, ip_v4_1, ip_v4_2}; + std::vector interleaved = {ip_v6_1, ip_v4_1, ip_v6_2, ip_v4_2, ip_v6_3, ip_v6_4}; + std::cerr << "------------------------------\n"; + EXPECT_EQ(interleaved, HappyEyeballsConnectionImpl::sortAddresses(v6_then_v4)); + auto out = HappyEyeballsConnectionImpl::sortAddresses(v6_then_v4); + for (auto i = out.begin(); i!=out.end(); ++i) { + std::cerr << "*: " << (*i)->asString()<<"\n"; + } + std::cerr << "------------------------------\n"; + } std::vector v6_then_v4 = {ip_v6_1, ip_v6_2, ip_v4_1, ip_v4_2}; std::vector interleaved = {ip_v6_1, ip_v4_1, ip_v6_2, ip_v4_2}; + std::cerr << "------------------------------\n"; EXPECT_EQ(interleaved, HappyEyeballsConnectionImpl::sortAddresses(v6_then_v4)); + auto out = HappyEyeballsConnectionImpl::sortAddresses(v6_then_v4); + for (auto i = out.begin(); i!=out.end(); ++i) { + std::cerr << "*: " << (*i)->asString()<<"\n"; + } + std::cerr << "------------------------------\n"; std::vector v6_then_single_v4 = {ip_v6_1, ip_v6_2, ip_v6_3, ip_v4_1}; From 3f3a4ac10e1a30ba0e335eb21164ad078b12534a Mon Sep 17 00:00:00 2001 From: Ryan Hamilton Date: Tue, 9 Nov 2021 20:09:44 +0000 Subject: [PATCH 4/9] Cleanup Signed-off-by: Ryan Hamilton --- .../network/happy_eyeballs_connection_impl.cc | 74 ++++--------------- .../happy_eyeballs_connection_impl_test.cc | 25 +------ 2 files changed, 17 insertions(+), 82 deletions(-) diff --git a/source/common/network/happy_eyeballs_connection_impl.cc b/source/common/network/happy_eyeballs_connection_impl.cc index 3b8d086268308..2694bc06c9699 100644 --- a/source/common/network/happy_eyeballs_connection_impl.cc +++ b/source/common/network/happy_eyeballs_connection_impl.cc @@ -380,80 +380,32 @@ void HappyEyeballsConnectionImpl::dumpState(std::ostream& os, int indent_level) } } -bool hasSameAddressFamily(const Address::InstanceConstSharedPtr& a, +bool hasDifferentAddressFamily(const Address::InstanceConstSharedPtr& a, const Address::InstanceConstSharedPtr& b) { - return (a->type() == Address::Type::Ip && - b->type() == Address::Type::Ip && - a->ip()->version() == b->ip()->version()); + return (a->type() != Address::Type::Ip || + b->type() != Address::Type::Ip || + a->ip()->version() != b->ip()->version()); } std::vector HappyEyeballsConnectionImpl::sortAddresses(const std::vector& in) { std::vector address_list = in; - auto current = address_list.begin(); - std::cerr << "CURRENT: " << (*current)->asString()<<"\n"; - while (current != address_list.end()) { - std::cerr << "current: " << (*current)->asString()<<"\n"; + for (auto current = address_list.begin(); current != address_list.end(); ++current) { + // Find the first address with a different address family than the current address. auto it = std::find_if(current, address_list.end(), - [&](const auto& val){ return !hasSameAddressFamily(*current, val); } ); + [&](const auto& val){ + return (val->type() != Address::Type::Ip || + (*current)->type() != Address::Type::Ip || + (*current)->ip()->version() != val->ip()->version()); + } ); + // If there are no more addresses with different families the sorting is finished. if (it == address_list.end()) { break; } - std::cerr << "it: " << (*it)->asString()<<"\n"; - std::cerr << "---\n"; - for (auto i = current; i!=address_list.end(); ++i) { - std::cerr << ": " << (*i)->asString()<<"\n"; - } - std::cerr << "===\n"; - - std::cerr << "it: " << (*it)->asString()<<"\n"; + // Bubble the address up to the current position. auto start = std::make_reverse_iterator(it+1); - std::cerr << "start: " << (*start)->asString()<<"\n"; - std::cerr << "it: " << (*it)->asString()<<"\n"; - auto end = std::make_reverse_iterator(current+1); - std::cerr << "start: " << (*start)->asString()<<"\n"; - std::cerr << "start+1: " << (*(start+1))->asString()<<"\n"; - std::cerr << "end: " << (*end)->asString()<<"\n"; std::rotate(start, start + 1, end); - std::cerr << "---\n"; - for (auto i = current; i!=address_list.end(); ++i) { - std::cerr << ": " << (*i)->asString()<<"\n"; - } - std::cerr << "===\n"; - current++; - } - if (0==0) return address_list; - - - for (size_t current = 1; current < address_list.size(); ++current) { - // If the current address has a different family than the previous address then - // it is in the correct position. - if (address_list[current]->type() != Address::Type::Ip || - address_list[current - 1]->type() != Address::Type::Ip || - address_list[current]->ip()->version() != address_list[current - 1]->ip()->version()) { - continue; - } - // Find the first address with a different address family than the current address. - size_t next = current + 1; - while (next < address_list.size()) { - if (address_list[next]->type() != Address::Type::Ip || - address_list[current]->ip()->version() != address_list[next]->ip()->version()) { - break; - } - ++next; - } - // There are no more addresses with different families so sorting is finished. - if (next >= address_list.size()) { - break; - } - - // Bubble the address up to the current position. - for (size_t i = next; i > current; i--) { - using std::swap; - swap(address_list[i], address_list[i - 1]); - } - current = next - 1; } return address_list; } diff --git a/test/common/network/happy_eyeballs_connection_impl_test.cc b/test/common/network/happy_eyeballs_connection_impl_test.cc index 404634db6e704..2588af571f164 100644 --- a/test/common/network/happy_eyeballs_connection_impl_test.cc +++ b/test/common/network/happy_eyeballs_connection_impl_test.cc @@ -1063,33 +1063,16 @@ TEST_F(HappyEyeballsConnectionImplTest, SortAddresses) { auto ip_v6_4 = std::make_shared("ff02::4", 0); // All v4 address so unchanged. - // std::vector v4_list = {ip_v4_1, ip_v4_2, ip_v4_3, ip_v4_4}; - // EXPECT_EQ(v4_list, HappyEyeballsConnectionImpl::sortAddresses(v4_list)); + std::vector v4_list = {ip_v4_1, ip_v4_2, ip_v4_3, ip_v4_4}; + EXPECT_EQ(v4_list, HappyEyeballsConnectionImpl::sortAddresses(v4_list)); // All v6 address so unchanged. - // std::vector v6_list = {ip_v6_1, ip_v6_2, ip_v6_3, ip_v6_4}; - // EXPECT_EQ(v6_list, HappyEyeballsConnectionImpl::sortAddresses(v6_list)); - { - std::vector v6_then_v4 = {ip_v6_1, ip_v6_2, ip_v6_3, ip_v6_4, ip_v4_1, ip_v4_2}; - std::vector interleaved = {ip_v6_1, ip_v4_1, ip_v6_2, ip_v4_2, ip_v6_3, ip_v6_4}; - std::cerr << "------------------------------\n"; - EXPECT_EQ(interleaved, HappyEyeballsConnectionImpl::sortAddresses(v6_then_v4)); - auto out = HappyEyeballsConnectionImpl::sortAddresses(v6_then_v4); - for (auto i = out.begin(); i!=out.end(); ++i) { - std::cerr << "*: " << (*i)->asString()<<"\n"; - } - std::cerr << "------------------------------\n"; - } + std::vector v6_list = {ip_v6_1, ip_v6_2, ip_v6_3, ip_v6_4}; + EXPECT_EQ(v6_list, HappyEyeballsConnectionImpl::sortAddresses(v6_list)); std::vector v6_then_v4 = {ip_v6_1, ip_v6_2, ip_v4_1, ip_v4_2}; std::vector interleaved = {ip_v6_1, ip_v4_1, ip_v6_2, ip_v4_2}; - std::cerr << "------------------------------\n"; EXPECT_EQ(interleaved, HappyEyeballsConnectionImpl::sortAddresses(v6_then_v4)); - auto out = HappyEyeballsConnectionImpl::sortAddresses(v6_then_v4); - for (auto i = out.begin(); i!=out.end(); ++i) { - std::cerr << "*: " << (*i)->asString()<<"\n"; - } - std::cerr << "------------------------------\n"; std::vector v6_then_single_v4 = {ip_v6_1, ip_v6_2, ip_v6_3, ip_v4_1}; From 0a3f5da7c56ba77c42063fb0724ecc48358a15e2 Mon Sep 17 00:00:00 2001 From: Ryan Hamilton Date: Tue, 9 Nov 2021 20:12:00 +0000 Subject: [PATCH 5/9] Format Signed-off-by: Ryan Hamilton --- .../network/happy_eyeballs_connection_impl.cc | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/source/common/network/happy_eyeballs_connection_impl.cc b/source/common/network/happy_eyeballs_connection_impl.cc index 2694bc06c9699..7889169188e38 100644 --- a/source/common/network/happy_eyeballs_connection_impl.cc +++ b/source/common/network/happy_eyeballs_connection_impl.cc @@ -380,31 +380,22 @@ void HappyEyeballsConnectionImpl::dumpState(std::ostream& os, int indent_level) } } -bool hasDifferentAddressFamily(const Address::InstanceConstSharedPtr& a, - const Address::InstanceConstSharedPtr& b) { - return (a->type() != Address::Type::Ip || - b->type() != Address::Type::Ip || - a->ip()->version() != b->ip()->version()); -} - std::vector HappyEyeballsConnectionImpl::sortAddresses(const std::vector& in) { std::vector address_list = in; for (auto current = address_list.begin(); current != address_list.end(); ++current) { // Find the first address with a different address family than the current address. - auto it = std::find_if(current, address_list.end(), - [&](const auto& val){ - return (val->type() != Address::Type::Ip || - (*current)->type() != Address::Type::Ip || - (*current)->ip()->version() != val->ip()->version()); - } ); + auto it = std::find_if(current, address_list.end(), [&](const auto& val) { + return (val->type() != Address::Type::Ip || (*current)->type() != Address::Type::Ip || + (*current)->ip()->version() != val->ip()->version()); + }); // If there are no more addresses with different families the sorting is finished. if (it == address_list.end()) { break; } // Bubble the address up to the current position. - auto start = std::make_reverse_iterator(it+1); - auto end = std::make_reverse_iterator(current+1); + auto start = std::make_reverse_iterator(it + 1); + auto end = std::make_reverse_iterator(current + 1); std::rotate(start, start + 1, end); } return address_list; From eb865776db5c1ad7ea21bed9529503b36a2bd0c5 Mon Sep 17 00:00:00 2001 From: Ryan Hamilton Date: Tue, 9 Nov 2021 14:26:03 -0800 Subject: [PATCH 6/9] Update source/common/network/happy_eyeballs_connection_impl.h Co-authored-by: Jose Ulises Nino Rivera Signed-off-by: Ryan Hamilton --- source/common/network/happy_eyeballs_connection_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/network/happy_eyeballs_connection_impl.h b/source/common/network/happy_eyeballs_connection_impl.h index b071e652bea05..9894c980b24a8 100644 --- a/source/common/network/happy_eyeballs_connection_impl.h +++ b/source/common/network/happy_eyeballs_connection_impl.h @@ -100,7 +100,7 @@ class HappyEyeballsConnectionImpl : public ClientConnection, // Returns a new vector containing the contents of |address_list| sorted // with address families interleaved, as per Section 4 of RFC 8305, Happy - // Eyeballs v2. It is assumed that The list must already sorted as per + // Eyeballs v2. It is assumed that the list must already sorted as per // Section 6 of RFC6724, which happens in ares_getaddrinfo(). static std::vector sortAddresses(const std::vector& address_list); From 05615935c22d9e9de0c88a0ea72e257b39e8abe2 Mon Sep 17 00:00:00 2001 From: Ryan Hamilton Date: Thu, 11 Nov 2021 01:09:23 +0000 Subject: [PATCH 7/9] Rewrite sortAddresses to be O(N) Signed-off-by: Ryan Hamilton --- .../network/happy_eyeballs_connection_impl.cc | 47 +++++++++++++------ .../network/happy_eyeballs_connection_impl.h | 3 +- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/source/common/network/happy_eyeballs_connection_impl.cc b/source/common/network/happy_eyeballs_connection_impl.cc index 7889169188e38..f2e4cb0f9d47b 100644 --- a/source/common/network/happy_eyeballs_connection_impl.cc +++ b/source/common/network/happy_eyeballs_connection_impl.cc @@ -380,24 +380,43 @@ void HappyEyeballsConnectionImpl::dumpState(std::ostream& os, int indent_level) } } +namespace { +bool hasMatchingAddressFamily(const Address::InstanceConstSharedPtr& a, + const Address::InstanceConstSharedPtr& b) { + return (a->type() == Address::Type::Ip && b->type() == Address::Type::Ip && + a->ip()->version() == b->ip()->version()); +} + +} // namespace + std::vector HappyEyeballsConnectionImpl::sortAddresses(const std::vector& in) { - std::vector address_list = in; - for (auto current = address_list.begin(); current != address_list.end(); ++current) { - // Find the first address with a different address family than the current address. - auto it = std::find_if(current, address_list.end(), [&](const auto& val) { - return (val->type() != Address::Type::Ip || (*current)->type() != Address::Type::Ip || - (*current)->ip()->version() != val->ip()->version()); - }); - // If there are no more addresses with different families the sorting is finished. - if (it == address_list.end()) { - break; + std::vector address_list; + address_list.reserve(in.size()); + // Iterator which will advance through all addresses matching the first family. + auto first = in.begin(); + // Iterator which will advance through all addresses not matching the first family. + // This initial value is ignored and will be overwritten in the loop below. + auto other = in.begin(); + while (first != in.end() || other != in.end()) { + if (first != in.end()) { + address_list.push_back(*first); + first = std::find_if(first + 1, in.end(), [&](const auto& val) { + return hasMatchingAddressFamily(in[0], val); + }); + } + + if (other != in.end()) { + other = std::find_if(other + 1, in.end(), [&](const auto& val) { + return !hasMatchingAddressFamily(in[0], val); + }); + + if (other != in.end()) { + address_list.push_back(*other); + } } - // Bubble the address up to the current position. - auto start = std::make_reverse_iterator(it + 1); - auto end = std::make_reverse_iterator(current + 1); - std::rotate(start, start + 1, end); } + ASSERT(address_list.size() == in.size()); return address_list; } diff --git a/source/common/network/happy_eyeballs_connection_impl.h b/source/common/network/happy_eyeballs_connection_impl.h index 9894c980b24a8..6ed57877c486e 100644 --- a/source/common/network/happy_eyeballs_connection_impl.h +++ b/source/common/network/happy_eyeballs_connection_impl.h @@ -101,7 +101,8 @@ class HappyEyeballsConnectionImpl : public ClientConnection, // Returns a new vector containing the contents of |address_list| sorted // with address families interleaved, as per Section 4 of RFC 8305, Happy // Eyeballs v2. It is assumed that the list must already sorted as per - // Section 6 of RFC6724, which happens in ares_getaddrinfo(). + // Section 6 of RFC6724, which happens in the DNS implementations (ares_getaddrinfo() + // and Apple DNS). static std::vector sortAddresses(const std::vector& address_list); From 9c84f2bd856064ce654052c6a3d2f9bc41037767 Mon Sep 17 00:00:00 2001 From: Ryan Hamilton Date: Thu, 11 Nov 2021 02:02:41 +0000 Subject: [PATCH 8/9] format Signed-off-by: Ryan Hamilton --- .../common/network/happy_eyeballs_connection_impl.cc | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/source/common/network/happy_eyeballs_connection_impl.cc b/source/common/network/happy_eyeballs_connection_impl.cc index f2e4cb0f9d47b..3d6ebf229971f 100644 --- a/source/common/network/happy_eyeballs_connection_impl.cc +++ b/source/common/network/happy_eyeballs_connection_impl.cc @@ -387,7 +387,7 @@ bool hasMatchingAddressFamily(const Address::InstanceConstSharedPtr& a, a->ip()->version() == b->ip()->version()); } -} // namespace +} // namespace std::vector HappyEyeballsConnectionImpl::sortAddresses(const std::vector& in) { @@ -401,15 +401,13 @@ HappyEyeballsConnectionImpl::sortAddresses(const std::vector Date: Tue, 16 Nov 2021 01:49:54 +0000 Subject: [PATCH 9/9] typo Signed-off-by: Ryan Hamilton --- source/common/network/happy_eyeballs_connection_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/network/happy_eyeballs_connection_impl.h b/source/common/network/happy_eyeballs_connection_impl.h index 6ed57877c486e..d2235dc581708 100644 --- a/source/common/network/happy_eyeballs_connection_impl.h +++ b/source/common/network/happy_eyeballs_connection_impl.h @@ -100,7 +100,7 @@ class HappyEyeballsConnectionImpl : public ClientConnection, // Returns a new vector containing the contents of |address_list| sorted // with address families interleaved, as per Section 4 of RFC 8305, Happy - // Eyeballs v2. It is assumed that the list must already sorted as per + // Eyeballs v2. It is assumed that the list must already be sorted as per // Section 6 of RFC6724, which happens in the DNS implementations (ares_getaddrinfo() // and Apple DNS). static std::vector