Skip to content

http3: applied cluster buffer limits to http/3 #16466

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.

Sign up for GitHub

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

Merged
alyssawilk merged 16 commits into from
May 25, 2021
Merged

http3: applied cluster buffer limits to http/3 #16466

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
4f47a7f
http3: applied cluster buffer limits to http/3
alyssawilk May 12, 2021
4d67310
format only, no tests
alyssawilk May 12, 2021
eb4b8cc
tests
alyssawilk May 12, 2021
809b70e
Merge branch 'main' into config
alyssawilk May 17, 2021
060eced
whitespace
alyssawilk May 17, 2021
9564a9d
test cleanup
alyssawilk May 17, 2021
e618b0b
test
alyssawilk May 17, 2021
0ed2bc9
buildifier
alyssawilk May 17, 2021
d530af6
test fix
alyssawilk May 18, 2021
97ebb87
comment cleanup
alyssawilk May 18, 2021
56a31b2
Merge branch 'main' into config
alyssawilk May 19, 2021
7ca1e64
Merge branch 'main' into config
alyssawilk May 20, 2021
4032998
fix bad merge
alyssawilk May 20, 2021
bafb1a8
16569 follow-up
alyssawilk May 20, 2021
8ae8ffd
Merge branch 'main' into config
alyssawilk May 20, 2021
4a2a3f8
Merge branch 'main' into config
alyssawilk May 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion source/common/http/http3/conn_pool.cc
View file
Open in desktop
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
10 changes: 5 additions & 5 deletions source/common/quic/client_connection_factory_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@ getContext(Network::TransportSocketFactory& transport_socket_factory) {

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 +64,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_);

@danzh2010 danzh2010 May 12, 2021

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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion test/integration/http_integration.cc
View file
Open in desktop
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
View file
Open in desktop
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