Repository: cilium/ebpf
Pull request #1708 (2025-03-03 14:38:09)

windows: more test / testutils fixups
testutils: make IsVersionLessThan cross platform

    We usually rely on feature tests to skip certain portions of a test. This
    causes a lot of overhead when the feature tests would only be used by the
    unit tests.

    For that reason the info tests skip some tests based on the current kernel
    version. Extend this concept to other platforms by reusing the selection
    logic introduced for feature tests.

    Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

testutils: add DupFD helper

    Add a cross-platform DupFD helper and use it from tests.

    Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

ebpf: run TestProgramTestRunInterrupt only on Linux

    TestProgramTestRunInterrupt tests Linux specific behaviour around signal 
    delivery. It doesn't make sense to run it on a different platform.

    The test was copied without making any changes.

    Signed-off-by: Lorenz Bauer <lmb@isovalent.com>



Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 3d313a5..353a673 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -240,6 +240,7 @@ jobs:
       GOMODCACHE: D:\gomodcache
       # Avoid putting temp on slow C:
       TEMP: D:\temp
+      CI_EFW_VERSION: "0.20.0"
 
     steps:
       - run: mkdir D:\temp
diff --git a/internal/feature.go b/internal/feature.go
index 356e924..2ec87a1 100644
--- a/internal/feature.go
+++ b/internal/feature.go
@@ -4,7 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"runtime"
-	"strings"
 	"sync"
 
 	"github.com/cilium/ebpf/internal/platform"
@@ -76,33 +75,12 @@ type FeatureTestFn func() error
 // Linux. Returns [ErrNotSupportedOnOS] if there is no version specified for the
 // current OS.
 func NewFeatureTest(name string, fn FeatureTestFn, versions ...string) func() error {
-	const nativePrefix = runtime.GOOS + ":"
-
-	if len(versions) == 0 {
-		return func() error {
-			return fmt.Errorf("feature test %q: no versions specified", name)
-		}
-	}
-
-	ft := &FeatureTest{
-		Name: name,
-		Fn:   fn,
-	}
-
-	for _, version := range versions {
-		if strings.HasPrefix(version, nativePrefix) {
-			ft.Version = strings.TrimPrefix(version, nativePrefix)
-			break
-		}
-
-		if platform.IsLinux && !strings.ContainsRune(version, ':') {
-			// Allow version numbers without a GOOS prefix on Linux.
-			ft.Version = version
-			break
-		}
+	version, err := platform.SelectVersion(versions)
+	if err != nil {
+		return func() error { return err }
 	}
 
-	if ft.Version == "" {
+	if version == "" {
 		return func() error {
 			// We don't return an UnsupportedFeatureError here, since that will
 			// trigger version checks which don't make sense.
@@ -110,6 +88,12 @@ func NewFeatureTest(name string, fn FeatureTestFn, versions ...string) func() er
 		}
 	}
 
+	ft := &FeatureTest{
+		Name:    name,
+		Version: version,
+		Fn:      fn,
+	}
+
 	return ft.execute
 }
 
diff --git a/internal/platform/platform.go b/internal/platform/platform.go
index 065c85d..1d9ffa1 100644
--- a/internal/platform/platform.go
+++ b/internal/platform/platform.go
@@ -1,6 +1,10 @@
 package platform
 
-import "runtime"
+import (
+	"errors"
+	"runtime"
+	"strings"
+)
 
 const (
 	Linux = "linux"
@@ -10,3 +14,28 @@ const (
 	IsLinux   = runtime.GOOS == "linux"
 	IsWindows = runtime.GOOS == "windows"
 )
+
+// SelectVersion extracts the platform-appropriate version from a list of strings like
+// `linux:6.1` or `windows:0.20.0`.
+//
+// Returns an empty string and nil if no version matched or an error if no strings were passed.
+func SelectVersion(versions []string) (string, error) {
+	const prefix = runtime.GOOS + ":"
+
+	if len(versions) == 0 {
+		return "", errors.New("no versions specified")
+	}
+
+	for _, version := range versions {
+		if strings.HasPrefix(version, prefix) {
+			return strings.TrimPrefix(version, prefix), nil
+		}
+
+		if IsLinux && !strings.ContainsRune(version, ':') {
+			// Allow version numbers without a GOOS prefix on Linux.
+			return version, nil
+		}
+	}
+
+	return "", nil
+}


Test patch:
diff --git a/info_test.go b/info_test.go
index febdf44..89f151d 100644
--- a/info_test.go
+++ b/info_test.go
@@ -105,48 +105,48 @@ func TestProgramInfo(t *testing.T) {
 	qt.Assert(t, qt.IsTrue(ok))
 	qt.Assert(t, qt.Not(qt.Equals(id, 0)))
 
-	if testutils.IsKernelLessThan(t, "4.15") {
+	if testutils.IsVersionLessThan(t, "4.15") {
 		qt.Assert(t, qt.Equals(info.Name, ""))
 	} else {
 		qt.Assert(t, qt.Equals(info.Name, "test"))
 	}
 
-	if jitedSize, err := info.JitedSize(); testutils.IsKernelLessThan(t, "4.13") {
+	if jitedSize, err := info.JitedSize(); testutils.IsVersionLessThan(t, "4.13") {
 		qt.Assert(t, qt.IsNotNil(err))
 	} else {
 		qt.Assert(t, qt.IsNil(err))
 		qt.Assert(t, qt.IsTrue(jitedSize > 0))
 	}
 
-	if xlatedSize, err := info.TranslatedSize(); testutils.IsKernelLessThan(t, "4.13") {
+	if xlatedSize, err := info.TranslatedSize(); testutils.IsVersionLessThan(t, "4.13") {
 		qt.Assert(t, qt.IsNotNil(err))
 	} else {
 		qt.Assert(t, qt.IsNil(err))
 		qt.Assert(t, qt.IsTrue(xlatedSize > 0))
 	}
 
-	if uid, ok := info.CreatedByUID(); testutils.IsKernelLessThan(t, "4.15") {
+	if uid, ok := info.CreatedByUID(); testutils.IsVersionLessThan(t, "4.15") {
 		qt.Assert(t, qt.IsFalse(ok))
 	} else {
 		qt.Assert(t, qt.IsTrue(ok))
 		qt.Assert(t, qt.Equals(uid, uint32(os.Getuid())))
 	}
 
-	if loadTime, ok := info.LoadTime(); testutils.IsKernelLessThan(t, "4.15") {
+	if loadTime, ok := info.LoadTime(); testutils.IsVersionLessThan(t, "4.15") {
 		qt.Assert(t, qt.IsFalse(ok))
 	} else {
 		qt.Assert(t, qt.IsTrue(ok))
 		qt.Assert(t, qt.IsTrue(loadTime > 0))
 	}
 
-	if verifiedInsns, ok := info.VerifiedInstructions(); testutils.IsKernelLessThan(t, "5.16") {
+	if verifiedInsns, ok := info.VerifiedInstructions(); testutils.IsVersionLessThan(t, "5.16") {
 		qt.Assert(t, qt.IsFalse(ok))
 	} else {
 		qt.Assert(t, qt.IsTrue(ok))
 		qt.Assert(t, qt.IsTrue(verifiedInsns > 0))
 	}
 
-	if insns, ok := info.JitedInsns(); testutils.IsKernelLessThan(t, "4.13") {
+	if insns, ok := info.JitedInsns(); testutils.IsVersionLessThan(t, "4.13") {
 		qt.Assert(t, qt.IsFalse(ok))
 	} else {
 		qt.Assert(t, qt.IsTrue(ok))
@@ -175,28 +175,28 @@ func TestProgramInfoBTF(t *testing.T) {
 
 	// On kernels before 5.x, nr_jited_ksyms is not set for programs without subprogs.
 	// It's included here since this test uses a bpf program with subprogs.
-	if addrs, ok := info.JitedKsymAddrs(); testutils.IsKernelLessThan(t, "4.18") {
+	if addrs, ok := info.JitedKsymAddrs(); testutils.IsVersionLessThan(t, "4.18") {
 		qt.Assert(t, qt.IsFalse(ok))
 	} else {
 		qt.Assert(t, qt.IsTrue(ok))
 		qt.Assert(t, qt.IsTrue(len(addrs) > 0))
 	}
 
-	if lens, ok := info.JitedFuncLens(); testutils.IsKernelLessThan(t, "4.18") {
+	if lens, ok := info.JitedFuncLens(); testutils.IsVersionLessThan(t, "4.18") {
 		qt.Assert(t, qt.IsFalse(ok))
 	} else {
 		qt.Assert(t, qt.IsTrue(ok))
 		qt.Assert(t, qt.IsTrue(len(lens) > 0))
 	}
 
-	if infos, ok := info.JitedLineInfos(); testutils.IsKernelLessThan(t, "5.0") {
+	if infos, ok := info.JitedLineInfos(); testutils.IsVersionLessThan(t, "5.0") {
 		qt.Assert(t, qt.IsFalse(ok))
 	} else {
 		qt.Assert(t, qt.IsTrue(ok))
 		qt.Assert(t, qt.IsTrue(len(infos) > 0))
 	}
 
-	if funcs, err := info.FuncInfos(); testutils.IsKernelLessThan(t, "5.0") {
+	if funcs, err := info.FuncInfos(); testutils.IsVersionLessThan(t, "5.0") {
 		qt.Assert(t, qt.IsNotNil(err))
 	} else {
 		qt.Assert(t, qt.IsNil(err))
@@ -205,7 +205,7 @@ func TestProgramInfoBTF(t *testing.T) {
 		qt.Assert(t, qt.ContentEquals(funcs[1].Func, btfFn))
 	}
 
-	if lines, err := info.LineInfos(); testutils.IsKernelLessThan(t, "5.0") {
+	if lines, err := info.LineInfos(); testutils.IsVersionLessThan(t, "5.0") {
 		qt.Assert(t, qt.IsNotNil(err))
 	} else {
 		qt.Assert(t, qt.IsNil(err))
@@ -234,7 +234,7 @@ func TestProgramInfoMapIDs(t *testing.T) {
 
 	ids, ok := info.MapIDs()
 	switch {
-	case testutils.IsKernelLessThan(t, "4.15"):
+	case testutils.IsVersionLessThan(t, "4.15"):
 		qt.Assert(t, qt.IsFalse(ok))
 		qt.Assert(t, qt.HasLen(ids, 0))
 
@@ -259,7 +259,7 @@ func TestProgramInfoMapIDsNoMaps(t *testing.T) {
 
 	ids, ok := info.MapIDs()
 	switch {
-	case testutils.IsKernelLessThan(t, "4.15"):
+	case testutils.IsVersionLessThan(t, "4.15"):
 		qt.Assert(t, qt.IsFalse(ok))
 		qt.Assert(t, qt.HasLen(ids, 0))
 
diff --git a/internal/testutils/fd_other.go b/internal/testutils/fd_other.go
new file mode 100644
index 0000000..f26b5c8
--- /dev/null
+++ b/internal/testutils/fd_other.go
@@ -0,0 +1,20 @@
+//go:build !windows
+
+package testutils
+
+import (
+	"testing"
+
+	"github.com/go-quicktest/qt"
+
+	"github.com/cilium/ebpf/internal/unix"
+)
+
+func DupFD(tb testing.TB, fd int) int {
+	tb.Helper()
+
+	dup, err := unix.FcntlInt(uintptr(fd), unix.F_DUPFD_CLOEXEC, 1)
+	qt.Assert(tb, qt.IsNil(err))
+
+	return dup
+}
diff --git a/internal/testutils/fd_windows.go b/internal/testutils/fd_windows.go
new file mode 100644
index 0000000..b6d7441
--- /dev/null
+++ b/internal/testutils/fd_windows.go
@@ -0,0 +1,17 @@
+package testutils
+
+import (
+	"testing"
+
+	"github.com/cilium/ebpf/internal/efw"
+	"github.com/go-quicktest/qt"
+)
+
+func DupFD(tb testing.TB, fd int) int {
+	tb.Helper()
+
+	dup, err := efw.EbpfDuplicateFd(fd)
+	qt.Assert(tb, qt.IsNil(err))
+
+	return dup
+}
diff --git a/internal/testutils/feature.go b/internal/testutils/feature.go
index 94f6048..42b4245 100644
--- a/internal/testutils/feature.go
+++ b/internal/testutils/feature.go
@@ -7,6 +7,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/go-quicktest/qt"
+
 	"github.com/cilium/ebpf/internal"
 	"github.com/cilium/ebpf/internal/platform"
 )
@@ -104,25 +106,27 @@ func SkipOnOldKernel(tb testing.TB, minVersion, feature string) {
 		return
 	}
 
-	if IsKernelLessThan(tb, minVersion) {
+	if IsVersionLessThan(tb, minVersion) {
 		tb.Skipf("Test requires at least kernel %s (due to missing %s)", minVersion, feature)
 	}
 }
 
-// Check whether the running Linux version is smaller than a specific version.
-//
-// Warning: this function always returns false on platforms other than Linux.
-func IsKernelLessThan(tb testing.TB, minVersion string) bool {
+// Check whether the current runtime version is less than some minimum.
+func IsVersionLessThan(tb testing.TB, minVersions ...string) bool {
 	tb.Helper()
 
-	if !platform.IsLinux {
-		tb.Logf("Ignoring version constraint %s on %s", minVersion, runtime.GOOS)
-		return false
+	version, err := platform.SelectVersion(minVersions)
+	qt.Assert(tb, qt.IsNil(err))
+
+	if version == "" {
+		// No matching version means that the platform
+		// doesn't support whatever feature.
+		return true
 	}
 
-	minv, err := internal.NewVersion(minVersion)
+	minv, err := internal.NewVersion(version)
 	if err != nil {
-		tb.Fatalf("Invalid version %s: %s", minVersion, err)
+		tb.Fatalf("Invalid version %s: %s", version, err)
 	}
 
 	return isPlatformVersionLessThan(tb, minv, platformVersion(tb))
diff --git a/internal/testutils/feature_windows.go b/internal/testutils/feature_windows.go
index fa4f146..f5a586d 100644
--- a/internal/testutils/feature_windows.go
+++ b/internal/testutils/feature_windows.go
@@ -11,8 +11,8 @@ import (
 
 func platformVersion(tb testing.TB) internal.Version {
 	tb.Helper()
-	versionStr, ok := os.LookupEnv("CI_MAX_EFW_VERSION")
-	qt.Assert(tb, qt.IsTrue(ok), qt.Commentf("Missing CI_MAX_EFW_VERSION environment variable"))
+	versionStr, ok := os.LookupEnv("CI_EFW_VERSION")
+	qt.Assert(tb, qt.IsTrue(ok), qt.Commentf("Missing CI_EFW_VERSION environment variable"))
 	version, err := internal.NewVersion(versionStr)
 	qt.Assert(tb, qt.IsNil(err), qt.Commentf("Parse eBPF for Windows version"))
 	return version
diff --git a/map_test.go b/map_test.go
index 6f4f1e9..ac7122c 100644
--- a/map_test.go
+++ b/map_test.go
@@ -804,7 +804,7 @@ func TestNewMapInMapFromFD(t *testing.T) {
 	nested := createMapInMap(t, ArrayOfMaps, Array)
 
 	// Do not copy this, use Clone instead.
-	another, err := NewMapFromFD(dupFD(t, nested.FD()))
+	another, err := NewMapFromFD(testutils.DupFD(t, nested.FD()))
 	if err != nil {
 		t.Fatal("Can't create a new nested map from an FD")
 	}
@@ -1388,7 +1388,7 @@ func TestMapFromFD(t *testing.T) {
 
 	// If you're thinking about copying this, don't. Use
 	// Clone() instead.
-	m2, err := NewMapFromFD(dupFD(t, m.FD()))
+	m2, err := NewMapFromFD(testutils.DupFD(t, m.FD()))
 	testutils.SkipIfNotSupported(t, err)
 	if err != nil {
 		t.Fatal(err)
diff --git a/prog_linux_test.go b/prog_linux_test.go
new file mode 100644
index 0000000..dcc1990
--- /dev/null
+++ b/prog_linux_test.go
@@ -0,0 +1,84 @@
+package ebpf
+
+import (
+	"math"
+	"runtime"
+	"testing"
+	"time"
+
+	"github.com/cilium/ebpf/internal"
+	"github.com/cilium/ebpf/internal/testutils"
+	"github.com/cilium/ebpf/internal/unix"
+)
+
+func TestProgramTestRunInterrupt(t *testing.T) {
+	testutils.SkipOnOldKernel(t, "5.0", "EINTR from BPF_PROG_TEST_RUN")
+
+	prog := createBasicProgram(t)
+
+	var (
+		tgid    = unix.Getpid()
+		tidChan = make(chan int, 1)
+		exit    = make(chan struct{})
+		errs    = make(chan error, 1)
+		timeout = time.After(5 * time.Second)
+	)
+
+	defer close(exit)
+
+	go func() {
+		runtime.LockOSThread()
+		defer func() {
+			// Wait for the test to allow us to unlock the OS thread, to
+			// ensure that we don't send SIGUSR1 to the wrong thread.
+			<-exit
+			runtime.UnlockOSThread()
+		}()
+
+		tidChan <- unix.Gettid()
+
+		// Block this thread in the BPF syscall, so that we can
+		// trigger EINTR by sending a signal.
+		opts := RunOptions{
+			Data:   internal.EmptyBPFContext,
+			Repeat: math.MaxInt32,
+			Reset: func() {
+				// We don't know how long finishing the
+				// test run would take, so flag that we've seen
+				// an interruption and abort the goroutine.
+				close(errs)
+				runtime.Goexit()
+			},
+		}
+		_, _, err := prog.run(&opts)
+
+		errs <- err
+	}()
+
+	tid := <-tidChan
+	for {
+		err := unix.Tgkill(tgid, tid, unix.SIGUSR1)
+		if err != nil {
+			t.Fatal("Can't send signal to goroutine thread:", err)
+		}
+
+		select {
+		case err, ok := <-errs:
+			if !ok {
+				return
+			}
+
+			testutils.SkipIfNotSupported(t, err)
+			if err == nil {
+				t.Fatal("testRun wasn't interrupted")
+			}
+
+			t.Fatal("testRun returned an error:", err)
+
+		case <-timeout:
+			t.Fatal("Timed out trying to interrupt the goroutine")
+
+		default:
+		}
+	}
+}
diff --git a/prog_test.go b/prog_test.go
index ff2fc07..0801cc2 100644
--- a/prog_test.go
+++ b/prog_test.go
@@ -8,12 +8,10 @@ import (
 	"math"
 	"os"
 	"path/filepath"
-	"runtime"
 	"slices"
 	"strings"
 	"syscall"
 	"testing"
-	"time"
 
 	"github.com/go-quicktest/qt"
 
@@ -182,78 +180,6 @@ func TestProgramBenchmark(t *testing.T) {
 	}
 }
 
-func TestProgramTestRunInterrupt(t *testing.T) {
-	testutils.SkipOnOldKernel(t, "5.0", "EINTR from BPF_PROG_TEST_RUN")
-
-	prog := createBasicProgram(t)
-
-	var (
-		tgid    = unix.Getpid()
-		tidChan = make(chan int, 1)
-		exit    = make(chan struct{})
-		errs    = make(chan error, 1)
-		timeout = time.After(5 * time.Second)
-	)
-
-	defer close(exit)
-
-	go func() {
-		runtime.LockOSThread()
-		defer func() {
-			// Wait for the test to allow us to unlock the OS thread, to
-			// ensure that we don't send SIGUSR1 to the wrong thread.
-			<-exit
-			runtime.UnlockOSThread()
-		}()
-
-		tidChan <- unix.Gettid()
-
-		// Block this thread in the BPF syscall, so that we can
-		// trigger EINTR by sending a signal.
-		opts := RunOptions{
-			Data:   internal.EmptyBPFContext,
-			Repeat: math.MaxInt32,
-			Reset: func() {
-				// We don't know how long finishing the
-				// test run would take, so flag that we've seen
-				// an interruption and abort the goroutine.
-				close(errs)
-				runtime.Goexit()
-			},
-		}
-		_, _, err := prog.run(&opts)
-
-		errs <- err
-	}()
-
-	tid := <-tidChan
-	for {
-		err := unix.Tgkill(tgid, tid, unix.SIGUSR1)
-		if err != nil {
-			t.Fatal("Can't send signal to goroutine thread:", err)
-		}
-
-		select {
-		case err, ok := <-errs:
-			if !ok {
-				return
-			}
-
-			testutils.SkipIfNotSupported(t, err)
-			if err == nil {
-				t.Fatal("testRun wasn't interrupted")
-			}
-
-			t.Fatal("testRun returned an error:", err)
-
-		case <-timeout:
-			t.Fatal("Timed out trying to interrupt the goroutine")
-
-		default:
-		}
-	}
-}
-
 func TestProgramClose(t *testing.T) {
 	prog := createBasicProgram(t)
 
@@ -592,7 +518,7 @@ func TestProgramFromFD(t *testing.T) {
 
 	// If you're thinking about copying this, don't. Use
 	// Clone() instead.
-	prog2, err := NewProgramFromFD(dupFD(t, prog.FD()))
+	prog2, err := NewProgramFromFD(testutils.DupFD(t, prog.FD()))
 	testutils.SkipIfNotSupported(t, err)
 	if err != nil {
 		t.Fatal(err)
@@ -1087,14 +1013,3 @@ func ExampleProgramSpec_Tag() {
 		fmt.Println("The programs are identical, tag is", tag)
 	}
 }
-
-func dupFD(tb testing.TB, fd int) int {
-	tb.Helper()
-
-	dup, err := unix.FcntlInt(uintptr(fd), unix.F_DUPFD_CLOEXEC, 1)
-	if err != nil {
-		tb.Fatal("Can't dup fd:", err)
-	}
-
-	return dup
-}