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

Add a tracepoint example and a percpu example #402

Merged
merged 3 commits into from
Sep 14, 2021
Merged

Conversation

hao-lee
Copy link
Contributor

@hao-lee hao-lee commented Sep 2, 2021

No description provided.

@hao-lee hao-lee changed the title Add a tracepoint example and some other related stuff Add a tracepoint example and a percpu example Sep 2, 2021
Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

Is adding a full example to demonstrate per-CPU maps the best way to go? There is a lot of code here that has nothing to do with per-CPU maps which the user has to ignore.

There is already an example of per CPU map usage here: https://pkg.go.dev/github.com/cilium/ebpf#example-Map-PerCPU How can we make that more discoverable? Maybe you saw it but it didn't help?

@@ -0,0 +1,3 @@
*.o
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we gitignore the artefacts it's not possible to go install or go run the examples, which I think is kind of neat.

Copy link
Contributor Author

@hao-lee hao-lee Sep 6, 2021

Choose a reason for hiding this comment

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

Thanks for pointing out this!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lmb Hi, after thinking carefully, I still think we should ignore all these intermediate files and instruct users on how to compile and build step by step. After all, go run -exec sudo hides many details.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lmb hello 😃

@@ -12,3 +12,12 @@
cd ebpf/examples/
go run -exec sudo [./kprobe, ./uretprobe, ./tracepoint, ...]
```

## How to compile
Copy link
Collaborator

Choose a reason for hiding this comment

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

This kind of duplicates the section above, is it necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I will merge them.

// +build linux

// This program demonstrates attaching an eBPF program to a kernel symbol.
// The eBPF program will be attached to the start of the sys_execve
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mention the use of a per CPU map instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😅 I overlooked these lines.

@hao-lee
Copy link
Contributor Author

hao-lee commented Sep 6, 2021

There is already an example of per CPU map usage here: https://pkg.go.dev/github.com/cilium/ebpf#example-Map-PerCPU How can we make that more discoverable? Maybe you saw it but it didn't help?

Yes. This example uses pure Go to manipulate the percpu map, but I think a C example is also necessary for users who want to put the main logic in C files.

@hao-lee hao-lee force-pushed the develop branch 4 times, most recently from 849de67 to 9a36214 Compare September 13, 2021 07:55
Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

Some small stuff, but otherwise I think this is good to merge. The percpu example is more verbose than I'd like, but we can fix that in a follow up.

examples/kprobe_percpu/main.go Outdated Show resolved Hide resolved
examples/tracepoint_in_c/bpf/handler.c Outdated Show resolved Hide resolved
examples/kprobe_percpu/main.go Outdated Show resolved Hide resolved
@lmb lmb force-pushed the develop branch 3 times, most recently from d7077eb to 9dde01c Compare September 14, 2021 09:34
This name should be more distinguishable.

Signed-off-by: Hao Lee <haolee@didiglobal.com>
This example uses C language to implement the handler, which should be more
straightforward than the existing pure-go implementation.

Signed-off-by: Hao Lee <haolee@didiglobal.com>
Signed-off-by: Hao Lee <haolee@didiglobal.com>
@lmb lmb merged commit 6f5c21e into cilium:master Sep 14, 2021
@hao-lee
Copy link
Contributor Author

hao-lee commented Sep 14, 2021

😅 Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants