address_impl: Return an Ipv4Instance for v4-mapped v6 addresses.#2309
Merged
zuercher merged 4 commits intoenvoyproxy:masterfrom Jan 5, 2018
Merged
address_impl: Return an Ipv4Instance for v4-mapped v6 addresses.#2309zuercher merged 4 commits intoenvoyproxy:masterfrom
zuercher merged 4 commits intoenvoyproxy:masterfrom
Conversation
Recognize v6-mapped v4 addresses and use IPv4 address instances to represent them so that rest of Envoy can treat v4 connections the same regardless if they were received from a v6 or a v4 socket. Signed-off-by: Jarno Rajahalme <[email protected]>
mattklein123
reviewed
Jan 4, 2018
Member
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
@jrajahalme seems reasonable to me and thanks for the change, but will need a test. Thank you.
htuch
reviewed
Jan 4, 2018
Member
htuch
left a comment
htuch
left a comment
There was a problem hiding this comment.
LGTM modulo tests. This is neat and has some benefits (e.g. logging). Thanks.
jrajahalme
force-pushed
the
transparent-v4-compatible-v6
branch
from
b404f30 to
06e0b1d
Compare
Contributor
Author
|
OK, thanks! Working on OSX fix for missing s6_addr32 and tests :-) |
Check that addressFromSockAddr() returns an Ipv4Instance for an IPv4-mapped IPv6 address. Signed-off-by: Jarno Rajahalme <[email protected]>
Contributor
Author
|
Sorry for the forced change on the first commit, added OSX compat macros there you may want to check out. |
Member
|
LGTM. @zuercher can you take a look at the OS X handling here? |
Signed-off-by: Jarno Rajahalme <[email protected]>
zuercher
reviewed
Jan 4, 2018
| return std::make_shared<Address::Ipv6Instance>(*sin6, v6only); | ||
| if (!v6only && IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr)) { | ||
| #ifndef s6_addr32 | ||
| #if defined(__APPLE__) && defined(__MACH__) |
Member
There was a problem hiding this comment.
We've left it as just #ifdef __APPLE__ elsewhere. I don't think this is actually mach-specific in any event, as it was copied from BSD.
Contributor
Author
There was a problem hiding this comment.
Changed as requested, thanks for the review!
Checking for __APPLE__ only is sufficient. Signed-off-by: Jarno Rajahalme <[email protected]>
htuch
approved these changes
Jan 5, 2018
mattklein123
approved these changes
Jan 5, 2018
zuercher
approved these changes
Jan 5, 2018
jrajahalme
added a commit
to jrajahalme/envoy
that referenced
this pull request
Feb 10, 2018
Currently the local address of a new connection is mapped to an IPv4 address instance if the listening socket is not in V6ONLY mode and the local address of the socket is an IPv4-mapped IPv6 address. This leads to connections having an (IPv4-mapped) IPv6 instance for the remote address and an IPv4 instance for the local address, which is confusing. Fix this by allowing the remote address to be v4-mapped if the local address is an IPv4 address. Or conversely, set the 'v6only' parameter to 'true' if the local address is an IPv6 address instance. Fixes: 3b3c009 ("address_impl: Return an Ipv4Instance for v4-mapped v6 addresses. (envoyproxy#2309)") Signed-off-by: Jarno Rajahalme <[email protected]>
htuch
pushed a commit
that referenced
this pull request
Feb 15, 2018
Currently the local address of a new connection is mapped to an IPv4 address instance if the listening socket is not in V6ONLY mode and the local address of the socket is an IPv4-mapped IPv6 address. This leads to connections having an (IPv4-mapped) IPv6 instance for the remote address and an IPv4 instance for the local address, which is confusing. Fix this by allowing the remote address to be v4-mapped if the local address is an IPv4 address. Or conversely, set the 'v6only' parameter to 'true' if the local address is an IPv6 address instance. Fixes: 3b3c009 ("address_impl: Return an Ipv4Instance for v4-mapped v6 addresses. (#2309)") Signed-off-by: Jarno Rajahalme [email protected] Risk Level: Low
Shikugawa
pushed a commit
to Shikugawa/envoy
that referenced
this pull request
Mar 28, 2020
jpsim
added a commit
that referenced
this pull request
Nov 28, 2022
Signed-off-by: GitHub Action <[email protected]> Co-authored-by: jpsim <[email protected]> Signed-off-by: JP Simard <[email protected]>
jpsim
added a commit
that referenced
this pull request
Nov 29, 2022
Signed-off-by: GitHub Action <[email protected]> Co-authored-by: jpsim <[email protected]> Signed-off-by: JP Simard <[email protected]>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Recognize v6-mapped v4 addresses and use IPv4 address
instances to represent them so that rest of Envoy can treat v4 connections
the same regardless if they were received from a v6 or a v4 socket.
Signed-off-by: Jarno Rajahalme [email protected]