Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improv: make errors more conclusive #1071

Merged
merged 1 commit into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions elf_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ func TestLoadCollectionSpec(t *testing.T) {
LogLevel: LogLevelBranch,
},
})

testutils.SkipIfNotSupported(t, err)
if err != nil {
t.Fatal(err)
Expand Down
7 changes: 3 additions & 4 deletions link/iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ type IterOptions struct {

// AttachIter attaches a BPF seq_file iterator.
func AttachIter(opts IterOptions) (*Iter, error) {
if err := haveBPFLink(); err != nil {
return nil, err
}

progFd := opts.Program.FD()
if progFd < 0 {
return nil, fmt.Errorf("invalid program: %s", sys.ErrClosedFd)
Expand All @@ -52,6 +48,9 @@ func AttachIter(opts IterOptions) (*Iter, error) {

fd, err := sys.LinkCreateIter(&attr)
if err != nil {
if haveFeatErr := haveBPFLink(); haveFeatErr != nil {
return nil, haveFeatErr
}
return nil, fmt.Errorf("can't link iterator: %w", err)
}

Expand Down
8 changes: 4 additions & 4 deletions link/kprobe_multi.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,6 @@ func kprobeMulti(prog *ebpf.Program, opts KprobeMultiOptions, flags uint32) (Lin
return nil, fmt.Errorf("Cookies must be exactly Symbols or Addresses in length: %w", errInvalidInput)
}

if err := haveBPFLinkKprobeMulti(); err != nil {
return nil, err
}

attr := &sys.LinkCreateKprobeMultiAttr{
ProgFd: uint32(prog.FD()),
AttachType: sys.BPF_TRACE_KPROBE_MULTI,
Expand Down Expand Up @@ -113,7 +109,11 @@ func kprobeMulti(prog *ebpf.Program, opts KprobeMultiOptions, flags uint32) (Lin
if errors.Is(err, unix.EINVAL) {
return nil, fmt.Errorf("%w (missing kernel symbol or prog's AttachType not AttachTraceKprobeMulti?)", err)
}

if err != nil {
if haveFeatErr := haveBPFLinkKprobeMulti(); haveFeatErr != nil {
return nil, haveFeatErr
}
return nil, err
}

Expand Down
15 changes: 7 additions & 8 deletions link/program.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ type RawAttachProgramOptions struct {
// You should use one of the higher level abstractions available in this
// package if possible.
func RawAttachProgram(opts RawAttachProgramOptions) error {
if err := haveProgAttach(); err != nil {
return err
}

var replaceFd uint32
if opts.Replace != nil {
replaceFd = uint32(opts.Replace.FD())
Expand All @@ -43,8 +39,12 @@ func RawAttachProgram(opts RawAttachProgramOptions) error {
}

if err := sys.ProgAttach(&attr); err != nil {
if haveFeatErr := haveProgAttach(); haveFeatErr != nil {
return haveFeatErr
}
return fmt.Errorf("can't attach program: %w", err)
}

return nil
}

Expand All @@ -59,16 +59,15 @@ type RawDetachProgramOptions struct {
// You should use one of the higher level abstractions available in this
// package if possible.
func RawDetachProgram(opts RawDetachProgramOptions) error {
if err := haveProgAttach(); err != nil {
return err
}

attr := sys.ProgDetachAttr{
TargetFd: uint32(opts.Target),
AttachBpfFd: uint32(opts.Program.FD()),
AttachType: uint32(opts.Attach),
}
if err := sys.ProgDetach(&attr); err != nil {
if haveFeatErr := haveProgAttach(); haveFeatErr != nil {
return haveFeatErr
}
return fmt.Errorf("can't detach program: %w", err)
}

Expand Down
11 changes: 7 additions & 4 deletions link/syscalls.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,11 @@ var haveProgAttachReplace = internal.NewFeatureTest("BPF_PROG_ATTACH atomic repl
asm.Return(),
},
})

if err != nil {
return internal.ErrNotSupported
}

defer prog.Close()

// We know that we have BPF_PROG_ATTACH since we can load CGroupSKB programs.
Expand Down Expand Up @@ -113,11 +115,12 @@ var haveProgQuery = internal.NewFeatureTest("BPF_PROG_QUERY", "4.15", func() err
}

err := sys.ProgQuery(&attr)
if errors.Is(err, unix.EINVAL) {
return internal.ErrNotSupported
lmb marked this conversation as resolved.
Show resolved Hide resolved
}

if errors.Is(err, unix.EBADF) {
return nil
}
return err
if err != nil {
return ErrNotSupported
}
return errors.New("syscall succeeded unexpectedly")
})
5 changes: 4 additions & 1 deletion link/uprobe.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@ var (
uprobeRefCtrOffsetShift = 32
haveRefCtrOffsetPMU = internal.NewFeatureTest("RefCtrOffsetPMU", "4.20", func() error {
_, err := os.Stat(uprobeRefCtrOffsetPMUPath)
if err != nil {
if errors.Is(err, os.ErrNotExist) {
return internal.ErrNotSupported
}
if err != nil {
return err
}
return nil
})

Expand Down
66 changes: 35 additions & 31 deletions map.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,27 +385,6 @@ func (spec *MapSpec) createMap(inner *sys.FD, opts MapOptions) (_ *Map, err erro
}
}

if spec.Flags&(unix.BPF_F_RDONLY_PROG|unix.BPF_F_WRONLY_PROG) > 0 || spec.Freeze {
if err := haveMapMutabilityModifiers(); err != nil {
return nil, fmt.Errorf("map create: %w", err)
}
}
if spec.Flags&unix.BPF_F_MMAPABLE > 0 {
if err := haveMmapableMaps(); err != nil {
return nil, fmt.Errorf("map create: %w", err)
}
}
if spec.Flags&unix.BPF_F_INNER_MAP > 0 {
if err := haveInnerMaps(); err != nil {
return nil, fmt.Errorf("map create: %w", err)
}
}
if spec.Flags&unix.BPF_F_NO_PREALLOC > 0 {
if err := haveNoPreallocMaps(); err != nil {
return nil, fmt.Errorf("map create: %w", err)
}
}

attr := sys.MapCreateAttr{
MapType: sys.MapType(spec.Type),
KeySize: spec.KeySize,
Expand Down Expand Up @@ -457,9 +436,35 @@ func (spec *MapSpec) createMap(inner *sys.FD, opts MapOptions) (_ *Map, err erro
if errors.Is(err, unix.EINVAL) && spec.Type == UnspecifiedMap {
return nil, fmt.Errorf("map create: cannot use type %s", UnspecifiedMap)
}

if spec.Flags&(unix.BPF_F_RDONLY_PROG|unix.BPF_F_WRONLY_PROG) > 0 || spec.Freeze {
lmb marked this conversation as resolved.
Show resolved Hide resolved
if haveFeatErr := haveMapMutabilityModifiers(); haveFeatErr != nil {
return nil, fmt.Errorf("map create: %w", haveFeatErr)
}
}

if spec.Flags&unix.BPF_F_MMAPABLE > 0 {
if haveFeatErr := haveMmapableMaps(); haveFeatErr != nil {
return nil, fmt.Errorf("map create: %w", haveFeatErr)
}
}

if spec.Flags&unix.BPF_F_INNER_MAP > 0 {
if haveFeatErr := haveInnerMaps(); haveFeatErr != nil {
return nil, fmt.Errorf("map create: %w", haveFeatErr)
}
}

if spec.Flags&unix.BPF_F_NO_PREALLOC > 0 {
if haveFeatErr := haveNoPreallocMaps(); haveFeatErr != nil {
return nil, fmt.Errorf("map create: %w", haveFeatErr)
}
}

if attr.BtfFd == 0 {
return nil, fmt.Errorf("map create: %w (without BTF k/v)", err)
}

return nil, fmt.Errorf("map create: %w", err)
}
defer closeOnError(fd)
Expand Down Expand Up @@ -991,9 +996,6 @@ func (m *Map) batchLookup(cmd sys.Cmd, startKey, nextKeyOut, keysOut, valuesOut
// "keys" and "values" must be of type slice, a pointer
// to a slice or buffer will not work.
func (m *Map) BatchUpdate(keys, values interface{}, opts *BatchOptions) (int, error) {
if err := haveBatchAPI(); err != nil {
return 0, err
}
if m.typ.hasPerCPUValue() {
return 0, ErrNotSupported
}
Expand Down Expand Up @@ -1035,6 +1037,9 @@ func (m *Map) BatchUpdate(keys, values interface{}, opts *BatchOptions) (int, er

err = sys.MapUpdateBatch(&attr)
if err != nil {
if haveFeatErr := haveBatchAPI(); haveFeatErr != nil {
return 0, haveFeatErr
}
return int(attr.Count), fmt.Errorf("batch update: %w", wrapMapError(err))
}

Expand All @@ -1044,9 +1049,6 @@ func (m *Map) BatchUpdate(keys, values interface{}, opts *BatchOptions) (int, er
// BatchDelete batch deletes entries in the map by keys.
// "keys" must be of type slice, a pointer to a slice or buffer will not work.
func (m *Map) BatchDelete(keys interface{}, opts *BatchOptions) (int, error) {
if err := haveBatchAPI(); err != nil {
return 0, err
}
if m.typ.hasPerCPUValue() {
return 0, ErrNotSupported
}
Expand All @@ -1072,6 +1074,9 @@ func (m *Map) BatchDelete(keys interface{}, opts *BatchOptions) (int, error) {
}

if err = sys.MapDeleteBatch(&attr); err != nil {
if haveFeatErr := haveBatchAPI(); haveFeatErr != nil {
return 0, haveFeatErr
}
return int(attr.Count), fmt.Errorf("batch delete: %w", wrapMapError(err))
}

Expand Down Expand Up @@ -1176,15 +1181,14 @@ func (m *Map) IsPinned() bool {
//
// It makes no changes to kernel-side restrictions.
func (m *Map) Freeze() error {
if err := haveMapMutabilityModifiers(); err != nil {
return fmt.Errorf("can't freeze map: %w", err)
}

attr := sys.MapFreezeAttr{
MapFd: m.fd.Uint(),
}

if err := sys.MapFreeze(&attr); err != nil {
if haveFeatErr := haveMapMutabilityModifiers(); haveFeatErr != nil {
return fmt.Errorf("can't freeze map: %w", haveFeatErr)
}
return fmt.Errorf("can't freeze map: %w", err)
}
return nil
Expand Down
7 changes: 3 additions & 4 deletions syscalls.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ var haveInnerMaps = internal.NewFeatureTest("inner maps", "5.10", func() error {
MaxEntries: 1,
MapFlags: unix.BPF_F_INNER_MAP,
})

if err != nil {
return internal.ErrNotSupported
}
Expand All @@ -135,6 +136,7 @@ var haveNoPreallocMaps = internal.NewFeatureTest("prealloc maps", "4.6", func()
MaxEntries: 1,
MapFlags: unix.BPF_F_NO_PREALLOC,
})

lmb marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return internal.ErrNotSupported
}
Expand Down Expand Up @@ -265,11 +267,8 @@ var haveBPFToBPFCalls = internal.NewFeatureTest("bpf2bpf calls", "4.16", func()
}

fd, err := progLoad(insns, SocketFilter, "MIT")
if errors.Is(err, unix.EINVAL) {
return internal.ErrNotSupported
}
if err != nil {
return err
return internal.ErrNotSupported
}
_ = fd.Close()
return nil
Expand Down