Skip to content

Commit ae10986

Browse files
committed
pin: make WalkDir harder to misuse and add Windows support
The current WalkDir API requires the user to close each object that is iterated over. This isn't very intuitive, the two uses of the function in the tetragon code base end up leaking objects. It's also not straight forward to implement WalkDir on Windows, since BPFFS isn't really a thing on that platform. Instead, turn WalkDir into an iterator which returns a new Pin object. By default, objects are closed once they have been iterated over. The user may call Pin.Take if they wish to retain the object. This is similar to the existing link.Iterator.Take. One downside is that directories are not returned by the iteration anymore. The existing code in tetragon doesn't use them and is therefore not affected. There is also no more fine grained control over whether to descent into a directory. Aborting an iteration is still supported and as efficient as before. Fixes: #1652 Signed-off-by: Lorenz Bauer <[email protected]>
1 parent a451857 commit ae10986

File tree

7 files changed

+186
-97
lines changed

7 files changed

+186
-97
lines changed

.github/workflows/ci.yml

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -346,23 +346,7 @@ jobs:
346346
- name: Test
347347
run: >
348348
gotestsum --raw-command --ignore-non-json-output-lines --junitfile junit.xml --
349-
go test -short -count 1 -json
350-
./
351-
./asm
352-
./btf
353-
./features
354-
./internal
355-
./internal/efw
356-
./internal/kallsyms
357-
./internal/kconfig
358-
./internal/linux
359-
./internal/sys
360-
./internal/sysenc
361-
./internal/testutils
362-
./internal/testutils/testmain
363-
./internal/tracefs
364-
./internal/unix
365-
./link
349+
go test -short -count 1 -json ./...
366350
367351
- name: Upload Test Results
368352
if: always()

pin/load_test.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,22 @@ import (
88

99
"github.com/cilium/ebpf"
1010
"github.com/cilium/ebpf/asm"
11+
"github.com/cilium/ebpf/internal/platform"
1112
"github.com/cilium/ebpf/internal/testutils"
1213
"github.com/cilium/ebpf/internal/testutils/testmain"
1314
)
1415

1516
func mustPinnedProgram(t *testing.T, path string) *ebpf.Program {
1617
t.Helper()
1718

19+
typ := ebpf.SocketFilter
20+
if platform.IsWindows {
21+
typ = ebpf.WindowsXDPTest
22+
}
23+
1824
spec := &ebpf.ProgramSpec{
1925
Name: "test",
20-
Type: ebpf.SocketFilter,
26+
Type: typ,
2127
Instructions: asm.Instructions{
2228
asm.LoadImm(asm.R0, 2, asm.DWord),
2329
asm.Return(),
@@ -41,9 +47,14 @@ func mustPinnedProgram(t *testing.T, path string) *ebpf.Program {
4147
func mustPinnedMap(t *testing.T, path string) *ebpf.Map {
4248
t.Helper()
4349

50+
typ := ebpf.Array
51+
if platform.IsWindows {
52+
typ = ebpf.WindowsArray
53+
}
54+
4455
spec := &ebpf.MapSpec{
4556
Name: "test",
46-
Type: ebpf.Array,
57+
Type: typ,
4758
KeySize: 4,
4859
ValueSize: 4,
4960
MaxEntries: 1,

pin/pin.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package pin
2+
3+
import "io"
4+
5+
// Pin represents an object and its pinned path.
6+
type Pin struct {
7+
Path string
8+
Object io.Closer
9+
}
10+
11+
func (p *Pin) close() {
12+
if p.Object != nil {
13+
p.Object.Close()
14+
}
15+
}
16+
17+
// Take ownership of Pin.Object.
18+
//
19+
// The caller is responsible for calling close on [Pin.Object].
20+
func (p *Pin) Take() {
21+
p.Object = nil
22+
}

pin/walk.go

Lines changed: 0 additions & 49 deletions
This file was deleted.

pin/walk_other.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
//go:build !windows
2+
3+
package pin
4+
5+
import (
6+
"fmt"
7+
"io/fs"
8+
"iter"
9+
"os"
10+
"path/filepath"
11+
12+
"github.com/cilium/ebpf"
13+
"github.com/cilium/ebpf/internal/linux"
14+
"github.com/cilium/ebpf/internal/unix"
15+
)
16+
17+
// WalkDir walks the file tree rooted at path and yields a [Pin] for each
18+
// BPF object below the path.
19+
//
20+
// Callers must invoke [Pin.Take] if they wish to hold on to the object.
21+
func WalkDir(root string, opts *ebpf.LoadPinOptions) iter.Seq2[*Pin, error] {
22+
return func(yield func(*Pin, error) bool) {
23+
fsType, err := linux.FSType(root)
24+
if err != nil {
25+
yield(nil, err)
26+
return
27+
}
28+
if fsType != unix.BPF_FS_MAGIC {
29+
yield(nil, fmt.Errorf("%s is not on a bpf filesystem", root))
30+
return
31+
}
32+
33+
fn := func(path string, d fs.DirEntry, err error) error {
34+
if err != nil {
35+
return err
36+
}
37+
38+
if d.IsDir() {
39+
return nil
40+
}
41+
42+
path = filepath.Join(root, path)
43+
obj, err := Load(path, opts)
44+
if err != nil {
45+
return err
46+
}
47+
48+
pin := &Pin{Path: path, Object: obj}
49+
defer pin.close()
50+
51+
if !yield(pin, nil) {
52+
return fs.SkipAll
53+
}
54+
55+
return nil
56+
}
57+
58+
err = fs.WalkDir(os.DirFS(root), ".", fn)
59+
if err != nil {
60+
yield(nil, fmt.Errorf("walk: %w", err))
61+
return
62+
}
63+
}
64+
}

pin/walk_test.go

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,67 @@
11
package pin
22

33
import (
4-
"io/fs"
4+
"iter"
55
"os"
66
"path/filepath"
7+
"reflect"
78
"testing"
89

910
"github.com/go-quicktest/qt"
1011

1112
"github.com/cilium/ebpf"
13+
"github.com/cilium/ebpf/internal/platform"
1214
"github.com/cilium/ebpf/internal/testutils"
1315
)
1416

1517
func TestWalkDir(t *testing.T) {
1618
testutils.SkipOnOldKernel(t, "4.10", "reading program fdinfo")
1719

1820
tmp := testutils.TempBPFFS(t)
21+
1922
dir := filepath.Join(tmp, "dir")
20-
qt.Assert(t, qt.IsNil(os.Mkdir(dir, 0755)))
23+
if !platform.IsWindows {
24+
// Windows doesn't have a BPF file system, so mkdir below fails.
25+
qt.Assert(t, qt.IsNil(os.Mkdir(dir, 0755)))
26+
}
2127

22-
mustPinnedProgram(t, filepath.Join(tmp, "pinned_prog"))
23-
mustPinnedMap(t, filepath.Join(dir, "pinned_map"))
28+
progPath := filepath.Join(tmp, "pinned_prog")
29+
mustPinnedProgram(t, progPath)
30+
mapPath := filepath.Join(dir, "pinned_map")
31+
mustPinnedMap(t, mapPath)
2432

25-
entries := make(map[string]string)
33+
next, stop := iter.Pull2(WalkDir(tmp, nil))
34+
defer stop()
2635

27-
bpffn := func(path string, d fs.DirEntry, obj Pinner, err error) error {
28-
qt.Assert(t, qt.IsNil(err))
36+
pin, err, ok := next()
37+
qt.Assert(t, qt.IsTrue(ok))
38+
qt.Assert(t, qt.IsNil(err))
39+
qt.Assert(t, qt.Equals(reflect.TypeOf(pin.Object), reflect.TypeFor[*ebpf.Map]()))
40+
qt.Assert(t, qt.Equals(pin.Path, mapPath))
2941

30-
if obj != nil {
31-
defer obj.Close()
32-
}
42+
pin, err, ok = next()
43+
qt.Assert(t, qt.IsTrue(ok))
44+
qt.Assert(t, qt.IsNil(err))
45+
qt.Assert(t, qt.Equals(reflect.TypeOf(pin.Object), reflect.TypeFor[*ebpf.Program]()))
46+
qt.Assert(t, qt.Equals(pin.Path, progPath))
3347

34-
if path == "." {
35-
return nil
36-
}
48+
_, _, ok = next()
49+
qt.Assert(t, qt.IsFalse(ok))
3750

38-
switch obj.(type) {
39-
case *ebpf.Program:
40-
entries[path] = "prog"
41-
case *ebpf.Map:
42-
entries[path] = "map"
43-
default:
44-
entries[path] = ""
51+
t.Run("Not BPFFS", func(t *testing.T) {
52+
if platform.IsWindows {
53+
t.Skip("Windows does not have BPFFS")
4554
}
4655

47-
return nil
48-
}
49-
qt.Assert(t, qt.IsNil(WalkDir(tmp, bpffn)))
56+
next, stop := iter.Pull2(WalkDir("/", nil))
57+
defer stop()
58+
59+
_, err, ok = next()
60+
qt.Assert(t, qt.IsTrue(ok))
61+
qt.Assert(t, qt.IsNotNil(err))
5062

51-
qt.Assert(t, qt.DeepEquals(entries, map[string]string{
52-
"pinned_prog": "prog",
53-
"dir": "",
54-
"dir/pinned_map": "map",
55-
}))
63+
_, _, ok = next()
64+
qt.Assert(t, qt.IsFalse(ok))
65+
})
5666

57-
qt.Assert(t, qt.IsNotNil(WalkDir("/", nil)))
5867
}

pin/walk_windows.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package pin
2+
3+
import (
4+
"errors"
5+
"iter"
6+
"strings"
7+
8+
"github.com/cilium/ebpf"
9+
"github.com/cilium/ebpf/internal/efw"
10+
)
11+
12+
// WalkDir walks the file tree rooted at path and yields a [Pin] for each
13+
// BPF object below the path.
14+
//
15+
// Callers must invoke [Pin.Take] if they wish to hold on to the object.
16+
func WalkDir(root string, opts *ebpf.LoadPinOptions) iter.Seq2[*Pin, error] {
17+
return func(yield func(*Pin, error) bool) {
18+
cursor := root
19+
for {
20+
next, _, err := efw.EbpfGetNextPinnedObjectPath(cursor, efw.EBPF_OBJECT_UNKNOWN)
21+
if errors.Is(err, efw.EBPF_NO_MORE_KEYS) {
22+
break
23+
} else if err != nil {
24+
yield(nil, err)
25+
return
26+
}
27+
28+
if !strings.HasPrefix(next, root) {
29+
break
30+
}
31+
32+
obj, err := Load(next, opts)
33+
if err != nil {
34+
yield(nil, err)
35+
return
36+
}
37+
38+
pin := &Pin{next, obj}
39+
ok := yield(pin, nil)
40+
pin.close()
41+
if !ok {
42+
return
43+
}
44+
45+
cursor = next
46+
}
47+
}
48+
}

0 commit comments

Comments
 (0)