Skip to content

Commit

Permalink
quiche: support draft 29 (#18647)
Browse files Browse the repository at this point in the history
Signed-off-by: Dan Zhang [email protected]

Commit Message: add back draft 29 support.

Risk Level: low
Testing: added unit test and integration test
Follow up on #16642
  • Loading branch information
danzh2010 authored Oct 22, 2021
1 parent 7d915a2 commit 4b5b338
Show file tree
Hide file tree
Showing 12 changed files with 74 additions and 32 deletions.
1 change: 1 addition & 0 deletions docs/root/configuration/http/http_conn_man/stats.rst
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ On the upstream side all http3 statistics are rooted at *cluster.<name>.http3.*
rx_reset, Counter, Total number of reset stream frames received by Envoy
tx_reset, Counter, Total number of reset stream frames transmitted by Envoy
metadata_not_supported_error, Counter, Total number of metadata dropped during HTTP/3 encoding
quic_version_h3_29, Counter, Total number of quic connections that use transport version h3-29. QUIC h3-29 is unsupported by default and this counter will be removed when h3-29 support is completely removed.
quic_version_rfc_v1, Counter, Total number of quic connections that use transport version rfc-v1.


Expand Down
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Minor Behavior Changes
* config: the log message for "gRPC config stream closed" now uses the most recent error message, and reports seconds instead of milliseconds for how long the most recent status has been received.
* dns: now respecting the returned DNS TTL for resolved hosts, rather than always relying on the hard-coded :ref:`dns_refresh_rate. <envoy_v3_api_field_config.cluster.v3.Cluster.dns_refresh_rate>` This behavior can be temporarily reverted by setting the runtime guard ``envoy.reloadable_features.use_dns_ttl`` to false.
* listener: destroy per network filter chain stats when a network filter chain is removed during the listener in place update.
* quic: add back the support for IETF draft 29 which is guarded via ``envoy.reloadable_features.FLAGS_quic_reloadable_flag_quic_disable_version_draft_29``. It is off by default so Envoy only supports RFCv1 without flipping this runtime guard explicitly. Draft 29 is not recommended for use.

Bug Fixes
---------
Expand Down
1 change: 1 addition & 0 deletions source/common/http/http3/codec_stats.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace Http3 {
COUNTER(rx_reset) \
COUNTER(tx_reset) \
COUNTER(metadata_not_supported_error) \
COUNTER(quic_version_h3_29) \
COUNTER(quic_version_rfc_v1) \
COUNTER(tx_flush_timeout)

Expand Down
5 changes: 1 addition & 4 deletions source/common/quic/active_quic_listener.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,7 @@ ActiveQuicListener::ActiveQuicListener(
kernel_worker_routing_(kernel_worker_routing),
packets_to_read_to_connection_count_ratio_(packets_to_read_to_connection_count_ratio),
crypto_server_stream_factory_(crypto_server_stream_factory) {
// This flag fix a QUICHE issue which may crash Envoy during connection close.
SetQuicReloadableFlag(quic_single_ack_in_packet2, true);
// Do not include 32-byte per-entry overhead while counting header size.
quiche::FlagRegistry::getInstance();
ASSERT(GetQuicReloadableFlag(quic_single_ack_in_packet2));
ASSERT(!GetQuicFlag(FLAGS_quic_header_size_limit_includes_overhead));

if (Runtime::LoaderSingleton::getExisting()) {
Expand Down
3 changes: 1 addition & 2 deletions source/common/quic/client_connection_factory_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ createQuicNetworkConnection(Http::PersistentQuicInfo& info, Event::Dispatcher& d
Network::Address::InstanceConstSharedPtr server_addr,
Network::Address::InstanceConstSharedPtr local_addr,
QuicStatNames& quic_stat_names, Stats::Scope& scope) {
// This flag fix a QUICHE issue which may crash Envoy during connection close.
SetQuicReloadableFlag(quic_single_ack_in_packet2, true);
ASSERT(GetQuicReloadableFlag(quic_single_ack_in_packet2));
PersistentQuicInfoImpl* info_impl = reinterpret_cast<PersistentQuicInfoImpl*>(&info);
auto config = info_impl->cryptoConfig();
if (config == nullptr) {
Expand Down
8 changes: 0 additions & 8 deletions source/common/quic/envoy_quic_client_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,6 @@ void EnvoyQuicClientSession::OnRstStream(const quic::QuicRstStreamFrame& frame)
/*from_self*/ false, /*is_upstream*/ true);
}

void EnvoyQuicClientSession::SetDefaultEncryptionLevel(quic::EncryptionLevel level) {
quic::QuicSpdyClientSession::SetDefaultEncryptionLevel(level);
if (level == quic::ENCRYPTION_FORWARD_SECURE) {
// This is only reached once, when handshake is done.
raiseConnectionEvent(Network::ConnectionEvent::Connected);
}
}

std::unique_ptr<quic::QuicSpdyClientStream> EnvoyQuicClientSession::CreateClientStream() {
ASSERT(codec_stats_.has_value() && http3_options_.has_value());
return std::make_unique<EnvoyQuicClientStream>(GetNextOutgoingBidirectionalStreamId(), this,
Expand Down
1 change: 0 additions & 1 deletion source/common/quic/envoy_quic_client_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ class EnvoyQuicClientSession : public QuicFilterManagerConnectionImpl,
quic::QuicStreamOffset bytes_written) override;
void OnRstStream(const quic::QuicRstStreamFrame& frame) override;
// quic::QuicSpdyClientSessionBase
void SetDefaultEncryptionLevel(quic::EncryptionLevel level) override;
bool ShouldKeepConnectionAlive() const override;
// quic::ProofHandler
void OnProofVerifyDetailsAvailable(const quic::ProofVerifyDetails& verify_details) override;
Expand Down
5 changes: 4 additions & 1 deletion source/common/quic/platform/quiche_flags_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ absl::flat_hash_map<absl::string_view, Flag*> makeFlagMap() {
QUIC_FLAG(FLAGS_quic_restart_flag_http2_testonly_default_false, false)
QUIC_FLAG(FLAGS_quic_restart_flag_http2_testonly_default_true, true)
#undef QUIC_FLAG
// Disable IETF draft 29 implementation. Envoy only supports RFC-v1.
// Envoy only supports RFC-v1 in the long term, so disable IETF draft 29 implementation by
// default.
FLAGS_quic_reloadable_flag_quic_disable_version_draft_29->setValue(true);
// This flag fixes a QUICHE issue which may crash Envoy during connection close.
FLAGS_quic_reloadable_flag_quic_single_ack_in_packet2->setValue(true);

#define QUIC_PROTOCOL_FLAG(type, flag, ...) flags.emplace(FLAGS_##flag->name(), FLAGS_##flag);
#include "quiche/quic/core/quic_protocol_flags_list.h"
Expand Down
9 changes: 7 additions & 2 deletions source/common/quic/quic_filter_manager_connection_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,14 @@ void QuicFilterManagerConnectionImpl::onConnectionCloseEvent(
// The connection was closed before it could be used. Stats are not recorded.
return;
}
if (version.transport_version == quic::QUIC_VERSION_IETF_RFC_V1) {
switch (version.transport_version) {
case quic::QUIC_VERSION_IETF_DRAFT_29:
codec_stats_->quic_version_h3_29_.inc();
return;
case quic::QUIC_VERSION_IETF_RFC_V1:
codec_stats_->quic_version_rfc_v1_.inc();
} else {
return;
default:
ENVOY_BUG(false, fmt::format("Unexpected QUIC version {}",
quic::QuicVersionToString(version.transport_version)));
}
Expand Down
43 changes: 29 additions & 14 deletions test/common/quic/envoy_quic_client_session_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,12 @@ class TestEnvoyQuicClientConnection : public EnvoyQuicClientConnection {
using EnvoyQuicClientConnection::connectionStats;
};

class EnvoyQuicClientSessionTest : public testing::Test {
class EnvoyQuicClientSessionTest : public testing::TestWithParam<quic::ParsedQuicVersion> {
public:
EnvoyQuicClientSessionTest()
: api_(Api::createApiForTest(time_system_)),
dispatcher_(api_->allocateDispatcher("test_thread")), connection_helper_(*dispatcher_),
alarm_factory_(*dispatcher_, *connection_helper_.GetClock()),
quic_version_([]() { return quic::CurrentSupportedHttp3Versions(); }()),
alarm_factory_(*dispatcher_, *connection_helper_.GetClock()), quic_version_({GetParam()}),
peer_addr_(Network::Utility::getAddressWithPort(*Network::Utility::getIpv6LoopbackAddress(),
12345)),
self_addr_(Network::Utility::getAddressWithPort(*Network::Utility::getIpv6LoopbackAddress(),
Expand Down Expand Up @@ -161,7 +160,10 @@ class EnvoyQuicClientSessionTest : public testing::Test {
QuicHttpClientConnectionImpl http_connection_;
};

TEST_F(EnvoyQuicClientSessionTest, NewStream) {
INSTANTIATE_TEST_SUITE_P(EnvoyQuicClientSessionTests, EnvoyQuicClientSessionTest,
testing::ValuesIn(quic::CurrentSupportedHttp3Versions()));

TEST_P(EnvoyQuicClientSessionTest, NewStream) {
Http::MockResponseDecoder response_decoder;
Http::MockStreamCallbacks stream_callbacks;
EnvoyQuicClientStream& stream = sendGetRequest(response_decoder, stream_callbacks);
Expand All @@ -178,7 +180,7 @@ TEST_F(EnvoyQuicClientSessionTest, NewStream) {
stream.OnStreamHeaderList(/*fin=*/true, headers.uncompressed_header_bytes(), headers);
}

TEST_F(EnvoyQuicClientSessionTest, PacketLimits) {
TEST_P(EnvoyQuicClientSessionTest, PacketLimits) {
// We always allow for reading packets, even if there's no stream.
EXPECT_EQ(0, envoy_quic_session_.GetNumActiveStreams());
EXPECT_EQ(16, envoy_quic_session_.numPacketsExpectedPerEventLoop());
Expand Down Expand Up @@ -217,7 +219,7 @@ TEST_F(EnvoyQuicClientSessionTest, PacketLimits) {
envoy_quic_session_.close(Network::ConnectionCloseType::NoFlush);
}

TEST_F(EnvoyQuicClientSessionTest, OnResetFrame) {
TEST_P(EnvoyQuicClientSessionTest, OnResetFrame) {
Http::MockResponseDecoder response_decoder;
Http::MockStreamCallbacks stream_callbacks;
EnvoyQuicClientStream& stream = sendGetRequest(response_decoder, stream_callbacks);
Expand All @@ -235,7 +237,7 @@ TEST_F(EnvoyQuicClientSessionTest, OnResetFrame) {
->value());
}

TEST_F(EnvoyQuicClientSessionTest, SendResetFrame) {
TEST_P(EnvoyQuicClientSessionTest, SendResetFrame) {
Http::MockResponseDecoder response_decoder;
Http::MockStreamCallbacks stream_callbacks;
EnvoyQuicClientStream& stream = sendGetRequest(response_decoder, stream_callbacks);
Expand All @@ -252,15 +254,15 @@ TEST_F(EnvoyQuicClientSessionTest, SendResetFrame) {
->value());
}

TEST_F(EnvoyQuicClientSessionTest, OnGoAwayFrame) {
TEST_P(EnvoyQuicClientSessionTest, OnGoAwayFrame) {
Http::MockResponseDecoder response_decoder;
Http::MockStreamCallbacks stream_callbacks;

EXPECT_CALL(http_connection_callbacks_, onGoAway(Http::GoAwayErrorCode::NoError));
envoy_quic_session_.OnHttp3GoAway(4u);
}

TEST_F(EnvoyQuicClientSessionTest, ConnectionClose) {
TEST_P(EnvoyQuicClientSessionTest, ConnectionClose) {
std::string error_details("dummy details");
quic::QuicErrorCode error(quic::QUIC_INVALID_FRAME_DATA);
quic::QuicConnectionCloseFrame frame(quic_version_[0].transport_version, error,
Expand All @@ -278,7 +280,7 @@ TEST_F(EnvoyQuicClientSessionTest, ConnectionClose) {
->value());
}

TEST_F(EnvoyQuicClientSessionTest, ConnectionCloseWithActiveStream) {
TEST_P(EnvoyQuicClientSessionTest, ConnectionCloseWithActiveStream) {
Http::MockResponseDecoder response_decoder;
Http::MockStreamCallbacks stream_callbacks;
EnvoyQuicClientStream& stream = sendGetRequest(response_decoder, stream_callbacks);
Expand All @@ -294,7 +296,7 @@ TEST_F(EnvoyQuicClientSessionTest, ConnectionCloseWithActiveStream) {
->value());
}

TEST_F(EnvoyQuicClientSessionTest, HandshakeTimesOutWithActiveStream) {
TEST_P(EnvoyQuicClientSessionTest, HandshakeTimesOutWithActiveStream) {
Http::MockResponseDecoder response_decoder;
Http::MockStreamCallbacks stream_callbacks;
EnvoyQuicClientStream& stream = sendGetRequest(response_decoder, stream_callbacks);
Expand All @@ -311,7 +313,7 @@ TEST_F(EnvoyQuicClientSessionTest, HandshakeTimesOutWithActiveStream) {
->value());
}

TEST_F(EnvoyQuicClientSessionTest, ConnectionClosePopulatesQuicVersionStats) {
TEST_P(EnvoyQuicClientSessionTest, ConnectionClosePopulatesQuicVersionStats) {
std::string error_details("dummy details");
quic::QuicErrorCode error(quic::QUIC_INVALID_FRAME_DATA);
quic::QuicConnectionCloseFrame frame(quic_version_[0].transport_version, error,
Expand All @@ -322,10 +324,23 @@ TEST_F(EnvoyQuicClientSessionTest, ConnectionClosePopulatesQuicVersionStats) {
EXPECT_EQ(absl::StrCat(quic::QuicErrorCodeToString(error), " with details: ", error_details),
envoy_quic_session_.transportFailureReason());
EXPECT_EQ(Network::Connection::State::Closed, envoy_quic_session_.state());
EXPECT_EQ(1U, TestUtility::findCounter(store_, "http3.quic_version_rfc_v1")->value());
std::string quic_version_stat_name;
switch (GetParam().transport_version) {
case quic::QUIC_VERSION_IETF_DRAFT_29:
quic_version_stat_name = "h3_29";
break;
case quic::QUIC_VERSION_IETF_RFC_V1:
quic_version_stat_name = "rfc_v1";
break;
default:
break;
}
EXPECT_EQ(1U, TestUtility::findCounter(
store_, absl::StrCat("http3.quic_version_", quic_version_stat_name))
->value());
}

TEST_F(EnvoyQuicClientSessionTest, IncomingUnidirectionalReadStream) {
TEST_P(EnvoyQuicClientSessionTest, IncomingUnidirectionalReadStream) {
quic::QuicStreamId stream_id = 1u;
quic::QuicStreamFrame stream_frame(stream_id, false, 0, "aaa");
envoy_quic_session_.OnStreamFrame(stream_frame);
Expand Down
2 changes: 2 additions & 0 deletions test/integration/fake_upstream.h
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,8 @@ class FakeUpstream : Logger::Loggable<Logger::Id::testing>,
#if defined(ENVOY_ENABLE_QUIC)
udp_listener_config_.listener_factory_ = std::make_unique<Quic::ActiveQuicListenerFactory>(
envoy::config::listener::v3::QuicProtocolOptions(), 1, parent_.quic_stat_names_);
// Initialize QUICHE flags.
quiche::FlagRegistry::getInstance();
#else
ASSERT(false, "Running a test that requires QUIC without compiling QUIC");
#endif
Expand Down
27 changes: 27 additions & 0 deletions test/integration/quic_http_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,33 @@ TEST_P(QuicHttpIntegrationTest, GetRequestAndEmptyResponse) {
testRouterHeaderOnlyRequestAndResponse();
}

TEST_P(QuicHttpIntegrationTest, Draft29NotSupportedByDefault) {
supported_versions_ = {quic::ParsedQuicVersion::Draft29()};
initialize();
codec_client_ = makeRawHttpConnection(makeClientConnection(lookupPort("http")), absl::nullopt);
EXPECT_TRUE(codec_client_->disconnected());
EXPECT_EQ(quic::QUIC_INVALID_VERSION,
static_cast<EnvoyQuicClientSession*>(codec_client_->connection())->error());
}

TEST_P(QuicHttpIntegrationTest, RuntimeEnableDraft29) {
supported_versions_ = {quic::ParsedQuicVersion::Draft29()};
config_helper_.addRuntimeOverride(
"envoy.reloadable_features.FLAGS_quic_reloadable_flag_quic_disable_version_draft_29",
"false");
initialize();

codec_client_ = makeRawHttpConnection(makeClientConnection(lookupPort("http")), absl::nullopt);
EXPECT_EQ(transport_socket_factory_->clientContextConfig().serverNameIndication(),
codec_client_->connection()->requestedServerName());
auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_);
waitForNextUpstreamRequest(0);
upstream_request_->encodeHeaders(default_response_headers_, true);
ASSERT_TRUE(response->waitForEndStream());
codec_client_->close();
test_server_->waitForCounterEq("http3.quic_version_h3_29", 1u);
}

TEST_P(QuicHttpIntegrationTest, ZeroRtt) {
// Make sure both connections use the same PersistentQuicInfoImpl.
concurrency_ = 1;
Expand Down

0 comments on commit 4b5b338

Please sign in to comment.