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.

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

Allow loadable modules #2053

Open
mattklein123 opened this issue Nov 11, 2017 · 21 comments
Open

Allow loadable modules #2053

mattklein123 opened this issue Nov 11, 2017 · 21 comments
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!

Comments

@mattklein123
Copy link
Member

In general, everywhere we use static registration (filters, stats sinks, loggers, tracers, etc.) it would not be difficult to also support dynamic loading. While some set of people want everything to be statically linked, others want dynamic loading flexibility. I have no objection to adding optional dynamic loading support if someone wants to take on this project.

@mattklein123 mattklein123 added enhancement Feature requests. Not bugs or questions. help wanted Needs help! labels Nov 11, 2017
@mattklein123 mattklein123 added this to the 1.6.0 milestone Dec 20, 2017
@mattklein123 mattklein123 modified the milestones: 1.6.0, 1.7.0 Mar 5, 2018
@mattklein123 mattklein123 removed this from the 1.7.0 milestone Apr 2, 2018
@glyn
Copy link

glyn commented May 22, 2018

Please can we define (either here or in a separate issue) in what circumstances it would be necessary to recompile loadable modules?

For example, envoy could apply semantic versioning:

  • loadable modules would need recompiling for distinct major versions of envoy,
  • loadable modules could be reused without recompilation between versions of envoy with the same major version.

@ggreenway
Copy link
Contributor

Without a large amount of work on ensuring binary compatibility of all the infrastructure that a module could depend on, my guess is that we would require modules to be compiled from the exact same source as the envoy that will load them.

@mattklein123
Copy link
Member Author

I agree with @ggreenway until #3390 is solved, which if I'm being realistic is probably 1-2 years out.

@glyn
Copy link

glyn commented May 22, 2018

Thanks. That's a clear definition. ;-)

@atrifan
Copy link
Contributor

atrifan commented Jun 19, 2019

I would think that you could have a generic http filter and network filter in which you specify what .so file to load and there you would call a predefined method - proxying the call to the dynamic .so file - passing as parameters - callback functions to be invoked for logging, setting meta variables or some other process in which you would do the logic in your dynamic .so file but logging and other envoy stuff would be delegated to the native implementation of the network / http filter.

Thoughts on this?

@mattklein123
Copy link
Member Author

Thoughts on this?

My preference is to actually support it more directly in the code at each extension point. Meaning, today we statically register all extension points and then search for them at load time by name. I think what we need to do is actually allow extensions to optionally be specified by an .so/.dll, and then just load that.

Shikugawa pushed a commit to Shikugawa/envoy that referenced this issue Mar 28, 2020
@davinci26
Copy link
Member

fyi @pravb

jpsim pushed a commit that referenced this issue Nov 28, 2022
Signed-off-by: Martin Conte Mac Donell <[email protected]>
Signed-off-by: JP Simard <[email protected]>
jpsim pushed a commit that referenced this issue Nov 29, 2022
Signed-off-by: Martin Conte Mac Donell <[email protected]>
Signed-off-by: JP Simard <[email protected]>
@cpakulski
Copy link
Contributor

I would like to go back to idea of dynamic loading of libraries. We have several extension mechanisms: Lua, wasm and golang. I would like to propose another one: very simple http filter which would load *.so file and look for C bindings. Those bindings would be basically equivalent to GRPC methods used in extern_proc: init, on req/resp headers/body/trailers.
The config specific to the loaded *.so would be delivered via xDS. The *.so library could be packaged along with Envoy in a container or xDS could be extended by binaryDS, which upon delivery to Envoy would load the *.so directly into memory or mount it into file system and then load. Envoy should provide to the library callbacks for local_reply, logging, metadata and stats.
My use case is requirement for high speed body validation/transformation and I would like to implement it in C++.
I can take on this work if there is an agreement.

@ggreenway
Copy link
Contributor

I don't think this would be too difficult as long as you accept that there's no ABI (or even API stability, see #3390), and thus all extensions must be compiled from the exact same source tree as the envoy they'll be run with, using the same compiler, same compiler settings, etc.

@mattklein123
Copy link
Member Author

yeah I'm fine with this also. My main request would be to factor out all of the SO loading config so it's shared among all users (you can take/move the config/code from the Go filter I think).

@kyessenov
Copy link
Contributor

What about Wasm ABI "null" version: https://github.com/proxy-wasm/proxy-wasm-cpp-sdk/blob/master/proxy_wasm_externs.h?

That would provide some stability, and should interop well with ext_proc, Rust, etc. AFAIK ext_proc can convert to proxy-wasm host. We'd certainly need to revise it before committing, there are some mistakes made in the first version.

@cpakulski
Copy link
Contributor

I don't think this would be too difficult as long as you accept that there's no ABI (or even API stability, see #3390), and thus all extensions must be compiled from the exact same source tree as the envoy they'll be run with, using the same compiler, same compiler settings, etc.

I understand that. I remember a case when a struct had #ifdef X statement and a library was compiled without X defined and main code was compiled with X defined and it took me ages to figure it out. You are right, structures has to be packed the same way and have to have the same sizes. But this becomes a user's problem to make sure that libraries are compatible.

My main request would be to factor out all of the SO loading config so it's shared among all users (you can take/move the config/code from the Go filter I think).

Agree. I looked at go L7 code and there is *so manager to share the library among multiple users.

What about Wasm ABI "null" version: https://github.com/proxy-wasm/proxy-wasm-cpp-sdk/blob/master/proxy_wasm_externs.h?

I was thinking about that, but heard that C++ may not compile nicely to wasm and also that there is some speed penalty. Need to investigate it more because I am not super familiar with null vm. Thanks for bringing it up.

@kyessenov
Copy link
Contributor

I was thinking about that, but heard that C++ may not compile nicely to wasm and also that there is some speed penalty. Need to investigate it more because I am not super familiar with null vm. Thanks for bringing it up.

The penalty is a general penalty for having a narrow and low-level interface. Unless you want some tighter integration with Envoy (e.g. stats system), the overhead is minimal and has to do with data serialization. C is great in that sense, it's painful to figure out C++ compilation artifacts (like packed strings, etc)

@kyessenov
Copy link
Contributor

kyessenov commented Feb 23, 2024

There's an open PR to add dynamically loaded Wasm ABI modules proxy-wasm/proxy-wasm-cpp-host#379. I like that approach since it brings the benefits of the decoupled build with less of a runtime hit. It's difficult to build a cross-language ABI without even taking Wasm into account - and the existing Envoy interfaces are C++-native - even for Rust it's non-trivial ownership/conversion implications.

Separately, I'm curious about the operational implications of the dlopen in k8s. I wonder if there's some accounting hit on the memory used caused by dlopen.

@cpakulski
Copy link
Contributor

Makes sense. I made a similar prototype to test idea of dynamically linking modules and it seems to work. The disadvantage is obviously increased memory requirements. For example, if the module uses json parser code, it must use it's own (linked with the module) as json parser linked with Envoy is not visible to such module.

@Protryon
Copy link

I wrote that dynamic NullVM model. What I do for the same use case in NGINX is link against the same libraries it's linking against (i.e. openssl) to reduce binary size and therefore memory. The same concept can apply here, you could import symbols from any library linked by Envoy provided you link against the same version -- dlopen will handle that case the same as importing symbols from Envoy/proxy-wasm-cpp-host.

@kyessenov
Copy link
Contributor

@Protryon does that require dynamic linking for the common shared library? Envoy is mostly statically linked right now. I was thinking you could use FFI ABI call to expose crypto, for example.

@htuch
Copy link
Member

htuch commented Feb 23, 2024

I think part of the goal here is to be writing dynamic modules against a stable, versioned ABI. FFI ABI to crypto violates that, since we can't make compatability guarantees for modules across Envoy releases. The hope was that Wasm ABI is this stable versioned ABI.

@kyessenov
Copy link
Contributor

@htuch Yeah, FFI is more of a loophole solution. It might be worth looking into the component model instead where some components are provided by Envoy natively.

@Protryon
Copy link

@kyessenov Yeah, it requires the component to be dynamically linked by both Envoy and the dynamic module.

As an alternative, the dynamic module can leave the shared library unlinked, and have unresolved symbols that Envoy must have or the module will fail to load.

In NGINX, since I rely on version-specific symbols, I'm recompiling for each version of NGINX supported. We are using Dynamic NullVM in production now, and just don't rely on any native symbols. Only Proxy-WASM defined ones, and we are calling system calls/doing custom threading work, so we haven't had to recompile for each version of Envoy.

@atrifan
Copy link
Contributor

atrifan commented Feb 24, 2024

I would like to go back to idea of dynamic loading of libraries. We have several extension mechanisms: Lua, wasm and golang. I would like to propose another one: very simple http filter which would load *.so file and look for C bindings. Those bindings would be basically equivalent to GRPC methods used in extern_proc: init, on req/resp headers/body/trailers. The config specific to the loaded *.so would be delivered via xDS. The *.so library could be packaged along with Envoy in a container or xDS could be extended by binaryDS, which upon delivery to Envoy would load the *.so directly into memory or mount it into file system and then load. Envoy should provide to the library callbacks for local_reply, logging, metadata and stats. My use case is requirement for high speed body validation/transformation and I would like to implement it in C++. I can take on this work if there is an agreement.

It's nice we are revisiting this 4 years later :)

mattklein123 pushed a commit that referenced this issue Aug 6, 2024
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 <[email protected]>
martinduke pushed a commit to martinduke/envoy that referenced this issue Aug 8, 2024
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.

envoyproxy#2053, envoyproxy#24230, envoyproxy#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 <[email protected]>
Signed-off-by: Martin Duke <[email protected]>
asingh-g pushed a commit to asingh-g/envoy that referenced this issue Aug 20, 2024
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.

envoyproxy#2053, envoyproxy#24230, envoyproxy#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 <[email protected]>
Signed-off-by: asingh-g <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!
Projects
None yet
Development

No branches or pull requests

9 participants