Skip to content

Commit

Permalink
Cache v6only option when we create ipv6 socket (envoyproxy#11793)
Browse files Browse the repository at this point in the history
On Windows, getsockopt fails if the socket driver is performing another
non-blocking operation. For example if we query for a specific socket
option while the socket is connecting then the getsockopt is going to
fail with error (10022).

When we create an ipv6 socket we set it to be ipv6 only. Now we cache
this configuration and we query it before we connect to the socket

This fixes IpVersions/TcpClientConnectionImplTest.BadConnectConnRefused/IPv6 test on Windows.

Signed-off-by: Sotiris Nanopoulos <[email protected]>
  • Loading branch information
davinci26 authored Jul 27, 2020
1 parent ce26fe1 commit ff0beb1
Show file tree
Hide file tree
Showing 8 changed files with 19 additions and 34 deletions.
5 changes: 3 additions & 2 deletions include/envoy/network/socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,11 @@ class SocketInterface {
* @param type type of socket requested
* @param addr_type type of address used with the socket
* @param version IP version if address type is IP
* @param socket_v6only if the socket is ipv6 version only
* @return @ref Network::IoHandlePtr that wraps the underlying socket file descriptor
*/
virtual IoHandlePtr socket(Socket::Type type, Address::Type addr_type,
Address::IpVersion version) PURE;
virtual IoHandlePtr socket(Socket::Type type, Address::Type addr_type, Address::IpVersion version,
bool socket_v6only) PURE;

/**
* Low level api to create a socket in the underlying host stack. Does not create an
Expand Down
19 changes: 1 addition & 18 deletions source/common/network/io_socket_handle_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -430,24 +430,7 @@ Address::InstanceConstSharedPtr IoSocketHandleImpl::localAddress() {
throw EnvoyException(fmt::format("getsockname failed for '{}': ({}) {}", fd_, result.errno_,
errorDetails(result.errno_)));
}
int socket_v6only = 0;
if (ss.ss_family == AF_INET6) {
socklen_t size_int = sizeof(socket_v6only);
result = os_sys_calls.getsockopt(fd_, IPPROTO_IPV6, IPV6_V6ONLY, &socket_v6only, &size_int);
#ifdef WIN32
// On Windows, it is possible for this getsockopt() call to fail.
// This can happen if the address we are trying to connect to has nothing
// listening. So we can't use RELEASE_ASSERT and instead must throw an
// exception
if (SOCKET_FAILURE(result.rc_)) {
throw EnvoyException(fmt::format("getsockopt failed for '{}': ({}) {}", fd_, result.errno_,
errorDetails(result.errno_)));
}
#else
RELEASE_ASSERT(result.rc_ == 0, "");
#endif
}
return Address::addressFromSockAddr(ss, ss_len, socket_v6only);
return Address::addressFromSockAddr(ss, ss_len, socket_v6only_);
}

Address::InstanceConstSharedPtr IoSocketHandleImpl::peerAddress() {
Expand Down
4 changes: 3 additions & 1 deletion source/common/network/io_socket_handle_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ namespace Network {
*/
class IoSocketHandleImpl : public IoHandle, protected Logger::Loggable<Logger::Id::io> {
public:
explicit IoSocketHandleImpl(os_fd_t fd = INVALID_SOCKET) : fd_(fd) {}
explicit IoSocketHandleImpl(os_fd_t fd = INVALID_SOCKET, bool socket_v6only = false)
: fd_(fd), socket_v6only_(socket_v6only) {}

// Close underlying socket if close() hasn't been call yet.
~IoSocketHandleImpl() override;
Expand Down Expand Up @@ -77,6 +78,7 @@ class IoSocketHandleImpl : public IoHandle, protected Logger::Loggable<Logger::I
}

os_fd_t fd_;
int socket_v6only_{false};

// The minimum cmsg buffer size to filled in destination address, packets dropped and gso
// size when receiving a packet. It is possible for a received packet to contain both IPv4
Expand Down
4 changes: 0 additions & 4 deletions source/common/network/socket_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@
namespace Envoy {
namespace Network {

SocketImpl::SocketImpl(Socket::Type type, Address::Type addr_type, Address::IpVersion version)
: io_handle_(SocketInterfaceSingleton::get().socket(type, addr_type, version)),
sock_type_(type), addr_type_(addr_type) {}

SocketImpl::SocketImpl(Socket::Type sock_type, const Address::InstanceConstSharedPtr addr)
: io_handle_(SocketInterfaceSingleton::get().socket(sock_type, addr)), sock_type_(sock_type),
addr_type_(addr->type()) {}
Expand Down
1 change: 0 additions & 1 deletion source/common/network/socket_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ namespace Network {

class SocketImpl : public virtual Socket {
public:
SocketImpl(Socket::Type type, Address::Type addr_type, Address::IpVersion version);
SocketImpl(Socket::Type socket_type, const Address::InstanceConstSharedPtr addr);

// Network::Socket
Expand Down
15 changes: 10 additions & 5 deletions source/common/network/socket_interface_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Envoy {
namespace Network {

IoHandlePtr SocketInterfaceImpl::socket(Socket::Type socket_type, Address::Type addr_type,
Address::IpVersion version) {
Address::IpVersion version, bool socket_v6only) {
#if defined(__APPLE__) || defined(WIN32)
int flags = 0;
#else
Expand Down Expand Up @@ -42,7 +42,7 @@ IoHandlePtr SocketInterfaceImpl::socket(Socket::Type socket_type, Address::Type
const Api::SysCallSocketResult result = Api::OsSysCallsSingleton::get().socket(domain, flags, 0);
RELEASE_ASSERT(SOCKET_VALID(result.rc_),
fmt::format("socket(2) failed, got error: {}", errorDetails(result.errno_)));
IoHandlePtr io_handle = std::make_unique<IoSocketHandleImpl>(result.rc_);
IoHandlePtr io_handle = std::make_unique<IoSocketHandleImpl>(result.rc_, socket_v6only);

#if defined(__APPLE__) || defined(WIN32)
// Cannot set SOCK_NONBLOCK as a ::socket flag.
Expand All @@ -56,10 +56,15 @@ IoHandlePtr SocketInterfaceImpl::socket(Socket::Type socket_type, Address::Type
IoHandlePtr SocketInterfaceImpl::socket(Socket::Type socket_type,
const Address::InstanceConstSharedPtr addr) {
Address::IpVersion ip_version = addr->ip() ? addr->ip()->version() : Address::IpVersion::v4;
IoHandlePtr io_handle = SocketInterfaceImpl::socket(socket_type, addr->type(), ip_version);
if (addr->type() == Address::Type::Ip && addr->ip()->version() == Address::IpVersion::v6) {
int v6only = 0;
if (addr->type() == Address::Type::Ip && ip_version == Address::IpVersion::v6) {
v6only = addr->ip()->ipv6()->v6only();
}

IoHandlePtr io_handle =
SocketInterfaceImpl::socket(socket_type, addr->type(), ip_version, v6only);
if (addr->type() == Address::Type::Ip && ip_version == Address::IpVersion::v6) {
// Setting IPV6_V6ONLY restricts the IPv6 socket to IPv6 connections only.
const int v6only = addr->ip()->ipv6()->v6only();
const Api::SysCallIntResult result = Api::OsSysCallsSingleton::get().setsockopt(
io_handle->fd(), IPPROTO_IPV6, IPV6_V6ONLY, reinterpret_cast<const char*>(&v6only),
sizeof(v6only));
Expand Down
4 changes: 2 additions & 2 deletions source/common/network/socket_interface_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ namespace Network {
class SocketInterfaceImpl : public SocketInterfaceBase {
public:
// SocketInterface
IoHandlePtr socket(Socket::Type socket_type, Address::Type addr_type,
Address::IpVersion version) override;
IoHandlePtr socket(Socket::Type socket_type, Address::Type addr_type, Address::IpVersion version,
bool socket_v6only) override;
IoHandlePtr socket(Socket::Type socket_type, const Address::InstanceConstSharedPtr addr) override;
IoHandlePtr socket(os_fd_t fd) override;
bool ipFamilySupported(int domain) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ TEST_F(QuicIoHandleWrapperTest, DelegateIoHandleCalls) {
EXPECT_CALL(os_sys_calls_, getsockname(_, _, _)).WillOnce(Return(Api::SysCallIntResult{0, 0}));
wrapper_->domain();

EXPECT_CALL(os_sys_calls_, getsockopt_(_, _, _, _, _)).WillOnce(Return(0));
EXPECT_CALL(os_sys_calls_, getsockname(_, _, _))
.WillOnce(Invoke([](os_fd_t, sockaddr* addr, socklen_t* addrlen) -> Api::SysCallIntResult {
addr->sa_family = AF_INET6;
Expand Down

0 comments on commit ff0beb1

Please sign in to comment.