Implement dual-stack support#861
Implement dual-stack support#861DerEnderKeks wants to merge 4 commits intohome-assistant-libs:masterfrom
Conversation
|
Please fix the CI issues |
DerEnderKeks
force-pushed
the
dual-stack
branch
from
91ebe3f to
1cc0a30
Compare
emontnemery
left a comment
emontnemery
left a comment
There was a problem hiding this comment.
Some hosts will have ipv6 disabled, and creating an ipv6 socket will fail, the IPV6 specifics needs to be wrapped with an OSError check with fall back to IPv4.
DerEnderKeks
force-pushed
the
dual-stack
branch
from
1cc0a30 to
4529903
Compare
|
The socket will now fall back to IPv4, for systems that are misconfigured to have IPv6 disabled ;) |
DerEnderKeks
force-pushed
the
dual-stack
branch
from
0df6d2a to
4f06028
Compare
|
@DerEnderKeks please don't amend your previous commit when making changes, it makes it hard to review |
|
Alright (although I always consider it an unnessacary loss of information when you squash PRs into a single commit). |
|
Is there any need/benefit of the new happy eyeball stuff you working on @bdraco |
|
All of that work is for asyncio so it wouldn’t apply here |
| # falling back to IPv4 on systems without IPv6 | ||
| new_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) | ||
|
|
||
| new_sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) |
There was a problem hiding this comment.
This looks strange to me. We are not making sure the socket type matches the data returned from getaddrinfo. The creation of the socket should be postponed until we have resolved the name using getaddrinfo
There was a problem hiding this comment.
There isn't really a need to match the socket to the resolved address type, as the dual-stack socket can connect to both types. If it falls back to a v4-only socket and only a v6 address is returned, it would fail to connect either way,
There was a problem hiding this comment.
I dont agree, if we are using getaddrinfo to get the host info, we should respect what it return.
By using an INET6 socket with IPV6_V6ONLY disabled, legacy IPv4 can still be reached. To connect to v4 IPs, the addresses are mapped to IPv6. IPv6 addresses are only used when a hostname resolves to one and the system has a non-local IPv6 available. For hosts with IPv6 completely disabled for some reason, the socket falls back to v4-only.
|
Since this sort of died out. I rebased the pull and added iteration over getaddrinfo results. @DerEnderKeks could you test if these changes work for your use case? |
4 participants
This PR adds support for IPv6 and thus dual-stack. It contains two changes:
Unsetting theHostheader when requesting the/setup/eureka_infoendpoint, as I noticed that my NVIDIA Shield blocks any request (empty body with status code 403) when a domain is set in the header, at least when requesting via IPv6.Let me know if I overlooked anything, but the Youtube example now works fine with both, IPv4 and IPv6.