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

quic: support in-place filterChainUpdate #13115

Closed
ggreenway opened this issue Sep 15, 2020 · 5 comments · Fixed by #17988
Closed

quic: support in-place filterChainUpdate #13115

ggreenway opened this issue Sep 15, 2020 · 5 comments · Fixed by #17988
Labels
area/quic help wanted Needs help! quic-mvp Required for QUIC MVP

Comments

@ggreenway
Copy link
Contributor

Need to add handling in udp case here:

void ConnectionHandlerImpl::addListener(absl::optional<uint64_t> overridden_listener,

@ggreenway ggreenway added area/quic quic-mvp Required for QUIC MVP labels Sep 15, 2020
@mattklein123 mattklein123 added the no stalebot Disables stalebot from closing an issue label Sep 15, 2020
@mattklein123 mattklein123 added help wanted Needs help! and removed no stalebot Disables stalebot from closing an issue labels Mar 8, 2021
@alyssawilk
Copy link
Contributor

lazy-ask: was this not accepting updates to the quic filter chain, or not being able to in-place add/remove quic listeners?

@ggreenway
Copy link
Contributor Author

The call to listener.second.tcpListener()->get().updateListenerConfig(config); is guarded by if (config.listenSocketFactory().socketType() == Network::Socket::Type::Stream) {. The ask is to do the same thing for datagram listeners.

@alyssawilk
Copy link
Contributor

OK, I think we need to either support this for alpha or make it really clear in docs it's not yet supported.

It shouldn't be too much work to forward packets for old connections to the old listener and new to the new listener, but it's not trivial.

@lambdai
Copy link
Contributor

lambdai commented Apr 6, 2021

The ownership of the filter chain is not clear to me: Quic listener need to decide whether the old and new config has filter chain only change.
Also, the per worker thread listener needs to track what connections are bind to the gone filter chain. AFAIK this is done in ActiveTcpListener, the quic listener needs a similar accounting.

@alyssawilk
Copy link
Contributor

Ok, I'm going to note lack of support in the docs PR, so removing this from the Alpha list.

alyssawilk pushed a commit that referenced this issue Sep 23, 2021
Commit Message: updateListenerConfig() and onFilterChainDraining() to ActiveListener interface and implement them in ActiveQuicListener. Unblock listener update for UDP listener.

Risk Level: high, touches LDS.
Testing: new integration tests
Fixes #13115
Signed-off-by: Dan Zhang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/quic help wanted Needs help! quic-mvp Required for QUIC MVP
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants