search

grandcat / zeroconf

mDNS / DNS-SD Service Discovery in pure Go (also known as Bonjour)
Other
748 stars 172 forks source link

Fix multicast xmit windows #110

Closed cpuchip closed 1 year ago

cpuchip commented 2 years ago

Adding to @davidflowerday's PR I've made this platform specific so windows/others uses SetMulticastInterface and linux/darwin uses ControlMessage, I've also updated server.go to include a fix for this problem as well.

I was having issues getting this library to even work on windows, only mDNS services I registered on localhost would show up in my browse/lookup requests, after digging in I found @davidflowerday's PR and a few other comments on other forks.

I've tests this fix on Windows and and linux (WSL) and both seem to be in good shape, I can resolve and register mDNS service both on and off box and see them. I do not have a mac ( Intel or Arm ) to test this on unfortunately.

from @davidflowerday's PR I'm attempting to use go-chromecast on Windows which leverages this library. Unfortunately, mDNS wasn't working. I was able to fix it by replacing the code that used ControlMessage in WriteTo() with a call to SetMulticastInterface() instead. I think this may be related to issues https://github.com/grandcat/zeroconf/issues/54 https://github.com/grandcat/zeroconf/issues/69 and https://github.com/grandcat/zeroconf/issues/75. I've tested this change on Windows and Linux and I will test on macOS shortly as well. I only changed client.go but it's possible a similar change is needed in server.go as well.

In the docs for the ipv4 package under Bugs there is this note:

On Windows, the ControlMessage for ReadFrom and WriteTo methods of PacketConn is not implemented.

Additionally, the ipv4 docs on Multicasting here show an example using SetMulticastInterface() as I've done in this PR which suggests to me that this is an OK way to handle this issue, but if you'd prefer something else I'd be happy to change it.

Thanks for making zeroconf and for considering this PR.

edaniels commented 1 year ago

I think this basically works! I did get one failure but it looks flaky

--- FAIL: TestNoRegister (5.00s)
    service_test.go:81: Expected empty service entries but got {{test--xxxxxxxxxxxx _test--xxxx._tcp [] local. _test--xxxx._tcp.local. test--xxxxxxxxxxxx._test--xxxx._tcp.local. _services._dns-sd._udp.local.} ermbp.local. 8888 [txtv=0 lo=1 la=2] 3200 [10.1.8.177 100.84.156.104] [fe80::1 fe80::646e:ff:feaa:b671 fe80::646e:ff:feaa:b672 fe80::646e:ff:feaa:b670 fd74:4071:a3e3:cbcf:1c85:aae2:74be:9eea fe80::88bd:54ff:fec5:c37c fe80::88bd:54ff:fec5:c37c fe80::b881:defd:d07e:f0c7 fe80::675b:f3fb:7934:fcd4 fe80::ce81:b1c:bd2c:69e fe80::29c8:2e74:2c05:9fb fe80::a49:5af5:601:b0ae fd7a:115c:a1e0:ab12:4843:cd96:6254:9c68]}

Pushed your changes to yet another fork https://github.com/edaniels/zeroconf/tree/register_dynamic_fork. Maybe upvote https://github.com/grandcat/zeroconf/issues/112!