From b526e9fc2d32ce4793de9c9369a9374cac059e52 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Tue, 14 Jan 2020 17:18:07 +0100 Subject: [PATCH 01/58] [WIP] Config switch to turn on/off Quic/Udp processing Signed-off-by: Kateryna Nezdolii --- api/envoy/api/v2/listener/quic_config.proto | 18 ++++++++++++++++ .../api/v2/listener/udp_listener_config.proto | 14 +++++++++++++ .../config/listener/v3alpha/quic_config.proto | 21 +++++++++++++++++++ .../v3alpha/udp_listener_config.proto | 16 ++++++++++++++ .../server/active_raw_udp_listener_config.cc | 4 +++- 5 files changed, 72 insertions(+), 1 deletion(-) diff --git a/api/envoy/api/v2/listener/quic_config.proto b/api/envoy/api/v2/listener/quic_config.proto index e134d44a586e..2f674841e72a 100644 --- a/api/envoy/api/v2/listener/quic_config.proto +++ b/api/envoy/api/v2/listener/quic_config.proto @@ -6,6 +6,7 @@ import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; import "udpa/annotations/migrate.proto"; +import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.api.v2.listener"; option java_outer_classname = "QuicConfigProto"; @@ -31,3 +32,20 @@ message QuicProtocolOptions { // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; } + +message ActiveQuicListenerConfig { + enum QuicShutDownMode { + // Stop accepting connections and reading packets, send resets to active sessions. + GRACEFUL_SHUTDOWN = 0; + // Stop accepting connections and reading packets, terminate sessions and close connections + // immediately. + HARD_SHUTDOWN = 1; + } + + QuicShutDownMode shutdown_mode = 1 [(validate.rules).enum = {defined_only: true}]; + + // If set to false, listener will stop reading new packets and shutdown active sessions. + // Shutdown type is configured with + // :ref:`shutdown_mode`. + bool enabled = 2; +} diff --git a/api/envoy/api/v2/listener/udp_listener_config.proto b/api/envoy/api/v2/listener/udp_listener_config.proto index c05a92ca6031..60f8f13ec591 100644 --- a/api/envoy/api/v2/listener/udp_listener_config.proto +++ b/api/envoy/api/v2/listener/udp_listener_config.proto @@ -6,6 +6,7 @@ import "google/protobuf/any.proto"; import "google/protobuf/struct.proto"; import "udpa/annotations/migrate.proto"; +import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.api.v2.listener"; option java_outer_classname = "UdpListenerConfigProto"; @@ -33,4 +34,17 @@ message UdpListenerConfig { } message ActiveRawUdpListenerConfig { + enum UdpShutDownMode { + // Stop reading new datagrams and send public resets to active sessions. + GRACEFUL_SHUTDOWN = 0; + // Stop processing new datagrams and destroy sessions immediately. + HARD_SHUTDOWN = 1; + } + + UdpShutDownMode shutdown_mode = 1 [(validate.rules).enum = {defined_only: true}]; + + // If set to false, listener will stop reading new datagrams and shutdown active sessions. + // Shutdown type is configured with + // :ref:`shutdown_mode`. + bool enabled = 2; } diff --git a/api/envoy/config/listener/v3alpha/quic_config.proto b/api/envoy/config/listener/v3alpha/quic_config.proto index 4aac7861545a..bde61fc25d6f 100644 --- a/api/envoy/config/listener/v3alpha/quic_config.proto +++ b/api/envoy/config/listener/v3alpha/quic_config.proto @@ -7,6 +7,8 @@ import "google/protobuf/wrappers.proto"; import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + option java_package = "io.envoyproxy.envoy.config.listener.v3alpha"; option java_outer_classname = "QuicConfigProto"; option java_multiple_files = true; @@ -31,3 +33,22 @@ message QuicProtocolOptions { // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; } + +message ActiveQuicListenerConfig { + option (udpa.annotations.versioning).previous_message_type = + "envoy.api.v2.listener.ActiveQuicListenerConfig"; + + enum QuicShutDownMode { + // Stop accepting connections and reading packets, send resets to active sessions. + GRACEFUL_SHUTDOWN = 0; + // Stop processing new packets and terminate sessions immediately. + HARD_SHUTDOWN = 1; + } + + QuicShutDownMode shutdown_mode = 1 [(validate.rules).enum = {defined_only: true}]; + + // If set to false, listener will stop reading new packets and shutdown active sessions. + // Shutdown type is configured with + // :ref:`shutdown_mode`. + bool enabled = 2; +} diff --git a/api/envoy/config/listener/v3alpha/udp_listener_config.proto b/api/envoy/config/listener/v3alpha/udp_listener_config.proto index d401c641bdec..556970761d04 100644 --- a/api/envoy/config/listener/v3alpha/udp_listener_config.proto +++ b/api/envoy/config/listener/v3alpha/udp_listener_config.proto @@ -7,6 +7,8 @@ import "google/protobuf/struct.proto"; import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + option java_package = "io.envoyproxy.envoy.config.listener.v3alpha"; option java_outer_classname = "UdpListenerConfigProto"; option java_multiple_files = true; @@ -37,4 +39,18 @@ message UdpListenerConfig { message ActiveRawUdpListenerConfig { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.listener.ActiveRawUdpListenerConfig"; + + enum UdpShutDownMode { + // Stop reading new datagrams and send public resets to active sessions. + GRACEFUL_SHUTDOWN = 0; + // Stop processing new datagrams and destroy sessions immediately. + HARD_SHUTDOWN = 1; + } + + UdpShutDownMode shutdown_mode = 1 [(validate.rules).enum = {defined_only: true}]; + + // If set to false, listener will stop reading new datagrams and shutdown active sessions. + // Shutdown type is configured with + // :ref:`shutdown_mode`. + bool enabled = 2; } diff --git a/source/server/active_raw_udp_listener_config.cc b/source/server/active_raw_udp_listener_config.cc index 192fbe0a7b53..90be1a40ea23 100644 --- a/source/server/active_raw_udp_listener_config.cc +++ b/source/server/active_raw_udp_listener_config.cc @@ -24,7 +24,9 @@ ProtobufTypes::MessagePtr ActiveRawUdpListenerConfigFactory::createEmptyConfigPr Network::ActiveUdpListenerFactoryPtr ActiveRawUdpListenerConfigFactory::createActiveUdpListenerFactory( - const Protobuf::Message& /*message*/) { + const Protobuf::Message& message) { + auto& config = + dynamic_cast(message); return std::make_unique(); } From fbbef3909b777730c22bd94ec2630aa50121f10d Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Fri, 31 Jan 2020 11:11:51 +0100 Subject: [PATCH 02/58] apply review comments Signed-off-by: Kateryna Nezdolii --- api/envoy/api/v2/listener/quic_config.proto | 1 + .../api/v2/listener/udp_listener_config.proto | 14 -------------- .../config/listener/v3alpha/quic_config.proto | 4 +++- .../listener/v3alpha/udp_listener_config.proto | 16 ---------------- .../quic_listeners/quiche/active_quic_listener.h | 2 +- 5 files changed, 5 insertions(+), 32 deletions(-) diff --git a/api/envoy/api/v2/listener/quic_config.proto b/api/envoy/api/v2/listener/quic_config.proto index 2f674841e72a..627f50d681d3 100644 --- a/api/envoy/api/v2/listener/quic_config.proto +++ b/api/envoy/api/v2/listener/quic_config.proto @@ -37,6 +37,7 @@ message ActiveQuicListenerConfig { enum QuicShutDownMode { // Stop accepting connections and reading packets, send resets to active sessions. GRACEFUL_SHUTDOWN = 0; + // Stop accepting connections and reading packets, terminate sessions and close connections // immediately. HARD_SHUTDOWN = 1; diff --git a/api/envoy/api/v2/listener/udp_listener_config.proto b/api/envoy/api/v2/listener/udp_listener_config.proto index 60f8f13ec591..c05a92ca6031 100644 --- a/api/envoy/api/v2/listener/udp_listener_config.proto +++ b/api/envoy/api/v2/listener/udp_listener_config.proto @@ -6,7 +6,6 @@ import "google/protobuf/any.proto"; import "google/protobuf/struct.proto"; import "udpa/annotations/migrate.proto"; -import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.api.v2.listener"; option java_outer_classname = "UdpListenerConfigProto"; @@ -34,17 +33,4 @@ message UdpListenerConfig { } message ActiveRawUdpListenerConfig { - enum UdpShutDownMode { - // Stop reading new datagrams and send public resets to active sessions. - GRACEFUL_SHUTDOWN = 0; - // Stop processing new datagrams and destroy sessions immediately. - HARD_SHUTDOWN = 1; - } - - UdpShutDownMode shutdown_mode = 1 [(validate.rules).enum = {defined_only: true}]; - - // If set to false, listener will stop reading new datagrams and shutdown active sessions. - // Shutdown type is configured with - // :ref:`shutdown_mode`. - bool enabled = 2; } diff --git a/api/envoy/config/listener/v3alpha/quic_config.proto b/api/envoy/config/listener/v3alpha/quic_config.proto index bde61fc25d6f..5bf7f185b9e3 100644 --- a/api/envoy/config/listener/v3alpha/quic_config.proto +++ b/api/envoy/config/listener/v3alpha/quic_config.proto @@ -41,7 +41,9 @@ message ActiveQuicListenerConfig { enum QuicShutDownMode { // Stop accepting connections and reading packets, send resets to active sessions. GRACEFUL_SHUTDOWN = 0; - // Stop processing new packets and terminate sessions immediately. + + // Stop accepting connections and reading packets, terminate sessions and close connections + // immediately. HARD_SHUTDOWN = 1; } diff --git a/api/envoy/config/listener/v3alpha/udp_listener_config.proto b/api/envoy/config/listener/v3alpha/udp_listener_config.proto index 556970761d04..d401c641bdec 100644 --- a/api/envoy/config/listener/v3alpha/udp_listener_config.proto +++ b/api/envoy/config/listener/v3alpha/udp_listener_config.proto @@ -7,8 +7,6 @@ import "google/protobuf/struct.proto"; import "udpa/annotations/versioning.proto"; -import "validate/validate.proto"; - option java_package = "io.envoyproxy.envoy.config.listener.v3alpha"; option java_outer_classname = "UdpListenerConfigProto"; option java_multiple_files = true; @@ -39,18 +37,4 @@ message UdpListenerConfig { message ActiveRawUdpListenerConfig { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.listener.ActiveRawUdpListenerConfig"; - - enum UdpShutDownMode { - // Stop reading new datagrams and send public resets to active sessions. - GRACEFUL_SHUTDOWN = 0; - // Stop processing new datagrams and destroy sessions immediately. - HARD_SHUTDOWN = 1; - } - - UdpShutDownMode shutdown_mode = 1 [(validate.rules).enum = {defined_only: true}]; - - // If set to false, listener will stop reading new datagrams and shutdown active sessions. - // Shutdown type is configured with - // :ref:`shutdown_mode`. - bool enabled = 2; } diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.h b/source/extensions/quic_listeners/quiche/active_quic_listener.h index faeb7b4ad96b..1cb2dfea5a71 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.h +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.h @@ -48,7 +48,7 @@ class ActiveQuicListener : public Network::UdpListenerCallbacks, private: friend class ActiveQuicListenerPeer; - + // todo (nezdolik) Propagate shutdown signal from udp listener to quick listener Network::UdpListenerPtr udp_listener_; uint8_t random_seed_[16]; std::unique_ptr crypto_config_; From 1377cd50f3234e27f39be6de8042f62fc549fa85 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Mon, 3 Feb 2020 15:32:46 +0100 Subject: [PATCH 03/58] use runtime flag instead Signed-off-by: Kateryna Nezdolii --- api/envoy/api/v2/listener/quic_config.proto | 20 +--------------- .../quiche/active_quic_listener.cc | 23 +++++++++++++++---- .../quiche/active_quic_listener.h | 13 +++++++---- .../server/active_raw_udp_listener_config.cc | 4 +--- 4 files changed, 29 insertions(+), 31 deletions(-) diff --git a/api/envoy/api/v2/listener/quic_config.proto b/api/envoy/api/v2/listener/quic_config.proto index 054ffe93726f..d9f60873e86e 100644 --- a/api/envoy/api/v2/listener/quic_config.proto +++ b/api/envoy/api/v2/listener/quic_config.proto @@ -31,22 +31,4 @@ message QuicProtocolOptions { // Connection timeout in milliseconds before the crypto handshake is finished. // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; -} - -message ActiveQuicListenerConfig { - enum QuicShutDownMode { - // Stop accepting connections and reading packets, send resets to active sessions. - GRACEFUL_SHUTDOWN = 0; - - // Stop accepting connections and reading packets, terminate sessions and close connections - // immediately. - HARD_SHUTDOWN = 1; - } - - QuicShutDownMode shutdown_mode = 1 [(validate.rules).enum = {defined_only: true}]; - - // If set to false, listener will stop reading new packets and shutdown active sessions. - // Shutdown type is configured with - // :ref:`shutdown_mode`. - bool enabled = 2; -} +} \ No newline at end of file diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index f278e2cd7320..ac9cd31a43cc 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -13,19 +13,21 @@ namespace Quic { ActiveQuicListener::ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, Network::ListenerConfig& listener_config, - const quic::QuicConfig& quic_config) + const quic::QuicConfig& quic_config, + Runtime::Loader& runtime) : ActiveQuicListener(dispatcher, parent, listener_config.listenSocketFactory().getListenSocket(), listener_config, - quic_config) {} + quic_config, runtime) {} ActiveQuicListener::ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, Network::SocketSharedPtr listen_socket, Network::ListenerConfig& listener_config, - const quic::QuicConfig& quic_config) + const quic::QuicConfig& quic_config, Runtime::Loader& runtime) : Server::ConnectionHandlerImpl::ActiveListenerImplBase(parent, listener_config), dispatcher_(dispatcher), version_manager_(quic::CurrentSupportedVersions()), - listen_socket_(*listen_socket) { + listen_socket_(*listen_socket), + enabled_(true, runtime) { udp_listener_ = dispatcher_.createUdpListener(std::move(listen_socket), *this); quic::QuicRandom* const random = quic::QuicRandom::GetInstance(); random->RandBytes(random_seed_, sizeof(random_seed_)); @@ -39,6 +41,7 @@ ActiveQuicListener::ActiveQuicListener(Event::Dispatcher& dispatcher, auto alarm_factory = std::make_unique(dispatcher_, *connection_helper->GetClock()); quic_dispatcher_ = std::make_unique( + crypto_config_.get(), quic_config, &version_manager_, std::move(connection_helper), std::move(alarm_factory), quic::kQuicDefaultConnectionIdLength, parent, config_, stats_, dispatcher, listen_socket_); @@ -53,6 +56,10 @@ void ActiveQuicListener::onListenerShutdown() { } void ActiveQuicListener::onData(Network::UdpRecvData& data) { + if (!enabled()) { + ENVOY_LOG(trace, "Quic listener {}: runtime disabled", config_.name()); + return; + } quic::QuicSocketAddress peer_address( envoyAddressInstanceToQuicSocketAddress(data.addresses_.peer_)); quic::QuicSocketAddress self_address( @@ -75,10 +82,18 @@ void ActiveQuicListener::onData(Network::UdpRecvData& data) { } void ActiveQuicListener::onReadReady() { + if (!enabled()) { + ENVOY_LOG(trace, "Quic listener {}: runtime disabled", config_.name()); + return; + } quic_dispatcher_->ProcessBufferedChlos(kNumSessionsToCreatePerLoop); } void ActiveQuicListener::onWriteReady(const Network::Socket& /*socket*/) { + if (!enabled()) { + ENVOY_LOG(trace, "Quic listener {}: runtime disabled", config_.name()); + return; + } quic_dispatcher_->OnCanWrite(); } diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.h b/source/extensions/quic_listeners/quiche/active_quic_listener.h index 8ddd7b8dc6c9..91eb21bdacbb 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.h +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.h @@ -5,6 +5,7 @@ #include "envoy/network/listener.h" #include "common/protobuf/utility.h" +#include "envoy/runtime/runtime.h" #include "server/connection_handler_impl.h" @@ -23,11 +24,11 @@ class ActiveQuicListener : public Network::UdpListenerCallbacks, static const size_t kNumSessionsToCreatePerLoop = 16; ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, - Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config); + Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, Runtime::Loader& runtime); ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, Network::SocketSharedPtr listen_socket, - Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config); + Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, Runtime::Loader& runtime); ~ActiveQuicListener() override; @@ -46,9 +47,10 @@ class ActiveQuicListener : public Network::UdpListenerCallbacks, Network::Listener* listener() override { return udp_listener_.get(); } void destroy() override { udp_listener_.reset(); } + bool enabled() { return enabled_.enabled(); } + private: friend class ActiveQuicListenerPeer; - // todo (nezdolik) Propagate shutdown signal from udp listener to quick listener Network::UdpListenerPtr udp_listener_; uint8_t random_seed_[16]; std::unique_ptr crypto_config_; @@ -56,6 +58,7 @@ class ActiveQuicListener : public Network::UdpListenerCallbacks, quic::QuicVersionManager version_manager_; std::unique_ptr quic_dispatcher_; Network::Socket& listen_socket_; + Runtime::FeatureFlag enabled_; }; using ActiveQuicListenerPtr = std::unique_ptr; @@ -83,9 +86,9 @@ class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory { // Network::ActiveUdpListenerFactory. Network::ConnectionHandler::ActiveListenerPtr - createActiveUdpListener(Network::ConnectionHandler& parent, Event::Dispatcher& disptacher, + createActiveUdpListener(Network::ConnectionHandler& parent, Event::Dispatcher& dispatcher, Network::ListenerConfig& config) const override { - return std::make_unique(disptacher, parent, config, quic_config_); + return std::make_unique(dispatcher, parent, config, quic_config_); } bool isTransportConnectionless() const override { return false; } diff --git a/source/server/active_raw_udp_listener_config.cc b/source/server/active_raw_udp_listener_config.cc index 6e5b2d6baacf..8be145719ecf 100644 --- a/source/server/active_raw_udp_listener_config.cc +++ b/source/server/active_raw_udp_listener_config.cc @@ -24,9 +24,7 @@ ProtobufTypes::MessagePtr ActiveRawUdpListenerConfigFactory::createEmptyConfigPr Network::ActiveUdpListenerFactoryPtr ActiveRawUdpListenerConfigFactory::createActiveUdpListenerFactory( - const Protobuf::Message& message) { - auto& config = - dynamic_cast(message); + const Protobuf::Message& /*message*/) { return std::make_unique(); } From ed1a3996c2befb5aef4537a37d29a246a3999737 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Mon, 3 Feb 2020 17:07:41 +0100 Subject: [PATCH 04/58] more wip Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener.h | 7 +++++-- .../quic_listeners/quiche/active_quic_listener_test.cc | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.h b/source/extensions/quic_listeners/quiche/active_quic_listener.h index 91eb21bdacbb..042ce777a80c 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.h +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.h @@ -6,6 +6,7 @@ #include "common/protobuf/utility.h" #include "envoy/runtime/runtime.h" +#include "common/runtime/runtime_protos.h" #include "server/connection_handler_impl.h" @@ -66,7 +67,7 @@ using ActiveQuicListenerPtr = std::unique_ptr; // A factory to create ActiveQuicListener based on given config. class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory { public: - ActiveQuicListenerFactory(const envoy::config::listener::v3::QuicProtocolOptions& config) { + ActiveQuicListenerFactory(const envoy::config::listener::v3::QuicProtocolOptions& config, const Runtime::Loader& runtime) { uint64_t idle_network_timeout_ms = config.has_idle_timeout() ? DurationUtil::durationToMilliseconds(config.idle_timeout()) : 300000; @@ -82,13 +83,14 @@ class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory { int32_t max_streams = PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_concurrent_streams, 100); quic_config_.SetMaxIncomingBidirectionalStreamsToSend(max_streams); quic_config_.SetMaxIncomingUnidirectionalStreamsToSend(max_streams); + runtime_(runtime); } // Network::ActiveUdpListenerFactory. Network::ConnectionHandler::ActiveListenerPtr createActiveUdpListener(Network::ConnectionHandler& parent, Event::Dispatcher& dispatcher, Network::ListenerConfig& config) const override { - return std::make_unique(dispatcher, parent, config, quic_config_); + return std::make_unique(dispatcher, parent, config, quic_config_, runtime_); } bool isTransportConnectionless() const override { return false; } @@ -96,6 +98,7 @@ class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory { friend class ActiveQuicListenerFactoryPeer; quic::QuicConfig quic_config_; + Runtime::Loader& runtime_; }; } // namespace Quic diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 47932955ac65..50a3d4e3e764 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -21,6 +21,7 @@ #include "test/test_common/simulated_time_system.h" #include "test/test_common/environment.h" #include "test/mocks/network/mocks.h" +#include "test/mocks/runtime/mocks.h" #include "test/test_common/utility.h" #include "test/test_common/network_utility.h" #include "absl/time/time.h" @@ -64,7 +65,7 @@ class ActiveQuicListenerTest : public testing::TestWithParamaddOptions(Network::SocketOptionFactory::buildRxQueueOverFlowOptions()); quic_listener_ = std::make_unique( - *dispatcher_, connection_handler_, listen_socket_, listener_config_, quic_config_); + *dispatcher_, connection_handler_, listen_socket_, listener_config_, quic_config_, runtime_); simulated_time_system_.sleep(std::chrono::milliseconds(100)); } @@ -84,7 +85,8 @@ class ActiveQuicListenerTest : public testing::TestWithParam quic_listener_; + NiceMock runtime_; std::list> client_sockets_; std::list> read_filters_; From 98148f11df53a7a2b5f84720222938ec954d2f15 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Tue, 4 Feb 2020 16:57:47 +0100 Subject: [PATCH 05/58] more wip Signed-off-by: Kateryna Nezdolii --- api/envoy/api/v2/listener/quic_config.proto | 1 - 1 file changed, 1 deletion(-) diff --git a/api/envoy/api/v2/listener/quic_config.proto b/api/envoy/api/v2/listener/quic_config.proto index d9f60873e86e..636789f38436 100644 --- a/api/envoy/api/v2/listener/quic_config.proto +++ b/api/envoy/api/v2/listener/quic_config.proto @@ -6,7 +6,6 @@ import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; import "udpa/annotations/migrate.proto"; -import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.api.v2.listener"; option java_outer_classname = "QuicConfigProto"; From 43905d870f6072fedacf6f6427c38f02f6e1d2a2 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Tue, 4 Feb 2020 17:09:30 +0100 Subject: [PATCH 06/58] more wip Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener.cc | 11 ++++++----- .../quic_listeners/quiche/active_quic_listener.h | 11 +++++++---- source/server/active_raw_udp_listener_config.cc | 2 +- .../quiche/active_quic_listener_test.cc | 9 +++++---- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index ac9cd31a43cc..1b35eb9496c9 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -23,11 +23,12 @@ ActiveQuicListener::ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, Network::SocketSharedPtr listen_socket, Network::ListenerConfig& listener_config, - const quic::QuicConfig& quic_config, Runtime::Loader& runtime) + const quic::QuicConfig& quic_config, + Runtime::Loader& runtime) : Server::ConnectionHandlerImpl::ActiveListenerImplBase(parent, listener_config), dispatcher_(dispatcher), version_manager_(quic::CurrentSupportedVersions()), - listen_socket_(*listen_socket), - enabled_(true, runtime) { + // todo(nezdolik) extract feature flag value from conf + listen_socket_(*listen_socket), enabled_(true, runtime) { udp_listener_ = dispatcher_.createUdpListener(std::move(listen_socket), *this); quic::QuicRandom* const random = quic::QuicRandom::GetInstance(); random->RandBytes(random_seed_, sizeof(random_seed_)); @@ -56,7 +57,7 @@ void ActiveQuicListener::onListenerShutdown() { } void ActiveQuicListener::onData(Network::UdpRecvData& data) { - if (!enabled()) { + if (!enabled()) { ENVOY_LOG(trace, "Quic listener {}: runtime disabled", config_.name()); return; } @@ -93,7 +94,7 @@ void ActiveQuicListener::onWriteReady(const Network::Socket& /*socket*/) { if (!enabled()) { ENVOY_LOG(trace, "Quic listener {}: runtime disabled", config_.name()); return; - } + } quic_dispatcher_->OnCanWrite(); } diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.h b/source/extensions/quic_listeners/quiche/active_quic_listener.h index 042ce777a80c..94c74688d157 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.h +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.h @@ -3,9 +3,9 @@ #include "envoy/config/listener/v3/quic_config.pb.h" #include "envoy/network/connection_handler.h" #include "envoy/network/listener.h" +#include "envoy/runtime/runtime.h" #include "common/protobuf/utility.h" -#include "envoy/runtime/runtime.h" #include "common/runtime/runtime_protos.h" #include "server/connection_handler_impl.h" @@ -25,11 +25,13 @@ class ActiveQuicListener : public Network::UdpListenerCallbacks, static const size_t kNumSessionsToCreatePerLoop = 16; ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, - Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, Runtime::Loader& runtime); + Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, + Runtime::Loader& runtime); ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, Network::SocketSharedPtr listen_socket, - Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, Runtime::Loader& runtime); + Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, + Runtime::Loader& runtime); ~ActiveQuicListener() override; @@ -67,7 +69,8 @@ using ActiveQuicListenerPtr = std::unique_ptr; // A factory to create ActiveQuicListener based on given config. class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory { public: - ActiveQuicListenerFactory(const envoy::config::listener::v3::QuicProtocolOptions& config, const Runtime::Loader& runtime) { + ActiveQuicListenerFactory(const envoy::config::listener::v3::QuicProtocolOptions& config, + const Runtime::Loader& runtime) { uint64_t idle_network_timeout_ms = config.has_idle_timeout() ? DurationUtil::durationToMilliseconds(config.idle_timeout()) : 300000; diff --git a/source/server/active_raw_udp_listener_config.cc b/source/server/active_raw_udp_listener_config.cc index 8be145719ecf..49350fb9f78f 100644 --- a/source/server/active_raw_udp_listener_config.cc +++ b/source/server/active_raw_udp_listener_config.cc @@ -24,7 +24,7 @@ ProtobufTypes::MessagePtr ActiveRawUdpListenerConfigFactory::createEmptyConfigPr Network::ActiveUdpListenerFactoryPtr ActiveRawUdpListenerConfigFactory::createActiveUdpListenerFactory( - const Protobuf::Message& /*message*/) { + const Protobuf::Message& /*message*/) { return std::make_unique(); } diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 50a3d4e3e764..447d11e0a640 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -64,8 +64,9 @@ class ActiveQuicListenerTest : public testing::TestWithParamaddOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions()); listen_socket_->addOptions(Network::SocketOptionFactory::buildRxQueueOverFlowOptions()); - quic_listener_ = std::make_unique( - *dispatcher_, connection_handler_, listen_socket_, listener_config_, quic_config_, runtime_); + quic_listener_ = + std::make_unique(*dispatcher_, connection_handler_, listen_socket_, + listener_config_, quic_config_, runtime_); simulated_time_system_.sleep(std::chrono::milliseconds(100)); } @@ -85,8 +86,8 @@ class ActiveQuicListenerTest : public testing::TestWithParam Date: Tue, 4 Feb 2020 17:28:43 +0100 Subject: [PATCH 07/58] remove dummy changes Signed-off-by: Kateryna Nezdolii --- api/envoy/api/v2/listener/quic_config.proto | 2 +- .../config/listener/v3/quic_config.proto | 21 ------------------- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/api/envoy/api/v2/listener/quic_config.proto b/api/envoy/api/v2/listener/quic_config.proto index 636789f38436..69069f76b7e0 100644 --- a/api/envoy/api/v2/listener/quic_config.proto +++ b/api/envoy/api/v2/listener/quic_config.proto @@ -30,4 +30,4 @@ message QuicProtocolOptions { // Connection timeout in milliseconds before the crypto handshake is finished. // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; -} \ No newline at end of file +} diff --git a/generated_api_shadow/envoy/config/listener/v3/quic_config.proto b/generated_api_shadow/envoy/config/listener/v3/quic_config.proto index 5494c2a1971a..76345d2973cc 100644 --- a/generated_api_shadow/envoy/config/listener/v3/quic_config.proto +++ b/generated_api_shadow/envoy/config/listener/v3/quic_config.proto @@ -31,24 +31,3 @@ message QuicProtocolOptions { // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; } - -message ActiveQuicListenerConfig { - option (udpa.annotations.versioning).previous_message_type = - "envoy.api.v2.listener.ActiveQuicListenerConfig"; - - enum QuicShutDownMode { - // Stop accepting connections and reading packets, send resets to active sessions. - GRACEFUL_SHUTDOWN = 0; - - // Stop accepting connections and reading packets, terminate sessions and close connections - // immediately. - HARD_SHUTDOWN = 1; - } - - QuicShutDownMode shutdown_mode = 1 [(validate.rules).enum = {defined_only: true}]; - - // If set to false, listener will stop reading new packets and shutdown active sessions. - // Shutdown type is configured with - // :ref:`shutdown_mode`. - bool enabled = 2; -} From 6d3eaf82887bb27f5d28b2a581dab6cda77283d7 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Tue, 4 Feb 2020 17:53:53 +0100 Subject: [PATCH 08/58] fix default Signed-off-by: Kateryna Nezdolii --- source/extensions/quic_listeners/quiche/active_quic_listener.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index 1b35eb9496c9..10b9f18b702c 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -28,7 +28,7 @@ ActiveQuicListener::ActiveQuicListener(Event::Dispatcher& dispatcher, : Server::ConnectionHandlerImpl::ActiveListenerImplBase(parent, listener_config), dispatcher_(dispatcher), version_manager_(quic::CurrentSupportedVersions()), // todo(nezdolik) extract feature flag value from conf - listen_socket_(*listen_socket), enabled_(true, runtime) { + listen_socket_(*listen_socket), enabled_(envoy::config::core::v3::RuntimeFeatureFlag(), runtime) { udp_listener_ = dispatcher_.createUdpListener(std::move(listen_socket), *this); quic::QuicRandom* const random = quic::QuicRandom::GetInstance(); random->RandBytes(random_seed_, sizeof(random_seed_)); From 4c0c2fa16091d76ad5c59aafdbbcde9cc0e5eaa7 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Wed, 5 Feb 2020 12:33:40 +0100 Subject: [PATCH 09/58] more wip Signed-off-by: Kateryna Nezdolii --- api/envoy/api/v2/listener/quic_config.proto | 8 +++- .../config/listener/v3/quic_config.proto | 8 +++- .../envoy/api/v2/listener/quic_config.proto | 8 +++- .../config/listener/v3/quic_config.proto | 8 +++- source/extensions/quic_listeners/quiche/BUILD | 1 + .../quiche/active_quic_listener.cc | 9 ++--- .../quiche/active_quic_listener.h | 13 +++---- test/extensions/quic_listeners/quiche/BUILD | 1 + .../quiche/active_quic_listener_test.cc | 39 +++++++++++++++++-- 9 files changed, 76 insertions(+), 19 deletions(-) diff --git a/api/envoy/api/v2/listener/quic_config.proto b/api/envoy/api/v2/listener/quic_config.proto index 69069f76b7e0..cf0876301859 100644 --- a/api/envoy/api/v2/listener/quic_config.proto +++ b/api/envoy/api/v2/listener/quic_config.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package envoy.api.v2.listener; +import "envoy/api/v2/core/base.proto"; + import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; @@ -17,7 +19,7 @@ option (udpa.annotations.file_migrate).move_to_package = "envoy.config.listener. // [#protodoc-title: QUIC listener Config] // Configuration specific to the QUIC protocol. -// Next id: 4 +// Next id: 5 message QuicProtocolOptions { // Maximum number of streams that the client can negotiate per connection. 100 // if not specified. @@ -30,4 +32,8 @@ message QuicProtocolOptions { // Connection timeout in milliseconds before the crypto handshake is finished. // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; + + // Runtime flag that controls whether the listener is enabled or not. If not specified, defaults + // to enabled. + core.RuntimeFeatureFlag enabled = 4; } diff --git a/api/envoy/config/listener/v3/quic_config.proto b/api/envoy/config/listener/v3/quic_config.proto index 76345d2973cc..1522f8b6bdc9 100644 --- a/api/envoy/config/listener/v3/quic_config.proto +++ b/api/envoy/config/listener/v3/quic_config.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package envoy.config.listener.v3; +import "envoy/config/core/v3/base.proto"; + import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; @@ -14,7 +16,7 @@ option java_multiple_files = true; // [#protodoc-title: QUIC listener Config] // Configuration specific to the QUIC protocol. -// Next id: 4 +// Next id: 5 message QuicProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.listener.QuicProtocolOptions"; @@ -30,4 +32,8 @@ message QuicProtocolOptions { // Connection timeout in milliseconds before the crypto handshake is finished. // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; + + // Runtime flag that controls whether the listener is enabled or not. If not specified, defaults + // to enabled. + core.v3.RuntimeFeatureFlag enabled = 4; } diff --git a/generated_api_shadow/envoy/api/v2/listener/quic_config.proto b/generated_api_shadow/envoy/api/v2/listener/quic_config.proto index 69069f76b7e0..cf0876301859 100644 --- a/generated_api_shadow/envoy/api/v2/listener/quic_config.proto +++ b/generated_api_shadow/envoy/api/v2/listener/quic_config.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package envoy.api.v2.listener; +import "envoy/api/v2/core/base.proto"; + import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; @@ -17,7 +19,7 @@ option (udpa.annotations.file_migrate).move_to_package = "envoy.config.listener. // [#protodoc-title: QUIC listener Config] // Configuration specific to the QUIC protocol. -// Next id: 4 +// Next id: 5 message QuicProtocolOptions { // Maximum number of streams that the client can negotiate per connection. 100 // if not specified. @@ -30,4 +32,8 @@ message QuicProtocolOptions { // Connection timeout in milliseconds before the crypto handshake is finished. // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; + + // Runtime flag that controls whether the listener is enabled or not. If not specified, defaults + // to enabled. + core.RuntimeFeatureFlag enabled = 4; } diff --git a/generated_api_shadow/envoy/config/listener/v3/quic_config.proto b/generated_api_shadow/envoy/config/listener/v3/quic_config.proto index 76345d2973cc..1522f8b6bdc9 100644 --- a/generated_api_shadow/envoy/config/listener/v3/quic_config.proto +++ b/generated_api_shadow/envoy/config/listener/v3/quic_config.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package envoy.config.listener.v3; +import "envoy/config/core/v3/base.proto"; + import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; @@ -14,7 +16,7 @@ option java_multiple_files = true; // [#protodoc-title: QUIC listener Config] // Configuration specific to the QUIC protocol. -// Next id: 4 +// Next id: 5 message QuicProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.listener.QuicProtocolOptions"; @@ -30,4 +32,8 @@ message QuicProtocolOptions { // Connection timeout in milliseconds before the crypto handshake is finished. // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; + + // Runtime flag that controls whether the listener is enabled or not. If not specified, defaults + // to enabled. + core.v3.RuntimeFeatureFlag enabled = 4; } diff --git a/source/extensions/quic_listeners/quiche/BUILD b/source/extensions/quic_listeners/quiche/BUILD index 93dac2bac310..241d76470249 100644 --- a/source/extensions/quic_listeners/quiche/BUILD +++ b/source/extensions/quic_listeners/quiche/BUILD @@ -273,6 +273,7 @@ envoy_cc_library( "//include/envoy/network:listener_interface", "//source/common/network:listener_lib", "//source/common/protobuf:utility_lib", + "//source/common/runtime:runtime_lib", "//source/server:connection_handler_lib", "@envoy_api//envoy/config/listener/v3:pkg_cc_proto", ], diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index 10b9f18b702c..213a3e2b6e8a 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -14,21 +14,20 @@ ActiveQuicListener::ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, - Runtime::Loader& runtime) + const envoy::config::core::v3::RuntimeFeatureFlag enabled) : ActiveQuicListener(dispatcher, parent, listener_config.listenSocketFactory().getListenSocket(), listener_config, - quic_config, runtime) {} + quic_config, enabled) {} ActiveQuicListener::ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, Network::SocketSharedPtr listen_socket, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, - Runtime::Loader& runtime) + const envoy::config::core::v3::RuntimeFeatureFlag enabled) : Server::ConnectionHandlerImpl::ActiveListenerImplBase(parent, listener_config), dispatcher_(dispatcher), version_manager_(quic::CurrentSupportedVersions()), - // todo(nezdolik) extract feature flag value from conf - listen_socket_(*listen_socket), enabled_(envoy::config::core::v3::RuntimeFeatureFlag(), runtime) { + listen_socket_(*listen_socket), enabled_(enabled, Runtime::LoaderSingleton::get()) { udp_listener_ = dispatcher_.createUdpListener(std::move(listen_socket), *this); quic::QuicRandom* const random = quic::QuicRandom::GetInstance(); random->RandBytes(random_seed_, sizeof(random_seed_)); diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.h b/source/extensions/quic_listeners/quiche/active_quic_listener.h index 94c74688d157..9969ba90d8d2 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.h +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.h @@ -26,12 +26,12 @@ class ActiveQuicListener : public Network::UdpListenerCallbacks, ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, - Runtime::Loader& runtime); + const envoy::config::core::v3::RuntimeFeatureFlag enabled); ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, Network::SocketSharedPtr listen_socket, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, - Runtime::Loader& runtime); + const envoy::config::core::v3::RuntimeFeatureFlag enabled); ~ActiveQuicListener() override; @@ -69,8 +69,8 @@ using ActiveQuicListenerPtr = std::unique_ptr; // A factory to create ActiveQuicListener based on given config. class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory { public: - ActiveQuicListenerFactory(const envoy::config::listener::v3::QuicProtocolOptions& config, - const Runtime::Loader& runtime) { + ActiveQuicListenerFactory(const envoy::config::listener::v3::QuicProtocolOptions& config) + : enabled_(config.enabled()) { uint64_t idle_network_timeout_ms = config.has_idle_timeout() ? DurationUtil::durationToMilliseconds(config.idle_timeout()) : 300000; @@ -86,14 +86,13 @@ class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory { int32_t max_streams = PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_concurrent_streams, 100); quic_config_.SetMaxIncomingBidirectionalStreamsToSend(max_streams); quic_config_.SetMaxIncomingUnidirectionalStreamsToSend(max_streams); - runtime_(runtime); } // Network::ActiveUdpListenerFactory. Network::ConnectionHandler::ActiveListenerPtr createActiveUdpListener(Network::ConnectionHandler& parent, Event::Dispatcher& dispatcher, Network::ListenerConfig& config) const override { - return std::make_unique(dispatcher, parent, config, quic_config_, runtime_); + return std::make_unique(dispatcher, parent, config, quic_config_, enabled_); } bool isTransportConnectionless() const override { return false; } @@ -101,7 +100,7 @@ class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory { friend class ActiveQuicListenerFactoryPeer; quic::QuicConfig quic_config_; - Runtime::Loader& runtime_; + envoy::config::core::v3::RuntimeFeatureFlag enabled_; }; } // namespace Quic diff --git a/test/extensions/quic_listeners/quiche/BUILD b/test/extensions/quic_listeners/quiche/BUILD index f7b52aed56d2..0b1af433c046 100644 --- a/test/extensions/quic_listeners/quiche/BUILD +++ b/test/extensions/quic_listeners/quiche/BUILD @@ -150,6 +150,7 @@ envoy_cc_test( "//test/test_common:environment_lib", "//test/test_common:network_utility_lib", "//test/test_common:simulated_time_system_lib", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], ) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 447d11e0a640..57b271844585 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -6,6 +6,9 @@ #include +#include "envoy/config/core/v3/base.pb.h" +#include "envoy/config/core/v3/base.pb.validate.h" + #include "quiche/quic/core/crypto/crypto_protocol.h" #include "quiche/quic/test_tools/crypto_test_utils.h" #include "quiche/quic/test_tools/quic_dispatcher_peer.h" @@ -56,7 +59,11 @@ class ActiveQuicListenerTest : public testing::TestWithParamallocateDispatcher()), clock_(*dispatcher_), local_address_(Network::Test::getCanonicalLoopbackAddress(version_)), - connection_handler_(*dispatcher_, "test_thread") {} + connection_handler_(*dispatcher_, "test_thread") { + Runtime::LoaderSingleton::initialize(&runtime_); + } + + ~ActiveQuicListenerTest() { Runtime::LoaderSingleton::clear(); } void SetUp() override { listen_socket_ = @@ -66,7 +73,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam(*dispatcher_, connection_handler_, listen_socket_, - listener_config_, quic_config_, runtime_); + listener_config_, quic_config_, enabled_flag()); simulated_time_system_.sleep(std::chrono::milliseconds(100)); } @@ -87,7 +94,8 @@ class ActiveQuicListenerTest : public testing::TestWithParamrun(Event::Dispatcher::RunType::NonBlock); } + virtual envoy::config::core::v3::RuntimeFeatureFlag enabled_flag() const { + envoy::config::core::v3::RuntimeFeatureFlag enabled_proto; + std::string yaml(R"EOF( +runtime_key: "quic.enabled" +default_value: true +)EOF"); + TestUtility::loadFromYamlAndValidate(yaml, enabled_proto); + return enabled_proto; + } + Network::Address::IpVersion version_; Event::SimulatedTimeSystemHelper simulated_time_system_; Api::ApiPtr api_; @@ -260,5 +278,20 @@ TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { ReadFromClientSockets(); } +TEST_P(ActiveQuicListenerTest, QuicProcessingEnabled) {} + +class ActiveQuicListenerDisabledTest : public ActiveQuicListenerTest { +protected: + envoy::config::core::v3::RuntimeFeatureFlag enabled_flag() const override { + envoy::config::core::v3::RuntimeFeatureFlag enabled_proto; + std::string yaml(R"EOF( +runtime_key: "quic.enabled" +default_value: false +)EOF"); + TestUtility::loadFromYamlAndValidate(yaml, enabled_proto); + return enabled_proto; + } +}; + } // namespace Quic } // namespace Envoy From f9cdb783fb3ebf3462521b056689a0c1ebec0cc0 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Tue, 11 Feb 2020 12:51:29 +0100 Subject: [PATCH 10/58] more tests Signed-off-by: Kateryna Nezdolii --- .../quiche/active_quic_listener.h | 2 + .../active_quic_listener_config_test.cc | 12 +++++ .../quiche/active_quic_listener_test.cc | 51 +++++++++++-------- 3 files changed, 44 insertions(+), 21 deletions(-) diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.h b/source/extensions/quic_listeners/quiche/active_quic_listener.h index 9969ba90d8d2..1ad555bc4ed4 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.h +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.h @@ -1,5 +1,7 @@ #pragma once +#include + #include "envoy/config/listener/v3/quic_config.pb.h" #include "envoy/network/connection_handler.h" #include "envoy/network/listener.h" diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc index ca2699a1e721..ea6eae37ea3f 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc @@ -15,6 +15,10 @@ class ActiveQuicListenerFactoryPeer { static quic::QuicConfig& quicConfig(ActiveQuicListenerFactory& factory) { return factory.quic_config_; } + static envoy::config::core::v3::RuntimeFeatureFlag& + runtimeEnabled(ActiveQuicListenerFactory& factory) { + return factory.enabled_; + } }; TEST(ActiveQuicListenerConfigTest, CreateActiveQuicListenerFactory) { @@ -29,6 +33,9 @@ TEST(ActiveQuicListenerConfigTest, CreateActiveQuicListenerFactory) { idle_timeout: { seconds: 2 } + enabled: + default_value: true + runtime_key: foo_key )EOF"; TestUtility::loadFromYaml(yaml, *config); Network::ActiveUdpListenerFactoryPtr listener_factory = @@ -41,6 +48,11 @@ TEST(ActiveQuicListenerConfigTest, CreateActiveQuicListenerFactory) { EXPECT_EQ(2000u, quic_config.IdleNetworkTimeout().ToMilliseconds()); // Default value if not present in config. EXPECT_EQ(20000u, quic_config.max_time_before_crypto_handshake().ToMilliseconds()); + envoy::config::core::v3::RuntimeFeatureFlag& runtime_enabled = + ActiveQuicListenerFactoryPeer::runtimeEnabled( + dynamic_cast(*listener_factory)); + EXPECT_EQ(true, runtime_enabled.default_value().value()); + EXPECT_EQ("foo_key", runtime_enabled.runtime_key()); } } // namespace Quic diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 57b271844585..2f9da27bb1ea 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -63,8 +63,6 @@ class ActiveQuicListenerTest : public testing::TestWithParam(local_address_, nullptr, /*bind*/ true); @@ -77,7 +75,7 @@ class ActiveQuicListenerTest : public testing::TestWithParamonListenerShutdown(); // Trigger alarm to fire before listener destruction. dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + Runtime::LoaderSingleton::clear(); } - virtual envoy::config::core::v3::RuntimeFeatureFlag enabled_flag() const { +protected: + envoy::config::core::v3::RuntimeFeatureFlag enabled_flag() const { envoy::config::core::v3::RuntimeFeatureFlag enabled_proto; std::string yaml(R"EOF( runtime_key: "quic.enabled" @@ -206,6 +207,8 @@ default_value: true return enabled_proto; } + virtual bool enabled() const { return true; } + Network::Address::IpVersion version_; Event::SimulatedTimeSystemHelper simulated_time_system_; Api::ApiPtr api_; @@ -236,9 +239,15 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, ActiveQuicListenerTest, TestUtility::ipTestParamsToString); TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { - ConfigureMocks(/* connection_count = */ 1); + EnvoyQuicDispatcher* const envoy_quic_dispatcher = + ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); + quic::QuicBufferedPacketStore* const buffered_packets = + quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); + ConfigureMocks(/* connection_count = */ 1, /* runtime_enabled = */ true); SendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + EXPECT_FALSE(buffered_packets->HasChlosBuffered()); + EXPECT_FALSE(envoy_quic_dispatcher->session_map().empty()); ReadFromClientSockets(); } @@ -248,7 +257,7 @@ TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { quic::QuicBufferedPacketStore* const buffered_packets = quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); - ConfigureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2); + ConfigureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2, /* runtime_enabled = */ true); // Generate one more CHLO than can be processed immediately. for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop + 1; ++i) { @@ -264,6 +273,7 @@ TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { EXPECT_TRUE(buffered_packets->HasBufferedPackets( quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 1))); EXPECT_TRUE(buffered_packets->HasChlosBuffered()); + EXPECT_FALSE(envoy_quic_dispatcher->session_map().empty()); // Generate more data to trigger a socket read during the next event loop. SendFullCHLO(quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2)); @@ -278,20 +288,19 @@ TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { ReadFromClientSockets(); } -TEST_P(ActiveQuicListenerTest, QuicProcessingEnabled) {} - -class ActiveQuicListenerDisabledTest : public ActiveQuicListenerTest { -protected: - envoy::config::core::v3::RuntimeFeatureFlag enabled_flag() const override { - envoy::config::core::v3::RuntimeFeatureFlag enabled_proto; - std::string yaml(R"EOF( -runtime_key: "quic.enabled" -default_value: false -)EOF"); - TestUtility::loadFromYamlAndValidate(yaml, enabled_proto); - return enabled_proto; - } -}; +TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { + EnvoyQuicDispatcher* const envoy_quic_dispatcher = + ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); + // quic::QuicBufferedPacketStore* const buffered_packets = + // quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); + EXPECT_CALL(runtime_.snapshot_, getBoolean("quic.enabled", true)).WillRepeatedly(Return(false)); + SendFullCHLO(quic::test::TestConnectionId(1)); + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + // if listener was enabled, there should have been session created for active connection + EXPECT_TRUE(envoy_quic_dispatcher->session_map().empty()); + // EXPECT_FALSE(buffered_packets->HasBufferedPackets(quic::test::TestConnectionId(1))); + ReadFromClientSockets(); +} } // namespace Quic } // namespace Envoy From 9502c3e052bc3247eed301b13f7c7a5fea6f9014 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 5 Mar 2020 13:17:06 +0100 Subject: [PATCH 11/58] fix format Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener.cc | 6 +++--- .../quic_listeners/quiche/active_quic_listener.h | 5 +++-- test/extensions/quic_listeners/quiche/BUILD | 1 - .../quiche/active_quic_listener_test.cc | 11 ++++++----- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index b28526894b5f..f92b380b8b27 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -21,7 +21,7 @@ ActiveQuicListener::ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, Network::Socket::OptionsSharedPtr options, - const envoy::config::core::v3::RuntimeFeatureFlag enabled) + const envoy::config::core::v3::RuntimeFeatureFlag enabled) : ActiveQuicListener(dispatcher, parent, listener_config.listenSocketFactory().getListenSocket(), listener_config, quic_config, std::move(options), enabled) {} @@ -32,7 +32,7 @@ ActiveQuicListener::ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, Network::Socket::OptionsSharedPtr options, - const envoy::config::core::v3::RuntimeFeatureFlag enabled) + const envoy::config::core::v3::RuntimeFeatureFlag enabled) : Server::ConnectionHandlerImpl::ActiveListenerImplBase(parent, listener_config), dispatcher_(dispatcher), version_manager_(quic::CurrentSupportedVersions()), listen_socket_(*listen_socket), enabled_(enabled, Runtime::LoaderSingleton::get()) { @@ -197,7 +197,7 @@ ActiveQuicListenerFactory::createActiveUdpListener(Network::ConnectionHandler& p } #endif return std::make_unique(disptacher, parent, config, quic_config_, - std::move(options)); + std::move(options), enabled_); } } // namespace Quic diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.h b/source/extensions/quic_listeners/quiche/active_quic_listener.h index b856f8a2d690..a4353618207d 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.h +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.h @@ -28,13 +28,13 @@ class ActiveQuicListener : public Network::UdpListenerCallbacks, ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, Network::Socket::OptionsSharedPtr options, - const envoy::config::core::v3::RuntimeFeatureFlag enabled); + const envoy::config::core::v3::RuntimeFeatureFlag enabled); ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, Network::SocketSharedPtr listen_socket, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, Network::Socket::OptionsSharedPtr options, - const envoy::config::core::v3::RuntimeFeatureFlag enabled); + const envoy::config::core::v3::RuntimeFeatureFlag enabled); ~ActiveQuicListener() override; @@ -89,6 +89,7 @@ class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory, quic::QuicConfig quic_config_; const uint32_t concurrency_; absl::once_flag install_bpf_once_; + envoy::config::core::v3::RuntimeFeatureFlag enabled_; }; } // namespace Quic diff --git a/test/extensions/quic_listeners/quiche/BUILD b/test/extensions/quic_listeners/quiche/BUILD index 0b1af433c046..f7b52aed56d2 100644 --- a/test/extensions/quic_listeners/quiche/BUILD +++ b/test/extensions/quic_listeners/quiche/BUILD @@ -150,7 +150,6 @@ envoy_cc_test( "//test/test_common:environment_lib", "//test/test_common:network_utility_lib", "//test/test_common:simulated_time_system_lib", - "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], ) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 7822d6080899..31b886543745 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -68,8 +68,9 @@ class ActiveQuicListenerTest : public testing::TestWithParamaddOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions()); listen_socket_->addOptions(Network::SocketOptionFactory::buildRxQueueOverFlowOptions()); - quic_listener_ = std::make_unique( - *dispatcher_, connection_handler_, listen_socket_, listener_config_, quic_config_, nullptr, enabled_flag()); + quic_listener_ = std::make_unique(*dispatcher_, connection_handler_, + listen_socket_, listener_config_, + quic_config_, nullptr, enabled_flag()); simulated_time_system_.sleep(std::chrono::milliseconds(100)); } @@ -301,14 +302,14 @@ TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { EnvoyQuicDispatcher* const envoy_quic_dispatcher = ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); - // quic::QuicBufferedPacketStore* const buffered_packets = - // quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); + quic::QuicBufferedPacketStore* const buffered_packets = + quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); EXPECT_CALL(runtime_.snapshot_, getBoolean("quic.enabled", true)).WillRepeatedly(Return(false)); SendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); // if listener was enabled, there should have been session created for active connection EXPECT_TRUE(envoy_quic_dispatcher->session_map().empty()); - // EXPECT_FALSE(buffered_packets->HasBufferedPackets(quic::test::TestConnectionId(1))); + EXPECT_FALSE(buffered_packets->HasBufferedPackets(quic::test::TestConnectionId(1))); ReadFromClientSockets(); } From 69403889d0d8e768026a66a135082d32b443f525 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 5 Mar 2020 17:03:28 +0100 Subject: [PATCH 12/58] clean up Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_test.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 31b886543745..445258ac252a 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -206,8 +206,6 @@ default_value: true return enabled_proto; } - virtual bool enabled() const { return true; } - Network::Address::IpVersion version_; Event::SimulatedTimeSystemHelper simulated_time_system_; Api::ApiPtr api_; From 2f8e4deede5120353a74a87064320b18e51d5023 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 5 Mar 2020 18:00:00 +0100 Subject: [PATCH 13/58] Fix test Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 445258ac252a..bb1b7b70ccc8 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -243,7 +243,7 @@ TEST_P(ActiveQuicListenerTest, FailSocketOptionUponCreation) { options->emplace_back(std::move(option)); EXPECT_THROW_WITH_REGEX(std::make_unique(*dispatcher_, connection_handler_, listen_socket_, listener_config_, - quic_config_, options), + quic_config_, options, enabled_flag()), EnvoyException, "Failed to apply socket options."); } From ccf4bb18f24e35556fb61f5dc4b13f786c3ff826 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Fri, 6 Mar 2020 11:17:09 +0100 Subject: [PATCH 14/58] fix test Signed-off-by: Kateryna Nezdolii --- test/extensions/quic_listeners/quiche/BUILD | 1 + .../quiche/active_quic_listener_test.cc | 11 +++++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/BUILD b/test/extensions/quic_listeners/quiche/BUILD index f7b52aed56d2..0b1af433c046 100644 --- a/test/extensions/quic_listeners/quiche/BUILD +++ b/test/extensions/quic_listeners/quiche/BUILD @@ -150,6 +150,7 @@ envoy_cc_test( "//test/test_common:environment_lib", "//test/test_common:network_utility_lib", "//test/test_common:simulated_time_system_lib", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], ) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index bb1b7b70ccc8..035f40d24dc1 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -8,6 +8,9 @@ #include +#include "envoy/config/core/v3/base.pb.h" +#include "envoy/config/core/v3/base.pb.validate.h" + #include "quiche/quic/core/crypto/crypto_protocol.h" #include "quiche/quic/test_tools/crypto_test_utils.h" #include "quiche/quic/test_tools/quic_dispatcher_peer.h" @@ -241,10 +244,10 @@ TEST_P(ActiveQuicListenerTest, FailSocketOptionUponCreation) { .WillOnce(Return(false)); auto options = std::make_shared>(); options->emplace_back(std::move(option)); - EXPECT_THROW_WITH_REGEX(std::make_unique(*dispatcher_, connection_handler_, - listen_socket_, listener_config_, - quic_config_, options, enabled_flag()), - EnvoyException, "Failed to apply socket options."); + EXPECT_THROW_WITH_REGEX( + std::make_unique(*dispatcher_, connection_handler_, listen_socket_, + listener_config_, quic_config_, options, enabled_flag()), + EnvoyException, "Failed to apply socket options."); } TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { From cef470d83868505bf2dc2e2f055944420bca62e9 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Wed, 18 Mar 2020 12:58:05 +0100 Subject: [PATCH 15/58] apply review comments Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener.cc | 8 -------- .../quic_listeners/quiche/active_quic_listener_test.cc | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index f92b380b8b27..06702ba2de39 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -74,10 +74,6 @@ void ActiveQuicListener::onListenerShutdown() { } void ActiveQuicListener::onData(Network::UdpRecvData& data) { - if (!enabled()) { - ENVOY_LOG(trace, "Quic listener {}: runtime disabled", config_.name()); - return; - } quic::QuicSocketAddress peer_address( envoyAddressInstanceToQuicSocketAddress(data.addresses_.peer_)); quic::QuicSocketAddress self_address( @@ -108,10 +104,6 @@ void ActiveQuicListener::onReadReady() { } void ActiveQuicListener::onWriteReady(const Network::Socket& /*socket*/) { - if (!enabled()) { - ENVOY_LOG(trace, "Quic listener {}: runtime disabled", config_.name()); - return; - } quic_dispatcher_->OnCanWrite(); } diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 035f40d24dc1..c02df7ea160a 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -308,7 +308,7 @@ TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { EXPECT_CALL(runtime_.snapshot_, getBoolean("quic.enabled", true)).WillRepeatedly(Return(false)); SendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - // if listener was enabled, there should have been session created for active connection + // If listener was enabled, there should have been session created for active connection. EXPECT_TRUE(envoy_quic_dispatcher->session_map().empty()); EXPECT_FALSE(buffered_packets->HasBufferedPackets(quic::test::TestConnectionId(1))); ReadFromClientSockets(); From 80556e1cfd34d0c111bdca6659805c1c791f65ca Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Wed, 18 Mar 2020 16:41:04 +0100 Subject: [PATCH 16/58] apply review comments Signed-off-by: Kateryna Nezdolii --- .../quiche/active_quic_listener.cc | 4 +++ .../active_quic_listener_config_test.cc | 25 +++++++++++++++++++ .../quiche/active_quic_listener_test.cc | 1 - 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index 06702ba2de39..0140e5026789 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -104,6 +104,10 @@ void ActiveQuicListener::onReadReady() { } void ActiveQuicListener::onWriteReady(const Network::Socket& /*socket*/) { + if (!enabled()) { + ENVOY_LOG(trace, "Quic listener {}: runtime disabled", config_.name()); + return; + } quic_dispatcher_->OnCanWrite(); } diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc index d71cbd711de5..7e43de1114fb 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc @@ -55,5 +55,30 @@ TEST(ActiveQuicListenerConfigTest, CreateActiveQuicListenerFactory) { EXPECT_EQ("foo_key", runtime_enabled.runtime_key()); } +TEST(ActiveQuicListenerConfigTest, QuicEnabledByDefault) { + std::string listener_name = QuicListenerName; + auto& config_factory = + Config::Utility::getAndCheckFactoryByName( + listener_name); + ProtobufTypes::MessagePtr config = config_factory.createEmptyConfigProto(); + + std::string yaml = R"EOF( + max_concurrent_streams: 10 + idle_timeout: { + seconds: 2 + } + )EOF"; + TestUtility::loadFromYaml(yaml, *config); + Network::ActiveUdpListenerFactoryPtr listener_factory = + config_factory.createActiveUdpListenerFactory(*config, /*concurrency=*/1); + EXPECT_NE(nullptr, listener_factory); + quic::QuicConfig& quic_config = ActiveQuicListenerFactoryPeer::quicConfig( + dynamic_cast(*listener_factory)); + envoy::config::core::v3::RuntimeFeatureFlag& runtime_enabled = + ActiveQuicListenerFactoryPeer::runtimeEnabled( + dynamic_cast(*listener_factory)); + EXPECT_EQ(true, runtime_enabled.default_value().value()); +} + } // namespace Quic } // namespace Envoy diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index c02df7ea160a..b25ac95ed1fd 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -310,7 +310,6 @@ TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { dispatcher_->run(Event::Dispatcher::RunType::NonBlock); // If listener was enabled, there should have been session created for active connection. EXPECT_TRUE(envoy_quic_dispatcher->session_map().empty()); - EXPECT_FALSE(buffered_packets->HasBufferedPackets(quic::test::TestConnectionId(1))); ReadFromClientSockets(); } From d77c6a1461a073b9a60275ac2af8cfa76abc3edd Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Wed, 18 Mar 2020 17:11:18 +0100 Subject: [PATCH 17/58] remove unused var Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_config_test.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc index 7e43de1114fb..b0c4dd7b5e00 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc @@ -72,8 +72,6 @@ TEST(ActiveQuicListenerConfigTest, QuicEnabledByDefault) { Network::ActiveUdpListenerFactoryPtr listener_factory = config_factory.createActiveUdpListenerFactory(*config, /*concurrency=*/1); EXPECT_NE(nullptr, listener_factory); - quic::QuicConfig& quic_config = ActiveQuicListenerFactoryPeer::quicConfig( - dynamic_cast(*listener_factory)); envoy::config::core::v3::RuntimeFeatureFlag& runtime_enabled = ActiveQuicListenerFactoryPeer::runtimeEnabled( dynamic_cast(*listener_factory)); From 132cce0f58bcca6c0692309d87a5271db73c497f Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Wed, 18 Mar 2020 17:27:17 +0100 Subject: [PATCH 18/58] remove unused var Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_test.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index b25ac95ed1fd..9dc58b9b6dd6 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -303,8 +303,6 @@ TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { EnvoyQuicDispatcher* const envoy_quic_dispatcher = ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); - quic::QuicBufferedPacketStore* const buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); EXPECT_CALL(runtime_.snapshot_, getBoolean("quic.enabled", true)).WillRepeatedly(Return(false)); SendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); From e3ac7b0ead7840431134b23ad27f8b412741d76d Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 19 Mar 2020 12:05:44 +0100 Subject: [PATCH 19/58] remove invalid test Signed-off-by: Kateryna Nezdolii --- .../active_quic_listener_config_test.cc | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc index b0c4dd7b5e00..d71cbd711de5 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc @@ -55,28 +55,5 @@ TEST(ActiveQuicListenerConfigTest, CreateActiveQuicListenerFactory) { EXPECT_EQ("foo_key", runtime_enabled.runtime_key()); } -TEST(ActiveQuicListenerConfigTest, QuicEnabledByDefault) { - std::string listener_name = QuicListenerName; - auto& config_factory = - Config::Utility::getAndCheckFactoryByName( - listener_name); - ProtobufTypes::MessagePtr config = config_factory.createEmptyConfigProto(); - - std::string yaml = R"EOF( - max_concurrent_streams: 10 - idle_timeout: { - seconds: 2 - } - )EOF"; - TestUtility::loadFromYaml(yaml, *config); - Network::ActiveUdpListenerFactoryPtr listener_factory = - config_factory.createActiveUdpListenerFactory(*config, /*concurrency=*/1); - EXPECT_NE(nullptr, listener_factory); - envoy::config::core::v3::RuntimeFeatureFlag& runtime_enabled = - ActiveQuicListenerFactoryPeer::runtimeEnabled( - dynamic_cast(*listener_factory)); - EXPECT_EQ(true, runtime_enabled.default_value().value()); -} - } // namespace Quic } // namespace Envoy From 0091d589c5770c7ec245612b1db72eb207594f2b Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Fri, 20 Mar 2020 12:30:08 +0100 Subject: [PATCH 20/58] Check if flag is enabled by default Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_test.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 9dc58b9b6dd6..69378ef69fca 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -95,8 +95,10 @@ class ActiveQuicListenerTest : public testing::TestWithParam Date: Fri, 20 Mar 2020 14:35:28 +0100 Subject: [PATCH 21/58] Remove temp check Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_test.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 69378ef69fca..9dc58b9b6dd6 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -95,10 +95,8 @@ class ActiveQuicListenerTest : public testing::TestWithParam Date: Mon, 30 Mar 2020 12:31:39 +0200 Subject: [PATCH 22/58] apply review comments Signed-off-by: Kateryna Nezdolii --- .../quiche/active_quic_listener_test.cc | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 9dc58b9b6dd6..d2e4dfb22671 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -77,7 +77,12 @@ class ActiveQuicListenerTest : public testing::TestWithParam(); @@ -255,7 +257,8 @@ TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); quic::QuicBufferedPacketStore* const buffered_packets = quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); - ConfigureMocks(/* connection_count = */ 1, /* runtime_enabled = */ true); + configureQuicRuntimeFlag(/* runtime_enabled = */ true); + ConfigureMocks(/* connection_count = */ 1); SendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); EXPECT_FALSE(buffered_packets->HasChlosBuffered()); @@ -263,13 +266,28 @@ TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { ReadFromClientSockets(); } -TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { +/* Not configuring runtime key for `quic.enabled` +and checking that quic processing is enabled by default. */ +TEST_P(ActiveQuicListenerTest, ReceiveFullChloQuicEnabledByDefault) { EnvoyQuicDispatcher* const envoy_quic_dispatcher = ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); quic::QuicBufferedPacketStore* const buffered_packets = quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); + ConfigureMocks(/* connection_count = */ 1); + SendFullCHLO(quic::test::TestConnectionId(1)); + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + EXPECT_FALSE(buffered_packets->HasChlosBuffered()); + EXPECT_FALSE(envoy_quic_dispatcher->session_map().empty()); + ReadFromClientSockets(); +} - ConfigureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2, /* runtime_enabled = */ true); +TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { + EnvoyQuicDispatcher* const envoy_quic_dispatcher = + ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); + quic::QuicBufferedPacketStore* const buffered_packets = + quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); + configureQuicRuntimeFlag(/* runtime_enabled = */ true); + ConfigureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2); // Generate one more CHLO than can be processed immediately. for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop + 1; ++i) { @@ -303,12 +321,11 @@ TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { EnvoyQuicDispatcher* const envoy_quic_dispatcher = ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); - EXPECT_CALL(runtime_.snapshot_, getBoolean("quic.enabled", true)).WillRepeatedly(Return(false)); + configureQuicRuntimeFlag(/* runtime_enabled = */ false); SendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); // If listener was enabled, there should have been session created for active connection. EXPECT_TRUE(envoy_quic_dispatcher->session_map().empty()); - ReadFromClientSockets(); } } // namespace Quic From ed64f3a69d56120e9c4ef48328f1f1236f370730 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Fri, 3 Apr 2020 12:50:09 +0200 Subject: [PATCH 23/58] make clang happy, not creating temp copy Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index d2e4dfb22671..46cce914e53b 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -129,7 +129,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam(local_address_, nullptr, /*bind*/ false)); + client_sockets_.emplace_back(std::make_unique(local_address_, nullptr, /*bind*/ false)); quic::CryptoHandshakeMessage chlo = quic::test::crypto_test_utils::GenerateDefaultInchoateCHLO( &clock_, quic::AllSupportedVersions()[0].transport_version, &ActiveQuicListenerPeer::crypto_config(*quic_listener_)); From d914ed03799bcbdd16c330ec4bbcd2dd765d163f Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Fri, 3 Apr 2020 15:22:07 +0200 Subject: [PATCH 24/58] Not call virtual func from desctructor Signed-off-by: Kateryna Nezdolii --- source/common/network/listen_socket_impl.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/source/common/network/listen_socket_impl.h b/source/common/network/listen_socket_impl.h index 519960522c82..2acddf6a8ee0 100644 --- a/source/common/network/listen_socket_impl.h +++ b/source/common/network/listen_socket_impl.h @@ -15,7 +15,11 @@ namespace Network { class SocketImpl : public virtual Socket { public: - ~SocketImpl() override { close(); } + ~SocketImpl() override { + if (io_handle_->isOpen()) { + io_handle_->close(); + } + } // Network::Socket const Address::InstanceConstSharedPtr& localAddress() const override { return local_address_; } From dd5b6ed251de8a03c7fc12b23de95a9d20c7bb25 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Fri, 3 Apr 2020 17:20:28 +0200 Subject: [PATCH 25/58] fix naming for clang_tidy Signed-off-by: Kateryna Nezdolii --- source/common/network/listen_socket_impl.h | 6 +----- .../quiche/active_quic_listener_test.cc | 12 ++++++------ 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/source/common/network/listen_socket_impl.h b/source/common/network/listen_socket_impl.h index 2acddf6a8ee0..a0082017c7aa 100644 --- a/source/common/network/listen_socket_impl.h +++ b/source/common/network/listen_socket_impl.h @@ -15,11 +15,7 @@ namespace Network { class SocketImpl : public virtual Socket { public: - ~SocketImpl() override { - if (io_handle_->isOpen()) { - io_handle_->close(); - } - } + ~SocketImpl() override{}; // Network::Socket const Address::InstanceConstSharedPtr& localAddress() const override { return local_address_; } diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 46cce914e53b..cd77bcefbb94 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -73,7 +73,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam(*dispatcher_, connection_handler_, listen_socket_, listener_config_, - quic_config_, nullptr, enabled_flag()); + quic_config_, nullptr, enabledFlag()); simulated_time_system_.sleep(std::chrono::milliseconds(100)); } @@ -82,7 +82,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam(local_address_, nullptr, /*bind*/ false)); + void sendFullCHLO(quic::QuicConnectionId connection_id) { + client_sockets_.push_back(std::make_unique(local_address_, nullptr, /*bind*/ false)); quic::CryptoHandshakeMessage chlo = quic::test::crypto_test_utils::GenerateDefaultInchoateCHLO( &clock_, quic::AllSupportedVersions()[0].transport_version, &ActiveQuicListenerPeer::crypto_config(*quic_listener_)); @@ -201,7 +201,7 @@ class ActiveQuicListenerTest : public testing::TestWithParamemplace_back(std::move(option)); EXPECT_THROW_WITH_REGEX( std::make_unique(*dispatcher_, connection_handler_, listen_socket_, - listener_config_, quic_config_, options, enabled_flag()), + listener_config_, quic_config_, options, enabledFlag()), EnvoyException, "Failed to apply socket options."); } From 60b70346fecbc359acf0be1f05603b02abc641bc Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Mon, 6 Apr 2020 13:11:36 +0200 Subject: [PATCH 26/58] fix api shadow Signed-off-by: Kateryna Nezdolii --- .../envoy/api/v2/listener/quic_config.proto | 2 +- .../envoy/config/listener/v3/quic_config.proto | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/generated_api_shadow/envoy/api/v2/listener/quic_config.proto b/generated_api_shadow/envoy/api/v2/listener/quic_config.proto index 6e915baee74f..b29513a07e4d 100644 --- a/generated_api_shadow/envoy/api/v2/listener/quic_config.proto +++ b/generated_api_shadow/envoy/api/v2/listener/quic_config.proto @@ -21,7 +21,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // [#protodoc-title: QUIC listener Config] // Configuration specific to the QUIC protocol. -// Next id: 4 +// Next id: 5 message QuicProtocolOptions { // Maximum number of streams that the client can negotiate per connection. 100 // if not specified. diff --git a/generated_api_shadow/envoy/config/listener/v3/quic_config.proto b/generated_api_shadow/envoy/config/listener/v3/quic_config.proto index 1a8f9d49840b..8b606b016d1e 100644 --- a/generated_api_shadow/envoy/config/listener/v3/quic_config.proto +++ b/generated_api_shadow/envoy/config/listener/v3/quic_config.proto @@ -4,8 +4,6 @@ package envoy.config.listener.v3; import "envoy/config/core/v3/base.proto"; -import "envoy/config/core/v3/base.proto"; - import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; @@ -37,13 +35,6 @@ message QuicProtocolOptions { // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; - // Runtime flag that controls whether the listener is enabled or not. If not specified, defaults - // to enabled. - core.v3.RuntimeFeatureFlag enabled = 4; -import "udpa/annotations/status.proto"; - option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE; - // Next id: 5 - // Runtime flag that controls whether the listener is enabled or not. If not specified, defaults // to enabled. core.v3.RuntimeFeatureFlag enabled = 4; From d27bc454546cbfefc0ac7fd6dbaf924f70830cba Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Mon, 6 Apr 2020 14:10:45 +0200 Subject: [PATCH 27/58] fix namings in test Signed-off-by: Kateryna Nezdolii --- .../quiche/active_quic_listener_test.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index cd77bcefbb94..4ff2c6db8636 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -258,8 +258,8 @@ TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { quic::QuicBufferedPacketStore* const buffered_packets = quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); configureQuicRuntimeFlag(/* runtime_enabled = */ true); - ConfigureMocks(/* connection_count = */ 1); - SendFullCHLO(quic::test::TestConnectionId(1)); + configureMocks(/* connection_count = */ 1); + sendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); EXPECT_FALSE(buffered_packets->HasChlosBuffered()); EXPECT_FALSE(envoy_quic_dispatcher->session_map().empty()); @@ -273,8 +273,8 @@ TEST_P(ActiveQuicListenerTest, ReceiveFullChloQuicEnabledByDefault) { ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); quic::QuicBufferedPacketStore* const buffered_packets = quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); - ConfigureMocks(/* connection_count = */ 1); - SendFullCHLO(quic::test::TestConnectionId(1)); + configureMocks(/* connection_count = */ 1); + sendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); EXPECT_FALSE(buffered_packets->HasChlosBuffered()); EXPECT_FALSE(envoy_quic_dispatcher->session_map().empty()); @@ -287,11 +287,11 @@ TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { quic::QuicBufferedPacketStore* const buffered_packets = quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); configureQuicRuntimeFlag(/* runtime_enabled = */ true); - ConfigureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2); + configureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2); // Generate one more CHLO than can be processed immediately. for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop + 1; ++i) { - SendFullCHLO(quic::test::TestConnectionId(i)); + sendFullCHLO(quic::test::TestConnectionId(i)); } dispatcher_->run(Event::Dispatcher::RunType::NonBlock); @@ -306,7 +306,7 @@ TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { EXPECT_FALSE(envoy_quic_dispatcher->session_map().empty()); // Generate more data to trigger a socket read during the next event loop. - SendFullCHLO(quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2)); + sendFullCHLO(quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); // The socket read results in processing all CHLOs. @@ -322,7 +322,7 @@ TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { EnvoyQuicDispatcher* const envoy_quic_dispatcher = ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); configureQuicRuntimeFlag(/* runtime_enabled = */ false); - SendFullCHLO(quic::test::TestConnectionId(1)); + sendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); // If listener was enabled, there should have been session created for active connection. EXPECT_TRUE(envoy_quic_dispatcher->session_map().empty()); From e98f6732236b32f38e132ec0c75b50db7f1b5393 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Mon, 13 Apr 2020 14:26:21 +0200 Subject: [PATCH 28/58] apply review comments Signed-off-by: Kateryna Nezdolii --- .../quiche/active_quic_listener.cc | 4 --- .../active_quic_listener_config_test.cc | 26 +++++++++++++++++++ .../quiche/active_quic_listener_test.cc | 25 +++++++----------- 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index 500b082dd300..75fab083ec51 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -103,10 +103,6 @@ void ActiveQuicListener::onReadReady() { } void ActiveQuicListener::onWriteReady(const Network::Socket& /*socket*/) { - if (!enabled()) { - ENVOY_LOG(trace, "Quic listener {}: runtime disabled", config_.name()); - return; - } quic_dispatcher_->OnCanWrite(); } diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc index d71cbd711de5..2a35fa75e23d 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc @@ -55,5 +55,31 @@ TEST(ActiveQuicListenerConfigTest, CreateActiveQuicListenerFactory) { EXPECT_EQ("foo_key", runtime_enabled.runtime_key()); } +TEST(ActiveQuicListenerConfigTest, QuicListenerEnabledByDefault) { + std::string listener_name = QuicListenerName; + auto& config_factory = + Config::Utility::getAndCheckFactoryByName( + listener_name); + ProtobufTypes::MessagePtr config = config_factory.createEmptyConfigProto(); + + std::string yaml = R"EOF( + max_concurrent_streams: 10 + idle_timeout: { + seconds: 2 + } + )EOF"; + TestUtility::loadFromYaml(yaml, *config); + Network::ActiveUdpListenerFactoryPtr listener_factory = + config_factory.createActiveUdpListenerFactory(*config, /*concurrency=*/1); + EXPECT_NE(nullptr, listener_factory); + quic::QuicConfig& quic_config = ActiveQuicListenerFactoryPeer::quicConfig( + dynamic_cast(*listener_factory)); + envoy::config::core::v3::RuntimeFeatureFlag& runtime_enabled = + ActiveQuicListenerFactoryPeer::runtimeEnabled( + dynamic_cast(*listener_factory)); + EXPECT_EQ(true, runtime_enabled.default_value().value()); + EXPECT_EQ("foo_key", runtime_enabled.runtime_key()); +} + } // namespace Quic } // namespace Envoy diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 4ff2c6db8636..ec6dcf6a1385 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -74,6 +74,8 @@ class ActiveQuicListenerTest : public testing::TestWithParam(*dispatcher_, connection_handler_, listen_socket_, listener_config_, quic_config_, nullptr, enabledFlag()); + quic_dispatcher_ = std::make_unique( + ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_)); simulated_time_system_.sleep(std::chrono::milliseconds(100)); } @@ -225,6 +227,7 @@ default_value: true quic::QuicConfig quic_config_; Server::ConnectionHandlerImpl connection_handler_; std::unique_ptr quic_listener_; + std::unique_ptr quic_dispatcher_; NiceMock runtime_; std::list> client_sockets_; @@ -253,39 +256,33 @@ TEST_P(ActiveQuicListenerTest, FailSocketOptionUponCreation) { } TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { - EnvoyQuicDispatcher* const envoy_quic_dispatcher = - ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); quic::QuicBufferedPacketStore* const buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); + quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); configureQuicRuntimeFlag(/* runtime_enabled = */ true); configureMocks(/* connection_count = */ 1); sendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); EXPECT_FALSE(buffered_packets->HasChlosBuffered()); - EXPECT_FALSE(envoy_quic_dispatcher->session_map().empty()); + EXPECT_FALSE(quic_dispatcher_->session_map().empty()); ReadFromClientSockets(); } /* Not configuring runtime key for `quic.enabled` and checking that quic processing is enabled by default. */ TEST_P(ActiveQuicListenerTest, ReceiveFullChloQuicEnabledByDefault) { - EnvoyQuicDispatcher* const envoy_quic_dispatcher = - ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); quic::QuicBufferedPacketStore* const buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); + quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); configureMocks(/* connection_count = */ 1); sendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); EXPECT_FALSE(buffered_packets->HasChlosBuffered()); - EXPECT_FALSE(envoy_quic_dispatcher->session_map().empty()); + EXPECT_FALSE(quic_dispatcher_->session_map().empty()); ReadFromClientSockets(); } TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { - EnvoyQuicDispatcher* const envoy_quic_dispatcher = - ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); quic::QuicBufferedPacketStore* const buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(envoy_quic_dispatcher); + quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); configureQuicRuntimeFlag(/* runtime_enabled = */ true); configureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2); @@ -303,7 +300,7 @@ TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { EXPECT_TRUE(buffered_packets->HasBufferedPackets( quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 1))); EXPECT_TRUE(buffered_packets->HasChlosBuffered()); - EXPECT_FALSE(envoy_quic_dispatcher->session_map().empty()); + EXPECT_FALSE(quic_dispatcher_->session_map().empty()); // Generate more data to trigger a socket read during the next event loop. sendFullCHLO(quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2)); @@ -319,13 +316,11 @@ TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { } TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { - EnvoyQuicDispatcher* const envoy_quic_dispatcher = - ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); configureQuicRuntimeFlag(/* runtime_enabled = */ false); sendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); // If listener was enabled, there should have been session created for active connection. - EXPECT_TRUE(envoy_quic_dispatcher->session_map().empty()); + EXPECT_TRUE(quic_dispatcher_->session_map().empty()); } } // namespace Quic From 019dc490438e6d6cd681fb74ab54f50d0dc4ac42 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Mon, 13 Apr 2020 15:24:07 +0200 Subject: [PATCH 29/58] remove changes for v2 Signed-off-by: Kateryna Nezdolii --- api/envoy/api/v2/listener/quic_config.proto | 6 +----- .../quic_listeners/quiche/active_quic_listener_test.cc | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/api/envoy/api/v2/listener/quic_config.proto b/api/envoy/api/v2/listener/quic_config.proto index b0ac71ba4d7e..6e1f37097b29 100644 --- a/api/envoy/api/v2/listener/quic_config.proto +++ b/api/envoy/api/v2/listener/quic_config.proto @@ -21,7 +21,7 @@ option (udpa.annotations.file_status).package_version_status = FROZEN; // [#protodoc-title: QUIC listener Config] // Configuration specific to the QUIC protocol. -// Next id: 5 +// Next id: 4 message QuicProtocolOptions { // Maximum number of streams that the client can negotiate per connection. 100 // if not specified. @@ -34,8 +34,4 @@ message QuicProtocolOptions { // Connection timeout in milliseconds before the crypto handshake is finished. // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; - - // Runtime flag that controls whether the listener is enabled or not. If not specified, defaults - // to enabled. - core.RuntimeFeatureFlag enabled = 4; } diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index f6bd4cce9a97..4be21e8605b4 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -79,7 +79,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam( ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_)); - simulated_time_system_.sleep(std::chrono::milliseconds(100)); + simulated_time_system_.advanceTimeWait(std::chrono::milliseconds(100)); } void configureMocks(int connection_count) { From 740d3d2e4403327dc7b16fc0162269c27745002b Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Mon, 13 Apr 2020 15:36:07 +0200 Subject: [PATCH 30/58] remove changes to v2 Signed-off-by: Kateryna Nezdolii --- api/envoy/api/v2/listener/quic_config.proto | 2 -- 1 file changed, 2 deletions(-) diff --git a/api/envoy/api/v2/listener/quic_config.proto b/api/envoy/api/v2/listener/quic_config.proto index 6e1f37097b29..2a4616bb09c9 100644 --- a/api/envoy/api/v2/listener/quic_config.proto +++ b/api/envoy/api/v2/listener/quic_config.proto @@ -2,8 +2,6 @@ syntax = "proto3"; package envoy.api.v2.listener; -import "envoy/api/v2/core/base.proto"; - import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; From 1b7a7f501e1fa1a662dcc10b0bf934222baa5aec Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Mon, 13 Apr 2020 17:17:34 +0200 Subject: [PATCH 31/58] fix format Signed-off-by: Kateryna Nezdolii --- .../quiche/active_quic_listener_test.cc | 458 +++++++++--------- 1 file changed, 231 insertions(+), 227 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 4be21e8605b4..5de9e427434d 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -83,249 +83,253 @@ class ActiveQuicListenerTest : public testing::TestWithParam& filter_factories) { - EXPECT_EQ(1u, filter_factories.size()); - Server::Configuration::FilterChainUtility::buildFilterChain(connection, filter_factories); - return true; - })); - EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::Connected)) - .Times(connection_count); - EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::LocalClose)) - .Times(connection_count); - - testing::Sequence seq; - for (int i = 0; i < connection_count; ++i) { - auto read_filter = std::make_shared(); - filter_factories_.push_back( - {Network::FilterFactoryCb([read_filter, this](Network::FilterManager& filter_manager) { - filter_manager.addReadFilter(read_filter); - read_filter->callbacks_->connection().addConnectionCallbacks( - network_connection_callbacks_); - })}); - // Stop iteration to avoid calling getRead/WriteBuffer(). - EXPECT_CALL(*read_filter, onNewConnection()) - .WillOnce(Return(Network::FilterStatus::StopIteration)); - read_filters_.push_back(std::move(read_filter)); - - filter_chains_.emplace_back(); - EXPECT_CALL(filter_chains_.back(), networkFilterFactories()) - .WillOnce(ReturnRef(filter_factories_.back())); - - // A Sequence must be used to allow multiple EXPECT_CALL().WillOnce() - // calls for the same object. - EXPECT_CALL(filter_chain_manager_, findFilterChain(_)) - .InSequence(seq) - .WillOnce(Return(&filter_chains_.back())); + void configureMocks(int connection_count) { + EXPECT_CALL(listener_config_, filterChainManager()) + .Times(connection_count) + .WillRepeatedly(ReturnRef(filter_chain_manager_)); + EXPECT_CALL(listener_config_, filterChainFactory()).Times(connection_count); + EXPECT_CALL(listener_config_.filter_chain_factory_, createNetworkFilterChain(_, _)) + .Times(connection_count) + .WillRepeatedly(Invoke([](Network::Connection& connection, + const std::vector& filter_factories) { + EXPECT_EQ(1u, filter_factories.size()); + Server::Configuration::FilterChainUtility::buildFilterChain(connection, + filter_factories); + return true; + })); + EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::Connected)) + .Times(connection_count); + EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::LocalClose)) + .Times(connection_count); + + testing::Sequence seq; + for (int i = 0; i < connection_count; ++i) { + auto read_filter = std::make_shared(); + filter_factories_.push_back( + {Network::FilterFactoryCb([read_filter, this](Network::FilterManager& filter_manager) { + filter_manager.addReadFilter(read_filter); + read_filter->callbacks_->connection().addConnectionCallbacks( + network_connection_callbacks_); + })}); + // Stop iteration to avoid calling getRead/WriteBuffer(). + EXPECT_CALL(*read_filter, onNewConnection()) + .WillOnce(Return(Network::FilterStatus::StopIteration)); + read_filters_.push_back(std::move(read_filter)); + + filter_chains_.emplace_back(); + EXPECT_CALL(filter_chains_.back(), networkFilterFactories()) + .WillOnce(ReturnRef(filter_factories_.back())); + + // A Sequence must be used to allow multiple EXPECT_CALL().WillOnce() + // calls for the same object. + EXPECT_CALL(filter_chain_manager_, findFilterChain(_)) + .InSequence(seq) + .WillOnce(Return(&filter_chains_.back())); + } } - } - // TODO(bencebeky): Factor out parts common with - // EnvoyQuicDispatcherTest::createFullChloPacket() to test_utils. - void sendFullCHLO(quic::QuicConnectionId connection_id) { - client_sockets_.push_back(std::make_unique(local_address_, nullptr, /*bind*/ false)); - quic::CryptoHandshakeMessage chlo = quic::test::crypto_test_utils::GenerateDefaultInchoateCHLO( - &clock_, quic::AllSupportedVersions()[0].transport_version, - &ActiveQuicListenerPeer::crypto_config(*quic_listener_)); - chlo.SetVector(quic::kCOPT, quic::QuicTagVector{quic::kREJ}); - quic::CryptoHandshakeMessage full_chlo; - quic::QuicReferenceCountedPointer signed_config( - new quic::QuicSignedServerConfig); - quic::QuicCompressedCertsCache cache( - quic::QuicCompressedCertsCache::kQuicCompressedCertsCacheSize); - quic::test::crypto_test_utils::GenerateFullCHLO( - chlo, &ActiveQuicListenerPeer::crypto_config(*quic_listener_), - envoyAddressInstanceToQuicSocketAddress(local_address_), - envoyAddressInstanceToQuicSocketAddress(local_address_), - quic::AllSupportedVersions()[0].transport_version, &clock_, signed_config, &cache, - &full_chlo); - // Overwrite version label to highest current supported version. - full_chlo.SetVersion(quic::kVER, quic::CurrentSupportedVersions()[0]); - quic::QuicConfig quic_config; - quic_config.ToHandshakeMessage(&full_chlo, - quic::CurrentSupportedVersions()[0].transport_version); - - std::string packet_content(full_chlo.GetSerialized().AsStringPiece()); - auto encrypted_packet = std::unique_ptr( - quic::test::ConstructEncryptedPacket(connection_id, quic::EmptyQuicConnectionId(), - /*version_flag=*/true, /*reset_flag*/ false, - /*packet_number=*/1, packet_content)); - - Buffer::RawSlice first_slice{ - reinterpret_cast(const_cast(encrypted_packet->data())), - encrypted_packet->length()}; - // Send a full CHLO to finish 0-RTT handshake. - auto send_rc = Network::Utility::writeToSocket(client_sockets_.back()->ioHandle(), &first_slice, - 1, nullptr, *listen_socket_->localAddress()); - ASSERT_EQ(encrypted_packet->length(), send_rc.rc_); - } + // TODO(bencebeky): Factor out parts common with + // EnvoyQuicDispatcherTest::createFullChloPacket() to test_utils. + void sendFullCHLO(quic::QuicConnectionId connection_id) { + client_sockets_.push_back(std::make_unique(local_address_, nullptr, /*bind*/ false)); + quic::CryptoHandshakeMessage chlo = + quic::test::crypto_test_utils::GenerateDefaultInchoateCHLO( + &clock_, quic::AllSupportedVersions()[0].transport_version, + &ActiveQuicListenerPeer::crypto_config(*quic_listener_)); + chlo.SetVector(quic::kCOPT, quic::QuicTagVector{quic::kREJ}); + quic::CryptoHandshakeMessage full_chlo; + quic::QuicReferenceCountedPointer signed_config( + new quic::QuicSignedServerConfig); + quic::QuicCompressedCertsCache cache( + quic::QuicCompressedCertsCache::kQuicCompressedCertsCacheSize); + quic::test::crypto_test_utils::GenerateFullCHLO( + chlo, &ActiveQuicListenerPeer::crypto_config(*quic_listener_), + envoyAddressInstanceToQuicSocketAddress(local_address_), + envoyAddressInstanceToQuicSocketAddress(local_address_), + quic::AllSupportedVersions()[0].transport_version, &clock_, signed_config, &cache, + &full_chlo); + // Overwrite version label to highest current supported version. + full_chlo.SetVersion(quic::kVER, quic::CurrentSupportedVersions()[0]); + quic::QuicConfig quic_config; + quic_config.ToHandshakeMessage(&full_chlo, + quic::CurrentSupportedVersions()[0].transport_version); + + std::string packet_content(full_chlo.GetSerialized().AsStringPiece()); + auto encrypted_packet = std::unique_ptr( + quic::test::ConstructEncryptedPacket(connection_id, quic::EmptyQuicConnectionId(), + /*version_flag=*/true, /*reset_flag*/ false, + /*packet_number=*/1, packet_content)); + + Buffer::RawSlice first_slice{ + reinterpret_cast(const_cast(encrypted_packet->data())), + encrypted_packet->length()}; + // Send a full CHLO to finish 0-RTT handshake. + auto send_rc = + Network::Utility::writeToSocket(client_sockets_.back()->ioHandle(), &first_slice, 1, + nullptr, *listen_socket_->localAddress()); + ASSERT_EQ(encrypted_packet->length(), send_rc.rc_); + } - void ReadFromClientSockets() { - for (auto& client_socket : client_sockets_) { - Buffer::InstancePtr result_buffer(new Buffer::OwnedImpl()); - const uint64_t bytes_to_read = 11; - uint64_t bytes_read = 0; - int retry = 0; - - do { - Api::IoCallUint64Result result = - result_buffer->read(client_socket->ioHandle(), bytes_to_read - bytes_read); - - if (result.ok()) { - bytes_read += result.rc_; - } else if (retry == 10 || result.err_->getErrorCode() != Api::IoError::IoErrorCode::Again) { - break; - } - - if (bytes_read == bytes_to_read) { - break; - } - - retry++; - absl::SleepFor(absl::Milliseconds(10)); - } while (true); + void ReadFromClientSockets() { + for (auto& client_socket : client_sockets_) { + Buffer::InstancePtr result_buffer(new Buffer::OwnedImpl()); + const uint64_t bytes_to_read = 11; + uint64_t bytes_read = 0; + int retry = 0; + + do { + Api::IoCallUint64Result result = + result_buffer->read(client_socket->ioHandle(), bytes_to_read - bytes_read); + + if (result.ok()) { + bytes_read += result.rc_; + } else if (retry == 10 || + result.err_->getErrorCode() != Api::IoError::IoErrorCode::Again) { + break; + } + + if (bytes_read == bytes_to_read) { + break; + } + + retry++; + absl::SleepFor(absl::Milliseconds(10)); + } while (true); + } } - } - void TearDown() override { - quic_listener_->onListenerShutdown(); - // Trigger alarm to fire before listener destruction. - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - Runtime::LoaderSingleton::clear(); - } + void TearDown() override { + quic_listener_->onListenerShutdown(); + // Trigger alarm to fire before listener destruction. + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + Runtime::LoaderSingleton::clear(); + } -protected: - envoy::config::core::v3::RuntimeFeatureFlag enabledFlag() const { - envoy::config::core::v3::RuntimeFeatureFlag enabled_proto; - std::string yaml(R"EOF( + protected: + envoy::config::core::v3::RuntimeFeatureFlag enabledFlag() const { + envoy::config::core::v3::RuntimeFeatureFlag enabled_proto; + std::string yaml(R"EOF( runtime_key: "quic.enabled" default_value: true )EOF"); - TestUtility::loadFromYamlAndValidate(yaml, enabled_proto); - return enabled_proto; - } - - Network::Address::IpVersion version_; - Event::SimulatedTimeSystemHelper simulated_time_system_; - Api::ApiPtr api_; - Event::DispatcherPtr dispatcher_; - EnvoyQuicClock clock_; - Network::Address::InstanceConstSharedPtr local_address_; - Network::SocketSharedPtr listen_socket_; - Network::SocketPtr client_socket_; - std::shared_ptr read_filter_; - Network::MockConnectionCallbacks network_connection_callbacks_; - NiceMock listener_config_; - quic::QuicConfig quic_config_; - Server::ConnectionHandlerImpl connection_handler_; - std::unique_ptr quic_listener_; - std::unique_ptr quic_dispatcher_; - NiceMock runtime_; - - std::list> client_sockets_; - std::list> read_filters_; - Network::MockFilterChainManager filter_chain_manager_; - // The following two containers must guarantee pointer stability as addresses - // of elements are saved in expectations before new elements are added. - std::list> filter_factories_; - std::list filter_chains_; -}; + TestUtility::loadFromYamlAndValidate(yaml, enabled_proto); + return enabled_proto; + } -INSTANTIATE_TEST_SUITE_P(IpVersions, ActiveQuicListenerTest, - testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), - TestUtility::ipTestParamsToString); - -TEST_P(ActiveQuicListenerTest, FailSocketOptionUponCreation) { - auto option = std::make_unique(); - EXPECT_CALL(*option, setOption(_, envoy::config::core::v3::SocketOption::STATE_BOUND)) - .WillOnce(Return(false)); - auto options = std::make_shared>(); - options->emplace_back(std::move(option)); - EXPECT_THROW_WITH_REGEX( - std::make_unique(*dispatcher_, connection_handler_, listen_socket_, - listener_config_, quic_config_, options, enabledFlag()), - EnvoyException, "Failed to apply socket options."); -} - -TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { - quic::QuicBufferedPacketStore* const buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); - configureQuicRuntimeFlag(/* runtime_enabled = */ true); - configureMocks(/* connection_count = */ 1); - sendFullCHLO(quic::test::TestConnectionId(1)); - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - EXPECT_FALSE(buffered_packets->HasChlosBuffered()); - EXPECT_FALSE(quic_dispatcher_->session_map().empty()); - ReadFromClientSockets(); -} - -/* Not configuring runtime key for `quic.enabled` -and checking that quic processing is enabled by default. */ -TEST_P(ActiveQuicListenerTest, ReceiveFullChloQuicEnabledByDefault) { - quic::QuicBufferedPacketStore* const buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); - configureMocks(/* connection_count = */ 1); - sendFullCHLO(quic::test::TestConnectionId(1)); - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - EXPECT_FALSE(buffered_packets->HasChlosBuffered()); - EXPECT_FALSE(quic_dispatcher_->session_map().empty()); - ReadFromClientSockets(); -} - -TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { - quic::QuicBufferedPacketStore* const buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); - configureQuicRuntimeFlag(/* runtime_enabled = */ true); - configureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2); - - // Generate one more CHLO than can be processed immediately. - for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop + 1; ++i) { - sendFullCHLO(quic::test::TestConnectionId(i)); + Network::Address::IpVersion version_; + Event::SimulatedTimeSystemHelper simulated_time_system_; + Api::ApiPtr api_; + Event::DispatcherPtr dispatcher_; + EnvoyQuicClock clock_; + Network::Address::InstanceConstSharedPtr local_address_; + Network::SocketSharedPtr listen_socket_; + Network::SocketPtr client_socket_; + std::shared_ptr read_filter_; + Network::MockConnectionCallbacks network_connection_callbacks_; + NiceMock listener_config_; + quic::QuicConfig quic_config_; + Server::ConnectionHandlerImpl connection_handler_; + std::unique_ptr quic_listener_; + std::unique_ptr quic_dispatcher_; + NiceMock runtime_; + + std::list> client_sockets_; + std::list> read_filters_; + Network::MockFilterChainManager filter_chain_manager_; + // The following two containers must guarantee pointer stability as addresses + // of elements are saved in expectations before new elements are added. + std::list> filter_factories_; + std::list filter_chains_; + }; + + INSTANTIATE_TEST_SUITE_P(IpVersions, ActiveQuicListenerTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), + TestUtility::ipTestParamsToString); + + TEST_P(ActiveQuicListenerTest, FailSocketOptionUponCreation) { + auto option = std::make_unique(); + EXPECT_CALL(*option, setOption(_, envoy::config::core::v3::SocketOption::STATE_BOUND)) + .WillOnce(Return(false)); + auto options = std::make_shared>(); + options->emplace_back(std::move(option)); + EXPECT_THROW_WITH_REGEX(std::make_unique( + *dispatcher_, connection_handler_, listen_socket_, listener_config_, + quic_config_, options, enabledFlag()), + EnvoyException, "Failed to apply socket options."); } - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - // The first kNumSessionsToCreatePerLoop CHLOs are processed, - // the last one is buffered. - for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop; ++i) { - EXPECT_FALSE(buffered_packets->HasBufferedPackets(quic::test::TestConnectionId(i))); + TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { + quic::QuicBufferedPacketStore* const buffered_packets = + quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); + configureQuicRuntimeFlag(/* runtime_enabled = */ true); + configureMocks(/* connection_count = */ 1); + sendFullCHLO(quic::test::TestConnectionId(1)); + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + EXPECT_FALSE(buffered_packets->HasChlosBuffered()); + EXPECT_FALSE(quic_dispatcher_->session_map().empty()); + ReadFromClientSockets(); } - EXPECT_TRUE(buffered_packets->HasBufferedPackets( - quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 1))); - EXPECT_TRUE(buffered_packets->HasChlosBuffered()); - EXPECT_FALSE(quic_dispatcher_->session_map().empty()); - - // Generate more data to trigger a socket read during the next event loop. - sendFullCHLO(quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2)); - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - - // The socket read results in processing all CHLOs. - for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop + 2; ++i) { - EXPECT_FALSE(buffered_packets->HasBufferedPackets(quic::test::TestConnectionId(i))); + + /* Not configuring runtime key for `quic.enabled` + and checking that quic processing is enabled by default. */ + TEST_P(ActiveQuicListenerTest, ReceiveFullChloQuicEnabledByDefault) { + quic::QuicBufferedPacketStore* const buffered_packets = + quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); + configureMocks(/* connection_count = */ 1); + sendFullCHLO(quic::test::TestConnectionId(1)); + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + EXPECT_FALSE(buffered_packets->HasChlosBuffered()); + EXPECT_FALSE(quic_dispatcher_->session_map().empty()); + ReadFromClientSockets(); } - EXPECT_FALSE(buffered_packets->HasChlosBuffered()); - ReadFromClientSockets(); -} + TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { + quic::QuicBufferedPacketStore* const buffered_packets = + quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); + configureQuicRuntimeFlag(/* runtime_enabled = */ true); + configureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2); + + // Generate one more CHLO than can be processed immediately. + for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop + 1; ++i) { + sendFullCHLO(quic::test::TestConnectionId(i)); + } + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + + // The first kNumSessionsToCreatePerLoop CHLOs are processed, + // the last one is buffered. + for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop; ++i) { + EXPECT_FALSE(buffered_packets->HasBufferedPackets(quic::test::TestConnectionId(i))); + } + EXPECT_TRUE(buffered_packets->HasBufferedPackets( + quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 1))); + EXPECT_TRUE(buffered_packets->HasChlosBuffered()); + EXPECT_FALSE(quic_dispatcher_->session_map().empty()); + + // Generate more data to trigger a socket read during the next event loop. + sendFullCHLO(quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2)); + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + + // The socket read results in processing all CHLOs. + for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop + 2; ++i) { + EXPECT_FALSE(buffered_packets->HasBufferedPackets(quic::test::TestConnectionId(i))); + } + EXPECT_FALSE(buffered_packets->HasChlosBuffered()); + + ReadFromClientSockets(); + } -TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { - configureQuicRuntimeFlag(/* runtime_enabled = */ false); - sendFullCHLO(quic::test::TestConnectionId(1)); - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - // If listener was enabled, there should have been session created for active connection. - EXPECT_TRUE(quic_dispatcher_->session_map().empty()); -} + TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { + configureQuicRuntimeFlag(/* runtime_enabled = */ false); + sendFullCHLO(quic::test::TestConnectionId(1)); + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + // If listener was enabled, there should have been session created for active connection. + EXPECT_TRUE(quic_dispatcher_->session_map().empty()); + } } // namespace Quic -} // namespace Envoy +} // namespace Quic From 9ae7c4230013e5a9b21faaf60fdb96e7d13f7729 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Tue, 14 Apr 2020 14:49:19 +0200 Subject: [PATCH 32/58] align shadow api Signed-off-by: Kateryna Nezdolii --- .../envoy/config/listener/v3/quic_config.proto | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/generated_api_shadow/envoy/config/listener/v3/quic_config.proto b/generated_api_shadow/envoy/config/listener/v3/quic_config.proto index 9949da2e0d70..c024be95bace 100644 --- a/generated_api_shadow/envoy/config/listener/v3/quic_config.proto +++ b/generated_api_shadow/envoy/config/listener/v3/quic_config.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package envoy.config.listener.v3; +import "envoy/config/core/v3/base.proto"; + import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; @@ -16,7 +18,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // [#protodoc-title: QUIC listener Config] // Configuration specific to the QUIC protocol. -// Next id: 4 +// Next id: 5 message QuicProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.listener.QuicProtocolOptions"; @@ -32,4 +34,8 @@ message QuicProtocolOptions { // Connection timeout in milliseconds before the crypto handshake is finished. // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; + + // Runtime flag that controls whether the listener is enabled or not. If not specified, defaults + // to enabled. + core.v3.RuntimeFeatureFlag enabled = 4; } From b25447e64b17ec20b56f30dd3eaa781f41985068 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Tue, 14 Apr 2020 17:32:45 +0200 Subject: [PATCH 33/58] remove unused var Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_config_test.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc index 2a35fa75e23d..a22aa3723a50 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc @@ -72,8 +72,6 @@ TEST(ActiveQuicListenerConfigTest, QuicListenerEnabledByDefault) { Network::ActiveUdpListenerFactoryPtr listener_factory = config_factory.createActiveUdpListenerFactory(*config, /*concurrency=*/1); EXPECT_NE(nullptr, listener_factory); - quic::QuicConfig& quic_config = ActiveQuicListenerFactoryPeer::quicConfig( - dynamic_cast(*listener_factory)); envoy::config::core::v3::RuntimeFeatureFlag& runtime_enabled = ActiveQuicListenerFactoryPeer::runtimeEnabled( dynamic_cast(*listener_factory)); From 7be1fd4566e1805b2260ed7cc7ea80b1665f9fd5 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Wed, 15 Apr 2020 12:41:59 +0200 Subject: [PATCH 34/58] fix merge conflict Signed-off-by: Kateryna Nezdolii --- .../quiche/active_quic_listener_test.cc | 462 +++++++++--------- 1 file changed, 227 insertions(+), 235 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 5de9e427434d..d1049f574c2c 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -71,9 +71,6 @@ class ActiveQuicListenerTest : public testing::TestWithParamaddOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions()); listen_socket_->addOptions(Network::SocketOptionFactory::buildRxQueueOverFlowOptions()); - quic_listener_ = std::make_unique( - *dispatcher_, connection_handler_, listen_socket_, listener_config_, quic_config_, nullptr); - simulated_time_system_.advanceTimeWait(std::chrono::milliseconds(100)); quic_listener_ = std::make_unique(*dispatcher_, connection_handler_, listen_socket_, listener_config_, quic_config_, nullptr, enabledFlag()); @@ -82,254 +79,249 @@ class ActiveQuicListenerTest : public testing::TestWithParam& filter_factories) { - EXPECT_EQ(1u, filter_factories.size()); - Server::Configuration::FilterChainUtility::buildFilterChain(connection, - filter_factories); - return true; - })); - EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::Connected)) - .Times(connection_count); - EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::LocalClose)) - .Times(connection_count); - - testing::Sequence seq; - for (int i = 0; i < connection_count; ++i) { - auto read_filter = std::make_shared(); - filter_factories_.push_back( - {Network::FilterFactoryCb([read_filter, this](Network::FilterManager& filter_manager) { - filter_manager.addReadFilter(read_filter); - read_filter->callbacks_->connection().addConnectionCallbacks( - network_connection_callbacks_); - })}); - // Stop iteration to avoid calling getRead/WriteBuffer(). - EXPECT_CALL(*read_filter, onNewConnection()) - .WillOnce(Return(Network::FilterStatus::StopIteration)); - read_filters_.push_back(std::move(read_filter)); - - filter_chains_.emplace_back(); - EXPECT_CALL(filter_chains_.back(), networkFilterFactories()) - .WillOnce(ReturnRef(filter_factories_.back())); - - // A Sequence must be used to allow multiple EXPECT_CALL().WillOnce() - // calls for the same object. - EXPECT_CALL(filter_chain_manager_, findFilterChain(_)) - .InSequence(seq) - .WillOnce(Return(&filter_chains_.back())); - } + void configureMocks(int connection_count) { + EXPECT_CALL(listener_config_, filterChainManager()) + .Times(connection_count) + .WillRepeatedly(ReturnRef(filter_chain_manager_)); + EXPECT_CALL(listener_config_, filterChainFactory()).Times(connection_count); + EXPECT_CALL(listener_config_.filter_chain_factory_, createNetworkFilterChain(_, _)) + .Times(connection_count) + .WillRepeatedly(Invoke([](Network::Connection& connection, + const std::vector& filter_factories) { + EXPECT_EQ(1u, filter_factories.size()); + Server::Configuration::FilterChainUtility::buildFilterChain(connection, filter_factories); + return true; + })); + EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::Connected)) + .Times(connection_count); + EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::LocalClose)) + .Times(connection_count); + + testing::Sequence seq; + for (int i = 0; i < connection_count; ++i) { + auto read_filter = std::make_shared(); + filter_factories_.push_back( + {Network::FilterFactoryCb([read_filter, this](Network::FilterManager& filter_manager) { + filter_manager.addReadFilter(read_filter); + read_filter->callbacks_->connection().addConnectionCallbacks( + network_connection_callbacks_); + })}); + // Stop iteration to avoid calling getRead/WriteBuffer(). + EXPECT_CALL(*read_filter, onNewConnection()) + .WillOnce(Return(Network::FilterStatus::StopIteration)); + read_filters_.push_back(std::move(read_filter)); + + filter_chains_.emplace_back(); + EXPECT_CALL(filter_chains_.back(), networkFilterFactories()) + .WillOnce(ReturnRef(filter_factories_.back())); + + // A Sequence must be used to allow multiple EXPECT_CALL().WillOnce() + // calls for the same object. + EXPECT_CALL(filter_chain_manager_, findFilterChain(_)) + .InSequence(seq) + .WillOnce(Return(&filter_chains_.back())); } + } - // TODO(bencebeky): Factor out parts common with - // EnvoyQuicDispatcherTest::createFullChloPacket() to test_utils. - void sendFullCHLO(quic::QuicConnectionId connection_id) { - client_sockets_.push_back(std::make_unique(local_address_, nullptr, /*bind*/ false)); - quic::CryptoHandshakeMessage chlo = - quic::test::crypto_test_utils::GenerateDefaultInchoateCHLO( - &clock_, quic::AllSupportedVersions()[0].transport_version, - &ActiveQuicListenerPeer::crypto_config(*quic_listener_)); - chlo.SetVector(quic::kCOPT, quic::QuicTagVector{quic::kREJ}); - quic::CryptoHandshakeMessage full_chlo; - quic::QuicReferenceCountedPointer signed_config( - new quic::QuicSignedServerConfig); - quic::QuicCompressedCertsCache cache( - quic::QuicCompressedCertsCache::kQuicCompressedCertsCacheSize); - quic::test::crypto_test_utils::GenerateFullCHLO( - chlo, &ActiveQuicListenerPeer::crypto_config(*quic_listener_), - envoyAddressInstanceToQuicSocketAddress(local_address_), - envoyAddressInstanceToQuicSocketAddress(local_address_), - quic::AllSupportedVersions()[0].transport_version, &clock_, signed_config, &cache, - &full_chlo); - // Overwrite version label to highest current supported version. - full_chlo.SetVersion(quic::kVER, quic::CurrentSupportedVersions()[0]); - quic::QuicConfig quic_config; - quic_config.ToHandshakeMessage(&full_chlo, - quic::CurrentSupportedVersions()[0].transport_version); - - std::string packet_content(full_chlo.GetSerialized().AsStringPiece()); - auto encrypted_packet = std::unique_ptr( - quic::test::ConstructEncryptedPacket(connection_id, quic::EmptyQuicConnectionId(), - /*version_flag=*/true, /*reset_flag*/ false, - /*packet_number=*/1, packet_content)); - - Buffer::RawSlice first_slice{ - reinterpret_cast(const_cast(encrypted_packet->data())), - encrypted_packet->length()}; - // Send a full CHLO to finish 0-RTT handshake. - auto send_rc = - Network::Utility::writeToSocket(client_sockets_.back()->ioHandle(), &first_slice, 1, - nullptr, *listen_socket_->localAddress()); - ASSERT_EQ(encrypted_packet->length(), send_rc.rc_); - } + // TODO(bencebeky): Factor out parts common with + // EnvoyQuicDispatcherTest::createFullChloPacket() to test_utils. + void sendFullCHLO(quic::QuicConnectionId connection_id) { + client_sockets_.push_back(std::make_unique(local_address_, nullptr, /*bind*/ false)); + quic::CryptoHandshakeMessage chlo = quic::test::crypto_test_utils::GenerateDefaultInchoateCHLO( + &clock_, quic::AllSupportedVersions()[0].transport_version, + &ActiveQuicListenerPeer::crypto_config(*quic_listener_)); + chlo.SetVector(quic::kCOPT, quic::QuicTagVector{quic::kREJ}); + quic::CryptoHandshakeMessage full_chlo; + quic::QuicReferenceCountedPointer signed_config( + new quic::QuicSignedServerConfig); + quic::QuicCompressedCertsCache cache( + quic::QuicCompressedCertsCache::kQuicCompressedCertsCacheSize); + quic::test::crypto_test_utils::GenerateFullCHLO( + chlo, &ActiveQuicListenerPeer::crypto_config(*quic_listener_), + envoyAddressInstanceToQuicSocketAddress(local_address_), + envoyAddressInstanceToQuicSocketAddress(local_address_), + quic::AllSupportedVersions()[0].transport_version, &clock_, signed_config, &cache, + &full_chlo); + // Overwrite version label to highest current supported version. + full_chlo.SetVersion(quic::kVER, quic::CurrentSupportedVersions()[0]); + quic::QuicConfig quic_config; + quic_config.ToHandshakeMessage(&full_chlo, + quic::CurrentSupportedVersions()[0].transport_version); + + std::string packet_content(full_chlo.GetSerialized().AsStringPiece()); + auto encrypted_packet = std::unique_ptr( + quic::test::ConstructEncryptedPacket(connection_id, quic::EmptyQuicConnectionId(), + /*version_flag=*/true, /*reset_flag*/ false, + /*packet_number=*/1, packet_content)); + + Buffer::RawSlice first_slice{ + reinterpret_cast(const_cast(encrypted_packet->data())), + encrypted_packet->length()}; + // Send a full CHLO to finish 0-RTT handshake. + auto send_rc = Network::Utility::writeToSocket(client_sockets_.back()->ioHandle(), &first_slice, + 1, nullptr, *listen_socket_->localAddress()); + ASSERT_EQ(encrypted_packet->length(), send_rc.rc_); + } - void ReadFromClientSockets() { - for (auto& client_socket : client_sockets_) { - Buffer::InstancePtr result_buffer(new Buffer::OwnedImpl()); - const uint64_t bytes_to_read = 11; - uint64_t bytes_read = 0; - int retry = 0; - - do { - Api::IoCallUint64Result result = - result_buffer->read(client_socket->ioHandle(), bytes_to_read - bytes_read); - - if (result.ok()) { - bytes_read += result.rc_; - } else if (retry == 10 || - result.err_->getErrorCode() != Api::IoError::IoErrorCode::Again) { - break; - } - - if (bytes_read == bytes_to_read) { - break; - } - - retry++; - absl::SleepFor(absl::Milliseconds(10)); - } while (true); - } + void ReadFromClientSockets() { + for (auto& client_socket : client_sockets_) { + Buffer::InstancePtr result_buffer(new Buffer::OwnedImpl()); + const uint64_t bytes_to_read = 11; + uint64_t bytes_read = 0; + int retry = 0; + + do { + Api::IoCallUint64Result result = + result_buffer->read(client_socket->ioHandle(), bytes_to_read - bytes_read); + + if (result.ok()) { + bytes_read += result.rc_; + } else if (retry == 10 || result.err_->getErrorCode() != Api::IoError::IoErrorCode::Again) { + break; + } + + if (bytes_read == bytes_to_read) { + break; + } + + retry++; + absl::SleepFor(absl::Milliseconds(10)); + } while (true); } + } - void TearDown() override { - quic_listener_->onListenerShutdown(); - // Trigger alarm to fire before listener destruction. - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - Runtime::LoaderSingleton::clear(); - } + void TearDown() override { + quic_listener_->onListenerShutdown(); + // Trigger alarm to fire before listener destruction. + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + Runtime::LoaderSingleton::clear(); + } - protected: - envoy::config::core::v3::RuntimeFeatureFlag enabledFlag() const { - envoy::config::core::v3::RuntimeFeatureFlag enabled_proto; - std::string yaml(R"EOF( +protected: + envoy::config::core::v3::RuntimeFeatureFlag enabledFlag() const { + envoy::config::core::v3::RuntimeFeatureFlag enabled_proto; + std::string yaml(R"EOF( runtime_key: "quic.enabled" default_value: true )EOF"); - TestUtility::loadFromYamlAndValidate(yaml, enabled_proto); - return enabled_proto; - } - - Network::Address::IpVersion version_; - Event::SimulatedTimeSystemHelper simulated_time_system_; - Api::ApiPtr api_; - Event::DispatcherPtr dispatcher_; - EnvoyQuicClock clock_; - Network::Address::InstanceConstSharedPtr local_address_; - Network::SocketSharedPtr listen_socket_; - Network::SocketPtr client_socket_; - std::shared_ptr read_filter_; - Network::MockConnectionCallbacks network_connection_callbacks_; - NiceMock listener_config_; - quic::QuicConfig quic_config_; - Server::ConnectionHandlerImpl connection_handler_; - std::unique_ptr quic_listener_; - std::unique_ptr quic_dispatcher_; - NiceMock runtime_; - - std::list> client_sockets_; - std::list> read_filters_; - Network::MockFilterChainManager filter_chain_manager_; - // The following two containers must guarantee pointer stability as addresses - // of elements are saved in expectations before new elements are added. - std::list> filter_factories_; - std::list filter_chains_; - }; - - INSTANTIATE_TEST_SUITE_P(IpVersions, ActiveQuicListenerTest, - testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), - TestUtility::ipTestParamsToString); - - TEST_P(ActiveQuicListenerTest, FailSocketOptionUponCreation) { - auto option = std::make_unique(); - EXPECT_CALL(*option, setOption(_, envoy::config::core::v3::SocketOption::STATE_BOUND)) - .WillOnce(Return(false)); - auto options = std::make_shared>(); - options->emplace_back(std::move(option)); - EXPECT_THROW_WITH_REGEX(std::make_unique( - *dispatcher_, connection_handler_, listen_socket_, listener_config_, - quic_config_, options, enabledFlag()), - EnvoyException, "Failed to apply socket options."); + TestUtility::loadFromYamlAndValidate(yaml, enabled_proto); + return enabled_proto; } - TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { - quic::QuicBufferedPacketStore* const buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); - configureQuicRuntimeFlag(/* runtime_enabled = */ true); - configureMocks(/* connection_count = */ 1); - sendFullCHLO(quic::test::TestConnectionId(1)); - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - EXPECT_FALSE(buffered_packets->HasChlosBuffered()); - EXPECT_FALSE(quic_dispatcher_->session_map().empty()); - ReadFromClientSockets(); - } + Network::Address::IpVersion version_; + Event::SimulatedTimeSystemHelper simulated_time_system_; + Api::ApiPtr api_; + Event::DispatcherPtr dispatcher_; + EnvoyQuicClock clock_; + Network::Address::InstanceConstSharedPtr local_address_; + Network::SocketSharedPtr listen_socket_; + Network::SocketPtr client_socket_; + std::shared_ptr read_filter_; + Network::MockConnectionCallbacks network_connection_callbacks_; + NiceMock listener_config_; + quic::QuicConfig quic_config_; + Server::ConnectionHandlerImpl connection_handler_; + std::unique_ptr quic_listener_; + std::unique_ptr quic_dispatcher_; + NiceMock runtime_; + + std::list> client_sockets_; + std::list> read_filters_; + Network::MockFilterChainManager filter_chain_manager_; + // The following two containers must guarantee pointer stability as addresses + // of elements are saved in expectations before new elements are added. + std::list> filter_factories_; + std::list filter_chains_; +}; - /* Not configuring runtime key for `quic.enabled` - and checking that quic processing is enabled by default. */ - TEST_P(ActiveQuicListenerTest, ReceiveFullChloQuicEnabledByDefault) { - quic::QuicBufferedPacketStore* const buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); - configureMocks(/* connection_count = */ 1); - sendFullCHLO(quic::test::TestConnectionId(1)); - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - EXPECT_FALSE(buffered_packets->HasChlosBuffered()); - EXPECT_FALSE(quic_dispatcher_->session_map().empty()); - ReadFromClientSockets(); +INSTANTIATE_TEST_SUITE_P(IpVersions, ActiveQuicListenerTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), + TestUtility::ipTestParamsToString); + +TEST_P(ActiveQuicListenerTest, FailSocketOptionUponCreation) { + auto option = std::make_unique(); + EXPECT_CALL(*option, setOption(_, envoy::config::core::v3::SocketOption::STATE_BOUND)) + .WillOnce(Return(false)); + auto options = std::make_shared>(); + options->emplace_back(std::move(option)); + EXPECT_THROW_WITH_REGEX( + std::make_unique(*dispatcher_, connection_handler_, listen_socket_, + listener_config_, quic_config_, options, enabledFlag()), + EnvoyException, "Failed to apply socket options."); +} + +TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { + quic::QuicBufferedPacketStore* const buffered_packets = + quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); + configureQuicRuntimeFlag(/* runtime_enabled = */ true); + configureMocks(/* connection_count = */ 1); + sendFullCHLO(quic::test::TestConnectionId(1)); + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + EXPECT_FALSE(buffered_packets->HasChlosBuffered()); + EXPECT_FALSE(quic_dispatcher_->session_map().empty()); + ReadFromClientSockets(); +} + +/* Not configuring runtime key for `quic.enabled` +and checking that quic processing is enabled by default. */ +TEST_P(ActiveQuicListenerTest, ReceiveFullChloQuicEnabledByDefault) { + quic::QuicBufferedPacketStore* const buffered_packets = + quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); + configureMocks(/* connection_count = */ 1); + sendFullCHLO(quic::test::TestConnectionId(1)); + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + EXPECT_FALSE(buffered_packets->HasChlosBuffered()); + EXPECT_FALSE(quic_dispatcher_->session_map().empty()); + ReadFromClientSockets(); +} + +TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { + quic::QuicBufferedPacketStore* const buffered_packets = + quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); + configureQuicRuntimeFlag(/* runtime_enabled = */ true); + configureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2); + + // Generate one more CHLO than can be processed immediately. + for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop + 1; ++i) { + sendFullCHLO(quic::test::TestConnectionId(i)); } + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { - quic::QuicBufferedPacketStore* const buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); - configureQuicRuntimeFlag(/* runtime_enabled = */ true); - configureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2); - - // Generate one more CHLO than can be processed immediately. - for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop + 1; ++i) { - sendFullCHLO(quic::test::TestConnectionId(i)); - } - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - - // The first kNumSessionsToCreatePerLoop CHLOs are processed, - // the last one is buffered. - for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop; ++i) { - EXPECT_FALSE(buffered_packets->HasBufferedPackets(quic::test::TestConnectionId(i))); - } - EXPECT_TRUE(buffered_packets->HasBufferedPackets( - quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 1))); - EXPECT_TRUE(buffered_packets->HasChlosBuffered()); - EXPECT_FALSE(quic_dispatcher_->session_map().empty()); - - // Generate more data to trigger a socket read during the next event loop. - sendFullCHLO(quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2)); - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - - // The socket read results in processing all CHLOs. - for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop + 2; ++i) { - EXPECT_FALSE(buffered_packets->HasBufferedPackets(quic::test::TestConnectionId(i))); - } - EXPECT_FALSE(buffered_packets->HasChlosBuffered()); - - ReadFromClientSockets(); + // The first kNumSessionsToCreatePerLoop CHLOs are processed, + // the last one is buffered. + for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop; ++i) { + EXPECT_FALSE(buffered_packets->HasBufferedPackets(quic::test::TestConnectionId(i))); } - - TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { - configureQuicRuntimeFlag(/* runtime_enabled = */ false); - sendFullCHLO(quic::test::TestConnectionId(1)); - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - // If listener was enabled, there should have been session created for active connection. - EXPECT_TRUE(quic_dispatcher_->session_map().empty()); + EXPECT_TRUE(buffered_packets->HasBufferedPackets( + quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 1))); + EXPECT_TRUE(buffered_packets->HasChlosBuffered()); + EXPECT_FALSE(quic_dispatcher_->session_map().empty()); + + // Generate more data to trigger a socket read during the next event loop. + sendFullCHLO(quic::test::TestConnectionId(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2)); + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + + // The socket read results in processing all CHLOs. + for (size_t i = 1; i <= ActiveQuicListener::kNumSessionsToCreatePerLoop + 2; ++i) { + EXPECT_FALSE(buffered_packets->HasBufferedPackets(quic::test::TestConnectionId(i))); } + EXPECT_FALSE(buffered_packets->HasChlosBuffered()); + + ReadFromClientSockets(); +} + +TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { + configureQuicRuntimeFlag(/* runtime_enabled = */ false); + sendFullCHLO(quic::test::TestConnectionId(1)); + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + // If listener was enabled, there should have been session created for active connection. + EXPECT_TRUE(quic_dispatcher_->session_map().empty()); +} } // namespace Quic -} // namespace Quic +} // namespace Envoy \ No newline at end of file From 471a56f13c8bcd498ca4e381dd7d737e9c9e14d7 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Fri, 17 Apr 2020 14:48:45 +0200 Subject: [PATCH 35/58] Remove test that does not make sense Signed-off-by: Kateryna Nezdolii --- bazel/envoy_internal.bzl | 1 - .../active_quic_listener_config_test.cc | 6 +-- .../quiche/active_quic_listener_test.cc | 45 +++++++------------ 3 files changed, 19 insertions(+), 33 deletions(-) diff --git a/bazel/envoy_internal.bzl b/bazel/envoy_internal.bzl index 4406f08c4eda..4ba30a8117a7 100644 --- a/bazel/envoy_internal.bzl +++ b/bazel/envoy_internal.bzl @@ -7,7 +7,6 @@ def envoy_copts(repository, test = False): posix_options = [ "-Wall", "-Wextra", - "-Werror", "-Wnon-virtual-dtor", "-Woverloaded-virtual", "-Wold-style-cast", diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc index a22aa3723a50..1911782e74d5 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc @@ -55,7 +55,7 @@ TEST(ActiveQuicListenerConfigTest, CreateActiveQuicListenerFactory) { EXPECT_EQ("foo_key", runtime_enabled.runtime_key()); } -TEST(ActiveQuicListenerConfigTest, QuicListenerEnabledByDefault) { +TEST(ActiveQuicListenerConfigTest, QuicListenerDisabledByDefault) { std::string listener_name = QuicListenerName; auto& config_factory = Config::Utility::getAndCheckFactoryByName( @@ -75,8 +75,8 @@ TEST(ActiveQuicListenerConfigTest, QuicListenerEnabledByDefault) { envoy::config::core::v3::RuntimeFeatureFlag& runtime_enabled = ActiveQuicListenerFactoryPeer::runtimeEnabled( dynamic_cast(*listener_factory)); - EXPECT_EQ(true, runtime_enabled.default_value().value()); - EXPECT_EQ("foo_key", runtime_enabled.runtime_key()); + EXPECT_EQ(false, runtime_enabled.default_value().value()); + EXPECT_EQ("", runtime_enabled.runtime_key()); } } // namespace Quic diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index ecd3e5d4deb1..2c44eda27459 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -74,8 +74,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam(*dispatcher_, connection_handler_, listen_socket_, listener_config_, quic_config_, nullptr, enabledFlag()); - quic_dispatcher_ = std::unique_ptr( - ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_)); + quic_dispatcher_ = ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); simulated_time_system_.advanceTimeWait(std::chrono::milliseconds(100)); } @@ -227,7 +226,7 @@ default_value: true quic::QuicConfig quic_config_; Server::ConnectionHandlerImpl connection_handler_; std::unique_ptr quic_listener_; - std::unique_ptr quic_dispatcher_; + EnvoyQuicDispatcher* quic_dispatcher_; NiceMock runtime_; std::list> client_sockets_; @@ -243,21 +242,22 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, ActiveQuicListenerTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), TestUtility::ipTestParamsToString); -TEST_P(ActiveQuicListenerTest, FailSocketOptionUponCreation) { - auto option = std::make_unique(); - EXPECT_CALL(*option, setOption(_, envoy::config::core::v3::SocketOption::STATE_BOUND)) - .WillOnce(Return(false)); - auto options = std::make_shared>(); - options->emplace_back(std::move(option)); - EXPECT_THROW_WITH_REGEX( - std::make_unique(*dispatcher_, connection_handler_, listen_socket_, - listener_config_, quic_config_, options, enabledFlag()), - EnvoyException, "Failed to apply socket options."); -} +// TEST_P(ActiveQuicListenerTest, FailSocketOptionUponCreation) { +// auto option = std::make_unique(); +// EXPECT_CALL(*option, setOption(_, envoy::config::core::v3::SocketOption::STATE_BOUND)) +// .WillOnce(Return(false)); +// auto options = std::make_shared>(); +// options->emplace_back(std::move(option)); +// EXPECT_THROW_WITH_REGEX( +// std::make_unique(*dispatcher_, connection_handler_, listen_socket_, +// listener_config_, quic_config_, options, +// enabledFlag()), +// EnvoyException, "Failed to apply socket options."); +// } TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { quic::QuicBufferedPacketStore* const buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); + quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_); configureQuicRuntimeFlag(/* runtime_enabled = */ true); configureMocks(/* connection_count = */ 1); sendFullCHLO(quic::test::TestConnectionId(1)); @@ -267,22 +267,9 @@ TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { ReadFromClientSockets(); } -/* Not configuring runtime key for `quic.enabled` -and checking that quic processing is enabled by default. */ -TEST_P(ActiveQuicListenerTest, ReceiveFullChloQuicEnabledByDefault) { - quic::QuicBufferedPacketStore* const buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); - configureMocks(/* connection_count = */ 1); - sendFullCHLO(quic::test::TestConnectionId(1)); - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - EXPECT_FALSE(buffered_packets->HasChlosBuffered()); - EXPECT_FALSE(quic_dispatcher_->session_map().empty()); - ReadFromClientSockets(); -} - TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { quic::QuicBufferedPacketStore* const buffered_packets = - quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_.get()); + quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_); configureQuicRuntimeFlag(/* runtime_enabled = */ true); configureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2); From 598082c95ee1554f241d77b98691723cf228fa5d Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Fri, 17 Apr 2020 21:21:55 +0200 Subject: [PATCH 36/58] correct merge issue Signed-off-by: Kateryna Nezdolii --- source/extensions/quic_listeners/quiche/active_quic_listener.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index e223f739e30d..cca2d86c0838 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -96,7 +96,7 @@ void ActiveQuicListener::onData(Network::UdpRecvData& data) { void ActiveQuicListener::onReadReady() { if (!enabled()) { - ENVOY_LOG(trace, "Quic listener {}: runtime disabled", config_.name()); + ENVOY_LOG(trace, "Quic listener {}: runtime disabled", config_->name()); return; } quic_dispatcher_->ProcessBufferedChlos(kNumSessionsToCreatePerLoop); From 1828540465ecabb0758beb0c198ad7b0066badfe Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Mon, 20 Apr 2020 10:25:48 +0200 Subject: [PATCH 37/58] cleanup Signed-off-by: Kateryna Nezdolii --- bazel/envoy_internal.bzl | 1 + .../quiche/active_quic_listener_test.cc | 23 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/bazel/envoy_internal.bzl b/bazel/envoy_internal.bzl index 4ba30a8117a7..4406f08c4eda 100644 --- a/bazel/envoy_internal.bzl +++ b/bazel/envoy_internal.bzl @@ -7,6 +7,7 @@ def envoy_copts(repository, test = False): posix_options = [ "-Wall", "-Wextra", + "-Werror", "-Wnon-virtual-dtor", "-Woverloaded-virtual", "-Wold-style-cast", diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 2c44eda27459..e4569829f758 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -242,18 +242,17 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, ActiveQuicListenerTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), TestUtility::ipTestParamsToString); -// TEST_P(ActiveQuicListenerTest, FailSocketOptionUponCreation) { -// auto option = std::make_unique(); -// EXPECT_CALL(*option, setOption(_, envoy::config::core::v3::SocketOption::STATE_BOUND)) -// .WillOnce(Return(false)); -// auto options = std::make_shared>(); -// options->emplace_back(std::move(option)); -// EXPECT_THROW_WITH_REGEX( -// std::make_unique(*dispatcher_, connection_handler_, listen_socket_, -// listener_config_, quic_config_, options, -// enabledFlag()), -// EnvoyException, "Failed to apply socket options."); -// } +TEST_P(ActiveQuicListenerTest, FailSocketOptionUponCreation) { + auto option = std::make_unique(); + EXPECT_CALL(*option, setOption(_, envoy::config::core::v3::SocketOption::STATE_BOUND)) + .WillOnce(Return(false)); + auto options = std::make_shared>(); + options->emplace_back(std::move(option)); + EXPECT_THROW_WITH_REGEX( + std::make_unique(*dispatcher_, connection_handler_, listen_socket_, + listener_config_, quic_config_, options, enabledFlag()), + EnvoyException, "Failed to apply socket options."); +} TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { quic::QuicBufferedPacketStore* const buffered_packets = From 5e0fc6c7d90d7af3f0b6ea71574c002720ce490b Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Mon, 20 Apr 2020 10:41:01 +0200 Subject: [PATCH 38/58] cleanup Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_config_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc index 1911782e74d5..a883145acc0f 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc @@ -55,7 +55,7 @@ TEST(ActiveQuicListenerConfigTest, CreateActiveQuicListenerFactory) { EXPECT_EQ("foo_key", runtime_enabled.runtime_key()); } -TEST(ActiveQuicListenerConfigTest, QuicListenerDisabledByDefault) { +TEST(ActiveQuicListenerConfigTest, QuicListenerFlagNotConfigured) { std::string listener_name = QuicListenerName; auto& config_factory = Config::Utility::getAndCheckFactoryByName( From b865e6054143201b6b4ddce691f33202cf8140bc Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Sat, 25 Apr 2020 09:33:53 +0200 Subject: [PATCH 39/58] Use real loader in tests Signed-off-by: Kateryna Nezdolii --- bazel/envoy_internal.bzl | 2 +- test/extensions/quic_listeners/quiche/BUILD | 2 + .../active_quic_listener_config_test.cc | 2 +- .../quiche/active_quic_listener_test.cc | 96 ++++++++++++++++--- 4 files changed, 87 insertions(+), 15 deletions(-) diff --git a/bazel/envoy_internal.bzl b/bazel/envoy_internal.bzl index 4406f08c4eda..ff3d3f50ebf1 100644 --- a/bazel/envoy_internal.bzl +++ b/bazel/envoy_internal.bzl @@ -7,7 +7,7 @@ def envoy_copts(repository, test = False): posix_options = [ "-Wall", "-Wextra", - "-Werror", + #"-Werror", "-Wnon-virtual-dtor", "-Woverloaded-virtual", "-Wold-style-cast", diff --git a/test/extensions/quic_listeners/quiche/BUILD b/test/extensions/quic_listeners/quiche/BUILD index 0b1af433c046..630c7dfb7494 100644 --- a/test/extensions/quic_listeners/quiche/BUILD +++ b/test/extensions/quic_listeners/quiche/BUILD @@ -143,10 +143,12 @@ envoy_cc_test( tags = ["nofips"], deps = [ ":quic_test_utils_for_envoy_lib", + "//source/extensions/quic_listeners/quiche:active_quic_listener_config_lib", "//source/extensions/quic_listeners/quiche:active_quic_listener_lib", "//source/extensions/quic_listeners/quiche:envoy_quic_utils_lib", "//source/server:configuration_lib", "//test/mocks/network:network_mocks", + "//test/mocks/server:server_mocks", "//test/test_common:environment_lib", "//test/test_common:network_utility_lib", "//test/test_common:simulated_time_system_lib", diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc index a883145acc0f..986411f1341f 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc @@ -75,7 +75,7 @@ TEST(ActiveQuicListenerConfigTest, QuicListenerFlagNotConfigured) { envoy::config::core::v3::RuntimeFeatureFlag& runtime_enabled = ActiveQuicListenerFactoryPeer::runtimeEnabled( dynamic_cast(*listener_factory)); - EXPECT_EQ(false, runtime_enabled.default_value().value()); + EXPECT_FALSE(runtime_enabled.has_default_value()); EXPECT_EQ("", runtime_enabled.runtime_key()); } diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index e4569829f758..30a4d1746b4c 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -8,6 +8,8 @@ #include +#include "common/runtime/runtime_impl.h" + #include "envoy/config/core/v3/base.pb.h" #include "envoy/config/core/v3/base.pb.validate.h" @@ -26,12 +28,14 @@ #include "test/test_common/simulated_time_system.h" #include "test/test_common/environment.h" #include "test/mocks/network/mocks.h" +#include "test/mocks/server/mocks.h" #include "test/mocks/runtime/mocks.h" #include "test/test_common/utility.h" #include "test/test_common/network_utility.h" #include "absl/time/time.h" #include "gtest/gtest.h" #include "gmock/gmock.h" +#include "extensions/quic_listeners/quiche/active_quic_listener_config.h" #include "extensions/quic_listeners/quiche/platform/envoy_quic_clock.h" #include "extensions/quic_listeners/quiche/envoy_quic_utils.h" @@ -52,6 +56,14 @@ class ActiveQuicListenerPeer { } }; +class ActiveQuicListenerFactoryPeer { +public: + static envoy::config::core::v3::RuntimeFeatureFlag& + runtimeEnabled(ActiveQuicListenerFactory& factory) { + return factory.enabled_; + } +}; + class ActiveQuicListenerTest : public testing::TestWithParam { protected: using Socket = Network::NetworkListenSocket< @@ -61,11 +73,14 @@ class ActiveQuicListenerTest : public testing::TestWithParamallocateDispatcher("test_thread")), clock_(*dispatcher_), local_address_(Network::Test::getCanonicalLoopbackAddress(version_)), - connection_handler_(*dispatcher_) { - Runtime::LoaderSingleton::initialize(&runtime_); - } + connection_handler_(*dispatcher_) {} void SetUp() override { + envoy::config::bootstrap::v3::LayeredRuntime config; + config.add_layers()->mutable_admin_layer(); + loader_ = std::make_unique(Runtime::LoaderPtr{ + new Runtime::LoaderImpl(*dispatcher_, tls_, config, local_info_, init_manager_, store_, + generator_, validation_visitor_, *api_)}); listen_socket_ = std::make_shared(local_address_, nullptr, /*bind*/ true); listen_socket_->addOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions()); @@ -75,12 +90,8 @@ class ActiveQuicListenerTest : public testing::TestWithParam quic_listener_; EnvoyQuicDispatcher* quic_dispatcher_; - NiceMock runtime_; + std::unique_ptr loader_; + + NiceMock tls_; + Stats::TestUtil::TestStore store_; + Runtime::MockRandomGenerator generator_; + Runtime::MockRandomGenerator rand_; + NiceMock local_info_; + Init::MockManager init_manager_; + NiceMock validation_visitor_; std::list> client_sockets_; std::list> read_filters_; @@ -257,7 +276,6 @@ TEST_P(ActiveQuicListenerTest, FailSocketOptionUponCreation) { TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { quic::QuicBufferedPacketStore* const buffered_packets = quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_); - configureQuicRuntimeFlag(/* runtime_enabled = */ true); configureMocks(/* connection_count = */ 1); sendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); @@ -269,7 +287,7 @@ TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { quic::QuicBufferedPacketStore* const buffered_packets = quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_); - configureQuicRuntimeFlag(/* runtime_enabled = */ true); + // configureQuicRuntimeFlag(/* runtime_enabled = */ true); configureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2); // Generate one more CHLO than can be processed immediately. @@ -302,11 +320,63 @@ TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { } TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { - configureQuicRuntimeFlag(/* runtime_enabled = */ false); + Runtime::LoaderSingleton::getExisting()->mergeValues({{"quic.enabled", " false"}}); sendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); // If listener was enabled, there should have been session created for active connection. EXPECT_TRUE(quic_dispatcher_->session_map().empty()); + EXPECT_FALSE(quic_listener_->enabled()); +} + +TEST_P(ActiveQuicListenerTest, QuicProcessingDisabledAndEnabled) { + Runtime::LoaderSingleton::getExisting()->mergeValues({{"quic.enabled", " false"}}); + sendFullCHLO(quic::test::TestConnectionId(1)); + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + // If listener was enabled, there should have been session created for active connection. + EXPECT_TRUE(quic_dispatcher_->session_map().empty()); + EXPECT_FALSE(quic_listener_->enabled()); + Runtime::LoaderSingleton::getExisting()->mergeValues({{"quic.enabled", " true"}}); + configureMocks(/* connection_count = */ 1); + sendFullCHLO(quic::test::TestConnectionId(1)); + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + EXPECT_FALSE(quic_dispatcher_->session_map().empty()); + EXPECT_TRUE(quic_listener_->enabled()); +} + +class ActiveQuicListenerEmptyFlagConfigTest : public ActiveQuicListenerTest { +protected: + envoy::config::core::v3::RuntimeFeatureFlag enabledFlag() const override { + std::string listener_name = QuicListenerName; + auto& config_factory = + Config::Utility::getAndCheckFactoryByName( + listener_name); + ProtobufTypes::MessagePtr config = config_factory.createEmptyConfigProto(); + std::string yaml = R"EOF( + max_concurrent_streams: 10 + )EOF"; + TestUtility::loadFromYaml(yaml, *config); + Network::ActiveUdpListenerFactoryPtr listener_factory = + config_factory.createActiveUdpListenerFactory(*config, /*concurrency=*/1); + return ActiveQuicListenerFactoryPeer::runtimeEnabled( + dynamic_cast(*listener_factory)); + } +}; + +INSTANTIATE_TEST_SUITE_P(IpVersions, ActiveQuicListenerEmptyFlagConfigTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), + TestUtility::ipTestParamsToString); + +// Quic listener should be enabled by default, if not enabled expicitely in config. +TEST_P(ActiveQuicListenerEmptyFlagConfigTest, ReceiveFullQuicCHLO) { + quic::QuicBufferedPacketStore* const buffered_packets = + quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_); + configureMocks(/* connection_count = */ 1); + sendFullCHLO(quic::test::TestConnectionId(1)); + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + EXPECT_FALSE(buffered_packets->HasChlosBuffered()); + EXPECT_FALSE(quic_dispatcher_->session_map().empty()); + EXPECT_TRUE(quic_listener_->enabled()); + ReadFromClientSockets(); } } // namespace Quic From 2a0e3881b76b1b1cfefb5ac899a85ea7cf7693a8 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 30 Apr 2020 18:38:26 +0200 Subject: [PATCH 40/58] cleanup Signed-off-by: Kateryna Nezdolii --- bazel/envoy_internal.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bazel/envoy_internal.bzl b/bazel/envoy_internal.bzl index ff3d3f50ebf1..4406f08c4eda 100644 --- a/bazel/envoy_internal.bzl +++ b/bazel/envoy_internal.bzl @@ -7,7 +7,7 @@ def envoy_copts(repository, test = False): posix_options = [ "-Wall", "-Wextra", - #"-Werror", + "-Werror", "-Wnon-virtual-dtor", "-Woverloaded-virtual", "-Wold-style-cast", From 0bd459cf48e63dedb2506612a0bab319c1c3517c Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 30 Apr 2020 19:01:39 +0200 Subject: [PATCH 41/58] fix spelling Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 30a4d1746b4c..532cb2479699 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -366,7 +366,7 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, ActiveQuicListenerEmptyFlagConfigTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), TestUtility::ipTestParamsToString); -// Quic listener should be enabled by default, if not enabled expicitely in config. +// Quic listener should be enabled by default, if not enabled explicitly in config. TEST_P(ActiveQuicListenerEmptyFlagConfigTest, ReceiveFullQuicCHLO) { quic::QuicBufferedPacketStore* const buffered_packets = quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_); From ae894d9b7fb6caad628e65d123f6980d18901517 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Mon, 4 May 2020 09:31:21 +0200 Subject: [PATCH 42/58] Apply review comments Signed-off-by: Kateryna Nezdolii --- bazel/envoy_internal.bzl | 2 +- source/common/network/base_listener_impl.cc | 1 - source/common/network/udp_listener_impl.cc | 1 - .../quiche/active_quic_listener.cc | 4 +- .../quiche/active_quic_listener.h | 2 - .../active_quic_listener_config_test.cc | 3 + .../quiche/active_quic_listener_test.cc | 82 +++++++++++-------- 7 files changed, 56 insertions(+), 39 deletions(-) diff --git a/bazel/envoy_internal.bzl b/bazel/envoy_internal.bzl index 4406f08c4eda..ff3d3f50ebf1 100644 --- a/bazel/envoy_internal.bzl +++ b/bazel/envoy_internal.bzl @@ -7,7 +7,7 @@ def envoy_copts(repository, test = False): posix_options = [ "-Wall", "-Wextra", - "-Werror", + #"-Werror", "-Wnon-virtual-dtor", "-Woverloaded-virtual", "-Wold-style-cast", diff --git a/source/common/network/base_listener_impl.cc b/source/common/network/base_listener_impl.cc index c096bfcfb522..1d1c8a9b70da 100644 --- a/source/common/network/base_listener_impl.cc +++ b/source/common/network/base_listener_impl.cc @@ -21,7 +21,6 @@ Address::InstanceConstSharedPtr BaseListenerImpl::getLocalAddress(os_fd_t fd) { BaseListenerImpl::BaseListenerImpl(Event::DispatcherImpl& dispatcher, SocketSharedPtr socket) : local_address_(nullptr), dispatcher_(dispatcher), socket_(std::move(socket)) { const auto ip = socket_->localAddress()->ip(); - // Only use the listen socket's local address for new connections if it is not the all hosts // address (e.g., 0.0.0.0 for IPv4). if (!(ip && ip->isAnyAddress())) { diff --git a/source/common/network/udp_listener_impl.cc b/source/common/network/udp_listener_impl.cc index f05817f19d18..8a33e7310dde 100644 --- a/source/common/network/udp_listener_impl.cc +++ b/source/common/network/udp_listener_impl.cc @@ -35,7 +35,6 @@ UdpListenerImpl::UdpListenerImpl(Event::DispatcherImpl& dispatcher, SocketShared Event::FileTriggerType::Edge, Event::FileReadyType::Read | Event::FileReadyType::Write); ASSERT(file_event_); - if (!Network::Socket::applyOptions(socket_->options(), *socket_, envoy::config::core::v3::SocketOption::STATE_BOUND)) { throw CreateListenerException(fmt::format("cannot set post-bound socket option on socket: {}", diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index cca2d86c0838..5bc057a50bf9 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -46,7 +46,6 @@ ActiveQuicListener::ActiveQuicListener(Event::Dispatcher& dispatcher, } listen_socket_.addOptions(options); } - udp_listener_ = dispatcher_.createUdpListener(std::move(listen_socket), *this); quic::QuicRandom* const random = quic::QuicRandom::GetInstance(); random->RandBytes(random_seed_, sizeof(random_seed_)); @@ -95,7 +94,7 @@ void ActiveQuicListener::onData(Network::UdpRecvData& data) { } void ActiveQuicListener::onReadReady() { - if (!enabled()) { + if (!enabled_.enabled()) { ENVOY_LOG(trace, "Quic listener {}: runtime disabled", config_->name()); return; } @@ -197,6 +196,7 @@ ActiveQuicListenerFactory::createActiveUdpListener(Network::ConnectionHandler& p #endif } #endif + return std::make_unique(disptacher, parent, config, quic_config_, std::move(options), enabled_); } diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.h b/source/extensions/quic_listeners/quiche/active_quic_listener.h index ea450c2a139a..94a3f5076fdb 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.h +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.h @@ -54,8 +54,6 @@ class ActiveQuicListener : public Network::UdpListenerCallbacks, void resumeListening() override; void shutdownListener() override; - bool enabled() { return enabled_.enabled(); } - private: friend class ActiveQuicListenerPeer; diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc index 986411f1341f..d116f816b6ca 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc @@ -75,7 +75,10 @@ TEST(ActiveQuicListenerConfigTest, QuicListenerFlagNotConfigured) { envoy::config::core::v3::RuntimeFeatureFlag& runtime_enabled = ActiveQuicListenerFactoryPeer::runtimeEnabled( dynamic_cast(*listener_factory)); + auto& quic_config = + dynamic_cast(*config); EXPECT_FALSE(runtime_enabled.has_default_value()); + EXPECT_FALSE(quic_config.has_enabled()); EXPECT_EQ("", runtime_enabled.runtime_key()); } diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 532cb2479699..b10bcfd83f92 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -39,6 +39,7 @@ #include "extensions/quic_listeners/quiche/platform/envoy_quic_clock.h" #include "extensions/quic_listeners/quiche/envoy_quic_utils.h" +using testing::AnyNumber; using testing::Return; using testing::ReturnRef; @@ -54,13 +55,15 @@ class ActiveQuicListenerPeer { static quic::QuicCryptoServerConfig& crypto_config(ActiveQuicListener& listener) { return *listener.crypto_config_; } + + static bool enabled_(ActiveQuicListener& listener) { return listener.enabled_.enabled(); } }; class ActiveQuicListenerFactoryPeer { public: static envoy::config::core::v3::RuntimeFeatureFlag& - runtimeEnabled(ActiveQuicListenerFactory& factory) { - return factory.enabled_; + runtimeEnabled(ActiveQuicListenerFactory* factory) { + return factory->enabled_; } }; @@ -75,6 +78,11 @@ class ActiveQuicListenerTest : public testing::TestWithParam + std::unique_ptr static_unique_pointer_cast(std::unique_ptr&& source) { + return std::unique_ptr{static_cast(source.release())}; + } + void SetUp() override { envoy::config::bootstrap::v3::LayeredRuntime config; config.add_layers()->mutable_admin_layer(); @@ -86,14 +94,32 @@ class ActiveQuicListenerTest : public testing::TestWithParamaddOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions()); listen_socket_->addOptions(Network::SocketOptionFactory::buildRxQueueOverFlowOptions()); - quic_listener_ = std::make_unique(*dispatcher_, connection_handler_, - listen_socket_, listener_config_, - quic_config_, nullptr, enabledFlag()); + EXPECT_CALL(listener_config_, listenSocketFactory()) + .Times(AnyNumber()) + .WillRepeatedly(ReturnRef(socket_factory_)); + EXPECT_CALL(socket_factory_, getListenSocket()) + .Times(AnyNumber()) + .WillRepeatedly(Return(listen_socket_)); + + listener_factory_ = createQuicListenerFactory(yamlForQuicConfig()); + quic_listener_ = static_unique_pointer_cast( + std::move(listener_factory_->createActiveUdpListener(connection_handler_, *dispatcher_, + listener_config_))); quic_dispatcher_ = ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); simulated_time_system_.advanceTimeWait(std::chrono::milliseconds(100)); } + Network::ActiveUdpListenerFactoryPtr createQuicListenerFactory(const std::string& yaml) { + std::string listener_name = QuicListenerName; + auto& config_factory = + Config::Utility::getAndCheckFactoryByName( + listener_name); + ProtobufTypes::MessagePtr config_proto = config_factory.createEmptyConfigProto(); + TestUtility::loadFromYaml(yaml, *config_proto); + return config_factory.createActiveUdpListenerFactory(*config_proto, /*concurrency=*/1); + } + void configureMocks(int connection_count) { EXPECT_CALL(listener_config_, filterChainManager()) .Times(connection_count) @@ -213,14 +239,12 @@ class ActiveQuicListenerTest : public testing::TestWithParam quic_listener_; + Network::ActiveUdpListenerFactoryPtr listener_factory_; + Network::MockListenSocketFactory socket_factory_; EnvoyQuicDispatcher* quic_dispatcher_; std::unique_ptr loader_; @@ -268,8 +294,11 @@ TEST_P(ActiveQuicListenerTest, FailSocketOptionUponCreation) { auto options = std::make_shared>(); options->emplace_back(std::move(option)); EXPECT_THROW_WITH_REGEX( - std::make_unique(*dispatcher_, connection_handler_, listen_socket_, - listener_config_, quic_config_, options, enabledFlag()), + std::make_unique( + *dispatcher_, connection_handler_, listen_socket_, listener_config_, quic_config_, + options, + ActiveQuicListenerFactoryPeer::runtimeEnabled( + static_cast(listener_factory_.get()))), EnvoyException, "Failed to apply socket options."); } @@ -287,7 +316,6 @@ TEST_P(ActiveQuicListenerTest, ReceiveFullQuicCHLO) { TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { quic::QuicBufferedPacketStore* const buffered_packets = quic::test::QuicDispatcherPeer::GetBufferedPackets(quic_dispatcher_); - // configureQuicRuntimeFlag(/* runtime_enabled = */ true); configureMocks(ActiveQuicListener::kNumSessionsToCreatePerLoop + 2); // Generate one more CHLO than can be processed immediately. @@ -325,7 +353,7 @@ TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { dispatcher_->run(Event::Dispatcher::RunType::NonBlock); // If listener was enabled, there should have been session created for active connection. EXPECT_TRUE(quic_dispatcher_->session_map().empty()); - EXPECT_FALSE(quic_listener_->enabled()); + EXPECT_FALSE(ActiveQuicListenerPeer::enabled_(*quic_listener_)); } TEST_P(ActiveQuicListenerTest, QuicProcessingDisabledAndEnabled) { @@ -334,31 +362,21 @@ TEST_P(ActiveQuicListenerTest, QuicProcessingDisabledAndEnabled) { dispatcher_->run(Event::Dispatcher::RunType::NonBlock); // If listener was enabled, there should have been session created for active connection. EXPECT_TRUE(quic_dispatcher_->session_map().empty()); - EXPECT_FALSE(quic_listener_->enabled()); + EXPECT_FALSE(ActiveQuicListenerPeer::enabled_(*quic_listener_)); Runtime::LoaderSingleton::getExisting()->mergeValues({{"quic.enabled", " true"}}); configureMocks(/* connection_count = */ 1); sendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); EXPECT_FALSE(quic_dispatcher_->session_map().empty()); - EXPECT_TRUE(quic_listener_->enabled()); + EXPECT_TRUE(ActiveQuicListenerPeer::enabled_(*quic_listener_)); } class ActiveQuicListenerEmptyFlagConfigTest : public ActiveQuicListenerTest { protected: - envoy::config::core::v3::RuntimeFeatureFlag enabledFlag() const override { - std::string listener_name = QuicListenerName; - auto& config_factory = - Config::Utility::getAndCheckFactoryByName( - listener_name); - ProtobufTypes::MessagePtr config = config_factory.createEmptyConfigProto(); - std::string yaml = R"EOF( + std::string yamlForQuicConfig() override { + return R"EOF( max_concurrent_streams: 10 )EOF"; - TestUtility::loadFromYaml(yaml, *config); - Network::ActiveUdpListenerFactoryPtr listener_factory = - config_factory.createActiveUdpListenerFactory(*config, /*concurrency=*/1); - return ActiveQuicListenerFactoryPeer::runtimeEnabled( - dynamic_cast(*listener_factory)); } }; @@ -375,7 +393,7 @@ TEST_P(ActiveQuicListenerEmptyFlagConfigTest, ReceiveFullQuicCHLO) { dispatcher_->run(Event::Dispatcher::RunType::NonBlock); EXPECT_FALSE(buffered_packets->HasChlosBuffered()); EXPECT_FALSE(quic_dispatcher_->session_map().empty()); - EXPECT_TRUE(quic_listener_->enabled()); + EXPECT_TRUE(ActiveQuicListenerPeer::enabled_(*quic_listener_)); ReadFromClientSockets(); } From 8c1a26ad9a1419120b754c8099799c40d4fd8442 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Wed, 6 May 2020 23:28:34 +0200 Subject: [PATCH 43/58] clean up Signed-off-by: Kateryna Nezdolii --- bazel/envoy_internal.bzl | 2 +- .../quic_listeners/quiche/active_quic_listener_config_test.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bazel/envoy_internal.bzl b/bazel/envoy_internal.bzl index ff3d3f50ebf1..4406f08c4eda 100644 --- a/bazel/envoy_internal.bzl +++ b/bazel/envoy_internal.bzl @@ -7,7 +7,7 @@ def envoy_copts(repository, test = False): posix_options = [ "-Wall", "-Wextra", - #"-Werror", + "-Werror", "-Wnon-virtual-dtor", "-Woverloaded-virtual", "-Wold-style-cast", diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc index d116f816b6ca..f2dd5a94c7db 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc @@ -76,7 +76,7 @@ TEST(ActiveQuicListenerConfigTest, QuicListenerFlagNotConfigured) { ActiveQuicListenerFactoryPeer::runtimeEnabled( dynamic_cast(*listener_factory)); auto& quic_config = - dynamic_cast(*config); + dynamic_cast(*config); EXPECT_FALSE(runtime_enabled.has_default_value()); EXPECT_FALSE(quic_config.has_enabled()); EXPECT_EQ("", runtime_enabled.runtime_key()); From 76bdeed47a2b8326218c09a03361030ef8431c1c Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Wed, 6 May 2020 23:39:19 +0200 Subject: [PATCH 44/58] Clean up Signed-off-by: Kateryna Nezdolii --- source/common/network/base_listener_impl.cc | 1 + source/common/network/udp_listener_impl.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/source/common/network/base_listener_impl.cc b/source/common/network/base_listener_impl.cc index 1d1c8a9b70da..b41c52a20854 100644 --- a/source/common/network/base_listener_impl.cc +++ b/source/common/network/base_listener_impl.cc @@ -21,6 +21,7 @@ Address::InstanceConstSharedPtr BaseListenerImpl::getLocalAddress(os_fd_t fd) { BaseListenerImpl::BaseListenerImpl(Event::DispatcherImpl& dispatcher, SocketSharedPtr socket) : local_address_(nullptr), dispatcher_(dispatcher), socket_(std::move(socket)) { const auto ip = socket_->localAddress()->ip(); + // Only use the listen socket's local address for new connections if it is not the all hosts // address (e.g., 0.0.0.0 for IPv4). if (!(ip && ip->isAnyAddress())) { diff --git a/source/common/network/udp_listener_impl.cc b/source/common/network/udp_listener_impl.cc index 8a33e7310dde..fb8550eae955 100644 --- a/source/common/network/udp_listener_impl.cc +++ b/source/common/network/udp_listener_impl.cc @@ -35,6 +35,7 @@ UdpListenerImpl::UdpListenerImpl(Event::DispatcherImpl& dispatcher, SocketShared Event::FileTriggerType::Edge, Event::FileReadyType::Read | Event::FileReadyType::Write); ASSERT(file_event_); + if (!Network::Socket::applyOptions(socket_->options(), *socket_, envoy::config::core::v3::SocketOption::STATE_BOUND)) { throw CreateListenerException(fmt::format("cannot set post-bound socket option on socket: {}", From c52961ef5f851d12c9e2ff4db2715b8728e4fc35 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Wed, 6 May 2020 23:42:33 +0200 Subject: [PATCH 45/58] ammend formatting of new lines Signed-off-by: Kateryna Nezdolii --- source/common/network/base_listener_impl.cc | 2 +- source/common/network/udp_listener_impl.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source/common/network/base_listener_impl.cc b/source/common/network/base_listener_impl.cc index b41c52a20854..c096bfcfb522 100644 --- a/source/common/network/base_listener_impl.cc +++ b/source/common/network/base_listener_impl.cc @@ -21,7 +21,7 @@ Address::InstanceConstSharedPtr BaseListenerImpl::getLocalAddress(os_fd_t fd) { BaseListenerImpl::BaseListenerImpl(Event::DispatcherImpl& dispatcher, SocketSharedPtr socket) : local_address_(nullptr), dispatcher_(dispatcher), socket_(std::move(socket)) { const auto ip = socket_->localAddress()->ip(); - + // Only use the listen socket's local address for new connections if it is not the all hosts // address (e.g., 0.0.0.0 for IPv4). if (!(ip && ip->isAnyAddress())) { diff --git a/source/common/network/udp_listener_impl.cc b/source/common/network/udp_listener_impl.cc index fb8550eae955..f05817f19d18 100644 --- a/source/common/network/udp_listener_impl.cc +++ b/source/common/network/udp_listener_impl.cc @@ -35,7 +35,7 @@ UdpListenerImpl::UdpListenerImpl(Event::DispatcherImpl& dispatcher, SocketShared Event::FileTriggerType::Edge, Event::FileReadyType::Read | Event::FileReadyType::Write); ASSERT(file_event_); - + if (!Network::Socket::applyOptions(socket_->options(), *socket_, envoy::config::core::v3::SocketOption::STATE_BOUND)) { throw CreateListenerException(fmt::format("cannot set post-bound socket option on socket: {}", From af9a8aa2f7157c4ae10fd1666e79464688bc0591 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 7 May 2020 00:06:51 +0200 Subject: [PATCH 46/58] fix format Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_config_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc index f2dd5a94c7db..d116f816b6ca 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_config_test.cc @@ -76,7 +76,7 @@ TEST(ActiveQuicListenerConfigTest, QuicListenerFlagNotConfigured) { ActiveQuicListenerFactoryPeer::runtimeEnabled( dynamic_cast(*listener_factory)); auto& quic_config = - dynamic_cast(*config); + dynamic_cast(*config); EXPECT_FALSE(runtime_enabled.has_default_value()); EXPECT_FALSE(quic_config.has_enabled()); EXPECT_EQ("", runtime_enabled.runtime_key()); From 8ab857621458d2309d8bf478ce065337df2e530e Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 7 May 2020 00:08:39 +0200 Subject: [PATCH 47/58] fix proto v4 Signed-off-by: Kateryna Nezdolii --- api/envoy/config/listener/v4alpha/quic_config.proto | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/api/envoy/config/listener/v4alpha/quic_config.proto b/api/envoy/config/listener/v4alpha/quic_config.proto index 97866e4b6ed8..f87d30f4def7 100644 --- a/api/envoy/config/listener/v4alpha/quic_config.proto +++ b/api/envoy/config/listener/v4alpha/quic_config.proto @@ -32,4 +32,8 @@ message QuicProtocolOptions { // Connection timeout in milliseconds before the crypto handshake is finished. // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; + + // Runtime flag that controls whether the listener is enabled or not. If not specified, defaults + // to enabled. + core.v3.RuntimeFeatureFlag enabled = 4; } From e4393dd948b6fed5b1943ed2b5f7752a462db948 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 7 May 2020 00:27:21 +0200 Subject: [PATCH 48/58] fix v4 protos Signed-off-by: Kateryna Nezdolii --- api/envoy/config/listener/v4alpha/BUILD | 1 + api/envoy/config/listener/v4alpha/quic_config.proto | 2 ++ 2 files changed, 3 insertions(+) diff --git a/api/envoy/config/listener/v4alpha/BUILD b/api/envoy/config/listener/v4alpha/BUILD index 1d1761a3e941..2916504fd893 100644 --- a/api/envoy/config/listener/v4alpha/BUILD +++ b/api/envoy/config/listener/v4alpha/BUILD @@ -9,6 +9,7 @@ api_proto_package( "//envoy/config/accesslog/v4alpha:pkg", "//envoy/config/core/v4alpha:pkg", "//envoy/config/listener/v3:pkg", + "//envoy/config/core/v3:pkg", "//envoy/type/v3:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", ], diff --git a/api/envoy/config/listener/v4alpha/quic_config.proto b/api/envoy/config/listener/v4alpha/quic_config.proto index f87d30f4def7..a1b537449dad 100644 --- a/api/envoy/config/listener/v4alpha/quic_config.proto +++ b/api/envoy/config/listener/v4alpha/quic_config.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package envoy.config.listener.v4alpha; +import "envoy/config/core/v3/base.proto"; + import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; From de45a8d160a9a2813b557e6ec7fe726b14543d5b Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 7 May 2020 10:55:26 +0200 Subject: [PATCH 49/58] fix api change Signed-off-by: Kateryna Nezdolii --- api/envoy/config/listener/v4alpha/BUILD | 2 +- api/envoy/config/listener/v4alpha/quic_config.proto | 2 +- .../quic_listeners/quiche/active_quic_listener_test.cc | 7 ++++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/api/envoy/config/listener/v4alpha/BUILD b/api/envoy/config/listener/v4alpha/BUILD index 2916504fd893..2e97029ceaea 100644 --- a/api/envoy/config/listener/v4alpha/BUILD +++ b/api/envoy/config/listener/v4alpha/BUILD @@ -7,9 +7,9 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ "//envoy/config/accesslog/v4alpha:pkg", + "//envoy/config/core/v3:pkg", "//envoy/config/core/v4alpha:pkg", "//envoy/config/listener/v3:pkg", - "//envoy/config/core/v3:pkg", "//envoy/type/v3:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", ], diff --git a/api/envoy/config/listener/v4alpha/quic_config.proto b/api/envoy/config/listener/v4alpha/quic_config.proto index a1b537449dad..31e2a1bd132c 100644 --- a/api/envoy/config/listener/v4alpha/quic_config.proto +++ b/api/envoy/config/listener/v4alpha/quic_config.proto @@ -37,5 +37,5 @@ message QuicProtocolOptions { // Runtime flag that controls whether the listener is enabled or not. If not specified, defaults // to enabled. - core.v3.RuntimeFeatureFlag enabled = 4; + core.v3.RuntimeFeatureFlag enabled = 4; } diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index b10bcfd83f92..ff480eb85e6c 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -86,9 +86,10 @@ class ActiveQuicListenerTest : public testing::TestWithParammutable_admin_layer(); - loader_ = std::make_unique(Runtime::LoaderPtr{ - new Runtime::LoaderImpl(*dispatcher_, tls_, config, local_info_, init_manager_, store_, - generator_, validation_visitor_, *api_)}); + loader_ = std::make_unique( + Runtime::LoaderPtr{new Runtime::LoaderImpl(*dispatcher_, tls_, config, local_info_, store_, + generator_, validation_visitor_, *api_)}); + listen_socket_ = std::make_shared(local_address_, nullptr, /*bind*/ true); listen_socket_->addOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions()); From bb53249ee1c611975c8d96a644d038948d175d2f Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 7 May 2020 12:12:48 +0200 Subject: [PATCH 50/58] fix clang Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index ff480eb85e6c..3868e4f7d4fd 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -103,9 +103,9 @@ class ActiveQuicListenerTest : public testing::TestWithParam( - std::move(listener_factory_->createActiveUdpListener(connection_handler_, *dispatcher_, - listener_config_))); + quic_listener_ = + static_unique_pointer_cast(listener_factory_->createActiveUdpListener( + connection_handler_, *dispatcher_, listener_config_)); quic_dispatcher_ = ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); simulated_time_system_.advanceTimeWait(std::chrono::milliseconds(100)); From 198c26702e2792286ad631985727f37180309d10 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 7 May 2020 13:39:06 +0200 Subject: [PATCH 51/58] fix v4 protos Signed-off-by: Kateryna Nezdolii --- api/envoy/config/listener/v4alpha/BUILD | 1 - api/envoy/config/listener/v4alpha/quic_config.proto | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/api/envoy/config/listener/v4alpha/BUILD b/api/envoy/config/listener/v4alpha/BUILD index 2e97029ceaea..1d1761a3e941 100644 --- a/api/envoy/config/listener/v4alpha/BUILD +++ b/api/envoy/config/listener/v4alpha/BUILD @@ -7,7 +7,6 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ "//envoy/config/accesslog/v4alpha:pkg", - "//envoy/config/core/v3:pkg", "//envoy/config/core/v4alpha:pkg", "//envoy/config/listener/v3:pkg", "//envoy/type/v3:pkg", diff --git a/api/envoy/config/listener/v4alpha/quic_config.proto b/api/envoy/config/listener/v4alpha/quic_config.proto index 31e2a1bd132c..b2b1df1e374f 100644 --- a/api/envoy/config/listener/v4alpha/quic_config.proto +++ b/api/envoy/config/listener/v4alpha/quic_config.proto @@ -2,7 +2,7 @@ syntax = "proto3"; package envoy.config.listener.v4alpha; -import "envoy/config/core/v3/base.proto"; +import "envoy/config/core/v4alpha/base.proto"; import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; @@ -18,7 +18,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // [#protodoc-title: QUIC listener Config] // Configuration specific to the QUIC protocol. -// Next id: 4 +// Next id: 5 message QuicProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.config.listener.v3.QuicProtocolOptions"; @@ -37,5 +37,5 @@ message QuicProtocolOptions { // Runtime flag that controls whether the listener is enabled or not. If not specified, defaults // to enabled. - core.v3.RuntimeFeatureFlag enabled = 4; + core.v4alpha.RuntimeFeatureFlag enabled = 4; } From 8ad4ae6cc5e38efaea784c4bd90f4c27fa566419 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 7 May 2020 14:22:58 +0200 Subject: [PATCH 52/58] fix generated api shadow Signed-off-by: Kateryna Nezdolii --- .../envoy/config/listener/v4alpha/quic_config.proto | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/generated_api_shadow/envoy/config/listener/v4alpha/quic_config.proto b/generated_api_shadow/envoy/config/listener/v4alpha/quic_config.proto index 97866e4b6ed8..d7871a4973f4 100644 --- a/generated_api_shadow/envoy/config/listener/v4alpha/quic_config.proto +++ b/generated_api_shadow/envoy/config/listener/v4alpha/quic_config.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package envoy.config.listener.v4alpha; +import "envoy/config/core/v4alpha/base.proto"; + import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; @@ -16,7 +18,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // [#protodoc-title: QUIC listener Config] // Configuration specific to the QUIC protocol. -// Next id: 4 +// Next id: 5 message QuicProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.config.listener.v3.QuicProtocolOptions"; @@ -32,4 +34,8 @@ message QuicProtocolOptions { // Connection timeout in milliseconds before the crypto handshake is finished. // 20000ms if not specified. google.protobuf.Duration crypto_handshake_timeout = 3; + + // Runtime flag that controls whether the listener is enabled or not. If not specified, defaults + // to enabled. + core.v4alpha.RuntimeFeatureFlag enabled = 4; } From 99d1e9216f7543678f364cab5dbaa017c57e157a Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 7 May 2020 17:54:51 +0200 Subject: [PATCH 53/58] fix proto again Signed-off-by: Kateryna Nezdolii --- .../envoy/config/listener/v4alpha/quic_config.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generated_api_shadow/envoy/config/listener/v4alpha/quic_config.proto b/generated_api_shadow/envoy/config/listener/v4alpha/quic_config.proto index d7871a4973f4..b2b1df1e374f 100644 --- a/generated_api_shadow/envoy/config/listener/v4alpha/quic_config.proto +++ b/generated_api_shadow/envoy/config/listener/v4alpha/quic_config.proto @@ -37,5 +37,5 @@ message QuicProtocolOptions { // Runtime flag that controls whether the listener is enabled or not. If not specified, defaults // to enabled. - core.v4alpha.RuntimeFeatureFlag enabled = 4; + core.v4alpha.RuntimeFeatureFlag enabled = 4; } From a328c6bb6c6a43e613ffa5209cdaba283a9ef578 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 7 May 2020 19:25:56 +0200 Subject: [PATCH 54/58] more clang tidy Signed-off-by: Kateryna Nezdolii --- .../quiche/active_quic_listener_test.cc | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 3868e4f7d4fd..5502016d37e3 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -48,15 +48,15 @@ namespace Quic { class ActiveQuicListenerPeer { public: - static EnvoyQuicDispatcher* quic_dispatcher(ActiveQuicListener& listener) { + static EnvoyQuicDispatcher* quicDispatcher(ActiveQuicListener& listener) { return listener.quic_dispatcher_.get(); } - static quic::QuicCryptoServerConfig& crypto_config(ActiveQuicListener& listener) { + static quic::QuicCryptoServerConfig& cryptoConfig(ActiveQuicListener& listener) { return *listener.crypto_config_; } - static bool enabled_(ActiveQuicListener& listener) { return listener.enabled_.enabled(); } + static bool enabled(ActiveQuicListener& listener) { return listener.enabled_.enabled(); } }; class ActiveQuicListenerFactoryPeer { @@ -79,7 +79,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam - std::unique_ptr static_unique_pointer_cast(std::unique_ptr&& source) { + std::unique_ptr staticUniquePointerCast(std::unique_ptr&& source) { return std::unique_ptr{static_cast(source.release())}; } @@ -104,9 +104,9 @@ class ActiveQuicListenerTest : public testing::TestWithParam(listener_factory_->createActiveUdpListener( + staticUniquePointerCast(listener_factory_->createActiveUdpListener( connection_handler_, *dispatcher_, listener_config_)); - quic_dispatcher_ = ActiveQuicListenerPeer::quic_dispatcher(*quic_listener_); + quic_dispatcher_ = ActiveQuicListenerPeer::quicDispatcher(*quic_listener_); simulated_time_system_.advanceTimeWait(std::chrono::milliseconds(100)); } @@ -171,7 +171,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam(local_address_, nullptr, /*bind*/ false)); quic::CryptoHandshakeMessage chlo = quic::test::crypto_test_utils::GenerateDefaultInchoateCHLO( &clock_, quic::AllSupportedVersions()[0].transport_version, - &ActiveQuicListenerPeer::crypto_config(*quic_listener_)); + &ActiveQuicListenerPeer::cryptoConfig(*quic_listener_)); chlo.SetVector(quic::kCOPT, quic::QuicTagVector{quic::kREJ}); quic::CryptoHandshakeMessage full_chlo; quic::QuicReferenceCountedPointer signed_config( @@ -179,7 +179,7 @@ class ActiveQuicListenerTest : public testing::TestWithParamrun(Event::Dispatcher::RunType::NonBlock); // If listener was enabled, there should have been session created for active connection. EXPECT_TRUE(quic_dispatcher_->session_map().empty()); - EXPECT_FALSE(ActiveQuicListenerPeer::enabled_(*quic_listener_)); + EXPECT_FALSE(ActiveQuicListenerPeer::enabled(*quic_listener_)); } TEST_P(ActiveQuicListenerTest, QuicProcessingDisabledAndEnabled) { @@ -363,13 +363,13 @@ TEST_P(ActiveQuicListenerTest, QuicProcessingDisabledAndEnabled) { dispatcher_->run(Event::Dispatcher::RunType::NonBlock); // If listener was enabled, there should have been session created for active connection. EXPECT_TRUE(quic_dispatcher_->session_map().empty()); - EXPECT_FALSE(ActiveQuicListenerPeer::enabled_(*quic_listener_)); + EXPECT_FALSE(ActiveQuicListenerPeer::enabled(*quic_listener_)); Runtime::LoaderSingleton::getExisting()->mergeValues({{"quic.enabled", " true"}}); configureMocks(/* connection_count = */ 1); sendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); EXPECT_FALSE(quic_dispatcher_->session_map().empty()); - EXPECT_TRUE(ActiveQuicListenerPeer::enabled_(*quic_listener_)); + EXPECT_TRUE(ActiveQuicListenerPeer::enabled(*quic_listener_)); } class ActiveQuicListenerEmptyFlagConfigTest : public ActiveQuicListenerTest { @@ -394,7 +394,7 @@ TEST_P(ActiveQuicListenerEmptyFlagConfigTest, ReceiveFullQuicCHLO) { dispatcher_->run(Event::Dispatcher::RunType::NonBlock); EXPECT_FALSE(buffered_packets->HasChlosBuffered()); EXPECT_FALSE(quic_dispatcher_->session_map().empty()); - EXPECT_TRUE(ActiveQuicListenerPeer::enabled_(*quic_listener_)); + EXPECT_TRUE(ActiveQuicListenerPeer::enabled(*quic_listener_)); ReadFromClientSockets(); } From e055f8ac37dbe57dcd14d5faa15bc7f6e283cc62 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 14 May 2020 18:02:38 +0200 Subject: [PATCH 55/58] apply review comments Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener.cc | 4 ++-- .../quic_listeners/quiche/active_quic_listener.h | 4 ++-- .../quic_listeners/quiche/active_quic_listener_test.cc | 10 +--------- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.cc b/source/extensions/quic_listeners/quiche/active_quic_listener.cc index 5bc057a50bf9..30c65d443e8a 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.cc +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.cc @@ -21,7 +21,7 @@ ActiveQuicListener::ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, Network::Socket::OptionsSharedPtr options, - const envoy::config::core::v3::RuntimeFeatureFlag enabled) + const envoy::config::core::v3::RuntimeFeatureFlag& enabled) : ActiveQuicListener(dispatcher, parent, listener_config.listenSocketFactory().getListenSocket(), listener_config, quic_config, std::move(options), enabled) {} @@ -32,7 +32,7 @@ ActiveQuicListener::ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, Network::Socket::OptionsSharedPtr options, - const envoy::config::core::v3::RuntimeFeatureFlag enabled) + const envoy::config::core::v3::RuntimeFeatureFlag& enabled) : Server::ConnectionHandlerImpl::ActiveListenerImplBase(parent, &listener_config), dispatcher_(dispatcher), version_manager_(quic::CurrentSupportedVersions()), listen_socket_(*listen_socket), enabled_(enabled, Runtime::LoaderSingleton::get()) { diff --git a/source/extensions/quic_listeners/quiche/active_quic_listener.h b/source/extensions/quic_listeners/quiche/active_quic_listener.h index 94a3f5076fdb..8d0d5c9dd46e 100644 --- a/source/extensions/quic_listeners/quiche/active_quic_listener.h +++ b/source/extensions/quic_listeners/quiche/active_quic_listener.h @@ -28,13 +28,13 @@ class ActiveQuicListener : public Network::UdpListenerCallbacks, ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, Network::Socket::OptionsSharedPtr options, - const envoy::config::core::v3::RuntimeFeatureFlag enabled); + const envoy::config::core::v3::RuntimeFeatureFlag& enabled); ActiveQuicListener(Event::Dispatcher& dispatcher, Network::ConnectionHandler& parent, Network::SocketSharedPtr listen_socket, Network::ListenerConfig& listener_config, const quic::QuicConfig& quic_config, Network::Socket::OptionsSharedPtr options, - const envoy::config::core::v3::RuntimeFeatureFlag enabled); + const envoy::config::core::v3::RuntimeFeatureFlag& enabled); ~ActiveQuicListener() override; diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 5502016d37e3..fdf2d9cb607a 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -348,16 +348,8 @@ TEST_P(ActiveQuicListenerTest, ProcessBufferedChlos) { ReadFromClientSockets(); } -TEST_P(ActiveQuicListenerTest, QuicProcessingDisabled) { - Runtime::LoaderSingleton::getExisting()->mergeValues({{"quic.enabled", " false"}}); - sendFullCHLO(quic::test::TestConnectionId(1)); - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - // If listener was enabled, there should have been session created for active connection. - EXPECT_TRUE(quic_dispatcher_->session_map().empty()); - EXPECT_FALSE(ActiveQuicListenerPeer::enabled(*quic_listener_)); -} - TEST_P(ActiveQuicListenerTest, QuicProcessingDisabledAndEnabled) { + EXPECT_TRUE(ActiveQuicListenerPeer::enabled(*quic_listener_)); Runtime::LoaderSingleton::getExisting()->mergeValues({{"quic.enabled", " false"}}); sendFullCHLO(quic::test::TestConnectionId(1)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); From 07375501e7c0ded35de692efca2d98e50ed92101 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Thu, 14 May 2020 18:26:47 +0200 Subject: [PATCH 56/58] nits in test Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_test.cc | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index fdf2d9cb607a..41c91fd3debd 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -95,12 +95,8 @@ class ActiveQuicListenerTest : public testing::TestWithParamaddOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions()); listen_socket_->addOptions(Network::SocketOptionFactory::buildRxQueueOverFlowOptions()); - EXPECT_CALL(listener_config_, listenSocketFactory()) - .Times(AnyNumber()) - .WillRepeatedly(ReturnRef(socket_factory_)); - EXPECT_CALL(socket_factory_, getListenSocket()) - .Times(AnyNumber()) - .WillRepeatedly(Return(listen_socket_)); + ON_CALL(listener_config_, listenSocketFactory()).WillByDefault(ReturnRef(socket_factory_)); + ON_CALL(socket_factory_, getListenSocket()).WillByDefault(Return(listen_socket_)); listener_factory_ = createQuicListenerFactory(yamlForQuicConfig()); quic_listener_ = @@ -263,7 +259,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam quic_listener_; Network::ActiveUdpListenerFactoryPtr listener_factory_; - Network::MockListenSocketFactory socket_factory_; + NiceMock socket_factory_; EnvoyQuicDispatcher* quic_dispatcher_; std::unique_ptr loader_; From a507b8b8b26a4fc15269f1a1713d065f848af489 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Fri, 15 May 2020 10:26:21 +0200 Subject: [PATCH 57/58] dummy to retrigger ci Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_test.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 41c91fd3debd..04265856b984 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -103,7 +103,6 @@ class ActiveQuicListenerTest : public testing::TestWithParam(listener_factory_->createActiveUdpListener( connection_handler_, *dispatcher_, listener_config_)); quic_dispatcher_ = ActiveQuicListenerPeer::quicDispatcher(*quic_listener_); - simulated_time_system_.advanceTimeWait(std::chrono::milliseconds(100)); } From 6ce2379a4dbc8c3ec5a21cb4e434114690993c52 Mon Sep 17 00:00:00 2001 From: Kateryna Nezdolii Date: Fri, 15 May 2020 11:29:57 +0200 Subject: [PATCH 58/58] remove unused import Signed-off-by: Kateryna Nezdolii --- .../quic_listeners/quiche/active_quic_listener_test.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index 04265856b984..c3836ac38163 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -39,7 +39,6 @@ #include "extensions/quic_listeners/quiche/platform/envoy_quic_clock.h" #include "extensions/quic_listeners/quiche/envoy_quic_utils.h" -using testing::AnyNumber; using testing::Return; using testing::ReturnRef;