-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
listener: enable UdpListenerWorkerRouter to support multiple addresses #21940
listener: enable UdpListenerWorkerRouter to support multiple addresses #21940
Conversation
Signed-off-by: He Jie Xu <[email protected]>
Signed-off-by: He Jie Xu <[email protected]>
Signed-off-by: He Jie Xu <[email protected]>
Signed-off-by: He Jie Xu <[email protected]>
Signed-off-by: He Jie Xu <[email protected]>
/assign @mattklein123 |
Signed-off-by: He Jie Xu <[email protected]>
Signed-off-by: He Jie Xu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with small comment, thank you.
/wait
source/server/listener_impl.h
Outdated
@@ -494,6 +499,7 @@ class ListenerImpl final : public Network::ListenerConfig, | |||
|
|||
Quic::QuicStatNames& quic_stat_names_; | |||
MissingListenerConfigStats missing_listener_config_stats_; | |||
uint32_t concurrency_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: const. As an aside it would be nice to not have to store this in every listener, but there probably isn't a great way to do that without passing through all server options so I guess it's ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I can get that from the server instance directly, let me try, also thanks point this out, it indeed better to not pass through it, and it doesn't feel like something the ListenerImpl
need to hold one.
Signed-off-by: He Jie Xu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Commit Message: listener: enable UdpWorkerRouter to support multiple addresses
Additional Description:
This PR enable the
ListenerImpl
to createUdpListenerWorkerRouter
for each address. And pass the correctUdpListenerWorkerRouter
to theActiveUdpListener
.Risk Level: high
Testing: unittest
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a
Part of #11184