Skip to content

Commit

Permalink
Use os_sys_calls in all code paths to replace raw syscalls (#10107)
Browse files Browse the repository at this point in the history 
This PR seeks to address all remaining changes to the linux
and os/x code paths, in order to consolidate the necessary
patch review requirements.

The subsequent PR(s) should not include changes impacting the
current working linux and os/x code paths.

Primarilly adds usage of os_sys_calls for various network and
test sources.

Addresses a reoccuring issue that only linux/posix allows for
connection -to- an any-address, on Windows and in general,
this makes no sense. So several tests are adjusted to explicitly
connect to the loopback interface of the "any" if listener.

Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io>
Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io>
Co-authored-by: William A Rowe Jr <wrowe@pivotal.io>
  • Loading branch information
@sunjayBhatia @wrowe
3 people authored Feb 28, 2020
1 parent 64384d0 commit d173319
Show file tree
Hide file tree
Showing 21 changed files with 504 additions and 299 deletions.
32 changes: 32 additions & 0 deletions include/envoy/common/platform.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,20 @@
#include <io.h>
#include <stdint.h>

#define htole16(x) (x)
#define htole32(x) (x)
#define htole64(x) (x)
#define le16toh(x) (x)
#define le32toh(x) (x)
#define le64toh(x) (x)

#define htobe16(x) htons((x))
#define htobe32(x) htonl((x))
#define htobe64(x) htonll((x))
#define be16toh(x) ntohs((x))
#define be32toh(x) ntohl((x))
#define be64toh(x) ntohll((x))

#define PACKED_STRUCT(definition, ...) \
__pragma(pack(push, 1)) definition, ##__VA_ARGS__; \
__pragma(pack(pop))
Expand Down Expand Up @@ -122,6 +136,24 @@ struct msghdr {
#include <sys/wait.h>
#include <unistd.h>

#ifdef __APPLE__
#include <libkern/OSByteOrder.h>
#define htole16(x) OSSwapHostToLittleInt16((x))
#define htole32(x) OSSwapHostToLittleInt32((x))
#define htole64(x) OSSwapHostToLittleInt64((x))
#define le16toh(x) OSSwapLittleToHostInt16((x))
#define le32toh(x) OSSwapLittleToHostInt32((x))
#define le64toh(x) OSSwapLittleToHostInt64((x))
#define htobe16(x) OSSwapHostToBigInt16((x))
#define htobe32(x) OSSwapHostToBigInt32((x))
#define htobe64(x) OSSwapHostToBigInt64((x))
#define be16toh(x) OSSwapBigToHostInt16((x))
#define be32toh(x) OSSwapBigToHostInt32((x))
#define be64toh(x) OSSwapBigToHostInt64((x))
#else
#include <endian.h>
#endif

#if defined(__linux__)
#include <linux/netfilter_ipv4.h>
#endif
Expand Down
40 changes: 1 addition & 39 deletions source/common/common/byte_order.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,47 +3,9 @@
#include <cstddef>
#include <cstdint>

// NOLINT(namespace-envoy)

#ifdef __APPLE__

#include <libkern/OSByteOrder.h>

#define htole16(x) OSSwapHostToLittleInt16((x))
#define htole32(x) OSSwapHostToLittleInt32((x))
#define htole64(x) OSSwapHostToLittleInt64((x))
#define le16toh(x) OSSwapLittleToHostInt16((x))
#define le32toh(x) OSSwapLittleToHostInt32((x))
#define le64toh(x) OSSwapLittleToHostInt64((x))

#define htobe16(x) OSSwapHostToBigInt16((x))
#define htobe32(x) OSSwapHostToBigInt32((x))
#define htobe64(x) OSSwapHostToBigInt64((x))
#define be16toh(x) OSSwapBigToHostInt16((x))
#define be32toh(x) OSSwapBigToHostInt32((x))
#define be64toh(x) OSSwapBigToHostInt64((x))

#elif WIN32

#include "envoy/common/platform.h"

#define htole16(x) (x)
#define htole32(x) (x)
#define htole64(x) (x)
#define le16toh(x) (x)
#define le32toh(x) (x)
#define le64toh(x) (x)

#define htobe16(x) htons((x))
#define htobe32(x) htonl((x))
#define htobe64(x) htonll((x))
#define be16toh(x) ntohs((x))
#define be32toh(x) ntohl((x))
#define be64toh(x) ntohll((x))

#else
#include <endian.h>
#endif
// NOLINT(namespace-envoy)

enum class ByteOrder { Host, LittleEndian, BigEndian };

Expand Down
72 changes: 48 additions & 24 deletions source/common/network/address_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ Address::InstanceConstSharedPtr addressFromSockAddr(const sockaddr_storage& ss,
#if defined(__APPLE__)
struct sockaddr_in sin = {
{}, AF_INET, sin6->sin6_port, {sin6->sin6_addr.__u6_addr.__u6_addr32[3]}, {}};
#elif defined(WIN32)
struct in_addr in_v4 = {};
in_v4.S_un.S_addr = reinterpret_cast<const uint32_t*>(sin6->sin6_addr.u.Byte)[3];
struct sockaddr_in sin = {AF_INET, sin6->sin6_port, in_v4, {}};
#else
struct sockaddr_in sin = {AF_INET, sin6->sin6_port, {sin6->sin6_addr.s6_addr32[3]}, {}};
#endif
Expand All @@ -102,38 +106,57 @@ Address::InstanceConstSharedPtr addressFromSockAddr(const sockaddr_storage& ss,
InstanceConstSharedPtr addressFromFd(os_fd_t fd) {
sockaddr_storage ss;
socklen_t ss_len = sizeof ss;
int rc = ::getsockname(fd, reinterpret_cast<sockaddr*>(&ss), &ss_len);
if (rc != 0) {
throw EnvoyException(
fmt::format("getsockname failed for '{}': ({}) {}", fd, errno, strerror(errno)));
auto& os_sys_calls = Api::OsSysCallsSingleton::get();
Api::SysCallIntResult result =
os_sys_calls.getsockname(fd, reinterpret_cast<sockaddr*>(&ss), &ss_len);
if (result.rc_ != 0) {
throw EnvoyException(fmt::format("getsockname failed for '{}': ({}) {}", fd, result.errno_,
strerror(result.errno_)));
}
int socket_v6only = 0;
if (ss.ss_family == AF_INET6) {
socklen_t size_int = sizeof(socket_v6only);
RELEASE_ASSERT(::getsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &socket_v6only, &size_int) == 0, "");
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_,
strerror(result.errno_)));
}
#else
RELEASE_ASSERT(result.rc_ == 0, "");
#endif
}
return addressFromSockAddr(ss, ss_len, rc == 0 && socket_v6only);
return addressFromSockAddr(ss, ss_len, socket_v6only);
}

InstanceConstSharedPtr peerAddressFromFd(os_fd_t fd) {
sockaddr_storage ss;
socklen_t ss_len = sizeof ss;
const int rc = ::getpeername(fd, reinterpret_cast<sockaddr*>(&ss), &ss_len);
if (rc != 0) {
throw EnvoyException(fmt::format("getpeername failed for '{}': {}", fd, strerror(errno)));
auto& os_sys_calls = Api::OsSysCallsSingleton::get();
Api::SysCallIntResult result =
os_sys_calls.getpeername(fd, reinterpret_cast<sockaddr*>(&ss), &ss_len);
if (result.rc_ != 0) {
throw EnvoyException(
fmt::format("getpeername failed for '{}': {}", fd, strerror(result.errno_)));
}
#ifdef __APPLE__
if (ss_len == sizeof(sockaddr) && ss.ss_family == AF_UNIX) {
if (ss_len == sizeof(sockaddr) && ss.ss_family == AF_UNIX)
#else
if (ss_len == sizeof(sa_family_t) && ss.ss_family == AF_UNIX) {
if (ss_len == sizeof(sa_family_t) && ss.ss_family == AF_UNIX)
#endif
{
// For Unix domain sockets, can't find out the peer name, but it should match our own
// name for the socket (i.e. the path should match, barring any namespace or other
// mechanisms to hide things, of which there are many).
ss_len = sizeof ss;
const int rc = ::getsockname(fd, reinterpret_cast<sockaddr*>(&ss), &ss_len);
if (rc != 0) {
throw EnvoyException(fmt::format("getsockname failed for '{}': {}", fd, strerror(errno)));
result = os_sys_calls.getsockname(fd, reinterpret_cast<sockaddr*>(&ss), &ss_len);
if (result.rc_ != 0) {
throw EnvoyException(
fmt::format("getsockname failed for '{}': {}", fd, strerror(result.errno_)));
}
}
return addressFromSockAddr(ss, ss_len);
Expand Down Expand Up @@ -166,14 +189,16 @@ IoHandlePtr InstanceBase::socketFromSocketType(SocketType socket_type) const {
domain = AF_UNIX;
}

const Api::SysCallSocketResult result = Api::OsSysCallsSingleton::get().socket(domain, flags, 0);
auto os_sys_calls = Api::OsSysCallsSingleton::get();
const Api::SysCallSocketResult result = os_sys_calls.socket(domain, flags, 0);
RELEASE_ASSERT(SOCKET_VALID(result.rc_),
fmt::format("socket(2) failed, got error: {}", strerror(result.errno_)));
IoHandlePtr io_handle = std::make_unique<IoSocketHandleImpl>(result.rc_);

#ifdef __APPLE__
// Cannot set SOCK_NONBLOCK as a ::socket flag.
RELEASE_ASSERT(fcntl(io_handle->fd(), F_SETFL, O_NONBLOCK) != -1, "");
const int rc = os_sys_calls.setsocketblocking(io_handle->fd(), false).rc_;
RELEASE_ASSERT(!SOCKET_FAILURE(rc), "");
#endif

return io_handle;
Expand Down Expand Up @@ -230,8 +255,7 @@ Api::SysCallIntResult Ipv4Instance::bind(os_fd_t fd) const {
}

Api::SysCallIntResult Ipv4Instance::connect(os_fd_t fd) const {
const int rc = ::connect(fd, sockAddr(), sockAddrLen());
return {rc, errno};
return Api::OsSysCallsSingleton::get().connect(fd, sockAddr(), sockAddrLen());
}

IoHandlePtr Ipv4Instance::socket(SocketType type) const { return socketFromSocketType(type); }
Expand Down Expand Up @@ -319,16 +343,17 @@ Api::SysCallIntResult Ipv6Instance::bind(os_fd_t fd) const {
}

Api::SysCallIntResult Ipv6Instance::connect(os_fd_t fd) const {
const int rc = ::connect(fd, sockAddr(), sockAddrLen());
return {rc, errno};
return Api::OsSysCallsSingleton::get().connect(fd, sockAddr(), sockAddrLen());
}

IoHandlePtr Ipv6Instance::socket(SocketType type) const {
IoHandlePtr io_handle = socketFromSocketType(type);
// Setting IPV6_V6ONLY restricts the IPv6 socket to IPv6 connections only.
const int v6only = ip_.v6only_;
RELEASE_ASSERT(
::setsockopt(io_handle->fd(), IPPROTO_IPV6, IPV6_V6ONLY, &v6only, sizeof(v6only)) != -1, "");
const Api::SysCallIntResult result = Api::OsSysCallsSingleton::get().setsockopt(
io_handle->fd(), IPPROTO_IPV6, IPV6_V6ONLY, reinterpret_cast<const char*>(&v6only),
sizeof(v6only));
RELEASE_ASSERT(!SOCKET_FAILURE(result.rc_), "");
return io_handle;
}

Expand Down Expand Up @@ -416,8 +441,7 @@ Api::SysCallIntResult PipeInstance::bind(os_fd_t fd) const {
}

Api::SysCallIntResult PipeInstance::connect(os_fd_t fd) const {
const int rc = ::connect(fd, sockAddr(), sockAddrLen());
return {rc, errno};
return Api::OsSysCallsSingleton::get().connect(fd, sockAddr(), sockAddrLen());
}

IoHandlePtr PipeInstance::socket(SocketType type) const { return socketFromSocketType(type); }
Expand Down
48 changes: 35 additions & 13 deletions source/common/network/connection_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "envoy/event/timer.h"
#include "envoy/network/filter.h"

#include "common/api/os_sys_calls_impl.h"
#include "common/common/assert.h"
#include "common/common/empty_string.h"
#include "common/common/enum_to_int.h"
Expand Down Expand Up @@ -54,17 +55,23 @@ ConnectionImpl::ConnectionImpl(Event::Dispatcher& dispatcher, ConnectionSocketPt
write_end_stream_(false), current_write_end_stream_(false), dispatch_buffered_data_(false) {
// Treat the lack of a valid fd (which in practice only happens if we run out of FDs) as an OOM
// condition and just crash.
RELEASE_ASSERT(ioHandle().fd() != -1, "");
RELEASE_ASSERT(SOCKET_VALID(ioHandle().fd()), "");

if (!connected) {
connecting_ = true;
}

// Libevent only supports Level trigger on Windows.
#ifdef WIN32
Event::FileTriggerType trigger = Event::FileTriggerType::Level;
#else
Event::FileTriggerType trigger = Event::FileTriggerType::Edge;
#endif
// We never ask for both early close and read at the same time. If we are reading, we want to
// consume all available data.
file_event_ = dispatcher_.createFileEvent(
ioHandle().fd(), [this](uint32_t events) -> void { onFileEvent(events); },
Event::FileTriggerType::Edge, Event::FileReadyType::Read | Event::FileReadyType::Write);
ioHandle().fd(), [this](uint32_t events) -> void { onFileEvent(events); }, trigger,
Event::FileReadyType::Read | Event::FileReadyType::Write);

transport_socket_->setTransportSocketCallbacks(*this);
}
Expand Down Expand Up @@ -205,6 +212,7 @@ void ConnectionImpl::closeSocket(ConnectionEvent close_type) {
connection_stats_.reset();

file_event_.reset();

socket_->close();

raiseEvent(close_type);
Expand All @@ -223,27 +231,39 @@ void ConnectionImpl::noDelay(bool enable) {
}

// Don't set NODELAY for unix domain sockets
sockaddr addr;
sockaddr_storage addr;
socklen_t len = sizeof(addr);
int rc = getsockname(ioHandle().fd(), &addr, &len);
RELEASE_ASSERT(rc == 0, "");

if (addr.sa_family == AF_UNIX) {
auto os_sys_calls = Api::OsSysCallsSingleton::get();
Api::SysCallIntResult result =
os_sys_calls.getsockname(ioHandle().fd(), reinterpret_cast<struct sockaddr*>(&addr), &len);

RELEASE_ASSERT(result.rc_ == 0, "");

if (addr.ss_family == AF_UNIX) {
return;
}

// Set NODELAY
int new_value = enable;
rc = setsockopt(ioHandle().fd(), IPPROTO_TCP, TCP_NODELAY, &new_value, sizeof(new_value));
#ifdef __APPLE__
if (-1 == rc && errno == EINVAL) {
result = os_sys_calls.setsockopt(ioHandle().fd(), IPPROTO_TCP, TCP_NODELAY, &new_value,
sizeof(new_value));
#if defined(__APPLE__)
if (SOCKET_FAILURE(result.rc_) && result.errno_ == EINVAL) {
// Sometimes occurs when the connection is not yet fully formed. Empirically, TCP_NODELAY is
// enabled despite this result.
return;
}
#elif defined(WIN32)
if (SOCKET_FAILURE(result.rc_) &&
(result.errno_ == WSAEWOULDBLOCK || result.errno_ == WSAEINVAL)) {
// Sometimes occurs when the connection is not yet fully formed. Empirically, TCP_NODELAY is
// enabled despite this result.
return;
}
#endif

RELEASE_ASSERT(0 == rc, "");
RELEASE_ASSERT(result.rc_ == 0, "");
}

void ConnectionImpl::onRead(uint64_t read_buffer_size) {
Expand Down Expand Up @@ -562,8 +582,10 @@ void ConnectionImpl::onWriteReady() {
if (connecting_) {
int error;
socklen_t error_size = sizeof(error);
int rc = getsockopt(ioHandle().fd(), SOL_SOCKET, SO_ERROR, &error, &error_size);
ASSERT(0 == rc);
RELEASE_ASSERT(Api::OsSysCallsSingleton::get()
.getsockopt(ioHandle().fd(), SOL_SOCKET, SO_ERROR, &error, &error_size)
.rc_ == 0,
"");

if (error == 0) {
ENVOY_CONN_LOG(debug, "connected", *this);
Expand Down
Loading

0 comments on commit d173319

Please sign in to comment.