Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tracingpolicy: add BPF operations support #2943

Merged
merged 5 commits into from
Sep 30, 2024

Conversation

tixxdz
Copy link
Member

@tixxdz tixxdz commented Sep 20, 2024

Description

Add BPF operations support.

Changelog

tracingpolicy: add BPF operations support 

@tixxdz tixxdz requested a review from a team as a code owner September 20, 2024 10:32
@tixxdz tixxdz marked this pull request as draft September 20, 2024 10:32
Copy link

netlify bot commented Sep 20, 2024

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 6c4bd3d
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/66f3ea15f1b21200087c82c3
😎 Deploy Preview https://deploy-preview-2943--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@tixxdz tixxdz force-pushed the pr/tixxdz/2024-09-api-bpf-kprobes branch 2 times, most recently from 152045b to 1c50078 Compare September 22, 2024 16:38
@tixxdz tixxdz added the release-note/minor This PR introduces a minor user-visible change label Sep 22, 2024
@tixxdz tixxdz force-pushed the pr/tixxdz/2024-09-api-bpf-kprobes branch from 1c50078 to d72fdf0 Compare September 22, 2024 16:41
@tixxdz tixxdz marked this pull request as ready for review September 22, 2024 16:41
@tixxdz tixxdz requested a review from kaworu September 22, 2024 21:16
@olsajiri
Copy link
Contributor

so IIUC you want the event to show the bpf command numbers ... IMO the types should be just for kernel objects
not for int values interpretation

could we do that without changing the ebpf code? we just need to get the int value from kernel (which we already have support for, no need to add new) and transform that number into bpf command enum value

I think we could factor the printer code to do that.. to have kernel type and user side type.. so you'd configure the rinter and argument to have int type value for kernel code and 'bpf_cmd' type for user space processing code

Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
@tixxdz tixxdz force-pushed the pr/tixxdz/2024-09-api-bpf-kprobes branch from d72fdf0 to 6c4bd3d Compare September 25, 2024 10:46
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
@tixxdz tixxdz force-pushed the pr/tixxdz/2024-09-api-bpf-kprobes branch from 6c4bd3d to 944c6e5 Compare September 25, 2024 10:49
@tixxdz
Copy link
Member Author

tixxdz commented Sep 25, 2024

so IIUC you want the event to show the bpf command numbers ... IMO the types should be just for kernel objects not for int values interpretation

could we do that without changing the ebpf code? we just need to get the int value from kernel (which we already have support for, no need to add new) and transform that number into bpf command enum value

I think we could factor the printer code to do that.. to have kernel type and user side type.. so you'd configure the rinter and argument to have int type value for kernel code and 'bpf_cmd' type for user space processing code

Yes makes sense, can you please check again Jiri, thank you!

Suggested by Jiri.

Introduce userspace types that will be used for pretty printing.
This allows to add different enum types into proto definition, have
a high level representation that allows to pretty print data to users
without propagating those same types into bpf part.

We define the bpfCmd enum in proto, add its type in userspace, but
we do not propagate this into kernel, we just keep using int types for
kernel and we do the translation back into userspace where it makes
sense.

Suggested-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
@tixxdz tixxdz force-pushed the pr/tixxdz/2024-09-api-bpf-kprobes branch from 944c6e5 to f1a4175 Compare September 25, 2024 11:08
Copy link
Member

@kaworu kaworu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protobuf changes LGTM

@kkourt kkourt self-requested a review September 25, 2024 13:10
Copy link
Contributor

@kkourt kkourt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM.

@@ -6970,3 +6970,67 @@ tetragon_missed_prog_probes_total{attach="wake_up_new_task",policy="__base__"} 0
prometheus.BuildFQName(consts.MetricsNamespace, "", "missed_prog_probes_total")))

}

func TestKprobeBpfCmd(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess the idea of the test is to detect the tetragon bpf program / map loading?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes kept it small, but it can be used to detect all bpf commands.

@tixxdz tixxdz merged commit 9915897 into main Sep 30, 2024
48 checks passed
@tixxdz tixxdz deleted the pr/tixxdz/2024-09-api-bpf-kprobes branch September 30, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR introduces a minor user-visible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants