Allow to lookup the initial local IP address #943
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ralith
requested changes
Dec 21, 2020
Matthias247
force-pushed
the
dest_ip
branch
4 times, most recently
from
7b8bc69 to
be481ab
Compare
Ralith
previously approved these changes
Dec 22, 2020
I added some minimal coverage by reading the field and checking that it's available on Linux, and points to the loopback client. I think this covers the most important part (reading the cmsgs). Everything else is pretty much just plumbing. |
Ralith
previously approved these changes
Dec 22, 2020
djc
reviewed
Dec 27, 2020
quinn-proto/src/connection/mod.rs
Outdated
| /// This can be different from the address the endpoint is bound to, in case | ||
| /// the endpoint is bound to a wildcard address like `0.0.0.0` or `::`. | ||
| /// | ||
| /// This will return `None` for clients, as well for servers if capturing |
There was a problem hiding this comment.
as well for seems a bit odd, add an extra as? I also think this docstring would be more useful if it explicitly lists supported platforms.
There was a problem hiding this comment.
Alternatively, and might be a bit more concise.
There was a problem hiding this comment.
I went for listing the supported platform[s]
Comment on lines
+380
to
+389
| if cfg!(target_os = "linux") { | ||
| let local_ip = incoming.local_ip().expect("Local IP must be available"); | ||
| assert!(local_ip.is_loopback()); | ||
| } else { | ||
| assert_eq!(None, incoming.local_ip()); | ||
| } |
There was a problem hiding this comment.
Add a note here to update the getter docs in case the platform support falls out of sync?
There was a problem hiding this comment.
Here, or rather to the platform files? This might be the place where people realize that more changes are necessary - because CI fails. But I somehow don't think that "update doc comments" is best kept in the tests. In the platform files - maybe. But if someone works on adding it to windows they won't look at the unix file.
There was a problem hiding this comment.
I mean, it could be both, but as you say, I'm not sure someone adding support for a different platform would review the code for unix. So I think adding it here would definitely be useful, maybe do the platform files as well (I didn't see a straightforward single location where it could be added, but I could easily be wrong about that).
Ralith
previously approved these changes
Dec 28, 2020
When a listener is bound to multiple network interfaces (e.g. `::0`), it is not obvious which IP the peer used to send a packet. We however might need this information to send packets back to the peer with the same source address. This problem is described in quinn-rs#508. This change makes the destination IP address which was used to send the initial packet available in the `Conneting` and `Connection` types. The information is far available only on Linux due to missing test on other platforms.
djc
approved these changes
Dec 29, 2020
|
Thanks for working through this! |
Matthias247
pushed a commit
to Matthias247/quinn
that referenced
this pull request
Jan 4, 2021
This is a follow-up for quinn-rs#943. When a socket is bound to a wildcard IP address, sending the outgoing IP might use a different source IP address than the one the packet was received on, since the OS might not be able to identify the necessary route. This would lead packets not allowing to reach the client. This change adds a setting which will set an explicit source address in all outgoing packets. The source address which will be used is the local IP address which was used to receive the initial incoming packet.
Matthias247
pushed a commit
to Matthias247/quinn
that referenced
this pull request
Jan 4, 2021
This is a follow-up for quinn-rs#943. When a socket is bound to a wildcard IP address, sending the outgoing IP might use a different source IP address than the one the packet was received on, since the OS might not be able to identify the necessary route. This would lead packets not allowing to reach the client. This change adds a setting which will set an explicit source address in all outgoing packets. The source address which will be used is the local IP address which was used to receive the initial incoming packet.
Matthias247
pushed a commit
to Matthias247/quinn
that referenced
this pull request
Jan 4, 2021
This is a follow-up for quinn-rs#943. When a socket is bound to a wildcard IP address, sending the outgoing IP might use a different source IP address than the one the packet was received on, since the OS might not be able to identify the necessary route. This would lead packets not allowing to reach the client. This change adds a setting which will set an explicit source address in all outgoing packets. The source address which will be used is the local IP address which was used to receive the initial incoming packet.
djc
pushed a commit
to Matthias247/quinn
that referenced
this pull request
Jan 5, 2021
This is a follow-up for quinn-rs#943. When a socket is bound to a wildcard IP address, sending the outgoing IP might use a different source IP address than the one the packet was received on, since the OS might not be able to identify the necessary route. This would lead packets not allowing to reach the client. This change adds a setting which will set an explicit source address in all outgoing packets. The source address which will be used is the local IP address which was used to receive the initial incoming packet.
djc
pushed a commit
to Matthias247/quinn
that referenced
this pull request
Jan 5, 2021
This is a follow-up for quinn-rs#943. When a socket is bound to a wildcard IP address, sending the outgoing IP might use a different source IP address than the one the packet was received on, since the OS might not be able to identify the necessary route. This would lead packets not allowing to reach the client. This change adds a setting which will set an explicit source address in all outgoing packets. The source address which will be used is the local IP address which was used to receive the initial incoming packet.
Matthias247
pushed a commit
to Matthias247/quinn
that referenced
this pull request
Jan 6, 2021
This is a follow-up for quinn-rs#943. When a socket is bound to a wildcard IP address, sending the outgoing IP might use a different source IP address than the one the packet was received on, since the OS might not be able to identify the necessary route. This would lead packets not allowing to reach the client. This change adds a setting which will set an explicit source address in all outgoing packets. The source address which will be used is the local IP address which was used to receive the initial incoming packet.
Matthias247
pushed a commit
to Matthias247/quinn
that referenced
this pull request
Jan 6, 2021
This is a follow-up for quinn-rs#943. When a socket is bound to a wildcard IP address, sending the outgoing IP might use a different source IP address than the one the packet was received on, since the OS might not be able to identify the necessary route. This would lead packets not allowing to reach the client. This change adds a setting which will set an explicit source address in all outgoing packets. The source address which will be used is the local IP address which was used to receive the initial incoming packet.
djc
pushed a commit
that referenced
this pull request
Jan 6, 2021
This is a follow-up for #943. When a socket is bound to a wildcard IP address, sending the outgoing IP might use a different source IP address than the one the packet was received on, since the OS might not be able to identify the necessary route. This would lead packets not allowing to reach the client. This change adds a setting which will set an explicit source address in all outgoing packets. The source address which will be used is the local IP address which was used to receive the initial incoming packet.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a listener is bound to multiple network interfaces (e.g.
::0),it is not obvious which IP the peer used to send a packet. We however
might need this information to send packets back to the peer with the
same source address.
This problem is described in #508.
This change makes the destination IP address which was used to send
the initial packet available in the
ConnetingandConnectiontypes.The information is far available only on Linux due to missing test on
other platforms.