grid: Plumb the AlternateProtocolCache down to the grid from the UpstreamClusterManager

api/envoy/config/core/v3/protocol.proto

@@ -71,6 +71,10 @@ message UpstreamHttpProtocolOptions {
   bool auto_san_validation = 2;
 }
 
+// Configures the alternate protocols cache which tracks alternate protocols that can be used to
+// make an HTTP connection to an origin server. See https://tools.ietf.org/html/rfc7838 for
+// HTTP Alternate Services and https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-svcb-https-04
+// for the "HTTPS" DNS resource record.
 message AlternateProtocolsCacheOptions {
   // The name of the cache. Multiple named caches allow independent alternate protocols cache
   // configurations to operate within a single Envoy process using different configurations. All
@@ -84,7 +88,7 @@ message AlternateProtocolsCacheOptions {
   // .. note:
   //
   //   The implementation is approximate and enforced independently on each worker thread, thus
-  //   it is possible for the maximum hosts in the cache to go slightly above the configured
+  //   it is possible for the maximum entries in the cache to go slightly above the configured
   //   value depending on timing. This is similar to how other circuit breakers work.
   google.protobuf.UInt32Value max_entries = 2 [(validate.rules).uint32 = {gt: 0}];
 }

source/common/http/alternate_protocols_cache_impl.cc

@@ -3,20 +3,15 @@
 namespace Envoy {
 namespace Http {
 
-AlternateProtocolsCacheImpl::AlternateProtocolsCacheImpl(ThreadLocal::Instance& tls,
-                                                         TimeSource& time_source)
-    : time_source_(time_source), slot_(tls) {
-  slot_.set([](Event::Dispatcher& /*dispatcher*/) { return std::make_shared<State>(); });
-}
+AlternateProtocolsCacheImpl::AlternateProtocolsCacheImpl(TimeSource& time_source)
+    : time_source_(time_source) {}
 
 AlternateProtocolsCacheImpl::~AlternateProtocolsCacheImpl() = default;
 
 void AlternateProtocolsCacheImpl::setAlternatives(const Origin& origin,
                                                   const std::vector<AlternateProtocol>& protocols,
                                                   const MonotonicTime& expiration) {
-  // TODO(RyanTheOptimist): Propagate state changes across threads.
-  State& state = *(slot_.get());
-  Entry& entry = state.protocols_[origin];
+  Entry& entry = protocols_[origin];
   if (entry.protocols_ != protocols) {
     entry.protocols_ = protocols;
   }
@@ -27,26 +22,24 @@ void AlternateProtocolsCacheImpl::setAlternatives(const Origin& origin,
 
 OptRef<const std::vector<AlternateProtocolsCache::AlternateProtocol>>
 AlternateProtocolsCacheImpl::findAlternatives(const Origin& origin) {
-  State& state = *(slot_.get());
-  auto entry_it = state.protocols_.find(origin);
-  if (entry_it == state.protocols_.end()) {
-    return makeOptRefFromPtr<const std::vector<AlternateProtocol>>(nullptr);
+  auto entry_it = protocols_.find(origin);
+  if (entry_it == protocols_.end()) {
+    return makeOptRefFromPtr<const std::vector<AlternateProtocolsCache::AlternateProtocol>>(
+        nullptr);
   }
 
   const Entry& entry = entry_it->second;
   if (time_source_.monotonicTime() > entry.expiration_) {
-    // TODO(RyanTheOptimist): expire entries based on a timer.
     // Expire the entry.
-    state.protocols_.erase(entry_it);
-    return makeOptRefFromPtr<const std::vector<AlternateProtocol>>(nullptr);
+    // TODO(RyanTheOptimist): expire entries based on a timer.
+    protocols_.erase(entry_it);
+    return makeOptRefFromPtr<const std::vector<AlternateProtocolsCache::AlternateProtocol>>(
+        nullptr);
   }
   return makeOptRef(entry.protocols_);
 }
 
-size_t AlternateProtocolsCacheImpl::size() const {
-  State& state = *(slot_.get());
-  return state.protocols_.size();
-}
+size_t AlternateProtocolsCacheImpl::size() const { return protocols_.size(); }
 
 } // namespace Http
 } // namespace Envoy

source/common/http/alternate_protocols_cache_impl.h

@@ -8,16 +8,17 @@
 #include "envoy/common/optref.h"
 #include "envoy/common/time.h"
 #include "envoy/http/alternate_protocols_cache.h"
-#include "envoy/thread_local/thread_local.h"
 
 #include "absl/strings/string_view.h"
 
 namespace Envoy {
 namespace Http {
 
+// An implementation of AlternateProtocolsCache.
+// See: source/docs/http3_upstream.md
 class AlternateProtocolsCacheImpl : public AlternateProtocolsCache {
 public:
-  AlternateProtocolsCacheImpl(ThreadLocal::Instance& tls, TimeSource& time_source);
+  explicit AlternateProtocolsCacheImpl(TimeSource& time_source);
   ~AlternateProtocolsCacheImpl() override;
 
   // AlternateProtocolsCache
@@ -32,16 +33,12 @@ class AlternateProtocolsCacheImpl : public AlternateProtocolsCache {
     MonotonicTime expiration_;
   };
 
-  struct State : ThreadLocal::ThreadLocalObject {
-    // Map from hostname to list of alternate protocols.
-    // TODO(RyanTheOptimist): Add a limit to the size of this map and evict based on usage.
-    std::map<Origin, Entry> protocols_;
-  };
-
   // Time source used to check expiration of entries.
   TimeSource& time_source_;
-  // Thread local state for the cache
-  ThreadLocal::TypedSlot<State> slot_;
+
+  // Map from hostname to list of alternate protocols.
+  // TODO(RyanTheOptimist): Add a limit to the size of this map and evict based on usage.
+  std::map<Origin, Entry> protocols_;
 };
 
 } // namespace Http

source/common/http/alternate_protocols_cache_manager_impl.cc

@@ -10,23 +10,29 @@ namespace Http {
 
 SINGLETON_MANAGER_REGISTRATION(alternate_protocols_cache_manager);
 
+AlternateProtocolsCacheManagerImpl::AlternateProtocolsCacheManagerImpl(TimeSource& time_source,
+                                                                       ThreadLocal::Instance& tls)
+    : time_source_(time_source), slot_(tls) {
+  slot_.set([](Event::Dispatcher& /*dispatcher*/) { return std::make_shared<State>(); });
+}
+
 AlternateProtocolsCacheSharedPtr AlternateProtocolsCacheManagerImpl::getCache(
     const envoy::config::core::v3::AlternateProtocolsCacheOptions& options) {
-  const auto& existing_cache = caches_.find(options.name());
-  if (existing_cache != caches_.end()) {
+  const auto& existing_cache = (*slot_).caches_.find(options.name());
+  if (existing_cache != (*slot_).caches_.end()) {
     if (!Protobuf::util::MessageDifferencer::Equivalent(options, existing_cache->second.options_)) {
       throw EnvoyException(fmt::format(
           "options specified alternate protocols cache '{}' with different settings"
-          " old '{}' new '{}'",
+          " first '{}' second '{}'",
           options.name(), existing_cache->second.options_.DebugString(), options.DebugString()));
     }
 
     return existing_cache->second.cache_;
   }
 
   AlternateProtocolsCacheSharedPtr new_cache =
-      std::make_shared<AlternateProtocolsCacheImpl>(tls_, time_source_);
-  caches_.emplace(options.name(), CacheWithOptions{options, new_cache});
+      std::make_shared<AlternateProtocolsCacheImpl>(time_source_);
+  (*slot_).caches_.emplace(options.name(), CacheWithOptions{options, new_cache});
   return new_cache;
 }
 

source/common/http/alternate_protocols_cache_manager_impl.h

@@ -14,8 +14,7 @@ namespace Http {
 class AlternateProtocolsCacheManagerImpl : public AlternateProtocolsCacheManager,
                                            public Singleton::Instance {
 public:
-  AlternateProtocolsCacheManagerImpl(TimeSource& time_source, ThreadLocal::Instance& tls)
-      : time_source_(time_source), tls_(tls) {}
+  AlternateProtocolsCacheManagerImpl(TimeSource& time_source, ThreadLocal::Instance& tls);
 
   // AlternateProtocolsCacheManager
   AlternateProtocolsCacheSharedPtr
@@ -32,11 +31,16 @@ class AlternateProtocolsCacheManagerImpl : public AlternateProtocolsCacheManager
     AlternateProtocolsCacheSharedPtr cache_;
   };
 
+  // Per-thread state.
+  struct State : public ThreadLocal::ThreadLocalObject {
+    // Map from config name to cache for that config.
+    absl::flat_hash_map<std::string, CacheWithOptions> caches_;
+  };
+
   TimeSource& time_source_;
-  ThreadLocal::Instance& tls_;
 
-  // Map from config name to cache for that config.
-  absl::flat_hash_map<std::string, CacheWithOptions> caches_;
+  // Thread local state for the cache.
+  ThreadLocal::TypedSlot<State> slot_;
 };
 
 class AlternateProtocolsCacheManagerFactoryImpl : public AlternateProtocolsCacheManagerFactory {

source/common/upstream/BUILD

@@ -56,7 +56,6 @@ envoy_cc_library(
         ":subset_lb_lib",
         "//include/envoy/api:api_interface",
         "//include/envoy/event:dispatcher_interface",
-        "@envoy_api//envoy/extensions/clusters/dynamic_forward_proxy/v3:pkg_cc_proto",
         "//include/envoy/http:codes_interface",
         "//include/envoy/local_info:local_info_interface",
         "//include/envoy/network:dns_interface",

source/docs/http3_upstream.md

@@ -1,7 +1,17 @@
 ### Overview
 
 Support for Upstream HTTP/3 connections is slightly more complex than for HTTP/1 or HTTP/2
-and this document attempts to describe it.
+and requires specific configurations to enable it. HTTP/3 is only attempted to servers which
+advertise HTTP/3 support either via [HTTP Alternative Services](https://tools.ietf.org/html/rfc7838)
+or the [HTTPS DNS resource record](https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-svcb-https-04).
+If no such advertisement exists, then HTTP/2 or HTTP/1 will be used instead. Further,
+HTTP/3 runs over QUIC (which uses UDP) and not over TCP (which HTTP/1 and HTTP/2 use).
+It is not uncommon for network devices to block UDP traffic, and hence block HTTP/3. This
+means that Upstream HTTP/3 connection attempts might be blocked by the network and should fall
+back to using HTTP/2. On such networks, the Upstream connection code needs to
+track that HTTP/3 connects attempts are not succeeding and avoid making connections
+which are doomed to fail. On networks where HTTP/3 is working correctly, however, the
+Upstream connection code should avoid attempting HTTP/2 unnecessarily.
 
 ### Components
 
@@ -29,7 +39,7 @@ sent to the mixed pool, and whichever succeeds first will be passed back up to t
 
 ### Required configuration
 
-Specific Envoy configuration is required in order to enable the previous. components
+Specific Envoy configuration is required in order to enable the previously described components.
 
 #### Auto Cluster Pool
 

test/common/http/BUILD

@@ -433,7 +433,6 @@ envoy_cc_test(
         "//test/mocks/network:connection_mocks",
         "//test/mocks/router:router_mocks",
         "//test/mocks/runtime:runtime_mocks",
-        "//test/mocks/thread_local:thread_local_mocks",
         "//test/mocks/stats:stats_mocks",
         "//source/common/quic:quic_factory_lib",
         "//source/common/quic:quic_transport_socket_factory_lib",
@@ -458,7 +457,6 @@ envoy_cc_test(
         ":common_lib",
         "//source/common/http:alternate_protocols_cache",
         "//test/mocks:common_lib",
-        "//test/mocks/thread_local:thread_local_mocks",
         "//test/test_common:simulated_time_system_lib",
     ],
 )

test/common/http/alternate_protocols_cache_impl_test.cc

@@ -1,6 +1,5 @@
 #include "common/http/alternate_protocols_cache_impl.h"
 
-#include "test/mocks/thread_local/mocks.h"
 #include "test/test_common/simulated_time_system.h"
 
 #include "gtest/gtest.h"
@@ -11,9 +10,7 @@ namespace Http {
 namespace {
 class AlternateProtocolsCacheImplTest : public testing::Test, public Event::TestUsingSimulatedTime {
 public:
-  AlternateProtocolsCacheImplTest() : protocols_(tls_, simTime()) {}
-
-  testing::NiceMock<ThreadLocal::MockInstance> tls_;
+  AlternateProtocolsCacheImplTest() : protocols_(simTime()) {}
 
   AlternateProtocolsCacheImpl protocols_;
   const std::string hostname1_ = "hostname1";

test/common/http/alternate_protocols_cache_manager_test.cc

@@ -56,14 +56,9 @@ TEST_F(AlternateProtocolsCacheManagerTest, GetCacheForDifferentOptions) {
 TEST_F(AlternateProtocolsCacheManagerTest, GetCacheForConflictingOptions) {
   AlternateProtocolsCacheSharedPtr cache1 = manager_->getCache(options1_);
   options2_.set_name(options1_.name());
-  try {
-    AlternateProtocolsCacheSharedPtr cache2 = manager_->getCache(options2_);
-    FAIL();
-  } catch (const EnvoyException& e) {
-    EXPECT_THAT(e.what(),
-                testing::HasSubstr(
-                    "options specified alternate protocols cache 'name1' with different settings"));
-  }
+  EXPECT_THROW_WITH_REGEX(
+      manager_->getCache(options2_), EnvoyException,
+      "options specified alternate protocols cache 'name1' with different settings.*");
 }
 
 } // namespace

test/common/http/conn_pool_grid_test.cc

@@ -12,7 +12,6 @@
 #include "test/mocks/http/stream_encoder.h"
 #include "test/mocks/network/connection.h"
 #include "test/mocks/ssl/mocks.h"
-#include "test/mocks/thread_local/mocks.h"
 #include "test/mocks/upstream/cluster_info.h"
 #include "test/test_common/simulated_time_system.h"
 #include "test/test_common/utility.h"
@@ -117,7 +116,7 @@ class ConnectivityGridTestBase : public Event::TestUsingSimulatedTime, public te
     if (!use_alternate_protocols) {
       return nullptr;
     }
-    return std::make_shared<AlternateProtocolsCacheImpl>(tls_, simTime());
+    return std::make_shared<AlternateProtocolsCacheImpl>(simTime());
   }
 
   void addHttp3AlternateProtocol() {
@@ -135,7 +134,6 @@ class ConnectivityGridTestBase : public Event::TestUsingSimulatedTime, public te
   NiceMock<Event::MockDispatcher> dispatcher_;
   std::shared_ptr<Upstream::MockClusterInfo> cluster_{new NiceMock<Upstream::MockClusterInfo>()};
   NiceMock<Random::MockRandomGenerator> random_;
-  NiceMock<ThreadLocal::MockInstance> tls_;
   AlternateProtocolsCacheSharedPtr alternate_protocols_;
   ConnectivityGridForTest grid_;
   Upstream::HostDescriptionConstSharedPtr host_;