Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dynamic_modules: HTTP filter config implementation #37070

Merged
merged 16 commits into from
Nov 21, 2024
Merged

dynamic_modules: HTTP filter config implementation #37070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
b10dea9
dynamic_modules: HTTP filter config implementation
mathetake Nov 8, 2024
8f31852
more
mathetake Nov 8, 2024
8192a7b
more fix
mathetake Nov 8, 2024
509e200
try
mathetake Nov 8, 2024
5e0b677
try 2
mathetake Nov 8, 2024
523d623
simplifies
mathetake Nov 8, 2024
6bca4a7
simplifies
mathetake Nov 8, 2024
e110ee2
more consistent word
mathetake Nov 8, 2024
7ecd263
doc
mathetake Nov 8, 2024
f59880c
review: isolate 'safe' parts in entrypoints
mathetake Nov 18, 2024
2854092
Fixes asan failure
mathetake Nov 18, 2024
93ca63f
switch to OnceLock for simplicity
mathetake Nov 18, 2024
9aaebd3
clippy
mathetake Nov 18, 2024
2b5fe1d
disable tests only for tsan
mathetake Nov 19, 2024
403c7c3
typo
mathetake Nov 19, 2024
87503c9
clarify more why TSAN skipped
mathetake Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 70 additions & 7 deletions source/extensions/dynamic_modules/abi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,46 @@ extern "C" {
// -----------------------------------------------------------------------------
//
// Types used in the ABI. The name of a type must be prefixed with "envoy_dynamic_module_type_".
// Types with "_module_ptr" suffix are pointers owned by the module, i.e. memory space allocated by
// the module. Types with "_envoy_ptr" suffix are pointers owned by Envoy, i.e. memory space
// allocated by Envoy.

/**
* envoy_dynamic_module_type_abi_version represents a null-terminated string that contains the ABI
* version of the dynamic module. This is used to ensure that the dynamic module is built against
* the compatible version of the ABI.
* envoy_dynamic_module_type_abi_version_envoy_ptr represents a null-terminated string that
* contains the ABI version of the dynamic module. This is used to ensure that the dynamic module is
* built against the compatible version of the ABI.
*
* OWNERSHIP: Envoy owns the pointer.
*/
typedef const char* // NOLINT(modernize-use-using)
envoy_dynamic_module_type_abi_version_envoy_ptr;

/**
* envoy_dynamic_module_type_http_filter_config_envoy_ptr is a raw pointer to
* the DynamicModuleHttpFilterConfig class in Envoy. This is passed to the module when
* creating a new in-module HTTP filter configuration and used to access the HTTP filter-scoped
* information such as metadata, metrics, etc.
*
* This has 1:1 correspondence with envoy_dynamic_module_type_http_filter_config_module_ptr in
* the module.
*
* OWNERSHIP: Envoy owns the pointer.
*/
typedef const void* // NOLINT(modernize-use-using)
envoy_dynamic_module_type_http_filter_config_envoy_ptr;
Comment on lines +50 to +51
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand the purpose of passing void* here if the user has to have the actual C++ definition. Is this just to avoid any C++ leaking into the ABI itself?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a valid question. The users (modules) do not need to understand the C++ definition, but instead users can just call the corresponding ABI functions with this opaque pointers. For example, one notable example about filter-config level ABI function is "creating metrics". Then possible function signature for that ABI would look like

envoy_dynamic_module_type_counter_metric_envoy_ptr envoy_dynamic_module_define_counter_metric(
    envoy_dynamic_module_type_http_filter_config_envoy_ptr envoy_filter_config_ptr,
    const char* name,
    int name_length,
);

where the module->Envoy ABI function has this opaque pointer as a first argument. Then on the Envoy side, we can provide the functionality

envoy_dynamic_module_type_counter_metric_envoy_ptr envoy_dynamic_module_define_counter_metric(
    envoy_dynamic_module_type_http_filter_config_envoy_ptr envoy_filter_config_ptr,
    const char* name,
    int name_length,
) {
  const std::string_view name_view(static_cast<const char*>(name), name_length);
  auto scope = static_cast<DynamicModuleHttpFilterConfig*>(envoy_filter_config_ptr)->stat_scope_;
  Stats::StatNameManagedStorage storage(name_view, scope->symbolTable());
  Stats::StatName stat_name = storage.statName();
  Stats::Counter& counter = scope->counterFromStatName(stat_name);
  return &counter;
}

without leaking the implementation details of this envoy side "HttpFilterConfig" class into dynamic modules.

Apologies this is difficult to see the rationale behind this pointer passing without the concrete impl example like this. Later when I add these ABI functions, then we will be able to see the concrete picture more clearly. Hope this helps!


/**
* envoy_dynamic_module_type_http_filter_config_module_ptr is a pointer to an in-module HTTP
* configuration corresponding to an Envoy HTTP filter configuration. The config is responsible for
* creating a new HTTP filter that corresponds to each HTTP stream.
*
* This has 1:1 correspondence with the DynamicModuleHttpFilterConfig class in Envoy.
*
* OWNERSHIP: The module is responsible for managing the lifetime of the pointer. The pointer can be
* released when envoy_dynamic_module_on_http_filter_config_destroy is called for the same pointer.
*/
typedef const char* envoy_dynamic_module_type_abi_version; // NOLINT(modernize-use-using)
typedef const void* // NOLINT(modernize-use-using)
envoy_dynamic_module_type_http_filter_config_module_ptr;

// -----------------------------------------------------------------------------
// ------------------------------- Event Hooks ---------------------------------
Expand All @@ -54,10 +87,40 @@ typedef const char* envoy_dynamic_module_type_abi_version; // NOLINT(modernize-u
* to check compatibility and gracefully fail the initialization because there is no way to
* report an error to Envoy.
*
* @return envoy_dynamic_module_type_abi_version is the ABI version of the dynamic module. Null
* means the error and the module will be unloaded immediately.
* @return envoy_dynamic_module_type_abi_version_envoy_ptr is the ABI version of the dynamic
* module. Null means the error and the module will be unloaded immediately.
*/
envoy_dynamic_module_type_abi_version_envoy_ptr envoy_dynamic_module_on_program_init();

/**
* envoy_dynamic_module_on_http_filter_config_new is called by the main thread when the http
* filter config is loaded. The function returns a
* envoy_dynamic_module_type_http_filter_config_module_ptr for given name and config.
*
* @param filter_config_envoy_ptr is the pointer to the DynamicModuleHttpFilterConfig object for the
* corresponding config.
* @param name_ptr is the name of the filter.
* @param name_size is the size of the name.
* @param config_ptr is the configuration for the module.
* @param config_size is the size of the configuration.
* @return envoy_dynamic_module_type_http_filter_config_module_ptr is the pointer to the
* in-module HTTP filter configuration. Returning nullptr indicates a failure to initialize the
* module. When it fails, the filter configuration will be rejected.
*/
envoy_dynamic_module_type_http_filter_config_module_ptr
envoy_dynamic_module_on_http_filter_config_new(
envoy_dynamic_module_type_http_filter_config_envoy_ptr filter_config_envoy_ptr,
const char* name_ptr, int name_size, const char* config_ptr, int config_size);

/**
* envoy_dynamic_module_on_http_filter_config_destroy is called when the HTTP filter configuration
* is destroyed in Envoy. The module should release any resources associated with the corresponding
* in-module HTTP filter configuration.
* @param filter_config_ptr is a pointer to the in-module HTTP filter configuration whose
* corresponding Envoy HTTP filter configuration is being destroyed.
*/
envoy_dynamic_module_type_abi_version envoy_dynamic_module_on_program_init();
void envoy_dynamic_module_on_http_filter_config_destroy(
envoy_dynamic_module_type_http_filter_config_module_ptr filter_config_ptr);

#ifdef __cplusplus
}
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/dynamic_modules/abi_version.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace DynamicModules {
#endif
// This is the ABI version calculated as a sha256 hash of the ABI header files. When the ABI
// changes, this value must change, and the correctness of this value is checked by the test.
const char* kAbiVersion = "4293760426255b24c25b97a18d9fd31b4d1956f10ba0ff2f723580a46ee8fa21";
const char* kAbiVersion = "164a60ff214ca3cd62526ddb7c3fe21cf943e8721a115c87feca81a58510072c";

#ifdef __cplusplus
} // namespace DynamicModules
Expand Down
12 changes: 6 additions & 6 deletions source/extensions/dynamic_modules/dynamic_modules.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ namespace DynamicModules {

constexpr char DYNAMIC_MODULES_SEARCH_PATH[] = "ENVOY_DYNAMIC_MODULES_SEARCH_PATH";

absl::StatusOr<DynamicModuleSharedPtr> newDynamicModule(const absl::string_view object_file_path,
const bool do_not_close) {
absl::StatusOr<DynamicModulePtr> newDynamicModule(const absl::string_view object_file_path,
const bool do_not_close) {
// RTLD_LOCAL is always needed to avoid collisions between multiple modules.
// RTLD_LAZY is required for not only performance but also simply to load the module, otherwise
// dlopen results in Invalid argument.
Expand All @@ -33,15 +33,15 @@ absl::StatusOr<DynamicModuleSharedPtr> newDynamicModule(const absl::string_view
absl::StrCat("Failed to load dynamic module: ", object_file_path, " : ", dlerror()));
}

DynamicModuleSharedPtr dynamic_module = std::make_shared<DynamicModule>(handle);
DynamicModulePtr dynamic_module = std::make_unique<DynamicModule>(handle);

const auto init_function =
dynamic_module->getFunctionPointer<decltype(&envoy_dynamic_module_on_program_init)>(
"envoy_dynamic_module_on_program_init");

if (init_function == nullptr) {
return absl::InvalidArgumentError(
absl::StrCat("Failed to resolve envoy_dynamic_module_on_program_init: ", dlerror()));
"Failed to resolve symbol envoy_dynamic_module_on_program_init");
}

const char* abi_version = (*init_function)();
Expand All @@ -57,8 +57,8 @@ absl::StatusOr<DynamicModuleSharedPtr> newDynamicModule(const absl::string_view
return dynamic_module;
}

absl::StatusOr<DynamicModuleSharedPtr> newDynamicModuleByName(const absl::string_view module_name,
const bool do_not_close) {
absl::StatusOr<DynamicModulePtr> newDynamicModuleByName(const absl::string_view module_name,
const bool do_not_close) {
const char* module_search_path = getenv(DYNAMIC_MODULES_SEARCH_PATH);
if (module_search_path == nullptr) {
return absl::InvalidArgumentError(absl::StrCat("Failed to load dynamic module: ", module_name,
Expand Down
10 changes: 5 additions & 5 deletions source/extensions/dynamic_modules/dynamic_modules.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class DynamicModule {
void* handle_;
};

using DynamicModuleSharedPtr = std::shared_ptr<DynamicModule>;
using DynamicModulePtr = std::unique_ptr<DynamicModule>;

/**
* Creates a new DynamicModule. This is mainly exposed for testing purposes. Use
Expand All @@ -58,8 +58,8 @@ using DynamicModuleSharedPtr = std::shared_ptr<DynamicModule>;
* terminated. For example, c-shared objects compiled by Go doesn't support dlclose
* https://github.com/golang/go/issues/11100.
*/
absl::StatusOr<DynamicModuleSharedPtr> newDynamicModule(const absl::string_view object_file_path,
const bool do_not_close);
absl::StatusOr<DynamicModulePtr> newDynamicModule(const absl::string_view object_file_path,
const bool do_not_close);

/**
* Creates a new DynamicModule by name under the search path specified by the environment variable
Expand All @@ -70,8 +70,8 @@ absl::StatusOr<DynamicModuleSharedPtr> newDynamicModule(const absl::string_view
* will not be destroyed. This is useful when an object has some global state that should not be
* terminated.
*/
absl::StatusOr<DynamicModuleSharedPtr> newDynamicModuleByName(const absl::string_view module_name,
const bool do_not_close);
absl::StatusOr<DynamicModulePtr> newDynamicModuleByName(const absl::string_view module_name,
const bool do_not_close);

} // namespace DynamicModules
} // namespace Extensions
Expand Down
Loading