diff --git a/envoy/stream_info/BUILD b/envoy/stream_info/BUILD index cb79d23921d8f..0071678073743 100644 --- a/envoy/stream_info/BUILD +++ b/envoy/stream_info/BUILD @@ -36,6 +36,7 @@ envoy_cc_library( deps = [ "//envoy/config:typed_config_interface", "//source/common/common:fmt_lib", + "//source/common/common:inline_map_registry", "//source/common/common:utility_lib", "//source/common/protobuf", ], diff --git a/envoy/stream_info/filter_state.h b/envoy/stream_info/filter_state.h index 7e7cfd7fdef0f..532c95eb0b0b4 100644 --- a/envoy/stream_info/filter_state.h +++ b/envoy/stream_info/filter_state.h @@ -7,6 +7,7 @@ #include "envoy/config/typed_config.h" #include "source/common/common/fmt.h" +#include "source/common/common/inline_map_registry.h" #include "source/common/common/utility.h" #include "source/common/protobuf/protobuf.h" @@ -16,6 +17,13 @@ namespace Envoy { namespace StreamInfo { +class FilterStateInlineMapScope { +public: + static absl::string_view name() { return "inline_map_scope_filter_state"; } +}; + +using InlineKey = InlineMapRegistry::Handle; + class FilterState; using FilterStateSharedPtr = std::shared_ptr; @@ -173,6 +181,25 @@ class FilterState { LifeSpan life_span = LifeSpan::FilterChain, StreamSharingMayImpactPooling stream_sharing = StreamSharingMayImpactPooling::None) PURE; + /** + * @param data_key the handle of the data being set. + * @param data an owning pointer to the data to be stored. + * @param state_type indicates whether the object is mutable or not. + * @param life_span indicates the life span of the object: bound to the filter chain, a + * request, or a connection. + * + * Note that it is an error to call setData() twice with the same + * data_name, if the existing object is immutable. Similarly, it is an + * error to call setData() with same data_name but different state_types + * (mutable and readOnly, or readOnly and mutable) or different life_span. + * This is to enforce a single authoritative source for each piece of + * data stored in FilterState. + */ + virtual void + setData(InlineKey data_key, std::shared_ptr data, StateType state_type, + LifeSpan life_span = LifeSpan::FilterChain, + StreamSharingMayImpactPooling stream_sharing = StreamSharingMayImpactPooling::None) PURE; + /** * @param data_name the name of the data being looked up (mutable/readonly). * @return a typed pointer to the stored data or nullptr if the data does not exist or the data @@ -182,12 +209,27 @@ class FilterState { return dynamic_cast(getDataReadOnlyGeneric(data_name)); } + /** + * @param data_key the handle of the data being looked up (mutable/readonly). + * @return a typed pointer to the stored data or nullptr if the data does not exist or the data + * type does not match the expected type. + */ + template const T* getDataReadOnly(InlineKey data_key) const { + return dynamic_cast(getDataReadOnlyGeneric(data_key)); + } + /** * @param data_name the name of the data being looked up (mutable/readonly). * @return a const pointer to the stored data or nullptr if the data does not exist. */ virtual const Object* getDataReadOnlyGeneric(absl::string_view data_name) const PURE; + /** + * @param data_key the handle of the data being looked up (mutable/readonly). + * @return a const pointer to the stored data or nullptr if the data does not exist. + */ + virtual const Object* getDataReadOnlyGeneric(InlineKey data_key) const PURE; + /** * @param data_name the name of the data being looked up (mutable/readonly). * @return a typed pointer to the stored data or nullptr if the data does not exist or the data @@ -197,18 +239,39 @@ class FilterState { return dynamic_cast(getDataMutableGeneric(data_name)); } + /** + * @param data_name the name of the data being looked up (mutable/readonly). + * @return a typed pointer to the stored data or nullptr if the data does not exist or the data + * type does not match the expected type. + */ + template T* getDataMutable(InlineKey data_key) { + return dynamic_cast(getDataMutableGeneric(data_key)); + } + /** * @param data_name the name of the data being looked up (mutable/readonly). * @return a pointer to the stored data or nullptr if the data does not exist. */ virtual Object* getDataMutableGeneric(absl::string_view data_name) PURE; + /** + * @param data_key the handle of the data being looked up (mutable/readonly). + * @return a pointer to the stored data or nullptr if the data does not exist. + */ + virtual Object* getDataMutableGeneric(InlineKey data_key) PURE; + /** * @param data_name the name of the data being looked up (mutable/readonly). * @return a shared pointer to the stored data or nullptr if the data does not exist. */ virtual std::shared_ptr getDataSharedMutableGeneric(absl::string_view data_name) PURE; + /** + * @param data_key the handle of the data being looked up (mutable/readonly). + * @return a shared pointer to the stored data or nullptr if the data does not exist. + */ + virtual std::shared_ptr getDataSharedMutableGeneric(InlineKey data_key) PURE; + /** * @param data_name the name of the data being probed. * @return Whether data of the type and name specified exists in the @@ -218,10 +281,34 @@ class FilterState { return getDataReadOnly(data_name) != nullptr; } + /** + * @param data_key the handle of the data being probed. + * @return Whether data of the type and name specified exists in the + * data store. + */ + template bool hasData(InlineKey data_key) const { + return getDataReadOnly(data_key) != nullptr; + } + + /** + * @param data_name the name of the data being probed. + * @return Whether data of any type and the name specified exists in the + * data store. + */ + virtual bool hasDataGeneric(absl::string_view data_name) const PURE; + + /** + * @param data_key the handle of the data being probed. + * @return Whether data of any type and the name specified exists in the + * data store. + */ + virtual bool hasDataGeneric(InlineKey data_key) const PURE; + /** * @param data_name the name of the data being probed. * @return Whether data of any type and the name specified exists in the * data store. + * NOTE: Please use hasDataUnTyped() instead. */ virtual bool hasDataWithName(absl::string_view data_name) const PURE; diff --git a/source/common/common/BUILD b/source/common/common/BUILD index 14bcf9917df77..5db79d6f818ac 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -592,8 +592,22 @@ envoy_cc_library( "abseil_node_hash_set", ], deps = [ + "//envoy/common:optref_lib", "//source/common/common:assert_lib", "//source/common/common:macros", "//source/common/common:utility_lib", ], ) + +envoy_cc_library( + name = "inline_map_registry", + srcs = [ + "inline_map_registry.cc", + ], + hdrs = [ + "inline_map_registry.h", + ], + deps = [ + ":inline_map", + ], +) diff --git a/source/common/common/inline_map.h b/source/common/common/inline_map.h index 4992a5971bf6d..f11a8c572e599 100644 --- a/source/common/common/inline_map.h +++ b/source/common/common/inline_map.h @@ -1,7 +1,9 @@ #pragma once +#include #include #include +#include #include #include @@ -125,13 +127,9 @@ template class InlineMapDescriptor { /** * Finalize this descriptor. No further changes are allowed after this point. This guarantees that - * all map created by the process have the same variable size and custom inline key adding. This - * method should only be called once. + * all map created by the process have the same variable size and custom inline key adding. */ - void finalize() { - ASSERT(!finalized_, "Cannot finalize() an already finalized descriptor"); - finalized_ = true; - } + void finalize() { finalized_ = true; } /** * @return true if the descriptor is finalized. @@ -178,7 +176,12 @@ template class InlineMapDescriptor { return inline_keys_map_.size(); } - bool finalized_{}; + // The finalize()/finalized() methods of cross-modules descriptor may be called at same time + // from different threads. So we need to use atomic to protect the finalized_ flag. + // This is only happens in the multiple threads tests because all cross-modules descriptors + // should be finalized when the first server instance is initialized in formal running. + std::atomic finalized_{}; + InlineKeys inline_keys_; InlineKeysMap inline_keys_map_; }; @@ -414,41 +417,42 @@ template class InlineMap : public InlineStorage { return inline_entries_[handle.inlineId()]; } - /** - * Create an inline map with the given descriptor. If the descriptor is not finalized, it will be - * finalized before creating the inline map. - * @param descriptor the descriptor that contains the inline keys. - * @return the created inline map. - */ - static std::unique_ptr create(TypedInlineMapDescriptor& descriptor) { - if (!descriptor.finalized()) { - // Call finalize() to make sure that the descriptor is finalized and no any new inline - // keys could be registered. - descriptor.finalize(); - } - - return std::unique_ptr(new ((descriptor.inlineKeysNum() * sizeof(Value))) - InlineMap(descriptor)); + InlineMap(const TypedInlineMapDescriptor& descriptor) : descriptor_(descriptor) { + ASSERT(descriptor_.finalized(), "Cannot create inline map before finalize()"); } private: + using StoragePtr = std::unique_ptr; friend class InlineMapDescriptor; - InlineMap(TypedInlineMapDescriptor& descriptor) : descriptor_(descriptor) { - // Initialize the inline entries to nullptr. - uint64_t inline_keys_num = descriptor_.inlineKeysNum(); - inline_entries_valid_.resize(inline_keys_num, false); - memset(inline_entries_storage_, 0, inline_keys_num * sizeof(Value)); - inline_entries_ = reinterpret_cast(inline_entries_storage_); + void lazyAllocateMemoryInlineEntries() { + if (inline_entries_storage_ != nullptr) { + return; + } + + const uint64_t inline_keys_num = descriptor_.inlineKeysNum(); + const uint64_t value_memory_size = inline_keys_num * sizeof(Value); + const uint64_t flag_memory_size = inline_keys_num * sizeof(bool); + const uint64_t memory_size = value_memory_size + flag_memory_size; + + inline_entries_storage_.reset(new uint8_t[memory_size]); + memset(inline_entries_storage_.get(), 0, memory_size); + + inline_entries_ = reinterpret_cast(inline_entries_storage_.get()); + inline_entries_valid_ = + reinterpret_cast(inline_entries_storage_.get() + value_memory_size); } template void resetInlineMapEntry(uint64_t inline_entry_id, SetValue&& value) { ASSERT(inline_entry_id < descriptor_.inlineKeysNum()); + // Only allocate the memory for inline entries when the first inline entry is added. + lazyAllocateMemoryInlineEntries(); clearInlineMapEntry(inline_entry_id); // Construct the new entry in the inline array. new (inline_entries_ + inline_entry_id) Value(std::forward(value)); + setInlineEntryValid(inline_entry_id, true); } @@ -484,10 +488,16 @@ template class InlineMap : public InlineStorage { bool inlineEntryValid(uint64_t inline_entry_id) const { ASSERT(inline_entry_id < descriptor_.inlineKeysNum()); + + if (inline_entries_storage_ == nullptr) { + // If the inline entries storage is not allocated, then the inline entry is not valid. + return false; + } return inline_entries_valid_[inline_entry_id]; } void setInlineEntryValid(uint64_t inline_entry_id, bool flag) { ASSERT(inline_entry_id < descriptor_.inlineKeysNum()); + ASSERT(inline_entries_storage_ != nullptr); if (flag) { inline_entries_size_++; @@ -506,20 +516,13 @@ template class InlineMap : public InlineStorage { // This is the underlay hash map for the dynamic map entries. DynamicHashMap dynamic_entries_; - // These are flags to indicate if the inline entries are valid. - // TODO(wbpcode): this will add additional one time memory allocation when constructing inline - // map. It is possible to use memory in the inline_entries_storage_ to store the flags in the - // future. - std::vector inline_entries_valid_; - uint64_t inline_entries_size_{}; Value* inline_entries_{}; + // These are flags to indicate if the inline entries are valid. + bool* inline_entries_valid_{}; - // This should be the last member of the class and no member should be added after this. - uint8_t inline_entries_storage_[]; + StoragePtr inline_entries_storage_; }; -template using InlineMapPtr = std::unique_ptr>; - } // namespace Envoy diff --git a/source/common/common/inline_map_registry.cc b/source/common/common/inline_map_registry.cc new file mode 100644 index 0000000000000..7bc62a4e0a03e --- /dev/null +++ b/source/common/common/inline_map_registry.cc @@ -0,0 +1,51 @@ +#include "source/common/common/inline_map_registry.h" + +namespace Envoy { + +InlineMapRegistry::Descriptor* InlineMapRegistry::createDescriptor(absl::string_view scope_name) { + // This should never be triggered because the developer should ensure all used/managed + // descriptors are registered before main() function by static variable initialization. + RELEASE_ASSERT(!finalized_, "Registry is finalized, cannot create new descriptor"); + + // Create the descriptor and insert it into the registry. + auto descriptor = new Descriptor(); + auto [_, inserted] = descriptors_.emplace(scope_name, descriptor); + + // Check whether the descriptor is created repeatedly. + RELEASE_ASSERT(inserted, fmt::format("Create descriptor {} repeatedly", scope_name)); + + return descriptor; +} + +const InlineMapRegistry::Descriptors& InlineMapRegistry::descriptors() const { + RELEASE_ASSERT(finalized_, "Registry is not finalized, cannot get registry"); + return descriptors_; +} + +void InlineMapRegistry::finalize(const ScopeInlineKeysVector& scope_inline_keys_vector) { + // Call once to finalize the registry and all managed descriptors. This is used to ensure + // the registry is finalized only once on multi-thread environment. + std::call_once(once_flag_, [this, &scope_inline_keys_vector]() { + finalized_ = true; + + // Add inline keys to the descriptor based on the scope name. This is last chance to add + // inline keys to the descriptor. + for (const auto& inline_keys : scope_inline_keys_vector) { + auto iter = descriptors_.find(inline_keys.name); + if (iter == descriptors_.end()) { + // Skip the scope that is not registered. + ENVOY_LOG_MISC(warn, "Skip the scope {} that is not registered", inline_keys.name); + continue; + } + + for (const auto& key : inline_keys.keys) { + iter->second->addInlineKey(key); + } + } + + // Finalize all managed descriptors. + std::for_each(descriptors_.begin(), descriptors_.end(), [](auto& p) { p.second->finalize(); }); + }); +} + +} // namespace Envoy diff --git a/source/common/common/inline_map_registry.h b/source/common/common/inline_map_registry.h new file mode 100644 index 0000000000000..9b3fbd4684806 --- /dev/null +++ b/source/common/common/inline_map_registry.h @@ -0,0 +1,160 @@ +#pragma once + +#include "source/common/common/inline_map.h" + +namespace Envoy { + +/** + * Registry class to get inline map descriptor singleton based on the scope. If cross-modules inline + * map descriptor is necessary, this class should be used. + * + * There are only two allowed ways to update or modify the registry: + * 1. Add inline map descriptors or inline keys to the registry before the main() function is called + * by the REGISTER_INLINE_MAP_DESCRIPTOR or REGISTER_INLINE_MAP_KEY macro. + * 2. Add inline map descriptors or inline keys in the finalize() function. + * + * The finalize() function will be called once when the first server is initialized. This is safe + * even there are multiple main threads or servers. + */ +class InlineMapRegistry { +public: + /** + * Helper template class to initialize descriptor based on the scope explicitly. + */ + template class InlineMapDescriptorRegister { + public: + InlineMapDescriptorRegister(); + }; + + /** + * Helper template class to add inline key to the descriptor based on the scope. + */ + template class InlineMapKeyRegister { + public: + InlineMapKeyRegister(absl::string_view); + }; + + using Descriptor = InlineMapDescriptor; + using Handle = Descriptor::Handle; + + /** + * All registered descriptors. This is a map from scope name to the descriptor pointer. + */ + using Descriptors = absl::flat_hash_map; + + struct ScopeInlineKeys { + std::string name; + absl::flat_hash_set keys; + }; + + /** + * Scope inline keys vector type. This is used to finalize the registry and add inline keys to + * the descriptor based on the scope name. + */ + using ScopeInlineKeysVector = std::vector; + +private: + template friend class InlineMapDescriptorRegister; + template friend class InlineMapKeyRegister; + + /** + * Default constructor. This is private because the registry should be a singleton that is only + * created once and get by the InlineMapRegistry::get() function. + */ + InlineMapRegistry() = default; + + /** + * Create the inline map descriptor based on the scope name and insert it into the registry. + */ + Descriptor* createDescriptor(absl::string_view scope_name); + + /** + * Get or create the inline map descriptor singleton based on the scope type and insert + * it into the registry. + */ + template static Descriptor& getOrCreateDescriptor() { + // The function level static variable will be initialized only once and is thread safe. + // This is same with MUTABLE_CONSTRUCT_ON_FIRST_USE but has complex initialization. + static Descriptor* initialize_once_descriptor = get().createDescriptor(Scope::name()); + return *initialize_once_descriptor; + } + +public: + /** + * Get the inline map registry singleton. + */ + static InlineMapRegistry& get() { MUTABLE_CONSTRUCT_ON_FIRST_USE(InlineMapRegistry); } + + /** + * Get the inline handle by the key name. + */ + template absl::optional static getHandleByKey(absl::string_view key) { + return getOrCreateDescriptor().getHandleByKey(key); + } + + /** + * Get the finalized inline map descriptor singleton based on the scope type. This is used to get + * the finalized descriptor to construct the inline map. + */ + template static const Descriptor& finalizedDescriptor() { + auto& descriptor = getOrCreateDescriptor(); + descriptor.finalize(); + return descriptor; + } + + /** + * Get the registered descriptors. This is used to get all registered descriptors that are + * managed by this registry. + */ + const Descriptors& descriptors() const; + + /** + * Finalize the registry and all managed descriptors by this registry. This only be called once + * when the server is initialized. This is safe even there are multiple main threads or servers. + */ + void finalize(const ScopeInlineKeysVector& scope_inline_keys_vector = {}); + +private: + Descriptors descriptors_; + std::once_flag once_flag_; + bool finalized_{}; +}; + +template +InlineMapRegistry::InlineMapDescriptorRegister::InlineMapDescriptorRegister() { + InlineMapRegistry::getOrCreateDescriptor(); +} + +template +InlineMapRegistry::InlineMapKeyRegister::InlineMapKeyRegister(absl::string_view key) { + InlineMapRegistry::getOrCreateDescriptor().addInlineKey(key); +} + +/** + * Helper macro to register inline map descriptor based on the scope explicitly. If cross-modules + * inline map descriptor is necessary and InlineMapRegistry is used, then this macro should be used. + */ +#define REGISTER_INLINE_MAP_DESCRIPTOR(Scope) \ + static const Envoy::InlineMapRegistry::InlineMapDescriptorRegister< \ + Scope>##InlineMapDescriptorRegister; + +/** + * Helper macro to add inline key to the descriptor based on the scope. If cross-modules inline map + * descriptor is necessary and InlineMapRegistry is used, then this macro should be used. + */ +#define REGISTER_INLINE_MAP_KEY(Scope, Key) \ + static const Envoy::InlineMapRegistry::InlineMapKeyRegister Key##InlineMapKeyRegister(Key); + +/** + * Helper macro to get inline handle by the key name. Note this macro should be used in the function + * that is called to get the inline handle. And please ensure the key is registered before the + * function is called. + */ +#define INLINE_HANDLE_BY_KEY_ON_FIRST_USE(Scope, Key) \ + do { \ + static auto handle = Envoy::InlineMapRegistry::getHandleByKey(Key); \ + ASSERT(handle.has_value(), fmt::format("Cannot get inline handle by key {}", Key)); \ + return handle.value(); \ + } while (0) + +} // namespace Envoy diff --git a/source/common/network/application_protocol.cc b/source/common/network/application_protocol.cc index 6794194ac90d5..ebb40ccd9043b 100644 --- a/source/common/network/application_protocol.cc +++ b/source/common/network/application_protocol.cc @@ -5,8 +5,15 @@ namespace Envoy { namespace Network { -const std::string& ApplicationProtocols::key() { - CONSTRUCT_ON_FIRST_USE(std::string, "envoy.network.application_protocols"); +constexpr absl::string_view ApplicationProtocolsKey = "envoy.network.application_protocols"; + +REGISTER_INLINE_MAP_KEY(StreamInfo::FilterStateInlineMapScope, ApplicationProtocolsKey); + +using Scope = StreamInfo::FilterStateInlineMapScope; + +const StreamInfo::InlineKey ApplicationProtocols::key() { + INLINE_HANDLE_BY_KEY_ON_FIRST_USE(StreamInfo::FilterStateInlineMapScope, ApplicationProtocolsKey); } + } // namespace Network } // namespace Envoy diff --git a/source/common/network/application_protocol.h b/source/common/network/application_protocol.h index 177e584a5eb55..0628a4d3f8b7b 100644 --- a/source/common/network/application_protocol.h +++ b/source/common/network/application_protocol.h @@ -14,7 +14,7 @@ class ApplicationProtocols : public StreamInfo::FilterState::Object { explicit ApplicationProtocols(const std::vector& application_protocols) : application_protocols_(application_protocols) {} const std::vector& value() const { return application_protocols_; } - static const std::string& key(); + static const StreamInfo::InlineKey key(); private: const std::vector application_protocols_; diff --git a/source/common/network/filter_state_proxy_info.cc b/source/common/network/filter_state_proxy_info.cc index 0f43ef5e17341..f3ac8f0b22705 100644 --- a/source/common/network/filter_state_proxy_info.cc +++ b/source/common/network/filter_state_proxy_info.cc @@ -3,8 +3,14 @@ namespace Envoy { namespace Network { -const std::string& Http11ProxyInfoFilterState::key() { - CONSTRUCT_ON_FIRST_USE(std::string, "envoy.network.transport_socket.http_11_proxy.info"); +constexpr absl::string_view Http11ProxyInfoFilterStateKey = + "envoy.network.http_connection_manager.http1_proxy"; + +REGISTER_INLINE_MAP_KEY(StreamInfo::FilterStateInlineMapScope, Http11ProxyInfoFilterStateKey); + +const StreamInfo::InlineKey Http11ProxyInfoFilterState::key() { + INLINE_HANDLE_BY_KEY_ON_FIRST_USE(StreamInfo::FilterStateInlineMapScope, + Http11ProxyInfoFilterStateKey); } } // namespace Network diff --git a/source/common/network/filter_state_proxy_info.h b/source/common/network/filter_state_proxy_info.h index e06beeccbd488..33521e753de6c 100644 --- a/source/common/network/filter_state_proxy_info.h +++ b/source/common/network/filter_state_proxy_info.h @@ -15,7 +15,7 @@ namespace Network { class Http11ProxyInfoFilterState : public StreamInfo::FilterState::Object { public: // Returns the key for looking up the Http11ProxyInfoFilterState in the FilterState. - static const std::string& key(); + static const StreamInfo::InlineKey key(); Http11ProxyInfoFilterState(absl::string_view hostname, Network::Address::InstanceConstSharedPtr address) diff --git a/source/common/network/proxy_protocol_filter_state.cc b/source/common/network/proxy_protocol_filter_state.cc index 37940bbad746a..12946d24dcb81 100644 --- a/source/common/network/proxy_protocol_filter_state.cc +++ b/source/common/network/proxy_protocol_filter_state.cc @@ -5,8 +5,13 @@ namespace Envoy { namespace Network { -const std::string& ProxyProtocolFilterState::key() { - CONSTRUCT_ON_FIRST_USE(std::string, "envoy.network.proxy_protocol_options"); +constexpr absl::string_view ProxyProtocolFilterStateKey = "envoy.network.proxy_protocol_options"; + +REGISTER_INLINE_MAP_KEY(StreamInfo::FilterStateInlineMapScope, ProxyProtocolFilterStateKey); + +const StreamInfo::InlineKey ProxyProtocolFilterState::key() { + INLINE_HANDLE_BY_KEY_ON_FIRST_USE(StreamInfo::FilterStateInlineMapScope, + ProxyProtocolFilterStateKey); } } // namespace Network diff --git a/source/common/network/proxy_protocol_filter_state.h b/source/common/network/proxy_protocol_filter_state.h index 9cb35a9ee8781..83710c662eec6 100644 --- a/source/common/network/proxy_protocol_filter_state.h +++ b/source/common/network/proxy_protocol_filter_state.h @@ -13,7 +13,7 @@ class ProxyProtocolFilterState : public StreamInfo::FilterState::Object { public: ProxyProtocolFilterState(Network::ProxyProtocolData options) : options_(options) {} const Network::ProxyProtocolData& value() const { return options_; } - static const std::string& key(); + static const StreamInfo::InlineKey key(); private: const Network::ProxyProtocolData options_; diff --git a/source/common/network/upstream_server_name.cc b/source/common/network/upstream_server_name.cc index c6bc2559679eb..b216a720526ca 100644 --- a/source/common/network/upstream_server_name.cc +++ b/source/common/network/upstream_server_name.cc @@ -5,8 +5,13 @@ namespace Envoy { namespace Network { -const std::string& UpstreamServerName::key() { - CONSTRUCT_ON_FIRST_USE(std::string, "envoy.network.upstream_server_name"); +constexpr absl::string_view UpstreamServerNameKey = "envoy.network.upstream_server_name"; + +REGISTER_INLINE_MAP_KEY(StreamInfo::FilterStateInlineMapScope, UpstreamServerNameKey); + +const StreamInfo::InlineKey UpstreamServerName::key() { + INLINE_HANDLE_BY_KEY_ON_FIRST_USE(StreamInfo::FilterStateInlineMapScope, UpstreamServerNameKey); } + } // namespace Network } // namespace Envoy diff --git a/source/common/network/upstream_server_name.h b/source/common/network/upstream_server_name.h index e50bb906af5c6..9f244f3b09c17 100644 --- a/source/common/network/upstream_server_name.h +++ b/source/common/network/upstream_server_name.h @@ -16,7 +16,7 @@ class UpstreamServerName : public StreamInfo::FilterState::Object { public: UpstreamServerName(absl::string_view server_name) : server_name_(server_name) {} const std::string& value() const { return server_name_; } - static const std::string& key(); + static const StreamInfo::InlineKey key(); private: const std::string server_name_; diff --git a/source/common/network/upstream_subject_alt_names.cc b/source/common/network/upstream_subject_alt_names.cc index df8aa9adb1842..b08369c2cd41e 100644 --- a/source/common/network/upstream_subject_alt_names.cc +++ b/source/common/network/upstream_subject_alt_names.cc @@ -5,8 +5,14 @@ namespace Envoy { namespace Network { -const std::string& UpstreamSubjectAltNames::key() { - CONSTRUCT_ON_FIRST_USE(std::string, "envoy.network.upstream_subject_alt_names"); +constexpr absl::string_view UpstreamSubjectAltNamesKey = "envoy.network.upstream_subject_alt_names"; + +REGISTER_INLINE_MAP_KEY(StreamInfo::FilterStateInlineMapScope, UpstreamSubjectAltNamesKey); + +const StreamInfo::InlineKey UpstreamSubjectAltNames::key() { + INLINE_HANDLE_BY_KEY_ON_FIRST_USE(StreamInfo::FilterStateInlineMapScope, + UpstreamSubjectAltNamesKey); } + } // namespace Network } // namespace Envoy diff --git a/source/common/network/upstream_subject_alt_names.h b/source/common/network/upstream_subject_alt_names.h index a7ed77905508d..4ac7ab5189982 100644 --- a/source/common/network/upstream_subject_alt_names.h +++ b/source/common/network/upstream_subject_alt_names.h @@ -14,7 +14,7 @@ class UpstreamSubjectAltNames : public StreamInfo::FilterState::Object { explicit UpstreamSubjectAltNames(const std::vector& upstream_subject_alt_names) : upstream_subject_alt_names_(upstream_subject_alt_names) {} const std::vector& value() const { return upstream_subject_alt_names_; } - static const std::string& key(); + static const StreamInfo::InlineKey key(); private: const std::vector upstream_subject_alt_names_; diff --git a/source/common/router/debug_config.cc b/source/common/router/debug_config.cc index 4b6d4f3b5cb41..eeb239a9a6903 100644 --- a/source/common/router/debug_config.cc +++ b/source/common/router/debug_config.cc @@ -5,6 +5,10 @@ namespace Envoy { namespace Router { +constexpr absl::string_view DebugConfigKey = "envoy.router.debug_config"; + +REGISTER_INLINE_MAP_KEY(StreamInfo::FilterStateInlineMapScope, DebugConfigKey); + DebugConfig::DebugConfig(bool append_cluster, absl::optional cluster_header, bool append_upstream_host, absl::optional hostname_header, @@ -16,8 +20,8 @@ DebugConfig::DebugConfig(bool append_cluster, absl::optionallength() : 0; } @@ -546,7 +554,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, absl::string_view sni_value = parsed_authority.host_; if (should_set_sni && upstream_http_protocol_options.value().auto_sni() && - !callbacks_->streamInfo().filterState()->hasDataWithName( + !callbacks_->streamInfo().filterState()->hasDataGeneric( Network::UpstreamServerName::key())) { callbacks_->streamInfo().filterState()->setData( Network::UpstreamServerName::key(), @@ -555,7 +563,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } if (upstream_http_protocol_options.value().auto_san_validation() && - !callbacks_->streamInfo().filterState()->hasDataWithName( + !callbacks_->streamInfo().filterState()->hasDataGeneric( Network::UpstreamSubjectAltNames::key())) { callbacks_->streamInfo().filterState()->setData( Network::UpstreamSubjectAltNames::key(), @@ -1755,13 +1763,13 @@ bool Filter::convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& do // redirects allowed for this route. StreamInfo::UInt32Accessor* num_internal_redirect{}; - if (num_internal_redirect = filter_state->getDataMutable( - NumInternalRedirectsFilterStateName); + if (num_internal_redirect = + filter_state->getDataMutable(internalRedirectCountKey()); num_internal_redirect == nullptr) { auto state = std::make_shared(0); num_internal_redirect = state.get(); - filter_state->setData(NumInternalRedirectsFilterStateName, std::move(state), + filter_state->setData(internalRedirectCountKey(), std::move(state), StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::Request); } diff --git a/source/common/stream_info/filter_state_impl.cc b/source/common/stream_info/filter_state_impl.cc index e59e1138cdaac..12a5d8fbcdced 100644 --- a/source/common/stream_info/filter_state_impl.cc +++ b/source/common/stream_info/filter_state_impl.cc @@ -8,89 +8,47 @@ namespace StreamInfo { void FilterStateImpl::setData(absl::string_view data_name, std::shared_ptr data, FilterState::StateType state_type, FilterState::LifeSpan life_span, StreamSharingMayImpactPooling stream_sharing) { - if (life_span > life_span_) { - if (hasDataWithNameInternally(data_name)) { - IS_ENVOY_BUG("FilterStateAccessViolation: FilterState::setData called twice with " - "conflicting life_span on the same data_name."); - return; - } - maybeCreateParent(ParentAccessMode::ReadWrite); - parent_->setData(data_name, data, state_type, life_span, stream_sharing); - return; - } - if (parent_ && parent_->hasDataWithName(data_name)) { - IS_ENVOY_BUG("FilterStateAccessViolation: FilterState::setData called twice with " - "conflicting life_span on the same data_name."); - return; - } - const auto& it = data_storage_.find(data_name); - if (it != data_storage_.end()) { - // We have another object with same data_name. Check for mutability - // violations namely: readonly data cannot be overwritten, mutable data - // cannot be overwritten by readonly data. - const FilterStateImpl::FilterObject* current = it->second.get(); - if (current->state_type_ == FilterState::StateType::ReadOnly) { - IS_ENVOY_BUG("FilterStateAccessViolation: FilterState::setData called twice on same " - "ReadOnly state."); - return; - } - - if (current->state_type_ != state_type) { - IS_ENVOY_BUG("FilterStateAccessViolation: FilterState::setData called twice with " - "different state types."); - return; - } - } + setDataInternal(data_name, data, state_type, life_span, stream_sharing); +} - std::unique_ptr filter_object(new FilterStateImpl::FilterObject()); - filter_object->data_ = data; - filter_object->state_type_ = state_type; - filter_object->stream_sharing_ = stream_sharing; - data_storage_[data_name] = std::move(filter_object); +void FilterStateImpl::setData(InlineKey data_key, std::shared_ptr data, + StateType state_type, LifeSpan life_span, + StreamSharingMayImpactPooling stream_sharing) { + setDataInternal(data_key, data, state_type, life_span, stream_sharing); } bool FilterStateImpl::hasDataWithName(absl::string_view data_name) const { - return hasDataWithNameInternally(data_name) || (parent_ && parent_->hasDataWithName(data_name)); + return hasDataInternal(data_name) || (parent_ && parent_->hasDataWithName(data_name)); +} +bool FilterStateImpl::hasDataGeneric(absl::string_view data_name) const { + return hasDataInternal(data_name) || (parent_ && parent_->hasDataGeneric(data_name)); +} +bool FilterStateImpl::hasDataGeneric(InlineKey data_key) const { + return hasDataInternal(data_key) || (parent_ && parent_->hasDataGeneric(data_key)); } const FilterState::Object* FilterStateImpl::getDataReadOnlyGeneric(absl::string_view data_name) const { - const auto it = data_storage_.find(data_name); - - if (it == data_storage_.end()) { - if (parent_) { - return parent_->getDataReadOnlyGeneric(data_name); - } - return nullptr; - } - - const FilterStateImpl::FilterObject* current = it->second.get(); - return current->data_.get(); + return getDataReadOnlyGenericInternal(data_name); +} +const FilterState::Object* FilterStateImpl::getDataReadOnlyGeneric(InlineKey data_key) const { + return getDataReadOnlyGenericInternal(data_key); } FilterState::Object* FilterStateImpl::getDataMutableGeneric(absl::string_view data_name) { return getDataSharedMutableGeneric(data_name).get(); } +FilterState::Object* FilterStateImpl::getDataMutableGeneric(InlineKey data_key) { + return getDataSharedMutableGeneric(data_key).get(); +} std::shared_ptr FilterStateImpl::getDataSharedMutableGeneric(absl::string_view data_name) { - const auto& it = data_storage_.find(data_name); - - if (it == data_storage_.end()) { - if (parent_) { - return parent_->getDataSharedMutableGeneric(data_name); - } - return nullptr; - } - - FilterStateImpl::FilterObject* current = it->second.get(); - if (current->state_type_ == FilterState::StateType::ReadOnly) { - IS_ENVOY_BUG("FilterStateAccessViolation: FilterState accessed immutable data as mutable."); - // To reduce the chances of a crash, allow the mutation in this case instead of returning a - // nullptr. - } - - return current->data_; + return getDataSharedMutableGenericInternal(data_name); +} +std::shared_ptr +FilterStateImpl::getDataSharedMutableGeneric(InlineKey data_key) { + return getDataSharedMutableGenericInternal(data_key); } bool FilterStateImpl::hasDataAtOrAboveLifeSpan(FilterState::LifeSpan life_span) const { @@ -103,24 +61,24 @@ bool FilterStateImpl::hasDataAtOrAboveLifeSpan(FilterState::LifeSpan life_span) FilterState::ObjectsPtr FilterStateImpl::objectsSharedWithUpstreamConnection() const { auto objects = parent_ ? parent_->objectsSharedWithUpstreamConnection() : std::make_unique(); - for (const auto& [name, object] : data_storage_) { - switch (object->stream_sharing_) { + + data_storage_.iterate([&objects](const std::string& name, const FilterObject& object) -> bool { + switch (object.stream_sharing_) { case StreamSharingMayImpactPooling::SharedWithUpstreamConnection: - objects->push_back({object->data_, object->state_type_, object->stream_sharing_, name}); + objects->push_back( + {object.data_, object.state_type_, object.stream_sharing_, std::string(name)}); break; case StreamSharingMayImpactPooling::SharedWithUpstreamConnectionOnce: - objects->push_back( - {object->data_, object->state_type_, StreamSharingMayImpactPooling::None, name}); + objects->push_back({object.data_, object.state_type_, StreamSharingMayImpactPooling::None, + std::string(name)}); break; default: break; } - } - return objects; -} + return true; + }); -bool FilterStateImpl::hasDataWithNameInternally(absl::string_view data_name) const { - return data_storage_.contains(data_name); + return objects; } void FilterStateImpl::maybeCreateParent(ParentAccessMode parent_access_mode) { diff --git a/source/common/stream_info/filter_state_impl.h b/source/common/stream_info/filter_state_impl.h index b6c736ffc8470..cc148a309ed5e 100644 --- a/source/common/stream_info/filter_state_impl.h +++ b/source/common/stream_info/filter_state_impl.h @@ -14,7 +14,9 @@ namespace StreamInfo { class FilterStateImpl : public FilterState { public: - FilterStateImpl(FilterState::LifeSpan life_span) : life_span_(life_span) { + FilterStateImpl(FilterState::LifeSpan life_span) + : life_span_(life_span), + data_storage_(InlineMapRegistry::finalizedDescriptor()) { maybeCreateParent(ParentAccessMode::ReadOnly); } @@ -23,7 +25,8 @@ class FilterStateImpl : public FilterState { * @param life_span the life span this is handling. */ FilterStateImpl(FilterStateSharedPtr ancestor, FilterState::LifeSpan life_span) - : ancestor_(ancestor), life_span_(life_span) { + : ancestor_(ancestor), life_span_(life_span), + data_storage_(InlineMapRegistry::finalizedDescriptor()) { maybeCreateParent(ParentAccessMode::ReadOnly); } @@ -35,7 +38,8 @@ class FilterStateImpl : public FilterState { * @param life_span the life span this is handling. */ FilterStateImpl(LazyCreateAncestor lazy_create_ancestor, FilterState::LifeSpan life_span) - : ancestor_(lazy_create_ancestor), life_span_(life_span) { + : ancestor_(lazy_create_ancestor), life_span_(life_span), + data_storage_(InlineMapRegistry::finalizedDescriptor()) { maybeCreateParent(ParentAccessMode::ReadOnly); } @@ -44,10 +48,24 @@ class FilterStateImpl : public FilterState { absl::string_view data_name, std::shared_ptr data, FilterState::StateType state_type, FilterState::LifeSpan life_span = FilterState::LifeSpan::FilterChain, StreamSharingMayImpactPooling stream_sharing = StreamSharingMayImpactPooling::None) override; + void setData( + InlineKey data_key, std::shared_ptr data, StateType state_type, + LifeSpan life_span = LifeSpan::FilterChain, + StreamSharingMayImpactPooling stream_sharing = StreamSharingMayImpactPooling::None) override; + bool hasDataWithName(absl::string_view) const override; + bool hasDataGeneric(absl::string_view data_name) const override; + bool hasDataGeneric(InlineKey data_key) const override; + const Object* getDataReadOnlyGeneric(absl::string_view data_name) const override; + const Object* getDataReadOnlyGeneric(InlineKey data_key) const override; + Object* getDataMutableGeneric(absl::string_view data_name) override; + Object* getDataMutableGeneric(InlineKey data_key) override; + std::shared_ptr getDataSharedMutableGeneric(absl::string_view data_name) override; + std::shared_ptr getDataSharedMutableGeneric(InlineKey data_key) override; + bool hasDataAtOrAboveLifeSpan(FilterState::LifeSpan life_span) const override; FilterState::ObjectsPtr objectsSharedWithUpstreamConnection() const override; @@ -55,15 +73,97 @@ class FilterStateImpl : public FilterState { FilterStateSharedPtr parent() const override { return parent_; } private: + template + void setDataInternal(DataKeyType data_key, std::shared_ptr data, StateType state_type, + LifeSpan life_span, StreamSharingMayImpactPooling stream_sharing) { + + if (life_span > life_span_) { + if (hasDataInternal(data_key)) { + IS_ENVOY_BUG("FilterStateAccessViolation: FilterState::setData called twice with " + "conflicting life_span on the same data_name."); + return; + } + maybeCreateParent(ParentAccessMode::ReadWrite); + parent_->setData(data_key, data, state_type, life_span, stream_sharing); + return; + } + if (parent_ && parent_->hasDataGeneric(data_key)) { + IS_ENVOY_BUG("FilterStateAccessViolation: FilterState::setData called twice with " + "conflicting life_span on the same data_name."); + return; + } + const auto current = data_storage_.get(data_key); + if (current.has_value()) { + // We have another object with same data_name. Check for mutability + // violations namely: readonly data cannot be overwritten, mutable data + // cannot be overwritten by readonly data. + if (current->state_type_ == FilterState::StateType::ReadOnly) { + IS_ENVOY_BUG("FilterStateAccessViolation: FilterState::setData called twice on same " + "ReadOnly state."); + return; + } + + if (current->state_type_ != state_type) { + IS_ENVOY_BUG("FilterStateAccessViolation: FilterState::setData called twice with " + "different state types."); + return; + } + } + + FilterStateImpl::FilterObject filter_object; + filter_object.data_ = data; + filter_object.state_type_ = state_type; + filter_object.stream_sharing_ = stream_sharing; + data_storage_[data_key] = std::move(filter_object); + } + // This only checks the local data_storage_ for data_name existence. - bool hasDataWithNameInternally(absl::string_view data_name) const; + template bool hasDataInternal(DataKeyType data_key) const { + return data_storage_.get(data_key).has_value(); + } + + template + const FilterState::Object* getDataReadOnlyGenericInternal(DataKeyType data_key) const { + const auto current = data_storage_.get(data_key); + + if (!current.has_value()) { + if (parent_) { + return parent_->getDataReadOnlyGeneric(data_key); + } + return nullptr; + } + + return current->data_.get(); + } + + template + std::shared_ptr getDataSharedMutableGenericInternal(DataKeyType data_key) { + const auto current = data_storage_.get(data_key); + + if (!current.has_value()) { + if (parent_) { + return parent_->getDataSharedMutableGeneric(data_key); + } + return nullptr; + } + + if (current->state_type_ == FilterState::StateType::ReadOnly) { + IS_ENVOY_BUG("FilterStateAccessViolation: FilterState accessed immutable data as mutable."); + // To reduce the chances of a crash, allow the mutation in this case instead of returning a + // nullptr. + } + + return current->data_; + } + enum class ParentAccessMode { ReadOnly, ReadWrite }; void maybeCreateParent(ParentAccessMode parent_access_mode); absl::variant ancestor_; FilterStateSharedPtr parent_; const FilterState::LifeSpan life_span_; - absl::flat_hash_map> data_storage_; + + InlineMap data_storage_; }; } // namespace StreamInfo diff --git a/source/server/server.cc b/source/server/server.cc index ef1fe2c01de1c..072a23396492f 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -27,6 +27,7 @@ #include "source/common/api/api_impl.h" #include "source/common/api/os_sys_calls_impl.h" #include "source/common/common/enum_to_int.h" +#include "source/common/common/inline_map_registry.h" #include "source/common/common/mutex_tracer_impl.h" #include "source/common/common/utility.h" #include "source/common/config/utility.h" @@ -473,6 +474,13 @@ void InstanceImpl::initialize(Network::Address::InstanceConstSharedPtr local_add absl::StrJoin(info.registered_headers_, ",")); } + // Finalize all inline map descriptors that managed by the inline map registry. + InlineMapRegistry::get().finalize(); + ENVOY_LOG(info, "Inline map registry info:"); + for (const auto& info : InlineMapRegistry::get().descriptors()) { + ENVOY_LOG(info, " {}: {}", info.first, info.second->inlineKeysAsString()); + }; + // Initialize the regex engine and inject to singleton. // Needs to happen before stats store initialization because the stats // matcher config can include regexes. diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index beb5dad69afe2..a13510ee6236d 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -408,7 +408,7 @@ TEST_F(HttpConnectionManagerImplTest, PopulateStreamInfo) { EXPECT_EQ(filter_callbacks_.connection_.id_, decoder_->streamInfo().downstreamAddressProvider().connectionID().value()); EXPECT_EQ(server_name_, decoder_->streamInfo().downstreamAddressProvider().requestedServerName()); - EXPECT_TRUE(decoder_->streamInfo().filterState()->hasDataWithName( + EXPECT_TRUE(decoder_->streamInfo().filterState()->hasDataGeneric( Network::ProxyProtocolFilterState::key())); // Clean up. diff --git a/test/common/http/conn_manager_impl_test_2.cc b/test/common/http/conn_manager_impl_test_2.cc index 5daf2b1a45e6a..051498ed3c22c 100644 --- a/test/common/http/conn_manager_impl_test_2.cc +++ b/test/common/http/conn_manager_impl_test_2.cc @@ -3762,7 +3762,7 @@ TEST_F(HttpConnectionManagerImplTest, NoProxyProtocolAdded) { startRequest(false); - EXPECT_FALSE(decoder_->streamInfo().filterState()->hasDataWithName( + EXPECT_FALSE(decoder_->streamInfo().filterState()->hasDataGeneric( Network::ProxyProtocolFilterState::key())); // Clean up. filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose);