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
69 changes: 63 additions & 6 deletions library/common/network/configurator.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "library/common/network/configurator.h"

#include <net/if.h>

#include "envoy/common/platform.h"

#include "source/common/common/assert.h"
Expand Down Expand Up @@ -96,14 +98,18 @@ void Configurator::refreshDns(envoy_network_t network) {
}

std::vector<std::string> Configurator::enumerateV4Interfaces() {
return enumerateInterfaces(AF_INET);
return enumerateInterfaces(AF_INET, 0, 0);
}

std::vector<std::string> Configurator::enumerateV6Interfaces() {
return enumerateInterfaces(AF_INET6);
return enumerateInterfaces(AF_INET6, 0, 0);
}

Socket::OptionsSharedPtr Configurator::getUpstreamSocketOptions(envoy_network_t network, bool) {
Socket::OptionsSharedPtr Configurator::getUpstreamSocketOptions(envoy_network_t network,
bool override_interface) {
if (override_interface && network != ENVOY_NET_GENERIC) {
return getAlternateInterfaceSocketOptions(network);
}
// Envoy uses the hash signature of overridden socket options to choose a connection pool.
// Setting a dummy socket option is a hack that allows us to select a different
// connection pool without materially changing the socket configuration.
Expand All @@ -116,7 +122,54 @@ Socket::OptionsSharedPtr Configurator::getUpstreamSocketOptions(envoy_network_t
return options;
}

std::vector<std::string> Configurator::enumerateInterfaces([[maybe_unused]] unsigned short family) {
Socket::OptionsSharedPtr Configurator::getAlternateInterfaceSocketOptions(envoy_network_t network) {
auto& v4_interface = getActiveAlternateInterface(network, AF_INET);
auto& v6_interface = getActiveAlternateInterface(network, AF_INET6);

auto options = std::make_shared<Socket::Options>();

// Android
#ifdef SO_BINDTODEVICE
options->push_back(std::make_shared<AddrFamilyAwareSocketOptionImpl>(
envoy::config::core::v3::SocketOption::STATE_PREBIND, ENVOY_SOCKET_SO_BINDTODEVICE,
v4_interface, ENVOY_SOCKET_SO_BINDTODEVICE, v6_interface));
#endif // SO_BINDTODEVICE

// iOS
#ifdef IP_BOUND_IF
int v4_idx = if_nametoindex(v4_interface.c_str());
int v6_idx = if_nametoindex(v6_interface.c_str());
options->push_back(std::make_shared<AddrFamilyAwareSocketOptionImpl>(
envoy::config::core::v3::SocketOption::STATE_PREBIND, ENVOY_SOCKET_IP_BOUND_IF, v4_idx,
ENVOY_SOCKET_IPV6_BOUND_IF, v6_idx));
#endif // IP_BOUND_IF

return options;
}

const std::string Configurator::getActiveAlternateInterface(envoy_network_t network,
unsigned short family) {
// Attempt to derive an active interface that differs from the passed network parameter.
if (network == ENVOY_NET_WWAN) {
// Network is cellular, so look for a WiFi interface.
// WiFi should always support multicast, and will not be point-to-point.
auto interfaces =
enumerateInterfaces(family, IFF_UP | IFF_MULTICAST, IFF_LOOPBACK | IFF_POINTOPOINT);
return interfaces.size() > 0 ? interfaces[0] : "";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

does it matter if we send the first available interface? is there any heuristic for preference?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
return interfaces.size() > 0 ? interfaces[0] : "";
return interfaces.size() > 0 ? interfaces.front() : "";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There's no documentation I'm aware that asserts any meaning with respect to order, and absent that, a slight bias towards whatever matches first seems reasonable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't really have a strong opinion with respect to [0] vs front(), but the former seems perfectly clear to me, and subjectively, better indicates to me that I'm picking an element from a vector. May I ask your basis for preferring front()?

} else if (network == ENVOY_NET_WLAN) {
// Network is WiFi, so look for a cellular interface.
// Cellular networks should be point-to-point.
auto interfaces = enumerateInterfaces(family, IFF_UP | IFF_POINTOPOINT, IFF_LOOPBACK);
return interfaces.size() > 0 ? interfaces[0] : "";
} else {
return "";
}
}

std::vector<std::string>
Configurator::enumerateInterfaces([[maybe_unused]] unsigned short family,
[[maybe_unused]] unsigned int select_flags,
[[maybe_unused]] unsigned int reject_flags) {
std::vector<std::string> names{};

#ifdef SUPPORTS_GETIFADDRS
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

note: I am adding full support for getifaddrs in Envoy here envoyproxy/envoy#18513. So we will be able to get rid of the ifdef.

Non-blocking of course! I can update this code once that lands :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, we can revisit. I actually think this still might be needed here though, but since this is non-blocking, let's discuss after.

Expand All @@ -127,9 +180,13 @@ std::vector<std::string> Configurator::enumerateInterfaces([[maybe_unused]] unsi
RELEASE_ASSERT(!rc, "getifaddrs failed");

for (ifa = interfaces; ifa != nullptr; ifa = ifa->ifa_next) {
if (ifa->ifa_addr && ifa->ifa_addr->sa_family == family) {
names.push_back(std::string{ifa->ifa_name});
if (!ifa->ifa_addr || ifa->ifa_addr->sa_family != family) {
continue;
}
if ((ifa->ifa_flags & (select_flags ^ reject_flags)) != select_flags) {
continue;
}
names.push_back(std::string{ifa->ifa_name});
}

freeifaddrs(interfaces);
Expand Down
12 changes: 11 additions & 1 deletion library/common/network/configurator.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@ class Configurator : public Logger::Loggable<Logger::Id::upstream>, public Singl
*/
std::vector<std::string> enumerateV6Interfaces();

/**
* @param family, network family of the interface.
* @param select_flags, flags which MUST be set for each returned interface.
* @param reject_flags, flags which MUST NOT be set for any returned interface.
* @returns a list of local network interfaces filtered by the providered flags.
*/
std::vector<std::string> enumerateInterfaces(unsigned short family, unsigned int select_flags,
unsigned int reject_flags);

/**
* @returns the current OS default/preferred network class.
*/
Expand Down Expand Up @@ -62,7 +71,8 @@ class Configurator : public Logger::Loggable<Logger::Id::upstream>, public Singl
bool override_interface);

private:
std::vector<std::string> enumerateInterfaces(unsigned short family);
Socket::OptionsSharedPtr getAlternateInterfaceSocketOptions(envoy_network_t network);
const std::string getActiveAlternateInterface(envoy_network_t network, unsigned short family);
DnsCacheManagerSharedPtr dns_cache_manager_;
static std::atomic<envoy_network_t> preferred_network_;
};
Expand Down
18 changes: 18 additions & 0 deletions test/common/network/configurator_test.cc
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#include <net/if.h>

#include "test/extensions/common/dynamic_forward_proxy/mocks.h"

#include "gtest/gtest.h"
Expand Down Expand Up @@ -37,5 +39,21 @@ TEST_F(ConfiguratorTest, RefreshDnsForOtherNetworkDoesntTriggerDnsRefresh) {
configurator_->refreshDns(ENVOY_NET_WWAN);
}

TEST_F(ConfiguratorTest, EnumerateInterfacesFiltersByFlags) {
// Select loopback.
auto loopbacks = configurator_->enumerateInterfaces(AF_INET, IFF_LOOPBACK, 0);
EXPECT_EQ(loopbacks[0].rfind("lo", 0), 0);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
EXPECT_EQ(loopbacks[0].rfind("lo", 0), 0);
EXPECT_TRUE(absl::StrContains(loopbacks.front(), "lo"))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Again, I don't have a strong preference here, but I think the existing check is a little more specific. This suggestion would match an interface named "hello_world", for instance.


// Reject loopback.
auto nonloopbacks = configurator_->enumerateInterfaces(AF_INET, 0, IFF_LOOPBACK);
for (const auto& interface : nonloopbacks) {
EXPECT_NE(interface.rfind("lo", 0), 0);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
EXPECT_NE(interface.rfind("lo", 0), 0);
EXPECT_FALSE(absl::StrContains(interface, "lo"));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Replied above.

}

// Select AND reject loopback.
auto empty = configurator_->enumerateInterfaces(AF_INET, IFF_LOOPBACK, IFF_LOOPBACK);
EXPECT_EQ(empty.size(), 0);
}

} // namespace Network
} // namespace Envoy