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

UDP listener filter ECDS experiments---draft only #21606

Closed

Conversation

yanjunxiang-google
Copy link
Contributor

UDP listener filter ECDS experiments---draft only

This is continue for issue: #20049

UDP listener filter chain is created during listener creation. Thus ECDS may not work in UDP listener.

Created this draft PR so folks can discuss on it.

Signed-off-by: Yanjun Xiang [email protected]

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@yanjunxiang-google yanjunxiang-google marked this pull request as draft June 7, 2022 13:09
@yanjunxiang-google
Copy link
Contributor Author

This PR is for folks involved to discuss ECDS on UDP listener.
Some experiments shows the config update to UDP ECDS listener filter is ignored due to UDP listener filter chain is created during listener creation. Thus config update is not taken in actual traffic handling. This may mean ECDS may not applicable to UDP listener filter.

Any way, throw this PR out so folks can refer this when discussing.

@yanjunxiang-google
Copy link
Contributor Author

/wait

@soulxu
Copy link
Member

soulxu commented Jun 7, 2022

This PR is for folks involved to discuss ECDS on UDP listener. Some experiments shows the config update to UDP ECDS listener filter is ignored due to UDP listener filter chain is created during listener creation. Thus config update is not taken in actual traffic handling. This may mean ECDS may not applicable to UDP listener filter.

Any way, throw this PR out so folks can refer this when discussing.

How about to recreate the udp listener filter when the ECDS updates the config?

Signed-off-by: Yanjun Xiang <[email protected]>
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Jul 23, 2022
@github-actions
Copy link

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot closed this Jul 30, 2022
@yanjunxiang-google
Copy link
Contributor Author

This PR is for folks involved to discuss ECDS on UDP listener. Some experiments shows the config update to UDP ECDS listener filter is ignored due to UDP listener filter chain is created during listener creation. Thus config update is not taken in actual traffic handling. This may mean ECDS may not applicable to UDP listener filter.
Any way, throw this PR out so folks can refer this when discussing.

How about to recreate the udp listener filter when the ECDS updates the config?

Currently UDP listener filter chain is created during Envoy instance creation:

config_->filterChainFactory().createUdpListenerFilterChain(*this, *this);

If we recreated the UDP listener filter chian whenever ECDS updates received, which could be frequently, could this cause data loss? For example, when the UDP listener filters are processing live traffic, and ECDS updates received, what should Envoy do with that traffic?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale stalebot believes this issue/PR has not been touched recently
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants