-
Notifications
You must be signed in to change notification settings - Fork 24
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 support for symbolizing BPF kernel program addresses #854
Conversation
This PR will need more love to get CI green, but other than that it should be finished. @anakryiko can you take a look when you get a chance? |
9b39eac
to
388f207
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #854 +/- ##
==========================================
+ Coverage 94.76% 95.10% +0.33%
==========================================
Files 52 55 +3
Lines 9897 10479 +582
==========================================
+ Hits 9379 9966 +587
+ Misses 518 513 -5 ☔ View full report in Codecov by Sentry. |
6566de2
to
0040376
Compare
src/kernel/bpf/prog.rs
Outdated
format!("failed to retrieve BPF program information for program {prog_id}") | ||
})?; | ||
|
||
let mut line_records = HashMap::with_capacity(info.nr_jited_line_info as _); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you misinterpreting line records. They are ranges, not points. See https://github.com/torvalds/linux/blob/master/kernel/bpf/log.c#L332-L371 for how kernel queries it, it finds the closest match and then it's assumed that this line info applies to all instructions up to next line_info.
So, you don't need hashmap, it will be returned as an already sorted array, so you'll be doing a binary search (but not the exact match, closest match, one of those lower/upper bound kinds)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes more sense, will adjust. Allow me to question the "already sorted" aspect, though.
kernel reported JIT information is not sorted: [
0xffffffffc0058c38,
0xffffffffc0058c50,
0xffffffffc0058c69,
0xffffffffc0058c6e,
0xffffffffc0058c89,
0xffffffffc0058c95,
0xffffffffc0058c99,
0xffffffffc0058c9d,
0xffffffffc0058cb4,
0xffffffffc0058ccc,
0xffffffffc0058cd6,
0xffffffffc0058cea,
0xffffffffc0045a4c,
0xffffffffc0045a69,
0xffffffffc0045a70,
0xffffffffc0045a74,
0xffffffffc0045a7e,
0xffffffffc0045a82,
]
Add the infrastructure necessary for building BPF programs that we intend to use for testing purposes. Also, add a first test program that that will attach to the getpid system call and which provides the means for conveying the address of a function that is part of this program to user space, where it will eventually act as symbolization input. Signed-off-by: Daniel Müller <[email protected]>
Add infrastructure for loading and then triggering a BPF program from a test. This will form the basis for testing of BPF kernel program symbolization. Signed-off-by: Daniel Müller <[email protected]>
Rename the Ksym type to Kfunc. This will allow us to repurpose Ksym with upcoming changes. Signed-off-by: Daniel Müller <[email protected]>
Adjust the functionality for converting from Kfunc to ResolvedSym to be fallible and stop relying on the From trait for that purpose. We will eventually end up including a bit more information in the conversion and so let's instead roll with a dedicated conversion method on the Kfunc type itself. We need fallibility because this new method acts as an "abstract interface" for additional implementations coming in the future and which will be fallible. Signed-off-by: Daniel Müller <[email protected]>
Similar to what we did earlier for the conversion from Kfunc to ResolvedSym, make the conversion from Kfunc to SymInfo fallible. Signed-off-by: Daniel Müller <[email protected]>
Introduce the Ksym enumeration, which currently only has a single variant containing a Kfunc. With upcoming changes it will also provide support for BPF programs. Signed-off-by: Daniel Müller <[email protected]>
Add support for parsing BPF programs listed in kallsyms. Currently, while parsed specially, they don't provide more symbol information than regular kernel functions, but the BPF sub-system provides the means for retrieving symbol and even source code information about each. This will be tapped into subsequently. As a first step towards exposing this functionality, make sure to parse them as first-class entities. Signed-off-by: Daniel Müller <[email protected]>
Add low-level system call bindings for querying BPF program information from the kernel. We need this logic to iterate over available programs and for retrieving meta-data such as the program's tag. The alternative of relying on libbpf-rs and higher level primitives was discarded given the minimal nature of support we require here. There is simply no justification for all the dependency bloat that libbpf-sys et al pull in. Signed-off-by: Daniel Müller <[email protected]>
This change adds basic BTF support, as later required. Specifically, we add the means for opening BTF information via a system call, as well as the means for parsing a BTF blob and extracting strings at given offsets. Signed-off-by: Daniel Müller <[email protected]>
The kernel does not seem to allow us to query BPF program information given a BPF program tag. Rather, the workflow is to iterate over all BPF programs and find the one with a given tag this way. Because there could potentially be many programs active, this can be a costly operation that we may not want to execute all the time. To make the result of BPF program iteration reusable, this change introduces an easy to use cache for this data. Signed-off-by: Daniel Müller <[email protected]>
This change adds the remaining plumbing for symbolizing BPF program kernel addresses. When a kernel address falls into a BPF program, we query all the necessary information to see if the kernel is able to provide us with source code information about said address and furnish up the corresponding CodeInfo object to include it in the symbolization result. Closes: libbpf#826 Signed-off-by: Daniel Müller <[email protected]>
Enable necessary kernel options for us to run our BPF based testing infrastructure. Signed-off-by: Daniel Müller <[email protected]>
With the introduction of libbpf-rs as a dev-dependency we require additional system dependencies as part of various CI jobs. Install them. Signed-off-by: Daniel Müller <[email protected]>
0040376
to
acee209
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
skimmed through the logic, all looks good
Please see individual commits for descriptions.