prog: remove PROG_RUN retry on EINTR and deprecated RunOptions.Reset option

RunOptions.Reset was primarily added to make resetting testing.B possible in
case of EINTR. However, ebpf-go now masks SIGPROF during BPF_PROG_LOAD and
BPF_PROG_RUN to prevent the Go profiler from endlessely interrupting the
verifier when (continuous) profiling is enabled.

Of course, the process can still get signaled by other processes, but this
would likely be some form of kill or interrupt signal. In the case of
PROG_RUN with a significant Repeat value, this signal should probably be
respected and Program.Run shouldn't stubbornly continue on.

Signed-off-by: Timo Beckers <timo@isovalent.com>

Author: ti-mo

prog.go

@@ -745,11 +745,6 @@ type RunOptions struct {
 	// CPU to run Program on. Optional field.
 	// Note not all program types support this field.
 	CPU uint32
-	// Called whenever the syscall is interrupted, and should be set to testing.B.ResetTimer
-	// or similar. Typically used during benchmarking. Optional field.
-	//
-	// Deprecated: use [testing.B.ReportMetric] with unit "ns/op" instead.
-	Reset func()
 }
 
 // Test runs the Program in the kernel with the given input and returns the
@@ -802,19 +797,17 @@ func (p *Program) Run(opts *RunOptions) (uint32, error) {
 // and returns the time taken per iteration.
 //
 // Returns the result of the last execution of the program and the time per
-// run or an error. reset is called whenever the benchmark syscall is
-// interrupted, and should be set to testing.B.ResetTimer or similar.
+// run or an error.
 //
 // This function requires at least Linux 4.12.
-func (p *Program) Benchmark(in []byte, repeat int, reset func()) (uint32, time.Duration, error) {
+func (p *Program) Benchmark(in []byte, repeat int) (uint32, time.Duration, error) {
 	if uint(repeat) > math.MaxUint32 {
 		return 0, 0, fmt.Errorf("repeat is too high")
 	}
 
 	opts := RunOptions{
 		Data:   in,
 		Repeat: uint32(repeat),
-		Reset:  reset,
 	}
 
 	ret, total, err := p.run(&opts)
@@ -923,40 +916,14 @@ func (p *Program) run(opts *RunOptions) (uint32, time.Duration, error) {
 		ctxOut = ctxIn
 	}
 
-retry:
-	for {
-		err := sys.ProgRun(&attr)
-		if err == nil {
-			break retry
-		}
-
-		if errors.Is(err, unix.EINTR) {
-			if attr.Repeat <= 1 {
-				// Older kernels check whether enough repetitions have been
-				// executed only after checking for pending signals.
-				//
-				//     run signal? done? run ...
-				//
-				// As a result we can get EINTR for repeat==1 even though
-				// the program was run exactly once. Treat this as a
-				// successful run instead.
-				//
-				// Since commit 607b9cc92bd7 ("bpf: Consolidate shared test timing code")
-				// the conditions are reversed:
-				//     run done? signal? ...
-				break retry
-			}
-
-			if opts.Reset != nil {
-				opts.Reset()
-			}
-			continue retry
-		}
-
-		if errors.Is(err, sys.ENOTSUPP) {
-			return 0, 0, fmt.Errorf("kernel doesn't support running %s: %w", p.Type(), ErrNotSupported)
-		}
-
+	err := sys.ProgRun(&attr)
+	if errors.Is(err, unix.EINTR) {
+		return 0, 0, fmt.Errorf("program run interrupted: %w", err)
+	}
+	if errors.Is(err, sys.ENOTSUPP) {
+		return 0, 0, fmt.Errorf("kernel doesn't support running %s: %w", p.Type(), ErrNotSupported)
+	}
+	if err != nil {
 		return 0, 0, err
 	}
 

prog_linux_test.go

@@ -48,13 +48,6 @@ func TestProgramTestRunInterrupt(t *testing.T) {
 		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)
 
@@ -63,28 +56,19 @@ func TestProgramTestRunInterrupt(t *testing.T) {
 
 	tid := <-tidChan
 	for {
-		err := unix.Tgkill(tgid, tid, unix.SIGUSR1)
-		if err != nil {
-			t.Fatal("Can't send signal to goroutine thread:", err)
-		}
+		qt.Assert(t, qt.IsNil(unix.Tgkill(tgid, tid, unix.SIGUSR1)))
 
 		select {
-		case err, ok := <-errs:
-			if !ok {
-				return
-			}
-
+		case err := <-errs:
 			testutils.SkipIfNotSupported(t, err)
-			if err == nil {
-				t.Fatal("testRun wasn't interrupted")
-			}
-
-			t.Fatal("testRun returned an error:", err)
+			qt.Assert(t, qt.ErrorIs(err, unix.EINTR))
+			return
 
 		case <-timeout:
 			t.Fatal("Timed out trying to interrupt the goroutine")
 
 		default:
+			time.Sleep(time.Millisecond * 100)
 		}
 	}
 }

prog_test.go

@@ -158,7 +158,7 @@ func TestProgramBenchmark(t *testing.T) {
 
 	prog := createBasicProgram(t)
 
-	ret, duration, err := prog.Benchmark(internal.EmptyBPFContext, 1, nil)
+	ret, duration, err := prog.Benchmark(internal.EmptyBPFContext, 1)
 	testutils.SkipIfNotSupported(t, err)
 	if err != nil {
 		t.Fatal("Error from Benchmark:", err)