Repository: cilium/ebpf
Pull request #1003 (2023-04-06 15:59:16)

docs: document API stability
Write down the guidelines we've been using to decide whether a breaking change is acceptable or not.


Patch:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index f71b8a1..bf57da9 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -12,6 +12,17 @@ a better understanding for the high-level goals.
 2. (*optional*) Create a draft PR if you want to discuss the implementation or have hit a problem. It's fine if this doesn't compile or contains debug statements.
 3. Create a PR that is ready to merge. This must pass CI and have tests.
 
+### API stability
+
+The library doesn't guarantee the stability of its API at the moment.
+
+1. If possible avoid breakage by introducing new API and deprecating the old one
+   at the same time. If an API was deprecated in v0.x it can be removed in v0.x+1.
+2. Breaking API in a way that causes compilation failures is acceptable but must
+   have good reasons.
+3. Changing the semantics of the API without causing compilation failures is
+   heavily discouraged.
+
 ## Running the tests
 
 Many of the tests require privileges to set resource limits and load eBPF code.
diff --git a/btf/btf.go b/btf/btf.go
index 8682357..1fa7fcc 100644
--- a/btf/btf.go
+++ b/btf/btf.go
@@ -240,6 +240,10 @@ func loadRawSpec(btf io.ReaderAt, bo binary.ByteOrder, base *Spec) (*Spec, error
 			return nil, fmt.Errorf("can't use split BTF as base")
 		}
 
+		if base.strings == nil {
+			return nil, fmt.Errorf("parse split BTF: base must be loaded from an ELF")
+		}
+
 		baseStrings = base.strings
 
 		firstTypeID, err = base.nextTypeID()
@@ -770,10 +774,6 @@ func (s *Spec) TypeByName(name string, typ interface{}) error {
 // Types from base are used to resolve references in the split BTF.
 // The returned Spec only contains types from the split BTF, not from the base.
 func LoadSplitSpecFromReader(r io.ReaderAt, base *Spec) (*Spec, error) {
-	if base.strings == nil {
-		return nil, fmt.Errorf("parse split BTF: base must be loaded from an ELF")
-	}
-
 	return loadRawSpec(r, internal.NativeEndian, base)
 }
 
diff --git a/btf/handle.go b/btf/handle.go
index 87ec2d7..ebcc01c 100644
--- a/btf/handle.go
+++ b/btf/handle.go
@@ -110,7 +110,10 @@ func NewHandleFromID(id ID) (*Handle, error) {
 }
 
 // Spec parses the kernel BTF into Go types.
-func (h *Handle) Spec() (*Spec, error) {
+//
+// base must contain type information for vmlinux if the handle is for
+// a kernel module. It may be nil otherwise.
+func (h *Handle) Spec(base *Spec) (*Spec, error) {
 	var btfInfo sys.BtfInfo
 	btfBuffer := make([]byte, h.size)
 	btfInfo.Btf, btfInfo.BtfSize = sys.NewSlicePointerLen(btfBuffer)
@@ -119,18 +122,8 @@ func (h *Handle) Spec() (*Spec, error) {
 		return nil, err
 	}
 
-	if !h.needsKernelBase {
-		return loadRawSpec(bytes.NewReader(btfBuffer), internal.NativeEndian, nil)
-	}
-
-	base, fallback, err := kernelSpec()
-	if err != nil {
-		return nil, fmt.Errorf("load BTF base: %w", err)
-	}
-	base = base.Copy()
-
-	if fallback {
-		return nil, fmt.Errorf("can't load split BTF without access to /sys")
+	if h.needsKernelBase && base == nil {
+		return nil, fmt.Errorf("missing base types")
 	}
 
 	return loadRawSpec(bytes.NewReader(btfBuffer), internal.NativeEndian, base)
diff --git a/link/tracing.go b/link/tracing.go
index 76340fd..1e1a783 100644
--- a/link/tracing.go
+++ b/link/tracing.go
@@ -49,7 +49,7 @@ func AttachFreplace(targetProg *ebpf.Program, name string, prog *ebpf.Program) (
 		}
 		defer btfHandle.Close()
 
-		spec, err := btfHandle.Spec()
+		spec, err := btfHandle.Spec(nil)
 		if err != nil {
 			return nil, err
 		}
diff --git a/prog.go b/prog.go
index cb79522..781fb1c 100644
--- a/prog.go
+++ b/prog.go
@@ -895,7 +895,7 @@ func findTargetInKernel(name string, progType ProgramType, attachType AttachType
 
 	err = spec.TypeByName(typeName, &target)
 	if errors.Is(err, btf.ErrNotFound) {
-		module, id, err := findTargetInModule(typeName, target)
+		module, id, err := findTargetInModule(spec, typeName, target)
 		if errors.Is(err, btf.ErrNotFound) {
 			return nil, 0, &internal.UnsupportedFeatureError{Name: featureName}
 		}
@@ -923,7 +923,7 @@ func findTargetInKernel(name string, progType ProgramType, attachType AttachType
 // vmlinux must contain the kernel's types and is used to parse kmod BTF.
 //
 // Returns btf.ErrNotFound if the target can't be found in any module.
-func findTargetInModule(typeName string, target btf.Type) (*btf.Handle, btf.TypeID, error) {
+func findTargetInModule(base *btf.Spec, typeName string, target btf.Type) (*btf.Handle, btf.TypeID, error) {
 	it := new(btf.HandleIterator)
 	defer it.Handle.Close()
 
@@ -937,7 +937,7 @@ func findTargetInModule(typeName string, target btf.Type) (*btf.Handle, btf.Type
 			continue
 		}
 
-		spec, err := it.Handle.Spec()
+		spec, err := it.Handle.Spec(base)
 		if err != nil {
 			return nil, 0, fmt.Errorf("parse types for module %s: %w", info.Name, err)
 		}
@@ -987,7 +987,7 @@ func findTargetInProgram(prog *Program, name string, progType ProgramType, attac
 	}
 	defer btfHandle.Close()
 
-	spec, err := btfHandle.Spec()
+	spec, err := btfHandle.Spec(nil)
 	if err != nil {
 		return 0, err
 	}


Test patch:
diff --git a/btf/handle_test.go b/btf/handle_test.go
index a6c22e0..4634d5b 100644
--- a/btf/handle_test.go
+++ b/btf/handle_test.go
@@ -70,7 +70,12 @@ func TestParseModuleSplitSpec(t *testing.T) {
 	}
 	defer vmlinux.Close()
 
-	_, err = module.Spec()
+	base, err := vmlinux.Spec(nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = module.Spec(base)
 	if err != nil {
 		t.Fatal("Parse module BTF:", err)
 	}