-
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): allow listening on ipv4 and ipv6 separately #4289
Conversation
simultaneously
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 @jxs!
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.
Also needs a changelog entry. Is there an easy way to test this? I think attempting to listen on the same port should do it right? We don't actually have to establish connections so it should be a fairly small test :)
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.
Also needs a changelog entry. Is there an easy way to test this? I think attempting to listen on the same port should do it right? We don't actually have to establish connections so it should be a fairly small test :)
Thanks, updated changelog and added a test ptal Thomas.
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.
A few more things, otherwise LGTM!
Thanks @jxs! :)
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.
Small suggestion. Otherwise looks good to me. Thank you @jxs.
@jxs The lockfile doesn't seem to be up-to-date. |
Thanks for the ping Thomas, updated! |
can we release 0.9.1-alpha @mxinden? I can then submit a PR to your |
Just created #4312.
No need. Will be included in the monorepo soon. See #4311. |
Following #4289 (comment), hereby is the PR to also improve the `create_socket` using [`for_addr`](https://docs.rs/socket2/latest/socket2/struct.Domain.html#method.for_address). We also add a test for listening on IPv4 and IPv6 separately. Pull-Request: #4328.
Description
Resolves #4165.
Notes & open questions
Should we also add a test for this?
Change checklist