aot: Enable aot built bpftrace programs to run on their own #3104

arthurshau · 2024-04-06T00:26:19Z

The first commit fixes a couple issues with aot built binaries that was preventing runtime from functioning.

We now look in the right place in the ELF file for the custom .btaot section that contains the metadata + generated ELF, that we then restore to create BpfBytecode. Second, we now also serialize the map of BpfMaps owned by BpfBytecode into the bytecode as well, which is necessary for runtime. In order to do this, I needed to add a default constructor to BpfMap, as well as a hacky solution to be able to serialize the name of the map which is a cstring_view (which may or may not have defeated the purpose of having the cstring_view in the first place - would appreciate a review of this). Finally, I fixed maps not being printed out correctly when bpftrace exits.

The second commit allows running aot binaries directly without calling having to call runtime shim (e.g. you can now just call ./a.btaot instead of bpftrace-aotrt a.btaot. This is possible because the aot binaries already contain all the info necessary to run themselves.

Address part 1 of #2918.

Example:

$ ./bpftrace -e 'profile:hz:599 { @[tid] = count(); exit(); }' --aot a.btaot
$ ./a.btaot
Attaching 1 probe...


@[679916]: 1
@[12686]: 1
@[680061]: 1
@[0]: 4

Checklist

Language changes are updated in man/adoc/bpftrace.adoc
User-visible and non-trivial changes updated in CHANGELOG.md
The new behaviour is covered by tests

src/bpfmap.cpp

danobi

overall looks reasonable. i'm assuming this involved some nice debugging work :)

src/aot/aot.cpp

src/bpfmap.h

src/aot/aot_main.cpp

src/container/cstring_view.h

src/aot/aot.cpp

arthurshau · 2024-04-10T20:02:36Z

Addressed comments, thanks!

src/aot/aot.cpp

jordalgo · 2024-04-11T15:06:55Z

src/aot/aot.cpp

+  Elf_Data *data = NULL;
+  uint8_t *btaot_section = NULL;
+  const Header *hdr;
+  int i = 0;


Nit: can we move this closer to the while loop?

Yeah, I put it here (along with the others) because of the goto statements bypassing initialization errors. But I can split the initialization onto a second line to make it work.

src/aot/aot.cpp

src/aot/aot_main.cpp

arthurshau · 2024-04-12T01:53:47Z

Address comments

src/aot/aot_main.cpp

src/aot/aot.h

src/aot/aot_main.cpp

src/bpfmap.h

ajor

Agreed that we can't store a cstring_view in the serialised data now - just got a couple of other places that we can still reduce string copying in

src/bpfmap.h

ajor

Looking pretty good to me, just a couple of minor points.

What's the testing situation looking like for AOT? It'd be good to have it exercised by the runtime test suite to ensure it doesn't break again

src/run_bpftrace.h

src/run_bpftrace.cpp

arthurshau · 2024-04-24T17:15:42Z

Address comments and rebase.

What's the testing situation looking like for AOT? It'd be good to have it exercised by the runtime test suite to ensure it doesn't break again

The immediate next order of business to enable this PoC will be to get runtime-tests.sh --run-aot-tests to completely pass. As noted in #2918, this may require some work in portability_analyser.cpp to determine which features are/are not supported for AOT at the moment.

ajor

Nice!

arthurshau · 2024-04-26T21:29:38Z

Maybe also for a follow-up diff but the generated binary still seems rather large

How do we cut down on the file size?

Yeah, one of the items on the list in #2918 is to re-remove the large list of runtime deps from the shim, which should hopefully help a lot.

Are we going to ship bpftrace-aotrt with the normal bpftrace binary for all distros?

I believe bpftrace-aotrt is already getting shipped? At least, I found it in my /usr/bin/ alongside my normal bpftrace install. Currently, the code searches for bpftrace-aotrt in $PATH to copy the shim, so it will need to exist somewhere.

General question: With this PR, is this feature ready enough to add to the --help output?

At what point do we add documentation?

Probably not ready to add to --help yet, ideally we should get runtime-tests.sh --run-aot-tests to pass first I think.

src/aot/aot.cpp

viktormalik

First of all, sorry for the delayed review! I really like this work and hope that we'll be able to get it in soon. With that, I apologize if the following will cause any delays but I think that it's an important thing to mention.

I think that there's a major flaw in the way we serialize bytecode at the moment. The current workflow is:

codegen -> ELF -> BpfBytecode -> serialization to .btaot
deserialization from .btaot -> BpfBytecode -> load BPF programs

With the upcoming move to libbpf, this will not let us leverage the full power of libbpf which is able to deduce a lot of information about the BPF maps and programs from the ELF. I think that the correct workflow should be:

codegen -> ELF -> serialization to .btaot
deserialization from .btaot -> ELF -> BpfBytecode -> load BPF programs

This is also the reason why you had to serialize BpfMap which I specifically tried to avoid when I was implementing it. Ideally, BpfMap instances should be created by running libbpf parsing on the ELF produced by bpftrace.

In addition, I plan to drop BpfBytecode.sections_ completely and use libbpf's struct bpf_object instead (which is created from the ELF). When that happens, the current AOT approach would stop working and we'd have to move to the workflow suggested above anyways.

So, I suggest we move to serializing the produced ELF instead of BpfBytecode before merging this PR.

arthurshau · 2024-04-29T20:19:34Z

Hi Viktor, thanks for the comment. You make good points, and I forgot to consider your work moving bpftrace to use libbpf when opening this PR. Given what you said about removing BpfBytecode.sections_, I agree that it makes sense to switch the AOT workflow now so we can leverage libbpf. I'll look into it.

viktormalik · 2024-04-30T06:29:53Z

Hi Viktor, thanks for the comment. You make good points, and I forgot to consider your work moving bpftrace to use libbpf when opening this PR. Given what you said about removing BpfBytecode.sections_, I agree that it makes sense to switch the AOT workflow now so we can leverage libbpf. I'll look into it.

Thanks @arthurshau! I think that it could be as simple as serializing the ELF produced by codegen (as raw binary data) and moving creation of BpfBytecode to run_bpftrace.

arthurshau · 2024-05-21T02:35:29Z

@viktormalik Sorry for the delay, I've implemented the requested changes. Can you double check that the serialization portion in particular is done correctly? I basically just call emit and then append the vector to the end after the AOT header and RequiredResources.

Also, when running the generated binary, I'm now getting these libbpf warnings for some reason:

libbpf: elf: skipping unrecognized data section(15) .eh_frame
libbpf: elf: skipping relo section(16) .rel.eh_frame for section(15) .eh_frame

This happens when calling elf::parseBpfBytecodeFromElfObject. Might you know why?

ajor · 2024-05-21T13:57:11Z

Just had a quick skim, but we should try and keep the ELF file in memory until the actual serialisation step.

CodegenLLVM::emit_elf is just a tool for debugging bpftrace. Take a look at CodegenLLVM::emit to see how we can operate on the ELF file in memory instead of writing it to disk.

src/main.cpp

viktormalik

Just a couple of small remarks, otherwise looks great. Thanks for this work!

src/run_bpftrace.h

src/aot/aot.cpp

arthurshau · 2024-05-22T20:30:03Z

Addressed comments, thanks! Also moved the libbpf_print and libbpf_print_level_string functions from main.cpp to the shared run_bpftrace.cpp because aot_main.cpp needs them as well - it wasn't properly setting the log callback function for libbpf warnings.

Fixes a couple issues with aot built binaries that was preventing runtime from functioning. Namely, we were looking in the wrong place in the ELF file for the custom .btaot section that we added. We were also not serializing the map of BpfMaps contained in BpfBytecode, which is necessary for runtime. Finally, I fixed maps not being printed out correctly when bpftrace exits.

Currently, even though we make a copy of the runtime shim and then embed the necessary metadata + bytecode into the binary, you still need to pass that binary as an argument to the runtime shim in order to run it (i.e. you need to do 'bpftrace-aotrt <file_name>'). Since the binary can fully function on its own, we don't need to do this anymore. This commit makes the executable just look inside itself for the metadata and bytecode, skipping the need to call bpftrace-aotrt. (i.e. you can now run the binary with just ./<file_name>.)

viktormalik

LGTM, I'm just not approving the PR, yet, as I'd like to take some time trying this out.

viktormalik · 2024-05-23T07:13:20Z

Also moved the libbpf_print and libbpf_print_level_string functions from main.cpp to the shared run_bpftrace.cpp because aot_main.cpp needs them as well - it wasn't properly setting the log callback function for libbpf warnings.

Yeah, makes sense.

@viktormalik Sorry for the delay, I've implemented the requested changes. Can you double check that the serialization portion in particular is done correctly? I basically just call emit and then append the vector to the end after the AOT header and RequiredResources.

Yes, that should be correct.

Also, when running the generated binary, I'm now getting these libbpf warnings for some reason:
libbpf: elf: skipping unrecognized data section(15) .eh_frame
libbpf: elf: skipping relo section(16) .rel.eh_frame for section(15) .eh_frame
This happens when calling elf::parseBpfBytecodeFromElfObject. Might you know why?

I'm not sure why this is happening but I'm seeing this when running in non-AOT mode, too. It's certainly something that we should address but it shouldn't be caused by this PR.

danobi · 2024-05-23T20:29:14Z

🚀

viktormalik

Basic probes work nice, some others don't but since this is still undocumented, I'm ok with merging this as-is and fixing things in the follow-up work.

My remarks:

Using --aot searches $PATH for bpftrace-aotrt which is not very convenient when doing development. Maybe we could make it also (recursively) search $PWD (or the directory of the bpftrace binary) so that developers don't have to update their $PATH manually?
BTF-based probes (e.g. kfunc) currently do not work b/c the AOT main doesn't run bpftrace.parse_btf(). Here, it'll be tricky to support kernel modules as in normal mode, we parse the probes to determine which kernel modules BTF needs to be parsed. Since we don't have that information in pre-compiled binary, we'll have to figure out a different way to get the list of modules that the program may attach to.
Since this will likely get merged shortly before we do the new release, I just want to make sure that we agree that we're leaving this undocumented for now.

arthurshau · 2024-05-24T09:14:00Z

Since this will likely get merged shortly before we do the new release, I just want to make sure that we agree that we're leaving this undocumented for now.

Sounds good, I agree with that.

danobi · 2024-05-24T15:59:03Z

Using --aot searches $PATH for bpftrace-aotrt which is not very convenient when doing development. Maybe we could make it also (recursively) search $PWD (or the directory of the bpftrace binary) so that developers don't have to update their $PATH manually?

Yeah, need to find a better way.

Since this will likely get merged shortly before we do the new release, I just want to make sure that we agree that we're leaving this undocumented for now.

Agreed. We don't wanna document / announce anything until at least #2918 is completed.

arthurshau force-pushed the aot branch 2 times, most recently from 80c876d to c5aa371 Compare April 6, 2024 00:56

github-advanced-security bot found potential problems Apr 6, 2024

View reviewed changes

src/bpfmap.cpp Fixed Show fixed Hide fixed

arthurshau changed the title ~~aot: Enable aot built binaries to run on their own~~ aot: Enable aot built bpftrace programs to run on their own Apr 6, 2024

arthurshau force-pushed the aot branch from c5aa371 to cc1dcaa Compare April 7, 2024 00:08

danobi reviewed Apr 9, 2024

View reviewed changes

ajor reviewed Apr 9, 2024

View reviewed changes

src/container/cstring_view.h Outdated Show resolved Hide resolved

arthurshau force-pushed the aot branch from cc1dcaa to 8b1b28a Compare April 10, 2024 19:22

github-advanced-security bot found potential problems Apr 10, 2024

View reviewed changes

src/aot/aot.cpp Fixed Show fixed Hide fixed

arthurshau force-pushed the aot branch 2 times, most recently from 1b30ca2 to 2f79f41 Compare April 10, 2024 19:53

arthurshau marked this pull request as ready for review April 10, 2024 20:02

arthurshau requested review from viktormalik and fbs as code owners April 10, 2024 20:02

jordalgo reviewed Apr 11, 2024

View reviewed changes

src/aot/aot.cpp Outdated Show resolved Hide resolved

jordalgo reviewed Apr 11, 2024

View reviewed changes

src/aot/aot.cpp Outdated Show resolved Hide resolved

jordalgo reviewed Apr 11, 2024

View reviewed changes

src/aot/aot_main.cpp Outdated Show resolved Hide resolved

arthurshau force-pushed the aot branch from 2f79f41 to 24d77e0 Compare April 12, 2024 01:53

ajor reviewed Apr 12, 2024

View reviewed changes

src/aot/aot_main.cpp Outdated Show resolved Hide resolved

src/aot/aot.h Outdated Show resolved Hide resolved

src/aot/aot_main.cpp Outdated Show resolved Hide resolved

src/bpfmap.h Outdated Show resolved Hide resolved

ajor reviewed Apr 12, 2024

View reviewed changes

src/bpfmap.h Outdated Show resolved Hide resolved

src/bpfmap.h Outdated Show resolved Hide resolved

arthurshau force-pushed the aot branch 3 times, most recently from 05ba971 to fbad3cf Compare April 12, 2024 23:10

ajor reviewed Apr 24, 2024

View reviewed changes

src/run_bpftrace.h Outdated Show resolved Hide resolved

src/run_bpftrace.cpp Outdated Show resolved Hide resolved

arthurshau force-pushed the aot branch from fbad3cf to 973846c Compare April 24, 2024 17:12

ajor approved these changes Apr 25, 2024

View reviewed changes

arthurshau force-pushed the aot branch from d112086 to d2b4653 Compare April 26, 2024 21:21

danobi approved these changes Apr 27, 2024

View reviewed changes

jordalgo approved these changes Apr 29, 2024

View reviewed changes

viktormalik reviewed Apr 29, 2024

View reviewed changes

src/aot/aot.cpp Outdated Show resolved Hide resolved

viktormalik requested changes Apr 29, 2024

View reviewed changes

arthurshau force-pushed the aot branch 3 times, most recently from 3d86e50 to afa9814 Compare May 21, 2024 02:30

arthurshau requested a review from viktormalik May 21, 2024 06:55

arthurshau force-pushed the aot branch 2 times, most recently from 53eee74 to 4c198fe Compare May 21, 2024 21:33

github-advanced-security bot found potential problems May 21, 2024

View reviewed changes

src/main.cpp Fixed Show fixed Hide fixed

src/main.cpp Fixed Show fixed Hide fixed

arthurshau force-pushed the aot branch from 4c198fe to 7f9595b Compare May 21, 2024 22:17

viktormalik reviewed May 22, 2024

View reviewed changes

src/run_bpftrace.h Outdated Show resolved Hide resolved

src/aot/aot.cpp Show resolved Hide resolved

src/aot/aot.cpp Outdated Show resolved Hide resolved

src/aot/aot.cpp Outdated Show resolved Hide resolved

arthurshau force-pushed the aot branch from 7f9595b to 1300bac Compare May 22, 2024 20:25

arthurshau added 2 commits May 22, 2024 13:38

arthurshau force-pushed the aot branch from 1300bac to 318d4bd Compare May 22, 2024 20:39

viktormalik reviewed May 23, 2024

View reviewed changes

viktormalik approved these changes May 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

aot: Enable aot built bpftrace programs to run on their own #3104

aot: Enable aot built bpftrace programs to run on their own #3104

arthurshau commented Apr 6, 2024 •

edited

danobi left a comment

arthurshau commented Apr 10, 2024

jordalgo Apr 11, 2024 •

edited

arthurshau Apr 12, 2024

arthurshau commented Apr 12, 2024

ajor left a comment

ajor left a comment

arthurshau commented Apr 24, 2024

ajor left a comment

arthurshau commented Apr 26, 2024

viktormalik left a comment

arthurshau commented Apr 29, 2024

viktormalik commented Apr 30, 2024

arthurshau commented May 21, 2024 •

edited

ajor commented May 21, 2024

viktormalik left a comment

arthurshau commented May 22, 2024

viktormalik left a comment

viktormalik commented May 23, 2024

danobi commented May 23, 2024

viktormalik left a comment

arthurshau commented May 24, 2024

danobi commented May 24, 2024

aot: Enable aot built bpftrace programs to run on their own #3104

Are you sure you want to change the base?

aot: Enable aot built bpftrace programs to run on their own #3104

Conversation

arthurshau commented Apr 6, 2024 • edited

Checklist

danobi left a comment

Choose a reason for hiding this comment

arthurshau commented Apr 10, 2024

jordalgo Apr 11, 2024 • edited

Choose a reason for hiding this comment

arthurshau Apr 12, 2024

Choose a reason for hiding this comment

arthurshau commented Apr 12, 2024

ajor left a comment

Choose a reason for hiding this comment

ajor left a comment

Choose a reason for hiding this comment

arthurshau commented Apr 24, 2024

ajor left a comment

Choose a reason for hiding this comment

arthurshau commented Apr 26, 2024

viktormalik left a comment

Choose a reason for hiding this comment

arthurshau commented Apr 29, 2024

viktormalik commented Apr 30, 2024

arthurshau commented May 21, 2024 • edited

ajor commented May 21, 2024

viktormalik left a comment

Choose a reason for hiding this comment

arthurshau commented May 22, 2024

viktormalik left a comment

Choose a reason for hiding this comment

viktormalik commented May 23, 2024

danobi commented May 23, 2024

viktormalik left a comment

Choose a reason for hiding this comment

arthurshau commented May 24, 2024

danobi commented May 24, 2024

arthurshau commented Apr 6, 2024 •

edited

jordalgo Apr 11, 2024 •

edited

arthurshau commented May 21, 2024 •

edited