From 303611b6088aa7198df961eda682fd904d61df7d Mon Sep 17 00:00:00 2001 From: Mahe Tardy Date: Thu, 13 Jun 2024 16:53:12 +0200 Subject: [PATCH 1/6] bpf: remove rate limit maps and structs for kernel <5.3 [ upstream commit ed824b3ae42515feb5fdd044a4b6cafde9af4b66 ] Since the rate limit feature is only available for LARGE_BPF_PROG, let's remove the unnecessary map and the struct from the small BPF progs. Signed-off-by: Mahe Tardy --- bpf/process/types/basic.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bpf/process/types/basic.h b/bpf/process/types/basic.h index 58409ce1230..639a9519880 100644 --- a/bpf/process/types/basic.h +++ b/bpf/process/types/basic.h @@ -1792,6 +1792,8 @@ do_action_signal(int signal) */ #define KEY_BYTES_PER_ARG 40 +#ifdef __LARGE_BPF_PROG + struct ratelimit_key { __u64 func_id; __u64 retprobe_id; @@ -1829,7 +1831,6 @@ struct { __type(value, __u8[sizeof(struct ratelimit_key) + 128]); } ratelimit_ro_heap SEC(".maps"); -#ifdef __LARGE_BPF_PROG static inline __attribute__((always_inline)) bool rate_limit(__u64 ratelimit_interval, struct msg_generic_kprobe *e) { From 16f4c33119389cbbdc98fcc1d0adfdf414e6b30c Mon Sep 17 00:00:00 2001 From: Mahe Tardy Date: Thu, 13 Jun 2024 16:59:17 +0200 Subject: [PATCH 2/6] pkg/sensors: pin the ratelimit_map to the fs [ upstream commit 38ab0122a8d69368833457aad6d3d68858c0b561 ] Signed-off-by: Mahe Tardy --- pkg/sensors/tracing/generickprobe.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/sensors/tracing/generickprobe.go b/pkg/sensors/tracing/generickprobe.go index 98dc17abac9..a7d2b976c53 100644 --- a/pkg/sensors/tracing/generickprobe.go +++ b/pkg/sensors/tracing/generickprobe.go @@ -310,6 +310,11 @@ func createMultiKprobeSensor(sensorPath string, multiIDs, multiRetIDs []idtable. maps = append(maps, socktrack) } + if kernels.EnableLargeProgs() { + ratelimitMap := program.MapBuilderPin("ratelimit_map", sensors.PathJoin(sensorPath, "ratelimit_map"), load) + maps = append(maps, ratelimitMap) + } + filterMap.SetMaxEntries(len(multiIDs)) configMap.SetMaxEntries(len(multiIDs)) @@ -950,6 +955,11 @@ func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn, selMaps out.maps = append(out.maps, socktrack) } + if kernels.EnableLargeProgs() { + ratelimitMap := program.MapBuilderPin("ratelimit_map", sensors.PathJoin(in.sensorPath, "ratelimit_map"), load) + out.maps = append(out.maps, ratelimitMap) + } + if setRetprobe { pinRetProg := sensors.PathJoin(pinPath, fmt.Sprintf("%s_ret_prog", kprobeEntry.funcName)) loadret := program.Builder( From 083d49ea27befa820d312852945b0c75f7dfe2da Mon Sep 17 00:00:00 2001 From: Mahe Tardy Date: Thu, 13 Jun 2024 17:09:23 +0200 Subject: [PATCH 3/6] pkg/sensors: reduce ratelimit map memory footprint [ upstream commit 850410baea83122cb419be887c29b07b088bd818 ] This commit is very similar to 22510d98a16594b19b6ef8c68d3db5da5cb0a8cc For every ratelimit map loaded, we add ~10MB of kernel memory, and each kprobe added was adding a ratelimit map. We now only load that map if the user used the rateLimit field in a matchActions to reduce the memory footprint of this feature when unused. Signed-off-by: Mahe Tardy --- bpf/process/types/basic.h | 2 +- pkg/sensors/tracing/generickprobe.go | 32 ++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/bpf/process/types/basic.h b/bpf/process/types/basic.h index 639a9519880..4eeb6af7098 100644 --- a/bpf/process/types/basic.h +++ b/bpf/process/types/basic.h @@ -1808,7 +1808,7 @@ struct ratelimit_value { struct { __uint(type, BPF_MAP_TYPE_LRU_HASH); - __uint(max_entries, 32768); + __uint(max_entries, 1); // Agent is resizing this if the feature is needed during kprobe load __type(key, struct ratelimit_key); __type(value, struct ratelimit_value); } ratelimit_map SEC(".maps"); diff --git a/pkg/sensors/tracing/generickprobe.go b/pkg/sensors/tracing/generickprobe.go index a7d2b976c53..ce90c45e6c8 100644 --- a/pkg/sensors/tracing/generickprobe.go +++ b/pkg/sensors/tracing/generickprobe.go @@ -66,7 +66,10 @@ const ( CharBufErrorTooLarge = -3 CharBufSavedForRetprobe = -4 - stackTraceMapMaxEntries = 32768 // this value could be fine tuned + // The following values could be fine tuned if either those feature use too + // much kernel memory when enabled. + stackTraceMapMaxEntries = 32768 + ratelimitMapMaxEntries = 32768 ) func kprobeCharBufErrorToString(e int32) string { @@ -138,6 +141,9 @@ type genericKprobe struct { // is there stacktrace defined in the kprobe hasStackTrace bool + // is there ratelimit defined in the kprobe + hasRatelimit bool + customHandler eventhandler.Handler } @@ -217,13 +223,15 @@ func createMultiKprobeSensor(sensorPath string, multiIDs, multiRetIDs []idtable. var maps []*program.Map oneKprobeHasStackTrace := false + oneKprobeHasRatelimit := false for _, id := range multiIDs { gk, err := genericKprobeTableGet(id) if err != nil { - logger.GetLogger().WithField("id", id).WithError(err).Warn("createMultiKprobeSensor: failed to retrieve generic kprobe from table, stacktrace could malfunction") + logger.GetLogger().WithField("id", id).WithError(err).Warn("createMultiKprobeSensor: failed to retrieve generic kprobe from table, stacktrace or ratelimit could malfunction") continue } oneKprobeHasStackTrace = oneKprobeHasStackTrace || gk.hasStackTrace + oneKprobeHasRatelimit = oneKprobeHasRatelimit || gk.hasRatelimit } loadProgName := "bpf_multi_kprobe_v53.o" @@ -312,6 +320,9 @@ func createMultiKprobeSensor(sensorPath string, multiIDs, multiRetIDs []idtable. if kernels.EnableLargeProgs() { ratelimitMap := program.MapBuilderPin("ratelimit_map", sensors.PathJoin(sensorPath, "ratelimit_map"), load) + if oneKprobeHasRatelimit { + ratelimitMap.SetMaxEntries(ratelimitMapMaxEntries) + } maps = append(maps, ratelimitMap) } @@ -811,6 +822,7 @@ func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn, selMaps hasOverride: selectors.HasOverride(f), customHandler: in.customHandler, hasStackTrace: selectorsHaveStackTrace(f.Selectors), + hasRatelimit: selectorsHaveRateLimit(f.Selectors), } // Parse Filters into kernel filter logic @@ -957,6 +969,11 @@ func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn, selMaps if kernels.EnableLargeProgs() { ratelimitMap := program.MapBuilderPin("ratelimit_map", sensors.PathJoin(in.sensorPath, "ratelimit_map"), load) + if kprobeEntry.hasRatelimit { + // similarly as for stacktrace, we expand the max size only if + // needed to reduce the memory footprint when unused + ratelimitMap.SetMaxEntries(ratelimitMapMaxEntries) + } out.maps = append(out.maps, ratelimitMap) } @@ -1864,3 +1881,14 @@ func selectorsHaveStackTrace(selectors []v1alpha1.KProbeSelector) bool { } return false } + +func selectorsHaveRateLimit(selectors []v1alpha1.KProbeSelector) bool { + for _, selector := range selectors { + for _, matchAction := range selector.MatchActions { + if len(matchAction.RateLimit) > 0 { + return true + } + } + } + return false +} From 0120ac35a0d767774def46f22b58c78f4fe3a275 Mon Sep 17 00:00:00 2001 From: Kevin Sheldrake Date: Thu, 13 Jun 2024 18:29:35 +0100 Subject: [PATCH 4/6] pkg/sensors: add rate limit test [ upstream commit 1eea47b8522f3d9a09daa02d7498bb86255dfb46 ] Add a NoRateLimit test and a RateLimitTest. Signed-off-by: Kevin Sheldrake --- pkg/sensors/tracing/kprobe_test.go | 120 +++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/pkg/sensors/tracing/kprobe_test.go b/pkg/sensors/tracing/kprobe_test.go index bdd1b28d292..4071ea2f9b9 100644 --- a/pkg/sensors/tracing/kprobe_test.go +++ b/pkg/sensors/tracing/kprobe_test.go @@ -19,6 +19,7 @@ import ( "sync" "syscall" "testing" + "time" "unsafe" "github.com/cilium/ebpf" @@ -5368,6 +5369,125 @@ spec: assert.NoError(t, err) } +func testKprobeRateLimit(t *testing.T, rateLimit bool) { + hook := `apiVersion: cilium.io/v1alpha1 +kind: TracingPolicy +metadata: + name: "datagram" +spec: + kprobes: + - call: "ip_send_skb" + syscall: false + args: + - index: 1 + type: "skb" + label: "datagram" + selectors: + - matchArgs: + - index: 1 + operator: "DAddr" + values: + - "127.0.0.1" + - index: 1 + operator: "DPort" + values: + - "9468" + - index: 1 + operator: "Protocol" + values: + - "IPPROTO_UDP" +` + + if rateLimit { + hook += ` + matchActions: + - action: Post + rateLimit: "5" +` + } + + var doneWG, readyWG sync.WaitGroup + defer doneWG.Wait() + + ctx, cancel := context.WithTimeout(context.Background(), tus.Conf().CmdWaitTime) + defer cancel() + + createCrdFile(t, hook) + obs, err := observertesthelper.GetDefaultObserverWithFile(t, ctx, testConfigFile, tus.Conf().TetragonLib) + if err != nil { + t.Fatalf("GetDefaultObserverWithFile error: %s", err) + } + observertesthelper.LoopEvents(ctx, t, &doneWG, &readyWG, obs) + readyWG.Wait() + + server := "nc.openbsd" + cmdServer := exec.Command(server, "-unvlp", "9468", "-s", "127.0.0.1") + assert.NoError(t, cmdServer.Start()) + time.Sleep(1 * time.Second) + + // Generate 5 datagrams + socket, err := net.Dial("udp", "127.0.0.1:9468") + if err != nil { + fmt.Printf("ERROR dialing socket\n") + panic(err) + } + + for i := 0; i < 5; i++ { + _, err := socket.Write([]byte("data")) + if err != nil { + fmt.Printf("ERROR writing to socket\n") + panic(err) + } + } + + kpChecker := ec.NewProcessKprobeChecker("datagram-checker"). + WithFunctionName(sm.Full("ip_send_skb")). + WithArgs(ec.NewKprobeArgumentListMatcher(). + WithOperator(lc.Ordered). + WithValues( + ec.NewKprobeArgumentChecker().WithLabel(sm.Full("datagram")). + WithSkbArg(ec.NewKprobeSkbChecker(). + WithDaddr(sm.Full("127.0.0.1")). + WithDport(9468). + WithProtocol(sm.Full("IPPROTO_UDP")), + ), + )) + + var checkerSuccess *ec.UnorderedEventChecker + var checkerFailure *ec.UnorderedEventChecker + if rateLimit { + // Rate limit. We should have 1. We shouldn't have 2 (or more) + checkerSuccess = ec.NewUnorderedEventChecker(kpChecker) + checkerFailure = ec.NewUnorderedEventChecker(kpChecker, kpChecker) + } else { + // No rate limit. We should have 5. We shouldn't have 6. + checkerSuccess = ec.NewUnorderedEventChecker(kpChecker, kpChecker, kpChecker, kpChecker, kpChecker) + checkerFailure = ec.NewUnorderedEventChecker(kpChecker, kpChecker, kpChecker, kpChecker, kpChecker, kpChecker) + } + cmdServer.Process.Kill() + + err = jsonchecker.JsonTestCheck(t, checkerSuccess) + assert.NoError(t, err) + err = jsonchecker.JsonTestCheckExpect(t, checkerFailure, true) + assert.NoError(t, err) +} + +func TestKprobeNoRateLimit(t *testing.T) { + if !kernels.EnableLargeProgs() { + t.Skip("Test requires kernel 5.4") + } + + testKprobeRateLimit(t, false) +} + +func TestKprobeRateLimit(t *testing.T) { + if !kernels.EnableLargeProgs() { + t.Skip("Test requires kernel 5.4") + } + + testKprobeRateLimit(t, true) +} + func TestKprobeListSyscallDupsRange(t *testing.T) { if !kernels.MinKernelVersion("5.3.0") { t.Skip("TestCopyFd requires at least 5.3.0 version") From ca3509c648aba2c4d3e4f4222999439ce27f2a4e Mon Sep 17 00:00:00 2001 From: Mahe Tardy Date: Wed, 19 Jun 2024 12:14:28 +0200 Subject: [PATCH 5/6] pkg/sensors: fix for ratelimit_map wrong path pinning [ upstream commit a1a24993b71df6e7065aa3772f4b53eb7b6e3d14 ] Commit 38ab0122a8d69368833457aad6d3d68858c0b561 pinned the ratelimit_map to the fs but used sensorPath instead pinPath since this is a per kprobe map and not a per sensor map. Signed-off-by: Mahe Tardy --- pkg/sensors/tracing/generickprobe.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/sensors/tracing/generickprobe.go b/pkg/sensors/tracing/generickprobe.go index ce90c45e6c8..e58cdf62b41 100644 --- a/pkg/sensors/tracing/generickprobe.go +++ b/pkg/sensors/tracing/generickprobe.go @@ -319,7 +319,7 @@ func createMultiKprobeSensor(sensorPath string, multiIDs, multiRetIDs []idtable. } if kernels.EnableLargeProgs() { - ratelimitMap := program.MapBuilderPin("ratelimit_map", sensors.PathJoin(sensorPath, "ratelimit_map"), load) + ratelimitMap := program.MapBuilderPin("ratelimit_map", sensors.PathJoin(pinPath, "ratelimit_map"), load) if oneKprobeHasRatelimit { ratelimitMap.SetMaxEntries(ratelimitMapMaxEntries) } @@ -968,7 +968,7 @@ func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn, selMaps } if kernels.EnableLargeProgs() { - ratelimitMap := program.MapBuilderPin("ratelimit_map", sensors.PathJoin(in.sensorPath, "ratelimit_map"), load) + ratelimitMap := program.MapBuilderPin("ratelimit_map", sensors.PathJoin(pinPath, "ratelimit_map"), load) if kprobeEntry.hasRatelimit { // similarly as for stacktrace, we expand the max size only if // needed to reduce the memory footprint when unused From cef36a395126a3cbe483f7328a3db06d75140734 Mon Sep 17 00:00:00 2001 From: Mahe Tardy Date: Wed, 17 Jul 2024 19:49:59 +0200 Subject: [PATCH 6/6] vmtests: bump base image used to root-images:20240717.161638 [ upstream commit 9e8f00533ffa2293232161869c4e574ce7f30453 ] For the base image to have nc.openbsd, see cilium/little-vm-helper-images@2af406c7fe7a50cb8b00b2dbbc2344708365ceac. Signed-off-by: Mahe Tardy --- tests/vmtests/fetch-data.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/vmtests/fetch-data.sh b/tests/vmtests/fetch-data.sh index 81aa94ba7d8..0232784a25a 100755 --- a/tests/vmtests/fetch-data.sh +++ b/tests/vmtests/fetch-data.sh @@ -4,7 +4,7 @@ set -eu -o pipefail OCIORG=quay.io/lvh-images -ROOTIMG=$OCIORG/root-images +ROOTIMG=$OCIORG/root-images:20240717.161638@sha256:62a9890111ab39749792fda4f59c8f736fa350ecaedb0667e3eecbbe790d82ed KERNIMG=$OCIORG/kernel-images CONTAINER_ENGINE=${CONTAINER_ENGINE:-docker} KERNEL_VERS="$@"