Skip to content
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.

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

http3: further cleanup and extension removal #15752

Merged
merged 12 commits into from
Mar 31, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;

// [#protodoc-title: quic transport]
// [#comment:#extension: envoy.transport_sockets.quic]
// [#comment:TODO(#12829): Remove this as an extension point.]

// Configuration for Downstream QUIC transport socket. This provides Google's implementation of Google QUIC and IETF QUIC to Envoy.
message QuicDownstreamTransport {
Expand Down

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

8 changes: 8 additions & 0 deletions bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,14 @@ config_setting(
},
)

selects.config_setting_group(
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @lizan can you do a pass of just the bazel magic?

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @keith also

Copy link
Member

Choose a reason for hiding this comment

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

As long as this works it's definitely fine, I'm impressed it does though, we should separately change the apple config to use this instead, looking at the skylib impl it's very hard to parse, but I believe it's doing that same hack https://github.com/bazelbuild/bazel-skylib/blob/f80bc733d4b9f83d427ce3442be2e07427b2cc8d/lib/selects.bzl#L145-L186

Copy link
Member Author

Choose a reason for hiding this comment

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

Believe it or not, I found this trick by googling, where I found the bazel issue you opened about the apple hack, where the person commented it could be done with this thing, but it wasn't merged yet. Now it's merged. :)

Copy link
Member

Choose a reason for hiding this comment

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

Amazing, I forgot this, here's the update for the other ones that can benefit from this #15797

name = "disable_http3_all_reasons",
match_any = [
":disable_http3",
":boringssl_fips",
],
)

config_setting(
name = "disable_hot_restart",
values = {"define": "hot_restart=disabled"},
Expand Down
2 changes: 1 addition & 1 deletion bazel/envoy_select.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def envoy_select_google_grpc(xs, repository = ""):
# Selects the given values if http3 is enabled in the current build.
def envoy_select_enable_http3(xs, repository = ""):
return select({
repository + "//bazel:disable_http3": [],
repository + "//bazel:disable_http3_all_reasons": [],
"//conditions:default": xs,
})

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.

9 changes: 4 additions & 5 deletions source/common/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ load(
"//bazel:envoy_build_system.bzl",
"envoy_cc_library",
"envoy_package",
"envoy_select_enable_http3",
)

licenses(["notice"]) # Apache 2
Expand Down Expand Up @@ -65,10 +66,10 @@ envoy_cc_library(
"//source/common/config:utility_lib",
"//source/common/http/http1:codec_lib",
"//source/common/http/http2:codec_lib",
"//source/common/http/http3:quic_codec_factory_lib",
"//source/common/http/http3:well_known_names",
"//source/common/network:filter_lib",
],
] + envoy_select_enable_http3([
"//source/common/quic:codec_lib",
]),
)

envoy_cc_library(
Expand Down Expand Up @@ -259,8 +260,6 @@ envoy_cc_library(
"//source/common/config:utility_lib",
"//source/common/http/http1:codec_lib",
"//source/common/http/http2:codec_lib",
"//source/common/http/http3:quic_codec_factory_lib",
"//source/common/http/http3:well_known_names",
"//source/common/http/match_wrapper:config",
"//source/common/network:utility_lib",
"//source/common/router:config_lib",
Expand Down
17 changes: 11 additions & 6 deletions source/common/http/codec_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@
#include "common/http/exception.h"
#include "common/http/http1/codec_impl.h"
#include "common/http/http2/codec_impl.h"
#include "common/http/http3/quic_codec_factory.h"
#include "common/http/http3/well_known_names.h"
#include "common/http/status.h"
#include "common/http/utility.h"

#ifdef ENVOY_ENABLE_QUIC
#include "common/quic/codec_impl.h"
#endif

namespace Envoy {
namespace Http {

Expand Down Expand Up @@ -182,11 +184,14 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne
break;
}
case Type::HTTP3: {
codec_ = std::unique_ptr<ClientConnection>(
Config::Utility::getAndCheckFactoryByName<Http::QuicHttpClientConnectionFactory>(
Http::QuicCodecNames::get().Quiche)
.createQuicClientConnection(*connection_, *this));
#ifdef ENVOY_ENABLE_QUIC
codec_ = std::make_unique<Quic::QuicHttpClientConnectionImpl>(
dynamic_cast<Quic::EnvoyQuicClientSession&>(*connection_), *this);
break;
#else
// Should be blocked by configuration checking at an earlier point.
NOT_REACHED_GCOVR_EXCL_LINE;
#endif
}
}
}
Expand Down
23 changes: 1 addition & 22 deletions source/common/http/http3/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,32 +18,11 @@ envoy_cc_library(
"//include/envoy/upstream:upstream_interface",
"//source/common/http:codec_client_lib",
"//source/common/http:conn_pool_base_lib",
],
)

envoy_cc_library(
name = "quic_codec_factory_lib",
hdrs = ["quic_codec_factory.h"],
deps = [
"//include/envoy/config:typed_config_interface",
"//include/envoy/http:codec_interface",
"//include/envoy/network:connection_interface",
"//source/common/quic:client_connection_factory_lib",
],
)

envoy_cc_library(
name = "quic_client_connection_factory_lib",
hdrs = ["quic_client_connection_factory.h"],
deps = [
"//include/envoy/config:typed_config_interface",
"//include/envoy/network:connection_interface",
"//include/envoy/ssl:context_config_interface",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
],
)

envoy_cc_library(
name = "well_known_names",
hdrs = ["well_known_names.h"],
deps = ["//source/common/singleton:const_singleton"],
)
22 changes: 11 additions & 11 deletions source/common/http/http3/conn_pool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,17 @@

#include "common/config/utility.h"
#include "common/http/http3/quic_client_connection_factory.h"
#include "common/http/http3/well_known_names.h"
#include "common/http/utility.h"
#include "common/network/address_impl.h"
#include "common/network/utility.h"
#include "common/runtime/runtime_features.h"

#ifdef ENVOY_ENABLE_QUIC
#include "common/quic/client_connection_factory_impl.h"
#else
#error "http3 conn pool should not be built with QUIC disabled"
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
#endif

namespace Envoy {
namespace Http {
namespace Http3 {
Expand All @@ -36,11 +41,9 @@ class Http3ConnPoolImpl : public FixedHttpConnPoolImpl {
source_address = Network::Utility::getLocalAddress(host_address->ip()->version());
}
Network::TransportSocketFactory& transport_socket_factory = host->transportSocketFactory();
quic_info_ =
Config::Utility::getAndCheckFactoryByName<Http::QuicClientConnectionFactory>(
Http::QuicCodecNames::get().Quiche)
.createNetworkConnectionInfo(dispatcher, transport_socket_factory,
host->cluster().statsScope(), time_source, source_address);
quic_info_ = std::make_unique<Quic::PersistentQuicInfoImpl>(
dispatcher, transport_socket_factory, host->cluster().statsScope(), time_source,
source_address);
}

// Make sure all connections are torn down before quic_info_ is deleted.
Expand Down Expand Up @@ -68,11 +71,8 @@ allocateConnPool(Event::Dispatcher& dispatcher, Random::RandomGenerator& random_
if (!source_address.get()) {
source_address = Network::Utility::getLocalAddress(host_address->ip()->version());
}
data.connection_ =
Config::Utility::getAndCheckFactoryByName<Http::QuicClientConnectionFactory>(
Http::QuicCodecNames::get().Quiche)
.createQuicNetworkConnection(*h3_pool->quic_info_, pool->dispatcher(), host_address,
source_address);
data.connection_ = Quic::createQuicNetworkConnection(
*h3_pool->quic_info_, pool->dispatcher(), host_address, source_address);
return std::make_unique<ActiveClient>(*pool, data);
},
[](Upstream::Host::CreateConnectionData& data, HttpConnPoolImplBase* pool) {
Expand Down
30 changes: 0 additions & 30 deletions source/common/http/http3/quic_client_connection_factory.h
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
#pragma once

#include <string>

#include "envoy/config/core/v3/protocol.pb.h"
#include "envoy/config/typed_config.h"
#include "envoy/network/connection.h"
#include "envoy/ssl/context_config.h"

namespace Envoy {
namespace Http {

Expand All @@ -16,28 +9,5 @@ struct PersistentQuicInfo {
virtual ~PersistentQuicInfo() = default;
};

// A factory to create EnvoyQuicClientSession and EnvoyQuicClientConnection for QUIC
class QuicClientConnectionFactory : public Config::UntypedFactory {
public:
~QuicClientConnectionFactory() override = default;

// Allows thread-local creation of PersistentQuicInfo.
// Each (thread local) connection pool can call createNetworkConnectionInfo to create a
// PersistentQuicInfo, then use that PersistentQuicInfo. when calling createQuicNetworkConnection
// for all the connections in that pool.
virtual std::unique_ptr<PersistentQuicInfo>
createNetworkConnectionInfo(Event::Dispatcher& dispatcher,
Network::TransportSocketFactory& transport_socket_factory,
Stats::Scope& stats_scope, TimeSource& time_source,
Network::Address::InstanceConstSharedPtr server_addr) PURE;

virtual std::unique_ptr<Network::ClientConnection>
createQuicNetworkConnection(PersistentQuicInfo& info, Event::Dispatcher& dispatcher,
Network::Address::InstanceConstSharedPtr server_addr,
Network::Address::InstanceConstSharedPtr local_addr) PURE;

std::string category() const override { return "envoy.quic_connection"; }
};

} // namespace Http
} // namespace Envoy
35 changes: 0 additions & 35 deletions source/common/http/http3/quic_codec_factory.h

This file was deleted.

20 changes: 0 additions & 20 deletions source/common/http/http3/well_known_names.h

This file was deleted.

4 changes: 0 additions & 4 deletions source/common/quic/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,6 @@ envoy_cc_library(
"//include/envoy/http:codec_interface",
"//include/envoy/registry",
"//source/common/http/http3:quic_client_connection_factory_lib",
"//source/common/http/http3:quic_codec_factory_lib",
"//source/common/http/http3:well_known_names",
"//source/extensions/transport_sockets/tls:ssl_socket_lib",
"@com_googlesource_quiche//:quic_core_http_spdy_session_lib",
],
Expand All @@ -170,8 +168,6 @@ envoy_cc_library(
":envoy_quic_utils_lib",
"//include/envoy/http:codec_interface",
"//include/envoy/registry",
"//source/common/http/http3:quic_codec_factory_lib",
"//source/common/http/http3:well_known_names",
"@com_googlesource_quiche//:quic_core_http_spdy_session_lib",
],
)
Expand Down
26 changes: 18 additions & 8 deletions source/common/quic/client_connection_factory_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,21 @@ PersistentQuicInfoImpl::PersistentQuicInfoImpl(
std::make_unique<quic::QuicCryptoClientConfig>(std::make_unique<EnvoyQuicProofVerifier>(
stats_scope, getConfig(transport_socket_factory), time_source))) {}

namespace {
// TODO(mattklein123): This is mutable static info that is required for the QUICHE code. This was
// preexisting but should be removed as we shouldn't be using global mutable static data.
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
struct StaticInfo {
quic::QuicConfig quic_config_;
quic::QuicClientPushPromiseIndex push_promise_index_;

static StaticInfo& get() { MUTABLE_CONSTRUCT_ON_FIRST_USE(StaticInfo); }
};
} // namespace

std::unique_ptr<Network::ClientConnection>
QuicClientConnectionFactoryImpl::createQuicNetworkConnection(
Http::PersistentQuicInfo& info, Event::Dispatcher& dispatcher,
Network::Address::InstanceConstSharedPtr server_addr,
Network::Address::InstanceConstSharedPtr local_addr) {
createQuicNetworkConnection(Http::PersistentQuicInfo& info, Event::Dispatcher& dispatcher,
Network::Address::InstanceConstSharedPtr server_addr,
Network::Address::InstanceConstSharedPtr local_addr) {
// This flag fix a QUICHE issue which may crash Envoy during connection close.
SetQuicReloadableFlag(quic_single_ack_in_packet2, true);
PersistentQuicInfoImpl* info_impl = reinterpret_cast<PersistentQuicInfoImpl*>(&info);
Expand All @@ -37,14 +47,14 @@ QuicClientConnectionFactoryImpl::createQuicNetworkConnection(
quic::QuicUtils::CreateRandomConnectionId(), server_addr, info_impl->conn_helper_,
info_impl->alarm_factory_, quic::ParsedQuicVersionVector{info_impl->supported_versions_[0]},
local_addr, dispatcher, nullptr);
auto& static_info = StaticInfo::get();
auto ret = std::make_unique<EnvoyQuicClientSession>(
quic_config_, info_impl->supported_versions_, std::move(connection), info_impl->server_id_,
info_impl->crypto_config_.get(), &push_promise_index_, dispatcher, 0);
static_info.quic_config_, info_impl->supported_versions_, std::move(connection),
info_impl->server_id_, info_impl->crypto_config_.get(), &static_info.push_promise_index_,
dispatcher, 0);
ret->Initialize();
return ret;
}

REGISTER_FACTORY(QuicClientConnectionFactoryImpl, Http::QuicClientConnectionFactory);

} // namespace Quic
} // namespace Envoy
Loading