diff --git a/source/common/http/http3/conn_pool.cc b/source/common/http/http3/conn_pool.cc index 40740a32e8ae8..3ff377138a620 100644 --- a/source/common/http/http3/conn_pool.cc +++ b/source/common/http/http3/conn_pool.cc @@ -44,8 +44,9 @@ Http3ConnPoolImpl::Http3ConnPoolImpl( source_address = Network::Utility::getLocalAddress(host_address->ip()->version()); } Network::TransportSocketFactory& transport_socket_factory = host->transportSocketFactory(); - quic_info_ = std::make_unique(dispatcher, transport_socket_factory, - time_source, source_address); + quic_info_ = std::make_unique( + dispatcher, transport_socket_factory, time_source, source_address, + host->cluster().perConnectionBufferLimitBytes()); setQuicConfigFromClusterConfig(host_->cluster(), quic_info_->quic_config_); } diff --git a/source/common/quic/client_connection_factory_impl.cc b/source/common/quic/client_connection_factory_impl.cc index 829ea0ddc67eb..fcc871f382670 100644 --- a/source/common/quic/client_connection_factory_impl.cc +++ b/source/common/quic/client_connection_factory_impl.cc @@ -42,11 +42,13 @@ std::shared_ptr PersistentQuicInfoImpl::cryptoConf PersistentQuicInfoImpl::PersistentQuicInfoImpl( Event::Dispatcher& dispatcher, Network::TransportSocketFactory& transport_socket_factory, - TimeSource& time_source, Network::Address::InstanceConstSharedPtr server_addr) + TimeSource& time_source, Network::Address::InstanceConstSharedPtr server_addr, + uint32_t buffer_limit) : conn_helper_(dispatcher), alarm_factory_(dispatcher, *conn_helper_.GetClock()), server_id_{getConfig(transport_socket_factory).serverNameIndication(), static_cast(server_addr->ip()->port()), false}, - transport_socket_factory_(transport_socket_factory), time_source_(time_source) { + transport_socket_factory_(transport_socket_factory), time_source_(time_source), + buffer_limit_(buffer_limit) { quiche::FlagRegistry::getInstance(); } @@ -69,12 +71,10 @@ createQuicNetworkConnection(Http::PersistentQuicInfo& info, Event::Dispatcher& d ASSERT(!info_impl->supported_versions_.empty()); // QUICHE client session always use the 1st version to start handshake. - // TODO(alyssawilk) pass in ClusterInfo::perConnectionBufferLimitBytes() for - // send_buffer_limit instead of using 0. auto ret = std::make_unique( info_impl->quic_config_, info_impl->supported_versions_, std::move(connection), info_impl->server_id_, std::move(config), &info_impl->push_promise_index_, dispatcher, - /*send_buffer_limit=*/0); + info_impl->buffer_limit_); return ret; } diff --git a/source/common/quic/client_connection_factory_impl.h b/source/common/quic/client_connection_factory_impl.h index ad8334589af36..991249bebee47 100644 --- a/source/common/quic/client_connection_factory_impl.h +++ b/source/common/quic/client_connection_factory_impl.h @@ -20,7 +20,8 @@ struct PersistentQuicInfoImpl : public Http::PersistentQuicInfo { PersistentQuicInfoImpl(Event::Dispatcher& dispatcher, Network::TransportSocketFactory& transport_socket_factory, TimeSource& time_source, - Network::Address::InstanceConstSharedPtr server_addr); + Network::Address::InstanceConstSharedPtr server_addr, + uint32_t buffer_limit); // Returns the most recent crypto config from transport_socket_factory_; std::shared_ptr cryptoConfig(); @@ -42,6 +43,8 @@ struct PersistentQuicInfoImpl : public Http::PersistentQuicInfo { const quic::ParsedQuicVersionVector supported_versions_{quic::CurrentSupportedVersions()}; // TODO(alyssawilk) actually set this up properly. quic::QuicConfig quic_config_; + // The cluster buffer limits. + const uint32_t buffer_limit_; // This arguably should not be shared across connections but as Envoy doesn't // support push promise it's really moot point. quic::QuicClientPushPromiseIndex push_promise_index_; diff --git a/source/common/quic/envoy_quic_proof_verifier.h b/source/common/quic/envoy_quic_proof_verifier.h index f12d570848f01..cdbfc70109b6a 100644 --- a/source/common/quic/envoy_quic_proof_verifier.h +++ b/source/common/quic/envoy_quic_proof_verifier.h @@ -12,7 +12,9 @@ namespace Quic { class EnvoyQuicProofVerifier : public EnvoyQuicProofVerifierBase { public: EnvoyQuicProofVerifier(Envoy::Ssl::ClientContextSharedPtr&& context) - : context_(std::move(context)) {} + : context_(std::move(context)) { + ASSERT(context_.get()); + } // EnvoyQuicProofVerifierBase quic::QuicAsyncStatus diff --git a/source/common/quic/quic_filter_manager_connection_impl.h b/source/common/quic/quic_filter_manager_connection_impl.h index 7922810e362c9..f0f666434c7fa 100644 --- a/source/common/quic/quic_filter_manager_connection_impl.h +++ b/source/common/quic/quic_filter_manager_connection_impl.h @@ -32,6 +32,8 @@ class TestPauseFilterForQuic; namespace Quic { +class QuicNetworkConnectionTest; + // Act as a Network::Connection to HCM and a FilterManager to FilterFactoryCb. class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase, public SendBufferMonitor { @@ -171,6 +173,7 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase, private: friend class Envoy::TestPauseFilterForQuic; + friend class Envoy::Quic::QuicNetworkConnectionTest; // Called when aggregated buffered bytes across all the streams exceeds high watermark. void onSendBufferHighWatermark(); diff --git a/test/common/http/http3/BUILD b/test/common/http/http3/BUILD index dcb9e6549a62d..b2ab320ff0aff 100644 --- a/test/common/http/http3/BUILD +++ b/test/common/http/http3/BUILD @@ -13,22 +13,21 @@ envoy_cc_test( name = "conn_pool_test", srcs = envoy_select_enable_http3(["conn_pool_test.cc"]), tags = ["nofips"], - deps = - envoy_select_enable_http3([ - "//source/common/event:dispatcher_lib", - "//source/common/http/http3:conn_pool_lib", - "//source/common/network:utility_lib", - "//source/common/upstream:upstream_includes", - "//source/common/upstream:upstream_lib", - "//test/common/http:common_lib", - "//test/common/upstream:utility_lib", - "//test/mocks/event:event_mocks", - "//test/mocks/http:http_mocks", - "//test/mocks/network:network_mocks", - "//test/mocks/runtime:runtime_mocks", - "//test/mocks/server:transport_socket_factory_context_mocks", - "//test/mocks/upstream:cluster_info_mocks", - "//test/mocks/upstream:transport_socket_match_mocks", - "//test/test_common:test_runtime_lib", - ]), + deps = envoy_select_enable_http3([ + "//source/common/event:dispatcher_lib", + "//source/common/http/http3:conn_pool_lib", + "//source/common/network:utility_lib", + "//source/common/upstream:upstream_includes", + "//source/common/upstream:upstream_lib", + "//test/common/http:common_lib", + "//test/common/upstream:utility_lib", + "//test/mocks/event:event_mocks", + "//test/mocks/http:http_mocks", + "//test/mocks/network:network_mocks", + "//test/mocks/runtime:runtime_mocks", + "//test/mocks/server:transport_socket_factory_context_mocks", + "//test/mocks/upstream:cluster_info_mocks", + "//test/mocks/upstream:transport_socket_match_mocks", + "//test/test_common:test_runtime_lib", + ]), ) diff --git a/test/common/http/http3/conn_pool_test.cc b/test/common/http/http3/conn_pool_test.cc index 5dccea1dacc41..3c16ae1d6eb78 100644 --- a/test/common/http/http3/conn_pool_test.cc +++ b/test/common/http/http3/conn_pool_test.cc @@ -52,6 +52,7 @@ class Http3ConnPoolImplTest : public Event::TestUsingSimulatedTime, public testi Upstream::MockHost& mockHost() { return static_cast(*host_); } NiceMock dispatcher_; + std::shared_ptr cluster_{new NiceMock()}; Upstream::HostSharedPtr host_{new NiceMock}; NiceMock random_; Upstream::ClusterConnectivityState state_; @@ -64,6 +65,11 @@ class Http3ConnPoolImplTest : public Event::TestUsingSimulatedTime, public testi ConnectionPool::InstancePtr pool_; }; +TEST_F(Http3ConnPoolImplTest, CreationWithBufferLimits) { + EXPECT_CALL(mockHost().cluster_, perConnectionBufferLimitBytes); + initialize(); +} + TEST_F(Http3ConnPoolImplTest, CreationWithConfig) { // Set a couple of options from setQuicConfigFromClusterConfig to make sure they are applied. auto* options = mockHost().cluster_.http3_options_.mutable_quic_protocol_options(); diff --git a/test/common/quic/client_connection_factory_impl_test.cc b/test/common/quic/client_connection_factory_impl_test.cc index 87e6111cc870e..b7d26e2e2f40c 100644 --- a/test/common/quic/client_connection_factory_impl_test.cc +++ b/test/common/quic/client_connection_factory_impl_test.cc @@ -27,6 +27,10 @@ class QuicNetworkConnectionTest : public Event::TestUsingSimulatedTime, public t context_); } + uint32_t highWatermark(EnvoyQuicClientSession* session) { + return session->write_buffer_watermark_simulation_.highWatermark(); + } + NiceMock dispatcher_; std::shared_ptr cluster_{new NiceMock()}; Upstream::HostSharedPtr host_{new NiceMock}; @@ -41,7 +45,7 @@ class QuicNetworkConnectionTest : public Event::TestUsingSimulatedTime, public t TEST_F(QuicNetworkConnectionTest, BufferLimits) { initialize(); - PersistentQuicInfoImpl info{dispatcher_, *factory_, simTime(), test_address_}; + PersistentQuicInfoImpl info{dispatcher_, *factory_, simTime(), test_address_, 45}; std::unique_ptr client_connection = createQuicNetworkConnection(info, dispatcher_, test_address_, test_address_); @@ -50,6 +54,7 @@ TEST_F(QuicNetworkConnectionTest, BufferLimits) { client_connection->connect(); EXPECT_TRUE(client_connection->connecting()); ASSERT(session != nullptr); + EXPECT_EQ(highWatermark(session), 45); EXPECT_EQ(absl::nullopt, session->unixSocketPeerCredentials()); EXPECT_EQ(absl::nullopt, session->lastRoundTripTime()); client_connection->close(Network::ConnectionCloseType::NoFlush); diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 2c225369cb8f4..7522ef79ed9e4 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -341,7 +341,7 @@ void HttpIntegrationTest::initialize() { "udp://{}:{}", Network::Test::getLoopbackAddressUrlString(version_), lookupPort("http"))); // Needs to outlive all QUIC connections. auto quic_connection_persistent_info = std::make_unique( - *dispatcher_, *quic_transport_socket_factory_, timeSystem(), server_addr); + *dispatcher_, *quic_transport_socket_factory_, timeSystem(), server_addr, 0); // Config IETF QUIC flow control window. quic_connection_persistent_info->quic_config_ .SetInitialMaxStreamDataBytesIncomingBidirectionalToSend( diff --git a/test/integration/utility.cc b/test/integration/utility.cc index 7d99f7e4dcc8b..465ba1d2b4942 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -212,7 +212,7 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt "spiffe://lyft.com/backend-team"); std::unique_ptr persistent_info; persistent_info = std::make_unique( - *dispatcher, *transport_socket_factory, time_system, addr); + *dispatcher, *transport_socket_factory, time_system, addr, 0); Network::Address::InstanceConstSharedPtr local_address; if (addr->ip()->version() == Network::Address::IpVersion::v4) {