diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index e300d19c81ab7..b5d3472ad8f57 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -279,8 +279,8 @@ void ConnectionImpl::noDelay(bool enable) { return; } - // Don't set NODELAY for unix domain sockets - if (socket_->addressType() == Address::Type::Pipe) { + // Don't set NODELAY for unix domain sockets or internal socket. + if (socket_->addressType() != Address::Type::Ip) { return; } @@ -834,12 +834,21 @@ ClientConnectionImpl::ClientConnectionImpl( const Network::Address::InstanceConstSharedPtr& source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options) - : ConnectionImpl(dispatcher, std::make_unique(remote_address, options), - std::move(transport_socket), stream_info_, false), + : ClientConnectionImpl(dispatcher, std::make_unique(remote_address, options), + source_address, std::move(transport_socket), options) {} + +ClientConnectionImpl::ClientConnectionImpl( + Event::Dispatcher& dispatcher, std::unique_ptr socket, + const Address::InstanceConstSharedPtr& source_address, + Network::TransportSocketPtr&& transport_socket, + const Network::ConnectionSocket::OptionsSharedPtr& options) + : ConnectionImpl(dispatcher, std::move(socket), std::move(transport_socket), stream_info_, + false), stream_info_(dispatcher.timeSource(), socket_->connectionInfoProviderSharedPtr()) { + // There are no meaningful socket options or source address semantics for // non-IP sockets, so skip. - if (remote_address->ip() == nullptr) { + if (socket_->connectionInfoProviderSharedPtr()->remoteAddress()->ip() == nullptr) { return; } if (!Network::Socket::applyOptions(options, *socket_, diff --git a/source/common/network/connection_impl.h b/source/common/network/connection_impl.h index ab8b9ff8f8567..e20119ffb095a 100644 --- a/source/common/network/connection_impl.h +++ b/source/common/network/connection_impl.h @@ -245,6 +245,10 @@ class ClientConnectionImpl : public ConnectionImpl, virtual public ClientConnect const Address::InstanceConstSharedPtr& source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options); + ClientConnectionImpl(Event::Dispatcher& dispatcher, std::unique_ptr socket, + const Address::InstanceConstSharedPtr& source_address, + Network::TransportSocketPtr&& transport_socket, + const Network::ConnectionSocket::OptionsSharedPtr& options); // Network::ClientConnection void connect() override; diff --git a/source/extensions/io_socket/user_space/BUILD b/source/extensions/io_socket/user_space/BUILD index 334fe51a916e4..01a5948f4c9e4 100644 --- a/source/extensions/io_socket/user_space/BUILD +++ b/source/extensions/io_socket/user_space/BUILD @@ -13,6 +13,7 @@ envoy_cc_extension( name = "config", srcs = ["config.h"], deps = [ + ":io_handle_impl_lib", ], ) diff --git a/source/extensions/io_socket/user_space/io_handle_impl.cc b/source/extensions/io_socket/user_space/io_handle_impl.cc index bbdb37e3b09ef..55f425ec542cc 100644 --- a/source/extensions/io_socket/user_space/io_handle_impl.cc +++ b/source/extensions/io_socket/user_space/io_handle_impl.cc @@ -274,17 +274,38 @@ Network::IoHandlePtr IoHandleImpl::accept(struct sockaddr*, socklen_t*) { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } -Api::SysCallIntResult IoHandleImpl::connect(Network::Address::InstanceConstSharedPtr) { - // Buffered Io handle should always be considered as connected. - // Use write or read to determine if peer is closed. - return {0, 0}; +Api::SysCallIntResult IoHandleImpl::connect(Network::Address::InstanceConstSharedPtr address) { + if (peer_handle_ != nullptr) { + // Buffered Io handle should always be considered as connected unless the server peer cannot be + // found. Use write or read to determine if peer is closed. + return {0, 0}; + } else { + ENVOY_LOG(debug, "user namespace handle {} connect to previously closed peer {}.", + static_cast(this), address->asStringView()); + return Api::SysCallIntResult{-1, SOCKET_ERROR_INVAL}; + } } Api::SysCallIntResult IoHandleImpl::setOption(int, int, const void*, socklen_t) { return makeInvalidSyscallResult(); } -Api::SysCallIntResult IoHandleImpl::getOption(int, int, void*, socklen_t*) { +Api::SysCallIntResult IoHandleImpl::getOption(int level, int optname, void* optval, + socklen_t* optlen) { + // Check result of connect(). It is either connected or closed. + if (level == SOL_SOCKET && optname == SO_ERROR) { + if (peer_handle_ != nullptr) { + // The peer is valid at this comment. Consider it as connected. + *optlen = sizeof(int); + *static_cast(optval) = 0; + return Api::SysCallIntResult{0, 0}; + } else { + // The peer is closed. Reset the option value to non-zero. + *optlen = sizeof(int); + *static_cast(optval) = SOCKET_ERROR_INVAL; + return Api::SysCallIntResult{0, 0}; + } + } return makeInvalidSyscallResult(); } diff --git a/source/extensions/io_socket/user_space/io_handle_impl.h b/source/extensions/io_socket/user_space/io_handle_impl.h index 71d3248d47223..71d2161be17f3 100644 --- a/source/extensions/io_socket/user_space/io_handle_impl.h +++ b/source/extensions/io_socket/user_space/io_handle_impl.h @@ -143,6 +143,8 @@ class IoHandleImpl final : public Network::IoHandle, ASSERT(!peer_handle_); ASSERT(!write_shutdown_); peer_handle_ = writable_peer; + ENVOY_LOG(trace, "io handle {} set peer handle to {}.", static_cast(this), + static_cast(writable_peer)); } private: diff --git a/test/extensions/io_socket/user_space/BUILD b/test/extensions/io_socket/user_space/BUILD index 989d56ec9466a..458d6345d36da 100644 --- a/test/extensions/io_socket/user_space/BUILD +++ b/test/extensions/io_socket/user_space/BUILD @@ -39,3 +39,22 @@ envoy_extension_cc_test( "//test/mocks/event:event_mocks", ], ) + +envoy_extension_cc_test( + name = "connection_compatbility_test", + srcs = ["connection_compatbility_test.cc"], + extension_names = ["envoy.io_socket.user_space"], + deps = [ + "//source/common/buffer:buffer_lib", + "//source/common/common:utility_lib", + "//source/common/event:dispatcher_includes", + "//source/common/network:address_lib", + "//source/common/network:connection_lib", + "//source/common/network:listen_socket_lib", + "//source/extensions/io_socket/user_space:io_handle_impl_lib", + "//test/mocks/api:api_mocks", + "//test/mocks/event:event_mocks", + "//test/mocks/network:network_mocks", + "//test/test_common:network_utility_lib", + ], +) diff --git a/test/extensions/io_socket/user_space/connection_compatbility_test.cc b/test/extensions/io_socket/user_space/connection_compatbility_test.cc new file mode 100644 index 0000000000000..e7410aa7bb3dc --- /dev/null +++ b/test/extensions/io_socket/user_space/connection_compatbility_test.cc @@ -0,0 +1,100 @@ +#include + +#include "source/common/network/address_impl.h" +#include "source/common/network/connection_impl.h" +#include "source/common/network/io_socket_handle_impl.h" +#include "source/common/network/listen_socket_impl.h" +#include "source/common/network/raw_buffer_socket.h" +#include "source/common/network/utility.h" +#include "source/extensions/io_socket/user_space/io_handle_impl.h" + +#include "test/mocks/api/mocks.h" +#include "test/mocks/event/mocks.h" +#include "test/mocks/network/mocks.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using testing::Invoke; + +namespace Envoy { +namespace Extensions { +namespace IoSocket { +namespace UserSpace { +namespace { + +// This class verifies client connection can be established with user space socket. +class InternalClientConnectionImplTest : public testing::Test { +public: + InternalClientConnectionImplTest() + : api_(Api::createApiForTest()), dispatcher_(api_->allocateDispatcher("test_thread")) {} + + void SetUp() override { + std::tie(io_handle_, io_handle_peer_) = IoHandleFactory::createIoHandlePair(); + local_addr_ = io_handle_->localAddress(); + remote_addr_ = io_handle_->peerAddress(); + } + Api::ApiPtr api_; + Event::DispatcherPtr dispatcher_; + std::unique_ptr io_handle_; + std::unique_ptr io_handle_peer_; + Network::MockConnectionCallbacks connection_callbacks; + std::unique_ptr client_; + Network::Address::InstanceConstSharedPtr local_addr_; + Network::Address::InstanceConstSharedPtr remote_addr_; +}; + +TEST_F(InternalClientConnectionImplTest, Basic) { + client_ = std::make_unique( + *dispatcher_, + std::make_unique(std::move(io_handle_), local_addr_, + remote_addr_), + nullptr, std::make_unique(), nullptr); + client_->connect(); + client_->noDelay(true); + dispatcher_->run(Event::Dispatcher::RunType::Block); + + client_->close(Network::ConnectionCloseType::NoFlush); +} + +TEST_F(InternalClientConnectionImplTest, ConnectCallbacksAreInvoked) { + client_ = std::make_unique( + *dispatcher_, + std::make_unique(std::move(io_handle_), local_addr_, + remote_addr_), + nullptr, std::make_unique(), nullptr); + client_->addConnectionCallbacks(connection_callbacks); + client_->connect(); + client_->noDelay(true); + EXPECT_CALL(connection_callbacks, onEvent(_)) + .WillOnce(Invoke([&](Network::ConnectionEvent event) -> void { + EXPECT_EQ(event, Network::ConnectionEvent::Connected); + dispatcher_->exit(); + })); + dispatcher_->run(Event::Dispatcher::RunType::Block); + EXPECT_CALL(connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)); + + client_->close(Network::ConnectionCloseType::NoFlush); +} + +TEST_F(InternalClientConnectionImplTest, ConnectFailed) { + client_ = std::make_unique( + *dispatcher_, + std::make_unique(std::move(io_handle_), local_addr_, + remote_addr_), + nullptr, std::make_unique(), nullptr); + client_->addConnectionCallbacks(connection_callbacks); + client_->connect(); + client_->noDelay(true); + + io_handle_peer_->close(); + EXPECT_CALL(connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose)); + dispatcher_->run(Event::Dispatcher::RunType::Block); + + client_->close(Network::ConnectionCloseType::NoFlush); +} +} // namespace +} // namespace UserSpace +} // namespace IoSocket +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/io_socket/user_space/io_handle_impl_test.cc b/test/extensions/io_socket/user_space/io_handle_impl_test.cc index fab734bb395d6..d75246348a8f3 100644 --- a/test/extensions/io_socket/user_space/io_handle_impl_test.cc +++ b/test/extensions/io_socket/user_space/io_handle_impl_test.cc @@ -20,6 +20,8 @@ namespace IoSocket { namespace UserSpace { namespace { +constexpr int CONNECTED = 0; + MATCHER(IsInvalidAddress, "") { return arg.err_->getErrorCode() == Api::IoError::IoErrorCode::NoSupport; } @@ -1007,6 +1009,41 @@ TEST_F(IoHandleImplTest, Connect) { auto address_is_ignored = std::make_shared("listener_id"); EXPECT_EQ(0, io_handle_->connect(address_is_ignored).return_value_); + + // Below is emulation of the connect(). + int immediate_error_value = -1; + socklen_t error_value_len = 0; + EXPECT_EQ(0, io_handle_->getOption(SOL_SOCKET, SO_ERROR, &immediate_error_value, &error_value_len) + .return_value_); + EXPECT_EQ(sizeof(int), error_value_len); + EXPECT_EQ(CONNECTED, immediate_error_value); + + // If the peer shutdown write but not yet closes, this io_handle should consider it + // as connected because the socket may be readable. + immediate_error_value = -1; + error_value_len = 0; + EXPECT_EQ(io_handle_peer_->shutdown(ENVOY_SHUT_WR).return_value_, 0); + EXPECT_EQ(0, io_handle_->getOption(SOL_SOCKET, SO_ERROR, &immediate_error_value, &error_value_len) + .return_value_); + EXPECT_EQ(sizeof(int), error_value_len); + EXPECT_EQ(CONNECTED, immediate_error_value); +} + +TEST_F(IoHandleImplTest, ConnectToClosedIoHandle) { + auto address_is_ignored = + std::make_shared("listener_id"); + io_handle_peer_->close(); + auto result = io_handle_->connect(address_is_ignored); + EXPECT_EQ(-1, result.return_value_); + EXPECT_EQ(SOCKET_ERROR_INVAL, result.errno_); + + // Below is emulation of the connect(). + int immediate_error_value = -1; + socklen_t error_value_len = 0; + EXPECT_EQ(0, io_handle_->getOption(SOL_SOCKET, SO_ERROR, &immediate_error_value, &error_value_len) + .return_value_); + EXPECT_EQ(sizeof(int), error_value_len); + EXPECT_NE(CONNECTED, immediate_error_value); } TEST_F(IoHandleImplTest, ActivateEvent) {