quic: change QuicNetworkConnectivityObserver interface

changelogs/current.yaml

@@ -58,6 +58,10 @@ minor_behavior_changes:
   change: |
     HTTP/3 alt-svc headers will now be respected from IP-address-based hostnames. This change is
     guarded by runtime guard ``envoy.reloadable_features.allow_alt_svc_for_ips``.
+- area: http3
+  change: |
+    Make upstream connections respond to default network change on mobile devices. This change is guarded
+    by runtime guard ``envoy.reloadable_features.quic_upstream_connection_handle_network_change``.
 - area: lua
   change: |
     When Lua script executes httpCall, backpressure is exercised when receiving body from downstream client. This behavior can be reverted

envoy/network/connection.h

@@ -413,6 +413,19 @@ class ServerConnection : public virtual Connection {
 
 using ServerConnectionPtr = std::unique_ptr<ServerConnection>;
 
+/**
+ * Used by a client connection to report any changes to its underlying network.
+ */
+class NetworkChangeCallbacks {
+public:
+  virtual ~NetworkChangeCallbacks() = default;
+
+  /**
+   * Called by a connection if its underlying network has changed.
+   */
+  virtual void onConnectionNetworkChanged() PURE;
+};
+
 /**
  * Connections capable of outbound connects.
  */
@@ -423,6 +436,12 @@ class ClientConnection : public virtual Connection {
    * registered via addConnectionCallbacks().
    */
   virtual void connect() PURE;
+
+  /**
+   * Register network change callbacks to be called when the underlying network has changed.
+   * @param callback must outlive this connection.
+   */
+  virtual void setNetworkChangeCallbacks(NetworkChangeCallbacks& callback) PURE;
 };
 
 using ClientConnectionPtr = std::unique_ptr<ClientConnection>;

mobile/library/common/internal_engine.cc

@@ -288,7 +288,7 @@ void InternalEngine::onDefaultNetworkAvailable() {
 void InternalEngine::onDefaultNetworkChanged(NetworkType network) {
   ENVOY_LOG_MISC(trace, "Calling the default network changed callback");
   dispatcher_->post([&, network]() -> void {
-    envoy_netconf_t configuration = Network::ConnectivityManagerImpl::setPreferredNetwork(network);
+    envoy_netconf_t configuration = connectivity_manager_->onNetworkMadeDefault(network);
     if (Runtime::runtimeFeatureEnabled(
             "envoy.reloadable_features.dns_cache_set_ip_version_to_remove")) {
       // The IP version to remove flag must be set first before refreshing the DNS cache so that

mobile/library/common/network/BUILD

@@ -4,6 +4,16 @@ licenses(["notice"])  # Apache 2
 
 envoy_mobile_package()
 
+envoy_cc_library(
+    name = "envoy_mobile_quic_network_observer_registry_factory_lib",
+    srcs = ["envoy_mobile_quic_network_observer_registry_factory.cc"],
+    hdrs = ["envoy_mobile_quic_network_observer_registry_factory.h"],
+    repository = "@envoy",
+    deps = [
+        "@envoy//source/common/quic:envoy_quic_client_session_lib",
+    ],
+)
+
 envoy_cc_library(
     name = "connectivity_manager_lib",
     srcs = [
@@ -14,6 +24,7 @@ envoy_cc_library(
     ],
     repository = "@envoy",
     deps = [
+        ":envoy_mobile_quic_network_observer_registry_factory_lib",
         ":proxy_settings_lib",
         "//library/common:engine_types_lib",
         "//library/common/network:src_addr_socket_option_lib",

mobile/library/common/network/connectivity_manager.cc

@@ -77,6 +77,14 @@ constexpr unsigned int InitialFaultThreshold = 1;
 // L7 bytes) before switching socket mode.
 constexpr unsigned int MaxFaultThreshold = 3;
 
+ConnectivityManagerImpl::ConnectivityManagerImpl(Upstream::ClusterManager& cluster_manager,
+                                                 DnsCacheManagerSharedPtr dns_cache_manager)
+    : cluster_manager_(cluster_manager), dns_cache_manager_(dns_cache_manager) {
+  if (quic_upstream_connection_handle_network_change_) {
+    cluster_manager_.createNetworkObserverRegistries(quic_observer_registry_factory_);
+  }
+}
+
 ConnectivityManagerImpl::NetworkState ConnectivityManagerImpl::network_state_{
     1, NetworkType::Generic, MaxFaultThreshold, SocketMode::DefaultPreferredNetworkMode,
     Thread::MutexBasicLockable{}};
@@ -102,6 +110,16 @@ envoy_netconf_t ConnectivityManagerImpl::setPreferredNetwork(NetworkType network
   return network_state_.configuration_key_;
 }
 
+envoy_netconf_t ConnectivityManagerImpl::onNetworkMadeDefault(NetworkType network) {
+  ENVOY_LOG_MISC(trace, "Default network changed to {}", static_cast<int>(network));
+  envoy_netconf_t configuration_key = setPreferredNetwork(network);
+  for (std::reference_wrapper<Quic::EnvoyMobileQuicNetworkObserverRegistry> registry :
+       quic_observer_registry_factory_.getCreatedObserverRegistries()) {
+    registry.get().onNetworkMadeDefault();
+  }
+  return configuration_key;
+}
+
 void ConnectivityManagerImpl::setProxySettings(ProxySettingsConstSharedPtr new_proxy_settings) {
   if (proxy_settings_ == nullptr && new_proxy_settings != nullptr) {
     ENVOY_LOG_EVENT(info, "netconf_proxy_change", "{}", new_proxy_settings->asString());
@@ -305,16 +323,17 @@ Socket::OptionsSharedPtr ConnectivityManagerImpl::getUpstreamSocketOptions(Netwo
       network != NetworkType::Generic) {
     return getAlternateInterfaceSocketOptions(network);
   }
-
-  // Envoy uses the hash signature of overridden socket options to choose a connection pool.
-  // Setting a dummy socket option is a hack that allows us to select a different
-  // connection pool without materially changing the socket configuration.
   ASSERT(static_cast<int>(network) >= 0 && static_cast<int>(network) < 3);
-  int ttl_value = DEFAULT_IP_TTL + static_cast<int>(network);
   auto options = std::make_shared<Socket::Options>();
-  options->push_back(std::make_shared<AddrFamilyAwareSocketOptionImpl>(
-      envoy::config::core::v3::SocketOption::STATE_PREBIND, ENVOY_SOCKET_IP_TTL,
-      ENVOY_SOCKET_IPV6_UNICAST_HOPS, ttl_value));
+  if (!quic_upstream_connection_handle_network_change_) {
+    // Envoy uses the hash signature of overridden socket options to choose a connection pool.
+    // Setting a dummy socket option is a hack that allows us to select a different
+    // connection pool without materially changing the socket configuration.
+    int ttl_value = DEFAULT_IP_TTL + static_cast<int>(network);
+    options->push_back(std::make_shared<AddrFamilyAwareSocketOptionImpl>(
+        envoy::config::core::v3::SocketOption::STATE_PREBIND, ENVOY_SOCKET_IP_TTL,
+        ENVOY_SOCKET_IPV6_UNICAST_HOPS, ttl_value));
+  }
   return options;
 }
 

mobile/library/common/network/connectivity_manager.h

@@ -11,6 +11,7 @@
 #include "source/extensions/common/dynamic_forward_proxy/dns_cache_impl.h"
 
 #include "library/common/engine_types.h"
+#include "library/common/network/envoy_mobile_quic_network_observer_registry_factory.h"
 #include "library/common/network/proxy_settings.h"
 #include "library/common/types/c_types.h"
 
@@ -186,6 +187,13 @@ class ConnectivityManager
    * @returns the default DNS cache set up in base configuration or nullptr.
    */
   virtual Extensions::Common::DynamicForwardProxy::DnsCacheSharedPtr dnsCache() PURE;
+
+  /**
+   * Called when the OS changes the preferred network.
+   * @param network, the OS-preferred network.
+   * @returns configuration key of the latest snapshot of network configuration state.
+   */
+  virtual envoy_netconf_t onNetworkMadeDefault(NetworkType network) PURE;
 };
 
 class ConnectivityManagerImpl : public ConnectivityManager,
@@ -201,8 +209,7 @@ class ConnectivityManagerImpl : public ConnectivityManager,
   static envoy_netconf_t setPreferredNetwork(NetworkType network);
 
   ConnectivityManagerImpl(Upstream::ClusterManager& cluster_manager,
-                          DnsCacheManagerSharedPtr dns_cache_manager)
-      : cluster_manager_(cluster_manager), dns_cache_manager_(dns_cache_manager) {}
+                          DnsCacheManagerSharedPtr dns_cache_manager);
 
   // Extensions::Common::DynamicForwardProxy::DnsCache::UpdateCallbacks
   void onDnsHostAddOrUpdate(
@@ -232,6 +239,7 @@ class ConnectivityManagerImpl : public ConnectivityManager,
                                                     SocketMode socket_mode) override;
   envoy_netconf_t addUpstreamSocketOptions(Socket::OptionsSharedPtr options) override;
   Extensions::Common::DynamicForwardProxy::DnsCacheSharedPtr dnsCache() override;
+  envoy_netconf_t onNetworkMadeDefault(NetworkType network) override;
 
 private:
   struct NetworkState {
@@ -252,9 +260,13 @@ class ConnectivityManagerImpl : public ConnectivityManager,
   Extensions::Common::DynamicForwardProxy::DnsCache::AddUpdateCallbacksHandlePtr
       dns_callbacks_handle_{nullptr};
   Upstream::ClusterManager& cluster_manager_;
+  Quic::EnvoyMobileQuicNetworkObserverRegistryFactory quic_observer_registry_factory_;
   DnsCacheManagerSharedPtr dns_cache_manager_;
   ProxySettingsConstSharedPtr proxy_settings_;
   static NetworkState network_state_;
+  // If true, all QUIC connections will respond to network changes.
+  const bool quic_upstream_connection_handle_network_change_{Runtime::runtimeFeatureEnabled(
+      "envoy.reloadable_features.quic_upstream_connection_handle_network_change")};
 };
 
 using ConnectivityManagerSharedPtr = std::shared_ptr<ConnectivityManager>;

mobile/library/common/network/envoy_mobile_quic_network_observer_registry_factory.cc

@@ -0,0 +1,34 @@
+#include "library/common/network/envoy_mobile_quic_network_observer_registry_factory.h"
+
+#include "source/common/runtime/runtime_features.h"
+
+namespace Envoy {
+namespace Quic {
+
+void EnvoyMobileQuicNetworkObserverRegistry::onNetworkMadeDefault() {
+  ENVOY_LOG_MISC(trace, "Default network changed.");
+  ASSERT(dispatcher_.isThreadSafe());
+  ASSERT(Runtime::runtimeFeatureEnabled(
+      "envoy.reloadable_features.quic_upstream_connection_handle_network_change"));
+  // Retain the existing observers in a list and iterate on the list as new
+  // connections might be created and registered during iteration.
+  std::vector<QuicNetworkConnectivityObserver*> existing_observers;
+  existing_observers.reserve(registeredQuicObservers().size());
+  for (QuicNetworkConnectivityObserver* observer : registeredQuicObservers()) {
+    existing_observers.push_back(observer);
+  }
+  for (QuicNetworkConnectivityObserver* observer : existing_observers) {
+    observer->onNetworkChanged();
+  }
+}
+
+EnvoyQuicNetworkObserverRegistryPtr
+EnvoyMobileQuicNetworkObserverRegistryFactory::createQuicNetworkObserverRegistry(
+    Event::Dispatcher& dispatcher) {
+  auto result = std::make_unique<EnvoyMobileQuicNetworkObserverRegistry>(dispatcher);
+  thread_local_observer_registries_.emplace_back(*result);
+  return result;
+}
+
+} // namespace Quic
+} // namespace Envoy

mobile/library/common/network/envoy_mobile_quic_network_observer_registry_factory.h

@@ -0,0 +1,44 @@
+#pragma once
+
+#include "source/common/quic/envoy_quic_network_observer_registry_factory.h"
+#include "source/common/quic/quic_network_connectivity_observer.h"
+
+namespace Envoy {
+namespace Quic {
+
+// A mobile implementation that also handles network change events.
+class EnvoyMobileQuicNetworkObserverRegistry : public EnvoyQuicNetworkObserverRegistry {
+public:
+  explicit EnvoyMobileQuicNetworkObserverRegistry(Event::Dispatcher& dispatcher)
+      : dispatcher_(dispatcher) {}
+
+  // Called when the default network has changed to notify each registered observer.
+  void onNetworkMadeDefault();
+
+private:
+  Event::Dispatcher& dispatcher_;
+};
+
+class EnvoyMobileQuicNetworkObserverRegistryFactory
+    : public EnvoyQuicNetworkObserverRegistryFactory {
+public:
+  EnvoyMobileQuicNetworkObserverRegistryFactory() { thread_local_observer_registries_.reserve(1); }
+
+  EnvoyQuicNetworkObserverRegistryPtr
+  createQuicNetworkObserverRegistry(Event::Dispatcher& dispatcher) override;
+
+  std::vector<std::reference_wrapper<EnvoyMobileQuicNetworkObserverRegistry>>&
+  getCreatedObserverRegistries() {
+    return thread_local_observer_registries_;
+  }
+
+private:
+  std::vector<std::reference_wrapper<EnvoyMobileQuicNetworkObserverRegistry>>
+      thread_local_observer_registries_;
+};
+
+using EnvoyMobileQuicNetworkObserverRegistryPtr =
+    std::unique_ptr<EnvoyMobileQuicNetworkObserverRegistry>;
+
+} // namespace Quic
+} // namespace Envoy

mobile/test/common/extensions/filters/http/network_configuration/network_configuration_filter_test.cc

@@ -57,6 +57,7 @@ class MockConnectivityManager : public Network::ConnectivityManager {
                const Extensions::Common::DynamicForwardProxy::DnsHostInfoSharedPtr&,
                Network::DnsResolver::ResolutionStatus));
   MOCK_METHOD(Extensions::Common::DynamicForwardProxy::DnsCacheSharedPtr, dnsCache, ());
+  MOCK_METHOD(envoy_netconf_t, onNetworkMadeDefault, (NetworkType network));
 };
 
 class NetworkConfigurationFilterTest : public testing::Test {

mobile/test/common/integration/client_integration_test.cc

@@ -1334,15 +1334,20 @@ TEST_P(ClientIntegrationTest, TestProxyResolutionApi) {
 }
 #endif
 
-// This test is simply to test the IPv6 connectivity check and DNS refresh and make sure the code
-// doesn't crash. It doesn't really test the actual network change event.
+// Tests the IPv6 connectivity check and DNS refresh happen upon network change. And idle HTTP/3
+// connections get closed.
 TEST_P(ClientIntegrationTest, OnNetworkChanged) {
   builder_.addRuntimeGuard("dns_cache_set_ip_version_to_remove", true);
+  builder_.addRuntimeGuard("quic_upstream_connection_handle_network_change", true);
   initialize();
-  internalEngine()->onDefaultNetworkChanged(NetworkType::WLAN);
   basicTest();
+  internalEngine()->onDefaultNetworkChanged(NetworkType::WLAN);
   if (upstreamProtocol() == Http::CodecType::HTTP1) {
     ASSERT_EQ(cc_.on_complete_received_byte_count_, 67);
+  } else if (upstreamProtocol() == Http::CodecType::HTTP3) {
+    ASSERT_TRUE(waitForCounterGe("http3.upstream.tx.quic_connection_close_error_code_QUIC_"
+                                 "CONNECTION_MIGRATION_NO_MIGRATABLE_STREAMS",
+                                 1));
   }
 }
 

mobile/test/common/network/connectivity_manager_test.cc

@@ -1,6 +1,7 @@
 #include <net/if.h>
 
 #include "test/extensions/common/dynamic_forward_proxy/mocks.h"
+#include "test/mocks/event/mocks.h"
 #include "test/mocks/upstream/cluster_manager.h"
 
 #include "gtest/gtest.h"
@@ -48,6 +49,24 @@ TEST_F(ConnectivityManagerTest,
   EXPECT_EQ(original_key, connectivity_manager_->getConfigurationKey());
 }
 
+TEST_F(ConnectivityManagerTest, OnNetworkMadeDefault) {
+  Runtime::maybeSetRuntimeGuard(
+      "envoy.reloadable_features.quic_upstream_connection_handle_network_change", true);
+  Quic::EnvoyMobileQuicNetworkObserverRegistryPtr registry;
+  Event::MockDispatcher dispatcher;
+  EXPECT_CALL(cm_, createNetworkObserverRegistries(_))
+      .WillOnce(Invoke([&](Quic::EnvoyQuicNetworkObserverRegistryFactory& factory) {
+        registry.reset(static_cast<Quic::EnvoyMobileQuicNetworkObserverRegistry*>(
+            factory.createQuicNetworkObserverRegistry(dispatcher).release()));
+      }));
+  connectivity_manager_ = std::make_shared<ConnectivityManagerImpl>(cm_, dns_cache_manager_);
+  EXPECT_NE(nullptr, registry);
+  envoy_netconf_t original_key = connectivity_manager_->getConfigurationKey();
+  EXPECT_CALL(dispatcher, isThreadSafe());
+  envoy_netconf_t new_key = connectivity_manager_->onNetworkMadeDefault(NetworkType::WWAN);
+  EXPECT_NE(original_key, new_key);
+}
+
 TEST_F(ConnectivityManagerTest, RefreshDnsForCurrentConfigurationTriggersDnsRefresh) {
   EXPECT_CALL(*dns_cache_, forceRefreshHosts());
   envoy_netconf_t configuration_key = connectivity_manager_->getConfigurationKey();