diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 93ba386848d87..6ab055a5002c3 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -70,6 +70,11 @@ minor_behavior_changes: change: | Aws request signing common code uses http async client by default, moving curl to deprecation path. This behavior change can be reverted by setting the ``envoy_reloadable_features_use_http_client_to_fetch_aws_credentials`` runtime flag to ``false``. +- area: quic + change: | + Connect the QUIC UDP client connection sockets before use and sockets will only bind if + the local address is specified. This behavior change can be reverted by setting the + ``envoy_reloadable_features_quic_connect_client_udp_sockets`` runtime flag to ``false``. bug_fixes: # *Changes expected to improve the state of the world and are unlikely to have negative effects* diff --git a/contrib/vcl/source/vcl_io_handle.cc b/contrib/vcl/source/vcl_io_handle.cc index f345857f83658..8078d35e65a98 100644 --- a/contrib/vcl/source/vcl_io_handle.cc +++ b/contrib/vcl/source/vcl_io_handle.cc @@ -166,6 +166,8 @@ Api::IoCallUint64Result VclIoHandle::close() { bool VclIoHandle::isOpen() const { return VCL_SH_VALID(sh_); } +bool VclIoHandle::wasConnected() const { return false; } + Api::IoCallUint64Result VclIoHandle::readv(uint64_t max_length, Buffer::RawSlice* slices, uint64_t num_slice) { if (!VCL_SH_VALID(sh_)) { diff --git a/contrib/vcl/source/vcl_io_handle.h b/contrib/vcl/source/vcl_io_handle.h index 819fb5600b545..167f9c2370c63 100644 --- a/contrib/vcl/source/vcl_io_handle.h +++ b/contrib/vcl/source/vcl_io_handle.h @@ -31,6 +31,7 @@ class VclIoHandle : public Envoy::Network::IoHandle, os_fd_t fdDoNotUse() const override { return fd_; } Api::IoCallUint64Result close() override; bool isOpen() const override; + bool wasConnected() const override; Api::IoCallUint64Result readv(uint64_t max_length, Buffer::RawSlice* slices, uint64_t num_slice) override; Api::IoCallUint64Result read(Buffer::Instance& buffer, diff --git a/envoy/network/io_handle.h b/envoy/network/io_handle.h index ce1dcd802b4fe..474c81f7959d3 100644 --- a/envoy/network/io_handle.h +++ b/envoy/network/io_handle.h @@ -65,6 +65,12 @@ class IoHandle { */ virtual bool isOpen() const PURE; + /** + * Return true if the socket has had connect() successfully called on it. + * Use isOpen() to check if the socket is still connected or not. + */ + virtual bool wasConnected() const PURE; + /** * Read data into given slices. * @param max_length supplies the maximum length to read. diff --git a/source/common/http/http3/conn_pool.cc b/source/common/http/http3/conn_pool.cc index 236dad0eb550c..f18fe88bd8068 100644 --- a/source/common/http/http3/conn_pool.cc +++ b/source/common/http/http3/conn_pool.cc @@ -148,16 +148,12 @@ Http3ConnPoolImpl::createClientConnection(Quic::QuicStatNames& quic_stat_names, auto upstream_local_address_selector = host()->cluster().getUpstreamLocalAddressSelector(); auto upstream_local_address = upstream_local_address_selector->getUpstreamLocalAddress(address, socketOptions()); - auto source_address = upstream_local_address.address_; - - if (source_address == nullptr) { - source_address = Network::Utility::getLocalAddress(address->ip()->version()); - } return Quic::createQuicNetworkConnection( - quic_info_, std::move(crypto_config), server_id_, dispatcher(), address, source_address, - quic_stat_names, rtt_cache, scope, upstream_local_address.socket_options_, - transportSocketOptions(), connection_id_generator_, host_->transportSocketFactory()); + quic_info_, std::move(crypto_config), server_id_, dispatcher(), address, + upstream_local_address.address_, quic_stat_names, rtt_cache, scope, + upstream_local_address.socket_options_, transportSocketOptions(), connection_id_generator_, + host_->transportSocketFactory()); } std::unique_ptr diff --git a/source/common/network/io_socket_handle_base_impl.cc b/source/common/network/io_socket_handle_base_impl.cc index 0db749c7a239f..3a2e8f3e79dc6 100644 --- a/source/common/network/io_socket_handle_base_impl.cc +++ b/source/common/network/io_socket_handle_base_impl.cc @@ -29,6 +29,8 @@ IoSocketHandleBaseImpl::~IoSocketHandleBaseImpl() { bool IoSocketHandleBaseImpl::isOpen() const { return SOCKET_VALID(fd_); } +bool IoSocketHandleBaseImpl::wasConnected() const { return was_connected_; } + bool IoSocketHandleBaseImpl::supportsMmsg() const { return Api::OsSysCallsSingleton::get().supportsMmsg(); } diff --git a/source/common/network/io_socket_handle_base_impl.h b/source/common/network/io_socket_handle_base_impl.h index 7873154c0c0f7..be02fb00b1f0b 100644 --- a/source/common/network/io_socket_handle_base_impl.h +++ b/source/common/network/io_socket_handle_base_impl.h @@ -19,6 +19,7 @@ class IoSocketHandleBaseImpl : public IoHandle, protected Logger::Loggable domain_; + bool was_connected_ = false; }; } // namespace Network diff --git a/source/common/network/io_socket_handle_impl.cc b/source/common/network/io_socket_handle_impl.cc index 956054ad4532b..deba098a38d52 100644 --- a/source/common/network/io_socket_handle_impl.cc +++ b/source/common/network/io_socket_handle_impl.cc @@ -539,7 +539,11 @@ Api::SysCallIntResult IoSocketHandleImpl::connect(Address::InstanceConstSharedPt } #endif - return Api::OsSysCallsSingleton::get().connect(fd_, sockaddr_to_use, sockaddr_len_to_use); + auto result = Api::OsSysCallsSingleton::get().connect(fd_, sockaddr_to_use, sockaddr_len_to_use); + if (result.return_value_ != -1) { + was_connected_ = true; + } + return result; } IoHandlePtr IoSocketHandleImpl::duplicate() { diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index d1b2e3734d796..f177756b5f86a 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -515,16 +515,27 @@ Api::IoCallUint64Result Utility::writeToSocket(IoHandle& handle, Buffer::RawSlic const Address::Instance& peer_address) { Api::IoCallUint64Result send_result( /*rc=*/0, /*err=*/Api::IoError::none()); + + const bool is_connected = handle.wasConnected(); do { - send_result = handle.sendmsg(slices, num_slices, 0, local_ip, peer_address); + if (is_connected) { + // The socket is already connected, so the local and peer addresses should not be specified. + // Instead, a writev is called. + send_result = handle.writev(slices, num_slices); + } else { + // For non-connected sockets(), calling sendmsg with the peer address specified ensures the + // connection happens first. + send_result = handle.sendmsg(slices, num_slices, 0, local_ip, peer_address); + } } while (!send_result.ok() && // Send again if interrupted. send_result.err_->getErrorCode() == Api::IoError::IoErrorCode::Interrupt); if (send_result.ok()) { - ENVOY_LOG_MISC(trace, "sendmsg bytes {}", send_result.return_value_); + ENVOY_LOG_MISC(trace, "{} bytes {}", is_connected ? "writev" : "sendmsg", + send_result.return_value_); } else { - ENVOY_LOG_MISC(debug, "sendmsg failed with error code {}: {}", + ENVOY_LOG_MISC(debug, "{} failed with error code {}: {}", is_connected ? "writev" : "sendmsg", static_cast(send_result.err_->getErrorCode()), send_result.err_->getErrorDetails()); } diff --git a/source/common/quic/envoy_quic_client_connection.cc b/source/common/quic/envoy_quic_client_connection.cc index 7b234bdeedbd3..d3321085458ac 100644 --- a/source/common/quic/envoy_quic_client_connection.cc +++ b/source/common/quic/envoy_quic_client_connection.cc @@ -128,7 +128,10 @@ void EnvoyQuicClientConnection::setUpConnectionSocket(Network::ConnectionSocket& connection_socket.close(); } } - if (!connection_socket.isOpen()) { + if (!connection_socket.isOpen() && connectionSocket().get() == &connection_socket) { + // Only close the connection if the connection socket is the current one. If it is a probing + // socket that isn't the current socket, do not close the connection upon failure, as the + // current socket is still usable. CloseConnection(quic::QUIC_CONNECTION_CANCELLED, "Fail to set up connection socket.", quic::ConnectionCloseBehavior::SILENT_CLOSE); } @@ -164,7 +167,7 @@ void EnvoyQuicClientConnection::maybeMigratePort() { probeWithNewPort(peer_address(), quic::PathValidationReason::kPortMigration); } -void EnvoyQuicClientConnection::probeWithNewPort(const quic::QuicSocketAddress& peer_address, +void EnvoyQuicClientConnection::probeWithNewPort(const quic::QuicSocketAddress& peer_addr, quic::PathValidationReason reason) { const Network::Address::InstanceConstSharedPtr& current_local_address = connectionSocket()->connectionInfoProvider().localAddress(); @@ -179,9 +182,10 @@ void EnvoyQuicClientConnection::probeWithNewPort(const quic::QuicSocketAddress& } // The probing socket will have the same host but a different port. - auto probing_socket = - createConnectionSocket(connectionSocket()->connectionInfoProvider().remoteAddress(), - new_local_address, connectionSocket()->options(), prefer_gro_); + auto probing_socket = createConnectionSocket( + peer_addr == peer_address() ? connectionSocket()->connectionInfoProvider().remoteAddress() + : quicAddressToEnvoyAddressInstance(peer_addr), + new_local_address, connectionSocket()->options(), prefer_gro_); setUpConnectionSocket(*probing_socket, delegate_); auto writer = std::make_unique( std::make_unique(probing_socket->ioHandle())); @@ -189,7 +193,7 @@ void EnvoyQuicClientConnection::probeWithNewPort(const quic::QuicSocketAddress& probing_socket->connectionInfoProvider().localAddress()->ip()); auto context = std::make_unique( - self_address, peer_address, std::move(writer), std::move(probing_socket)); + self_address, peer_addr, std::move(writer), std::move(probing_socket)); ValidatePath(std::move(context), std::make_unique(*this), reason); } diff --git a/source/common/quic/envoy_quic_client_connection.h b/source/common/quic/envoy_quic_client_connection.h index 77a21e6cabdba..197f12ebf726c 100644 --- a/source/common/quic/envoy_quic_client_connection.h +++ b/source/common/quic/envoy_quic_client_connection.h @@ -146,7 +146,7 @@ class EnvoyQuicClientConnection : public quic::QuicConnection, void maybeMigratePort(); - void probeWithNewPort(const quic::QuicSocketAddress& peer_address, + void probeWithNewPort(const quic::QuicSocketAddress& peer_addr, quic::PathValidationReason reason); OptRef delegate_; diff --git a/source/common/quic/envoy_quic_utils.cc b/source/common/quic/envoy_quic_utils.cc index 3b01794a68e64..f2ec95d6faf62 100644 --- a/source/common/quic/envoy_quic_utils.cc +++ b/source/common/quic/envoy_quic_utils.cc @@ -14,6 +14,18 @@ namespace Envoy { namespace Quic { +namespace { + +Network::Address::InstanceConstSharedPtr +getLoopbackAddress(const Network::Address::IpVersion version) { + if (version == Network::Address::IpVersion::v6) { + return std::make_shared("::1"); + } + return std::make_shared("127.0.0.1"); +} + +} // namespace + // TODO(danzh): this is called on each write. Consider to return an address instance on the stack if // the heap allocation is too expensive. Network::Address::InstanceConstSharedPtr @@ -168,16 +180,23 @@ createConnectionSocket(const Network::Address::InstanceConstSharedPtr& peer_addr Network::Address::InstanceConstSharedPtr& local_addr, const Network::ConnectionSocket::OptionsSharedPtr& options, const bool prefer_gro) { - if (local_addr == nullptr) { - local_addr = Network::Utility::getLocalAddress(peer_addr->ip()->version()); - } + ASSERT(peer_addr != nullptr); + const bool should_connect = + Runtime::runtimeFeatureEnabled("envoy.reloadable_features.quic_connect_client_udp_sockets"); size_t max_addresses_cache_size = Runtime::runtimeFeatureEnabled( "envoy.reloadable_features.quic_upstream_socket_use_address_cache_for_read") ? 4u : 0u; + + if (local_addr == nullptr && !should_connect) { + local_addr = Network::Utility::getLocalAddress(peer_addr->ip()->version()); + } auto connection_socket = std::make_unique( - Network::Socket::Type::Datagram, local_addr, peer_addr, + Network::Socket::Type::Datagram, + // Use the loopback address if `local_addr` is null, to pass in the socket interface used to + // create the IoHandle, without having to make the more expensive `getifaddrs` call. + local_addr ? local_addr : getLoopbackAddress(peer_addr->ip()->version()), peer_addr, Network::SocketCreationOptions{false, max_addresses_cache_size}); connection_socket->setDetectedTransportProtocol("quic"); if (!connection_socket->isOpen()) { @@ -201,8 +220,20 @@ createConnectionSocket(const Network::Address::InstanceConstSharedPtr& peer_addr ENVOY_LOG_MISC(error, "Fail to apply pre-bind options"); return connection_socket; } - connection_socket->bind(local_addr); - ASSERT(local_addr->ip()); + + if (local_addr != nullptr) { + connection_socket->bind(local_addr); + ASSERT(local_addr->ip()); + } + if (should_connect) { + if (auto result = connection_socket->connect(peer_addr); result.return_value_ == -1) { + connection_socket->close(); + ENVOY_LOG_MISC(error, "Fail to connect socket: ({}) {}", result.errno_, + errorDetails(result.errno_)); + return connection_socket; + } + } + local_addr = connection_socket->connectionInfoProvider().localAddress(); if (!Network::Socket::applyOptions(connection_socket->options(), *connection_socket, envoy::config::core::v3::SocketOption::STATE_BOUND)) { diff --git a/source/common/quic/quic_io_handle_wrapper.h b/source/common/quic/quic_io_handle_wrapper.h index 8df726c590559..73f752d877771 100644 --- a/source/common/quic/quic_io_handle_wrapper.h +++ b/source/common/quic/quic_io_handle_wrapper.h @@ -24,6 +24,7 @@ class QuicIoHandleWrapper : public Network::IoHandle { return Api::ioCallUint64ResultNoError(); } bool isOpen() const override { return !closed_; } + bool wasConnected() const override { return io_handle_.wasConnected(); } Api::IoCallUint64Result readv(uint64_t max_length, Buffer::RawSlice* slices, uint64_t num_slice) override { if (closed_) { diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index ce42fe31e4c40..9dbe893087ca4 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -73,6 +73,7 @@ RUNTIME_GUARD(envoy_reloadable_features_original_dst_rely_on_idle_timeout); RUNTIME_GUARD(envoy_reloadable_features_prefer_ipv6_dns_on_macos); RUNTIME_GUARD(envoy_reloadable_features_proxy_104); RUNTIME_GUARD(envoy_reloadable_features_proxy_status_mapping_more_core_response_flags); +RUNTIME_GUARD(envoy_reloadable_features_quic_connect_client_udp_sockets); RUNTIME_GUARD(envoy_reloadable_features_quic_receive_ecn); // Ignore the automated "remove this flag" issue: we should keep this for 1 year. Confirm with // @danzh2010 or @RyanTheOptimist before removing. 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 c953862bc9bd2..cf86c0700dc7a 100644 --- a/source/extensions/io_socket/user_space/io_handle_impl.cc +++ b/source/extensions/io_socket/user_space/io_handle_impl.cc @@ -90,6 +90,8 @@ Api::IoCallUint64Result IoHandleImpl::close() { bool IoHandleImpl::isOpen() const { return !closed_; } +bool IoHandleImpl::wasConnected() const { return false; } + Api::IoCallUint64Result IoHandleImpl::readv(uint64_t max_length, Buffer::RawSlice* slices, uint64_t num_slice) { if (!isOpen()) { 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 3b7bbcb6b59af..e2f1b8587f993 100644 --- a/source/extensions/io_socket/user_space/io_handle_impl.h +++ b/source/extensions/io_socket/user_space/io_handle_impl.h @@ -49,6 +49,7 @@ class IoHandleImpl final : public Network::IoHandle, } Api::IoCallUint64Result close() override; bool isOpen() const override; + bool wasConnected() const override; Api::IoCallUint64Result readv(uint64_t max_length, Buffer::RawSlice* slices, uint64_t num_slice) override; Api::IoCallUint64Result read(Buffer::Instance& buffer, diff --git a/test/common/http/conn_pool_grid_test.cc b/test/common/http/conn_pool_grid_test.cc index dd14534da4a61..3a5606ffc3167 100644 --- a/test/common/http/conn_pool_grid_test.cc +++ b/test/common/http/conn_pool_grid_test.cc @@ -1413,24 +1413,8 @@ TEST_F(ConnectivityGridTest, ConnectionCloseDuringAysnConnect) { ASSERT_TRUE(pool != nullptr); EXPECT_EQ("HTTP/3", pool->protocolDescription()); - const bool supports_getifaddrs = Api::OsSysCallsSingleton::get().supportsGetifaddrs(); - Api::InterfaceAddressVector interfaces{}; - if (supports_getifaddrs) { - ASSERT_EQ(0, Api::OsSysCallsSingleton::get().getifaddrs(interfaces).return_value_); - } - NiceMock os_sys_calls; TestThreadsafeSingletonInjector os_calls(&os_sys_calls); - EXPECT_CALL(os_sys_calls, supportsGetifaddrs()).WillOnce(Return(supports_getifaddrs)); - if (supports_getifaddrs) { - EXPECT_CALL(os_sys_calls, getifaddrs(_)) - .WillOnce( - Invoke([&](Api::InterfaceAddressVector& interface_vector) -> Api::SysCallIntResult { - interface_vector.insert(interface_vector.begin(), interfaces.begin(), - interfaces.end()); - return {0, 0}; - })); - } EXPECT_CALL(os_sys_calls, socket(_, _, _)).WillOnce(Return(Api::SysCallSocketResult{1, 0})); #if defined(__APPLE__) || defined(WIN32) EXPECT_CALL(os_sys_calls, setsocketblocking(1, false)) @@ -1439,7 +1423,16 @@ TEST_F(ConnectivityGridTest, ConnectionCloseDuringAysnConnect) { EXPECT_CALL(os_sys_calls, setsockopt_(_, _, _, _, _)) .Times(testing::AtLeast(0u)) .WillRepeatedly(Return(0)); - EXPECT_CALL(os_sys_calls, bind(_, _, _)).WillOnce(Return(Api::SysCallIntResult{1, 0})); + EXPECT_CALL(os_sys_calls, connect(_, _, _)).WillOnce(Return(Api::SysCallIntResult{1, 0})); + EXPECT_CALL(os_sys_calls, getsockname(_, _, _)) + .WillOnce(Invoke([](os_fd_t, sockaddr* addr, socklen_t* addrlen) -> Api::SysCallIntResult { + sockaddr_in* addr_in = reinterpret_cast(addr); + addr_in->sin_family = AF_INET; + addr_in->sin_port = 0; + inet_pton(AF_INET, "127.0.0.1", &addr_in->sin_addr.s_addr); + *addrlen = sizeof(sockaddr_in); + return Api::SysCallIntResult{0, 0}; + })); EXPECT_CALL(os_sys_calls, setsockopt_(_, _, _, _, _)).WillRepeatedly(Return(0)); auto* async_connect_callback = new NiceMock(&dispatcher_); @@ -1448,7 +1441,8 @@ TEST_F(ConnectivityGridTest, ConnectionCloseDuringAysnConnect) { /*can_use_http3_=*/true}); EXPECT_NE(nullptr, cancel); - EXPECT_CALL(os_sys_calls, sendmsg(_, _, _)).WillOnce(Return(Api::SysCallSizeResult{-1, 101})); + // When there is only 1 slice, the IoHandle's writev method will call the `send` system call. + EXPECT_CALL(os_sys_calls, send(_, _, _, _)).WillOnce(Return(Api::SysCallSizeResult{-1, 101})); EXPECT_CALL(callbacks_.pool_failure_, ready()); async_connect_callback->invokeCallback(); } diff --git a/test/common/quic/client_connection_factory_impl_test.cc b/test/common/quic/client_connection_factory_impl_test.cc index 5ff69be5455a7..455a819bcbafd 100644 --- a/test/common/quic/client_connection_factory_impl_test.cc +++ b/test/common/quic/client_connection_factory_impl_test.cc @@ -23,6 +23,8 @@ using testing::Return; namespace Envoy { namespace Quic { +constexpr int PEER_PORT = 54321; + class QuicNetworkConnectionTest : public Event::TestUsingSimulatedTime, public testing::TestWithParam { protected: @@ -60,8 +62,8 @@ class QuicNetworkConnectionTest : public Event::TestUsingSimulatedTime, } EXPECT_EQ(quic_ccopts, "6RTOAKD4"); - test_address_ = *Network::Utility::resolveUrl( - absl::StrCat("tcp://", Network::Test::getLoopbackAddressUrlString(GetParam()), ":30")); + test_address_ = *Network::Utility::resolveUrl(absl::StrCat( + "tcp://", Network::Test::getLoopbackAddressUrlString(GetParam()), ":", PEER_PORT)); Ssl::ClientContextSharedPtr context{new Ssl::MockClientContext()}; EXPECT_CALL(context_.context_manager_, createSslClientContext(_, _)).WillOnce(Return(context)); factory_ = *Quic::QuicClientTransportSocketFactory::create( @@ -94,10 +96,9 @@ class QuicNetworkConnectionTest : public Event::TestUsingSimulatedTime, TEST_P(QuicNetworkConnectionTest, BufferLimits) { initialize(); - const int port = 30; std::unique_ptr client_connection = createQuicNetworkConnection( *quic_info_, crypto_config_, - quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), port, false}, + quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT, false}, dispatcher_, test_address_, test_address_, quic_stat_names_, {}, *store_.rootScope(), nullptr, nullptr, connection_id_generator_, *factory_); EnvoyQuicClientSession* session = static_cast(client_connection.get()); @@ -118,14 +119,13 @@ TEST_P(QuicNetworkConnectionTest, SocketOptions) { auto socket_option = std::make_shared(); auto socket_options = std::make_shared(); socket_options->push_back(socket_option); - const int port = 30; EXPECT_CALL(*socket_option, setOption(_, envoy::config::core::v3::SocketOption::STATE_PREBIND)); EXPECT_CALL(*socket_option, setOption(_, envoy::config::core::v3::SocketOption::STATE_BOUND)); EXPECT_CALL(*socket_option, setOption(_, envoy::config::core::v3::SocketOption::STATE_LISTENING)); std::unique_ptr client_connection = createQuicNetworkConnection( *quic_info_, crypto_config_, - quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), port, false}, + quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT, false}, dispatcher_, test_address_, test_address_, quic_stat_names_, {}, *store_.rootScope(), socket_options, nullptr, connection_id_generator_, *factory_); EnvoyQuicClientSession* session = static_cast(client_connection.get()); @@ -134,6 +134,26 @@ TEST_P(QuicNetworkConnectionTest, SocketOptions) { client_connection->close(Network::ConnectionCloseType::NoFlush); } +TEST_P(QuicNetworkConnectionTest, LocalAddress) { + initialize(); + Network::Address::InstanceConstSharedPtr local_addr = + (GetParam() == Network::Address::IpVersion::v6) + ? Network::Utility::getIpv6LoopbackAddress() + : Network::Utility::getCanonicalIpv4LoopbackAddress(); + std::unique_ptr client_connection = createQuicNetworkConnection( + *quic_info_, crypto_config_, + quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT, false}, + dispatcher_, test_address_, local_addr, quic_stat_names_, {}, *store_.rootScope(), nullptr, + nullptr, connection_id_generator_, *factory_); + EnvoyQuicClientSession* session = static_cast(client_connection.get()); + session->Initialize(); + client_connection->connect(); + EXPECT_TRUE(client_connection->connecting()); + EXPECT_EQ(Network::Connection::State::Open, client_connection->state()); + EXPECT_THAT(client_connection->connectionInfoProvider().localAddress(), testing::NotNull()); + client_connection->close(Network::ConnectionCloseType::NoFlush); +} + TEST_P(QuicNetworkConnectionTest, Srtt) { initialize(); @@ -142,10 +162,9 @@ TEST_P(QuicNetworkConnectionTest, Srtt) { EXPECT_CALL(rtt_cache, getSrtt).WillOnce(Return(std::chrono::microseconds(5))); - const int port = 30; std::unique_ptr client_connection = createQuicNetworkConnection( info, crypto_config_, - quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), port, false}, + quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT, false}, dispatcher_, test_address_, test_address_, quic_stat_names_, rtt_cache, *store_.rootScope(), nullptr, nullptr, connection_id_generator_, *factory_); diff --git a/test/common/quic/envoy_quic_client_session_test.cc b/test/common/quic/envoy_quic_client_session_test.cc index 258bbe0f85b59..0b03d78a5e9d5 100644 --- a/test/common/quic/envoy_quic_client_session_test.cc +++ b/test/common/quic/envoy_quic_client_session_test.cc @@ -100,13 +100,16 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam(peer_addr_, /*options=*/nullptr, + /*bind=*/true)), crypto_config_(std::make_shared( quic::test::crypto_test_utils::ProofVerifierForTesting())), quic_stat_names_(store_.symbolTable()), transport_socket_options_(std::make_shared()), stats_({ALL_HTTP3_CODEC_STATS(POOL_COUNTER_PREFIX(store_, "http3."), POOL_GAUGE_PREFIX(store_, "http3."))}) { + // After binding the listen peer socket, set the bound IP address of the peer. + peer_addr_ = peer_socket_->connectionInfoProvider().localAddress(); http3_options_.mutable_quic_protocol_options() ->mutable_num_timeouts_to_trigger_port_migration() ->set_value(1); @@ -187,7 +190,7 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam("127.0.0.1"); + Network::Address::InstanceConstSharedPtr peer_addr = + std::make_shared("127.0.0.1", 54321, nullptr); + auto connection_socket = createConnectionSocket(peer_addr, local_addr, nullptr); + EXPECT_TRUE(connection_socket->isOpen()); + EXPECT_TRUE(connection_socket->ioHandle().wasConnected()); + connection_socket->close(); + + Network::Address::InstanceConstSharedPtr no_local_addr = nullptr; + connection_socket = createConnectionSocket(peer_addr, no_local_addr, nullptr); + EXPECT_TRUE(connection_socket->isOpen()); + EXPECT_TRUE(connection_socket->ioHandle().wasConnected()); + EXPECT_EQ("127.0.0.1", no_local_addr->ip()->addressAsString()); + connection_socket->close(); + + Network::Address::InstanceConstSharedPtr local_addr_v6 = + std::make_shared("::1"); + Network::Address::InstanceConstSharedPtr peer_addr_v6 = + std::make_shared("::1", 54321, nullptr); + connection_socket = createConnectionSocket(peer_addr_v6, local_addr_v6, nullptr); + EXPECT_TRUE(connection_socket->isOpen()); + EXPECT_TRUE(connection_socket->ioHandle().wasConnected()); + connection_socket->close(); + + Network::Address::InstanceConstSharedPtr no_local_addr_v6 = nullptr; + connection_socket = createConnectionSocket(peer_addr_v6, no_local_addr_v6, nullptr); + EXPECT_TRUE(connection_socket->isOpen()); + EXPECT_TRUE(connection_socket->ioHandle().wasConnected()); + EXPECT_EQ("::1", no_local_addr_v6->ip()->addressAsString()); + connection_socket->close(); +} + } // namespace Quic } // namespace Envoy diff --git a/test/extensions/filters/udp/udp_proxy/udp_proxy_filter_test.cc b/test/extensions/filters/udp/udp_proxy/udp_proxy_filter_test.cc index 69ec1199f3eaa..f887a1831c367 100644 --- a/test/extensions/filters/udp/udp_proxy/udp_proxy_filter_test.cc +++ b/test/extensions/filters/udp/udp_proxy/udp_proxy_filter_test.cc @@ -116,28 +116,46 @@ class UdpProxyFilterTest : public UdpProxyFilterBase { EXPECT_CALL(*socket_->io_handle_, connect(_)).Times(0); } if (!connect_sys_errno) { - EXPECT_CALL(*socket_->io_handle_, sendmsg(_, 1, 0, _, _)) - .WillOnce(Invoke( - [this, data, local_ip, sys_errno]( - const Buffer::RawSlice* slices, uint64_t, int, - const Network::Address::Ip* self_ip, - const Network::Address::Instance& peer_address) -> Api::IoCallUint64Result { - EXPECT_EQ(data, absl::string_view(static_cast(slices[0].mem_), - slices[0].len_)); - EXPECT_EQ(peer_address, *upstream_address_); - if (local_ip == nullptr) { - EXPECT_EQ(nullptr, self_ip); - } else { - EXPECT_EQ(self_ip->addressAsString(), local_ip->addressAsString()); - } - // For suppression of clang-tidy NewDeleteLeaks rule, don't use the ternary - // operator. - if (sys_errno == 0) { - return makeNoError(data.size()); - } else { - return makeError(sys_errno); - } - })); + if (expect_connect) { + EXPECT_CALL(*socket_->io_handle_, wasConnected()).WillOnce(Return(true)); + EXPECT_CALL(*socket_->io_handle_, writev(_, 1)) + .WillOnce(Invoke([data, sys_errno](const Buffer::RawSlice* slices, + uint64_t) -> Api::IoCallUint64Result { + EXPECT_EQ(data, absl::string_view(static_cast(slices[0].mem_), + slices[0].len_)); + // For suppression of clang-tidy NewDeleteLeaks rule, don't use the ternary + // operator. + if (sys_errno == 0) { + return makeNoError(data.size()); + } else { + return makeError(sys_errno); + } + })); + } else { + EXPECT_CALL(*socket_->io_handle_, wasConnected()).WillOnce(Return(false)); + EXPECT_CALL(*socket_->io_handle_, sendmsg(_, 1, 0, _, _)) + .WillOnce(Invoke( + [this, data, local_ip, sys_errno]( + const Buffer::RawSlice* slices, uint64_t, int, + const Network::Address::Ip* self_ip, + const Network::Address::Instance& peer_address) -> Api::IoCallUint64Result { + EXPECT_EQ(data, absl::string_view(static_cast(slices[0].mem_), + slices[0].len_)); + EXPECT_EQ(peer_address, *upstream_address_); + if (local_ip == nullptr) { + EXPECT_EQ(nullptr, self_ip); + } else { + EXPECT_EQ(self_ip->addressAsString(), local_ip->addressAsString()); + } + // For suppression of clang-tidy NewDeleteLeaks rule, don't use the ternary + // operator. + if (sys_errno == 0) { + return makeNoError(data.size()); + } else { + return makeError(sys_errno); + } + })); + } } } diff --git a/test/extensions/upstreams/http/udp/upstream_request_test.cc b/test/extensions/upstreams/http/udp/upstream_request_test.cc index 42e2e81e8bb22..216d4e1fe31ed 100644 --- a/test/extensions/upstreams/http/udp/upstream_request_test.cc +++ b/test/extensions/upstreams/http/udp/upstream_request_test.cc @@ -83,10 +83,9 @@ TEST_F(UdpUpstreamTest, ExchangeCapsules) { "a1a2a3a4a5a6a7" // UDP Proxying Payload ); Buffer::OwnedImpl sent_capsule(sent_capsule_fragment); - EXPECT_CALL(*mock_socket_->io_handle_, sendmsg(_, _, _, _, _)) - .WillOnce([](const Buffer::RawSlice* slices, uint64_t num_slice, int /*flags*/, - const Network::Address::Ip* /*self_ip*/, - const Network::Address::Instance& /*peer_address*/) { + EXPECT_CALL(*mock_socket_->io_handle_, wasConnected()).WillOnce(Return(true)); + EXPECT_CALL(*mock_socket_->io_handle_, writev(_, _)) + .WillOnce([](const Buffer::RawSlice* slices, uint64_t num_slice) { Buffer::OwnedImpl buffer(absl::HexStringToBytes("a1a2a3a4a5a6a7")); EXPECT_TRUE(TestUtility::rawSlicesEqual(buffer.getRawSlices().data(), slices, num_slice)); return Api::ioCallUint64ResultNoError(); diff --git a/test/integration/filters/test_socket_interface.cc b/test/integration/filters/test_socket_interface.cc index 9099a770072b4..cda8afa719471 100644 --- a/test/integration/filters/test_socket_interface.cc +++ b/test/integration/filters/test_socket_interface.cc @@ -52,7 +52,7 @@ Api::IoCallUint64Result TestIoSocketHandle::writev(const Buffer::RawSlice* slice Address::InstanceConstSharedPtr dnat_peer_address; if (write_override_) { auto result = write_override_(this, slices, num_slice, dnat_peer_address); - ENVOY_BUG(dnat_peer_address == nullptr, "Only works for sendmsg, not writev"); + peer_address_override_.reset(); if (result.has_value()) { return std::move(result).value(); } @@ -83,9 +83,10 @@ IoHandlePtr TestIoSocketHandle::duplicate() { Api::SysCallIntResult TestIoSocketHandle::connect(Address::InstanceConstSharedPtr address) { if (connect_override_) { - auto result = connect_override_(this); - if (result.has_value()) + auto result = connect_override_(this, address); + if (result.has_value()) { return Api::SysCallIntResult{-1, EINPROGRESS}; + } } return Test::IoSocketHandlePlatformImpl::connect(address); diff --git a/test/integration/filters/test_socket_interface.h b/test/integration/filters/test_socket_interface.h index c1905c62f7923..8f4488d7b728f 100644 --- a/test/integration/filters/test_socket_interface.h +++ b/test/integration/filters/test_socket_interface.h @@ -27,8 +27,8 @@ class TestIoSocketHandle : public Test::IoSocketHandlePlatformImpl { Address::InstanceConstSharedPtr& peer_address_override_out); using WriteOverrideProc = std::function; using ReadOverrideProc = std::function; - using ConnectOverrideProc = - std::function(TestIoSocketHandle* io_handle)>; + using ConnectOverrideProc = std::function( + TestIoSocketHandle* io_handle, Address::InstanceConstSharedPtr& peer_address_override_out)>; TestIoSocketHandle(ConnectOverrideProc connect_override_proc, WriteOverrideProc write_override_proc, ReadOverrideProc read_override_proc, diff --git a/test/integration/quic_http_integration_test.cc b/test/integration/quic_http_integration_test.cc index 9a39068e7e752..e431aa45828e3 100644 --- a/test/integration/quic_http_integration_test.cc +++ b/test/integration/quic_http_integration_test.cc @@ -2101,7 +2101,7 @@ TEST_P(QuicHttpIntegrationSPATest, UsesPreferredAddressDNAT) { } auto listener_port = lookupPort("http"); - // Setup DNAT for 0.0.0.0:12345-->127.0.0.2:listener_port + // Setup DNAT for 1.2.3.4:12345-->127.0.0.2:listener_port socket_swap.write_matcher_->setDnat( Network::Utility::parseInternetAddressNoThrow("1.2.3.4", 12345), Network::Utility::parseInternetAddressNoThrow("127.0.0.2", listener_port)); diff --git a/test/integration/socket_interface_swap.cc b/test/integration/socket_interface_swap.cc index 7d29f0691a47f..b5bb5e921bf82 100644 --- a/test/integration/socket_interface_swap.cc +++ b/test/integration/socket_interface_swap.cc @@ -5,9 +5,12 @@ namespace Envoy { SocketInterfaceSwap::SocketInterfaceSwap(Network::Socket::Type socket_type) : write_matcher_(std::make_shared(socket_type)), test_socket_interface_loader_(std::make_unique( - [write_matcher = write_matcher_](Envoy::Network::TestIoSocketHandle* io_handle) + [write_matcher = + write_matcher_](Envoy::Network::TestIoSocketHandle* io_handle, + Network::Address::InstanceConstSharedPtr& peer_address_override_out) -> absl::optional { - Api::IoErrorPtr error_override = write_matcher->returnConnectOverride(io_handle); + Api::IoErrorPtr error_override = + write_matcher->returnConnectOverride(io_handle, peer_address_override_out); if (error_override) { return Api::IoCallUint64Result(0, std::move(error_override)); } diff --git a/test/integration/socket_interface_swap.h b/test/integration/socket_interface_swap.h index 1e16aaaf61565..a35981a799e7d 100644 --- a/test/integration/socket_interface_swap.h +++ b/test/integration/socket_interface_swap.h @@ -39,13 +39,22 @@ class SocketInterfaceSwap { return Api::IoError::none(); } - Api::IoErrorPtr returnConnectOverride(Envoy::Network::TestIoSocketHandle* io_handle) { + Api::IoErrorPtr + returnConnectOverride(Envoy::Network::TestIoSocketHandle* io_handle, + Network::Address::InstanceConstSharedPtr& peer_address_override_out) { absl::MutexLock lock(&mutex_); if (block_connect_ && socket_type_ == io_handle->getSocketType() && (io_handle->localAddress()->ip()->port() == src_port_ || (dst_port_ && io_handle->peerAddress()->ip()->port() == dst_port_))) { return Network::IoSocketError::getIoSocketEagainError(); } + + if (orig_dnat_address_ != nullptr && peer_address_override_out != nullptr && + *orig_dnat_address_ == *peer_address_override_out) { + ASSERT(translated_dnat_address_ != nullptr); + peer_address_override_out = translated_dnat_address_; + } + return Api::IoError::none(); } diff --git a/test/mocks/network/io_handle.h b/test/mocks/network/io_handle.h index 4fac8500e2361..8830e75584e4b 100644 --- a/test/mocks/network/io_handle.h +++ b/test/mocks/network/io_handle.h @@ -22,6 +22,7 @@ class MockIoHandle : public IoHandle { MOCK_METHOD(os_fd_t, fdDoNotUse, (), (const)); MOCK_METHOD(Api::IoCallUint64Result, close, ()); MOCK_METHOD(bool, isOpen, (), (const)); + MOCK_METHOD(bool, wasConnected, (), (const)); MOCK_METHOD(Api::IoCallUint64Result, readv, (uint64_t max_length, Buffer::RawSlice* slices, uint64_t num_slice)); MOCK_METHOD(Api::IoCallUint64Result, read,