golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.22k stars 17.7k forks source link

net: ListenMulticastUDP fails if Ethernet is unplugged #70132

Open rittneje opened 3 weeks ago

rittneje commented 3 weeks ago

Go version

go version 1.22.8 linux/amd64

Output of go env in your module/workspace:

n/a

What did you do?

My VirtualBox VM has two Ethernet interfaces - enp0s3 and enp0s8. I have noticed a few issues with how net.ListenMulticastUDP works, all seemingly due to how it does IP_ADD_MEMBERSHIP.

package main

import (
    "fmt"
    "net"
    "syscall"
)

func main() {
    intf, err := net.InterfaceByName("enp0s3") // or enp0s8, depending on the test
    if err != nil {
        panic(err)
    }

    addr := &net.UDPAddr{IP: net.IPv4(239, 255, 255, 250).To4(), Port: 1900}

    c, err := net.ListenMulticastUDP("udp", intf, addr)
    if err != nil {
        panic(err)
    }
    defer c.Close()

    buf := make([]byte, 1000)
    for {
        n, _, err := c.ReadFrom(buf)
        if err != nil {
            panic(err)
        }
        fmt.Println(string(buf[:n]))
    }
}

What did you see happen?

If I run the above code after telling VirtualBox to disconnect the cable from enp0s3, I get:

panic: listen udp 239.255.255.250:1900: setsockopt: no such device

I have tracked the error down to this specific call. https://github.com/golang/go/blob/6d39245514c675cdea5c7fd7f778e97bf0728dd5/src/net/sockoptip_posix.go#L19

Meanwhile, if I change the sample program to bind to enp0s8 instead, and I tell VirtualBox to disconnect enp0s8 (and re-connect enp0s3), what actually happens is it ends up bound to enp0s3, as confirmed by netstat.

$ netstat -gn | grep 239.255.255.250 enp0s3 1 239.255.255.250

I believe what is happening is that setIPv4MreqToInterface ends up leaving mreq.Interface as 0.0.0.0 because the interface in question has no IP addresses when the cable is unplugged. Since enp0s3 is my default interface:

  1. If I intend to bind to enp0s3 and its cable is unplugged, the kernel is unable to do whatever it needs to (due to some missing route table entry?) and gives the aforementioned error.
  2. If I intend to bind to enp0s8 and its cable is unplugged, the kernel defaults to enp0s3 instead.

It is worth noting we have also observed this issue in "the real world" (i.e., not in a VM) - net.ListenMulticastUDP will hard fail if the kernel has no route table entry at that moment, which can happen if the Ethernet cable happens to be unplugged.

My experiments show that using SetsockoptIPMreqn instead of SetsockoptIPMreq to do IP_ADD_MEMBERSHIP (and passing the interface index instead of its IP address) seems to make it work as expected in both cases.

syscall.SetsockoptIPMreqn(int(fd), syscall.IPPROTO_IP, syscall.IP_ADD_MEMBERSHIP, &syscall.IPMreqn{
    Multiaddr: [4]byte{addr.IP[0], addr.IP[1], addr.IP[2], addr.IP[3]},
    Ifindex: int32(intf.Index),
})

I also noticed that x/net uses MCAST_JOIN_GROUP instead of IP_ADD_MEMBERSHIP.

https://github.com/golang/net/blob/f35fec92ec9213ee211cf45f451a5970386f7978/ipv4/sys_linux.go#L34

However, that sockopt is entirely undocumented and seems to be an internal detail of the kernel.

What did you expect to see?

I expected net.ListenMulticastUDP to work as advertised regardless of the state of the Ethernet cable.

In particular, on Linux it should use SetsockoptIPMreqn so that it always does the right thing. On non-Linux, it should fail if a net.Interface was provided and the interface in question has no IP address.

gabyhelp commented 3 weeks ago

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

cagedmantis commented 3 weeks ago

cc @neild @ianlancetaylor

rittneje commented 1 week ago

@neild @ianlancetaylor Are these changes that you would be amenable to? We'd be willing to contribute the fixes. In particular, we want to make two changes.

First, fix joinIPv4Group to use SetsockoptIPMreqn on Linux and pass the interface index.

Second, fix setIPv4MreqToInterface to check whether mreq.Interface is 0.0.0.0 after the loop instead of mreq.Multiaddr, which we suspect was the intent. https://github.com/golang/go/blob/3730814f2f2bf24550920c39a16841583de2dac1/src/net/sockopt_posix.go#L69-L71