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

upstream: make upstream_bind_config work in udp upstream #30970

Merged

Conversation

jeongseokson
Copy link
Contributor

Commit Message:
The existing implementation of UDP upstream did not make use of the upstream_bind_config. I modified the UDP Upstream code to check whether the bind config is specified and set the local address and socket option accordingly.
Additional Description:
Risk Level: Low, this is a bug fix.
Testing: Added unit tests covering added code.
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

The existing implementation of UDP upstream did not make use of the
upstream_bind_config. I modified the UDP Upstream code to check whether
the bind config is specified and set the local address and socket option
accordingly.

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>
@jeongseokson
Copy link
Contributor Author

Hi Ryan, can I ask you to review the code since you reviewed the previous connect-udp PRs? Thank you.
/assign @RyanTheOptimist

@jeongseokson
Copy link
Contributor Author

/wait on CI

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>
@jeongseokson
Copy link
Contributor Author

/retest

Signed-off-by: Jeongseok Son <jeongseok.son@gmail.com>
Copy link
Contributor

@RyanTheOptimist RyanTheOptimist left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

@RyanTheOptimist
Copy link
Contributor

/assign @lizan
for cross-company review

@jeongseokson
Copy link
Contributor Author

Thanks all for reviewing the PR! Can any of you initiate the merge?

@RyanTheOptimist RyanTheOptimist merged commit 2de016d into envoyproxy:main Nov 22, 2023
105 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants