Skip to content

Add functionality to set downstream socket option from L7 filter callback #39200

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

Jump to bottom
Open
wants to merge 40 commits into
base: main
Choose a base branch
from
Open

Add functionality to set downstream socket option from L7 filter callback #39200

wants to merge 40 commits into from

Conversation

jtlisi
Copy link

@jtlisi jtlisi commented Apr 22, 2025 •
edited
Loading

Commit Message: Add functionality to set downstream socket option from L7 filter callback.
Additional Description:
Risk Level: Low (Optional feature)
Testing: Unit tests
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: No

@repokitteh-read-only RepoKitteh - Read Only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #39200 was opened by jtlisi.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added api deps Approval required for changes to Envoy's external dependencies labels Apr 24, 2025
@repokitteh-read-only RepoKitteh - Read Only
Copy link

CC @envoyproxy/coverage-shephards: FYI only for changes made to (test/per_file_coverage.sh).
envoyproxy/coverage-shephards assignee is @RyanTheOptimist
CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @abeyad
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).
CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @mattklein123

🐱

Caused by: #39200 was synchronize by jtlisi.

see: more, trace.

@jtlisi jtlisi force-pushed the addDownstreamSocketOptions branch 2 times, most recently from 3c29939 to 532b938 Compare April 25, 2025 14:29
@jtlisi jtlisi changed the title Add setDownstreamSocketOption function to StreamDecoderFilterCallba… Add functionality to set downstream socket option from L7 filter callback Apr 25, 2025
@jtlisi jtlisi marked this pull request as ready for review April 25, 2025 17:03
@jtlisi jtlisi marked this pull request as draft April 25, 2025 17:08
@jtlisi
Copy link
Author

jtlisi commented Apr 25, 2025

/retest

@jtlisi jtlisi force-pushed the addDownstreamSocketOptions branch from ee4ba56 to 93e5e27 Compare April 28, 2025 14:39
jtlisi added 13 commits April 28, 2025 14:01
@jtlisi
Add setDownstreamSocketOption function to StreamDecoderFilterCallba…
a5088d7 
…cks interface.

Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
add additional socket apis
278072a 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
fixup function names
1dbc8be 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
Add socket option function to every needed class
9fde3b8 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
Update interface function comments
9ee2424 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
Add basic unit test skeleton
d95e8e4 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
Fixup test
fdae6f1 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
Restrict usage to DownstreamFilterManager.
6c895db 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
cleanup
928a888 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
cleanup
06c222e 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
Kick CI
26d7321 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
Remove unused parameter
c7f3b78 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
update mocks
20df375 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi jtlisi force-pushed the addDownstreamSocketOptions branch from 26a2f6e to 20df375 Compare April 28, 2025 18:05
jtlisi added 2 commits April 28, 2025 14:28
@jtlisi
fix quic socket
abf8383 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
clang-format
55c700f 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
Merge branch 'main' into addDownstreamSocketOptions
76ee6c3 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi jtlisi requested a review from paul-r-gall May 15, 2025 19:19
paul-r-gall
paul-r-gall approved these changes May 15, 2025
View reviewed changes
Copy link
Contributor

@paul-r-gall paul-r-gall left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks!

@paul-r-gall
Copy link
Contributor

@abeyad wdyt from an API view?

jtlisi added 2 commits May 16, 2025 11:12
@jtlisi
Merge remote-tracking branch 'upstream/main' into addDownstreamSocket…
837afb3 
…Options

Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
update socket option from BOUND to LISTENING
cf4e900 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@paul-r-gall
Copy link
Contributor

Assigning to Greg for non-google senior maintainer + socket expertise. Greg, if you are overloaded, please let me know and I can assign to someone else.

/assign @ggreenway

ggreenway
ggreenway requested changes May 20, 2025
View reviewed changes
Copy link
Member

@ggreenway ggreenway left a comment

Choose a reason for hiding this comment

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

/wait

@ggreenway ggreenway removed deps Approval required for changes to Envoy's external dependencies api labels May 20, 2025
@jtlisi
address review feedback
61dba5a 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi jtlisi requested a review from ggreenway May 20, 2025 19:17
ggreenway
ggreenway requested changes May 21, 2025
View reviewed changes
Copy link
Member

@ggreenway ggreenway left a comment

Choose a reason for hiding this comment

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

/wait

jtlisi added 2 commits May 22, 2025 12:06
@jtlisi
Merge remote-tracking branch 'upstream/main' into addDownstreamSocket…
ccb1a4c 
…Options

Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
address review feedback and remove unnecessary interfaces
3e4c7e7 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
jtlisi added 6 commits May 22, 2025 13:27
@jtlisi
address review feedback
90fe00c 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
simplify implementation to use socket option lib
6642e61 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
formatting fix
815b060 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
formatting fix
b1f9373 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
update interface
cb25417 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
update changelog
30c3d31 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi jtlisi requested a review from ggreenway May 23, 2025 14:03
jtlisi added 2 commits May 23, 2025 10:04
@jtlisi
formatting fix
6363535 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
@jtlisi
formatting fix
11e8a6d 
Signed-off-by: Jacob Lisi <jtlisi@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paul-r-gall paul-r-gall

@ggreenway ggreenway

Requested changes must be addressed to merge this pull request.

Assignees

@abeyad abeyad

@ggreenway ggreenway

@paul-r-gall paul-r-gall

Labels
api
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jtlisi @RyanTheOptimist @jmarantz @paul-r-gall @ggreenway @abeyad @mattklein123