-
Notifications
You must be signed in to change notification settings - Fork 550
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
Expose service to both IPv4 and IPv6 #1307
base: dev
Are you sure you want to change the base?
Conversation
Just so it's clear, what patforms did you test? |
"Technically", I guess this would fail on platforms where the IPv6 stack doesn't exist (i.e. if you compile the kernel by explicitly disabling IPv6, which I guess nobody does unless you want to torture yourself). But I guess the number of people doing that is way smaller that the people that want/need IPv6 (i.e. those using NAT64, etc..) |
Looks good to me. Can you add an entry to the changelog? Then we can merge. |
This doesn't work for IPv4 on Windows :( Dual-stack sockets aren't the default. Perhaps best to use |
Looks like you're right: rust-lang/rfcs#957
That's really interesting actually
|
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.
Please refer to #1307 (comment) and add an entry to the changelog. Thanks!
@@ -251,7 +252,7 @@ pub struct DiscoveryServer { | |||
impl DiscoveryServer { | |||
pub fn new(config: Config, port: &mut u16) -> Result<Self, Error> { | |||
let (discovery, cred_rx) = RequestHandler::new(config); | |||
let address = SocketAddr::new(Ipv4Addr::UNSPECIFIED.into(), *port); | |||
let address = SocketAddr::new(IpAddr::from_str("::0").unwrap(), *port); |
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.
Please refer to: #1307 (comment)
Change to listen to [::0] given this will make the application listen on both ipv4 and ipv6. This is useful to enable support for ipv6 only networks.
Given most users will be on v4 only or dual stacked networks it is of course important to make sure that works, in my testing i have not found any case where this change created a problem for those devices.