Skip to content

Always use IPv6 sockets on Android #21803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattklein123 merged 8 commits into from
Jun 22, 2022
Merged

Always use IPv6 sockets on Android #21803

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e2afd07
tmp: always use v6 on android
mattklein123 Jun 16, 2022
253fd88
Merge remote-tracking branch 'origin/main' into android-v6
mattklein123 Jun 16, 2022
5d32db8
fix
mattklein123 Jun 17, 2022
834822d
Merge remote-tracking branch 'origin/main' into android-v6
mattklein123 Jun 21, 2022
6f2721f
add runtime
mattklein123 Jun 21, 2022
1f1588c
comments
mattklein123 Jun 21, 2022
eff6d25
Merge remote-tracking branch 'origin/main' into android-v6
mattklein123 Jun 21, 2022
10ff948
comment
mattklein123 Jun 21, 2022
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
32 changes: 31 additions & 1 deletion source/common/network/io_socket_handle_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "source/common/event/file_event_impl.h"
#include "source/common/network/address_impl.h"
#include "source/common/network/socket_interface_impl.h"
#include "source/common/runtime/runtime_features.h"

#include "absl/container/fixed_array.h"
#include "absl/types/optional.h"
Expand Down Expand Up @@ -63,6 +64,14 @@ constexpr int messageTruncatedOption() {

namespace Network {

bool IoSocketHandleImpl::alwaysUseV6OnAndroid() {
Comment thread
mattklein123 marked this conversation as resolved.
Outdated
#ifndef __ANDROID_API__
return false;
#else
return Runtime::runtimeFeatureEnabled("envoy.reloadable_features.android_always_use_v6");
#endif
}

IoSocketHandleImpl::~IoSocketHandleImpl() {
if (SOCKET_VALID(fd_)) {
IoSocketHandleImpl::close();
Expand Down Expand Up @@ -466,7 +475,28 @@ IoHandlePtr IoSocketHandleImpl::accept(struct sockaddr* addr, socklen_t* addrlen
}

Api::SysCallIntResult IoSocketHandleImpl::connect(Address::InstanceConstSharedPtr address) {
return Api::OsSysCallsSingleton::get().connect(fd_, address->sockAddr(), address->sockAddrLen());
auto sockaddr_to_use = address->sockAddr();
auto sockaddr_len_to_use = address->sockAddrLen();
#ifdef __ANDROID_API__
sockaddr_in6 sin6;
if (sockaddr_to_use->sa_family == AF_INET && alwaysUseV6OnAndroid()) {
const sockaddr_in& sin4 = reinterpret_cast<const sockaddr_in&>(*sockaddr_to_use);

// Android always uses IPv6 dual stack. Convert IPv4 to the IPv6 mapped address when
// connecting.
memset(&sin6, 0, sizeof(sin6));
sin6.sin6_family = AF_INET6;
sin6.sin6_port = sin4.sin_port;
sin6.sin6_addr.s6_addr32[2] = htonl(0xffff);
sin6.sin6_addr.s6_addr32[3] = sin4.sin_addr.s_addr;
ASSERT(IN6_IS_ADDR_V4MAPPED(&sin6.sin6_addr));

sockaddr_to_use = reinterpret_cast<sockaddr*>(&sin6);
sockaddr_len_to_use = sizeof(sin6);
Comment thread
mattklein123 marked this conversation as resolved.
}
#endif

return Api::OsSysCallsSingleton::get().connect(fd_, sockaddr_to_use, sockaddr_len_to_use);
}

Api::SysCallIntResult IoSocketHandleImpl::setOption(int level, int optname, const void* optval,
Expand Down
6 changes: 6 additions & 0 deletions source/common/network/io_socket_handle_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ namespace Network {
*/
class IoSocketHandleImpl : public IoHandle, protected Logger::Loggable<Logger::Id::io> {
public:
/**
* Check whether we are a) on Android and b) configured via runtime to always use v6 sockets.
* This appears to be what Android OS does for all platform sockets.
*/
static bool alwaysUseV6OnAndroid();

explicit IoSocketHandleImpl(os_fd_t fd = INVALID_SOCKET, bool socket_v6only = false,
absl::optional<int> domain = absl::nullopt)
: fd_(fd), socket_v6only_(socket_v6only), domain_(domain) {}
Expand Down
5 changes: 3 additions & 2 deletions source/common/network/socket_interface_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ IoHandlePtr SocketInterfaceImpl::socket(Socket::Type socket_type, Address::Type

int domain;
if (addr_type == Address::Type::Ip) {
if (version == Address::IpVersion::v6) {
if (version == Address::IpVersion::v6 || IoSocketHandleImpl::alwaysUseV6OnAndroid()) {
domain = AF_INET6;
} else {
ASSERT(version == Address::IpVersion::v4);
Expand Down Expand Up @@ -91,7 +91,8 @@ IoHandlePtr SocketInterfaceImpl::socket(Socket::Type socket_type,

IoHandlePtr io_handle =
SocketInterfaceImpl::socket(socket_type, addr->type(), ip_version, v6only, options);
if (addr->type() == Address::Type::Ip && ip_version == Address::IpVersion::v6) {
if (addr->type() == Address::Type::Ip && ip_version == Address::IpVersion::v6 &&
!IoSocketHandleImpl::alwaysUseV6OnAndroid()) {
// Setting IPV6_V6ONLY restricts the IPv6 socket to IPv6 connections only.
const Api::SysCallIntResult result = io_handle->setOption(
IPPROTO_IPV6, IPV6_V6ONLY, reinterpret_cast<const char*>(&v6only), sizeof(v6only));
Expand Down
2 changes: 2 additions & 0 deletions source/common/runtime/runtime_features.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ FALSE_RUNTIME_GUARD(envoy_reloadable_features_thrift_connection_draining);
FALSE_RUNTIME_GUARD(envoy_reloadable_features_http2_use_oghttp2);
// Used to track if runtime is initialized.
FALSE_RUNTIME_GUARD(envoy_reloadable_features_runtime_initialized);
// TODO(mattklein123): Flip this to true and/or remove completely once verified by Envoy Mobile.
FALSE_RUNTIME_GUARD(envoy_reloadable_features_android_always_use_v6);

@alyssawilk alyssawilk Jun 21, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also please unit test, or add unit tests to the TODO here if the timing is too tight.

@mattklein123 mattklein123 Jun 21, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Do you have a suggestion of how to unit test this in this repo? This is all behind an android only ifdef.

@davinci26 davinci26 Jun 21, 2022

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.

random drive by comment (ignore me if this doesnt make sense) can you make this:

#if defined(__ANDROID_API__) || defined(ENVOY_FORCE_ANDROID)

and add a bazel defines to test it?

if you want to resuse this you can wrap it with a constexpr bool function and reuse it in the codebase.

@mattklein123 mattklein123 Jun 21, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think I would prefer to not have a build flag for it since that will be harder to test. I think Alyssa's suggestion is good to just refactor it and add a unit test. I can do that but will do it as a follow up.


// Block of non-boolean flags. These are deprecated. Do not add more.
ABSL_FLAG(uint64_t, envoy_headermap_lazy_map_min_size, 3, ""); // NOLINT
Expand Down