dynamic_modules: add dynamic modules support for Network filters #42605
Conversation
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
agrawroh
force-pushed
the
feat-dm-ntwrk
branch
5 times, most recently
from
7f04216 to
9dc4adc
Compare
agrawroh
changed the title
agrawroh
force-pushed
the
feat-dm-ntwrk
branch
2 times, most recently
from
f5ee101 to
fb3bdd9
Compare
Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
agrawroh
force-pushed
the
feat-dm-ntwrk
branch
from
fb3bdd9 to
3f09fad
Compare
Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
agrawroh
force-pushed
the
feat-dm-ntwrk
branch
from
91df618 to
ec3d5a3
Compare
agrawroh
force-pushed
the
feat-dm-ntwrk
branch
from
ec3d5a3 to
a0d786e
Compare
|
/lgtm api |
|
I have quick check to the ABI, LGTM. Thanks for this great change. |
| * @param size is the pointer to the variable where the number of slices will be stored. | ||
| * @return true if the buffer is available, false otherwise. | ||
| */ | ||
| bool envoy_dynamic_module_callback_network_filter_get_read_buffer_slices_size( |
There was a problem hiding this comment.
i think we decided to stick with "chunks" for the buffer vector, so let's use it here too for consistency
| bool envoy_dynamic_module_callback_network_filter_get_read_buffer_slices_size( | |
| bool envoy_dynamic_module_callback_network_filter_get_read_buffer_chunks_size( |
| if (buffer == nullptr) { | ||
| return false; | ||
| } | ||
| *size = buffer->getRawSlices(std::nullopt).size(); |
There was a problem hiding this comment.
this is not a comment for this PR but the ABI in general including HTTP: i think we should create a fast pass to return the vector size as well as directly store the chunks into the vector allocated in a dynamic module so that we could avoid getRawSlices being called twice -- i think it shouldn't be that hard to make change to the core
mathetake
left a comment
mathetake
left a comment
There was a problem hiding this comment.
I am boarding the flight in a few minutes but looks good!! This is really exciting!
Thanks, @mathetake! I'll go ahead and merge this and open a new PR to address the additional feedback you left. Thanks again for the reviews :) |
4 participants
Description
This PR adds support for a new Network filter for Dynamic Modules. With this it should be possible to write new Network Filters similar to HTTP Filters in Go, RUST, etc.
Commit Message: dynamic_modules: add dynamic modules support for Network filters
Additional Description: Adds support for a new Network filter for Dynamic Modules.
Risk Level: Low
Testing: Added Unit + Integration Tests
Docs Changes: Added
Release Notes: Added