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

Move from bcc to libbpf #2265

Merged
merged 12 commits into from
Jun 27, 2022
Merged

Conversation

viktormalik
Copy link
Contributor

@viktormalik viktormalik commented Jun 14, 2022

This features a number of changes related to using bcc and libbpf.

The main goal is to get rid of the situation where libbpf is linked (and used) twice, possibly in different versions:

bpftrace --> libbcc_bpf --> libbpf
bpftrace --> libbpf

which confuses the dynamic linker and causes bugs such as #2173.

This is now resolved by not using any bcc function which would further call libbpf. These include especially functions for program loading and map creation. Now, there are only two features used from bcc/libbpf.h:

  • probe attachment which shouldn't be calling anything from libbpf (except for kfunc, those are handled purely by libbpf)
  • definitions of BPF instructions (used in feature detection)

Besides these, bpftrace still uses bcc for:

  • USDT probes handling
  • ELF/symbols parsing
  • library path resolution

Since program loading now depends on libbpf, it is required, which resolves #1874.

This PR also cleans up vendoring of libbpf enums - vendored definitions are used everywhere in bpftrace.

There are some changes to the CI, too. Since the used Ubuntu 20.04 doesn't package libbpf, it is installed from source. Individual runs can specify libbpf version, most of them use the current latest (0.8.0) and there's one case added for an older version (0.5.0) to test some code paths.

Fixes #2173.

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

Necessary for building bpftrace on systems which do not have
BPF_TRACE_KPROBE_MULTI.
The goal is to move from bcc to libbpf as much as possible. This will
include BPF program loading, so libbpf will become required.

CI uses Ubuntu Focal (18.04) which doesn't package libbpf, so it must be
built from source.
Use bpf_map_create instead of bcc_create_map. No remarkable changes were
done as bcc does not do anything special that we would need.
BCC's load function doesn't do much extra work which bpftrace would
need with only some exceptions:
- It checks BPF program names for magic prefixes (describing the type of
  the attach point) and sets some additional options based on them.
  These options are now set directly in AttachedProbe::load_prog.
- When attaching to kfunc/kretfunc, it finds the BTF id of the attach
  point function. This is now done directly in AttachedProbe::load_prog.
  As we already have BTF located and parsed, this prevents some
  unnecessary work that BCC was doing.
- If the libbpf loading fails, it tries to reload the program with
  various tweaks. The most important one for bpftrace is reloading
  without the program name, if the name is rejected due to containing
  unsupported characters. These are now properly removed in
  AttachedProbe::load_prog, so we now have a better control over the
  loaded program name.
This mostly involves replacing bcc_prog_load by bpf_prog_load. For more
details on this replacement, see the previous commit.

bcc_prog_load is now not used anywhere, remove its detection from
CMakeLists.txt.
bcc's bpf_attach_kfunc simply calls bpf_raw_tracepoint_open, so it can
be called directly.

An advantage of this approach is that kfuncs are now completely handled
by libbpf. This requires some improvements of feature detection. Also,
test skipping is changed since kfunc support is now not checked on
compile time.
Default is the current latest (0.8.0)
Use vendored libbpf/bpf.h enum definitions everywhere in bpftrace.
Improve including of this header (do not undef __BPF_FUNC_MAPPER on each
include, instead undef it once inside the vendored header).

Also cleanup included headers related to libbpf/bcc (especially remove
unnecesary bcc includes). After this change, bcc/libbpf.h is needed in
2 places only:
- in attachpoint_parser for probe attachment
- in bpffeature as it contains definitions of BPF instructions
  (vendoring of these instructions seems impossible as that would cause
  redefinition errors)
libbpf is now required so it must be installed from source in LGTM (as
it is not available as a package on the Ubuntu version used by LGTM).
@lgtm-com
Copy link

lgtm-com bot commented Jun 14, 2022

This pull request introduces 2 alerts when merging 7d51201 into 14bd1ef - view on LGTM.com

new alerts:

  • 1 for Declaration hides parameter
  • 1 for Time-of-check time-of-use filesystem race condition

A local variable is shadowing a parameter, so rename it.
@lgtm-com
Copy link

lgtm-com bot commented Jun 14, 2022

This pull request introduces 1 alert when merging eff7cc5 into 14bd1ef - view on LGTM.com

new alerts:

  • 1 for Time-of-check time-of-use filesystem race condition

@viktormalik
Copy link
Contributor Author

The remaining LGTM alert is not related to this PR, it just showed now b/c libbpf and BTF are newly available inside the LGTM environment.

@fbs
Copy link
Member

fbs commented Jun 19, 2022

+1 but wonder if we should do anything special release wise as its a big change. I'm find with just merging it as we just did a release

@viktormalik
Copy link
Contributor Author

+1 but wonder if we should do anything special release wise as its a big change. I'm find with just merging it as we just did a release

Agreed. And since it's a big change, it'll be useful to leave it unreleased for some time, to discover more potential bugs.

@fbs
Copy link
Member

fbs commented Jun 27, 2022

Agree lets just merge it

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.

Linking both latest libbcc_bpf + libbpf library causes segfaults in bpftrace Require libbpf
2 participants