Merge branch 'master' into fix/ext_authz_dns_san

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

api/envoy/api/v2/listener/listener.proto

@@ -188,6 +188,11 @@ message FilterChain {
 
   // See :ref:`base.TransportSocket<envoy_api_msg_core.TransportSocket>` description.
   core.TransportSocket transport_socket = 6;
+
+  // [#not-implemented-hide:] The unique name (or empty) by which this filter chain is known. If no
+  // name is provided, Envoy will allocate an internal UUID for the filter chain. If the filter
+  // chain is to be dynamically updated or removed via FCDS a unique name must be provided.
+  string name = 7;
 }
 
 message ListenerFilter {

docs/root/_static/css/envoy.css

@@ -1,4 +1,9 @@
-@import "theme.css";
+@import url("theme.css");
+
+/*Changing content max-width 100% ( 800px is default )*/
+.wy-nav-content {
+  max-width: 100% !important;
+}
 
 /* Splits a long line descriptions in tables in to multiple lines */
 .wy-table-responsive table td, .wy-table-responsive table th {
@@ -8,4 +13,4 @@
 /* align multi line csv table columns */
 table.docutils div.line-block {
     margin-left: 0;
-}
+}

docs/root/intro/version_history.rst

@@ -28,6 +28,7 @@ Version history
 * http: added the ability to :ref:`merge adjacent slashes<envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.merge_slashes>` in the path.
 * listeners: added :ref:`continue_on_listener_filters_timeout <envoy_api_field_Listener.continue_on_listener_filters_timeout>` to configure whether a listener will still create a connection when listener filters time out.
 * listeners: added :ref:`HTTP inspector listener filter <config_listener_filters_http_inspector>`.
+* performance: stats symbol table implementation (disabled by default; to test it, add "--use-fake-symbol-table 0" to the command-line arguments when starting Envoy).
 * redis: added :ref:`read_policy <envoy_api_field_config.filter.network.redis_proxy.v2.RedisProxy.ConnPoolSettings.read_policy>` to allow reading from redis replicas for Redis Cluster deployments.
 * rbac: added support for DNS SAN as :ref:`principal_name <envoy_api_field_config.rbac.v2.Principal.Authenticated.principal_name>`.
 * lua: extended `httpCall()` and `respond()` APIs to accept headers with entry values that can be a string or table of strings.

include/envoy/router/rds.h

@@ -48,6 +48,11 @@ class RouteConfigProvider {
    * Callback used to notify RouteConfigProvider about configuration changes.
    */
   virtual void onConfigUpdate() PURE;
+
+  /**
+   * Validate if the route configuration can be applied to the context of the route config provider.
+   */
+  virtual void validateConfig(const envoy::api::v2::RouteConfiguration& config) const PURE;
 };
 
 using RouteConfigProviderPtr = std::unique_ptr<RouteConfigProvider>;

include/envoy/router/route_config_update_receiver.h

@@ -28,6 +28,7 @@ class RouteConfigUpdateReceiver {
    */
   virtual bool onRdsUpdate(const envoy::api::v2::RouteConfiguration& rc,
                            const std::string& version_info) PURE;
+
   /**
    * Called on updates via VHDS.
    * @param added_resources supplies Resources (each containing a VirtualHost) that have been

include/envoy/server/options.h

@@ -184,6 +184,11 @@ class Options {
    */
   virtual bool libeventBufferEnabled() const PURE;
 
+  /**
+   * @return whether to use the fake symbol table implementation.
+   */
+  virtual bool fakeSymbolTableEnabled() const PURE;
+
   /**
    * @return bool indicating whether cpuset size should determine the number of worker threads.
    */

include/envoy/stats/symbol_table.h

@@ -185,7 +185,7 @@ class SymbolTable {
   virtual StoragePtr encode(absl::string_view name) PURE;
 };
 
-using SharedSymbolTable = std::shared_ptr<SymbolTable>;
+using SymbolTablePtr = std::unique_ptr<SymbolTable>;
 
 } // namespace Stats
 } // namespace Envoy

source/common/protobuf/message_validator_impl.cc

@@ -4,7 +4,6 @@
 
 #include "common/common/assert.h"
 #include "common/common/hash.h"
-#include "common/common/logger.h"
 #include "common/common/macros.h"
 
 #include "absl/strings/str_cat.h"

source/common/protobuf/message_validator_impl.h

@@ -3,6 +3,8 @@
 #include "envoy/protobuf/message_validator.h"
 #include "envoy/stats/stats.h"
 
+#include "common/common/logger.h"
+
 #include "absl/container/flat_hash_set.h"
 
 namespace Envoy {

source/common/protobuf/utility.cc

@@ -88,6 +88,21 @@ ProtoValidationException::ProtoValidationException(const std::string& validation
   ENVOY_LOG_MISC(debug, "Proto validation error; throwing {}", what());
 }
 
+void MessageUtil::checkUnknownFields(const Protobuf::Message& message,
+                                     ProtobufMessage::ValidationVisitor& validation_visitor) {
+  const auto& unknown_fields = message.GetReflection()->GetUnknownFields(message);
+  // If there are no unknown fields, we're done here.
+  if (unknown_fields.empty()) {
+    return;
+  }
+  std::string error_msg;
+  for (int n = 0; n < unknown_fields.field_count(); ++n) {
+    error_msg += absl::StrCat(n > 0 ? ", " : "", unknown_fields.field(n).number());
+  }
+  validation_visitor.onUnknownField("type " + message.GetTypeName() + " with unknown field set {" +
+                                    error_msg + "}");
+}
+
 void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& message,
                                ProtobufMessage::ValidationVisitor& validation_visitor) {
   Protobuf::util::JsonParseOptions options;

source/common/protobuf/utility.h

@@ -207,11 +207,7 @@ class MessageUtil {
   }
 
   static void checkUnknownFields(const Protobuf::Message& message,
-                                 ProtobufMessage::ValidationVisitor& validation_visitor) {
-    if (!message.GetReflection()->GetUnknownFields(message).empty()) {
-      validation_visitor.onUnknownField("type " + message.GetTypeName());
-    }
-  }
+                                 ProtobufMessage::ValidationVisitor& validation_visitor);
 
   static void loadFromJson(const std::string& json, Protobuf::Message& message,
                            ProtobufMessage::ValidationVisitor& validation_visitor);

source/common/router/rds_impl.cc

@@ -99,6 +99,11 @@ void RdsRouteConfigSubscription::onConfigUpdate(
     throw EnvoyException(fmt::format("Unexpected RDS configuration (expecting {}): {}",
                                      route_config_name_, route_config.name()));
   }
+  for (auto* provider : route_config_providers_) {
+    // This seems inefficient, though it is necessary to validate config in each context,
+    // especially when it comes with per_filter_config,
+    provider->validateConfig(route_config);
+  }
 
   if (config_update_info_->onRdsUpdate(route_config, version_info)) {
     stats_.config_reload_.inc();
@@ -198,6 +203,12 @@ void RdsRouteConfigProviderImpl::onConfigUpdate() {
       [this, new_config]() -> void { tls_->getTyped<ThreadLocalConfig>().config_ = new_config; });
 }
 
+void RdsRouteConfigProviderImpl::validateConfig(
+    const envoy::api::v2::RouteConfiguration& config) const {
+  // TODO(lizan): consider cache the config here until onConfigUpdate.
+  ConfigImpl validation_config(config, factory_context_, false);
+}
+
 RouteConfigProviderManagerImpl::RouteConfigProviderManagerImpl(Server::Admin& admin) {
   config_tracker_entry_ =
       admin.getConfigTracker().add("routes", [this] { return dumpRouteConfigs(); });

source/common/router/rds_impl.h

@@ -66,6 +66,7 @@ class StaticRouteConfigProviderImpl : public RouteConfigProvider {
   }
   SystemTime lastUpdated() const override { return last_updated_; }
   void onConfigUpdate() override {}
+  void validateConfig(const envoy::api::v2::RouteConfiguration&) const override {}
 
 private:
   ConfigConstSharedPtr config_;
@@ -159,14 +160,15 @@ class RdsRouteConfigProviderImpl : public RouteConfigProvider,
   ~RdsRouteConfigProviderImpl() override;
 
   RdsRouteConfigSubscription& subscription() { return *subscription_; }
-  void onConfigUpdate() override;
 
   // Router::RouteConfigProvider
   Router::ConfigConstSharedPtr config() override;
   absl::optional<ConfigInfo> configInfo() const override {
     return config_update_info_->configInfo();
   }
   SystemTime lastUpdated() const override { return config_update_info_->lastUpdated(); }
+  void onConfigUpdate() override;
+  void validateConfig(const envoy::api::v2::RouteConfiguration& config) const override;
 
 private:
   struct ThreadLocalConfig : public ThreadLocal::ThreadLocalObject {

source/common/stats/BUILD

@@ -50,6 +50,7 @@ envoy_cc_library(
         ":scope_prefixer_lib",
         ":stats_lib",
         ":store_impl_lib",
+        ":symbol_table_creator_lib",
         "//include/envoy/stats:stats_macros",
         "//source/common/stats:allocator_lib",
     ],
@@ -155,6 +156,17 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "symbol_table_creator_lib",
+    srcs = ["symbol_table_creator.cc"],
+    hdrs = ["symbol_table_creator.h"],
+    external_deps = ["abseil_base"],
+    deps = [
+        ":fake_symbol_table_lib",
+        ":symbol_table_lib",
+    ],
+)
+
 envoy_cc_library(
     name = "fake_symbol_table_lib",
     hdrs = ["fake_symbol_table_impl.h"],

source/common/stats/isolated_store_impl.cc

@@ -8,13 +8,13 @@
 #include "common/stats/fake_symbol_table_impl.h"
 #include "common/stats/histogram_impl.h"
 #include "common/stats/scope_prefixer.h"
+#include "common/stats/symbol_table_creator.h"
 #include "common/stats/utility.h"
 
 namespace Envoy {
 namespace Stats {
 
-IsolatedStoreImpl::IsolatedStoreImpl()
-    : IsolatedStoreImpl(std::make_unique<FakeSymbolTableImpl>()) {}
+IsolatedStoreImpl::IsolatedStoreImpl() : IsolatedStoreImpl(SymbolTableCreator::makeSymbolTable()) {}
 
 IsolatedStoreImpl::IsolatedStoreImpl(std::unique_ptr<SymbolTable>&& symbol_table)
     : IsolatedStoreImpl(*symbol_table) {

source/common/stats/isolated_store_impl.h

@@ -133,7 +133,7 @@ class IsolatedStoreImpl : public StoreImpl {
 private:
   IsolatedStoreImpl(std::unique_ptr<SymbolTable>&& symbol_table);
 
-  std::unique_ptr<SymbolTable> symbol_table_storage_;
+  SymbolTablePtr symbol_table_storage_;
   AllocatorImpl alloc_;
   IsolatedStatsCache<Counter> counters_;
   IsolatedStatsCache<Gauge> gauges_;

source/common/stats/symbol_table_creator.cc

@@ -0,0 +1,24 @@
+#include "common/stats/symbol_table_creator.h"
+
+namespace Envoy {
+namespace Stats {
+
+bool SymbolTableCreator::initialized_ = false;
+bool SymbolTableCreator::use_fake_symbol_tables_ = true;
+
+SymbolTablePtr SymbolTableCreator::initAndMakeSymbolTable(bool use_fake) {
+  ASSERT(!initialized_ || (use_fake_symbol_tables_ == use_fake));
+  use_fake_symbol_tables_ = use_fake;
+  return makeSymbolTable();
+}
+
+SymbolTablePtr SymbolTableCreator::makeSymbolTable() {
+  initialized_ = true;
+  if (use_fake_symbol_tables_) {
+    return std::make_unique<FakeSymbolTableImpl>();
+  }
+  return std::make_unique<SymbolTableImpl>();
+}
+
+} // namespace Stats
+} // namespace Envoy

source/common/stats/symbol_table_creator.h

@@ -0,0 +1,57 @@
+#pragma once
+
+#include "common/stats/fake_symbol_table_impl.h"
+#include "common/stats/symbol_table_impl.h"
+
+namespace Envoy {
+namespace Stats {
+
+namespace TestUtil {
+class SymbolTableCreatorTestPeer;
+}
+
+class SymbolTableCreator {
+public:
+  /**
+   * Initializes the symbol-table creation system. Once this is called, it is a
+   * runtime assertion to call this again in production code, changing the
+   * use_fakes setting. However, tests can change the setting via
+   * TestUtil::SymbolTableCreatorTestPeer::setUseFakeSymbolTables(use_fakes).
+   *
+   * @param use_fakes Whether to use fake symbol tables; typically from a command-line option.
+   * @return a SymbolTable.
+   */
+  static SymbolTablePtr initAndMakeSymbolTable(bool use_fakes);
+
+  /**
+   * Factory method to create SymbolTables. This is needed to help make it
+   * possible to flag-flip use of real symbol tables, and ultimately should be
+   * removed.
+   *
+   * @return a SymbolTable.
+   */
+  static SymbolTablePtr makeSymbolTable();
+
+  /**
+   * @return whether the system is initialized to use fake symbol tables.
+   */
+  static bool useFakeSymbolTables() { return use_fake_symbol_tables_; }
+
+private:
+  friend class TestUtil::SymbolTableCreatorTestPeer;
+
+  /**
+   * Sets whether fake or real symbol tables should be used. Tests that alter
+   * this should restore previous value at the end of the test. This must be
+   * called via TestUtil::SymbolTableCreatorTestPeer.
+   *
+   * *param use_fakes whether to use fake symbol tables.
+   */
+  static void setUseFakeSymbolTables(bool use_fakes) { use_fake_symbol_tables_ = use_fakes; }
+
+  static bool initialized_;
+  static bool use_fake_symbol_tables_;
+};
+
+} // namespace Stats
+} // namespace Envoy

source/common/thread_local/thread_local_impl.cc

@@ -1,5 +1,6 @@
 #include "common/thread_local/thread_local_impl.h"
 
+#include <algorithm>
 #include <atomic>
 #include <cstdint>
 #include <list>
@@ -24,16 +25,16 @@ SlotPtr InstanceImpl::allocateSlot() {
   ASSERT(std::this_thread::get_id() == main_thread_id_);
   ASSERT(!shutdown_);
 
-  for (uint64_t i = 0; i < slots_.size(); i++) {
-    if (slots_[i] == nullptr) {
-      std::unique_ptr<SlotImpl> slot(new SlotImpl(*this, i));
-      slots_[i] = slot.get();
-      return slot;
-    }
+  if (free_slot_indexes_.empty()) {
+    std::unique_ptr<SlotImpl> slot(new SlotImpl(*this, slots_.size()));
+    slots_.push_back(slot.get());
+    return slot;
   }
-
-  std::unique_ptr<SlotImpl> slot(new SlotImpl(*this, slots_.size()));
-  slots_.push_back(slot.get());
+  const uint32_t idx = free_slot_indexes_.front();
+  free_slot_indexes_.pop_front();
+  ASSERT(idx < slots_.size());
+  std::unique_ptr<SlotImpl> slot(new SlotImpl(*this, idx));
+  slots_[idx] = slot.get();
   return slot;
 }
 
@@ -73,6 +74,10 @@ void InstanceImpl::removeSlot(SlotImpl& slot) {
 
   const uint64_t index = slot.index_;
   slots_[index] = nullptr;
+  ASSERT(std::find(free_slot_indexes_.begin(), free_slot_indexes_.end(), index) ==
+             free_slot_indexes_.end(),
+         fmt::format("slot index {} already in free slot set!", index));
+  free_slot_indexes_.push_back(index);
   runOnAllThreads([index]() -> void {
     // This runs on each thread and clears the slot, making it available for a new allocations.
     // This is safe even if a new allocation comes in, because everything happens with post() and

source/common/thread_local/thread_local_impl.h

@@ -57,6 +57,9 @@ class InstanceImpl : Logger::Loggable<Logger::Id::main>, public Instance {
 
   static thread_local ThreadLocalData thread_local_data_;
   std::vector<SlotImpl*> slots_;
+  // A list of index of freed slots.
+  std::list<uint32_t> free_slot_indexes_;
+
   std::list<std::reference_wrapper<Event::Dispatcher>> registered_threads_;
   std::thread::id main_thread_id_;
   Event::Dispatcher* main_thread_dispatcher_{};

source/exe/BUILD

@@ -70,7 +70,7 @@ envoy_cc_library(
         "//source/common/api:os_sys_calls_lib",
         "//source/common/common:compiler_requirements_lib",
         "//source/common/common:perf_annotation_lib",
-        "//source/common/stats:fake_symbol_table_lib",
+        "//source/common/stats:symbol_table_creator_lib",
         "//source/server:hot_restart_lib",
         "//source/server:hot_restart_nop_lib",
         "//source/server/config_validation:server_lib",

source/exe/main_common.cc

@@ -7,6 +7,7 @@
 #include "common/common/compiler_requirements.h"
 #include "common/common/perf_annotation.h"
 #include "common/network/utility.h"
+#include "common/stats/symbol_table_creator.h"
 #include "common/stats/thread_local_store.h"
 
 #include "server/config_validation/server.h"
@@ -45,7 +46,9 @@ MainCommonBase::MainCommonBase(const OptionsImpl& options, Event::TimeSystem& ti
                                Filesystem::Instance& file_system,
                                std::unique_ptr<ProcessContext> process_context)
     : options_(options), component_factory_(component_factory), thread_factory_(thread_factory),
-      file_system_(file_system), stats_allocator_(symbol_table_) {
+      file_system_(file_system), symbol_table_(Stats::SymbolTableCreator::initAndMakeSymbolTable(
+                                     options_.fakeSymbolTableEnabled())),
+      stats_allocator_(*symbol_table_) {
   switch (options_.mode()) {
   case Server::Mode::InitOnly:
   case Server::Mode::Serve: {