Add BPF map reuse function #346

javierhonduco · 2023-08-01T15:03:19Z

Wrap bpf_map__reuse_fd to reuse a BPF map accross BPF programs. This
is useful in complex programs that long chains of calls, for example,
via bpf_tail_call that may be loaded in different programs and share
state via reused maps.

Why is this useful

Our global state map, called heap is shared by multiple programs.
Map reuse will soon become a load loading functionality for Parca Agent.

Test Plan

Added selftest +

[javierhonduco@fedora parca-agent]$ sudo bpftool map  | grep heap
3403: percpu_array  name heap  flags 0x0

2812: perf_event  name walk_user_stacktrace_impl  tag 25cb7e352f197144  gpl
        loaded_at 2023-08-01T16:06:01+0100  uid 0
        xlated 7984B  jited 4827B  memlock 12288B  map_ids 3399,3411,3401,3407,3400,3408,3409,3402,3406,3405,3403
        btf_id 1993
        pids parca-agent(1049424)
        metadata:
                name = "parca-agent (https://github.com/parca-dev/parca-agent)"
2814: perf_event  name profile_cpu  tag 3f2053ff90208ef1  gpl
        loaded_at 2023-08-01T16:06:02+0100  uid 0
        xlated 4368B  jited 2850B  memlock 8192B  map_ids 3411,3404,3399,3401,3408,3409,3405,3407,3403,3402
        btf_id 1993
        pids parca-agent(1049424)
        metadata:
                name = "parca-agent (https://github.com/parca-dev/parca-agent)"
2816: perf_event  name walk_ruby_stack  tag 97bb3ee23c47e22d  gpl
        loaded_at 2023-08-01T16:06:02+0100  uid 0
        xlated 87600B  jited 56572B  memlock 98304B  map_ids 3413,3414,3419,3415,3399,3403,3420,3417,3418
        btf_id 1994
        pids parca-agent(1049424)

We are thoroughly testing it, across multiple kernels and with ASAN
enabled, too.

If there were any problems in the future we will quickly notice them and
will be happy to submit bugfixes.

geyslan

Thanks @javierhonduco for this contribution. Could you also provide a selftest for this new helper?

libbpfgo.go

javierhonduco · 2023-08-03T17:39:52Z

selftest/common/common.sh

@@ -33,7 +33,7 @@ okcontinue()   { okay  "$1";         }
 kern_version() {
  _oper=$1; _version=$2; _notfatal=$3;
  _given=$(($(echo $_version | sed 's:\.::g')))
-  _current=$(($(uname -r | cut -d'.' -f1,2 | sed 's:\.::g')))
+  _current=$(($(uname -r | cut -d'.' -f1,2,3 | sed 's:\.::g')))


Updated this check to make it work with my release 6.3.11-200.fc38.x86_64

echo '6.3.11-200.fc38.x86_64' | cut -d'.' -f1,2 | sed 's:\.::g'
63

It seems ok, we need to merge only the first and second field of the numeric version.

Could you please recheck this?

On #356

[javierhonduco@fedora reuse-fd]$ ./run.sh [!] ERROR: kernel 64 not gt than 418

(Now with kernel 6.4.6-200.fc38.x86_64)

Hmmm, it seems a logic issue. I'm going to take a look at this later. Thanks.

Wrap `bpf_map__reuse_fd` to reuse a BPF map accross BPF programs. This is useful in complex programs that long chains of calls, for example, via `bpf_tail_call` that may be loaded in different programs and share state via reused maps. Why is this useful ================= Our global state map, called `heap` is shared by multiple programs. Map reuse will soon become a load loading functionality for Parca Agent. Test Plan ========= Added selftest + ``` [javierhonduco@fedora parca-agent]$ sudo bpftool map | grep heap 3403: percpu_array name heap flags 0x0 ``` ``` 2812: perf_event name walk_user_stacktrace_impl tag 25cb7e352f197144 gpl loaded_at 2023-08-01T16:06:01+0100 uid 0 xlated 7984B jited 4827B memlock 12288B map_ids 3399,3411,3401,3407,3400,3408,3409,3402,3406,3405,3403 btf_id 1993 pids parca-agent(1049424) metadata: name = "parca-agent (https://github.com/parca-dev/parca-agent)" 2814: perf_event name profile_cpu tag 3f2053ff90208ef1 gpl loaded_at 2023-08-01T16:06:02+0100 uid 0 xlated 4368B jited 2850B memlock 8192B map_ids 3411,3404,3399,3401,3408,3409,3405,3407,3403,3402 btf_id 1993 pids parca-agent(1049424) metadata: name = "parca-agent (https://github.com/parca-dev/parca-agent)" 2816: perf_event name walk_ruby_stack tag 97bb3ee23c47e22d gpl loaded_at 2023-08-01T16:06:02+0100 uid 0 xlated 87600B jited 56572B memlock 98304B map_ids 3413,3414,3419,3415,3399,3403,3420,3417,3418 btf_id 1994 pids parca-agent(1049424) ``` We are thoroughly testing it, across multiple kernels and with ASAN enabled, too. If there were any problems in the future we will quickly notice them and will be happy to submit bugfixes. Signed-off-by: Francisco Javier Honduvilla Coto <[email protected]>

geyslan · 2023-08-07T17:02:28Z

@javierhonduco I'm going to review this today 👍🏼

geyslan

@javierhonduco thank your for your changes. As I commented I'll cherry-pick this and line it in with the on going changes.

geyslan · 2023-08-07T18:55:42Z

libbpfgo.go

+	if errC != 0 {
+		return fmt.Errorf("could not reuse bpf map fd %d: %w", fd, syscall.Errno(-errC))
+	}
+	b.fd = C.int(fd)


The fd to be updated is an open/dupped one, not the provided. I also noticed that the map name needs to be updated.

Well, my current work is changing BPFMap to:

https://github.com/aquasecurity/libbpfgo/pull/356/files#diff-5ed7da7003773d8e0a4dca00d4fc08fc5582695eefe22a400db15edfd5aab840R20-R24

As I'm introducing a bunch of changes, I'm going to stop to create conflicts to you 😅, cherry-picking your contribution and doing the required adjustments.

Thanks for taking care of this, did not know about the refactor!

geyslan · 2023-08-07T19:10:43Z

selftest/common/common.sh

@@ -33,7 +33,7 @@ okcontinue()   { okay  "$1";         }
 kern_version() {
  _oper=$1; _version=$2; _notfatal=$3;
  _given=$(($(echo $_version | sed 's:\.::g')))
-  _current=$(($(uname -r | cut -d'.' -f1,2 | sed 's:\.::g')))
+  _current=$(($(uname -r | cut -d'.' -f1,2,3 | sed 's:\.::g')))


echo '6.3.11-200.fc38.x86_64' | cut -d'.' -f1,2 | sed 's:\.::g'
63

It seems ok, we need to merge only the first and second field of the numeric version.

Could you please recheck this?

selftest/create-map/main.go

geyslan · 2023-08-07T21:31:51Z

@javierhonduco thank your for your changes. As I commented I'll cherry-pick this and line it in with the on going changes.

Done: 9acf966

Thanks again @javierhonduco. 🚀

The `ReuseFD` function links the current BPFMap instance to a provided map file descriptor. This enables the reuse of a map that was initiated by another process. Initial effort: aquasecurity#346 Co-authored-by: Francisco Javier Honduvilla Coto <[email protected]>

The `ReuseFD` function links the current BPFMap instance to a provided map file descriptor. This enables the reuse of a map that was initiated by another process. Initial effort: #346 Co-authored-by: Francisco Javier Honduvilla Coto <[email protected]>

javierhonduco force-pushed the add-map-reuse branch 2 times, most recently from c1531f2 to 7ea3e99 Compare August 1, 2023 15:08

rafaeldtinoco requested a review from geyslan August 2, 2023 14:40

geyslan requested changes Aug 2, 2023

View reviewed changes

libbpfgo.go Outdated Show resolved Hide resolved

libbpfgo.go Show resolved Hide resolved

javierhonduco force-pushed the add-map-reuse branch 4 times, most recently from dc8d5c1 to 4339c97 Compare August 3, 2023 17:37

javierhonduco commented Aug 3, 2023

View reviewed changes

javierhonduco force-pushed the add-map-reuse branch from 4339c97 to f0374b4 Compare August 3, 2023 17:40

javierhonduco force-pushed the add-map-reuse branch from f0374b4 to d668e72 Compare August 3, 2023 17:41

javierhonduco requested a review from geyslan August 3, 2023 18:10

geyslan reviewed Aug 7, 2023

View reviewed changes

geyslan closed this Aug 7, 2023

javierhonduco deleted the add-map-reuse branch August 8, 2023 07:34

geyslan mentioned this pull request Aug 8, 2023

fix(selftest): kern_version logic #362

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BPF map reuse function #346

Add BPF map reuse function #346

javierhonduco commented Aug 1, 2023 •

edited

Loading

geyslan left a comment

javierhonduco Aug 3, 2023

geyslan Aug 7, 2023

javierhonduco Aug 8, 2023

geyslan Aug 8, 2023

geyslan commented Aug 7, 2023

geyslan left a comment

geyslan Aug 7, 2023

javierhonduco Aug 8, 2023

geyslan Aug 7, 2023

geyslan commented Aug 7, 2023 •

edited

Loading

Add BPF map reuse function #346

Add BPF map reuse function #346

Conversation

javierhonduco commented Aug 1, 2023 • edited Loading

Why is this useful

Test Plan

geyslan left a comment

Choose a reason for hiding this comment

javierhonduco Aug 3, 2023

Choose a reason for hiding this comment

geyslan Aug 7, 2023

Choose a reason for hiding this comment

javierhonduco Aug 8, 2023

Choose a reason for hiding this comment

geyslan Aug 8, 2023

Choose a reason for hiding this comment

geyslan commented Aug 7, 2023

geyslan left a comment

Choose a reason for hiding this comment

geyslan Aug 7, 2023

Choose a reason for hiding this comment

javierhonduco Aug 8, 2023

Choose a reason for hiding this comment

geyslan Aug 7, 2023

Choose a reason for hiding this comment

geyslan commented Aug 7, 2023 • edited Loading

javierhonduco commented Aug 1, 2023 •

edited

Loading

geyslan commented Aug 7, 2023 •

edited

Loading