Repository: miekg/dns
Pull request #1623 (2024-12-06 09:09:49)

Allow SO_REUSEADDR and SO_REUSEPORT to be set simultaneously
Currently it is impossible to set both `SO_REUSEADDR` and `SO_REUSEPORT` simultaneously. If a user tries to set both, there is no error and none of the options are actually set which can be quite confusing.

This PR not only fixes this but adds a test for all combinations of the socket options. In contrast to the existing tests for `SO_REUSEADDR` and  `SO_REUSEPORT`, this test does not check that the options do what they are supposed to do (this is the kernels responsibility) but it tests whether or not the options are actually set as requested by the user (which is the library's responsibility).

miekg: looks like a nice addition thanks!

Patch:
diff --git a/listen_no_reuseport.go b/listen_no_socket_options.go
similarity index 61%
rename from listen_no_reuseport.go
rename to listen_no_socket_options.go
index 8cebb2f1..9e4010bd 100644
--- a/listen_no_reuseport.go
+++ b/listen_no_socket_options.go
@@ -3,9 +3,15 @@
 
 package dns
 
-import "net"
+import (
+	"fmt"
+	"net"
+)
 
-const supportsReusePort = false
+const (
+	supportsReusePort = false
+	supportsReuseAddr = false
+)
 
 func listenTCP(network, addr string, reuseport, reuseaddr bool) (net.Listener, error) {
 	if reuseport || reuseaddr {
@@ -15,8 +21,6 @@ func listenTCP(network, addr string, reuseport, reuseaddr bool) (net.Listener, e
 	return net.Listen(network, addr)
 }
 
-const supportsReuseAddr = false
-
 func listenUDP(network, addr string, reuseport, reuseaddr bool) (net.PacketConn, error) {
 	if reuseport || reuseaddr {
 		// TODO(tmthrgd): return an error?
@@ -24,3 +28,13 @@ func listenUDP(network, addr string, reuseport, reuseaddr bool) (net.PacketConn,
 
 	return net.ListenPacket(network, addr)
 }
+
+// this is just for test compatibility
+func checkReuseport(fd uintptr) (bool, error) {
+	return false, fmt.Errorf("not supported")
+}
+
+// this is just for test compatibility
+func checkReuseaddr(fd uintptr) (bool, error) {
+	return false, fmt.Errorf("not supported")
+}
diff --git a/listen_reuseport.go b/listen_socket_options.go
similarity index 66%
rename from listen_reuseport.go
rename to listen_socket_options.go
index 41326f20..35dfc949 100644
--- a/listen_reuseport.go
+++ b/listen_socket_options.go
@@ -39,10 +39,40 @@ func reuseaddrControl(network, address string, c syscall.RawConn) error {
 	return opErr
 }
 
+func reuseaddrandportControl(network, address string, c syscall.RawConn) error {
+	err := reuseaddrControl(network, address, c)
+	if err != nil {
+		return err
+	}
+
+	return reuseportControl(network, address, c)
+}
+
+// this is just for test compatibility
+func checkReuseport(fd uintptr) (bool, error) {
+	v, err := unix.GetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_REUSEPORT)
+	if err != nil {
+		return false, err
+	}
+
+	return v == 1, nil
+}
+
+// this is just for test compatibility
+func checkReuseaddr(fd uintptr) (bool, error) {
+	v, err := unix.GetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_REUSEADDR)
+	if err != nil {
+		return false, err
+	}
+
+	return v == 1, nil
+}
+
 func listenTCP(network, addr string, reuseport, reuseaddr bool) (net.Listener, error) {
 	var lc net.ListenConfig
 	switch {
 	case reuseaddr && reuseport:
+		lc.Control = reuseaddrandportControl
 	case reuseport:
 		lc.Control = reuseportControl
 	case reuseaddr:
@@ -56,6 +86,7 @@ func listenUDP(network, addr string, reuseport, reuseaddr bool) (net.PacketConn,
 	var lc net.ListenConfig
 	switch {
 	case reuseaddr && reuseport:
+		lc.Control = reuseaddrandportControl
 	case reuseport:
 		lc.Control = reuseportControl
 	case reuseaddr:


Test patch:
diff --git a/server_test.go b/server_test.go
index 4fc2af32..a9e2339b 100644
--- a/server_test.go
+++ b/server_test.go
@@ -996,6 +996,101 @@ func TestServerStartStopRace(t *testing.T) {
 	wg.Wait()
 }
 
+func TestSocketOptions(t *testing.T) {
+	if !supportsReuseAddr || !supportsReusePort {
+		t.Skip("reuseaddr or reuseport is not supported")
+	}
+
+	testSocketOptions := func(t *testing.T, reuseAddr bool, reusePort bool) {
+		wait := make(chan struct{})
+
+		srv := &Server{
+			Net:       "udp",
+			Addr:      ":0",
+			ReuseAddr: reuseAddr,
+			ReusePort: reusePort,
+		}
+
+		srv.NotifyStartedFunc = func() {
+			defer close(wait)
+
+			conn, ok := srv.PacketConn.(*net.UDPConn)
+			if !ok {
+				t.Errorf("unexpected conn type: %T", srv.PacketConn)
+				return
+			}
+
+			syscallConn, err := conn.SyscallConn()
+			if err != nil {
+				t.Errorf("cannot cast UDP conn to syscall conn: %v", err)
+				return
+
+			}
+
+			err = syscallConn.Control(func(fd uintptr) {
+				actualReusePort, err := checkReuseport(fd)
+				if err != nil {
+					t.Errorf("cannot get SO_REUSEPORT socket option: %v", err)
+					return
+				}
+
+				if actualReusePort != reusePort {
+					t.Errorf("SO_REUSEPORT is %v instead of %v", actualReusePort, reusePort)
+				}
+
+				actualReuseAddr, err := checkReuseaddr(fd)
+				if err != nil {
+					t.Errorf("cannot get SO_REUSEADDR socket option: %v", err)
+					return
+				}
+
+				if actualReuseAddr != reuseAddr {
+					t.Errorf("SO_REUSEADDR is %v instead of %v", actualReuseAddr, reusePort)
+				}
+			})
+			if err != nil {
+				t.Errorf("cannot check socket options: %v", err)
+			}
+		}
+
+		fin := make(chan error, 1)
+		go func() {
+			fin <- srv.ListenAndServe()
+		}()
+
+		select {
+		case <-wait:
+			err := srv.Shutdown()
+			if err != nil {
+				t.Fatalf("cannot shutdown server: %v", err)
+			}
+
+			err = <-fin
+			if err != nil {
+				t.Fatalf("listen adn serve: %v", err)
+			}
+		case err := <-fin:
+			t.Fatalf("listen adn serve: %v", err)
+		}
+	}
+
+	t.Run("no socket options", func(t *testing.T) {
+		testSocketOptions(t, false, false)
+	})
+
+	t.Run("SO_REUSEPORT", func(t *testing.T) {
+		testSocketOptions(t, false, true)
+	})
+
+	t.Run("SO_REUSEADDR", func(t *testing.T) {
+		testSocketOptions(t, true, false)
+	})
+
+	t.Run("SO_REUSEADDR and SO_REUSEPORT", func(t *testing.T) {
+		testSocketOptions(t, true, true)
+	})
+}
+
 func TestServerReuseport(t *testing.T) {
 	if !supportsReusePort {
 		t.Skip("reuseport is not supported")
@@ -1329,7 +1424,6 @@ func TestResponseWriteSinglePacket(t *testing.T) {
 	m.SetQuestion("miek.nl.", TypeTXT)
 	m.Response = true
 	err := rw.WriteMsg(m)
-
 	if err != nil {
 		t.Fatalf("failed to write: %v", err)
 	}