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

info: expose more prog jited info #1598

Merged
merged 5 commits into from
Dec 4, 2024
Merged

Conversation

Asphaltt
Copy link
Contributor

@Asphaltt Asphaltt commented Oct 24, 2024

For pwru cilium/pwru#451, I'm going to inspect some inner details of bpf progs with LBR. But it requires the JITed info of bpf prog.

Then, expose more prog jited info:

  1. JITed machine native instructions torvalds/linux@1e2709769086 ("bpf: Add BPF_OBJ_GET_INFO_BY_FD")
  2. JITed line info torvalds/linux@c454a46b5efd ("bpf: Add bpf_line_info support")
  3. JITed image lengths torvalds/linux@815581c11cc2 ("bpf: get JITed image lengths of functions via syscall")

@Asphaltt Asphaltt requested review from dylandreimerink and a team as code owners October 24, 2024 12:20
info.go Outdated Show resolved Hide resolved
info.go Outdated Show resolved Hide resolved
info.go Outdated Show resolved Hide resolved
@Asphaltt Asphaltt force-pushed the features/dump-jited branch from 8c0466b to d5a4c9e Compare November 2, 2024 05:24
Copy link
Collaborator

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

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

Thanks, sorry this took so long. Looks good overall, I've suggested a few changes. Please do any refactoring in a separate commit from the main change (adding the jited fields).

info.go Outdated Show resolved Hide resolved
btf/ext_info.go Outdated Show resolved Hide resolved
It's to export the raw line infos alongside with prog's jited infos.

Signed-off-by: Leon Hwang <hffilwlqm@gmail.com>
Expose these prog jited info:

1. JITed machine native instructions torvalds/linux@1e2709769086 ("bpf: Add BPF_OBJ_GET_INFO_BY_FD")
2. JITed line info torvalds/linux@c454a46b5efd ("bpf: Add bpf_line_info support")
3. JITed image lengths torvalds/linux@815581c11cc2 ("bpf: get JITed image lengths of functions via syscall")

Signed-off-by: Leon Hwang <hffilwlqm@gmail.com>
@Asphaltt Asphaltt force-pushed the features/dump-jited branch from d5a4c9e to a34ec46 Compare November 25, 2024 13:07
@github-actions github-actions bot added the breaking-change Changes exported API label Nov 25, 2024
@Asphaltt Asphaltt requested a review from ti-mo December 3, 2024 08:12
@ti-mo ti-mo force-pushed the features/dump-jited branch from f35b146 to 882863e Compare December 4, 2024 09:54
ti-mo added 2 commits December 4, 2024 12:23
Since the original field is called jited_ksyms, reflect that in the Go API as well.

Signed-off-by: Timo Beckers <timo@isovalent.com>
This was getting hard to follow with the 'name == "proc"' statements sprinkled
around. These are actually separate tests, proc only populates type and tag.

Remove negative tests from TestProgramInfoProc as newProgramInfoFromProc only
sets those two fields.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo force-pushed the features/dump-jited branch from 882863e to f417957 Compare December 4, 2024 12:15
Added tests for:
- FuncInfos()
- LineInfos()
- JitedKsymAddrs()
- JitedInsns()
- JitedLineInfos()
- JitedFuncLens()

The existing test for JitedKsymAddrs was removed because it's not strictly
necessary to load a CollectionSpec to exercise it. It obscured the fact that
it seems like kernels before at least 5.4 need to have a subprog for jited
addrs to be returned in prog info.

TestProgInfoFuncInfos was removed for a similar reason. It's possible to
craft a program using the asm package that meets the criteria for exercising
FuncInfos() and LineInfos() in a deterministic way, no need to load a Collection.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo force-pushed the features/dump-jited branch from f417957 to 7514cdf Compare December 4, 2024 12:42
@ti-mo
Copy link
Collaborator

ti-mo commented Dec 4, 2024

@Asphaltt I added some tests for the new proginfo APIs, please include them next time. I also ended up renaming KsymAddrs to JitedKsymAddrs since that makes it clear that it concerns jited symbols and looks nicer alongside the other methods. This will break your build if you're importing unreleased main into your app.

Also moved around some test-related things.

@ti-mo ti-mo merged commit 9895aae into cilium:main Dec 4, 2024
17 checks passed
@Asphaltt
Copy link
Contributor Author

Asphaltt commented Dec 4, 2024

I added some tests for the new proginfo APIs

Thank you very much.

please include them next time

Got it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Changes exported API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants