Repository: mdlayher/netlink
Pull request #147 (2019-08-31 00:28:03)

Provide configuration option for not locking go-routines to threads
This is an implementation of the configuration option that will allow callers to 
define that the thread lock must be bypassed. 

See discussion here for details: https://github.com/mdlayher/netlink/issues/146


Patch:
diff --git a/conn.go b/conn.go
index 46e0c30..c29f231 100644
--- a/conn.go
+++ b/conn.go
@@ -583,4 +583,16 @@ type Config struct {
 	// CAP_SYS_ADMIN are required), and most applications should leave this set
 	// to 0.
 	NetNS int
+
+	// DisableNSLockThread disables package netlink's default goroutine thread locking behavior.
+	//
+	// By default, the library will lock the processing goroutine to its corresponding
+	// thread in order to enable communication over netlink to a different namespaces.
+	// In some cases, where the caller already knows that the netlink socket
+	// is in the same namespace as the calling thread, this can introduce a performance
+	// impact. In these cases, the caller can disable thread locking if performance
+	// considerations are of interest. If disabled, it is the responsibility of
+	// the caller to make sure that all threads are running in the correct namespace.
+	// When DisableNSLockThread is set, the caller cannot set the NetNS value.
+	DisableNSLockThread bool
 }
diff --git a/conn_linux.go b/conn_linux.go
index 7ac144c..b4d19d3 100644
--- a/conn_linux.go
+++ b/conn_linux.go
@@ -3,6 +3,7 @@
 package netlink
 
 import (
+	"errors"
 	"math"
 	"os"
 	"runtime"
@@ -15,6 +16,8 @@ import (
 	"golang.org/x/sys/unix"
 )
 
+var errNetNSCannotLockThread = errors.New("netns cannot be set when disableNSThreadLock is set")
+
 var _ Socket = &conn{}
 
 var _ deadlineSetter = &conn{}
@@ -339,7 +342,7 @@ type sysSocket struct {
 // to a single thread.
 func newSysSocket(config *Config) (*sysSocket, error) {
 	// Determine network namespaces using the threadNetNS function.
-	g, err := newLockedNetNSGoroutine(config.NetNS, threadNetNS)
+	g, err := newLockedNetNSGoroutine(config.NetNS, threadNetNS, !config.DisableNSLockThread)
 	if err != nil {
 		return nil, err
 	}
@@ -624,10 +627,16 @@ type lockedNetNSGoroutine struct {
 // newLockedNetNSGoroutine creates a lockedNetNSGoroutine that will enter the
 // specified network namespace netNS (by file descriptor), and will use the
 // getNS function to produce netNS handles.
-func newLockedNetNSGoroutine(netNS int, getNS func() (*netNS, error)) (*lockedNetNSGoroutine, error) {
+func newLockedNetNSGoroutine(netNS int, getNS func() (*netNS, error), lockThread bool) (*lockedNetNSGoroutine, error) {
 	// Any bare syscall errors (e.g. setns) should be wrapped with
 	// os.NewSyscallError for the remainder of this function.
 
+	// If the lockThread is set and the caller attempts to set a
+	// namespace, return an error.
+	if !lockThread && netNS != 0 {
+		return nil, errNetNSCannotLockThread
+	}
+
 	callerNS, err := getNS()
 	if err != nil {
 		return nil, err
@@ -660,9 +669,16 @@ func newLockedNetNSGoroutine(netNS int, getNS func() (*netNS, error)) (*lockedNe
 		// with the Go runtime for threads which are not unlocked, we have
 		// elected to temporarily unlock the thread when the goroutine terminates:
 		// https://github.com/golang/go/issues/25128#issuecomment-410764489.
+		//
+		// Locking the thread is not implemented if the caller explicitly asks
+		// for an unlocked thread.
+
+		// Only lock the tread, if the lockThread is set.
+		if lockThread {
+			runtime.LockOSThread()
+			defer runtime.UnlockOSThread()
+		}
 
-		runtime.LockOSThread()
-		defer runtime.UnlockOSThread()
 		defer g.wg.Done()
 
 		// Get the current namespace of the thread the goroutine is locked to.


Test patch:
diff --git a/conn_linux_test.go b/conn_linux_test.go
index 4224b7c..b5e24b9 100644
--- a/conn_linux_test.go
+++ b/conn_linux_test.go
@@ -486,6 +486,14 @@ func TestLinuxConnConfig(t *testing.T) {
 			config: &Config{Groups: 0x10 | 0x40},
 			groups: 0x50,
 		},
+		{
+			name: "Config with DisableNSLockThread and Groups RTMGRP_IPV4_IFADDR | RTMGRP_IPV4_ROUTE",
+			config: &Config{
+				Groups:              0x10 | 0x40,
+				DisableNSLockThread: true,
+			},
+			groups: 0x50,
+		},
 	}
 
 	for _, tt := range tests {
@@ -502,21 +510,38 @@ func TestLinuxConnConfig(t *testing.T) {
 
 func Test_newLockedNetNSGoroutineNetNSDisabled(t *testing.T) {
 	tests := []struct {
-		name string
-		ns   int
-		ok   bool
+		name       string
+		ns         int
+		ok         bool
+		lockThread bool
 	}{
 		{
 			// Network namespaces are disabled but none is set: this should
 			// succeed.
-			name: "not set",
-			ok:   true,
+			name:       "not set",
+			ok:         true,
+			lockThread: true,
 		},
 		{
 			// Network namespaces are disabled but one is set explicitly:
 			// this should fail.
-			name: "set",
-			ns:   1,
+			name:       "set",
+			ns:         1,
+			lockThread: true,
+		},
+		{
+			// thread locking is disabled but an ns is provided.
+			// this should fail.
+			name:       "disable lock thread with ns defined",
+			ns:         1,
+			lockThread: false,
+		},
+		{
+			// thread locking is disabled but an ns is not provided.
+			// this should succeed.
+			name:       "disable lock thread without ns defined",
+			lockThread: false,
+			ok:         true,
 		},
 	}
 
@@ -526,7 +551,7 @@ func Test_newLockedNetNSGoroutineNetNSDisabled(t *testing.T) {
 				// Network namespaces should be disabled due to a non-existent
 				// file.
 				return fileNetNS("/netlinktestdoesnotexist")
-			})
+			}, tt.lockThread)
 			if err != nil {
 				if tt.ok {
 					t.Fatalf("failed to create goroutine: %v", err)