Add bind_addr for udp stream #2191
Conversation
|
ping @bluejekyll @djc, may you take a look? Thanks very much! |
There's no need to ping us immediately after filing a PR. |
| func: UdpCreator<S>, | ||
| ) -> Self { | ||
| let bind_address = match bind_addr { | ||
| Some(ba) => *ba, |
There was a problem hiding this comment.
Personally, I prefer more expressive names to shortening them like this, Some(bind_addr) => *bind_addr, is more readable IMO.
There was a problem hiding this comment.
Looking at the logic below, do we need to check that the bind_addr matches the family of the name_server? That is, if this is a v4 address, but the name_server is a v6 address, than this UDP socket will either fail, or never get a response, maybe it will fail quickly with a "no route to host", though I'm not sure off the top of my head.
There was a problem hiding this comment.
Good point, I will take a look what other(such as std lib) do on this problem.
|
@bluejekyll see discussion in #2190 on whether this is the right solution. |
That's a good discussion, let's finish that out. |
| @@ -36,6 +36,7 @@ where | |||
| MF: MessageFinalizer, | |||
| { | |||
| name_server: SocketAddr, | |||
| bind_addr: Option<SocketAddr>, | |||
There was a problem hiding this comment.
Based on the discussion in #2190, this should probably be IPaddr instead. I do think we should still consider how we want to deal with IPv4 vs IPv6...
The bind_addr is missing recently, see #2190