Conversation
grandcat
left a comment
There was a problem hiding this comment.
Generally looks good to me.
Would be nice to have a test for this, e.g. two clients listening + one server answering. What do you think?
|
Making a test is a good idea. But it would be dependent on the OS/Kernel that is running the test. It would only work on platforms that support SO_REUSEPORT. |
| ) | ||
|
|
||
| func joinUdp6Multicast(interfaces []net.Interface) (*ipv6.PacketConn, error) { | ||
| udpConn, err := net.ListenUDP("udp6", mdnsWildcardAddrIPv6) |
There was a problem hiding this comment.
Why we need SO_REUSEPORT? For multicast address, SO_REUSEPORT is the same as SO_REUSEADDR which was already set by default when you call net.ListenUDP.
There was a problem hiding this comment.
On the Debian Linux system I am running this on, I have a few other mdns services. When starting a service using this library net.ListenUDP would fail due to the port already being in use.
After switching it to use SO_REUSEPORT to be inline with all the other mDNS services I was able to successfully run it with the other services at the same time without issue. [1,2]
It is my understanding that SO_REUSEPORT is not needed if all the services are part of the same program (PID), but if they are not; SO_REUSEPORT is required to tell the kernel it is ok for different processes to share the same address/port. Which is required for mDNS.
[1] https://github.com/udp-redux/udp-broadcast-relay-redux/blob/master/main.c#L382
[2] https://github.com/lathiat/avahi/blob/d1e71b320d96d0f213ecb0885c8313039a09f693/avahi-core/socket.c#L178
There was a problem hiding this comment.
Thanks @lanrat for the details. After some digging and exercises, I agree that set both SO_REUSEADDR and SO_REUSEPORT is better than just setting SO_REUSEADDR.
I think the reason net.LIstenUDP failed in your env is some other mdns services running only enabled SO_REUSEPORT.
"It is my understanding that SO_REUSEPORT is not needed if all the services are part of the same program (PID), but if they are not; SO_REUSEPORT is required to tell the kernel it is ok for different processes to share the same address/port. Which is required for mDNS."
-> I don't agree with above comments. Based on my test, SO_REUSEADDR works quite well for multiple processes for mDNS.
There was a problem hiding this comment.
"It is my understanding that SO_REUSEPORT is not needed if all the services are part of the same program (PID), but if they are not; SO_REUSEPORT is required to tell the kernel it is ok for different processes to share the same address/port. Which is required for mDNS."
-> I don't agree with above comments. Based on my test, SO_REUSEADDR works quite well for multiple processes for mDNS.
This comment was based on the results of the limited testing I did, so I could very well be wrong here. I'll research this a bit more to fully understand what's going on.
Either way, adding SO_REUSEPORT does appear to fix the problem as I'm running 5+ mdns services on the same host.
|
@grandcat any updates on getting this or any of the other PRs merged? |
I believe the last state was to provide a test for this, even though it would only be tested on OSes which support it. |
|
Thanks for the reminder. I'll add that to my backlog. ;) |
Adds support for SO_REUSEPORT.
This allows for multiple mDNS clients/servers/services running on the same host