fix: address non-working socket configuration #1563
Merged
Conversation
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
9c34623 to
410f1a2
Compare
CodSpeed Performance ReportMerging #1563 will not alter performanceComparing Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1563 +/- ##
==========================================
- Coverage 99.79% 99.76% -0.03%
==========================================
Files 33 33
Lines 3401 3400 -1
Branches 463 461 -2
==========================================
- Hits 3394 3392 -2
Misses 5 5
- Partials 2 3 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8edeb70 to
aefa298
Compare
aefa298 to
21e24b8
Compare
bdraco
reviewed
Apr 10, 2025
agners
commented
Apr 10, 2025
21e24b8 to
9819c84
Compare
This unifies the socket creation for all interface specific responder sockets to use the new_respond_socket function. If the responder socket isn't used for multicast, it won't get bound to the interface explicitly. This mostly deduplicates code and makes it easier to maintain.
f5a3b77 to
b685214
Compare
b685214 to
0349c71
Compare
a73cce7 to
962f1e1
Compare
For IPv6, a shared listener/responder socket is not really possible when sending to link-local IPv6 multicast addresses (ff02::/16): The kernel needs to know which interface to use for routing. On IPv4, this is historically a bit different, the kernel just uses what it deems the primary/best route interface based on the routing table. But for IPv6, a message is rejected by Linux with OSError no 99 "Cannot assign requested address" and OSError no 65 "No route to host" on macOS. Removing the InterfaceChoice.Default config when IPv6 is enabled would be a major breaking change. Instead, inform the user with an error log message and a DeprecationWarning that this is not a working configuration. As a further cleanup, move the socket options for sending multicast packets out of the common socket creation code. For listen only sockets those settings are not needed. Also don't use a shared listener/responder sockets for dual-stack mode. Using a shared socket for IPv4 and IPv6 is especially problematic on macOS, where the kernel does not support this and even rejects socket options for IPv4 multicast in this case. With a separate IPv4 socket, this actually fixes the IPv4 multicast query sending when using the default interface. The sending will only fail on the IPv6 socket, so IPv4 requests will make it through. With that, the macOS error addressed in #392 is not a problem anymore. Actually, we would like to get an exception in case we get into this combination, so remove the explicit exception handling.
c7e90d8 to
8f1bad1
Compare
|
verified HA zeroconf tests still pass |
bdraco
approved these changes
Apr 14, 2025
|
tested on production. no issues observed |
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.
For IPv6, a shared listener/responder socket is not really possible when sending to link-local IPv6 multicast addresses (ff02::/16): The kernel needs to know which interface to use for routing.
On IPv4, this is historically a bit different, the kernel just uses what it deems the primary/best route interface based on the routing table. But for IPv6, a message is rejected by Linux with OSError no 99 "Cannot assign requested address" and OSError no 65 "No route to host" on macOS.
Removing the InterfaceChoice.Default config when IPv6 is enabled would be a major breaking change. Instead, inform the user with an error log message and a DeprecationWarning that this is not a working configuration.
As a further cleanup, move the socket options for sending multicast packets out of the common socket creation code. For listen only sockets those settings are not needed.
Also don't use a shared listener/responder sockets for dual-stack mode. Using a shared socket for IPv4 and IPv6 is especially problematic on macOS, where the kernel does not support this and even rejects socket options for IPv4 multicast in this case.
With a separate IPv4 socket, this actually fixes the IPv4 multicast query sending when using the default interface. The sending will only fail on the IPv6 socket, so IPv4 requests will make it through.
With that, the macOS error addressed in #392 is not a problem anymore. Actually, we would like to get an exception in case we get into this combination, so remove the explicit exception handling.
Fixes: #1562
chore: use new_respond_socket for all responder sockets
This unifies the socket creation for all interface specific responder sockets to use the new_respond_socket function. If the responder socket isn't used for multicast, it won't get bound to the interface explicitly. This mostly deduplicates code and makes it easier to maintain.