Skip to content

Commit

Permalink
Add TCP_FASTOPEN listener option
Browse files Browse the repository at this point in the history
Signed-off-by: Bjoern Metzdorf <bmetzdorf@apple.com>
  • Loading branch information
bmetzdorf committed Mar 13, 2018
1 parent 96c645c commit 87bb932
Show file tree
Hide file tree
Showing 23 changed files with 208 additions and 127 deletions.
2 changes: 1 addition & 1 deletion bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ REPOSITORY_LOCATIONS = dict(
urls = ["https://github.com/google/protobuf/archive/v3.5.0.tar.gz"],
),
envoy_api = dict(
commit = "15dc537b6078988ac6f7de5ffec697e876a4652f",
commit = "b6925003b383f6f1213f5c62f96cd2cfd0c072be",
remote = "https://github.com/envoyproxy/data-plane-api",
),
grpc_httpjson_transcoding = dict(
Expand Down
1 change: 1 addition & 0 deletions include/envoy/event/dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ class Dispatcher {
*/
virtual Network::ListenerPtr createListener(Network::Socket& socket,
Network::ListenerCallbacks& cb, bool bind_to_port,
bool enable_tcp_fast_open,
bool hand_off_restored_destination_connections) PURE;

/**
Expand Down
5 changes: 5 additions & 0 deletions include/envoy/network/listener.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ class ListenerConfig {
*/
virtual bool bindToPort() PURE;

/**
* @return bool specifies whether the listener should support TCP Fast Open.
*/
virtual bool enableTcpFastOpen() PURE;

/**
* @return bool if a connection should be handed off to another Listener after the original
* destination address has been restored. 'true' when 'use_original_dst' flag in listener
Expand Down
4 changes: 3 additions & 1 deletion source/common/event/dispatcher_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,11 @@ Filesystem::WatcherPtr DispatcherImpl::createFilesystemWatcher() {

Network::ListenerPtr
DispatcherImpl::createListener(Network::Socket& socket, Network::ListenerCallbacks& cb,
bool bind_to_port, bool hand_off_restored_destination_connections) {
bool bind_to_port, bool enable_tcp_fast_open,
bool hand_off_restored_destination_connections) {
ASSERT(isThreadSafe());
return Network::ListenerPtr{new Network::ListenerImpl(*this, socket, cb, bind_to_port,
enable_tcp_fast_open,
hand_off_restored_destination_connections)};
}

Expand Down
2 changes: 1 addition & 1 deletion source/common/event/dispatcher_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class DispatcherImpl : Logger::Loggable<Logger::Id::main>, public Dispatcher {
uint32_t events) override;
Filesystem::WatcherPtr createFilesystemWatcher() override;
Network::ListenerPtr createListener(Network::Socket& socket, Network::ListenerCallbacks& cb,
bool bind_to_port,
bool bind_to_port, bool enable_tcp_fast_open,
bool hand_off_restored_destination_connections) override;
TimerPtr createTimer(TimerCb cb) override;
void deferredDelete(DeferredDeletablePtr&& to_delete) override;
Expand Down
56 changes: 55 additions & 1 deletion source/common/network/listener_impl.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "common/network/listener_impl.h"

#include <netinet/tcp.h>
#include <sys/un.h>

#include "envoy/common/exception.h"
Expand All @@ -13,6 +14,16 @@

#include "event2/listener.h"

// On macOS the socket MUST be listening already for TCP_FASTOPEN to be set and backlog MUST be 1
// (the actual value is set via the net.inet.tcp.fastopen_backlog kernel parameter.
// For Linux we default to 128, which libevent is using in
// https://github.com/libevent/libevent/blob/release-2.1.8-stable/listener.c#L176
#if defined(__APPLE__)
#define TFO_BACKLOG 1
#else
#define TFO_BACKLOG 128
#endif

namespace Envoy {
namespace Network {

Expand Down Expand Up @@ -45,7 +56,8 @@ void ListenerImpl::listenCallback(evconnlistener*, evutil_socket_t fd, sockaddr*
}

ListenerImpl::ListenerImpl(Event::DispatcherImpl& dispatcher, Socket& socket, ListenerCallbacks& cb,
bool bind_to_port, bool hand_off_restored_destination_connections)
bool bind_to_port, bool enable_tcp_fast_open,
bool hand_off_restored_destination_connections)
: local_address_(nullptr), cb_(cb),
hand_off_restored_destination_connections_(hand_off_restored_destination_connections),
listener_(nullptr) {
Expand All @@ -66,6 +78,48 @@ ListenerImpl::ListenerImpl(Event::DispatcherImpl& dispatcher, Socket& socket, Li
fmt::format("cannot listen on socket: {}", socket.localAddress()->asString()));
}

// TODO: cleanup redundant setsockopt logic
if (enable_tcp_fast_open) {
int backlog = TFO_BACKLOG;
int fd = socket.fd();
int rc = setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &backlog, sizeof(backlog));
if (rc == -1) {
if (errno == ENOPROTOOPT) {
throw EnvoyException(fmt::format(
"TCP Fast Open is unsupported on listening socket fd {} : {}", fd, strerror(errno)));
} else {
throw EnvoyException(
fmt::format("Failed to enable TCP Fast Open on listening socket fd {} : {}", fd,
strerror(errno)));
}
} else {
ENVOY_LOG_MISC(debug, "Enabled TFO on listening socket fd {}.", fd);
}
} else {
int curbacklog;
int fd = socket.fd();
socklen_t optlen = sizeof(curbacklog);
int rc = getsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &curbacklog, &optlen);
// curbacklog == 0 means TFO is supported and already disabled
if (rc != -1 && curbacklog != 0) {
int backlog = 0;
int rc = setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &backlog, sizeof(backlog));
if (rc == -1) {
if (errno == ENOPROTOOPT) {
throw EnvoyException(
fmt::format("TCP Fast Open is unsupported on listening socket fd {} : {}", fd,
strerror(errno)));
} else {
throw EnvoyException(
fmt::format("Failed to disable TCP Fast Open on listening socket fd {} : {}", fd,
strerror(errno)));
}
} else {
ENVOY_LOG_MISC(debug, "Disabled TFO on listening socket fd {}.", fd);
}
}
}

evconnlistener_set_error_cb(listener_.get(), errorCallback);
}
}
Expand Down
3 changes: 2 additions & 1 deletion source/common/network/listener_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ namespace Network {
class ListenerImpl : public Listener {
public:
ListenerImpl(Event::DispatcherImpl& dispatcher, Socket& socket, ListenerCallbacks& cb,
bool bind_to_port, bool hand_off_restored_destination_connections);
bool bind_to_port, bool enable_tcp_fast_open,
bool hand_off_restored_destination_connections);

protected:
virtual Address::InstanceConstSharedPtr getLocalAddress(int fd);
Expand Down
3 changes: 2 additions & 1 deletion source/server/config_validation/dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ Network::DnsResolverSharedPtr ValidationDispatcher::createDnsResolver(
}

Network::ListenerPtr ValidationDispatcher::createListener(Network::Socket&,
Network::ListenerCallbacks&, bool, bool) {
Network::ListenerCallbacks&, bool, bool,
bool) {
NOT_IMPLEMENTED;
}

Expand Down
2 changes: 1 addition & 1 deletion source/server/config_validation/dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class ValidationDispatcher : public DispatcherImpl {
Network::DnsResolverSharedPtr createDnsResolver(
const std::vector<Network::Address::InstanceConstSharedPtr>& resolvers) override;
Network::ListenerPtr createListener(Network::Socket&, Network::ListenerCallbacks&,
bool bind_to_port,
bool bind_to_port, bool enable_tcp_fast_open,
bool hand_off_restored_destination_connections) override;
};

Expand Down
10 changes: 5 additions & 5 deletions source/server/connection_handler_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ void ConnectionHandlerImpl::ActiveListener::removeConnection(ActiveConnection& c

ConnectionHandlerImpl::ActiveListener::ActiveListener(ConnectionHandlerImpl& parent,
Network::ListenerConfig& config)
: ActiveListener(
parent,
parent.dispatcher_.createListener(config.socket(), *this, config.bindToPort(),
config.handOffRestoredDestinationConnections()),
config) {}
: ActiveListener(parent,
parent.dispatcher_.createListener(
config.socket(), *this, config.bindToPort(), config.enableTcpFastOpen(),
config.handOffRestoredDestinationConnections()),
config) {}

ConnectionHandlerImpl::ActiveListener::ActiveListener(ConnectionHandlerImpl& parent,
Network::ListenerPtr&& listener,
Expand Down
1 change: 1 addition & 0 deletions source/server/http/admin.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ class AdminImpl : public Admin,
return parent_.transport_socket_factory_;
}
bool bindToPort() override { return true; }
bool enableTcpFastOpen() override { return false; }
bool handOffRestoredDestinationConnections() const override { return false; }
uint32_t perConnectionBufferLimitBytes() override { return 0; }
Stats::Scope& listenerScope() override { return *scope_; }
Expand Down
1 change: 1 addition & 0 deletions source/server/listener_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, ListenerManag
listener_scope_(
parent_.server_.stats().createScope(fmt::format("listener.{}.", address_->asString()))),
bind_to_port_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config.deprecated_v1(), bind_to_port, true)),
enable_tcp_fast_open_(config.enable_tcp_fast_open()),
hand_off_restored_destination_connections_(
PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, use_original_dst, false)),
per_connection_buffer_limit_bytes_(
Expand Down
3 changes: 2 additions & 1 deletion source/server/listener_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ class ProdListenerComponentFactory : public ListenerComponentFactory,
Configuration::ListenerFactoryContext& context) override {
return createListenerFilterFactoryList_(filters, context);
}

Network::SocketSharedPtr createListenSocket(Network::Address::InstanceConstSharedPtr address,
bool bind_to_port) override;
DrainManagerPtr createDrainManager(envoy::api::v2::Listener::DrainType drain_type) override;
Expand Down Expand Up @@ -213,6 +212,7 @@ class ListenerImpl : public Network::ListenerConfig,
Network::FilterChainFactory& filterChainFactory() override { return *this; }
Network::Socket& socket() override { return *socket_; }
bool bindToPort() override { return bind_to_port_; }
bool enableTcpFastOpen() override { return enable_tcp_fast_open_; }
bool handOffRestoredDestinationConnections() const override {
return hand_off_restored_destination_connections_;
}
Expand Down Expand Up @@ -270,6 +270,7 @@ class ListenerImpl : public Network::ListenerConfig,
std::vector<Ssl::ServerContextPtr> tls_contexts_;
std::vector<Network::TransportSocketFactoryPtr> transport_socket_factories_;
const bool bind_to_port_;
const bool enable_tcp_fast_open_;
const bool hand_off_restored_destination_connections_;
const uint32_t per_connection_buffer_limit_bytes_;
const uint64_t listener_tag_;
Expand Down
3 changes: 2 additions & 1 deletion test/common/http/codec_client_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ class CodecNetworkTest : public testing::TestWithParam<Network::Address::IpVersi
public:
CodecNetworkTest() {
dispatcher_.reset(new Event::DispatcherImpl);
upstream_listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false);
upstream_listener_ =
dispatcher_->createListener(socket_, listener_callbacks_, true, false, false);
Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection(
socket_.localAddress(), source_address_, Network::Test::createRawBufferSocket(), nullptr);
client_connection_ = client_connection.get();
Expand Down
6 changes: 3 additions & 3 deletions test/common/network/connection_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class ConnectionImplTest : public testing::TestWithParam<Address::IpVersion> {
if (dispatcher_.get() == nullptr) {
dispatcher_.reset(new Event::DispatcherImpl);
}
listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false);
listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false, false);

client_connection_ = dispatcher_->createClientConnection(
socket_.localAddress(), source_address_, Network::Test::createRawBufferSocket(), nullptr);
Expand Down Expand Up @@ -765,7 +765,7 @@ TEST_P(ConnectionImplTest, BindFailureTest) {
new Network::Address::Ipv6Instance(address_string, 0)};
}
dispatcher_.reset(new Event::DispatcherImpl);
listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false);
listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false, false);

client_connection_ = dispatcher_->createClientConnection(
socket_.localAddress(), source_address_, Network::Test::createRawBufferSocket(), nullptr);
Expand Down Expand Up @@ -1159,7 +1159,7 @@ class ReadBufferLimitTest : public ConnectionImplTest {
void readBufferLimitTest(uint32_t read_buffer_limit, uint32_t expected_chunk_size) {
const uint32_t buffer_size = 256 * 1024;
dispatcher_.reset(new Event::DispatcherImpl);
listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false);
listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false, false);

client_connection_ = dispatcher_->createClientConnection(
socket_.localAddress(), Network::Address::InstanceConstSharedPtr(),
Expand Down
2 changes: 1 addition & 1 deletion test/common/network/dns_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ class DnsImplTest : public testing::TestWithParam<Address::IpVersion> {
server_.reset(new TestDnsServer(dispatcher_));
socket_.reset(
new Network::TcpListenSocket(Network::Test::getCanonicalLoopbackAddress(GetParam()), true));
listener_ = dispatcher_.createListener(*socket_, *server_, true, false);
listener_ = dispatcher_.createListener(*socket_, *server_, true, false, false);

// Point c-ares at the listener with no search domains and TCP-only.
peer_.reset(new DnsResolverImplPeer(dynamic_cast<DnsResolverImpl*>(resolver_.get())));
Expand Down
16 changes: 9 additions & 7 deletions test/common/network/listener_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ static void errorCallbackTest(Address::IpVersion version) {
Network::MockListenerCallbacks listener_callbacks;
Network::MockConnectionHandler connection_handler;
Network::ListenerPtr listener =
dispatcher.createListener(socket, listener_callbacks, true, false);
dispatcher.createListener(socket, listener_callbacks, true, false, false);

Network::ClientConnectionPtr client_connection = dispatcher.createClientConnection(
socket.localAddress(), Network::Address::InstanceConstSharedPtr(),
Expand Down Expand Up @@ -62,8 +62,9 @@ TEST_P(ListenerImplDeathTest, ErrorCallback) {
class TestListenerImpl : public ListenerImpl {
public:
TestListenerImpl(Event::DispatcherImpl& dispatcher, Socket& socket, ListenerCallbacks& cb,
bool bind_to_port, bool hand_off_restored_destination_connections)
: ListenerImpl(dispatcher, socket, cb, bind_to_port,
bool bind_to_port, bool enable_tcp_fast_open,
bool hand_off_restored_destination_connections)
: ListenerImpl(dispatcher, socket, cb, bind_to_port, enable_tcp_fast_open,
hand_off_restored_destination_connections) {}

MOCK_METHOD1(getLocalAddress, Address::InstanceConstSharedPtr(int fd));
Expand All @@ -90,9 +91,10 @@ TEST_P(ListenerImplTest, UseActualDst) {
Network::MockListenerCallbacks listener_callbacks1;
Network::MockConnectionHandler connection_handler;
// Do not redirect since use_original_dst is false.
Network::TestListenerImpl listener(dispatcher, socket, listener_callbacks1, true, true);
Network::TestListenerImpl listener(dispatcher, socket, listener_callbacks1, true, false, true);
Network::MockListenerCallbacks listener_callbacks2;
Network::TestListenerImpl listenerDst(dispatcher, socketDst, listener_callbacks2, false, false);
Network::TestListenerImpl listenerDst(dispatcher, socketDst, listener_callbacks2, false, false,
false);

Network::ClientConnectionPtr client_connection = dispatcher.createClientConnection(
socket.localAddress(), Network::Address::InstanceConstSharedPtr(),
Expand Down Expand Up @@ -126,7 +128,7 @@ TEST_P(ListenerImplTest, WildcardListenerUseActualDst) {
Network::MockListenerCallbacks listener_callbacks;
Network::MockConnectionHandler connection_handler;
// Do not redirect since use_original_dst is false.
Network::TestListenerImpl listener(dispatcher, socket, listener_callbacks, true, true);
Network::TestListenerImpl listener(dispatcher, socket, listener_callbacks, true, false, true);

auto local_dst_address = Network::Utility::getAddressWithPort(
*Network::Test::getCanonicalLoopbackAddress(version_), socket.localAddress()->ip()->port());
Expand Down Expand Up @@ -168,7 +170,7 @@ TEST_P(ListenerImplTest, WildcardListenerIpv4Compat) {
ASSERT_TRUE(socket.localAddress()->ip()->isAnyAddress());

// Do not redirect since use_original_dst is false.
Network::TestListenerImpl listener(dispatcher, socket, listener_callbacks, true, true);
Network::TestListenerImpl listener(dispatcher, socket, listener_callbacks, true, false, true);

auto listener_address = Network::Utility::getAddressWithPort(
*Network::Test::getCanonicalLoopbackAddress(version_), socket.localAddress()->ip()->port());
Expand Down
2 changes: 2 additions & 0 deletions test/common/network/proxy_protocol_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class ProxyProtocolTest : public testing::TestWithParam<Address::IpVersion>,
return transport_socket_factory_;
}
bool bindToPort() override { return true; }
bool enableTcpFastOpen() override { return false; }
bool handOffRestoredDestinationConnections() const override { return false; }
uint32_t perConnectionBufferLimitBytes() override { return 0; }
Stats::Scope& listenerScope() override { return stats_store_; }
Expand Down Expand Up @@ -348,6 +349,7 @@ class WildcardProxyProtocolTest : public testing::TestWithParam<Address::IpVersi
Network::Socket& socket() override { return socket_; }
TransportSocketFactory& transportSocketFactory() override { return transport_socket_factory_; }
bool bindToPort() override { return true; }
bool enableTcpFastOpen() override { return false; }
bool handOffRestoredDestinationConnections() const override { return false; }
uint32_t perConnectionBufferLimitBytes() override { return 0; }
Stats::Scope& listenerScope() override { return stats_store_; }
Expand Down
Loading

0 comments on commit 87bb932

Please sign in to comment.