From 7a410f9236b3a5ea5aa81114533c4f82c73dd301 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Sun, 31 May 2020 14:29:54 -0700
Subject: [PATCH] net/netns: unindent, refactor to remove some redunant code

Also:
* always error on Control failing. That's very unexpected.
* pull out sockopt funcs into their own funcs for easier future testing
---
 net/netns/netns_linux.go | 73 +++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/net/netns/netns_linux.go b/net/netns/netns_linux.go
index 632fbf51a..09fc1982e 100644
--- a/net/netns/netns_linux.go
+++ b/net/netns/netns_linux.go
@@ -110,45 +110,42 @@ func control(network, address string, c syscall.RawConn) error {
 		return nil
 	}
 
-	if ipRuleAvailable() {
-		var controlErr error
-		err := c.Control(func(fd uintptr) {
-			controlErr = unix.SetsockoptInt(int(fd),
-				unix.SOL_SOCKET, unix.SO_MARK,
-				tailscaleBypassMark)
-		})
-		if (err != nil || controlErr != nil) && ignoreErrors() {
-			return nil
-		}
-		if err != nil {
-			return fmt.Errorf("setting socket mark1: %w", err)
-		}
-		if controlErr != nil {
-			return fmt.Errorf("setting socket mark2: %w", controlErr)
-		}
-	} else {
-		var controlErr error
-		err := c.Control(func(fd uintptr) {
-			ifc, err := defaultRouteInterface()
-			if err != nil {
-				// Make sure we bind to *some* interface,
-				// or we could get a routing loop.
-				// "lo" is always wrong, but if we don't have
-				// a default route anyway, it doesn't matter.
-				ifc = "lo"
-			}
-			controlErr = unix.SetsockoptString(int(fd),
-				unix.SOL_SOCKET, unix.SO_BINDTODEVICE, ifc)
-		})
-		if (err != nil || controlErr != nil) && ignoreErrors() {
-			return nil
-		}
-		if err != nil {
-			return fmt.Errorf("setting SO_BINDTODEVICE 1: %w", err)
-		}
-		if controlErr != nil {
-			return fmt.Errorf("setting SO_BINDTODEVICE 2: %w", controlErr)
+	var sockErr error
+	err := c.Control(func(fd uintptr) {
+		if ipRuleAvailable() {
+			sockErr = setBypassMark(fd)
+		} else {
+			sockErr = bindToDevice(fd)
 		}
+	})
+	if err != nil {
+		return fmt.Errorf("RawConn.Control on %T: %w", c, err)
+	}
+	if sockErr != nil && ignoreErrors() {
+		// TODO(bradfitz): maybe log once? probably too spammy for e.g. CLI tools like tailscale netcheck.
+		return nil
+	}
+	return sockErr
+}
+
+func setBypassMark(fd uintptr) error {
+	if err := unix.SetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_MARK, tailscaleBypassMark); err != nil {
+		return fmt.Errorf("setting SO_MARK bypass: %w", err)
+	}
+	return nil
+}
+
+func bindToDevice(fd uintptr) error {
+	ifc, err := defaultRouteInterface()
+	if err != nil {
+		// Make sure we bind to *some* interface,
+		// or we could get a routing loop.
+		// "lo" is always wrong, but if we don't have
+		// a default route anyway, it doesn't matter.
+		ifc = "lo"
+	}
+	if err := unix.SetsockoptString(int(fd), unix.SOL_SOCKET, unix.SO_BINDTODEVICE, ifc); err != nil {
+		return fmt.Errorf("setting SO_BINDTODEVICE: %w", err)
 	}
 	return nil
 }