Skip to content

Commit

Permalink
bpf: Remove Input interface and simply use bytes as the input.
Browse files Browse the repository at this point in the history
This removes the interface indirection from BPF evaluation, which is
a very hot path (runs for every application syscall for seccomp'd containers)
and simplifies the code in general.

```
            │   initial   │         hey_look_no_interfaces         │
            │   sec/op    │   sec/op     vs base                   │
Interpreter   26.31n ± 0%   21.12n ± 0%  -19.73% (p=0.000 n=21+20)
```

PiperOrigin-RevId: 576375947
  • Loading branch information
EtiennePerot authored and gvisor-bot committed Oct 25, 2023
1 parent d377e45 commit d497367
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 86 deletions.
24 changes: 17 additions & 7 deletions pkg/bpf/input_bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ import (
"encoding/binary"
)

// InputBytes implements the Input interface by providing access to a byte
// slice. Unaligned loads are supported.
type InputBytes struct {
// Input represents a source of input data for a BPF program. (BPF
// documentation sometimes refers to the input data as the "packet" due to its
// origins as a packet processing DSL.)
// Unaligned loads are supported.
type Input struct {
// Data is the data accessed through the Input interface.
Data []byte

Expand All @@ -29,30 +31,38 @@ type InputBytes struct {
}

// Load32 implements Input.Load32.
func (i InputBytes) Load32(off uint32) (uint32, bool) {
//
//go:nosplit
func (i *Input) Load32(off uint32) (uint32, bool) {
if uint64(off)+4 > uint64(len(i.Data)) {
return 0, false
}
return i.Order.Uint32(i.Data[int(off):]), true
}

// Load16 implements Input.Load16.
func (i InputBytes) Load16(off uint32) (uint16, bool) {
//
//go:nosplit
func (i *Input) Load16(off uint32) (uint16, bool) {
if uint64(off)+2 > uint64(len(i.Data)) {
return 0, false
}
return i.Order.Uint16(i.Data[int(off):]), true
}

// Load8 implements Input.Load8.
func (i InputBytes) Load8(off uint32) (uint8, bool) {
//
//go:nosplit
func (i *Input) Load8(off uint32) (uint8, bool) {
if uint64(off)+1 > uint64(len(i.Data)) {
return 0, false
}
return i.Data[int(off)], true
}

// Length implements Input.Length.
func (i InputBytes) Length() uint32 {
//
//go:nosplit
func (i *Input) Length() uint32 {
return uint32(len(i.Data))
}
29 changes: 0 additions & 29 deletions pkg/bpf/interpreter.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,35 +217,6 @@ func Compile(insns []Instruction) (Program, error) {
return Program{Optimize(insns)}, nil
}

// Input represents a source of input data for a BPF program. (BPF
// documentation sometimes refers to the input data as the "packet" due to its
// origins as a packet processing DSL.)
//
// For all of Input's Load methods:
//
// - The second (bool) return value is true if the load succeeded and false
// otherwise.
//
// - Inputs should not assume that the loaded range falls within the input
// data's length. Inputs should return false if the load falls outside of the
// input data.
//
// - Inputs should not assume that the offset is correctly aligned. Inputs may
// choose to service or reject loads to unaligned addresses.
type Input interface {
// Load32 reads 32 bits from the input starting at the given byte offset.
Load32(off uint32) (uint32, bool)

// Load16 reads 16 bits from the input starting at the given byte offset.
Load16(off uint32) (uint16, bool)

// Load8 reads 8 bits from the input starting at the given byte offset.
Load8(off uint32) (uint8, bool)

// Length returns the length of the input in bytes.
Length() uint32
}

// machine represents the state of a BPF virtual machine.
type machine struct {
A uint32
Expand Down
14 changes: 3 additions & 11 deletions pkg/bpf/interpreter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func TestExecErrors(t *testing.T) {
t.Errorf("%s: unexpected compilation error: %v", test.desc, err)
continue
}
ret, err := Exec(p, InputBytes{nil, binary.BigEndian})
ret, err := Exec(p, Input{nil, binary.BigEndian})
if err != test.expectedErr {
t.Errorf("%s: expected execution error %q, got (%d, %v)", test.desc, test.expectedErr, ret, err)
}
Expand Down Expand Up @@ -724,7 +724,7 @@ func TestValidInstructions(t *testing.T) {
t.Errorf("%s: unexpected compilation error: %v", test.desc, err)
continue
}
ret, err := Exec(p, InputBytes{test.input, binary.BigEndian})
ret, err := Exec(p, Input{test.input, binary.BigEndian})
if err != nil {
t.Errorf("%s: expected return value of %d, got execution error: %v", test.desc, test.expectedRet, err)
continue
Expand Down Expand Up @@ -798,17 +798,9 @@ func TestSimpleFilter(t *testing.T) {
}
}

// seccompData is equivalent to struct seccomp_data.
type seccompData struct {
nr uint32
arch uint32
instructionPointer uint64
args [6]uint64
}

// asInput converts a seccompData to a bpf.Input.
func dataAsInput(data *linux.SeccompData) Input {
return InputBytes{marshal.Marshal(data), hostarch.ByteOrder}
return Input{marshal.Marshal(data), hostarch.ByteOrder}
}

// BenchmarkInterpreter benchmarks the execution of the sample filter
Expand Down
2 changes: 1 addition & 1 deletion pkg/seccomp/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ go_library(
deps = [
"//pkg/abi/linux",
"//pkg/bpf",
"//pkg/hostarch",
"//pkg/log",
"@org_golang_x_sys//unix:go_default_library",
],
Expand All @@ -36,6 +37,5 @@ go_test(
deps = [
"//pkg/abi/linux",
"//pkg/bpf",
"//pkg/hostarch",
],
)
15 changes: 15 additions & 0 deletions pkg/seccomp/seccomp.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"gvisor.dev/gvisor/pkg/abi/linux"
"gvisor.dev/gvisor/pkg/bpf"
"gvisor.dev/gvisor/pkg/hostarch"
"gvisor.dev/gvisor/pkg/log"
)

Expand Down Expand Up @@ -491,3 +492,17 @@ func (n *node) traverse(fn traverseFunc, rules []RuleSet, program *syscallProgra
}
return n.right.traverse(fn, rules, program)
}

// DataAsBPFInput converts a linux.SeccompData to a bpf.Input.
// It uses `buf` as scratch buffer; this buffer must be wide enough
// to accommodate a mashaled version of `d`.
func DataAsBPFInput(d *linux.SeccompData, buf []byte) bpf.Input {
if len(buf) < d.SizeBytes() {
panic(fmt.Sprintf("buffer must be at least %d bytes long", d.SizeBytes()))
}
d.MarshalUnsafe(buf)
return bpf.Input{
Data: buf,
Order: hostarch.ByteOrder,
}
}
18 changes: 5 additions & 13 deletions pkg/seccomp/seccomp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (

"gvisor.dev/gvisor/pkg/abi/linux"
"gvisor.dev/gvisor/pkg/bpf"
"gvisor.dev/gvisor/pkg/hostarch"
)

//go:embed victim
Expand All @@ -56,17 +55,9 @@ func newVictim() (string, error) {
return path, nil
}

// dataAsInput converts a linux.SeccompData to a bpf.Input.
func dataAsInput(d *linux.SeccompData) bpf.Input {
buf := make([]byte, d.SizeBytes())
d.MarshalUnsafe(buf)
return bpf.InputBytes{
Data: buf,
Order: hostarch.ByteOrder,
}
}

func TestBasic(t *testing.T) {
buf := make([]byte, (&linux.SeccompData{}).SizeBytes())

type spec struct {
// desc is the test's description.
desc string
Expand Down Expand Up @@ -886,7 +877,7 @@ func TestBasic(t *testing.T) {
t.Fatalf("bpf.Compile() got error: %v", err)
}
for _, spec := range test.specs {
got, err := bpf.Exec(p, dataAsInput(&spec.data))
got, err := bpf.Exec(p, DataAsBPFInput(&spec.data, buf))
if err != nil {
t.Fatalf("%s: bpf.Exec() got error: %v", spec.desc, err)
}
Expand Down Expand Up @@ -926,9 +917,10 @@ func TestRandom(t *testing.T) {
if err != nil {
t.Fatalf("bpf.Compile() got error: %v", err)
}
buf := make([]byte, (&linux.SeccompData{}).SizeBytes())
for i := uint32(0); i < 200; i++ {
data := linux.SeccompData{Nr: int32(i), Arch: LINUX_AUDIT_ARCH}
got, err := bpf.Exec(p, dataAsInput(&data))
got, err := bpf.Exec(p, DataAsBPFInput(&data, buf))
if err != nil {
t.Errorf("bpf.Exec() got error: %v, for syscall %d", err, i)
continue
Expand Down
2 changes: 1 addition & 1 deletion pkg/sentry/kernel/seccomp.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const maxSyscallFilterInstructions = 1 << 15
func dataAsBPFInput(t *Task, d *linux.SeccompData) bpf.Input {
buf := t.CopyScratchBuffer(d.SizeBytes())
d.MarshalUnsafe(buf)
return bpf.InputBytes{
return bpf.Input{
Data: buf,
// Go-marshal always uses the native byte order.
Order: hostarch.ByteOrder,
Expand Down
3 changes: 1 addition & 2 deletions runsc/specutils/seccomp/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ go_test(
deps = [
"//pkg/abi/linux",
"//pkg/bpf",
"//pkg/hostarch",
"//pkg/marshal",
"//pkg/seccomp",
"@com_github_opencontainers_runtime_spec//specs-go:go_default_library",
"@org_golang_x_sys//unix:go_default_library",
],
Expand Down
16 changes: 6 additions & 10 deletions runsc/specutils/seccomp/seccomp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,9 @@ import (
"golang.org/x/sys/unix"
"gvisor.dev/gvisor/pkg/abi/linux"
"gvisor.dev/gvisor/pkg/bpf"
"gvisor.dev/gvisor/pkg/hostarch"
"gvisor.dev/gvisor/pkg/marshal"
"gvisor.dev/gvisor/pkg/seccomp"
)

// asInput converts a linux.SeccompData to a bpf.Input.
func asInput(d *linux.SeccompData) bpf.Input {
return bpf.InputBytes{marshal.Marshal(d), hostarch.ByteOrder}
}

// testInput creates an Input struct with given seccomp input values.
func testInput(arch uint32, syscallName string, args *[6]uint64) bpf.Input {
syscallNo, err := lookupSyscallNo(arch, syscallName)
Expand All @@ -49,8 +43,7 @@ func testInput(arch uint32, syscallName string, args *[6]uint64) bpf.Input {
Arch: arch,
Args: *args,
}

return asInput(&data)
return seccomp.DataAsBPFInput(&data, make([]byte, data.SizeBytes()))
}

// testCase holds a seccomp test case.
Expand Down Expand Up @@ -95,7 +88,10 @@ var (
},
// Syscall matches but the arch is AUDIT_ARCH_X86 so the return
// value is the bad arch action.
input: asInput(&linux.SeccompData{Nr: 183, Arch: 0x40000003}), //
input: seccomp.DataAsBPFInput(
&linux.SeccompData{Nr: 183, Arch: 0x40000003},
make([]byte, (&linux.SeccompData{}).SizeBytes()),
),
expected: uint32(killThreadAction),
},
{
Expand Down
2 changes: 0 additions & 2 deletions test/secbench/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ go_library(
deps = [
"//pkg/abi/linux",
"//pkg/bpf",
"//pkg/hostarch",
"//pkg/marshal",
"//pkg/seccomp",
"//pkg/test/testutil",
"//test/secbench/secbenchdef",
Expand Down
15 changes: 5 additions & 10 deletions test/secbench/secbench.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ import (
"golang.org/x/sys/unix"
"gvisor.dev/gvisor/pkg/abi/linux"
"gvisor.dev/gvisor/pkg/bpf"
"gvisor.dev/gvisor/pkg/hostarch"
"gvisor.dev/gvisor/pkg/marshal"
"gvisor.dev/gvisor/pkg/seccomp"
"gvisor.dev/gvisor/pkg/test/testutil"
"gvisor.dev/gvisor/test/secbench/secbenchdef"
Expand Down Expand Up @@ -107,8 +105,8 @@ func runRequest(runReq secbenchdef.BenchRunRequest) (secbenchdef.BenchRunRespons
return runResp, nil
}

func evalSyscall(program bpf.Program, arch uint32, sc secbenchdef.Syscall) (uint32, error) {
scData := &linux.SeccompData{
func evalSyscall(program bpf.Program, arch uint32, sc secbenchdef.Syscall, buf []byte) (uint32, error) {
return bpf.Exec(program, seccomp.DataAsBPFInput(&linux.SeccompData{
Nr: int32(sc.Sysno),
Arch: arch,
Args: [6]uint64{
Expand All @@ -119,11 +117,7 @@ func evalSyscall(program bpf.Program, arch uint32, sc secbenchdef.Syscall) (uint
uint64(sc.Args[4]),
uint64(sc.Args[5]),
},
}
return bpf.Exec(program, bpf.InputBytes{
Data: marshal.Marshal(scData),
Order: hostarch.ByteOrder,
})
}, buf))
}

// Number of times we scale b.N by.
Expand All @@ -139,6 +133,7 @@ func RunBench(b *testing.B, bn secbenchdef.Bench) {
randSeed := time.Now().UnixNano()
b.Logf("Running with %d iterations (scaled by %dx), random seed %d...", b.N, iterationScaleFactor, randSeed)
iterations := uint64(b.N * iterationScaleFactor)
buf := make([]byte, (&linux.SeccompData{}).SizeBytes())

// Check if there are any sequences where the syscall will be approved.
// If there is any, we will need to run the runner twice: Once with the
Expand All @@ -160,7 +155,7 @@ func RunBench(b *testing.B, bn secbenchdef.Bench) {
for i, seq := range bn.Profile.Sequences {
result := int64(-1)
for _, sc := range seq.Syscalls {
scResult, err := evalSyscall(program, bn.Profile.Arch, sc)
scResult, err := evalSyscall(program, bn.Profile.Arch, sc, buf)
if err != nil {
b.Fatalf("cannot eval program with syscall %v: %v", sc, err)
}
Expand Down

0 comments on commit d497367

Please sign in to comment.