fix: send mDNS queries on multicast sockets#162
Conversation
|
|
|
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
sendQuery only wrote to the multicast destination from unicast UDP sockets. Some devices only answer queries that originate on a multicast socket (RFC 6762). Send via ipv4MulticastConn / ipv6MulticastConn as well, while keeping the existing unicast writes so local loopback discovery tests still pass. Fixes hashicorp#144
tgross
left a comment
tgross
left a comment
There was a problem hiding this comment.
You haven't added tests or even bothered explaining how you've confirmed that this change works manually.
| var last error | ||
| if c.ipv4MulticastConn != nil { | ||
| if _, err = c.ipv4MulticastConn.WriteToUDP(buf, ipv4Addr); err != nil { | ||
| last = err |
There was a problem hiding this comment.
You've changed the error handling behavior of this function without explaining why. We're now falling-thru on error to try another the unicast conn, but still returning the error, which seems like the worst of both worlds.
Address review feedback: - Do not return a partial write error if any socket succeeds - Only surface an error when every attempted write fails - Add unit tests covering multicast+unicast writes and partial failure
2 participants
Description
sendQuerysent mDNS queries to the multicast destination (224.0.0.251/ff02::fb) only from unicast sockets. Some devices (e.g. Alfen EV Charger) only respond to queries that originate on a multicast socket (RFC 6762).Fix
Also write queries via
ipv4MulticastConn/ipv6MulticastConn, while keeping the existing unicast writes so local loopback discovery continues to work.Tests
go test ./... -count=1Passes on this machine (including
TestServer_Lookup).Linked Issue
Closes #144