restart flag

Signed-off-by: Ali Beyad <abeyad@google.com>

source/common/quic/BUILD

@@ -180,6 +180,7 @@ envoy_cc_library(
         "//envoy/http:codec_interface",
         "//envoy/http:persistent_quic_info_interface",
         "//envoy/registry",
+        "//source/common/runtime:runtime_lib",
         "//source/extensions/quic/crypto_stream:envoy_quic_crypto_client_stream_lib",
         "//source/extensions/transport_sockets/tls:ssl_socket_lib",
         "@com_github_google_quiche//:quic_core_http_spdy_session_lib",
@@ -362,7 +363,6 @@ envoy_cc_library(
         "//envoy/event:dispatcher_interface",
         "//source/common/network:socket_option_factory_lib",
         "//source/common/network:udp_packet_writer_handler_lib",
-        "//source/common/runtime:runtime_lib",
         "@com_github_google_quiche//:quic_core_connection_lib",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
     ],
@@ -436,7 +436,6 @@ envoy_cc_library(
         "//source/common/network:socket_option_factory_lib",
         "//source/common/protobuf:utility_lib",
         "//source/common/quic:quic_io_handle_wrapper_lib",
-        "//source/common/runtime:runtime_lib",
         "@com_github_google_quiche//:quic_core_config_lib",
         "@com_github_google_quiche//:quic_core_http_header_list_lib",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",

source/common/quic/client_connection_factory_impl.cc

@@ -1,5 +1,7 @@
 #include "source/common/quic/client_connection_factory_impl.h"
 
+#include "source/common/runtime/runtime_features.h"
+
 namespace Envoy {
 namespace Quic {
 
@@ -45,7 +47,8 @@ std::unique_ptr<Network::ClientConnection> createQuicNetworkConnection(
   ASSERT(!quic_versions.empty());
   auto connection = std::make_unique<EnvoyQuicClientConnection>(
       quic::QuicUtils::CreateRandomConnectionId(), server_addr, info_impl->conn_helper_,
-      info_impl->alarm_factory_, quic_versions, local_addr, dispatcher, options, generator);
+      info_impl->alarm_factory_, quic_versions, local_addr, dispatcher, options, generator,
+      Runtime::runtimeFeatureEnabled("envoy.restart_features.prefer_quic_client_udp_gro"));
 
   // TODO (danzh) move this temporary config and initial RTT configuration to h3 pool.
   quic::QuicConfig config = info_impl->quic_config_;

source/common/quic/envoy_quic_client_connection.cc

@@ -7,7 +7,6 @@
 #include "source/common/network/socket_option_factory.h"
 #include "source/common/network/udp_packet_writer_handler_impl.h"
 #include "source/common/quic/envoy_quic_utils.h"
-#include "source/common/runtime/runtime_features.h"
 
 namespace Envoy {
 namespace Quic {
@@ -31,35 +30,37 @@ EnvoyQuicClientConnection::EnvoyQuicClientConnection(
     const quic::ParsedQuicVersionVector& supported_versions,
     Network::Address::InstanceConstSharedPtr local_addr, Event::Dispatcher& dispatcher,
     const Network::ConnectionSocket::OptionsSharedPtr& options,
-    quic::ConnectionIdGeneratorInterface& generator)
+    quic::ConnectionIdGeneratorInterface& generator, const bool prefer_gro)
     : EnvoyQuicClientConnection(
           server_connection_id, helper, alarm_factory, supported_versions, dispatcher,
-          createConnectionSocket(initial_peer_address, local_addr, options), generator) {}
+          createConnectionSocket(initial_peer_address, local_addr, options, prefer_gro), generator,
+          prefer_gro) {}
 
 EnvoyQuicClientConnection::EnvoyQuicClientConnection(
     const quic::QuicConnectionId& server_connection_id, quic::QuicConnectionHelperInterface& helper,
     quic::QuicAlarmFactory& alarm_factory, const quic::ParsedQuicVersionVector& supported_versions,
     Event::Dispatcher& dispatcher, Network::ConnectionSocketPtr&& connection_socket,
-    quic::ConnectionIdGeneratorInterface& generator)
+    quic::ConnectionIdGeneratorInterface& generator, const bool prefer_gro)
     : EnvoyQuicClientConnection(
           server_connection_id, helper, alarm_factory,
           new EnvoyQuicPacketWriter(
               std::make_unique<Network::UdpDefaultWriter>(connection_socket->ioHandle())),
           /*owns_writer=*/true, supported_versions, dispatcher, std::move(connection_socket),
-          generator) {}
+          generator, prefer_gro) {}
 
 EnvoyQuicClientConnection::EnvoyQuicClientConnection(
     const quic::QuicConnectionId& server_connection_id, quic::QuicConnectionHelperInterface& helper,
     quic::QuicAlarmFactory& alarm_factory, quic::QuicPacketWriter* writer, bool owns_writer,
     const quic::ParsedQuicVersionVector& supported_versions, Event::Dispatcher& dispatcher,
     Network::ConnectionSocketPtr&& connection_socket,
-    quic::ConnectionIdGeneratorInterface& generator)
+    quic::ConnectionIdGeneratorInterface& generator, const bool prefer_gro)
     : quic::QuicConnection(server_connection_id, quic::QuicSocketAddress(),
                            envoyIpAddressToQuicSocketAddress(
                                connection_socket->connectionInfoProvider().remoteAddress()->ip()),
                            &helper, &alarm_factory, writer, owns_writer,
                            quic::Perspective::IS_CLIENT, supported_versions, generator),
-      QuicNetworkConnection(std::move(connection_socket)), dispatcher_(dispatcher) {}
+      QuicNetworkConnection(std::move(connection_socket)), dispatcher_(dispatcher),
+      prefer_gro_(prefer_gro) {}
 
 void EnvoyQuicClientConnection::processPacket(
     Network::Address::InstanceConstSharedPtr local_address,
@@ -156,7 +157,7 @@ void EnvoyQuicClientConnection::probeWithNewPort(const quic::QuicSocketAddress&
   // The probing socket will have the same host but a different port.
   auto probing_socket =
       createConnectionSocket(connectionSocket()->connectionInfoProvider().remoteAddress(),
-                             new_local_address, connectionSocket()->options());
+                             new_local_address, connectionSocket()->options(), prefer_gro_);
   setUpConnectionSocket(*probing_socket, delegate_);
   auto writer = std::make_unique<EnvoyQuicPacketWriter>(
       std::make_unique<Network::UdpDefaultWriter>(probing_socket->ioHandle()));
@@ -230,8 +231,7 @@ void EnvoyQuicClientConnection::onFileEvent(uint32_t events,
   if (connected() && (events & Event::FileReadyType::Read)) {
     Api::IoErrorPtr err = Network::Utility::readPacketsFromSocket(
         connection_socket.ioHandle(), *connection_socket.connectionInfoProvider().localAddress(),
-        *this, dispatcher_.timeSource(),
-        Runtime::runtimeFeatureEnabled("envoy.restart_features.prefer_quic_client_udp_gro"),
+        *this, dispatcher_.timeSource(), prefer_gro_,
         // For client connections, recvmmsg consumes more CPU with no performance improvement.
         /*allow_mmsg=*/false, packets_dropped_);
     if (err == nullptr) {

source/common/quic/envoy_quic_client_connection.h

@@ -60,7 +60,7 @@ class EnvoyQuicClientConnection : public quic::QuicConnection,
                             Network::Address::InstanceConstSharedPtr local_addr,
                             Event::Dispatcher& dispatcher,
                             const Network::ConnectionSocket::OptionsSharedPtr& options,
-                            quic::ConnectionIdGeneratorInterface& generator);
+                            quic::ConnectionIdGeneratorInterface& generator, bool prefer_gro);
 
   EnvoyQuicClientConnection(const quic::QuicConnectionId& server_connection_id,
                             quic::QuicConnectionHelperInterface& helper,
@@ -69,7 +69,7 @@ class EnvoyQuicClientConnection : public quic::QuicConnection,
                             const quic::ParsedQuicVersionVector& supported_versions,
                             Event::Dispatcher& dispatcher,
                             Network::ConnectionSocketPtr&& connection_socket,
-                            quic::ConnectionIdGeneratorInterface& generator);
+                            quic::ConnectionIdGeneratorInterface& generator, bool prefer_gro);
 
   // Network::UdpPacketProcessor
   void processPacket(Network::Address::InstanceConstSharedPtr local_address,
@@ -133,7 +133,7 @@ class EnvoyQuicClientConnection : public quic::QuicConnection,
                             const quic::ParsedQuicVersionVector& supported_versions,
                             Event::Dispatcher& dispatcher,
                             Network::ConnectionSocketPtr&& connection_socket,
-                            quic::ConnectionIdGeneratorInterface& generator);
+                            quic::ConnectionIdGeneratorInterface& generator, bool prefer_gro);
 
   void onFileEvent(uint32_t events, Network::ConnectionSocket& connection_socket);
 
@@ -147,6 +147,7 @@ class EnvoyQuicClientConnection : public quic::QuicConnection,
   Event::Dispatcher& dispatcher_;
   bool migrate_port_on_path_degrading_{false};
   uint8_t num_socket_switches_{0};
+  const bool prefer_gro_;
 };
 
 } // namespace Quic

source/common/quic/envoy_quic_utils.cc

@@ -10,7 +10,6 @@
 #include "source/common/network/socket_option_factory.h"
 #include "source/common/network/utility.h"
 #include "source/common/protobuf/utility.h"
-#include "source/common/runtime/runtime_features.h"
 
 namespace Envoy {
 namespace Quic {
@@ -139,7 +138,8 @@ Http::StreamResetReason quicErrorCodeToEnvoyRemoteResetReason(quic::QuicErrorCod
 Network::ConnectionSocketPtr
 createConnectionSocket(const Network::Address::InstanceConstSharedPtr& peer_addr,
                        Network::Address::InstanceConstSharedPtr& local_addr,
-                       const Network::ConnectionSocket::OptionsSharedPtr& options) {
+                       const Network::ConnectionSocket::OptionsSharedPtr& options,
+                       const bool prefer_gro) {
   if (local_addr == nullptr) {
     local_addr = Network::Utility::getLocalAddress(peer_addr->ip()->version());
   }
@@ -151,8 +151,7 @@ createConnectionSocket(const Network::Address::InstanceConstSharedPtr& peer_addr
   }
   connection_socket->addOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions());
   connection_socket->addOptions(Network::SocketOptionFactory::buildRxQueueOverFlowOptions());
-  if (Runtime::runtimeFeatureEnabled("envoy.restart_features.prefer_quic_client_udp_gro") &&
-      Api::OsSysCallsSingleton::get().supportsUdpGro()) {
+  if (prefer_gro && Api::OsSysCallsSingleton::get().supportsUdpGro()) {
     connection_socket->addOptions(Network::SocketOptionFactory::buildUdpGroOptions());
   }
   if (options != nullptr) {

source/common/quic/envoy_quic_utils.h

@@ -161,7 +161,8 @@ Http::StreamResetReason quicErrorCodeToEnvoyRemoteResetReason(quic::QuicErrorCod
 Network::ConnectionSocketPtr
 createConnectionSocket(const Network::Address::InstanceConstSharedPtr& peer_addr,
                        Network::Address::InstanceConstSharedPtr& local_addr,
-                       const Network::ConnectionSocket::OptionsSharedPtr& options);
+                       const Network::ConnectionSocket::OptionsSharedPtr& options,
+                       bool prefer_gro = false);
 
 // Convert a cert in string form to X509 object.
 // Return nullptr if the bytes passed cannot be passed.

source/common/runtime/runtime_features.cc

@@ -102,10 +102,10 @@ RUNTIME_GUARD(envoy_reloadable_features_validate_connect);
 RUNTIME_GUARD(envoy_reloadable_features_validate_grpc_header_before_log_grpc_status);
 RUNTIME_GUARD(envoy_reloadable_features_validate_upstream_headers);
 RUNTIME_GUARD(envoy_restart_features_allow_client_socket_creation_failure);
+RUNTIME_GUARD(envoy_restart_features_prefer_quic_client_udp_gro);
 RUNTIME_GUARD(envoy_restart_features_send_goaway_for_premature_rst_streams);
 RUNTIME_GUARD(envoy_restart_features_udp_read_normalize_addresses);
 RUNTIME_GUARD(envoy_restart_features_use_eds_cache_for_ads);
-RUNTIME_GUARD(envoy_restart_features_prefer_quic_client_udp_gro);
 
 // Begin false flags. Most of them should come with a TODO to flip true.
 

test/common/network/utility_test.cc

@@ -829,15 +829,17 @@ TEST(PacketLoss, LossTest) {
   NiceMock<MockUdpPacketProcessor> processor;
   MonotonicTime time(std::chrono::seconds(0));
   uint32_t packets_dropped = 0;
-  Utility::readFromSocket(handle, *address, processor, time, UdpRecvMsgMethod::RecvMmsg, &packets_dropped);
+  Utility::readFromSocket(handle, *address, processor, time, UdpRecvMsgMethod::RecvMmsg,
+                          &packets_dropped);
   EXPECT_EQ(1, packets_dropped);
 
   // Send another packet.
   EXPECT_EQ(ABSL_ARRAYSIZE(buf), sendto(fd, buf, ABSL_ARRAYSIZE(buf), 0,
                                         reinterpret_cast<sockaddr*>(&storage), sizeof(storage)));
 
   // Make sure the drop count is now 2.
-  Utility::readFromSocket(handle, *address, processor, time, UdpRecvMsgMethod::RecvMmsg, &packets_dropped);
+  Utility::readFromSocket(handle, *address, processor, time, UdpRecvMsgMethod::RecvMmsg,
+                          &packets_dropped);
   EXPECT_EQ(2, packets_dropped);
 }
 #endif

test/common/quic/envoy_quic_client_session_test.cc

@@ -47,7 +47,7 @@ class TestEnvoyQuicClientConnection : public EnvoyQuicClientConnection {
                                 quic::ConnectionIdGeneratorInterface& generator)
       : EnvoyQuicClientConnection(server_connection_id, helper, alarm_factory, &writer, false,
                                   supported_versions, dispatcher, std::move(connection_socket),
-                                  generator) {
+                                  generator, /*prefer_gro=*/true) {
     SetEncrypter(quic::ENCRYPTION_FORWARD_SECURE,
                  std::make_unique<quic::test::TaggingEncrypter>(quic::ENCRYPTION_FORWARD_SECURE));
     InstallDecrypter(quic::ENCRYPTION_FORWARD_SECURE,
@@ -77,7 +77,8 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam<quic::ParsedQui
         peer_socket_(createConnectionSocket(self_addr_, peer_addr_, nullptr)),
         quic_connection_(new TestEnvoyQuicClientConnection(
             quic::test::TestConnectionId(), connection_helper_, alarm_factory_, writer_,
-            quic_version_, *dispatcher_, createConnectionSocket(peer_addr_, self_addr_, nullptr),
+            quic_version_, *dispatcher_,
+            createConnectionSocket(peer_addr_, self_addr_, nullptr, /*prefer_gro=*/true),
             connection_id_generator_)),
         crypto_config_(std::make_shared<quic::QuicCryptoClientConfig>(
             quic::test::crypto_test_utils::ProofVerifierForTesting())),
@@ -502,7 +503,9 @@ TEST_P(EnvoyQuicClientSessionTest, UsesUdpGro) {
   // Make sure the option for GRO is set on the socket.
   int sock_opt;
   socklen_t sock_len = sizeof(int);
-  EXPECT_EQ(0, peer_socket_->getSocketOption(SOL_UDP, UDP_GRO, &sock_opt, &sock_len).return_value_);
+  EXPECT_EQ(0, quic_connection_->connectionSocket()
+                   ->getSocketOption(SOL_UDP, UDP_GRO, &sock_opt, &sock_len)
+                   .return_value_);
   EXPECT_EQ(1, sock_opt);
 
   // GRO uses `recvmsg`, not `recvmmsg`.

test/common/quic/envoy_quic_client_stream_test.cc

@@ -47,7 +47,8 @@ class EnvoyQuicClientStreamTest : public testing::Test {
         quic_connection_(new MockEnvoyQuicClientConnection(
             quic::test::TestConnectionId(), connection_helper_, alarm_factory_, &writer_,
             /*owns_writer=*/false, {quic_version_}, *dispatcher_,
-            createConnectionSocket(peer_addr_, self_addr_, nullptr), connection_id_generator_)),
+            createConnectionSocket(peer_addr_, self_addr_, nullptr, /*prefer_gro=*/true),
+            connection_id_generator_)),
         quic_session_(quic_config_, {quic_version_},
                       std::unique_ptr<EnvoyQuicClientConnection>(quic_connection_), *dispatcher_,
                       quic_config_.GetInitialStreamFlowControlWindowToSend() * 2,

test/common/quic/test_utils.h

@@ -75,7 +75,7 @@ class MockEnvoyQuicClientConnection : public EnvoyQuicClientConnection {
                                 quic::ConnectionIdGeneratorInterface& generator)
       : EnvoyQuicClientConnection(server_connection_id, helper, alarm_factory, writer, owns_writer,
                                   supported_versions, dispatcher, std::move(connection_socket),
-                                  generator) {}
+                                  generator, /*prefer_gro=*/true) {}
 
   MOCK_METHOD(quic::MessageStatus, SendMessage,
               (quic::QuicMessageId, absl::Span<quiche::QuicheMemSlice>, bool));

test/integration/quic_http_integration_test.cc

@@ -73,7 +73,8 @@ class TestEnvoyQuicClientConnection : public EnvoyQuicClientConnection {
                                 bool validation_failure_on_path_response,
                                 quic::ConnectionIdGeneratorInterface& generator)
       : EnvoyQuicClientConnection(server_connection_id, initial_peer_address, helper, alarm_factory,
-                                  supported_versions, local_addr, dispatcher, options, generator),
+                                  supported_versions, local_addr, dispatcher, options, generator,
+                                  /*prefer_gro=*/true),
         dispatcher_(dispatcher),
         validation_failure_on_path_response_(validation_failure_on_path_response) {}
 
@@ -772,7 +773,7 @@ TEST_P(QuicHttpIntegrationTest, PortMigration) {
   Network::Address::InstanceConstSharedPtr local_addr =
       Network::Test::getCanonicalLoopbackAddress(version_);
   quic_connection_->switchConnectionSocket(
-      createConnectionSocket(server_addr_, local_addr, nullptr));
+      createConnectionSocket(server_addr_, local_addr, nullptr, /*prefer_gro=*/true));
   EXPECT_NE(old_port, local_addr->ip()->port());
   // Send the rest data.
   codec_client_->sendData(*request_encoder_, 1024u, true);
@@ -801,7 +802,7 @@ TEST_P(QuicHttpIntegrationTest, PortMigration) {
   auto options = std::make_shared<Network::Socket::Options>();
   options->push_back(option);
   quic_connection_->switchConnectionSocket(
-      createConnectionSocket(server_addr_, local_addr, options));
+      createConnectionSocket(server_addr_, local_addr, options, /*prefer_gro=*/true));
   EXPECT_TRUE(codec_client_->disconnected());
   cleanupUpstreamAndDownstream();
 }