Skip to content
Merged
3 changes: 2 additions & 1 deletion source/common/http/http3/conn_pool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ class Http3ConnPoolImpl : public FixedHttpConnPoolImpl {
}
Network::TransportSocketFactory& transport_socket_factory = host->transportSocketFactory();
quic_info_ = std::make_unique<Quic::PersistentQuicInfoImpl>(
dispatcher, transport_socket_factory, time_source, source_address);
dispatcher, transport_socket_factory, time_source, source_address,
host->cluster().perConnectionBufferLimitBytes());
Quic::configQuicInitialFlowControlWindow(
host_->cluster().http3Options().quic_protocol_options(), quic_info_->quic_config_);
}
Expand Down
11 changes: 5 additions & 6 deletions source/common/quic/client_connection_factory_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,20 @@ getContext(Network::TransportSocketFactory& transport_socket_factory) {
auto* quic_socket_factory =
dynamic_cast<QuicClientTransportSocketFactory*>(&transport_socket_factory);
ASSERT(quic_socket_factory != nullptr);
ASSERT(quic_socket_factory->sslCtx() != nullptr);
return quic_socket_factory->sslCtx();
}

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<uint16_t>(server_addr->ip()->port()), false},
crypto_config_(std::make_unique<quic::QuicCryptoClientConfig>(
std::make_unique<EnvoyQuicProofVerifier>(getContext(transport_socket_factory)),
std::make_unique<EnvoyQuicSessionCache>(time_source))) {
std::make_unique<EnvoyQuicSessionCache>(time_source))),
buffer_limit_(buffer_limit) {
quiche::FlagRegistry::getInstance();
}

Expand Down Expand Up @@ -62,12 +63,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<EnvoyQuicClientSession>(
info_impl->quic_config_, info_impl->supported_versions_, std::move(connection),
info_impl->server_id_, info_impl->crypto_config_.get(), &static_info.push_promise_index_,
dispatcher, /*send_buffer_limit=*/0);
dispatcher, info_impl->buffer_limit_);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind adding some conn_pool tests for this?

return ret;
}

Expand Down
4 changes: 3 additions & 1 deletion source/common/quic/client_connection_factory_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

EnvoyQuicConnectionHelper conn_helper_;
EnvoyQuicAlarmFactory alarm_factory_;
Expand All @@ -32,6 +33,7 @@ struct PersistentQuicInfoImpl : public Http::PersistentQuicInfo {
// be updated with SDS.
std::unique_ptr<quic::QuicCryptoClientConfig> crypto_config_;
quic::QuicConfig quic_config_;
const uint32_t buffer_limit_;
};

std::unique_ptr<Network::ClientConnection>
Expand Down
31 changes: 31 additions & 0 deletions test/common/http/http3/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
load(
"//bazel:envoy_build_system.bzl",
"envoy_cc_test",
"envoy_package",
)

licenses(["notice"]) # Apache 2

envoy_package()

envoy_cc_test(
name = "conn_pool_test",
srcs = ["conn_pool_test.cc"],
deps = [
"//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",
],
)
51 changes: 51 additions & 0 deletions test/common/http/http3/conn_pool_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#include "common/http/http3/conn_pool.h"
#include "common/quic/quic_transport_socket_factory.h"

#include "test/common/upstream/utility.h"
#include "test/mocks/common.h"
#include "test/mocks/event/mocks.h"
#include "test/mocks/server/transport_socket_factory_context.h"
#include "test/mocks/ssl/mocks.h"
#include "test/mocks/upstream/cluster_info.h"
#include "test/mocks/upstream/host.h"
#include "test/test_common/simulated_time_system.h"

namespace Envoy {
namespace Http {
namespace Http3 {

class Http3ConnPoolImplTest : public Event::TestUsingSimulatedTime, public testing::Test {
public:
void initialize() {
EXPECT_CALL(*mock_host, address()).WillRepeatedly(Return(test_address_));
EXPECT_CALL(*mock_host, transportSocketFactory()).WillRepeatedly(testing::ReturnRef(factory_));
new Event::MockSchedulableCallback(&dispatcher_);
Network::ConnectionSocket::OptionsSharedPtr options;
Network::TransportSocketOptionsSharedPtr transport_options;
pool_ = allocateConnPool(dispatcher_, random_, host_, Upstream::ResourcePriority::Default,
options, transport_options, state_, simTime());
}

NiceMock<Event::MockDispatcher> dispatcher_;
std::shared_ptr<Upstream::MockClusterInfo> cluster_{new NiceMock<Upstream::MockClusterInfo>()};
Upstream::MockHost* mock_host = new NiceMock<Upstream::MockHost>;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming convention seems off for this field. Maybe just combine this and host_ into one field and avoid the raw pointer?

Upstream::HostSharedPtr host_{mock_host};
NiceMock<Random::MockRandomGenerator> random_;
Upstream::ClusterConnectivityState state_;
Network::Address::InstanceConstSharedPtr test_address_ =
Network::Utility::resolveUrl("tcp://127.0.0.1:3000");
NiceMock<Server::Configuration::MockTransportSocketFactoryContext> context_;
Quic::QuicClientTransportSocketFactory factory_{
std::unique_ptr<Envoy::Ssl::ClientContextConfig>(new NiceMock<Ssl::MockClientContextConfig>),
context_};
ConnectionPool::InstancePtr pool_;
};

TEST_F(Http3ConnPoolImplTest, CreationWithBufferLimits) {
EXPECT_CALL(mock_host->cluster_, perConnectionBufferLimitBytes);
initialize();
Comment thread
alyssawilk marked this conversation as resolved.
}

} // namespace Http3
} // namespace Http
} // namespace Envoy
2 changes: 1 addition & 1 deletion test/integration/http_integration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<Quic::PersistentQuicInfoImpl>(
*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(
Expand Down
2 changes: 1 addition & 1 deletion test/integration/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt
"spiffe://lyft.com/backend-team");
std::unique_ptr<Http::PersistentQuicInfo> persistent_info;
persistent_info = std::make_unique<Quic::PersistentQuicInfoImpl>(
*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) {
Expand Down