-
Notifications
You must be signed in to change notification settings - Fork 5.3k
connection: Add TCP_FASTOPEN listener option #2793
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.
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
Changes from all commits
87bb932
cdfeb38
982a27b
7cc3768
8dcf11d
6f55c84
dea8e2c
cc211dd
d9ba97a
e56a8d7
f2db864
9f3c97e
d2d4a27
a03533a
89a3b99
8289daa
e7ff081
8f40b58
42ad7e4
f90b61f
1d5734a
9e57d17
5abe337
2b21dd5
8626895
521faad
cb298fb
4219d2e
2f47e3d
b9312fd
06d426e
f9f608a
2ca16c3
470f934
e0dbd9f
3dd9372
3f11ee6
ec0fa87
4494d2c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |
|
|
||
| #include "server/configuration_impl.h" | ||
| #include "server/drain_manager_impl.h" | ||
| #include "server/listener_socket_option_impl.h" | ||
|
|
||
| #include "extensions/filters/listener/well_known_names.h" | ||
| #include "extensions/transport_sockets/well_known_names.h" | ||
|
|
@@ -110,16 +111,6 @@ ProdListenerComponentFactory::createDrainManager(envoy::api::v2::Listener::Drain | |
| return DrainManagerPtr{new DrainManagerImpl(server_, drain_type)}; | ||
| } | ||
|
|
||
| // Socket::Option implementation for API-defined listener socket options. | ||
| // This same object can be extended to handle additional listener socket options. | ||
| class ListenerSocketOption : public Network::SocketOptionImpl { | ||
| public: | ||
| ListenerSocketOption(const envoy::api::v2::Listener& config) | ||
| : Network::SocketOptionImpl( | ||
| PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, transparent, absl::optional<bool>{}), | ||
| PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, freebind, absl::optional<bool>{})) {} | ||
| }; | ||
|
|
||
| ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, ListenerManagerImpl& parent, | ||
| const std::string& name, bool modifiable, bool workers_started, | ||
| uint64_t hash) | ||
|
|
@@ -142,7 +133,7 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, ListenerManag | |
| ASSERT(config.filter_chains().size() >= 1); | ||
|
|
||
| // Add listen socket options from the config. | ||
| addListenSocketOption(std::make_shared<ListenerSocketOption>(config)); | ||
| addListenSocketOption(std::make_shared<ListenerSocketOptionImpl>(config)); | ||
|
|
||
| if (!config.listener_filters().empty()) { | ||
| listener_filter_factories_ = | ||
|
|
@@ -306,6 +297,10 @@ void ListenerImpl::setSocket(const Network::SocketSharedPtr& socket) { | |
| } else { | ||
| ENVOY_LOG(debug, "{}", message); | ||
| } | ||
|
|
||
| // Add the options to the socket_ so that SocketState::Listening options can be | ||
| // set in the worker after listen()/evconnlistener_new() is called. | ||
| socket_->addOption(option); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does adding these options here break anything else? |
||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| #include "server/listener_socket_option_impl.h" | ||
|
|
||
| #include "common/api/os_sys_calls_impl.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Server { | ||
|
|
||
| ListenerSocketOptionImpl::ListenerSocketOptionImpl(const envoy::api::v2::Listener& config) | ||
| : Network::SocketOptionImpl( | ||
| PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, transparent, absl::optional<bool>{}), | ||
| PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, freebind, absl::optional<bool>{})), | ||
| tcp_fast_open_queue_length_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( | ||
| config, tcp_fast_open_queue_length, absl::optional<uint32_t>{})) {} | ||
|
|
||
| ListenerSocketOptionImpl::ListenerSocketOptionImpl( | ||
| absl::optional<bool> transparent, absl::optional<bool> freebind, | ||
| absl::optional<uint32_t> tcp_fast_open_queue_length) | ||
| : Network::SocketOptionImpl(transparent, freebind), | ||
| tcp_fast_open_queue_length_(tcp_fast_open_queue_length) {} | ||
|
|
||
| bool ListenerSocketOptionImpl::setOption(Network::Socket& socket, | ||
| Network::Socket::SocketState state) const { | ||
| bool result = Network::SocketOptionImpl::setOption(socket, state); | ||
| if (!result) { | ||
| return result; | ||
| } | ||
|
|
||
| if (state == Network::Socket::SocketState::Listening) { | ||
| if (tcp_fast_open_queue_length_.has_value()) { | ||
| const int tfo_value = tcp_fast_open_queue_length_.value(); | ||
| const Network::SocketOptionName option_name = ENVOY_SOCKET_TCP_FASTOPEN; | ||
| if (option_name) { | ||
| const int error = Api::OsSysCallsSingleton::get().setsockopt( | ||
| socket.fd(), IPPROTO_TCP, option_name.value(), | ||
| reinterpret_cast<const void*>(&tfo_value), sizeof(tfo_value)); | ||
| if (error != 0) { | ||
| ENVOY_LOG(warn, "Setting TCP_FASTOPEN on listener socket failed: {}", strerror(errno)); | ||
| return false; | ||
| } else { | ||
| ENVOY_LOG(debug, "Successfully set socket option TCP_FASTOPEN to {}", tfo_value); | ||
| } | ||
| } else { | ||
| ENVOY_LOG(warn, "Unsupported socket option TCP_FASTOPEN"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This case is missing coverage. |
||
| return false; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| } // namespace Server | ||
| } // namespace Envoy | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| #pragma once | ||
|
|
||
| #include <netinet/tcp.h> | ||
|
|
||
| #include "envoy/api/v2/listener/listener.pb.h" | ||
|
|
||
| #include "common/network/listen_socket_impl.h" | ||
| #include "common/network/socket_option_impl.h" | ||
|
|
||
| #include "server/configuration_impl.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Server { | ||
|
|
||
| // Socket::Option implementation for API-defined listener socket options. | ||
| // This same object can be extended to handle additional listener socket options. | ||
| class ListenerSocketOptionImpl : public Network::SocketOptionImpl { | ||
| public: | ||
| ListenerSocketOptionImpl(const envoy::api::v2::Listener& config); | ||
|
|
||
| ListenerSocketOptionImpl(absl::optional<bool> transparent, absl::optional<bool> freebind, | ||
| absl::optional<uint32_t> tcp_fast_open_queue_length); | ||
|
|
||
| // Socket::Option | ||
| bool setOption(Network::Socket& socket, Network::Socket::SocketState state) const override; | ||
|
|
||
| private: | ||
| const absl::optional<uint32_t> tcp_fast_open_queue_length_; | ||
| }; | ||
|
|
||
| } // namespace Server | ||
| } // namespace Envoy |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,6 +86,35 @@ INSTANTIATE_TEST_CASE_P(IpVersions, ListenerImplTest, | |
| testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), | ||
| TestUtility::ipTestParamsToString); | ||
|
|
||
| // Test that socket options are set after the listener is setup. | ||
| TEST_P(ListenerImplTest, SetListeningSocketOptionsSuccess) { | ||
| Event::DispatcherImpl dispatcher; | ||
| Network::MockListenerCallbacks listener_callbacks; | ||
| Network::MockConnectionHandler connection_handler; | ||
|
|
||
| Network::TcpListenSocket socket(Network::Test::getCanonicalLoopbackAddress(version_), nullptr, | ||
| true); | ||
| std::shared_ptr<MockSocketOption> option = std::make_shared<MockSocketOption>(); | ||
| socket.addOption(option); | ||
| EXPECT_CALL(*option, setOption(_, Socket::SocketState::Listening)).WillOnce(Return(true)); | ||
| TestListenerImpl listener(dispatcher, socket, listener_callbacks, true, false); | ||
| } | ||
|
|
||
| // Test that an exception is thrown if there is an error setting socket options. | ||
| TEST_P(ListenerImplTest, SetListeningSocketOptionsError) { | ||
| Event::DispatcherImpl dispatcher; | ||
| Network::MockListenerCallbacks listener_callbacks; | ||
| Network::MockConnectionHandler connection_handler; | ||
|
|
||
| Network::TcpListenSocket socket(Network::Test::getCanonicalLoopbackAddress(version_), nullptr, | ||
| true); | ||
| std::shared_ptr<MockSocketOption> option = std::make_shared<MockSocketOption>(); | ||
| socket.addOption(option); | ||
| EXPECT_CALL(*option, setOption(_, Socket::SocketState::Listening)).WillOnce(Return(false)); | ||
| EXPECT_THROW(TestListenerImpl(dispatcher, socket, listener_callbacks, true, false), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could make this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I'm working on this one. |
||
| CreateListenerException); | ||
| } | ||
|
|
||
| TEST_P(ListenerImplTest, UseActualDst) { | ||
| Stats::IsolatedStoreImpl stats_store; | ||
| Event::DispatcherImpl dispatcher; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@htuch Do you mean just enforce TCP_FASTOPEN on all listeners by default and then run the tests?