Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix client multicast sending on Windows #81

Conversation

davidflowerday
Copy link
Contributor

@davidflowerday davidflowerday commented Feb 4, 2021

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 #54 #69 and #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.

@grandcat
Copy link
Owner

grandcat commented Jul 5, 2021

Thanks for this contribution.
Would you mind rebasing this Pr to latest master?

@tmm1
Copy link
Contributor

tmm1 commented Jul 5, 2021

I'm not sure about this. It might be worth doing on Windows only.

FYI I also opened PRs to golang to implement these methods on Windows, but they were not merged.

@davidflowerday
Copy link
Contributor Author

I know this is a bit old but it's still causing problems for me. I can work on making it Windows-only per @tmm1's suggestion if you prefer.

@cpuchip
Copy link

cpuchip commented Jul 26, 2022

I can confirm that @davidflowerday solution fixes this issue for me on windows. Before his changes I can only get mDns services that are registered locally on my computer, after his changes I can now see all mDns services on my local network. I do urge you to consider his changes.

I am running Windows 11 Go version 1.18.3

@cpuchip
Copy link

cpuchip commented Jul 26, 2022

a fix will probably need to be applied to https://github.com/grandcat/zeroconf/blob/master/server.go#L718 too since I cannot receive mDns message from my windows box across the network using this library but I can from off the shelf devices like _googlecast and _airplay devices.

@cpuchip
Copy link

cpuchip commented Jul 26, 2022

I have no experience in writing network safe go code here, but these changes worked for me to see services registered with this library across the network. Definitely some optimizations could be applied.... but the idea works.

wow code block isn't working... I'll try and post this as a PR later unless someone beats me to it.

`// multicastResponse us used to send a multicast response packet
func (s *Server) multicastResponse(msg *dns.Msg, ifIndex int) error {
buf, err := msg.Pack()
if err != nil {
return err
}
if s.ipv4conn != nil {
var wcm ipv4.ControlMessage
if ifIndex != 0 {
iface, _ := net.InterfaceByIndex(ifIndex)
if err := s.ipv4conn.SetMulticastInterface(iface); err != nil {
log.Printf("[WARN] mdns: Failed to set multicast interface: %v", err)
}
wcm.IfIndex = ifIndex
s.ipv4conn.WriteTo(buf, &wcm, ipv4Addr)
} else {
for _, intf := range s.ifaces {
iface, _ := net.InterfaceByIndex(intf.Index)
if err := s.ipv4conn.SetMulticastInterface(iface); err != nil {
log.Printf("[WARN] mdns: Failed to set multicast interface: %v", err)
continue
}
wcm.IfIndex = intf.Index
s.ipv4conn.WriteTo(buf, &wcm, ipv4Addr)
}
}
}

if s.ipv6conn != nil {
	var wcm ipv6.ControlMessage
	if ifIndex != 0 {
		iface, _ := net.InterfaceByIndex(ifIndex)
		if err := s.ipv6conn.SetMulticastInterface(iface); err != nil {
			log.Printf("[WARN] mdns: Failed to set multicast interface: %v", err)
		}
		wcm.IfIndex = ifIndex
		s.ipv6conn.WriteTo(buf, &wcm, ipv6Addr)
	} else {
		for _, intf := range s.ifaces {
			iface, _ := net.InterfaceByIndex(intf.Index)
			if err := s.ipv6conn.SetMulticastInterface(iface); err != nil {
				log.Printf("[WARN] mdns: Failed to set multicast interface: %v", err)
				continue
			}
			wcm.IfIndex = intf.Index
			s.ipv6conn.WriteTo(buf, &wcm, ipv6Addr)
		}
	}
}
return nil

}`

@cpuchip
Copy link

cpuchip commented Jul 27, 2022

I've opened up a new PR with platform specific changes
#110

@cpuchip
Copy link

cpuchip commented Jul 29, 2022

@davidflowerday I don't know if you just needed an active repo with this fix in it, but on my fork: https://github.com/cpuchip/zeroconf
I went ahead and pulled in those changes to my master branch and tagged it v2.2.0 It's a mashup of grandcat/zeroconf master and libp2p/zeroconf master I've just barely started using it and it seems to be working alright on windows.

I honestly do not understand the inners of this library well at the moment, and your find with SetMulticastInterface was brilliant, I was pulling my hair our trying to get a project working on windows/linux. So thank you for that work.

@grandcat
Copy link
Owner

Superseded by #110.

@grandcat grandcat closed this Jan 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants