From 71dc79e653490ec386ca3f56970b412b6790b2c4 Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Mon, 9 Oct 2023 17:51:20 -0700 Subject: [PATCH] `secbench`: Benchmark optimization duration and compression ratio. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Current values for the Sentry filters: ``` │ current │ │ build-sec │ SentrySystrap 13.73m ± 0% SentryKVM 16.36m ± 0% │ current │ │ compression-ratio │ SentrySystrap 2.165 ± 0% SentryKVM 2.132 ± 0% │ current │ │ gen-instr │ SentrySystrap 1.288k ± 0% SentryKVM 1.373k ± 0% │ current │ │ opt-instr │ SentrySystrap 595.0 ± 0% SentryKVM 644.0 ± 0% │ current │ │ opt-sec │ SentrySystrap 819.0µ ± 2% SentryKVM 897.0µ ± 1% ``` PiperOrigin-RevId: 572089103 --- pkg/seccomp/seccomp.go | 36 +++++++++++++++---- pkg/seccomp/seccomp_test.go | 4 +-- pkg/sentry/platform/kvm/machine.go | 2 +- .../platform/ptrace/subprocess_linux.go | 2 +- .../platform/systrap/subprocess_linux.go | 2 +- pkg/sentry/platform/systrap/sysmsg_thread.go | 2 +- runsc/specutils/seccomp/seccomp.go | 2 +- test/secbench/runner.go | 2 +- test/secbench/secbench.go | 18 ++++++---- test/secbench/secbenchdef/BUILD | 1 + test/secbench/secbenchdef/secbenchdef.go | 7 ++-- 11 files changed, 56 insertions(+), 22 deletions(-) diff --git a/pkg/seccomp/seccomp.go b/pkg/seccomp/seccomp.go index 5dfa9cf3d..1f018fd96 100644 --- a/pkg/seccomp/seccomp.go +++ b/pkg/seccomp/seccomp.go @@ -19,6 +19,7 @@ package seccomp import ( "fmt" "sort" + "time" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/bpf" @@ -67,7 +68,7 @@ func Install(rules SyscallRules, denyRules SyscallRules) error { log.Infof("Installing seccomp filters for %d syscalls (action=%v)", len(rules), defaultAction) - instrs, err := BuildProgram([]RuleSet{ + instrs, _, err := BuildProgram([]RuleSet{ { Rules: denyRules, Action: defaultAction, @@ -269,9 +270,25 @@ func (l *labelSet) Push(labelSuffix string, newRuleMatch, newRuleMismatch label) } } +// BuildStats contains information about seccomp program generation. +type BuildStats struct { + // SizeBeforeOptimizations and SizeAfterOptimizations correspond to the + // number of instructions in the program before vs after optimization. + SizeBeforeOptimizations, SizeAfterOptimizations int + + // BuildDuration is the amount of time it took to build the program (before + // BPF bytecode optimizations). + BuildDuration time.Duration + + // OptimizeDuration is the amount of time it took to run BPF bytecode + // optimizations. + OptimizeDuration time.Duration +} + // BuildProgram builds a BPF program from the given map of actions to matching // SyscallRules. The single generated program covers all provided RuleSets. -func BuildProgram(rules []RuleSet, defaultAction, badArchAction linux.BPFAction) ([]bpf.Instruction, error) { +func BuildProgram(rules []RuleSet, defaultAction, badArchAction linux.BPFAction) ([]bpf.Instruction, BuildStats, error) { + start := time.Now() program := &syscallProgram{ program: bpf.NewProgramBuilder(), } @@ -284,7 +301,7 @@ func BuildProgram(rules []RuleSet, defaultAction, badArchAction linux.BPFAction) program.Stmt(bpf.Ld|bpf.Abs|bpf.W, seccompDataOffsetArch) program.IfNot(bpf.Jmp|bpf.Jeq|bpf.K, LINUX_AUDIT_ARCH, badArchLabel) if err := buildIndex(rules, program); err != nil { - return nil, err + return nil, BuildStats{}, err } // Default label if none of the rules matched: @@ -297,13 +314,20 @@ func BuildProgram(rules []RuleSet, defaultAction, badArchAction linux.BPFAction) insns, err := program.program.Instructions() if err != nil { - return insns, err + return nil, BuildStats{}, err } beforeOpt := len(insns) + buildDuration := time.Since(start) insns = bpf.Optimize(insns) + optimizeDuration := time.Since(start) - buildDuration afterOpt := len(insns) - log.Debugf("Seccomp program optimized from %d to %d instructions", beforeOpt, afterOpt) - return insns, nil + log.Debugf("Seccomp program optimized from %d to %d instructions; took %v to build and %v to optimize", beforeOpt, afterOpt, buildDuration, optimizeDuration) + return insns, BuildStats{ + SizeBeforeOptimizations: beforeOpt, + SizeAfterOptimizations: afterOpt, + BuildDuration: buildDuration, + OptimizeDuration: optimizeDuration, + }, nil } // buildIndex builds a BST to quickly search through all syscalls. diff --git a/pkg/seccomp/seccomp_test.go b/pkg/seccomp/seccomp_test.go index bd7f21445..8954c5468 100644 --- a/pkg/seccomp/seccomp_test.go +++ b/pkg/seccomp/seccomp_test.go @@ -877,7 +877,7 @@ func TestBasic(t *testing.T) { }, } { t.Run(test.name, func(t *testing.T) { - instrs, err := BuildProgram(test.ruleSets, test.defaultAction, test.badArchAction) + instrs, _, err := BuildProgram(test.ruleSets, test.defaultAction, test.badArchAction) if err != nil { t.Fatalf("BuildProgram() got error: %v", err) } @@ -913,7 +913,7 @@ func TestRandom(t *testing.T) { } t.Logf("Testing filters: %v", syscallRules) - instrs, err := BuildProgram([]RuleSet{ + instrs, _, err := BuildProgram([]RuleSet{ { Rules: syscallRules, Action: linux.SECCOMP_RET_ALLOW, diff --git a/pkg/sentry/platform/kvm/machine.go b/pkg/sentry/platform/kvm/machine.go index 2708f7ddc..87cc4e883 100644 --- a/pkg/sentry/platform/kvm/machine.go +++ b/pkg/sentry/platform/kvm/machine.go @@ -791,7 +791,7 @@ func seccompMmapRules(m *machine) { Action: linux.SECCOMP_RET_TRAP, }, } - instrs, err := seccomp.BuildProgram(rules, linux.SECCOMP_RET_ALLOW, linux.SECCOMP_RET_ALLOW) + instrs, _, err := seccomp.BuildProgram(rules, linux.SECCOMP_RET_ALLOW, linux.SECCOMP_RET_ALLOW) if err != nil { panic(fmt.Sprintf("failed to build rules: %v", err)) } diff --git a/pkg/sentry/platform/ptrace/subprocess_linux.go b/pkg/sentry/platform/ptrace/subprocess_linux.go index a6ce81a56..02f4c31da 100644 --- a/pkg/sentry/platform/ptrace/subprocess_linux.go +++ b/pkg/sentry/platform/ptrace/subprocess_linux.go @@ -114,7 +114,7 @@ func attachedThread(flags uintptr, defaultAction linux.BPFAction) (*thread, erro }) } rules = appendArchSeccompRules(rules, defaultAction) - instrs, err := seccomp.BuildProgram(rules, defaultAction, defaultAction) + instrs, _, err := seccomp.BuildProgram(rules, defaultAction, defaultAction) if err != nil { return nil, err } diff --git a/pkg/sentry/platform/systrap/subprocess_linux.go b/pkg/sentry/platform/systrap/subprocess_linux.go index 6da1e3d01..33485fa7a 100644 --- a/pkg/sentry/platform/systrap/subprocess_linux.go +++ b/pkg/sentry/platform/systrap/subprocess_linux.go @@ -130,7 +130,7 @@ func attachedThread(flags uintptr, defaultAction linux.BPFAction) (*thread, erro rules = append(rules, ruleSet) rules = appendArchSeccompRules(rules) } - instrs, err := seccomp.BuildProgram(rules, defaultAction, defaultAction) + instrs, _, err := seccomp.BuildProgram(rules, defaultAction, defaultAction) if err != nil { return nil, err } diff --git a/pkg/sentry/platform/systrap/sysmsg_thread.go b/pkg/sentry/platform/systrap/sysmsg_thread.go index f6b809293..d63f73061 100644 --- a/pkg/sentry/platform/systrap/sysmsg_thread.go +++ b/pkg/sentry/platform/systrap/sysmsg_thread.go @@ -146,7 +146,7 @@ func sysmsgThreadRules(stubStart uintptr) []bpf.Instruction { Action: linux.SECCOMP_RET_ALLOW, }, }...) - instrs, err := seccomp.BuildProgram(rules, linux.SECCOMP_RET_TRAP, linux.SECCOMP_RET_TRAP) + instrs, _, err := seccomp.BuildProgram(rules, linux.SECCOMP_RET_TRAP, linux.SECCOMP_RET_TRAP) if err != nil { panic(fmt.Sprintf("failed to build rules for sysmsg threads: %v", err)) } diff --git a/runsc/specutils/seccomp/seccomp.go b/runsc/specutils/seccomp/seccomp.go index a8e74ed93..5e36eb3c0 100644 --- a/runsc/specutils/seccomp/seccomp.go +++ b/runsc/specutils/seccomp/seccomp.go @@ -51,7 +51,7 @@ func BuildProgram(s *specs.LinuxSeccomp) (bpf.Program, error) { return bpf.Program{}, fmt.Errorf("invalid seccomp rules: %w", err) } - instrs, err := seccomp.BuildProgram(ruleset, defaultAction, killThreadAction) + instrs, _, err := seccomp.BuildProgram(ruleset, defaultAction, killThreadAction) if err != nil { return bpf.Program{}, fmt.Errorf("building seccomp program: %w", err) } diff --git a/test/secbench/runner.go b/test/secbench/runner.go index 86f4f01cd..ee55ec28a 100644 --- a/test/secbench/runner.go +++ b/test/secbench/runner.go @@ -77,7 +77,7 @@ func run(req secbenchdef.BenchRunRequest) (secbenchdef.BenchRunResponse, error) // We're ready. Install the BPF program. if req.InstallFilter { - if err := install(bn.Program); err != nil { + if err := install(bn.Instructions); err != nil { panic(fmt.Sprintf("cannot install BPF program: %v", err)) } } diff --git a/test/secbench/secbench.go b/test/secbench/secbench.go index 54235deb6..9ff41bfd3 100644 --- a/test/secbench/secbench.go +++ b/test/secbench/secbench.go @@ -35,14 +35,14 @@ import ( "gvisor.dev/gvisor/test/secbench/secbenchdef" ) -// BenchFromSyscallRules returns a new Bench creates from SyscallRules. +// BenchFromSyscallRules returns a new Bench created from SyscallRules. func BenchFromSyscallRules(b *testing.B, name string, profile secbenchdef.Profile, rules seccomp.SyscallRules, denyRules seccomp.SyscallRules) secbenchdef.Bench { // If there is a rule allowing rt_sigreturn to be called, // also add a rule for the stand-in syscall number instead. if sigreturnRule, found := rules[unix.SYS_RT_SIGRETURN]; found { rules[uintptr(secbenchdef.RTSigreturn.Data(profile.Arch).Nr)] = sigreturnRule } - instrs, err := seccomp.BuildProgram([]seccomp.RuleSet{ + insns, buildStats, err := seccomp.BuildProgram([]seccomp.RuleSet{ { Rules: denyRules, Action: linux.SECCOMP_RET_ERRNO, @@ -56,9 +56,10 @@ func BenchFromSyscallRules(b *testing.B, name string, profile secbenchdef.Profil b.Fatalf("BuildProgram() failed: %v", err) } return secbenchdef.Bench{ - Name: name, - Profile: secbenchdef.Profile(profile), - Program: instrs, + Name: name, + Profile: secbenchdef.Profile(profile), + Instructions: insns, + BuildStats: buildStats, } } @@ -153,10 +154,15 @@ func RunBench(b *testing.B, bn secbenchdef.Bench) { // two runs. // If there are no syscall sequences that will be approved, then we can // skip running the runner the second time altogether. - program, err := bpf.Compile(bn.Program) + program, err := bpf.Compile(bn.Instructions) if err != nil { b.Fatalf("program does not compile: %v", err) } + b.ReportMetric(float64(bn.BuildStats.BuildDuration.Nanoseconds()), "build-ns") + b.ReportMetric(float64(bn.BuildStats.OptimizeDuration.Nanoseconds()), "opt-ns") + b.ReportMetric(float64(bn.BuildStats.SizeBeforeOptimizations), "gen-instr") + b.ReportMetric(float64(bn.BuildStats.SizeAfterOptimizations), "opt-instr") + b.ReportMetric(float64(bn.BuildStats.SizeBeforeOptimizations)/float64(bn.BuildStats.SizeAfterOptimizations), "compression-ratio") activeSequences := make([]bool, len(bn.Profile.Sequences)) positiveSequenceIndexes := make(map[int]struct{}, len(bn.Profile.Sequences)) for i, seq := range bn.Profile.Sequences { diff --git a/test/secbench/secbenchdef/BUILD b/test/secbench/secbenchdef/BUILD index 89fec50c3..1c960e4dd 100644 --- a/test/secbench/secbenchdef/BUILD +++ b/test/secbench/secbenchdef/BUILD @@ -17,6 +17,7 @@ go_library( deps = [ "//pkg/abi/linux", "//pkg/bpf", + "//pkg/seccomp", "@org_golang_x_sys//unix:go_default_library", ], ) diff --git a/test/secbench/secbenchdef/secbenchdef.go b/test/secbench/secbenchdef/secbenchdef.go index f5afd8225..338bfc1f5 100644 --- a/test/secbench/secbenchdef/secbenchdef.go +++ b/test/secbench/secbenchdef/secbenchdef.go @@ -21,6 +21,7 @@ import ( "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/bpf" + "gvisor.dev/gvisor/pkg/seccomp" ) // Bench represents a benchmark to run. @@ -29,8 +30,10 @@ type Bench struct { Name string `json:"name"` // Profile represents the syscall pattern profile being benchmarked. Profile Profile `json:"profile"` - // Program is the seccomp-bpf program to run the benchmark with. - Program []bpf.Instruction `json:"program"` + // Instructions is the seccomp-bpf program to run the benchmark with. + Instructions []bpf.Instruction `json:"instructions"` + // BuildStats contains information on timing and size of the program. + BuildStats seccomp.BuildStats `json:"buildStats"` // AllowRejected can be set to true if some sequences in the application // profile are expected to not be allowed. // If this is the case, the program's overall performance will not be