From cf14c27e798594232b43d32ae4042aeaa79b1fe2 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Tue, 11 Aug 2020 19:16:42 -0700 Subject: [PATCH 1/2] network: add tcp listener backlog config Signed-off-by: Florin Coras --- api/envoy/config/listener/v3/listener.proto | 6 +++- .../config/listener/v4alpha/listener.proto | 6 +++- .../envoy/config/listener/v3/listener.proto | 6 +++- .../config/listener/v4alpha/listener.proto | 6 +++- include/envoy/common/platform.h | 8 +++++ include/envoy/event/dispatcher.h | 5 +-- include/envoy/network/listener.h | 5 +++ source/common/event/dispatcher_impl.cc | 5 +-- source/common/event/dispatcher_impl.h | 3 +- source/common/network/listener_impl.cc | 8 ++--- source/common/network/listener_impl.h | 3 +- source/server/admin/admin.h | 1 + source/server/config_validation/dispatcher.cc | 3 +- source/server/config_validation/dispatcher.h | 2 +- source/server/connection_handler_impl.cc | 2 +- source/server/listener_impl.cc | 4 +++ source/server/listener_impl.h | 2 ++ test/common/network/listener_impl_test.cc | 4 +-- .../proxy_protocol_regression_test.cc | 1 + .../proxy_protocol/proxy_protocol_test.cc | 2 ++ test/integration/fake_upstream.h | 1 + test/mocks/event/mocks.h | 7 ++-- test/mocks/network/mocks.h | 1 + test/server/connection_handler_test.cc | 33 +++++++++++++++---- test/server/listener_manager_impl_test.cc | 17 ++++++++++ 25 files changed, 112 insertions(+), 29 deletions(-) diff --git a/api/envoy/config/listener/v3/listener.proto b/api/envoy/config/listener/v3/listener.proto index 8c5066909caf..88e8ae4ad5b1 100644 --- a/api/envoy/config/listener/v3/listener.proto +++ b/api/envoy/config/listener/v3/listener.proto @@ -36,7 +36,7 @@ message ListenerCollection { udpa.core.v1.CollectionEntry entries = 1; } -// [#next-free-field: 24] +// [#next-free-field: 25] message Listener { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.Listener"; @@ -259,4 +259,8 @@ message Listener { // If not present, treat it as "udp_default_writer". // [#not-implemented-hide:] core.v3.TypedExtensionConfig udp_writer_config = 23; + + // The maximum length a tcp listener's pending connections queue can grow to. If no value is + // provided net.core.somaxconn will be used on Linux and 128 otherwise. + google.protobuf.UInt32Value tcp_backlog_size = 24; } diff --git a/api/envoy/config/listener/v4alpha/listener.proto b/api/envoy/config/listener/v4alpha/listener.proto index c188ecb24490..753f6d733cc0 100644 --- a/api/envoy/config/listener/v4alpha/listener.proto +++ b/api/envoy/config/listener/v4alpha/listener.proto @@ -39,7 +39,7 @@ message ListenerCollection { udpa.core.v1.CollectionEntry entries = 1; } -// [#next-free-field: 24] +// [#next-free-field: 25] message Listener { option (udpa.annotations.versioning).previous_message_type = "envoy.config.listener.v3.Listener"; @@ -262,4 +262,8 @@ message Listener { // If not present, treat it as "udp_default_writer". // [#not-implemented-hide:] core.v4alpha.TypedExtensionConfig udp_writer_config = 23; + + // The maximum length a tcp listener's pending connections queue can grow to. If no value is + // provided net.core.somaxconn will be used on Linux and 128 otherwise. + google.protobuf.UInt32Value tcp_backlog_size = 24; } diff --git a/generated_api_shadow/envoy/config/listener/v3/listener.proto b/generated_api_shadow/envoy/config/listener/v3/listener.proto index 0d0dc5d817a9..d57b12950535 100644 --- a/generated_api_shadow/envoy/config/listener/v3/listener.proto +++ b/generated_api_shadow/envoy/config/listener/v3/listener.proto @@ -36,7 +36,7 @@ message ListenerCollection { udpa.core.v1.CollectionEntry entries = 1; } -// [#next-free-field: 24] +// [#next-free-field: 25] message Listener { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.Listener"; @@ -258,5 +258,9 @@ message Listener { // [#not-implemented-hide:] core.v3.TypedExtensionConfig udp_writer_config = 23; + // The maximum length a tcp listener's pending connections queue can grow to. If no value is + // provided net.core.somaxconn will be used on Linux and 128 otherwise. + google.protobuf.UInt32Value tcp_backlog_size = 24; + google.protobuf.BoolValue hidden_envoy_deprecated_use_original_dst = 4 [deprecated = true]; } diff --git a/generated_api_shadow/envoy/config/listener/v4alpha/listener.proto b/generated_api_shadow/envoy/config/listener/v4alpha/listener.proto index c188ecb24490..753f6d733cc0 100644 --- a/generated_api_shadow/envoy/config/listener/v4alpha/listener.proto +++ b/generated_api_shadow/envoy/config/listener/v4alpha/listener.proto @@ -39,7 +39,7 @@ message ListenerCollection { udpa.core.v1.CollectionEntry entries = 1; } -// [#next-free-field: 24] +// [#next-free-field: 25] message Listener { option (udpa.annotations.versioning).previous_message_type = "envoy.config.listener.v3.Listener"; @@ -262,4 +262,8 @@ message Listener { // If not present, treat it as "udp_default_writer". // [#not-implemented-hide:] core.v4alpha.TypedExtensionConfig udp_writer_config = 23; + + // The maximum length a tcp listener's pending connections queue can grow to. If no value is + // provided net.core.somaxconn will be used on Linux and 128 otherwise. + google.protobuf.UInt32Value tcp_backlog_size = 24; } diff --git a/include/envoy/common/platform.h b/include/envoy/common/platform.h index 71e0795c9a55..7d239287edb9 100644 --- a/include/envoy/common/platform.h +++ b/include/envoy/common/platform.h @@ -273,3 +273,11 @@ struct mmsghdr { #undef SUPPORTS_PTHREAD_NAMING #define SUPPORTS_PTHREAD_NAMING 1 #endif // defined(__ANDROID_API__) + +#if defined(__linux__) +// On Linux, default listen backlog size to net.core.somaxconn which is runtime configurable +#define ENVOY_TCP_BACKLOG_SIZE -1 +#else +// On non-Linux platforms use 128 which is libevent listener default +#define ENVOY_TCP_BACKLOG_SIZE 128 +#endif \ No newline at end of file diff --git a/include/envoy/event/dispatcher.h b/include/envoy/event/dispatcher.h index eca836980102..f6e3526f7f53 100644 --- a/include/envoy/event/dispatcher.h +++ b/include/envoy/event/dispatcher.h @@ -148,11 +148,12 @@ class Dispatcher { * @param socket supplies the socket to listen on. * @param cb supplies the callbacks to invoke for listener events. * @param bind_to_port controls whether the listener binds to a transport port or not. + * @param backlog_size controls listener pending connections backlog * @return Network::ListenerPtr a new listener that is owned by the caller. */ virtual Network::ListenerPtr createListener(Network::SocketSharedPtr&& socket, - Network::ListenerCallbacks& cb, - bool bind_to_port) PURE; + Network::ListenerCallbacks& cb, bool bind_to_port, + uint32_t backlog_size = ENVOY_TCP_BACKLOG_SIZE) PURE; /** * Creates a logical udp listener on a specific port. diff --git a/include/envoy/network/listener.h b/include/envoy/network/listener.h index 3d8257e69c5f..d07e9d01d0b4 100644 --- a/include/envoy/network/listener.h +++ b/include/envoy/network/listener.h @@ -161,6 +161,11 @@ class ListenerConfig { * @return std::vector access logs emitted by the listener. */ virtual const std::vector& accessLogs() const PURE; + + /** + * @return pending connection backlog for TCP listeners. + */ + virtual uint32_t tcpBacklogSize() const PURE; }; /** diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index 76f4a109039f..09827d0ed891 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -138,9 +138,10 @@ Filesystem::WatcherPtr DispatcherImpl::createFilesystemWatcher() { Network::ListenerPtr DispatcherImpl::createListener(Network::SocketSharedPtr&& socket, Network::ListenerCallbacks& cb, - bool bind_to_port) { + bool bind_to_port, uint32_t backlog_size) { ASSERT(isThreadSafe()); - return std::make_unique(*this, std::move(socket), cb, bind_to_port); + return std::make_unique(*this, std::move(socket), cb, bind_to_port, + backlog_size); } Network::UdpListenerPtr DispatcherImpl::createUdpListener(Network::SocketSharedPtr&& socket, diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index 0db663dd985b..f16abc2c420d 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -60,7 +60,8 @@ class DispatcherImpl : Logger::Loggable, uint32_t events) override; Filesystem::WatcherPtr createFilesystemWatcher() override; Network::ListenerPtr createListener(Network::SocketSharedPtr&& socket, - Network::ListenerCallbacks& cb, bool bind_to_port) override; + Network::ListenerCallbacks& cb, bool bind_to_port, + uint32_t backlog_size) override; Network::UdpListenerPtr createUdpListener(Network::SocketSharedPtr&& socket, Network::UdpListenerCallbacks& cb) override; TimerPtr createTimer(TimerCb cb) override; diff --git a/source/common/network/listener_impl.cc b/source/common/network/listener_impl.cc index ebf2f2b7731d..c1722c88239c 100644 --- a/source/common/network/listener_impl.cc +++ b/source/common/network/listener_impl.cc @@ -91,9 +91,7 @@ void ListenerImpl::onSocketEvent(short flags) { } void ListenerImpl::setupServerSocket(Event::DispatcherImpl& dispatcher, Socket& socket) { - // TODO(fcoras): make listen backlog configurable. For now use 128, which is what libevent - // defaults to for listeners configured with a negative (unspecified) backlog - socket.ioHandle().listen(128); + socket.ioHandle().listen(backlog_size_); // Although onSocketEvent drains to completion, use level triggered mode to avoid potential // loss of the trigger due to transient accept errors. @@ -109,8 +107,8 @@ void ListenerImpl::setupServerSocket(Event::DispatcherImpl& dispatcher, Socket& } ListenerImpl::ListenerImpl(Event::DispatcherImpl& dispatcher, SocketSharedPtr socket, - ListenerCallbacks& cb, bool bind_to_port) - : BaseListenerImpl(dispatcher, std::move(socket)), cb_(cb) { + ListenerCallbacks& cb, bool bind_to_port, uint32_t backlog_size) + : BaseListenerImpl(dispatcher, std::move(socket)), cb_(cb), backlog_size_(backlog_size) { if (bind_to_port) { setupServerSocket(dispatcher, *socket_); } diff --git a/source/common/network/listener_impl.h b/source/common/network/listener_impl.h index e26eafebe5d2..9f98102f1b4a 100644 --- a/source/common/network/listener_impl.h +++ b/source/common/network/listener_impl.h @@ -15,7 +15,7 @@ namespace Network { class ListenerImpl : public BaseListenerImpl { public: ListenerImpl(Event::DispatcherImpl& dispatcher, SocketSharedPtr socket, ListenerCallbacks& cb, - bool bind_to_port); + bool bind_to_port, uint32_t backlog_size); void disable() override; void enable() override; @@ -25,6 +25,7 @@ class ListenerImpl : public BaseListenerImpl { void setupServerSocket(Event::DispatcherImpl& dispatcher, Socket& socket); ListenerCallbacks& cb_; + uint32_t backlog_size_; private: void onSocketEvent(short flags); diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index f2446779f952..9d8c587eda42 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -390,6 +390,7 @@ class AdminImpl : public Admin, const std::vector& accessLogs() const override { return empty_access_logs_; } + uint32_t tcpBacklogSize() const override { return ENVOY_TCP_BACKLOG_SIZE; } AdminImpl& parent_; const std::string name_; diff --git a/source/server/config_validation/dispatcher.cc b/source/server/config_validation/dispatcher.cc index 1a75abe41935..91bb1dc66d62 100644 --- a/source/server/config_validation/dispatcher.cc +++ b/source/server/config_validation/dispatcher.cc @@ -22,7 +22,8 @@ Network::DnsResolverSharedPtr ValidationDispatcher::createDnsResolver( } Network::ListenerPtr ValidationDispatcher::createListener(Network::SocketSharedPtr&&, - Network::ListenerCallbacks&, bool) { + Network::ListenerCallbacks&, bool, + uint32_t) { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } diff --git a/source/server/config_validation/dispatcher.h b/source/server/config_validation/dispatcher.h index e322e3f1cdc1..d61d3bcc0b43 100644 --- a/source/server/config_validation/dispatcher.h +++ b/source/server/config_validation/dispatcher.h @@ -27,7 +27,7 @@ class ValidationDispatcher : public DispatcherImpl { createDnsResolver(const std::vector& resolvers, const bool use_tcp_for_dns_lookups) override; Network::ListenerPtr createListener(Network::SocketSharedPtr&&, Network::ListenerCallbacks&, - bool bind_to_port) override; + bool bind_to_port, uint32_t backlog_size) override; protected: std::shared_ptr dns_resolver_{ diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index 323ddf4df430..c06545703508 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -141,7 +141,7 @@ ConnectionHandlerImpl::ActiveTcpListener::ActiveTcpListener(ConnectionHandlerImp : ActiveTcpListener( parent, parent.dispatcher_.createListener(config.listenSocketFactory().getListenSocket(), *this, - config.bindToPort()), + config.bindToPort(), config.tcpBacklogSize()), config) {} ConnectionHandlerImpl::ActiveTcpListener::ActiveTcpListener(ConnectionHandlerImpl& parent, diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc index f3fe37ade187..32e3a3efa0c4 100644 --- a/source/server/listener_impl.cc +++ b/source/server/listener_impl.cc @@ -233,6 +233,8 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config, PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, per_connection_buffer_limit_bytes, 1024 * 1024)), listener_tag_(parent_.factory_.nextListenerTag()), name_(name), added_via_api_(added_via_api), workers_started_(workers_started), hash_(hash), + tcp_backlog_size_( + PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, tcp_backlog_size, ENVOY_TCP_BACKLOG_SIZE)), validation_visitor_( added_via_api_ ? parent_.server_.messageValidationContext().dynamicValidationVisitor() : parent_.server_.messageValidationContext().staticValidationVisitor()), @@ -310,6 +312,8 @@ ListenerImpl::ListenerImpl(ListenerImpl& origin, PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, per_connection_buffer_limit_bytes, 1024 * 1024)), listener_tag_(origin.listener_tag_), name_(name), added_via_api_(added_via_api), workers_started_(workers_started), hash_(hash), + tcp_backlog_size_( + PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, tcp_backlog_size, ENVOY_TCP_BACKLOG_SIZE)), validation_visitor_( added_via_api_ ? parent_.server_.messageValidationContext().dynamicValidationVisitor() : parent_.server_.messageValidationContext().staticValidationVisitor()), diff --git a/source/server/listener_impl.h b/source/server/listener_impl.h index 920f8a24e9b3..818056dfdbec 100644 --- a/source/server/listener_impl.h +++ b/source/server/listener_impl.h @@ -311,6 +311,7 @@ class ListenerImpl final : public Network::ListenerConfig, const std::vector& accessLogs() const override { return access_logs_; } + uint32_t tcpBacklogSize() const override { return tcp_backlog_size_; } Init::Manager& initManager(); envoy::config::core::v3::TrafficDirection direction() const override { return config().traffic_direction(); @@ -371,6 +372,7 @@ class ListenerImpl final : public Network::ListenerConfig, const bool added_via_api_; const bool workers_started_; const uint64_t hash_; + const uint32_t tcp_backlog_size_; ProtobufMessage::ValidationVisitor& validation_visitor_; // A target is added to Server's InitManager if workers_started_ is false. diff --git a/test/common/network/listener_impl_test.cc b/test/common/network/listener_impl_test.cc index 5aaef758ce4a..91b58a473999 100644 --- a/test/common/network/listener_impl_test.cc +++ b/test/common/network/listener_impl_test.cc @@ -65,8 +65,8 @@ TEST_P(ListenerImplDeathTest, ErrorCallback) { class TestListenerImpl : public ListenerImpl { public: TestListenerImpl(Event::DispatcherImpl& dispatcher, SocketSharedPtr socket, ListenerCallbacks& cb, - bool bind_to_port) - : ListenerImpl(dispatcher, std::move(socket), cb, bind_to_port) {} + bool bind_to_port, uint32_t tcp_backlog = ENVOY_TCP_BACKLOG_SIZE) + : ListenerImpl(dispatcher, std::move(socket), cb, bind_to_port, tcp_backlog) {} MOCK_METHOD(Address::InstanceConstSharedPtr, getLocalAddress, (os_fd_t fd)); }; diff --git a/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc b/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc index 7c1bd0d80ae1..da6866036fce 100644 --- a/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc +++ b/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc @@ -79,6 +79,7 @@ class ProxyProtocolRegressionTest : public testing::TestWithParam& accessLogs() const override { return empty_access_logs_; } + uint32_t tcpBacklogSize() const override { return ENVOY_TCP_BACKLOG_SIZE; } // Network::FilterChainManager const Network::FilterChain* findFilterChain(const Network::ConnectionSocket&) const override { diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index a270ee2f569d..8aa06aad929d 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -94,6 +94,7 @@ class ProxyProtocolTest : public testing::TestWithParam& accessLogs() const override { return empty_access_logs_; } + uint32_t tcpBacklogSize() const override { return ENVOY_TCP_BACKLOG_SIZE; } // Network::FilterChainManager const Network::FilterChain* findFilterChain(const Network::ConnectionSocket&) const override { @@ -1291,6 +1292,7 @@ class WildcardProxyProtocolTest : public testing::TestWithParam& accessLogs() const override { return empty_access_logs_; } + uint32_t tcpBacklogSize() const override { return ENVOY_TCP_BACKLOG_SIZE; } // Network::FilterChainManager const Network::FilterChain* findFilterChain(const Network::ConnectionSocket&) const override { diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index 0c23c42e4b41..c9dd430d2440 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -731,6 +731,7 @@ class FakeUpstream : Logger::Loggable, return empty_access_logs_; } ResourceLimit& openConnections() override { return connection_resource_; } + uint32_t tcpBacklogSize() const override { return ENVOY_TCP_BACKLOG_SIZE; } void setMaxConnections(const uint32_t num_connections) { connection_resource_.setMax(num_connections); diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index 55983c27c536..187df9f15e8d 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -64,8 +64,9 @@ class MockDispatcher : public Dispatcher { } Network::ListenerPtr createListener(Network::SocketSharedPtr&& socket, - Network::ListenerCallbacks& cb, bool bind_to_port) override { - return Network::ListenerPtr{createListener_(std::move(socket), cb, bind_to_port)}; + Network::ListenerCallbacks& cb, bool bind_to_port, + uint32_t backlog_size) override { + return Network::ListenerPtr{createListener_(std::move(socket), cb, bind_to_port, backlog_size)}; } Network::UdpListenerPtr createUdpListener(Network::SocketSharedPtr&& socket, @@ -115,7 +116,7 @@ class MockDispatcher : public Dispatcher { MOCK_METHOD(Filesystem::Watcher*, createFilesystemWatcher_, ()); MOCK_METHOD(Network::Listener*, createListener_, (Network::SocketSharedPtr && socket, Network::ListenerCallbacks& cb, - bool bind_to_port)); + bool bind_to_port, uint32_t backlog_size)); MOCK_METHOD(Network::UdpListener*, createUdpListener_, (Network::SocketSharedPtr && socket, Network::UdpListenerCallbacks& cb)); MOCK_METHOD(Timer*, createTimer_, (Event::TimerCb cb)); diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h index 144051c0d981..0097df388d7a 100644 --- a/test/mocks/network/mocks.h +++ b/test/mocks/network/mocks.h @@ -358,6 +358,7 @@ class MockListenerConfig : public ListenerConfig { MOCK_METHOD(Network::UdpPacketWriterFactoryOptRef, udpPacketWriterFactory, ()); MOCK_METHOD(ConnectionBalancer&, connectionBalancer, ()); MOCK_METHOD(ResourceLimit&, openConnections, ()); + MOCK_METHOD(uint32_t, tcpBacklogSize, (), (const)); envoy::config::core::v3::TrafficDirection direction() const override { return envoy::config::core::v3::UNSPECIFIED; diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index a53c44d6e341..7ee24468032b 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -56,9 +56,11 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable> filter_chain_manager = nullptr) + std::shared_ptr> filter_chain_manager = nullptr, + uint32_t tcp_backlog_size = ENVOY_TCP_BACKLOG_SIZE) : parent_(parent), socket_(std::make_shared>()), socket_factory_(std::move(socket_factory)), tag_(tag), bind_to_port_(bind_to_port), + tcp_backlog_size_(tcp_backlog_size), hand_off_restored_destination_connections_(hand_off_restored_destination_connections), name_(name), listener_filters_timeout_(listener_filters_timeout), continue_on_listener_filters_timeout_(continue_on_listener_filters_timeout), @@ -109,6 +111,7 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable> overridden_filter_chain_manager = - nullptr) { + nullptr, + uint32_t tcp_backlog_size = ENVOY_TCP_BACKLOG_SIZE) { listeners_.emplace_back(std::make_unique( *this, tag, bind_to_port, hand_off_restored_destination_connections, name, socket_type, listener_filters_timeout, continue_on_listener_filters_timeout, socket_factory_, - overridden_filter_chain_manager)); + overridden_filter_chain_manager, tcp_backlog_size)); EXPECT_CALL(*socket_factory_, socketType()).WillOnce(Return(socket_type)); if (listener == nullptr) { // Expecting listener config in place update. @@ -191,10 +196,10 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggablesocket_)); if (socket_type == Network::Socket::Type::Stream) { - EXPECT_CALL(dispatcher_, createListener_(_, _, _)) + EXPECT_CALL(dispatcher_, createListener_(_, _, _, _)) .WillOnce(Invoke([listener, listener_callbacks](Network::SocketSharedPtr&&, - Network::ListenerCallbacks& cb, - bool) -> Network::Listener* { + Network::ListenerCallbacks& cb, bool, + uint32_t) -> Network::Listener* { if (listener_callbacks != nullptr) { *listener_callbacks = &cb; } @@ -1085,6 +1090,22 @@ TEST_F(ConnectionHandlerTest, ShutdownUdpListener) { << "The read_filter_ should be deleted before the udp_listener_ is deleted."; } +TEST_F(ConnectionHandlerTest, TcpBacklogCustom) { + uint32_t custom_backlog = 100; + TestListener* test_listener = addListener( + 1, true, false, "test_tcp_backlog", nullptr, nullptr, nullptr, nullptr, + Network::Socket::Type::Stream, std::chrono::milliseconds(), false, nullptr, custom_backlog); + EXPECT_CALL(*socket_factory_, getListenSocket()).WillOnce(Return(listeners_.back()->socket_)); + EXPECT_CALL(*socket_factory_, localAddress()).WillOnce(ReturnRef(local_address_)); + EXPECT_CALL(dispatcher_, createListener_(_, _, _, _)) + .WillOnce(Invoke([custom_backlog](Network::SocketSharedPtr&&, Network::ListenerCallbacks&, + bool, uint32_t backlog) -> Network::Listener* { + EXPECT_EQ(custom_backlog, backlog); + return nullptr; + })); + handler_->addListener(absl::nullopt, *test_listener); +} + } // namespace } // namespace Server } // namespace Envoy diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index c47c22f44b6e..a1ccd242cdd3 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -4721,6 +4721,23 @@ TEST_F(ListenerManagerImplTest, UdpDefaultWriterConfig) { EXPECT_FALSE(udp_packet_writer->isBatchMode()); } +TEST_F(ListenerManagerImplTest, TcpBacklogCustomConfig) { + const std::string yaml = TestEnvironment::substitute(R"EOF( + name: TcpBacklogConfigListener + address: + socket_address: { address: 127.0.0.1, port_value: 1111 } + tcp_backlog_size: 100 + filter_chains: + - filters: + )EOF", + Network::Address::IpVersion::v4); + + EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, _)); + manager_->addOrUpdateListener(parseListenerFromV3Yaml(yaml), "", true); + EXPECT_EQ(1U, manager_->listeners().size()); + EXPECT_EQ(100U, manager_->listeners().back().get().tcpBacklogSize()); +} + } // namespace } // namespace Server } // namespace Envoy From 02f396a94b4224746c203a3c4181ba768f98c61f Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Thu, 13 Aug 2020 10:29:29 -0700 Subject: [PATCH 2/2] comments - const backlog_size in listener - no default param in createListener Signed-off-by: Florin Coras --- include/envoy/event/dispatcher.h | 2 +- source/common/network/listener_impl.h | 2 +- test/common/http/codec_client_test.cc | 3 +- test/common/network/connection_impl_test.cc | 9 +++-- test/common/network/dns_impl_test.cc | 2 +- test/common/network/listener_impl_test.cc | 6 ++- .../transport_sockets/tls/ssl_socket_test.cc | 39 ++++++++++++------- 7 files changed, 41 insertions(+), 22 deletions(-) diff --git a/include/envoy/event/dispatcher.h b/include/envoy/event/dispatcher.h index f6e3526f7f53..98bb16ea7c38 100644 --- a/include/envoy/event/dispatcher.h +++ b/include/envoy/event/dispatcher.h @@ -153,7 +153,7 @@ class Dispatcher { */ virtual Network::ListenerPtr createListener(Network::SocketSharedPtr&& socket, Network::ListenerCallbacks& cb, bool bind_to_port, - uint32_t backlog_size = ENVOY_TCP_BACKLOG_SIZE) PURE; + uint32_t backlog_size) PURE; /** * Creates a logical udp listener on a specific port. diff --git a/source/common/network/listener_impl.h b/source/common/network/listener_impl.h index 9f98102f1b4a..61ca54d9d14e 100644 --- a/source/common/network/listener_impl.h +++ b/source/common/network/listener_impl.h @@ -25,7 +25,7 @@ class ListenerImpl : public BaseListenerImpl { void setupServerSocket(Event::DispatcherImpl& dispatcher, Socket& socket); ListenerCallbacks& cb_; - uint32_t backlog_size_; + const uint32_t backlog_size_; private: void onSocketEvent(short flags); diff --git a/test/common/http/codec_client_test.cc b/test/common/http/codec_client_test.cc index 15979e8350b3..0fa919d17833 100644 --- a/test/common/http/codec_client_test.cc +++ b/test/common/http/codec_client_test.cc @@ -286,7 +286,8 @@ class CodecNetworkTest : public testing::TestWithParamcreateClientConnection( socket->localAddress(), source_address_, Network::Test::createRawBufferSocket(), nullptr); - upstream_listener_ = dispatcher_->createListener(std::move(socket), listener_callbacks_, true); + upstream_listener_ = dispatcher_->createListener(std::move(socket), listener_callbacks_, true, + ENVOY_TCP_BACKLOG_SIZE); client_connection_ = client_connection.get(); client_connection_->addConnectionCallbacks(client_callbacks_); diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index 0a86156963a0..72c8f691213f 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -108,7 +108,8 @@ class ConnectionImplTest : public testing::TestWithParam { } socket_ = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true); - listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true); + listener_ = + dispatcher_->createListener(socket_, listener_callbacks_, true, ENVOY_TCP_BACKLOG_SIZE); client_connection_ = std::make_unique( *dispatcher_, socket_->localAddress(), source_address_, Network::Test::createRawBufferSocket(), socket_options_); @@ -1132,7 +1133,8 @@ TEST_P(ConnectionImplTest, BindFailureTest) { dispatcher_ = api_->allocateDispatcher("test_thread"); socket_ = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true); - listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true); + listener_ = + dispatcher_->createListener(socket_, listener_callbacks_, true, ENVOY_TCP_BACKLOG_SIZE); client_connection_ = dispatcher_->createClientConnection( socket_->localAddress(), source_address_, Network::Test::createRawBufferSocket(), nullptr); @@ -2238,7 +2240,8 @@ class ReadBufferLimitTest : public ConnectionImplTest { dispatcher_ = api_->allocateDispatcher("test_thread"); socket_ = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true); - listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true); + listener_ = + dispatcher_->createListener(socket_, listener_callbacks_, true, ENVOY_TCP_BACKLOG_SIZE); client_connection_ = dispatcher_->createClientConnection( socket_->localAddress(), Network::Address::InstanceConstSharedPtr(), diff --git a/test/common/network/dns_impl_test.cc b/test/common/network/dns_impl_test.cc index 8b49d8035bca..678e97852e49 100644 --- a/test/common/network/dns_impl_test.cc +++ b/test/common/network/dns_impl_test.cc @@ -437,7 +437,7 @@ class DnsImplTest : public testing::TestWithParam { server_ = std::make_unique(*dispatcher_); socket_ = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true); - listener_ = dispatcher_->createListener(socket_, *server_, true); + listener_ = dispatcher_->createListener(socket_, *server_, true, ENVOY_TCP_BACKLOG_SIZE); // Point c-ares at the listener with no search domains and TCP-only. peer_ = std::make_unique(dynamic_cast(resolver_.get())); diff --git a/test/common/network/listener_impl_test.cc b/test/common/network/listener_impl_test.cc index 91b58a473999..d3a80fea73c0 100644 --- a/test/common/network/listener_impl_test.cc +++ b/test/common/network/listener_impl_test.cc @@ -34,7 +34,8 @@ static void errorCallbackTest(Address::IpVersion version) { Network::Test::getCanonicalLoopbackAddress(version), nullptr, true); Network::MockListenerCallbacks listener_callbacks; Network::MockConnectionHandler connection_handler; - Network::ListenerPtr listener = dispatcher->createListener(socket, listener_callbacks, true); + Network::ListenerPtr listener = + dispatcher->createListener(socket, listener_callbacks, true, ENVOY_TCP_BACKLOG_SIZE); Network::ClientConnectionPtr client_connection = dispatcher->createClientConnection( socket->localAddress(), Network::Address::InstanceConstSharedPtr(), @@ -150,7 +151,8 @@ TEST_P(ListenerImplTest, GlobalConnectionLimitEnforcement) { Network::Test::getCanonicalLoopbackAddress(version_), nullptr, true); Network::MockListenerCallbacks listener_callbacks; Network::MockConnectionHandler connection_handler; - Network::ListenerPtr listener = dispatcher_->createListener(socket, listener_callbacks, true); + Network::ListenerPtr listener = + dispatcher_->createListener(socket, listener_callbacks, true, ENVOY_TCP_BACKLOG_SIZE); std::vector client_connections; std::vector server_connections; diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 2eb348a432ec..6fa102a74ad4 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -309,7 +309,8 @@ void testUtil(const TestUtilOptions& options) { Network::Test::getCanonicalLoopbackAddress(options.version()), nullptr, true); Network::MockListenerCallbacks callbacks; Network::MockConnectionHandler connection_handler; - Network::ListenerPtr listener = dispatcher->createListener(socket, callbacks, true); + Network::ListenerPtr listener = + dispatcher->createListener(socket, callbacks, true, ENVOY_TCP_BACKLOG_SIZE); envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext client_tls_context; TestUtility::loadFromYaml(TestEnvironment::substitute(options.clientCtxYaml()), @@ -610,7 +611,8 @@ const std::string testUtilV2(const TestUtilOptionsV2& options) { Network::Test::getCanonicalLoopbackAddress(options.version()), nullptr, true); NiceMock callbacks; Network::MockConnectionHandler connection_handler; - Network::ListenerPtr listener = dispatcher->createListener(socket, callbacks, true); + Network::ListenerPtr listener = + dispatcher->createListener(socket, callbacks, true, ENVOY_TCP_BACKLOG_SIZE); Stats::TestUtil::TestStore client_stats_store; Api::ApiPtr client_api = Api::createApiForTest(client_stats_store, time_system); @@ -2401,7 +2403,8 @@ TEST_P(SslSocketTest, FlushCloseDuringHandshake) { Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true); Network::MockListenerCallbacks callbacks; Network::MockConnectionHandler connection_handler; - Network::ListenerPtr listener = dispatcher_->createListener(socket, callbacks, true); + Network::ListenerPtr listener = + dispatcher_->createListener(socket, callbacks, true, ENVOY_TCP_BACKLOG_SIZE); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->localAddress(), Network::Address::InstanceConstSharedPtr(), @@ -2457,7 +2460,8 @@ TEST_P(SslSocketTest, HalfClose) { Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true); Network::MockListenerCallbacks listener_callbacks; Network::MockConnectionHandler connection_handler; - Network::ListenerPtr listener = dispatcher_->createListener(socket, listener_callbacks, true); + Network::ListenerPtr listener = + dispatcher_->createListener(socket, listener_callbacks, true, ENVOY_TCP_BACKLOG_SIZE); std::shared_ptr server_read_filter(new Network::MockReadFilter()); std::shared_ptr client_read_filter(new Network::MockReadFilter()); @@ -2539,7 +2543,8 @@ TEST_P(SslSocketTest, ClientAuthMultipleCAs) { Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true); Network::MockListenerCallbacks callbacks; Network::MockConnectionHandler connection_handler; - Network::ListenerPtr listener = dispatcher_->createListener(socket, callbacks, true); + Network::ListenerPtr listener = + dispatcher_->createListener(socket, callbacks, true, ENVOY_TCP_BACKLOG_SIZE); const std::string client_ctx_yaml = R"EOF( common_tls_context: @@ -2637,8 +2642,10 @@ void testTicketSessionResumption(const std::string& server_ctx_yaml1, NiceMock callbacks; Network::MockConnectionHandler connection_handler; Event::DispatcherPtr dispatcher(server_api->allocateDispatcher("test_thread")); - Network::ListenerPtr listener1 = dispatcher->createListener(socket1, callbacks, true); - Network::ListenerPtr listener2 = dispatcher->createListener(socket2, callbacks, true); + Network::ListenerPtr listener1 = + dispatcher->createListener(socket1, callbacks, true, ENVOY_TCP_BACKLOG_SIZE); + Network::ListenerPtr listener2 = + dispatcher->createListener(socket2, callbacks, true, ENVOY_TCP_BACKLOG_SIZE); envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext client_tls_context; TestUtility::loadFromYaml(TestEnvironment::substitute(client_ctx_yaml), client_tls_context); @@ -2773,7 +2780,8 @@ void testSupportForStatelessSessionResumption(const std::string& server_ctx_yaml NiceMock callbacks; Network::MockConnectionHandler connection_handler; Event::DispatcherPtr dispatcher(server_api->allocateDispatcher("test_thread")); - Network::ListenerPtr listener = dispatcher->createListener(tcp_socket, callbacks, true); + Network::ListenerPtr listener = + dispatcher->createListener(tcp_socket, callbacks, true, ENVOY_TCP_BACKLOG_SIZE); envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext client_tls_context; TestUtility::loadFromYaml(TestEnvironment::substitute(client_ctx_yaml), client_tls_context); @@ -3215,8 +3223,10 @@ TEST_P(SslSocketTest, ClientAuthCrossListenerSessionResumption) { Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true); Network::MockListenerCallbacks callbacks; Network::MockConnectionHandler connection_handler; - Network::ListenerPtr listener = dispatcher_->createListener(socket, callbacks, true); - Network::ListenerPtr listener2 = dispatcher_->createListener(socket2, callbacks, true); + Network::ListenerPtr listener = + dispatcher_->createListener(socket, callbacks, true, ENVOY_TCP_BACKLOG_SIZE); + Network::ListenerPtr listener2 = + dispatcher_->createListener(socket2, callbacks, true, ENVOY_TCP_BACKLOG_SIZE); const std::string client_ctx_yaml = R"EOF( common_tls_context: tls_certificates: @@ -3330,7 +3340,8 @@ void SslSocketTest::testClientSessionResumption(const std::string& server_ctx_ya Network::MockConnectionHandler connection_handler; Api::ApiPtr api = Api::createApiForTest(server_stats_store, time_system_); Event::DispatcherPtr dispatcher(server_api->allocateDispatcher("test_thread")); - Network::ListenerPtr listener = dispatcher->createListener(socket, callbacks, true); + Network::ListenerPtr listener = + dispatcher->createListener(socket, callbacks, true, ENVOY_TCP_BACKLOG_SIZE); Network::ConnectionPtr server_connection; Network::MockConnectionCallbacks server_connection_callbacks; @@ -3590,7 +3601,8 @@ TEST_P(SslSocketTest, SslError) { Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true); Network::MockListenerCallbacks callbacks; Network::MockConnectionHandler connection_handler; - Network::ListenerPtr listener = dispatcher_->createListener(socket, callbacks, true); + Network::ListenerPtr listener = + dispatcher_->createListener(socket, callbacks, true, ENVOY_TCP_BACKLOG_SIZE); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->localAddress(), Network::Address::InstanceConstSharedPtr(), @@ -4347,7 +4359,8 @@ class SslReadBufferLimitTest : public SslSocketTest { socket_ = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true); - listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true); + listener_ = + dispatcher_->createListener(socket_, listener_callbacks_, true, ENVOY_TCP_BACKLOG_SIZE); TestUtility::loadFromYaml(TestEnvironment::substitute(client_ctx_yaml_), upstream_tls_context_); auto client_cfg =