tetragon: get rid of some programs #1783

olsajiri · 2023-11-22T11:24:30Z

There's no need for having separate programs for generic_kprobe_filter_arg* and
generic_kprobe_process_event* function, we can have just one instance called
recursively by tail jump.

b4:

# bpftool prog | grep gener | wc -l
14
# bpftool prog | grep gener 
32411: kprobe  name generic_kprobe_process_event3  tag 75d7c429743eb535  gpl
32412: kprobe  name generic_kprobe_filter_arg1  tag 9279987579ba117c  gpl
32413: kprobe  name generic_kprobe_process_event2  tag bf3fbe433d43e592  gpl
32414: kprobe  name generic_kprobe_output  tag 9e772542ab44369a  gpl
32415: kprobe  name generic_kprobe_filter_arg3  tag dbdb3573c7575198  gpl
32416: kprobe  name generic_kprobe_filter_arg4  tag c23b95feefd79e4b  gpl
32417: kprobe  name generic_kprobe_actions  tag dc8cb05c20800b5e  gpl
32418: kprobe  name generic_kprobe_process_filter  tag e003f29c78cce2e5  gpl
32419: kprobe  name generic_kprobe_filter_arg5  tag 300d8c52de7283fc  gpl
32420: kprobe  name generic_kprobe_process_event1  tag 4edd789b7e6c92bc  gpl
32421: kprobe  name generic_kprobe_process_event0  tag 3e6a42c39a871a4e  gpl
32422: kprobe  name generic_kprobe_process_event4  tag f6488e866179c36e  gpl
32423: kprobe  name generic_kprobe_event  tag 7d4eae34b0f0fead  gpl
32424: kprobe  name generic_kprobe_filter_arg2  tag 7b198f43675e7f7e  gpl

after:

# bpftool prog | grep gener
32388: kprobe  name generic_kprobe_setup_event  tag d680ace8a020dc3c  gpl
32389: kprobe  name generic_kprobe_process_event  tag 77be0f878db0213e  gpl
32390: kprobe  name generic_kprobe_actions  tag d1a085e4d7462bad  gpl
32391: kprobe  name generic_kprobe_output  tag 9e772542ab44369a  gpl
32392: kprobe  name generic_kprobe_event  tag fc0939c83ba8b9a7  gpl
32393: kprobe  name generic_kprobe_process_filter  tag 88d09489695c97d1  gpl
32394: kprobe  name generic_kprobe_filter_arg  tag 2b1a7047e1a02adb  gpl
# bpftool prog | grep gener | wc -l
7

netlify · 2023-11-22T11:25:08Z

✅ Deploy Preview for tetragon ready!

Name	Link
🔨 Latest commit	`8f15a30`
🔍 Latest deploy log	https://app.netlify.com/sites/tetragon/deploys/6564d6389d00430008f80c66
😎 Deploy Preview	https://deploy-preview-1783--tetragon.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

tpapagian

Nice cleanup!

bpf/process/generic_calls.h

tixxdz

Great work! LGTM , updating the comment would be perfect

tixxdz · 2023-11-24T23:39:24Z

bpf/process/bpf_generic_kprobe.c

@@ -229,51 +230,15 @@ generic_kprobe_process_filter(void *ctx)
 // see also: MIN_FILTER_TAILCALL, MAX_FILTER_TAILCALL


Jiri I would just remove the comment above about kprobe indexes or update it... also probably same for MIN_FILTER_TAILCALL or remove it.

ah, right, will remove that

tixxdz · 2023-11-24T23:44:15Z

bpf/process/types/basic.h

-			tail_call(ctx, tailcalls, MIN_FILTER_TAILCALL + index);
+		if (index <= MAX_SELECTORS && e->sel.active[index & 7]) {
+			e->index = index;
+			tail_call(ctx, tailcalls, MIN_FILTER_TAILCALL);


Another thing it would be readable for new comers if the programs are identified by constant during the tail_call(), or an enum with a max as _MAX_TG_TAIL_CALL_INDEX. Please note no strong opinion, just to make tail_call readable ;-)

good idea, enum values should be better I think

kkourt

Thanks for the cleanup!
LGTM, but I have some suggestions.

bpf/lib/generic.h

bpf/process/types/basic.h

bpf/process/generic_calls.h

The pass in msg_selector_data is treated as bool value, make it one then. Signed-off-by: Jiri Olsa <jolsa@kernel.org>

There's no need to keep separate kprobe programs for each filter_read_argX call, we can just tail call into single program with increased index. The number of loaded programs gets reduced, so we need to fix TestLoad* tests as well. Signed-off-by: Jiri Olsa <jolsa@kernel.org>

There's no need to keep separate kprobe programs for each generic_kprobe_process_event4X call, we can just tail call into single program with increased index. The number of loaded programs gets reduced, so we need to fix TestLoad* tests as well. I had to add several &= masks to make it verifiable under 5.4, can't find better solution yet. Signed-off-by: Jiri Olsa <jolsa@kernel.org>

Adding TAIL_CALL_* enum values to identify tail calls for generic probes and make it more human friendly. Suggested-by: Djalal Harouni <tixxdz@gmail.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

olsajiri added the release-note/minor This PR introduces a minor user-visible change label Nov 22, 2023

olsajiri force-pushed the pr/olsajiri/footprint branch 7 times, most recently from 2e9021b to 93512be Compare November 23, 2023 15:17

olsajiri marked this pull request as ready for review November 23, 2023 18:09

olsajiri requested a review from a team as a code owner November 23, 2023 18:09

olsajiri requested review from tixxdz, kkourt and tpapagian November 23, 2023 18:09

olsajiri marked this pull request as draft November 23, 2023 18:21

olsajiri force-pushed the pr/olsajiri/footprint branch 2 times, most recently from 8af5ffc to c20789f Compare November 24, 2023 09:15

olsajiri changed the title ~~Pr/olsajiri/footprint~~ tetragon: get rid of some programs Nov 24, 2023

olsajiri marked this pull request as ready for review November 24, 2023 09:57

tpapagian approved these changes Nov 24, 2023

View reviewed changes

bpf/process/generic_calls.h Outdated Show resolved Hide resolved

tixxdz approved these changes Nov 24, 2023

View reviewed changes

olsajiri force-pushed the pr/olsajiri/footprint branch from c20789f to 774d6d5 Compare November 25, 2023 20:38

kkourt reviewed Nov 27, 2023

View reviewed changes

bpf/lib/generic.h Outdated Show resolved Hide resolved

bpf/process/types/basic.h Outdated Show resolved Hide resolved

olsajiri force-pushed the pr/olsajiri/footprint branch from 774d6d5 to de76d92 Compare November 27, 2023 09:14

kkourt reviewed Nov 27, 2023

View reviewed changes

bpf/process/generic_calls.h Show resolved Hide resolved

olsajiri added 4 commits November 27, 2023 14:19

tetragon: Make pass in msg_selector_data bool

8b26cd5

The pass in msg_selector_data is treated as bool value, make it one then. Signed-off-by: Jiri Olsa <jolsa@kernel.org>

tetragon: Add TAIL_CALL_* enums for generic tail calls

8f15a30

Adding TAIL_CALL_* enum values to identify tail calls for generic probes and make it more human friendly. Suggested-by: Djalal Harouni <tixxdz@gmail.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

olsajiri force-pushed the pr/olsajiri/footprint branch from de76d92 to 8f15a30 Compare November 27, 2023 17:47

kkourt approved these changes Nov 28, 2023

View reviewed changes

kkourt merged commit 405f534 into main Nov 28, 2023
64 of 65 checks passed

kkourt deleted the pr/olsajiri/footprint branch November 28, 2023 09:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tetragon: get rid of some programs #1783

tetragon: get rid of some programs #1783

olsajiri commented Nov 22, 2023 •

edited

Loading

netlify bot commented Nov 22, 2023 •

edited

Loading

tpapagian left a comment

tixxdz left a comment

tixxdz Nov 24, 2023

olsajiri Nov 25, 2023

tixxdz Nov 24, 2023

olsajiri Nov 25, 2023

kkourt left a comment

		@@ -229,51 +230,15 @@ generic_kprobe_process_filter(void *ctx)
		// see also: MIN_FILTER_TAILCALL, MAX_FILTER_TAILCALL

tetragon: get rid of some programs #1783

tetragon: get rid of some programs #1783

Conversation

olsajiri commented Nov 22, 2023 • edited Loading

netlify bot commented Nov 22, 2023 • edited Loading

✅ Deploy Preview for tetragon ready!

tpapagian left a comment

Choose a reason for hiding this comment

tixxdz left a comment

Choose a reason for hiding this comment

tixxdz Nov 24, 2023

Choose a reason for hiding this comment

olsajiri Nov 25, 2023

Choose a reason for hiding this comment

tixxdz Nov 24, 2023

Choose a reason for hiding this comment

olsajiri Nov 25, 2023

Choose a reason for hiding this comment

kkourt left a comment

Choose a reason for hiding this comment

olsajiri commented Nov 22, 2023 •

edited

Loading

netlify bot commented Nov 22, 2023 •

edited

Loading