Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion source/common/network/happy_eyeballs_connection_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

food for thought: do we care about making this behavior configurable? i.e., opt-out via config?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. I'm inclined to say "no", or at least "not unless someone asks for it". Happy Eyeballs itself is already configurable and I can't really imagine why this would be something we'd want to disable if Happy Eyeballs were in use. That being said, if someone ask for it then we could definitely make it configurable. WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair. sg!

connection_construction_state_(
{source_address, socket_factory, transport_socket_options, options}),
next_attempt_timer_(dispatcher_.createTimer([this]() -> void { tryAnotherConnection(); })) {
Expand Down Expand Up @@ -379,6 +380,44 @@ 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<Address::InstanceConstSharedPtr>
HappyEyeballsConnectionImpl::sortAddresses(const std::vector<Address::InstanceConstSharedPtr>& in) {
std::vector<Address::InstanceConstSharedPtr> 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);
}
}
}
ASSERT(address_list.size() == in.size());
return address_list;
}

ClientConnectionPtr HappyEyeballsConnectionImpl::createNextConnection() {
ASSERT(next_address_ < address_list_.size());
auto connection = dispatcher_.createClientConnection(
Expand Down
10 changes: 9 additions & 1 deletion source/common/network/happy_eyeballs_connection_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@ class HappyEyeballsConnectionImpl : public ClientConnection,
void hashKey(std::vector<uint8_t>& 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 be sorted as per
// Section 6 of RFC6724, which happens in the DNS implementations (ares_getaddrinfo()
// and Apple DNS).
static std::vector<Address::InstanceConstSharedPtr>
sortAddresses(const std::vector<Address::InstanceConstSharedPtr>& address_list);

private:
// ConnectionCallbacks which will be set on an ClientConnection which
// sends connection events back to the HappyEyeballsConnectionImpl.
Expand Down Expand Up @@ -196,7 +204,7 @@ class HappyEyeballsConnectionImpl : public ClientConnection,
Event::Dispatcher& dispatcher_;

// List of addresses to attempt to connect to.
const std::vector<Address::InstanceConstSharedPtr>& address_list_;
const std::vector<Address::InstanceConstSharedPtr> address_list_;
// Index of the next address to use.
size_t next_address_ = 0;

Expand Down
47 changes: 42 additions & 5 deletions test/common/network/happy_eyeballs_connection_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,19 @@ class HappyEyeballsConnectionImplTest : public testing::Test {
: failover_timer_(new testing::StrictMock<Event::MockTimer>(&dispatcher_)),
transport_socket_options_(std::make_shared<TransportSocketOptionsImpl>()),
options_(std::make_shared<ConnectionSocket::Options>()),
address_list_({std::make_shared<Address::Ipv4Instance>("127.0.0.1"),
std::make_shared<Address::Ipv4Instance>("127.0.0.2"),
std::make_shared<Address::Ipv4Instance>("127.0.0.3")}) {
raw_address_list_({std::make_shared<Address::Ipv4Instance>("127.0.0.1"),
std::make_shared<Address::Ipv4Instance>("127.0.0.2"),
std::make_shared<Address::Ipv6Instance>("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(
this, &HappyEyeballsConnectionImplTest::createNextConnection));

next_connections_.push_back(std::make_unique<StrictMock<MockClientConnection>>());
impl_ = std::make_unique<HappyEyeballsConnectionImpl>(
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
Expand Down Expand Up @@ -94,6 +95,7 @@ class HappyEyeballsConnectionImplTest : public testing::Test {
MockTransportSocketFactory transport_socket_factory_;
TransportSocketOptionsConstSharedPtr transport_socket_options_;
const ConnectionSocket::OptionsSharedPtr options_;
const std::vector<Address::InstanceConstSharedPtr> raw_address_list_;
const std::vector<Address::InstanceConstSharedPtr> address_list_;
std::vector<StrictMock<MockClientConnection>*> created_connections_;
std::vector<ConnectionCallbacks*> connection_callbacks_;
Expand Down Expand Up @@ -1049,5 +1051,40 @@ TEST_F(HappyEyeballsConnectionImplTest, LastRoundTripTime) {
EXPECT_EQ(rtt, impl_->lastRoundTripTime());
}

TEST_F(HappyEyeballsConnectionImplTest, SortAddresses) {
auto ip_v4_1 = std::make_shared<Address::Ipv4Instance>("127.0.0.1");
auto ip_v4_2 = std::make_shared<Address::Ipv4Instance>("127.0.0.2");
auto ip_v4_3 = std::make_shared<Address::Ipv4Instance>("127.0.0.3");
auto ip_v4_4 = std::make_shared<Address::Ipv4Instance>("127.0.0.4");

auto ip_v6_1 = std::make_shared<Address::Ipv6Instance>("ff02::1", 0);
auto ip_v6_2 = std::make_shared<Address::Ipv6Instance>("ff02::2", 0);
auto ip_v6_3 = std::make_shared<Address::Ipv6Instance>("ff02::3", 0);
auto ip_v6_4 = std::make_shared<Address::Ipv6Instance>("ff02::4", 0);

// All v4 address so unchanged.
std::vector<Address::InstanceConstSharedPtr> 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<Address::InstanceConstSharedPtr> v6_list = {ip_v6_1, ip_v6_2, ip_v6_3, ip_v6_4};
EXPECT_EQ(v6_list, HappyEyeballsConnectionImpl::sortAddresses(v6_list));

std::vector<Address::InstanceConstSharedPtr> v6_then_v4 = {ip_v6_1, ip_v6_2, ip_v4_1, ip_v4_2};
std::vector<Address::InstanceConstSharedPtr> interleaved = {ip_v6_1, ip_v4_1, ip_v6_2, ip_v4_2};
EXPECT_EQ(interleaved, HappyEyeballsConnectionImpl::sortAddresses(v6_then_v4));

std::vector<Address::InstanceConstSharedPtr> v6_then_single_v4 = {ip_v6_1, ip_v6_2, ip_v6_3,
ip_v4_1};
std::vector<Address::InstanceConstSharedPtr> interleaved2 = {ip_v6_1, ip_v4_1, ip_v6_2, ip_v6_3};
EXPECT_EQ(interleaved2, HappyEyeballsConnectionImpl::sortAddresses(v6_then_single_v4));

std::vector<Address::InstanceConstSharedPtr> 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<Address::InstanceConstSharedPtr> 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));
}

} // namespace Network
} // namespace Envoy