quic: change QuicNetworkConnectivityObserver interface

source/common/quic/BUILD

@@ -310,13 +310,12 @@ envoy_cc_library(
     srcs = envoy_select_enable_http3([
         "envoy_quic_client_session.cc",
         "envoy_quic_client_stream.cc",
-        "quic_network_connectivity_observer.cc",
+        "quic_network_connectivity_observer_impl.cc",
     ]),
     hdrs = envoy_select_enable_http3([
         "envoy_quic_client_session.h",
         "envoy_quic_client_stream.h",
-        "envoy_quic_network_observer_registry_factory.h",
-        "quic_network_connectivity_observer.h",
+        "quic_network_connectivity_observer_impl.h",
     ]),
     deps = envoy_select_enable_http3([
         ":envoy_quic_client_connection_lib",
@@ -325,6 +324,8 @@ envoy_cc_library(
         ":envoy_quic_stream_lib",
         ":envoy_quic_utils_lib",
         ":quic_filter_manager_connection_lib",
+        ":quic_network_connectivity_observer_interface",
+        ":envoy_quic_network_observer_registry_factory_lib",
         ":quic_stat_names_lib",
         ":quic_transport_socket_factory_lib",
         "//envoy/http:http_server_properties_cache_interface",
@@ -340,15 +341,26 @@ envoy_cc_library(
 )
 
 envoy_cc_library(
-    name = "envoy_quic_network_observer_registry_factory_lib",
+    name = "quic_network_connectivity_observer_interface",
     hdrs = [
-        "envoy_quic_network_observer_registry_factory.h",
+        "quic_network_connectivity_observer.h",
     ],
     deps = envoy_select_enable_http3([
-        ":envoy_quic_client_session_lib",
+        "//source/common/common:logger_lib",
     ]),
 )
 
+envoy_cc_library(
+    name = "envoy_quic_network_observer_registry_factory_lib",
+    hdrs = [
+        "envoy_quic_network_observer_registry_factory.h",
+    ],
+    deps = [
+        ":quic_network_connectivity_observer_interface",
+        "//envoy/event:dispatcher_interface",
+    ],
+)
+
 envoy_cc_library(
     name = "quic_io_handle_wrapper_lib",
     hdrs = envoy_select_enable_http3(["quic_io_handle_wrapper.h"]),

source/common/quic/envoy_quic_client_session.cc

@@ -10,6 +10,7 @@
 #include "source/common/quic/envoy_quic_proof_verifier.h"
 #include "source/common/quic/envoy_quic_utils.h"
 #include "source/common/quic/quic_filter_manager_connection_impl.h"
+#include "source/common/quic/quic_network_connectivity_observer_impl.h"
 
 namespace Envoy {
 namespace Quic {
@@ -311,7 +312,7 @@ std::vector<std::string> EnvoyQuicClientSession::GetAlpnsToOffer() const {
 
 void EnvoyQuicClientSession::registerNetworkObserver(EnvoyQuicNetworkObserverRegistry& registry) {
   if (network_connectivity_observer_ == nullptr) {
-    network_connectivity_observer_ = std::make_unique<QuicNetworkConnectivityObserver>(*this);
+    network_connectivity_observer_ = std::make_unique<QuicNetworkConnectivityObserverImpl>(*this);
   }
   registry.registerObserver(*network_connectivity_observer_);
   registry_ = makeOptRef(registry);

source/common/quic/envoy_quic_network_observer_registry_factory.h

@@ -2,16 +2,13 @@
 
 #include <memory>
 
-#ifdef ENVOY_ENABLE_QUIC
 #include "envoy/event/dispatcher.h"
 
 #include "source/common/quic/quic_network_connectivity_observer.h"
-#endif
 
 namespace Envoy {
 namespace Quic {
 
-#ifdef ENVOY_ENABLE_QUIC
 // A registry of network connectivity observers.
 class EnvoyQuicNetworkObserverRegistry {
 public:
@@ -44,14 +41,6 @@ class EnvoyQuicNetworkObserverRegistryFactory {
   }
 };
 
-#else
-
-// Dumb definitions of QUIC classes if QUIC is compiled out.
-class EnvoyQuicNetworkObserverRegistry {};
-class EnvoyQuicNetworkObserverRegistryFactory {};
-
-#endif
-
 using EnvoyQuicNetworkObserverRegistryPtr = std::unique_ptr<EnvoyQuicNetworkObserverRegistry>;
 using EnvoyQuicNetworkObserverRegistryFactoryPtr =
     std::unique_ptr<EnvoyQuicNetworkObserverRegistryFactory>;

source/common/quic/quic_network_connectivity_observer.h

@@ -2,29 +2,25 @@
 
 #include <memory>
 
-#include "source/common/common/logger.h"
-
 namespace Envoy {
-namespace Quic {
 
-class EnvoyQuicClientSession;
+using NetworkHandle = int64_t;
 
-// TODO(danzh) deprecate this class once QUICHE has its own more detailed network observer.
-class QuicNetworkConnectivityObserver : protected Logger::Loggable<Logger::Id::connection> {
+namespace Quic {
+
+// An interface to get network change notifications from the underlying platform.
+class QuicNetworkConnectivityObserver {
 public:
-  // session must outlive this object.
-  explicit QuicNetworkConnectivityObserver(EnvoyQuicClientSession& session);
-  QuicNetworkConnectivityObserver(const QuicNetworkConnectivityObserver&) = delete;
-  QuicNetworkConnectivityObserver& operator=(const QuicNetworkConnectivityObserver&) = delete;
+  virtual ~QuicNetworkConnectivityObserver() = default;
 
   // Called when the device switches to a different network.
-  void onNetworkChanged() {
-    // TODO(danzh) close the connection if it's idle, otherwise mark it as go away.
-    (void)session_;
-  }
+  virtual void onNetworkMadeDefault(NetworkHandle network) PURE;
+
+  // Called when a new network is connected.
+  virtual void onNetworkConnected(NetworkHandle network) PURE;
 
-private:
-  EnvoyQuicClientSession& session_;
+  // Called when the given network gets disconnected.
+  virtual void onNetworkDisconnected(NetworkHandle network) PURE;
 };
 
 using QuicNetworkConnectivityObserverPtr = std::unique_ptr<QuicNetworkConnectivityObserver>;

source/common/quic/quic_network_connectivity_observer_impl.cc

@@ -0,0 +1,26 @@
+#include "source/common/quic/quic_network_connectivity_observer_impl.h"
+
+#include "source/common/quic/envoy_quic_client_session.h"
+
+namespace Envoy {
+namespace Quic {
+
+QuicNetworkConnectivityObserverImpl::QuicNetworkConnectivityObserverImpl(
+    EnvoyQuicClientSession& session)
+    : session_(session) {}
+
+void QuicNetworkConnectivityObserverImpl::onNetworkMadeDefault(NetworkHandle /*network*/) {
+  // TODO(danzh) propagate `network` to quic session.
+  (void)session_;
+}
+
+void QuicNetworkConnectivityObserverImpl::onNetworkConnected(NetworkHandle /*network*/) {
+  (void)session_;
+}
+
+void QuicNetworkConnectivityObserverImpl::onNetworkDisconnected(NetworkHandle /*network*/) {
+  (void)session_;
+}
+
+} // namespace Quic
+} // namespace Envoy

source/common/quic/quic_network_connectivity_observer_impl.h

@@ -0,0 +1,32 @@
+#pragma once
+
+#include <memory>
+
+#include "source/common/common/logger.h"
+#include "source/common/quic/quic_network_connectivity_observer.h"
+
+namespace Envoy {
+namespace Quic {
+
+class EnvoyQuicClientSession;
+
+class QuicNetworkConnectivityObserverImpl : public QuicNetworkConnectivityObserver,
+                                            protected Logger::Loggable<Logger::Id::connection> {
+public:
+  // session must outlive this object.
+  explicit QuicNetworkConnectivityObserverImpl(EnvoyQuicClientSession& session);
+  QuicNetworkConnectivityObserverImpl(const QuicNetworkConnectivityObserverImpl&) = delete;
+  QuicNetworkConnectivityObserverImpl&
+  operator=(const QuicNetworkConnectivityObserverImpl&) = delete;
+
+  // QuicNetworkConnectivityObserver
+  void onNetworkMadeDefault(NetworkHandle network) override;
+  void onNetworkConnected(NetworkHandle network) override;
+  void onNetworkDisconnected(NetworkHandle network) override;
+
+private:
+  EnvoyQuicClientSession& session_;
+};
+
+} // namespace Quic
+} // namespace Envoy

test/common/http/http3/conn_pool_test.cc

@@ -292,6 +292,14 @@ TEST_F(Http3ConnPoolImplTest, MigrationEnabledNoDrain) {
   EXPECT_FALSE(pool_->isIdle());
 }
 
+// These are no-op currently. Just test them to have test coverage.
+TEST_F(Http3ConnPoolImplTest, GetNetworkChangeEvents) {
+  createNewStream();
+  observers_.onNetworkConnected(-1);
+  observers_.onNetworkMadeDefault(-1);
+  observers_.onNetworkDisconnected(-1);
+}
+
 } // namespace Http3
 } // namespace Http
 } // namespace Envoy

test/common/quic/test_utils.h

@@ -378,15 +378,36 @@ REGISTER_FACTORY(TestEnvoyQuicConnectionDebugVisitorFactoryFactory,
 
 class TestNetworkObserverRegistry : public Quic::EnvoyQuicNetworkObserverRegistry {
 public:
-  void onNetworkChanged() {
+  void onNetworkMadeDefault(NetworkHandle network) {
     std::list<Quic::QuicNetworkConnectivityObserver*> existing_observers;
     for (Quic::QuicNetworkConnectivityObserver* observer : registeredQuicObservers()) {
       existing_observers.push_back(observer);
     }
     for (auto* observer : existing_observers) {
-      observer->onNetworkChanged();
+      observer->onNetworkMadeDefault(network);
     }
   }
+
+  void onNetworkDisconnected(NetworkHandle network) {
+    std::list<Quic::QuicNetworkConnectivityObserver*> existing_observers;
+    for (Quic::QuicNetworkConnectivityObserver* observer : registeredQuicObservers()) {
+      existing_observers.push_back(observer);
+    }
+    for (auto* observer : existing_observers) {
+      observer->onNetworkDisconnected(network);
+    }
+  }
+
+  void onNetworkConnected(NetworkHandle network) {
+    std::list<Quic::QuicNetworkConnectivityObserver*> existing_observers;
+    for (Quic::QuicNetworkConnectivityObserver* observer : registeredQuicObservers()) {
+      existing_observers.push_back(observer);
+    }
+    for (auto* observer : existing_observers) {
+      observer->onNetworkConnected(network);
+    }
+  }
+
   using Quic::EnvoyQuicNetworkObserverRegistry::registeredQuicObservers;
 };