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

Fix btf.FindType to avoid copy #424

Merged
merged 4 commits into from
Sep 17, 2021
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
6 changes: 3 additions & 3 deletions elf_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ func (ec *elfCode) loadBTFMaps(maps map[string]*MapSpec) error {
}

// Each section must appear as a DataSec in the ELF's BTF blob.
var ds btf.Datasec
var ds *btf.Datasec
if err := ec.btf.FindType(sec.Name, &ds); err != nil {
return fmt.Errorf("cannot find section '%s' in BTF: %w", sec.Name, err)
}
Expand Down Expand Up @@ -926,7 +926,7 @@ func (ec *elfCode) loadDataSections(maps map[string]*MapSpec) error {
return errors.New("data sections require BTF, make sure all consts are marked as static")
}

var datasec btf.Datasec
var datasec *btf.Datasec
if err := ec.btf.FindType(sec.Name, &datasec); err != nil {
return fmt.Errorf("data section %s: can't get BTF: %w", sec.Name, err)
}
Expand All @@ -947,7 +947,7 @@ func (ec *elfCode) loadDataSections(maps map[string]*MapSpec) error {
ValueSize: uint32(len(data)),
MaxEntries: 1,
Contents: []MapKV{{uint32(0), data}},
BTF: &btf.Map{Spec: ec.btf, Key: &btf.Void{}, Value: &datasec},
BTF: &btf.Map{Spec: ec.btf, Key: &btf.Void{}, Value: datasec},
}

switch sec.Name {
Expand Down
28 changes: 20 additions & 8 deletions internal/btf/btf.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,16 +467,28 @@ func (s *Spec) Program(name string, length uint64) (*Program, error) {

// FindType searches for a type with a specific name.
//
// hint determines the type of the returned Type.
// Called T a type that satisfies Type, typ must be a non-nil **T.
// On success, the address of the found type will be copied in typ.
//
// Returns an error wrapping ErrNotFound if no matching
// type exists in spec.
func (s *Spec) FindType(name string, typ Type) error {
var (
wanted = reflect.TypeOf(typ)
candidate Type
)
func (s *Spec) FindType(name string, typ interface{}) error {
typValue := reflect.ValueOf(typ)
if typValue.Kind() != reflect.Ptr {
return fmt.Errorf("%T is not a pointer", typ)
}

typPtr := typValue.Elem()
if !typPtr.CanSet() {
return fmt.Errorf("%T cannot be set", typ)
}

wanted := typPtr.Type()
if !wanted.AssignableTo(reflect.TypeOf((*Type)(nil)).Elem()) {
return fmt.Errorf("%T does not satisfy Type interface", typ)
}

var candidate Type
for _, typ := range s.namedTypes[essentialName(name)] {
if reflect.TypeOf(typ) != wanted {
continue
Expand All @@ -498,8 +510,8 @@ func (s *Spec) FindType(name string, typ Type) error {
return fmt.Errorf("type %s: %w", name, ErrNotFound)
}

value := reflect.Indirect(reflect.ValueOf(candidate))
reflect.Indirect(reflect.ValueOf(typ)).Set(value)
typPtr.Set(reflect.ValueOf(candidate))

return nil
}

Expand Down
63 changes: 57 additions & 6 deletions internal/btf/btf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,57 @@ import (
"github.com/cilium/ebpf/internal/testutils"
)

func TestFindType(t *testing.T) {
fh, err := os.Open("testdata/vmlinux-btf.gz")
if err != nil {
t.Fatal(err)
}
defer fh.Close()

rd, err := gzip.NewReader(fh)
if err != nil {
t.Fatal(err)
}
defer rd.Close()

spec, err := loadRawSpec(rd, binary.LittleEndian, nil, nil)
if err != nil {
t.Fatal("Can't load BTF:", err)
}

// spec.FindType MUST fail if typ is not a non-nil **T, where T satisfies btf.Type.
i := 0
p := &i
for _, typ := range []interface{}{
nil,
Struct{},
&Struct{},
[]Struct{},
&[]Struct{},
map[int]Struct{},
&map[int]Struct{},
p,
&p,
} {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add something like **int?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and now that I think about it, we should also check something like slices and maps as "strange" types.

if err := spec.FindType("iphdr", typ); err == nil {
t.Fatalf("FindType does not fail with type %T", typ)
}
}

// spec.FindType MUST return the same address for multiple calls with the same type name.
var iphdr1, iphdr2 *Struct
if err := spec.FindType("iphdr", &iphdr1); err != nil {
t.Fatal(err)
}
if err := spec.FindType("iphdr", &iphdr2); err != nil {
t.Fatal(err)
}

if iphdr1 != iphdr2 {
t.Fatal("multiple FindType calls for `iphdr` name do not return the same addresses")
}
}

func TestParseVmlinux(t *testing.T) {
fh, err := os.Open("testdata/vmlinux-btf.gz")
if err != nil {
Expand All @@ -29,7 +80,7 @@ func TestParseVmlinux(t *testing.T) {
t.Fatal("Can't load BTF:", err)
}

var iphdr Struct
var iphdr *Struct
err = spec.FindType("iphdr", &iphdr)
if err != nil {
t.Fatalf("unable to find `iphdr` struct: %s", err)
Expand Down Expand Up @@ -100,17 +151,17 @@ func TestLoadSpecFromElf(t *testing.T) {
t.Error("Missing BTF for the socket section")
}

var bpfMapDef Struct
var bpfMapDef *Struct
if err := spec.FindType("bpf_map_def", &bpfMapDef); err != nil {
t.Error("Can't find bpf_map_def:", err)
}

var tmp Void
var tmp *Void
if err := spec.FindType("totally_bogus_type", &tmp); !errors.Is(err, ErrNotFound) {
t.Error("FindType doesn't return ErrNotFound:", err)
}

var fn Func
var fn *Func
if err := spec.FindType("global_fn", &fn); err != nil {
t.Error("Can't find global_fn():", err)
} else {
Expand All @@ -119,7 +170,7 @@ func TestLoadSpecFromElf(t *testing.T) {
}
}

var v Var
var v *Var
if err := spec.FindType("key3", &v); err != nil {
t.Error("Cant find key3:", err)
} else {
Expand Down Expand Up @@ -198,7 +249,7 @@ func ExampleSpec_FindType() {
spec := new(Spec)

// Declare a variable of the desired type
var foo Struct
var foo *Struct

if err := spec.FindType("foo", &foo); err != nil {
// There is no struct with name foo, or there
Expand Down
10 changes: 7 additions & 3 deletions link/freplace.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ func AttachFreplace(targetProg *ebpf.Program, name string, prog *ebpf.Program) (
return nil, fmt.Errorf("eBPF program type %s is not an Extension: %w", prog.Type(), errInvalidInput)
}

var target int
var function btf.Func
var (
target int
typeID btf.TypeID
)
if targetProg != nil {
info, err := targetProg.Info()
if err != nil {
Expand All @@ -48,18 +50,20 @@ func AttachFreplace(targetProg *ebpf.Program, name string, prog *ebpf.Program) (
}
defer btfHandle.Close()

var function *btf.Func
if err := btfHandle.Spec().FindType(name, &function); err != nil {
return nil, err
}

target = targetProg.FD()
typeID = function.ID()
}

link, err := AttachRawLink(RawLinkOptions{
Target: target,
Program: prog,
Attach: ebpf.AttachNone,
BTF: function.ID(),
BTF: typeID,
})
if err != nil {
return nil, err
Expand Down
10 changes: 2 additions & 8 deletions prog.go
Original file line number Diff line number Diff line change
Expand Up @@ -720,24 +720,17 @@ func resolveBTFType(spec *btf.Spec, name string, progType ProgramType, attachTyp
a AttachType
}

var target btf.Type
var typeName, featureName string
switch (match{progType, attachType}) {
case match{LSM, AttachLSMMac}:
target = new(btf.Func)
typeName = "bpf_lsm_" + name
featureName = name + " LSM hook"

case match{Tracing, AttachTraceIter}:
target = new(btf.Func)
typeName = "bpf_iter_" + name
featureName = name + " iterator"

case match{Extension, AttachNone}:
target = new(btf.Func)
typeName = name
featureName = fmt.Sprintf("freplace %s", name)

default:
return nil, nil
}
Expand All @@ -750,7 +743,8 @@ func resolveBTFType(spec *btf.Spec, name string, progType ProgramType, attachTyp
}
}

err := spec.FindType(typeName, target)
var target *btf.Func
err := spec.FindType(typeName, &target)
if errors.Is(err, btf.ErrNotFound) {
return nil, &internal.UnsupportedFeatureError{
Name: featureName,
Expand Down