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

Allow REUSEPORT for listener sockets #8794

Closed
mattklein123 opened this issue Oct 29, 2019 · 8 comments · Fixed by #8884
Closed

Allow REUSEPORT for listener sockets #8794

mattklein123 opened this issue Oct 29, 2019 · 8 comments · Fixed by #8884
Assignees
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!
Milestone

Comments

@mattklein123
Copy link
Member

We should optionally allow listener sockets to be created on each worker with SO_REUSEPORT. This should probably be the default for UDP listeners and opt-in for TCP listeners?

cc @danzh2010 @euroelessar @antoniovicente

@mattklein123 mattklein123 added enhancement Feature requests. Not bugs or questions. help wanted Needs help! labels Oct 29, 2019
@mattklein123 mattklein123 self-assigned this Oct 29, 2019
@mattklein123 mattklein123 added this to the 1.13.0 milestone Oct 29, 2019
@danzh2010
Copy link
Contributor

Hi Matt, actually this is part of the plan in https://docs.google.com/document/d/1My-Tssc9d6rW-Xu96y4DdUxvzIKmptP6mm6i_zOXr58/edit?pli=1#. I was about to modify ListenerConfig to provide a listen socket factory interface instead of a socket as the first step of preparation for supporting multiple quic listeners on different worker threads.

What's your plan for this?

@mattklein123
Copy link
Member Author

What's your plan for this?

To assign this issue to you. ;)

We can use this for tracking.

@danzh2010
Copy link
Contributor

I can work on this. But one question here, is there any reason why this even needs to be an option for TCP listener? Do we want different TCP listeners on different worker threads to listen on different sockets?

@mattklein123
Copy link
Member Author

But one question here, is there any reason why this even needs to be an option for TCP listener? Do we want different TCP listeners on different worker threads to listen on different sockets?

It has been requested previously that we support this for TCP listeners also. In some cases this can lead to better performance in very high CPS use cases with many listening cores. IMO you don't need to implement it for TCP if you don't want to, but it would be nice to do it in such a way that someone could come along and do it for TCP later if they want?

@l8huang
Copy link
Contributor

l8huang commented Oct 30, 2019

@danzh2010 Considering this issue assigned to you, what's your plane? :) Are you working on both UDP and TCP change? Or only UDP?

I recently did some LnP tests for HTTP traffic, one finding is: usually Envoy consumes <24 CPUs and occasionally Envoy consumes 46 CPUs(the node has 32 cores with Hyper-thread, that's 64 CPUs). Envoy run with 64 worker threads.

Making this change for TCP is in my task list, just want to confirm if we have any overlapping ;) I'm willing to take the TCP part.

@danzh2010
Copy link
Contributor

Currently the one and only socket is passed to each listener on workder threads through Network:ListenerConfig. My plan is to change ListenerConfig to carry a socket factory instead of a socket. And create the socket during listener creation via the factory. UDP and TCP factory can have different socket management behavior.

As the first step, I will just refactor the code to pass around socket factory. TCP socket factory always provide the same socket, but UDP socket factory creates a new socket for each listener.

@l8huang
Copy link
Contributor

l8huang commented Nov 5, 2019

@danzh2010 I did a quick change to make SO_REUSEPORT works for TCP listeners, and run a load test, connections distributed to worker threads every well(checked downstream_cx_total and downstream_cx_active in /stats output).

A draft PR #8884 created, if you have time to do a quick view, that could be every helpful, just want to confirm this change is on correct direction.

@danzh2010
Copy link
Contributor

I just found that SO_REUSEPORT doesn't behave as expected in Mac OS. Mac doesn't dispatch UDP packets according to network 5 tuple within the SO_REUSEPORT socket group. For connection-oriented UDP traffic, this means we don't have stable connection as packets from the same connection can be sent to different sockets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!
Projects
None yet
3 participants