From 572e432085ea4fa63087f4cf84ca2d00f78cc546 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Tue, 6 Aug 2024 15:15:58 -0700 Subject: [PATCH] dynamic_modules: adds initial object loading logic (#35550) Commit Message: dynamic_modules: adds initial object loading logic Additional Description: This is the very first commit of the dynamic loading feature discussed among community members. This is the effort to upstream the playground repository https://github.com/mathetake/envoy-dynamic-modules as an Envoy core extension. Series of commits will follow this little by little. #2053, #24230, #32502 Risk Level: N/A (not compiled into the final build yet) Testing: unit Docs Changes: N/A Release Notes: N/A Platform Specific Features: N/A [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Takeshi Yoneda Signed-off-by: Martin Duke --- CODEOWNERS | 5 +- source/extensions/dynamic_modules/BUILD | 22 ++++++ .../dynamic_modules/dynamic_modules.cc | 44 ++++++++++++ .../dynamic_modules/dynamic_modules.h | 65 ++++++++++++++++++ test/extensions/dynamic_modules/BUILD | 22 ++++++ .../dynamic_modules/dynamic_modules_test.cc | 68 +++++++++++++++++++ .../dynamic_modules/test_data/BUILD | 7 ++ .../dynamic_modules/test_data/no_op.c | 5 ++ .../dynamic_modules/test_data/test_data.bzl | 14 ++++ tools/spelling/spelling_dictionary.txt | 5 ++ 10 files changed, 255 insertions(+), 2 deletions(-) create mode 100644 source/extensions/dynamic_modules/BUILD create mode 100644 source/extensions/dynamic_modules/dynamic_modules.cc create mode 100644 source/extensions/dynamic_modules/dynamic_modules.h create mode 100644 test/extensions/dynamic_modules/BUILD create mode 100644 test/extensions/dynamic_modules/dynamic_modules_test.cc create mode 100644 test/extensions/dynamic_modules/test_data/BUILD create mode 100644 test/extensions/dynamic_modules/test_data/no_op.c create mode 100644 test/extensions/dynamic_modules/test_data/test_data.bzl diff --git a/CODEOWNERS b/CODEOWNERS index 68e00305fd217..d8f7be277fc21 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -338,6 +338,7 @@ extensions/filters/http/oauth2 @derekargueta @mattklein123 /*/extensions/health_checkers/grpc @zuercher @botengyao /*/extensions/health_checkers/http @zuercher @botengyao /*/extensions/health_checkers/tcp @zuercher @botengyao +/*/extensions/health_checkers/common @zuercher @botengyao # Health check event sinks /*/extensions/health_check/event_sinks/file @botengyao @yanavlasov # IP Geolocation @@ -349,8 +350,8 @@ extensions/filters/http/oauth2 @derekargueta @mattklein123 /*/extensions/filters/http/match_delegate @wbpcode @jstraceski @tyxia # Generic proxy and related extensions /*/extensions/filters/network/generic_proxy/ @wbpcode @soulxu - -/*/extensions/health_checkers/common @zuercher @botengyao +# Dynamic Modules +/*/extensions/dynamic_modules @mattklein123 @mathetake @marc-barry # HTTP credential injector /*/extensions/filters/http/credential_injector @zhaohuabing @kyessenov diff --git a/source/extensions/dynamic_modules/BUILD b/source/extensions/dynamic_modules/BUILD new file mode 100644 index 0000000000000..008e2ad84734c --- /dev/null +++ b/source/extensions/dynamic_modules/BUILD @@ -0,0 +1,22 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_extension_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_extension_package() + +envoy_cc_library( + name = "dynamic_modules_lib", + srcs = [ + "dynamic_modules.cc", + ], + hdrs = [ + "dynamic_modules.h", + ], + deps = [ + "//envoy/common:exception_lib", + ], +) diff --git a/source/extensions/dynamic_modules/dynamic_modules.cc b/source/extensions/dynamic_modules/dynamic_modules.cc new file mode 100644 index 0000000000000..e28917fac16f7 --- /dev/null +++ b/source/extensions/dynamic_modules/dynamic_modules.cc @@ -0,0 +1,44 @@ +#include "source/extensions/dynamic_modules/dynamic_modules.h" + +#include + +#include +#include + +#include "envoy/common/exception.h" + +namespace Envoy { +namespace Extensions { +namespace DynamicModules { + +absl::StatusOr 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. + int mode = RTLD_LOCAL | RTLD_LAZY; + if (do_not_close) { + mode |= RTLD_NODELETE; + } + + const std::filesystem::path file_path_absolute = std::filesystem::absolute(object_file_path); + void* handle = dlopen(file_path_absolute.c_str(), mode); + if (handle == nullptr) { + return absl::InvalidArgumentError( + absl::StrCat("Failed to load dynamic module: ", object_file_path, " : ", dlerror())); + } + return std::make_shared(handle); +} + +DynamicModule::~DynamicModule() { dlclose(handle_); } + +void* DynamicModule::getSymbol(const absl::string_view symbol_ref) const { + // TODO(mathetake): maybe we should accept null-terminated const char* instead of string_view to + // avoid unnecessary copy because it is likely that this is only called for a constant string, + // though this is not a performance critical path. + return dlsym(handle_, std::string(symbol_ref).c_str()); +} + +} // namespace DynamicModules +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/dynamic_modules/dynamic_modules.h b/source/extensions/dynamic_modules/dynamic_modules.h new file mode 100644 index 0000000000000..5e20f729cb2a8 --- /dev/null +++ b/source/extensions/dynamic_modules/dynamic_modules.h @@ -0,0 +1,65 @@ +#pragma once + +#include +#include + +#include "absl/status/statusor.h" +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Extensions { +namespace DynamicModules { + +/** + * A class for loading and managing dynamic modules. This corresponds to a single dlopen handle. + * When the DynamicModule object is destroyed, the dlopen handle is closed. + * + * This class is supposed to be initialized once in the main thread and can be shared with other + * threads. + */ +class DynamicModule { +public: + DynamicModule(void* handle) : handle_(handle) {} + ~DynamicModule(); + + /** + * Get a function pointer from the dynamic module with a specific type. + * @param T the function pointer type to cast the symbol to. + * @param symbol_ref the symbol to look up. + * @return the symbol if found, otherwise nullptr. + */ + template T getFunctionPointer(const absl::string_view symbol_ref) const { + static_assert(std::is_pointer::value && + std::is_function::type>::value, + "T must be a function pointer type"); + return reinterpret_cast(getSymbol(symbol_ref)); + } + +private: + /** + * Get a symbol from the dynamic module. + * @param symbol_ref the symbol to look up. + * @return the symbol if found, otherwise nullptr. + */ + void* getSymbol(const absl::string_view symbol_ref) const; + + // The raw dlopen handle that can be used to look up symbols. + void* handle_; +}; + +using DynamicModuleSharedPtr = std::shared_ptr; + +/** + * Creates a new DynamicModule. + * @param object_file_path the path to the object file to load. + * @param do_not_close if true, the dlopen will be called with RTLD_NODELETE, so the loaded object + * will not be destroyed. This is useful when an object has some global state that should not be + * terminated. For example, c-shared objects compiled by Go doesn't support dlclose + * https://github.com/golang/go/issues/11100. + */ +absl::StatusOr newDynamicModule(const absl::string_view object_file_path, + const bool do_not_close); + +} // namespace DynamicModules +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/dynamic_modules/BUILD b/test/extensions/dynamic_modules/BUILD new file mode 100644 index 0000000000000..8c831226c66a5 --- /dev/null +++ b/test/extensions/dynamic_modules/BUILD @@ -0,0 +1,22 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_cc_test( + name = "dynamic_modules_test", + srcs = ["dynamic_modules_test.cc"], + data = [ + "//test/extensions/dynamic_modules/test_data:no_op", + ], + deps = [ + "//source/extensions/dynamic_modules:dynamic_modules_lib", + "//test/test_common:environment_lib", + "//test/test_common:utility_lib", + ], +) diff --git a/test/extensions/dynamic_modules/dynamic_modules_test.cc b/test/extensions/dynamic_modules/dynamic_modules_test.cc new file mode 100644 index 0000000000000..6f5f21f5988f3 --- /dev/null +++ b/test/extensions/dynamic_modules/dynamic_modules_test.cc @@ -0,0 +1,68 @@ +#include + +#include "envoy/common/exception.h" + +#include "source/extensions/dynamic_modules/dynamic_modules.h" + +#include "test/test_common/environment.h" +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace DynamicModules { + +// This loads a shared object file from the test_data directory. +std::string testSharedObjectPath(std::string name) { + return TestEnvironment::substitute( + "{{ test_rundir }}/test/extensions/dynamic_modules/test_data/") + + "lib" + name + ".so"; +} + +TEST(DynamicModuleTest, InvalidPath) { + absl::StatusOr result = newDynamicModule("invalid_name", false); + EXPECT_FALSE(result.ok()); + EXPECT_EQ(result.status().code(), absl::StatusCode::kInvalidArgument); +} + +TEST(DynamicModuleTest, LoadNoOp) { + using GetSomeVariableFuncType = int (*)(); + absl::StatusOr module = + newDynamicModule(testSharedObjectPath("no_op"), false); + EXPECT_TRUE(module.ok()); + const auto getSomeVariable = + module->get()->getFunctionPointer("getSomeVariable"); + EXPECT_EQ(getSomeVariable(), 1); + EXPECT_EQ(getSomeVariable(), 2); + EXPECT_EQ(getSomeVariable(), 3); + + // Release the module, and reload it. + module->reset(); + module = + newDynamicModule(testSharedObjectPath("no_op"), true); // This time, do not close the module. + EXPECT_TRUE(module.ok()); + + // This module must be reloaded and the variable must be reset. + const auto getSomeVariable2 = + (module->get()->getFunctionPointer("getSomeVariable")); + EXPECT_NE(getSomeVariable2, nullptr); + EXPECT_EQ(getSomeVariable2(), 1); // Start from 1 again. + EXPECT_EQ(getSomeVariable2(), 2); + EXPECT_EQ(getSomeVariable2(), 3); + + // Release the module, and reload it. + module->reset(); + module = newDynamicModule(testSharedObjectPath("no_op"), false); + EXPECT_TRUE(module.ok()); + + // This module must be the already loaded one, and the variable must be kept. + const auto getSomeVariable3 = + module->get()->getFunctionPointer("getSomeVariable"); + EXPECT_NE(getSomeVariable3, nullptr); + EXPECT_EQ(getSomeVariable3(), 4); // Start from 4. +} + +} // namespace DynamicModules +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/dynamic_modules/test_data/BUILD b/test/extensions/dynamic_modules/test_data/BUILD new file mode 100644 index 0000000000000..f199f10d32189 --- /dev/null +++ b/test/extensions/dynamic_modules/test_data/BUILD @@ -0,0 +1,7 @@ +load("//test/extensions/dynamic_modules/test_data:test_data.bzl", "test_program") + +licenses(["notice"]) # Apache 2 + +package(default_visibility = ["//visibility:public"]) + +test_program(name = "no_op") diff --git a/test/extensions/dynamic_modules/test_data/no_op.c b/test/extensions/dynamic_modules/test_data/no_op.c new file mode 100644 index 0000000000000..81815e007da7f --- /dev/null +++ b/test/extensions/dynamic_modules/test_data/no_op.c @@ -0,0 +1,5 @@ +int getSomeVariable() { + static int some_variable = 0; + some_variable++; + return some_variable; +} diff --git a/test/extensions/dynamic_modules/test_data/test_data.bzl b/test/extensions/dynamic_modules/test_data/test_data.bzl new file mode 100644 index 0000000000000..fbe1af7f580ec --- /dev/null +++ b/test/extensions/dynamic_modules/test_data/test_data.bzl @@ -0,0 +1,14 @@ +load("@rules_cc//cc:defs.bzl", "cc_library") + +# This declares a cc_library target that is used to build a shared library. +# name + ".c" is the source file that is compiled to create the shared library. +def test_program(name): + cc_library( + name = name, + srcs = [name + ".c"], + linkopts = [ + "-shared", + "-fPIC", + ], + linkstatic = False, + ) diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index eff88c22e99e5..04d4f7e0f3a37 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -93,6 +93,7 @@ OWS Preconnecting RCVBUF RTCP +RTLD RTP SOH SPC @@ -306,6 +307,7 @@ NOAUTH NOCHECKRESP NODATA NODELAY +NODELETE NOLINT NOLINTNEXTLINE NONAME @@ -721,6 +723,9 @@ dgst dir dirname djb +dlclose +dlopen +dlsym downcalls downcasted downcased