Repository: cilium/ebpf
Pull request #57 (2020-02-05 12:02:15)

use sentinel errors instead of functions
With the new errors.Is and errors.As functions from Go 1.13 we can remove
the various Is* functions without a loss in flexibility. Instead, we export
sentinel errors and take care to always return them in a wrapped form.

Users that are not yet on Go 1.13 can use the golang.org/x/xerrors package.


Patch:
diff --git a/collection.go b/collection.go
index 7dd46c5..7acc0d6 100644
--- a/collection.go
+++ b/collection.go
@@ -136,7 +136,7 @@ func NewCollectionWithOptions(spec *CollectionSpec, opts CollectionOptions) (col
 		var handle *btf.Handle
 		if mapSpec.BTF != nil {
 			handle, err = loadBTF(btf.MapSpec(mapSpec.BTF))
-			if err != nil && !btf.IsNotSupported(err) {
+			if err != nil && !xerrors.Is(err, btf.ErrNotSupported) {
 				return nil, err
 			}
 		}
@@ -176,7 +176,7 @@ func NewCollectionWithOptions(spec *CollectionSpec, opts CollectionOptions) (col
 		var handle *btf.Handle
 		if progSpec.BTF != nil {
 			handle, err = loadBTF(btf.ProgramSpec(progSpec.BTF))
-			if err != nil && !btf.IsNotSupported(err) {
+			if err != nil && !xerrors.Is(err, btf.ErrNotSupported) {
 				return nil, err
 			}
 		}
diff --git a/internal/btf/btf.go b/internal/btf/btf.go
index a0a1262..7d62103 100644
--- a/internal/btf/btf.go
+++ b/internal/btf/btf.go
@@ -18,6 +18,11 @@ import (
 
 const btfMagic = 0xeB9F
 
+// Errors returned by BTF functions.
+var (
+	ErrNotSupported = internal.ErrNotSupported
+)
+
 // Spec represents decoded BTF.
 type Spec struct {
 	rawTypes  []rawType
@@ -307,8 +312,7 @@ type Handle struct {
 
 // NewHandle loads BTF into the kernel.
 //
-// Returns an error if BTF is not supported, which can
-// be checked by IsNotSupported.
+// Returns ErrNotSupported if BTF is not supported.
 func NewHandle(spec *Spec) (*Handle, error) {
 	if err := haveBTF(); err != nil {
 		return nil, err
@@ -451,13 +455,6 @@ func ProgramLineInfos(s *Program) (recordSize uint32, bytes []byte, err error) {
 	return s.lineInfos.recordSize, bytes, nil
 }
 
-// IsNotSupported returns true if the error indicates that the kernel
-// doesn't support BTF.
-func IsNotSupported(err error) bool {
-	var ufe *internal.UnsupportedFeatureError
-	return xerrors.As(err, &ufe) && ufe.Name == "BTF"
-}
-
 type bpfLoadBTFAttr struct {
 	btf         internal.Pointer
 	logBuf      internal.Pointer
diff --git a/internal/feature.go b/internal/feature.go
index bb9e97e..5ecf954 100644
--- a/internal/feature.go
+++ b/internal/feature.go
@@ -7,6 +7,9 @@ import (
 	"golang.org/x/xerrors"
 )
 
+// ErrNotSupported indicates that a feature is not supported by the current kernel.
+var ErrNotSupported = xerrors.New("not supported")
+
 // UnsupportedFeatureError is returned by FeatureTest() functions.
 type UnsupportedFeatureError struct {
 	// The minimum Linux mainline version required for this feature.
@@ -21,6 +24,11 @@ func (ufe *UnsupportedFeatureError) Error() string {
 	return fmt.Sprintf("%s not supported (requires >= %s)", ufe.Name, ufe.MinimumVersion)
 }
 
+// Is indicates that UnsupportedFeatureError is ErrNotSupported.
+func (ufe *UnsupportedFeatureError) Is(target error) bool {
+	return target == ErrNotSupported
+}
+
 // FeatureTest wraps a function so that it is run at most once.
 //
 // name should identify the tested feature, while version must be in the
diff --git a/map.go b/map.go
index 36fcd69..869ea98 100644
--- a/map.go
+++ b/map.go
@@ -5,11 +5,16 @@ import (
 
 	"github.com/cilium/ebpf/internal"
 	"github.com/cilium/ebpf/internal/btf"
-	"github.com/cilium/ebpf/internal/unix"
 
 	"golang.org/x/xerrors"
 )
 
+// Errors returned by Map and MapIterator methods.
+var (
+	ErrKeyNotExist      = xerrors.New("key does not exist")
+	ErrIterationAborted = xerrors.New("iteration aborted")
+)
+
 // MapSpec defines a Map.
 type MapSpec struct {
 	// Name is passed to the kernel as a debug aid. Must only contain
@@ -87,7 +92,7 @@ func NewMap(spec *MapSpec) (*Map, error) {
 	}
 
 	handle, err := btf.NewHandle(btf.MapSpec(spec.BTF))
-	if err != nil && !btf.IsNotSupported(err) {
+	if err != nil && !xerrors.Is(err, btf.ErrNotSupported) {
 		return nil, xerrors.Errorf("can't load BTF: %w", err)
 	}
 
@@ -270,6 +275,8 @@ func (m *Map) Lookup(key, valueOut interface{}) error {
 }
 
 // LookupAndDelete retrieves and deletes a value from a Map.
+//
+// Returns ErrKeyNotExist if the key doesn't exist.
 func (m *Map) LookupAndDelete(key, valueOut interface{}) error {
 	valuePtr, valueBytes := makeBuffer(valueOut, m.fullValueSize)
 
@@ -293,7 +300,7 @@ func (m *Map) LookupBytes(key interface{}) ([]byte, error) {
 	valuePtr := internal.NewSlicePointer(valueBytes)
 
 	err := m.lookup(key, valuePtr)
-	if IsNotExist(err) {
+	if xerrors.Is(err, ErrKeyNotExist) {
 		return nil, nil
 	}
 
@@ -350,12 +357,16 @@ func (m *Map) Update(key, value interface{}, flags MapUpdateFlags) error {
 		return xerrors.Errorf("can't marshal value: %w", err)
 	}
 
-	return bpfMapUpdateElem(m.fd, keyPtr, valuePtr, uint64(flags))
+	if err = bpfMapUpdateElem(m.fd, keyPtr, valuePtr, uint64(flags)); err != nil {
+		return xerrors.Errorf("update failed: %w", err)
+	}
+
+	return nil
 }
 
 // Delete removes a value.
 //
-// Returns an error if the key does not exist, see IsNotExist.
+// Returns ErrKeyNotExist if the key does not exist.
 func (m *Map) Delete(key interface{}) error {
 	keyPtr, err := marshalPtr(key, int(m.abi.KeySize))
 	if err != nil {
@@ -363,7 +374,7 @@ func (m *Map) Delete(key interface{}) error {
 	}
 
 	if err = bpfMapDeleteElem(m.fd, keyPtr); err != nil {
-		return xerrors.Errorf("can't delete key: %w", err)
+		return xerrors.Errorf("delete failed: %w", err)
 	}
 	return nil
 }
@@ -371,6 +382,8 @@ func (m *Map) Delete(key interface{}) error {
 // NextKey finds the key following an initial key.
 //
 // See NextKeyBytes for details.
+//
+// Returns ErrKeyNotExist if there is no next key.
 func (m *Map) NextKey(key, nextKeyOut interface{}) error {
 	nextKeyPtr, nextKeyBytes := makeBuffer(nextKeyOut, int(m.abi.KeySize))
 
@@ -393,12 +406,14 @@ func (m *Map) NextKey(key, nextKeyOut interface{}) error {
 // Passing nil will return the first key.
 //
 // Use Iterate if you want to traverse all entries in the map.
+//
+// Returns nil if there are no more keys.
 func (m *Map) NextKeyBytes(key interface{}) ([]byte, error) {
 	nextKey := make([]byte, m.abi.KeySize)
 	nextKeyPtr := internal.NewSlicePointer(nextKey)
 
 	err := m.nextKey(key, nextKeyPtr)
-	if IsNotExist(err) {
+	if xerrors.Is(err, ErrKeyNotExist) {
 		return nil, nil
 	}
 
@@ -419,7 +434,7 @@ func (m *Map) nextKey(key interface{}, nextKeyOut internal.Pointer) error {
 	}
 
 	if err = bpfMapGetNextKey(m.fd, keyPtr, nextKeyOut); err != nil {
-		return xerrors.Errorf("can't get next key: %w", err)
+		return xerrors.Errorf("next key failed: %w", err)
 	}
 	return nil
 }
@@ -565,8 +580,6 @@ func newMapIterator(target *Map) *MapIterator {
 	}
 }
 
-var errIterationAborted = xerrors.New("iteration aborted")
-
 // Next decodes the next key and value.
 //
 // Iterating a hash map from which keys are being deleted is not
@@ -602,7 +615,7 @@ func (mi *MapIterator) Next(keyOut, valueOut interface{}) bool {
 		mi.prevKey = mi.prevBytes
 
 		mi.err = mi.target.Lookup(nextBytes, valueOut)
-		if IsNotExist(mi.err) {
+		if xerrors.Is(mi.err, ErrKeyNotExist) {
 			// Even though the key should be valid, we couldn't look up
 			// its value. If we're iterating a hash map this is probably
 			// because a concurrent delete removed the value before we
@@ -621,26 +634,15 @@ func (mi *MapIterator) Next(keyOut, valueOut interface{}) bool {
 		return mi.err == nil
 	}
 
-	mi.err = errIterationAborted
+	mi.err = xerrors.Errorf("%w", ErrIterationAborted)
 	return false
 }
 
 // Err returns any encountered error.
 //
 // The method must be called after Next returns nil.
+//
+// Returns ErrIterationAborted if it wasn't possible to do a full iteration.
 func (mi *MapIterator) Err() error {
 	return mi.err
 }
-
-// IsNotExist returns true if the error indicates that a
-// key doesn't exist.
-func IsNotExist(err error) bool {
-	return xerrors.Is(err, unix.ENOENT)
-}
-
-// IsIterationAborted returns true if the iteration was aborted.
-//
-// This occurs when keys are deleted from a hash map during iteration.
-func IsIterationAborted(err error) bool {
-	return xerrors.Is(err, errIterationAborted)
-}
diff --git a/perf/reader.go b/perf/reader.go
index 70bcbe4..c8962ab 100644
--- a/perf/reader.go
+++ b/perf/reader.go
@@ -379,7 +379,7 @@ func (pr *Reader) Pause() error {
 	}
 
 	for i := 0; i < len(pr.pauseFds); i++ {
-		if err := pr.array.Delete(uint32(i)); err != nil && !ebpf.IsNotExist(err) {
+		if err := pr.array.Delete(uint32(i)); err != nil && !xerrors.Is(err, ebpf.ErrKeyNotExist) {
 			return xerrors.Errorf("could't delete event fd for CPU %d: %w", i, err)
 		}
 	}
diff --git a/prog.go b/prog.go
index 62ff980..4e0c134 100644
--- a/prog.go
+++ b/prog.go
@@ -16,6 +16,9 @@ import (
 	"golang.org/x/xerrors"
 )
 
+// ErrNotSupported is returned whenever the kernel doesn't support a feature.
+var ErrNotSupported = internal.ErrNotSupported
+
 const (
 	// Number of bytes to pad the output buffer for BPF_PROG_TEST_RUN.
 	// This is currently the maximum of spare space allocated for SKB
@@ -97,7 +100,7 @@ func NewProgramWithOptions(spec *ProgramSpec, opts ProgramOptions) (*Program, er
 	}
 
 	handle, err := btf.NewHandle(btf.ProgramSpec(spec.BTF))
-	if err != nil && !btf.IsNotSupported(err) {
+	if err != nil && !xerrors.Is(err, btf.ErrNotSupported) {
 		return nil, xerrors.Errorf("can't load BTF: %w", err)
 	}
 
@@ -514,10 +517,3 @@ func SanitizeName(name string, replacement rune) string {
 		return char
 	}, name)
 }
-
-// IsNotSupported returns true if an error occurred because
-// the kernel does not have support for a specific feature.
-func IsNotSupported(err error) bool {
-	var ufe *internal.UnsupportedFeatureError
-	return xerrors.As(err, &ufe)
-}
diff --git a/syscalls.go b/syscalls.go
index 2d97819..7428322 100644
--- a/syscalls.go
+++ b/syscalls.go
@@ -217,7 +217,7 @@ func bpfMapLookupElem(m *internal.FD, key, valueOut internal.Pointer) error {
 		value: valueOut,
 	}
 	_, err = internal.BPF(_MapLookupElem, unsafe.Pointer(&attr), unsafe.Sizeof(attr))
-	return err
+	return wrapMapError(err)
 }
 
 func bpfMapLookupAndDelete(m *internal.FD, key, valueOut internal.Pointer) error {
@@ -232,7 +232,7 @@ func bpfMapLookupAndDelete(m *internal.FD, key, valueOut internal.Pointer) error
 		value: valueOut,
 	}
 	_, err = internal.BPF(_MapLookupAndDeleteElem, unsafe.Pointer(&attr), unsafe.Sizeof(attr))
-	return err
+	return wrapMapError(err)
 }
 
 func bpfMapUpdateElem(m *internal.FD, key, valueOut internal.Pointer, flags uint64) error {
@@ -248,7 +248,7 @@ func bpfMapUpdateElem(m *internal.FD, key, valueOut internal.Pointer, flags uint
 		flags: flags,
 	}
 	_, err = internal.BPF(_MapUpdateElem, unsafe.Pointer(&attr), unsafe.Sizeof(attr))
-	return err
+	return wrapMapError(err)
 }
 
 func bpfMapDeleteElem(m *internal.FD, key internal.Pointer) error {
@@ -262,7 +262,7 @@ func bpfMapDeleteElem(m *internal.FD, key internal.Pointer) error {
 		key:   key,
 	}
 	_, err = internal.BPF(_MapDeleteElem, unsafe.Pointer(&attr), unsafe.Sizeof(attr))
-	return err
+	return wrapMapError(err)
 }
 
 func bpfMapGetNextKey(m *internal.FD, key, nextKeyOut internal.Pointer) error {
@@ -277,7 +277,19 @@ func bpfMapGetNextKey(m *internal.FD, key, nextKeyOut internal.Pointer) error {
 		value: nextKeyOut,
 	}
 	_, err = internal.BPF(_MapGetNextKey, unsafe.Pointer(&attr), unsafe.Sizeof(attr))
-	return err
+	return wrapMapError(err)
+}
+
+func wrapMapError(err error) error {
+	if err == nil {
+		return nil
+	}
+
+	if xerrors.Is(err, unix.ENOENT) {
+		return ErrKeyNotExist
+	}
+
+	return xerrors.New(err.Error())
 }
 
 const bpfFSType = 0xcafe4a11


Test patch:
diff --git a/internal/feature_test.go b/internal/feature_test.go
index f7236fd..ad36092 100644
--- a/internal/feature_test.go
+++ b/internal/feature_test.go
@@ -1,8 +1,11 @@
 package internal
 
-import "testing"
+import (
+	"strings"
+	"testing"
 
-import "strings"
+	"golang.org/x/xerrors"
+)
 
 func TestFeatureTest(t *testing.T) {
 	var called bool
@@ -42,6 +45,10 @@ func TestFeatureTest(t *testing.T) {
 	if !strings.Contains(fte.Error(), "2.1.1") {
 		t.Error("UnsupportedFeatureError.Error doesn't contain version")
 	}
+
+	if !xerrors.Is(err, ErrNotSupported) {
+		t.Error("UnsupportedFeatureError is not ErrNotSupported")
+	}
 }
 
 func TestVersion(t *testing.T) {
diff --git a/map_test.go b/map_test.go
index c511c17..05d158e 100644
--- a/map_test.go
+++ b/map_test.go
@@ -150,7 +150,6 @@ func createArray(t *testing.T) *Map {
 }
 
 func TestMapQueue(t *testing.T) {
-
 	testutils.SkipOnOldKernel(t, "4.20", "map type queue")
 
 	m, err := NewMap(&MapSpec{
@@ -193,6 +192,7 @@ func TestMapQueue(t *testing.T) {
 	if v != 4242 {
 		t.Error("Want value 4242, got", v)
 	}
+
 	v = 0
 	if err := m.LookupAndDelete(nil, &v); err != nil {
 		t.Fatal("Can't lookup and delete element:", err)
@@ -201,7 +201,7 @@ func TestMapQueue(t *testing.T) {
 		t.Error("Want value 4242, got", v)
 	}
 
-	if err := m.LookupAndDelete(nil, &v); !IsNotExist(err) {
+	if err := m.LookupAndDelete(nil, &v); !xerrors.Is(err, ErrKeyNotExist) {
 		t.Fatal("Lookup and delete on empty Queue:", err)
 	}
 }
@@ -435,8 +435,8 @@ func TestNotExist(t *testing.T) {
 
 	var tmp uint32
 	err := hash.Lookup("hello", &tmp)
-	if !IsNotExist(err) {
-		t.Error("IsNotExist returns false for missing key")
+	if !xerrors.Is(err, ErrKeyNotExist) {
+		t.Error("Lookup doesn't return ErrKeyNotExist")
 	}
 
 	buf, err := hash.LookupBytes("hello")
@@ -447,11 +447,11 @@ func TestNotExist(t *testing.T) {
 		t.Error("LookupBytes returns non-nil buffer for non-existent key")
 	}
 
-	if err := hash.Delete("hello"); !IsNotExist(err) {
-		t.Error("Deleting unknown key doesn't return a non-existing error")
+	if err := hash.Delete("hello"); !xerrors.Is(err, ErrKeyNotExist) {
+		t.Error("Deleting unknown key doesn't return ErrKeyNotExist")
 	}
 
-	if err := hash.NextKey(nil, &tmp); !IsNotExist(err) {
+	if err := hash.NextKey(nil, &tmp); !xerrors.Is(err, ErrKeyNotExist) {
 		t.Error("Looking up next key in empty map doesn't return a non-existing error")
 	}
 }
@@ -787,7 +787,7 @@ func BenchmarkMap(b *testing.B) {
 
 		for i := 0; i < b.N; i++ {
 			err := m.Delete(unsafe.Pointer(&key))
-			if err != nil && !IsNotExist(err) {
+			if err != nil && !xerrors.Is(err, ErrKeyNotExist) {
 				b.Fatal(err)
 			}
 		}