Repository: cilium/ebpf
Pull request #623 (2022-04-12 04:44:05)

btf: reduce unnecessary copies when parsing BTF
We currently read all BTF into memory before starting decoding. This is wasteful,
especially for the large-ish string table. Be a bit more clever to remove intermediate
copies.

    name            old time/op    new time/op    delta
    ParseVmlinux-4     107ms ± 7%      77ms ±13%  -27.98%  (p=0.029 n=4+4)

    name            old alloc/op   new alloc/op   delta
    ParseVmlinux-4     119MB ± 0%      93MB ± 0%  -22.20%  (p=0.029 n=4+4)

    name            old allocs/op  new allocs/op  delta
    ParseVmlinux-4      786k ± 0%      786k ± 0%   -0.01%  (p=0.029 n=4+4)


Patch:
diff --git a/internal/btf/btf.go b/internal/btf/btf.go
index 899ee89..1271c70 100644
--- a/internal/btf/btf.go
+++ b/internal/btf/btf.go
@@ -1,6 +1,7 @@
 package btf
 
 import (
+	"bufio"
 	"bytes"
 	"debug/elf"
 	"encoding/binary"
@@ -165,7 +166,11 @@ func loadSpecFromELF(file *internal.SafeELFFile) (*Spec, error) {
 		return nil, err
 	}
 
-	spec, err := loadRawSpec(btfSection.Open(), file.ByteOrder, sectionSizes, vars)
+	if btfSection.ReaderAt == nil {
+		return nil, fmt.Errorf("compressed BTF is not supported")
+	}
+
+	spec, err := loadRawSpec(btfSection.ReaderAt, file.ByteOrder, sectionSizes, vars)
 	if err != nil {
 		return nil, err
 	}
@@ -298,7 +303,7 @@ func (spec *Spec) splitExtInfos(info *extInfo) error {
 	return nil
 }
 
-func loadRawSpec(btf io.Reader, bo binary.ByteOrder, sectionSizes map[string]uint32, variableOffsets map[variable]uint32) (*Spec, error) {
+func loadRawSpec(btf io.ReaderAt, bo binary.ByteOrder, sectionSizes map[string]uint32, variableOffsets map[variable]uint32) (*Spec, error) {
 	rawTypes, rawStrings, err := parseBTF(btf, bo)
 	if err != nil {
 		return nil, err
@@ -427,11 +432,13 @@ func parseBTFHeader(r io.Reader, bo binary.ByteOrder) (*btfHeader, error) {
 }
 
 func guessRawBTFByteOrder(r io.ReaderAt) binary.ByteOrder {
+	buf := new(bufio.Reader)
 	for _, bo := range []binary.ByteOrder{
 		binary.LittleEndian,
 		binary.BigEndian,
 	} {
-		if _, err := parseBTFHeader(io.NewSectionReader(r, 0, math.MaxInt64), bo); err == nil {
+		buf.Reset(io.NewSectionReader(r, 0, math.MaxInt64))
+		if _, err := parseBTFHeader(buf, bo); err == nil {
 			return bo
 		}
 	}
@@ -441,25 +448,20 @@ func guessRawBTFByteOrder(r io.ReaderAt) binary.ByteOrder {
 
 // parseBTF reads a .BTF section into memory and parses it into a list of
 // raw types and a string table.
-func parseBTF(btf io.Reader, bo binary.ByteOrder) ([]rawType, stringTable, error) {
-	rawBTF, err := io.ReadAll(btf)
-	if err != nil {
-		return nil, nil, fmt.Errorf("can't read BTF: %v", err)
-	}
-	rd := bytes.NewReader(rawBTF)
-
-	header, err := parseBTFHeader(rd, bo)
+func parseBTF(btf io.ReaderAt, bo binary.ByteOrder) ([]rawType, stringTable, error) {
+	buf := internal.NewBufferedSectionReader(btf, 0, math.MaxInt64)
+	header, err := parseBTFHeader(buf, bo)
 	if err != nil {
 		return nil, nil, fmt.Errorf("parsing .BTF header: %v", err)
 	}
 
-	buf := io.NewSectionReader(rd, header.stringStart(), int64(header.StringLen))
+	buf.Reset(io.NewSectionReader(btf, header.stringStart(), int64(header.StringLen)))
 	rawStrings, err := readStringTable(buf)
 	if err != nil {
 		return nil, nil, fmt.Errorf("can't read type names: %w", err)
 	}
 
-	buf = io.NewSectionReader(rd, header.typeStart(), int64(header.TypeLen))
+	buf.Reset(io.NewSectionReader(btf, header.typeStart(), int64(header.TypeLen)))
 	rawTypes, err := readTypes(buf, bo)
 	if err != nil {
 		return nil, nil, fmt.Errorf("can't read types: %w", err)
diff --git a/internal/btf/core.go b/internal/btf/core.go
index c43e121..61c9166 100644
--- a/internal/btf/core.go
+++ b/internal/btf/core.go
@@ -18,50 +18,55 @@ import (
 
 // COREFixup is the result of computing a CO-RE relocation for a target.
 type COREFixup struct {
-	Kind   FixupKind
-	Local  uint32
-	Target uint32
-	Poison bool
+	kind   coreKind
+	local  uint32
+	target uint32
+	// True if there is no valid fixup. The instruction is replaced with an
+	// invalid dummy.
+	poison bool
+	// True if the validation of the local value should be skipped. Used by
+	// some kinds of bitfield relocations.
+	skipLocalValidation bool
 }
 
 func (f COREFixup) equal(other COREFixup) bool {
-	return f.Local == other.Local && f.Target == other.Target
+	return f.local == other.local && f.target == other.target
 }
 
 func (f COREFixup) String() string {
-	if f.Poison {
-		return fmt.Sprintf("%s=poison", f.Kind)
+	if f.poison {
+		return fmt.Sprintf("%s=poison", f.kind)
 	}
-	return fmt.Sprintf("%s=%d->%d", f.Kind, f.Local, f.Target)
+	return fmt.Sprintf("%s=%d->%d", f.kind, f.local, f.target)
 }
 
 func (f COREFixup) apply(ins *asm.Instruction) error {
-	if f.Poison {
+	if f.poison {
 		return errors.New("can't poison individual instruction")
 	}
 
 	switch class := ins.OpCode.Class(); class {
 	case asm.LdXClass, asm.StClass, asm.StXClass:
-		if want := int16(f.Local); f.Kind.validateLocal && want != ins.Offset {
-			return fmt.Errorf("invalid offset %d, expected %d", ins.Offset, f.Local)
+		if want := int16(f.local); !f.skipLocalValidation && want != ins.Offset {
+			return fmt.Errorf("invalid offset %d, expected %d", ins.Offset, f.local)
 		}
 
-		if f.Target > math.MaxInt16 {
-			return fmt.Errorf("offset %d exceeds MaxInt16", f.Target)
+		if f.target > math.MaxInt16 {
+			return fmt.Errorf("offset %d exceeds MaxInt16", f.target)
 		}
 
-		ins.Offset = int16(f.Target)
+		ins.Offset = int16(f.target)
 
 	case asm.LdClass:
 		if !ins.IsConstantLoad(asm.DWord) {
 			return fmt.Errorf("not a dword-sized immediate load")
 		}
 
-		if want := int64(f.Local); f.Kind.validateLocal && want != ins.Constant {
+		if want := int64(f.local); !f.skipLocalValidation && want != ins.Constant {
 			return fmt.Errorf("invalid immediate %d, expected %d (fixup: %v)", ins.Constant, want, f)
 		}
 
-		ins.Constant = int64(f.Target)
+		ins.Constant = int64(f.target)
 
 	case asm.ALUClass:
 		if ins.OpCode.ALUOp() == asm.Swap {
@@ -75,15 +80,15 @@ func (f COREFixup) apply(ins *asm.Instruction) error {
 			return fmt.Errorf("invalid source %s", src)
 		}
 
-		if want := int64(f.Local); f.Kind.validateLocal && want != ins.Constant {
-			return fmt.Errorf("invalid immediate %d, expected %d (fixup: %v, kind: %v, ins: %v)", ins.Constant, want, f, f.Kind, ins)
+		if want := int64(f.local); !f.skipLocalValidation && want != ins.Constant {
+			return fmt.Errorf("invalid immediate %d, expected %d (fixup: %v, kind: %v, ins: %v)", ins.Constant, want, f, f.kind, ins)
 		}
 
-		if f.Target > math.MaxInt32 {
-			return fmt.Errorf("immediate %d exceeds MaxInt32", f.Target)
+		if f.target > math.MaxInt32 {
+			return fmt.Errorf("immediate %d exceeds MaxInt32", f.target)
 		}
 
-		ins.Constant = int64(f.Target)
+		ins.Constant = int64(f.target)
 
 	default:
 		return fmt.Errorf("invalid class %s", class)
@@ -93,7 +98,7 @@ func (f COREFixup) apply(ins *asm.Instruction) error {
 }
 
 func (f COREFixup) isNonExistant() bool {
-	return f.Kind.coreKind.checksForExistence() && f.Target == 0
+	return f.kind.checksForExistence() && f.target == 0
 }
 
 type COREFixups map[uint64]COREFixup
@@ -116,7 +121,7 @@ func (fs COREFixups) Apply(insns asm.Instructions) (asm.Instructions, error) {
 		}
 
 		ins := *iter.Ins
-		if fixup.Poison {
+		if fixup.poison {
 			const badRelo = asm.BuiltinFunc(0xbad2310)
 
 			cpy = append(cpy, badRelo.Call())
@@ -130,7 +135,7 @@ func (fs COREFixups) Apply(insns asm.Instructions) (asm.Instructions, error) {
 		}
 
 		if err := fixup.apply(&ins); err != nil {
-			return nil, fmt.Errorf("instruction %d, offset %d: %s: %w", iter.Index, iter.Offset.Bytes(), fixup.Kind, err)
+			return nil, fmt.Errorf("instruction %d, offset %d: %s: %w", iter.Index, iter.Offset.Bytes(), fixup.kind, err)
 		}
 
 		cpy = append(cpy, ins)
@@ -192,16 +197,6 @@ func (k coreKind) String() string {
 	}
 }
 
-// FixupKind is the type of CO-RE relocation.
-type FixupKind struct {
-	coreKind      coreKind
-	validateLocal bool
-}
-
-func (fk FixupKind) String() string {
-	return fk.coreKind.String()
-}
-
 func coreRelocate(local, target *Spec, relos CORERelos) (COREFixups, error) {
 	if local.byteOrder != target.byteOrder {
 		return nil, fmt.Errorf("can't relocate %s against %s", local.byteOrder, target.byteOrder)
@@ -219,10 +214,9 @@ func coreRelocate(local, target *Spec, relos CORERelos) (COREFixups, error) {
 			}
 
 			result[uint64(relo.insnOff)] = COREFixup{
-				FixupKind{coreKind: relo.kind},
-				uint32(relo.typeID),
-				uint32(relo.typeID),
-				false,
+				kind:   relo.kind,
+				local:  uint32(relo.typeID),
+				target: uint32(relo.typeID),
 			}
 			continue
 		}
@@ -296,7 +290,7 @@ func coreCalculateFixups(byteOrder binary.ByteOrder, local Type, targets []Type,
 			if err != nil {
 				return nil, fmt.Errorf("target %s: %w", target, err)
 			}
-			if fixup.Poison || fixup.isNonExistant() {
+			if fixup.poison || fixup.isNonExistant() {
 				score++
 			}
 			fixups = append(fixups, fixup)
@@ -318,7 +312,7 @@ func coreCalculateFixups(byteOrder binary.ByteOrder, local Type, targets []Type,
 		// the fixups agree with each other.
 		for i, fixup := range bestFixups {
 			if !fixup.equal(fixups[i]) {
-				return nil, fmt.Errorf("%s: multiple types match: %w", fixup.Kind, errAmbiguousRelocation)
+				return nil, fmt.Errorf("%s: multiple types match: %w", fixup.kind, errAmbiguousRelocation)
 			}
 		}
 	}
@@ -328,7 +322,7 @@ func coreCalculateFixups(byteOrder binary.ByteOrder, local Type, targets []Type,
 		// targets at all. Poison everything!
 		bestFixups = make([]COREFixup, len(relos))
 		for i, relo := range relos {
-			bestFixups[i] = COREFixup{Kind: FixupKind{coreKind: relo.kind}, Poison: true}
+			bestFixups[i] = COREFixup{kind: relo.kind, poison: true}
 		}
 	}
 
@@ -338,14 +332,17 @@ func coreCalculateFixups(byteOrder binary.ByteOrder, local Type, targets []Type,
 // coreCalculateFixup calculates the fixup for a single local type, target type
 // and relocation.
 func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID, target Type, targetID TypeID, relo CORERelocation) (COREFixup, error) {
-	fixup := func(local, target uint32, validateLocal bool) (COREFixup, error) {
-		return COREFixup{FixupKind{relo.kind, validateLocal}, local, target, false}, nil
+	fixup := func(local, target uint32) (COREFixup, error) {
+		return COREFixup{kind: relo.kind, local: local, target: target}, nil
+	}
+	fixupWithoutValidation := func(local, target uint32) (COREFixup, error) {
+		return COREFixup{kind: relo.kind, local: local, target: target, skipLocalValidation: true}, nil
 	}
 	poison := func() (COREFixup, error) {
 		if relo.kind.checksForExistence() {
-			return fixup(1, 0, true)
+			return fixup(1, 0)
 		}
-		return COREFixup{Kind: FixupKind{coreKind: relo.kind}, Local: 0, Target: 0, Poison: true}, nil
+		return COREFixup{kind: relo.kind, poison: true}, nil
 	}
 	zero := COREFixup{}
 
@@ -365,10 +362,10 @@ func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID,
 
 		switch relo.kind {
 		case reloTypeExists:
-			return fixup(1, 1, true)
+			return fixup(1, 1)
 
 		case reloTypeIDTarget:
-			return fixup(uint32(localID), uint32(targetID), true)
+			return fixup(uint32(localID), uint32(targetID))
 
 		case reloTypeSize:
 			localSize, err := Sizeof(local)
@@ -381,7 +378,7 @@ func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID,
 				return zero, err
 			}
 
-			return fixup(uint32(localSize), uint32(targetSize), true)
+			return fixup(uint32(localSize), uint32(targetSize))
 		}
 
 	case reloEnumvalValue, reloEnumvalExists:
@@ -395,24 +392,23 @@ func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID,
 
 		switch relo.kind {
 		case reloEnumvalExists:
-			return fixup(1, 1, true)
+			return fixup(1, 1)
 
 		case reloEnumvalValue:
-			return fixup(uint32(localValue.Value), uint32(targetValue.Value), true)
+			return fixup(uint32(localValue.Value), uint32(targetValue.Value))
 		}
 
 	case reloFieldSigned:
 		switch local.(type) {
 		case *Enum:
-			return fixup(1, 1, true)
+			return fixup(1, 1)
 		case *Int:
 			return fixup(
 				uint32(local.(*Int).Encoding&Signed),
 				uint32(target.(*Int).Encoding&Signed),
-				true,
 			)
 		default:
-			return fixup(0, 0, false)
+			return fixupWithoutValidation(0, 0)
 		}
 
 	case reloFieldByteOffset, reloFieldByteSize, reloFieldExists, reloFieldLShiftU64, reloFieldRShiftU64:
@@ -431,13 +427,17 @@ func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID,
 			return zero, fmt.Errorf("target %s: %w", target, err)
 		}
 
-		validateLocal := localField.bitfieldSize == 0
+		maybeSkipValidation := func(f COREFixup, err error) (COREFixup, error) {
+			f.skipLocalValidation = localField.bitfieldSize > 0
+			return f, err
+		}
+
 		switch relo.kind {
 		case reloFieldExists:
-			return fixup(1, 1, validateLocal)
+			return fixup(1, 1)
 
 		case reloFieldByteOffset:
-			return fixup(localField.offset, targetField.offset, validateLocal)
+			return maybeSkipValidation(fixup(localField.offset, targetField.offset))
 
 		case reloFieldByteSize:
 			localSize, err := Sizeof(localField.Type)
@@ -449,7 +449,7 @@ func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID,
 			if err != nil {
 				return zero, err
 			}
-			return fixup(uint32(localSize), uint32(targetSize), validateLocal)
+			return maybeSkipValidation(fixup(uint32(localSize), uint32(targetSize)))
 
 		case reloFieldLShiftU64:
 			var target uint32
@@ -468,7 +468,7 @@ func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID,
 
 				target = 64 - uint32(loadWidth)*8 + targetField.bitfieldOffset
 			}
-			return fixup(0, target, false)
+			return fixupWithoutValidation(0, target)
 
 		case reloFieldRShiftU64:
 			targetSize, err := targetField.sizeBits()
@@ -476,7 +476,7 @@ func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID,
 				return zero, err
 			}
 
-			return fixup(0, 64-targetSize, false)
+			return fixupWithoutValidation(0, 64-targetSize)
 		}
 	}
 
diff --git a/internal/io.go b/internal/io.go
index 7177e59..30b6641 100644
--- a/internal/io.go
+++ b/internal/io.go
@@ -18,7 +18,7 @@ import (
 // end up being read completely anyway.
 //
 // Use instead of the r.Seek() + io.LimitReader() pattern.
-func NewBufferedSectionReader(ra io.ReaderAt, off, n int64) io.Reader {
+func NewBufferedSectionReader(ra io.ReaderAt, off, n int64) *bufio.Reader {
 	// Clamp the size of the buffer to one page to avoid slurping large parts
 	// of a file into memory. bufio.NewReader uses a hardcoded default buffer
 	// of 4096. Allow arches with larger pages to allocate more, but don't
diff --git a/link/kprobe.go b/link/kprobe.go
index 6a84d96..f689a07 100644
--- a/link/kprobe.go
+++ b/link/kprobe.go
@@ -50,6 +50,10 @@ type KprobeOptions struct {
 	//
 	// Needs kernel 5.15+.
 	Cookie uint64
+	// Offset of the kprobe relative to the traced symbol.
+	// Can be used to insert kprobes at arbitrary offsets in kernel functions,
+	// e.g. in places where functions have been inlined.
+	Offset uint64
 }
 
 const (
@@ -163,6 +167,7 @@ func kprobe(symbol string, prog *ebpf.Program, opts *KprobeOptions, ret bool) (*
 
 	if opts != nil {
 		args.cookie = opts.Cookie
+		args.offset = opts.Offset
 	}
 
 	// Use kprobe PMU if the kernel has it available.
@@ -235,8 +240,12 @@ func pmuProbe(typ probeType, args probeArgs) (*perfEvent, error) {
 		}
 
 		attr = unix.PerfEventAttr{
+			// The minimum size required for PMU kprobes is PERF_ATTR_SIZE_VER1,
+			// since it added the config2 (Ext2) field. Use Ext2 as probe_offset.
+			Size:   unix.PERF_ATTR_SIZE_VER1,
 			Type:   uint32(et),          // PMU event type read from sysfs
 			Ext1:   uint64(uintptr(sp)), // Kernel symbol to trace
+			Ext2:   args.offset,         // Kernel symbol offset
 			Config: config,              // Retprobe flag
 		}
 	case uprobeType:
@@ -392,7 +401,7 @@ func createTraceFSProbeEvent(typ probeType, args probeArgs) error {
 		// subsampling or rate limiting logic can be more accurately implemented in
 		// the eBPF program itself.
 		// See Documentation/kprobes.txt for more details.
-		pe = fmt.Sprintf("%s:%s/%s %s", probePrefix(args.ret), args.group, sanitizedSymbol(args.symbol), args.symbol)
+		pe = fmt.Sprintf("%s:%s/%s %s", probePrefix(args.ret), args.group, sanitizedSymbol(args.symbol), kprobeToken(args))
 	case uprobeType:
 		// The uprobe_events syntax is as follows:
 		// p[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a probe
@@ -496,3 +505,14 @@ func kretprobeBit() (uint64, error) {
 	})
 	return kprobeRetprobeBit.value, kprobeRetprobeBit.err
 }
+
+// kprobeToken creates the SYM[+offs] token for the tracefs api.
+func kprobeToken(args probeArgs) string {
+	po := args.symbol
+
+	if args.offset != 0 {
+		po += fmt.Sprintf("+%#x", args.offset)
+	}
+
+	return po
+}


Test patch:
diff --git a/internal/btf/btf_test.go b/internal/btf/btf_test.go
index 65e8440..93c43a5 100644
--- a/internal/btf/btf_test.go
+++ b/internal/btf/btf_test.go
@@ -161,6 +161,7 @@ func TestTypeByName(t *testing.T) {
 
 func BenchmarkParseVmlinux(b *testing.B) {
 	rd := readVMLinux(b)
+	b.ReportAllocs()
 	b.ResetTimer()
 
 	for n := 0; n < b.N; n++ {
diff --git a/internal/btf/core_test.go b/internal/btf/core_test.go
index 2823d8e..3b23719 100644
--- a/internal/btf/core_test.go
+++ b/internal/btf/core_test.go
@@ -557,10 +557,10 @@ func TestCORERelocation(t *testing.T) {
 				}
 
 				for offset, relo := range relos {
-					if want := relo.Local; relo.Kind.validateLocal && want != relo.Target {
+					if want := relo.local; !relo.skipLocalValidation && want != relo.target {
 						// Since we're relocating against ourselves both values
 						// should match.
-						t.Errorf("offset %d: local %v doesn't match target %d (kind %s)", offset, relo.Local, relo.Target, relo.Kind)
+						t.Errorf("offset %d: local %v doesn't match target %d (kind %s)", offset, relo.local, relo.target, relo.kind)
 					}
 				}
 			})
diff --git a/link/kprobe_test.go b/link/kprobe_test.go
index 444033c..34b3306 100644
--- a/link/kprobe_test.go
+++ b/link/kprobe_test.go
@@ -2,6 +2,8 @@ package link
 
 import (
 	"errors"
+	"fmt"
+	"math"
 	"os"
 	"testing"
 
@@ -405,3 +407,50 @@ func TestKprobeCookie(t *testing.T) {
 	}
 	k.Close()
 }
+
+func TestKprobeToken(t *testing.T) {
+	tests := []struct {
+		args     probeArgs
+		expected string
+	}{
+		{probeArgs{symbol: "symbol"}, "symbol"},
+		{probeArgs{symbol: "symbol", offset: 1}, "symbol+0x1"},
+		{probeArgs{symbol: "symbol", offset: 65535}, "symbol+0xffff"},
+		{probeArgs{symbol: "symbol", offset: 65536}, "symbol+0x10000"},
+	}
+
+	for i, tt := range tests {
+		t.Run(fmt.Sprint(i), func(t *testing.T) {
+			po := kprobeToken(tt.args)
+			if tt.expected != po {
+				t.Errorf("Expected symbol+offset to be '%s', got '%s'", tt.expected, po)
+			}
+		})
+	}
+}
+
+// Test Kprobe with Offset
+func TestKprobeOffset(t *testing.T) {
+	tests := []struct {
+		offset uint64
+		ok     bool
+	}{
+		{0, true},
+		{math.MaxUint64, false},
+	}
+
+	prog := mustLoadProgram(t, ebpf.Kprobe, 0, "")
+	for i, tt := range tests {
+		t.Run(fmt.Sprint(i), func(t *testing.T) {
+			k, err := Kprobe(ksym, prog, &KprobeOptions{Offset: tt.offset})
+
+			ok := err == nil
+			if tt.ok != ok {
+				t.Errorf("Expected symbol+offset load %v', got '%v'", tt.ok, ok)
+			}
+			if ok {
+				k.Close()
+			}
+		})
+	}
+}