Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,13 @@ message HttpProtocolOptions {
config.core.v3.Http1ProtocolOptions http_protocol_options = 1;

config.core.v3.Http2ProtocolOptions http2_protocol_options = 2;

// [#not-implemented-hide:]
// Unlike HTTP/1 and HTTP/2, HTTP/3 will not be configured unless it is
// present. If HTTP/3 is present, attempts to connect will first be made
// via HTTP/3, and if the HTTP/3 connection fails, TCP (HTTP/1 or HTTP/2
// based on ALPN) will be used instead.
config.core.v3.Http3ProtocolOptions http3_protocol_options = 3;
}

// This contains options common across HTTP/1 and HTTP/2
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 2 additions & 14 deletions source/common/http/conn_pool_grid.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,6 @@
namespace Envoy {
namespace Http {

// Helper function to make sure each protocol in expected_protocols is present
// in protocols (only used for an ASSERT in debug builds)
bool contains(const std::vector<Http::Protocol>& protocols,
const std::vector<Http::Protocol>& expected_protocols) {
for (auto protocol : expected_protocols) {
if (std::find(protocols.begin(), protocols.end(), protocol) == protocols.end()) {
return false;
}
}
return true;
}

absl::string_view describePool(const ConnectionPool::Instance& pool) {
return pool.protocolDescription();
}
Expand Down Expand Up @@ -157,10 +145,10 @@ ConnectivityGrid::ConnectivityGrid(
: dispatcher_(dispatcher), random_generator_(random_generator), host_(host),
priority_(priority), options_(options), transport_socket_options_(transport_socket_options),
state_(state), next_attempt_duration_(next_attempt_duration), time_source_(time_source) {
// ProdClusterManagerFactory::allocateConnPool verifies the protocols are HTTP/1, HTTP/2 and
// HTTP/3.
// TODO(#15649) support v6/v4, WiFi/cellular.
ASSERT(connectivity_options.protocols_.size() == 3);
ASSERT(contains(connectivity_options.protocols_,
{Http::Protocol::Http11, Http::Protocol::Http2, Http::Protocol::Http3}));
}

ConnectivityGrid::~ConnectivityGrid() {
Expand Down
1 change: 1 addition & 0 deletions source/common/quic/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ envoy_cc_library(
"//source/common/common:assert_lib",
"//source/extensions/transport_sockets:well_known_names",
"//source/extensions/transport_sockets/tls:context_config_lib",
"//source/extensions/transport_sockets/tls:ssl_socket_lib",
"@envoy_api//envoy/extensions/transport_sockets/quic/v3:pkg_cc_proto",
],
)
Expand Down
14 changes: 11 additions & 3 deletions source/common/quic/quic_transport_socket_factory.cc
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
#include "common/quic/quic_transport_socket_factory.h"

// #include "envoy/extensions/transport_sockets/tls/v3/tls.pb.validate.h"
#include "envoy/extensions/transport_sockets/quic/v3/quic_transport.pb.h"
#include "envoy/extensions/transport_sockets/quic/v3/quic_transport.pb.validate.h"

Comment thread
alyssawilk marked this conversation as resolved.
#include "extensions/transport_sockets/tls/context_config_impl.h"

namespace Envoy {
Expand Down Expand Up @@ -34,7 +33,16 @@ QuicClientTransportSocketConfigFactory::createTransportSocketFactory(
config, context.messageValidationVisitor());
auto client_config = std::make_unique<Extensions::TransportSockets::Tls::ClientContextConfigImpl>(
quic_transport.upstream_tls_context(), context);
return std::make_unique<QuicClientTransportSocketFactory>(std::move(client_config));
return std::make_unique<QuicClientTransportSocketFactory>(std::move(client_config), context);
}

void QuicClientTransportSocketFactory::maybeCreateFallbackFactory() {
if (fallback_factory_) {
return;
}

fallback_factory_ = std::make_unique<Extensions::TransportSockets::Tls::ClientSslSocketFactory>(
std::move(config_), manager_, scope_);
}

ProtobufTypes::MessagePtr QuicClientTransportSocketConfigFactory::createEmptyConfigProto() {
Expand Down
39 changes: 35 additions & 4 deletions source/common/quic/quic_transport_socket_factory.h
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
#pragma once

#include "envoy/extensions/transport_sockets/quic/v3/quic_transport.pb.h"
#include "envoy/network/transport_socket.h"
#include "envoy/server/transport_socket_config.h"
#include "envoy/ssl/context_config.h"

#include "common/common/assert.h"

#include "extensions/transport_sockets/tls/ssl_socket.h"
#include "extensions/transport_sockets/well_known_names.h"

namespace Envoy {
Expand All @@ -25,6 +27,7 @@ class QuicTransportSocketFactoryBase : public Network::TransportSocketFactory {
}
bool implementsSecureTransport() const override { return true; }
bool usesProxyProtocolOptions() const override { return false; }
bool supportsAlpn() const override { return true; }
};

// TODO(danzh): when implement ProofSource, examine of it's necessary to
Expand All @@ -42,13 +45,41 @@ class QuicServerTransportSocketFactory : public QuicTransportSocketFactoryBase {

class QuicClientTransportSocketFactory : public QuicTransportSocketFactoryBase {
public:
QuicClientTransportSocketFactory(Envoy::Ssl::ClientContextConfigPtr config)
: config_(std::move(config)) {}
QuicClientTransportSocketFactory(
Ssl::ClientContextConfigPtr config,
Server::Configuration::TransportSocketFactoryContext& factory_context)
: config_(std::move(config)), manager_(factory_context.sslContextManager()),
scope_(factory_context.scope()) {}

// As documented above for QuicTransportSocketFactoryBase, the actual HTTP/3
// code does not create transport sockets.
// QuicClientTransportSocketFactory::createTransportSocket is called by the
// connection grid when upstream HTTP/3 fails over to TCP, and a raw SSL socket
// is needed. In this case the QuicClientTransportSocketFactory falls over to
// using the fallback factory.
Network::TransportSocketPtr
createTransportSocket(Network::TransportSocketOptionsSharedPtr options) const override {
// TODO(#15649) make createTransportSocket non-const to avoid the const cast.
const_cast<QuicClientTransportSocketFactory*>(this)->maybeCreateFallbackFactory();

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.

Are factories per worker or can createTransportSocket be called concurrently by multiple theads? If the later, there's a thread race in this code.

I would consider creating this fallback factory in QuicClientTransportSocketFactory's constructor in order to guarantee that it always exists if createTransportSocket is ever called.

return fallback_factory_->createTransportSocket(options);
}

const Ssl::ClientContextConfig& clientContextConfig() const { return *config_; }
// Creates fallback_factory_ if it does not already exist.
void maybeCreateFallbackFactory();

// Generally the QuicClientTransportSocketFactory owns its own client context
// config, but in the case of failover to TCP, it hands it off to the fallback_factory_
const Ssl::ClientContextConfig& clientContextConfig() const {
return fallback_factory_ ? fallback_factory_->config() : *config_;
}

private:
std::unique_ptr<const Ssl::ClientContextConfig> config_;
Envoy::Ssl::ClientContextConfigPtr config_;
Envoy::Ssl::ContextManager& manager_;
Stats::Scope& scope_;
envoy::extensions::transport_sockets::quic::v3::QuicUpstreamTransport quic_config_;
// The QUIC client transport socket can create TLS sockets for fallback to TCP.
std::unique_ptr<Extensions::TransportSockets::Tls::ClientSslSocketFactory> fallback_factory_;
};

// Base class to create above QuicTransportSocketFactory for server and client
Expand Down
1 change: 1 addition & 0 deletions source/common/upstream/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ envoy_cc_library(
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
] + envoy_select_enable_http3([
"//source/common/http/http3:conn_pool_lib",
"//source/common/http:conn_pool_grid",
]),
)

Expand Down
29 changes: 27 additions & 2 deletions source/common/upstream/cluster_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include "common/upstream/subset_lb.h"

#ifdef ENVOY_ENABLE_QUIC
#include "common/http/conn_pool_grid.h"
#include "common/http/http3/conn_pool.h"
#endif

Expand All @@ -59,6 +60,18 @@ void addOptionsIfNotNull(Network::Socket::OptionsSharedPtr& options,
}
}

// Helper function to make sure each protocol in expected_protocols is present
// in protocols (only used for an ASSERT in debug builds)
bool contains(const std::vector<Http::Protocol>& protocols,
const std::vector<Http::Protocol>& expected_protocols) {
for (auto protocol : expected_protocols) {
if (std::find(protocols.begin(), protocols.end(), protocol) == protocols.end()) {
return false;
}
}
return true;
}

} // namespace

void ClusterManagerInitHelper::addCluster(ClusterManagerCluster& cm_cluster) {
Expand Down Expand Up @@ -1505,14 +1518,26 @@ Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool(
const Network::ConnectionSocket::OptionsSharedPtr& options,
const Network::TransportSocketOptionsSharedPtr& transport_socket_options, TimeSource& source,
ClusterConnectivityState& state) {
if (protocols.size() == 2) {
if (protocols.size() == 3 && runtime_.snapshot().featureEnabled("upstream.use_http3", 100)) {
Comment thread
alyssawilk marked this conversation as resolved.
ASSERT(contains(protocols,
{Http::Protocol::Http11, Http::Protocol::Http2, Http::Protocol::Http3}));
#ifdef ENVOY_ENABLE_QUIC
Envoy::Http::ConnectivityGrid::ConnectivityOptions coptions{protocols};
return std::make_unique<Http::ConnectivityGrid>(
dispatcher, api_.randomGenerator(), host, priority, options, transport_socket_options,
state, source, std::chrono::milliseconds(300), coptions);

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.

Curious, are there plans to make next_attempt_duration configurable in the future?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, I figured I'd make @RyanTheOptimist take that on both as a simple API change, and to reduce bikeshedding on this PR

#else
// Should be blocked by configuration checking at an earlier point.
NOT_REACHED_GCOVR_EXCL_LINE;
#endif
}
if (protocols.size() >= 2) {
Comment thread
alyssawilk marked this conversation as resolved.
ASSERT((protocols[0] == Http::Protocol::Http2 && protocols[1] == Http::Protocol::Http11) ||
(protocols[1] == Http::Protocol::Http2 && protocols[0] == Http::Protocol::Http11));
return std::make_unique<Http::HttpConnPoolImplMixed>(dispatcher, api_.randomGenerator(), host,
priority, options,
transport_socket_options, state);
}

if (protocols.size() == 1 && protocols[0] == Http::Protocol::Http2 &&
runtime_.snapshot().featureEnabled("upstream.use_http2", 100)) {
return Http::Http2::allocateConnPool(dispatcher, api_.randomGenerator(), host, priority,
Expand Down
21 changes: 13 additions & 8 deletions source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -925,16 +925,21 @@ ClusterInfoImpl::upstreamHttpProtocol(absl::optional<Http::Protocol> downstream_
if (downstream_protocol.has_value() &&
features_ & Upstream::ClusterInfo::Features::USE_DOWNSTREAM_PROTOCOL) {
return {downstream_protocol.value()};
} else if (features_ & Upstream::ClusterInfo::Features::USE_ALPN) {
ASSERT(!(features_ & Upstream::ClusterInfo::Features::HTTP3));
return {Http::Protocol::Http2, Http::Protocol::Http11};
} else {
if (features_ & Upstream::ClusterInfo::Features::HTTP3) {
return {Http::Protocol::Http3};
}

if (features_ & Upstream::ClusterInfo::Features::USE_ALPN) {
if (!(features_ & Upstream::ClusterInfo::Features::HTTP3)) {
return {Http::Protocol::Http2, Http::Protocol::Http11};
}
Comment thread
alyssawilk marked this conversation as resolved.
return {(features_ & Upstream::ClusterInfo::Features::HTTP2) ? Http::Protocol::Http2
: Http::Protocol::Http11};
return {Http::Protocol::Http3, Http::Protocol::Http2, Http::Protocol::Http11};
}

if (features_ & Upstream::ClusterInfo::Features::HTTP3) {
return {Http::Protocol::Http3};
}

return {(features_ & Upstream::ClusterInfo::Features::HTTP2) ? Http::Protocol::Http2
: Http::Protocol::Http11};
}

ClusterImplBase::ClusterImplBase(
Expand Down
2 changes: 2 additions & 0 deletions source/extensions/transport_sockets/tls/ssl_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ class ClientSslSocketFactory : public Network::TransportSocketFactory,
// Secret::SecretCallbacks
void onAddOrUpdateSecret() override;

const Ssl::ClientContextConfig& config() const { return *config_; }

private:
Envoy::Ssl::ContextManager& manager_;
Stats::Scope& stats_scope_;
Expand Down
4 changes: 4 additions & 0 deletions source/extensions/upstreams/http/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ getHttp3Options(const envoy::extensions::upstreams::http::v3::HttpProtocolOption
options.use_downstream_protocol_config().has_http3_protocol_options()) {
return options.use_downstream_protocol_config().http3_protocol_options();
}
if (options.has_auto_config()) {
return options.auto_config().http3_protocol_options();
}
return options.explicit_http_config().http3_protocol_options();
}

Expand Down Expand Up @@ -107,6 +110,7 @@ ProtocolOptionsConfigImpl::ProtocolOptionsConfigImpl(
if (options.has_auto_config()) {
use_http2_ = true;
use_alpn_ = true;
use_http3_ = options.auto_config().has_http3_protocol_options();
}
}

Expand Down
1 change: 1 addition & 0 deletions test/common/upstream/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,7 @@ envoy_cc_test(
"//source/common/upstream:static_cluster_lib",
"//source/common/upstream:strict_dns_cluster_lib",
"//source/extensions/transport_sockets/raw_buffer:config",
"//source/extensions/transport_sockets/tls:config",
"//source/extensions/upstreams/http:config",
"//source/server:transport_socket_config_lib",
"//test/common/stats:stat_test_utility_lib",
Expand Down
Loading