features: centralize error semantics, remove maxMiscType, etc.

asm/instruction.go

@@ -20,7 +20,7 @@ const InstructionSize = 8
 type RawInstructionOffset uint64
 
 var ErrUnsatisfiedMapReference = errors.New("unsatisfied map reference")
-var ErrUnsatisfiedReference = errors.New("unsatisfied reference")
+var ErrUnsatisfiedProgramReference = errors.New("unsatisfied program reference")
 
 // Bytes returns the offset of an instruction in bytes.
 func (rio RawInstructionOffset) Bytes() uint64 {
@@ -180,17 +180,6 @@ func (ins *Instruction) RewriteMapOffset(offset uint32) error {
 	return nil
 }
 
-// RewriteJumpOffset sets the offset for a jump operation.
-//
-// Returns an error if the instruction is not a jump operation.
-func (ins *Instruction) RewriteJumpOffset(offset int16) error {
-	if ins.OpCode.JumpOp() == InvalidJumpOp {
-		return errors.New("not a jump operation")
-	}
-	ins.Offset = offset
-	return nil
-}
-
 func (ins *Instruction) mapOffset() uint32 {
 	return uint32(uint64(ins.Constant) >> 32)
 }
@@ -571,31 +560,26 @@ func (insns Instructions) FixupReferences() error {
 			continue
 		}
 
-		symOffset, ok := symbolOffsets[ins.Reference]
 		switch {
 		case ins.IsFunctionReference() && ins.Constant == -1:
+			symOffset, ok := symbolOffsets[ins.Reference]
 			if !ok {
-				break
+				return fmt.Errorf("%s at insn %d: symbol %q: %w", ins.OpCode, i, ins.Reference, ErrUnsatisfiedProgramReference)
 			}
 
 			ins.Constant = int64(symOffset - offset - 1)
-			continue
 
 		case ins.OpCode.Class().IsJump() && ins.Offset == -1:
+			symOffset, ok := symbolOffsets[ins.Reference]
 			if !ok {
-				break
+				return fmt.Errorf("%s at insn %d: symbol %q: %w", ins.OpCode, i, ins.Reference, ErrUnsatisfiedProgramReference)
 			}
 
 			ins.Offset = int16(symOffset - offset - 1)
-			continue
 
 		case ins.IsLoadFromMap() && ins.MapPtr() == -1:
 			return fmt.Errorf("map %s: %w", ins.Reference, ErrUnsatisfiedMapReference)
-		default:
-			// no fixup needed
-			continue
 		}
-		return fmt.Errorf("%s at insn %d: symbol %q: %w", ins.OpCode, i, ins.Reference, ErrUnsatisfiedReference)
 	}
 	return nil
 }

features/doc.go

@@ -1,2 +1,19 @@
 // Package features allows probing for BPF features available to the calling process.
+//
+// In general, the error return values from feature probes in this package
+// all have the following semantics unless otherwise specified:
+//
+//   err == nil: The feature is available.
+//   errors.Is(err, ebpf.ErrNotSupported): The feature is not available.
+//   err != nil: Any errors encountered during probe execution, wrapped.
+//
+// Note that the latter case may include false negatives, and that resource
+// creation may succeed despite an error being returned. For example, some
+// map and program types cannot reliably be probed and will return an
+// inconclusive error.
+//
+// As a rule, only `nil` and `ebpf.ErrNotSupported` are conclusive.
+//
+// Probe results are cached by the library and persist throughout any changes
+// to the process' environment, like capability changes.
 package features

features/map.go

@@ -96,17 +96,8 @@ func createMapTypeAttr(mt ebpf.MapType) *sys.MapCreateAttr {
 }
 
 // HaveMapType probes the running kernel for the availability of the specified map type.
-// Return values have the following semantics:
 //
-//   err == nil: The feature is available.
-//   errors.Is(err, ebpf.ErrNotSupported): The feature is not available.
-//   err != nil: Any errors encountered during probe execution, wrapped.
-//
-// Note that the latter case may include false negatives, and that map creation may succeed
-// despite an error being returned. Some map types cannot reliably be probed and will also
-// return error. Only `nil` and `ebpf.ErrNotSupported` are conclusive.
-//
-// Probe results are cached and persist throughout any process capability changes.
+// See the package documentation for the meaning of the error return value.
 func HaveMapType(mt ebpf.MapType) error {
 	if err := validateMaptype(mt); err != nil {
 		return err

features/misc.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
-	"os"
 	"sync"
 
 	"github.com/cilium/ebpf"
@@ -15,7 +14,7 @@ import (
 )
 
 func init() {
-	miscs.miscTypes = make(map[miscType]error, maxMiscType)
+	miscs.miscTypes = make(map[miscType]error)
 }
 
 var (
@@ -29,11 +28,6 @@ type miscCache struct {
 
 type miscType uint32
 
-// Max returns the latest supported MiscType.
-func (_ miscType) max() miscType {
-	return maxMiscType - 1
-}
-
 const (
 	// largeInsn support introduced in
 	// commit c04c0d2b968ac45d6ef020316808ef6c82325a82
@@ -47,8 +41,6 @@ const (
 	// v3ISA support introduced in
 	// commit 092ed0968bb648cd18e8a0430cd0a8a71727315c
 	v3ISA
-	// maxMiscType - Bound enum of FeatureTypes, has to be last in enum.
-	maxMiscType
 )
 
 const (
@@ -57,76 +49,36 @@ const (
 
 // HaveLargeInstructions probes the running kernel if more than 4096 instructions
 // per program are supported.
-// Return values have the following semantics:
-//
-//   err == nil: The feature is available.
-//   errors.Is(err, ebpf.ErrNotSupported): The feature is not available.
-//   err != nil: Any errors encountered during probe execution, wrapped.
 //
-// Note that the latter case may include false negatives, and that program creation may
-// succeed despite an error being returned. Some program types cannot reliably be probed and
-// will also return error. Only `nil` and `ebpf.ErrNotSupported` are conclusive.
-//
-// Probe results are cached and persist throughout any process capability changes.
+// See the package documentation for the meaning of the error return value.
 func HaveLargeInstructions() error {
 	return probeMisc(largeInsn)
 }
 
 // HaveBoundedLoops probes the running kernel if bounded loops are supported.
-// Return values have the following semantics:
-//
-//   err == nil: The feature is available.
-//   errors.Is(err, ebpf.ErrNotSupported): The feature is not available.
-//   err != nil: Any errors encountered during probe execution, wrapped.
-//
-// Note that the latter case may include false negatives, and that program creation may
-// succeed despite an error being returned. Some program types cannot reliably be probed and
-// will also return error. Only `nil` and `ebpf.ErrNotSupported` are conclusive.
 //
-// Probe results are cached and persist throughout any process capability changes.
+// See the package documentation for the meaning of the error return value.
 func HaveBoundedLoops() error {
 	return probeMisc(boundedLoops)
 }
 
 // HaveV2ISA probes the running kernel if instructions of the v2 ISA are supported.
-// Return values have the following semantics:
 //
-//   err == nil: The feature is available.
-//   errors.Is(err, ebpf.ErrNotSupported): The feature is not available.
-//   err != nil: Any errors encountered during probe execution, wrapped.
-//
-// Note that the latter case may include false negatives, and that program creation may
-// succeed despite an error being returned. Some program types cannot reliably be probed and
-// will also return error. Only `nil` and `ebpf.ErrNotSupported` are conclusive.
-//
-// Probe results are cached and persist throughout any process capability changes.
+// See the package documentation for the meaning of the error return value.
 func HaveV2ISA() error {
 	return probeMisc(v2ISA)
 }
 
 // HaveV3ISA probes the running kernel if instructions of the v3 ISA are supported.
-// Return values have the following semantics:
-//
-//   err == nil: The feature is available.
-//   errors.Is(err, ebpf.ErrNotSupported): The feature is not available.
-//   err != nil: Any errors encountered during probe execution, wrapped.
 //
-// Note that the latter case may include false negatives, and that program creation may
-// succeed despite an error being returned. Some program types cannot reliably be probed and
-// will also return error. Only `nil` and `ebpf.ErrNotSupported` are conclusive.
-//
-// Probe results are cached and persist throughout any process capability changes.
+// See the package documentation for the meaning of the error return value.
 func HaveV3ISA() error {
 	return probeMisc(v3ISA)
 }
 
 // probeMisc checks the kernel for a given supported misc by creating
 // a specialized program probe and loading it.
-// Results are cached and persist throughout any process capability changes.
 func probeMisc(mt miscType) error {
-	if mt > mt.max() {
-		return os.ErrInvalid
-	}
 	mc.Lock()
 	defer mc.Unlock()
 	err, ok := miscs.miscTypes[mt]
@@ -195,10 +147,8 @@ func createMiscProbeAttr(mt miscType) (*sys.ProgLoadAttr, error) {
 			asm.Return(),
 		}
 		// To test the v2 ISA we need a dedicated jump offset other
-		// than the one we would get from Instruction.FixupReferences().
-		if err := insns[1].RewriteJumpOffset(1); err != nil {
-			return nil, err
-		}
+		// than the one we would get from Instructions.FixupReferences().
+		insns[1].Offset = 1
 	case v3ISA:
 		label = "v3isa"
 		insns = asm.Instructions{

features/misc_test.go

@@ -1,21 +1,12 @@
 package features
 
 import (
-	"errors"
 	"fmt"
-	"math"
-	"os"
 	"testing"
 
 	"github.com/cilium/ebpf/internal/testutils"
 )
 
-func TestInvalidMisc(t *testing.T) {
-	if err := probeMisc(miscType(math.MaxUint32)); !errors.Is(err, os.ErrInvalid) {
-		t.Fatalf("Expected os.ErrInvalid but was: %v", err)
-	}
-}
-
 func TestHaveMisc(t *testing.T) {
 	tests := map[miscType]struct {
 		probe     func() error

features/prog.go

@@ -79,17 +79,8 @@ func createProgLoadAttr(pt ebpf.ProgramType) (*sys.ProgLoadAttr, error) {
 }
 
 // HaveProgType probes the running kernel for the availability of the specified program type.
-// Return values have the following semantics:
 //
-//   err == nil: The feature is available.
-//   errors.Is(err, ebpf.ErrNotSupported): The feature is not available.
-//   err != nil: Any errors encountered during probe execution, wrapped.
-//
-// Note that the latter case may include false negatives, and that program creation may
-// succeed despite an error being returned. Some program types cannot reliably be probed and
-// will also return error. Only `nil` and `ebpf.ErrNotSupported` are conclusive.
-//
-// Probe results are cached and persist throughout any process capability changes.
+// See the package documentation for the meaning of the error return value.
 func HaveProgType(pt ebpf.ProgramType) error {
 	if err := validateProgType(pt); err != nil {
 		return err

linker_test.go

@@ -74,8 +74,8 @@ func TestForwardFunctionDeclaration(t *testing.T) {
 
 		// This program calls an unimplemented forward function declaration.
 		_, err = NewProgram(spec)
-		if !errors.Is(err, asm.ErrUnsatisfiedReference) {
-			t.Fatal("Expected an error wrapping errUnsatisfiedProgram, got:", err)
+		if !errors.Is(err, asm.ErrUnsatisfiedProgramReference) {
+			t.Fatal("Expected an error wrapping ErrUnsatisfiedProgramReference, got:", err)
 		}
 
 		// Append the implementation of fwd().

prog_test.go

@@ -375,7 +375,7 @@ func TestProgramWithUnsatisfiedMap(t *testing.T) {
 
 	_, err = NewProgram(progSpec)
 	if !errors.Is(err, asm.ErrUnsatisfiedMapReference) {
-		t.Fatal("Expected an error wrapping errUnsatisfiedMapReference, got", err)
+		t.Fatal("Expected an error wrapping asm.ErrUnsatisfiedMapReference, got", err)
 	}
 	t.Log(err)
 }