Refactor netns handling, fix flakes, namespace some tests (#227)

* netns: remove iproute2 dependency

This commit introduces a breaking change to rtnetlink.NetNS.

The existing netns implementation had a few problems. It assumed that network
namespaces have names, that they would always be pinned to /var/run/netns, and
that numeric/integer references are pid references. This made the NetNS type
unusable for referring to existing netns by fd, such as ones created by other
libraries, or by opening procfs entries directly as demonstrated in the new
testutils.NetNS() function.

The forced dependency on the `ip` CLI tool also wasn't reasonable for a pure-Go
library. Using the old implementation in a scratch/distroless container would
quickly run into roadblocks.

This commit also removes the functionality of creating and pinning new netns.
There are plenty of options out in the Go ecosystem for that, and providing
your own is only a few lines of code.

Signed-off-by: Timo Beckers <timo@incline.eu>

* test: remove calls to unix.Setrlimit() in favor of rlimit.RemoveMemlock()

ebpf-go provides this out of the box and skips setting the rlimit on kernels
that support bpf memory cgroup accounting.

Signed-off-by: Timo Beckers <timo@incline.eu>

* neigh: fix flaky tests, add State field to Neigh entry

The flaky tests that were documented in the code are expected. Use the State
field to discard entries that can't reasonably be considered in tests.

Signed-off-by: Timo Beckers <timo@incline.eu>

* neigh: fix race in Conn.Neighbours

When running tests locally, I would frequently hit "too many/little matches,
expected 1, actual 0" due to other tests creating and deleting interfaces in
the common host netns used by all tests.

Neigh entries that fail the interface lookup can't have their Interface fields
populated and should be dropped from the result since the interface is no longer
there to begin with.

Signed-off-by: Timo Beckers <timo@incline.eu>

* xdp: refactor test suite to use test helpers and netns-driven tests

While running the test suite for testing netns-related changes, I noticed
some of the xdp tests started failing because they wanted to create a dummy
interface in the host network namespace.

Avoid the complexity of managing dummy interfaces altogether by running all
tests within their own netns and use the existing lo device that's present by
default.

Signed-off-by: Timo Beckers <timo@incline.eu>

* xdp,netkit: remove duplicate kernelMinReq in favor of testutils.SkipOnOldKernel

There were two implementations of this, so move them to common testutils.

Signed-off-by: Timo Beckers <timo@incline.eu>

---------

Signed-off-by: Timo Beckers <timo@incline.eu>

driver/bond_live_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/jsimonetti/rtnetlink/v2"
+	"github.com/jsimonetti/rtnetlink/v2/internal/testutils"
 	"github.com/mdlayher/netlink"
 )
 
@@ -34,23 +35,15 @@ func bondSlaveT(d rtnetlink.LinkDriver) *BondSlave {
 }
 
 func TestBond(t *testing.T) {
-	// establish a netlink connection
 	conn, err := rtnetlink.Dial(nil)
 	if err != nil {
 		t.Fatalf("failed to establish netlink socket: %v", err)
 	}
 	defer conn.Close()
 
-	bns, clean, err := createNS("bns1")
+	connNS, err := rtnetlink.Dial(&netlink.Config{NetNS: testutils.NetNS(t)})
 	if err != nil {
-		t.Fatal(err)
-	}
-	defer clean()
-
-	// use ns for testing arp ip targets
-	connNS, err := rtnetlink.Dial(&netlink.Config{NetNS: int(bns.Value())})
-	if err != nil {
-		t.Fatalf("failed to establish netlink socket to ns nkns: %v", err)
+		t.Fatalf("failed to establish netlink socket to netns: %v", err)
 	}
 	defer connNS.Close()
 

driver/driver_test.go

@@ -4,43 +4,10 @@
 package driver
 
 import (
-	"bytes"
-	"fmt"
-	"os/exec"
-	"testing"
-
 	"github.com/jsimonetti/rtnetlink/v2"
 	"golang.org/x/sys/unix"
 )
 
-func getKernelVersion() (kernel, major, minor int, err error) {
-	var uname unix.Utsname
-	if err := unix.Uname(&uname); err != nil {
-		return 0, 0, 0, err
-	}
-
-	end := bytes.IndexByte(uname.Release[:], 0)
-	versionStr := uname.Release[:end]
-
-	if count, _ := fmt.Sscanf(string(versionStr), "%d.%d.%d", &kernel, &major, &minor); count < 2 {
-		err = fmt.Errorf("failed to parse kernel version from: %q", string(versionStr))
-	}
-	return
-}
-
-// kernelMinReq checks if the runtime kernel is sufficient
-// for the test
-func kernelMinReq(t *testing.T, kernel, major int) {
-	k, m, _, err := getKernelVersion()
-	if err != nil {
-		t.Fatalf("failed to get host kernel version: %v", err)
-	}
-	if k < kernel || k == kernel && m < major {
-		t.Skipf("host kernel (%d.%d) does not meet test's minimum required version: (%d.%d)",
-			k, m, kernel, major)
-	}
-}
-
 // setupInterface create a interface for testing
 func setupInterface(conn *rtnetlink.Conn, name string, index, master uint32, driver rtnetlink.LinkDriver) error {
 	attrs := &rtnetlink.LinkAttributes{
@@ -74,30 +41,3 @@ func getInterface(conn *rtnetlink.Conn, index uint32) (*rtnetlink.LinkMessage, e
 	}
 	return &interf, err
 }
-
-// creates a network namespace by utilizing ip commandline tool
-// returns NetNS and clean function
-func createNS(name string) (*rtnetlink.NetNS, func(), error) {
-	cmdPath, err := exec.LookPath("ip")
-	if err != nil {
-		return nil, nil, fmt.Errorf("getting ip command path failed, %w", err)
-	}
-	_, err = exec.Command(cmdPath, "netns", "add", name).Output()
-	if err != nil {
-		return nil, nil, fmt.Errorf("ip netns add %s, failed: %w", name, err)
-	}
-
-	ns, err := rtnetlink.NewNetNS(name)
-	if err != nil {
-		return nil, nil, fmt.Errorf("reading ns %s, failed: %w", name, err)
-	}
-	return ns, func() {
-		if err := ns.Close(); err != nil {
-			fmt.Printf("closing ns file failed: %v", err)
-		}
-		_, err := exec.Command(cmdPath, "netns", "del", name).Output()
-		if err != nil {
-			fmt.Printf("removing netns %s failed, %v", name, err)
-		}
-	}, nil
-}

driver/netkit_live_test.go

@@ -8,30 +8,23 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/jsimonetti/rtnetlink/v2"
+	"github.com/jsimonetti/rtnetlink/v2/internal/testutils"
 	"github.com/mdlayher/netlink"
 )
 
 func TestNetkit(t *testing.T) {
-	kernelMinReq(t, 6, 7)
+	testutils.SkipOnOldKernel(t, "6.7", "netkit support")
 
-	// establish a netlink connection
 	conn, err := rtnetlink.Dial(nil)
 	if err != nil {
 		t.Fatalf("failed to establish netlink socket: %v", err)
 	}
 	defer conn.Close()
 
-	// create netns
-	nkns, clean, err := createNS("nkns1")
+	ns := testutils.NetNS(t)
+	connNS, err := rtnetlink.Dial(&netlink.Config{NetNS: ns})
 	if err != nil {
-		t.Fatal(err)
-	}
-	defer clean()
-
-	// establish a netlink connection with netns
-	connNS, err := rtnetlink.Dial(&netlink.Config{NetNS: int(nkns.Value())})
-	if err != nil {
-		t.Fatalf("failed to establish netlink socket to ns nkns: %v", err)
+		t.Fatalf("failed to establish netlink socket to netns: %v", err)
 	}
 	defer connNS.Close()
 
@@ -110,7 +103,7 @@ func TestNetkit(t *testing.T) {
 					Index: ifPeerIndex,
 					Attributes: &rtnetlink.LinkAttributes{
 						Name:  "nke",
-						NetNS: nkns,
+						NetNS: rtnetlink.NetNSForFD(uint32(ns)),
 					},
 				},
 			},

driver/veth_live_test.go

@@ -7,28 +7,21 @@ import (
 	"testing"
 
 	"github.com/jsimonetti/rtnetlink/v2"
+	"github.com/jsimonetti/rtnetlink/v2/internal/testutils"
 	"github.com/mdlayher/netlink"
 )
 
 func TestVeth(t *testing.T) {
-	// establish a netlink connection
 	conn, err := rtnetlink.Dial(nil)
 	if err != nil {
 		t.Fatalf("failed to establish netlink socket: %v", err)
 	}
 	defer conn.Close()
 
-	// create netns
-	vtns, clean, err := createNS("vtns1")
+	ns := testutils.NetNS(t)
+	connNS, err := rtnetlink.Dial(&netlink.Config{NetNS: ns})
 	if err != nil {
-		t.Fatal(err)
-	}
-	defer clean()
-
-	// establish a netlink connection with netns
-	connNS, err := rtnetlink.Dial(&netlink.Config{NetNS: int(vtns.Value())})
-	if err != nil {
-		t.Fatalf("failed to establish netlink socket to ns vtns1: %v", err)
+		t.Fatalf("failed to establish netlink socket to netns: %v", err)
 	}
 	defer connNS.Close()
 
@@ -74,7 +67,7 @@ func TestVeth(t *testing.T) {
 					Index: ifPeerIndex,
 					Attributes: &rtnetlink.LinkAttributes{
 						Name:  "vte",
-						NetNS: vtns,
+						NetNS: rtnetlink.NetNSForFD(uint32(ns)),
 					},
 				},
 			},

internal/testutils/netns.go

@@ -0,0 +1,55 @@
+package testutils
+
+import (
+	"fmt"
+	"os"
+	"runtime"
+	"testing"
+
+	"github.com/jsimonetti/rtnetlink/v2/internal/unix"
+	"golang.org/x/sync/errgroup"
+)
+
+// NetNS returns a file descriptor to a new network namespace.
+// The netns handle is automatically closed as part of test cleanup.
+func NetNS(tb testing.TB) int {
+	tb.Helper()
+
+	var ns *os.File
+	var eg errgroup.Group
+	eg.Go(func() error {
+		// Lock the new goroutine to its OS thread. Never unlock the goroutine so
+		// the thread dies when the goroutine ends to avoid having to restore the
+		// thread's netns.
+		runtime.LockOSThread()
+
+		// Move the current thread to a new network namespace.
+		if err := unix.Unshare(unix.CLONE_NEWNET); err != nil {
+			return fmt.Errorf("unsharing netns: %w", err)
+		}
+
+		f, err := os.OpenFile(fmt.Sprintf("/proc/%d/task/%d/ns/net", os.Getpid(), unix.Gettid()),
+			unix.O_RDONLY|unix.O_CLOEXEC, 0)
+		if err != nil {
+			return fmt.Errorf("opening netns handle: %w", err)
+		}
+
+		// Store a namespace reference in the outer scope.
+		ns = f
+
+		return nil
+	})
+
+	if err := eg.Wait(); err != nil {
+		tb.Fatal(err)
+	}
+
+	tb.Cleanup(func() {
+		// Maintain a reference to the namespace until the end of the test, where
+		// the handle will close automatically and the namespace potentially
+		// disappears if there are no other references (veth/netkit peers, ..) to it.
+		ns.Close()
+	})
+
+	return int(ns.Fd())
+}

internal/testutils/version.go

@@ -0,0 +1,41 @@
+package testutils
+
+import (
+	"bytes"
+	"fmt"
+	"testing"
+
+	"golang.org/x/sys/unix"
+)
+
+func getKernelVersion(tb testing.TB) (maj, min, patch uint32) {
+	tb.Helper()
+
+	var uname unix.Utsname
+	if err := unix.Uname(&uname); err != nil {
+		tb.Fatalf("getting uname: %s", err)
+	}
+
+	end := bytes.IndexByte(uname.Release[:], 0)
+	versionStr := uname.Release[:end]
+
+	if count, _ := fmt.Sscanf(string(versionStr), "%d.%d.%d", &maj, &min, &patch); count < 2 {
+		tb.Fatalf("failed to parse kernel version from %s", string(versionStr))
+	}
+	return
+}
+
+// SkipOnOldKernel skips the test if the host's kernel is lower than the given
+// x.y target version.
+func SkipOnOldKernel(tb testing.TB, target, reason string) {
+	maj, min, _ := getKernelVersion(tb)
+
+	var maj_t, min_t, patch_t uint32
+	if count, _ := fmt.Sscanf(target, "%d.%d.%d", &maj_t, &min_t, &patch_t); count < 2 {
+		tb.Fatalf("failed to parse target version from %s", target)
+	}
+
+	if maj < maj_t || maj == maj_t && min < min_t {
+		tb.Skipf("host kernel (%d.%d) too old (minimum %d.%d): %s", maj, min, maj_t, min_t, reason)
+	}
+}

internal/unix/types_linux.go

@@ -209,4 +209,10 @@ const (
 	NETKIT_REDIRECT                            = linux.NETKIT_REDIRECT
 	NETKIT_L2                                  = linux.NETKIT_L2
 	NETKIT_L3                                  = linux.NETKIT_L3
+	CLONE_NEWNET                               = linux.CLONE_NEWNET
+	O_RDONLY                                   = linux.O_RDONLY
+	O_CLOEXEC                                  = linux.O_CLOEXEC
 )
+
+var Gettid = linux.Gettid
+var Unshare = linux.Unshare

internal/unix/types_other.go

@@ -205,4 +205,15 @@ const (
 	NETKIT_REDIRECT                            = 0x7
 	NETKIT_L2                                  = 0x0
 	NETKIT_L3                                  = 0x1
+	CLONE_NEWNET                               = 0x40000000
+	O_RDONLY                                   = 0x0
+	O_CLOEXEC                                  = 0x80000
 )
+
+func Unshare(_ int) error {
+	return nil
+}
+
+func Gettid() int {
+	return 0
+}

link.go

@@ -403,7 +403,7 @@ func (a *LinkAttributes) encode(ae *netlink.AttributeEncoder) error {
 	}
 
 	if a.NetNS != nil {
-		ae.Uint32(a.NetNS.Type(), a.NetNS.Value())
+		ae.Uint32(a.NetNS.value())
 	}
 
 	return nil
@@ -771,8 +771,8 @@ func (xdp *LinkXDP) encode(ae *netlink.AttributeEncoder) error {
 	ae.Int32(unix.IFLA_XDP_FD, xdp.FD)
 	ae.Int32(unix.IFLA_XDP_EXPECTED_FD, xdp.ExpectedFD)
 	ae.Uint32(unix.IFLA_XDP_FLAGS, xdp.Flags)
-	// XDP_ATtACHED and XDP_PROG_ID are things that only can return from the kernel,
-	// not be send, so we don't encode them.
-	// source: https://elixir.bootlin.com/linux/v5.10.15/source/net/core/rtnetlink.c#L2894
+	// XDP_ATTACHED and XDP_PROG_ID are things that can only be returned by the
+	// kernel, so we don't encode them. source:
+	// https://elixir.bootlin.com/linux/v5.10.15/source/net/core/rtnetlink.c#L2894
 	return nil
 }