-
Notifications
You must be signed in to change notification settings - Fork 999
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
fix(quic): add support for reusing an existing socket for local dialing #4304
Conversation
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.
Neat! Thank you @arsenron for the work.
Would you mind adding a unit test as well? This should be relatively straight forward. Given two nodes A and B:
- Have A listen on
0.0.0.0:0
. - Given the wildcard port (
:0
) store the concrete port chosen by the OS across all interfaces. - Have B listen on
127.0.0.1:0
. - Have A dial B.
- Expect the source port of A at B to be the port stored in (2), i.e. to be A's listen port.
This pull request has merge conflicts. Could you please resolve them @arsenron? 🙏 |
ddb17c6
to
aee6ddb
Compare
Done. |
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.
Wow, that was fast. Thank you! Two suggestions.
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.
Thank you for adjusting the test.
@mxinden Rewrote using a |
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.
Well done @arsenron! Thank you for the help.
Description
Tracked in #4259. Now if a listener supports loopback interfaces, when a remote address is also a loopback address, we reuse an existing listener.
Notes & open questions
Change checklist