Re: [PATCH v3 bpf-next 0/7] Add libbpf support for USDTs

[Hengqi Chen <hengqi.chen@gmail.com>]

On 2022/4/5 7:41 AM, Andrii Nakryiko wrote:
> Add libbpf support for USDT (User Statically-Defined Tracing) probes.
> USDTs is important part of tracing, and BPF, ecosystem, widely used in
> mission-critical production applications for observability, performance
> analysis, and debugging.
> 
> And while USDTs themselves are pretty complicated abstraction built on top of
> uprobes, for end-users USDT is as natural a primitive as uprobes themselves.
> And thus it's important for libbpf to provide best possible user experience
> when it comes to build tracing applications relying on USDTs.
> 
> USDTs historically presented a lot of challenges for libbpf's no
> compilation-on-the-fly general approach to BPF tracing. BCC utilizes power of
> on-the-fly source code generation and compilation using its embedded Clang
> toolchain, which was impractical for more lightweight and thus more rigid
> libbpf-based approach. But still, with enough diligence and BPF cookies it's
> possible to implement USDT support that feels as natural as tracing any
> uprobe.
> 
> This patch set is the culmination of such effort to add libbpf USDT support
> following the spirit and philosophy of BPF CO-RE (even though it's not
> inherently relying on BPF CO-RE much, see patch #1 for some notes regarding
> this). Each respective patch has enough details and explanations, so I won't
> go into details here.
> 
> In the end, I think the overall usability of libbpf's USDT support *exceeds*
> the status quo set by BCC due to the elimination of awkward runtime USDT
> supporting code generation. It also exceeds BCC's capabilities due to the use
> of BPF cookie. This eliminates the need to determine a USDT call site (and
> thus specifics about how exactly to fetch arguments) based on its *absolute IP
> address*, which is impossible with shared libraries if no PID is specified (as
> we then just *can't* know absolute IP at which shared library is loaded,
> because it might be different for each process). With BPF cookie this is not
> a problem as we record "call site ID" directly in a BPF cookie value. This
> makes it possible to do a system-wide tracing of a USDT defined in a shared
> library. Think about tracing some USDT in libc across any process in the
> system, both running at the time of attachment and all the new processes
> started *afterwards*. This is a very powerful capability that allows more
> efficient observability and tracing tooling.
> 
> Once this functionality lands, the plan is to extend libbpf-bootstrap ([0])
> with an USDT example. It will also become possible to start converting BCC
> tools that rely on USDTs to their libbpf-based counterparts ([1]).
> 
> It's worth noting that preliminary version of this code was currently used and
> tested in production code running fleet-wide observability toolkit.
> 
> Libbpf functionality is broken down into 5 mostly logically independent parts,
> for ease of reviewing:
>   - patch #1 adds BPF-side implementation;
>   - patch #2 adds user-space APIs and wires bpf_link for USDTs;
>   - patch #3 adds the most mundate pieces: handling ELF, parsing USDT notes,
>     dealing with memory segments, relative vs absolute addresses, etc;
>   - patch #4 adds internal ID allocation and setting up/tearing down of
>     BPF-side state (spec and IP-to-ID mapping);
>   - patch #5 implements x86/x86-64-specific logic of parsing USDT argument
>     specifications;
>   - patch #6 adds testing of various basic aspects of handling of USDT;
>   - patch #7 extends the set of tests with more combinations of semaphore,
>     executable vs shared library, and PID filter options.
> 
>   [0] https://github.com/libbpf/libbpf-bootstrap
>   [1] https://github.com/iovisor/bcc/tree/master/libbpf-tools
> 
> v2->v3:
>   - fix typos, leave link to systemtap doc, acks, etc (Dave);
>   - include sys/sdt.h to avoid extra system-wide package dependencies;
> v1->v2:
>   - huge high-level comment describing how all the moving parts fit together
>     (Alan, Alexei);
>   - switched from `__hidden __weak` to `static inline __noinline` for now, as
>     there is a bug in BPF linker breaking final BPF object file due to invalid
>     .BTF.ext data; I want to fix it separately at which point I'll switch back
>     to __hidden __weak again. The fix isn't trivial, so I don't want to block
>     on that. Same for __weak variable lookup bug that Henqi reported.
>   - various fixes and improvements, addressing other feedback (Alan, Hengqi);
> 
> Cc: Alan Maguire <alan.maguire@oracle.com>
> Cc: Dave Marchevsky <davemarchevsky@fb.com>
> Cc: Hengqi Chen <hengqi.chen@gmail.com>
> 
> Andrii Nakryiko (7):
>   libbpf: add BPF-side of USDT support
>   libbpf: wire up USDT API and bpf_link integration
>   libbpf: add USDT notes parsing and resolution logic
>   libbpf: wire up spec management and other arch-independent USDT logic
>   libbpf: add x86-specific USDT arg spec parsing logic
>   selftests/bpf: add basic USDT selftests
>   selftests/bpf: add urandom_read shared lib and USDTs
> 
>  tools/lib/bpf/Build                           |    3 +-
>  tools/lib/bpf/Makefile                        |    2 +-
>  tools/lib/bpf/libbpf.c                        |  115 +-
>  tools/lib/bpf/libbpf.h                        |   31 +
>  tools/lib/bpf/libbpf.map                      |    1 +
>  tools/lib/bpf/libbpf_internal.h               |   19 +
>  tools/lib/bpf/usdt.bpf.h                      |  256 ++++
>  tools/lib/bpf/usdt.c                          | 1280 +++++++++++++++++
>  tools/testing/selftests/bpf/Makefile          |   25 +-
>  tools/testing/selftests/bpf/prog_tests/usdt.c |  421 ++++++
>  .../selftests/bpf/progs/test_urandom_usdt.c   |   70 +
>  tools/testing/selftests/bpf/progs/test_usdt.c |   96 ++
>  .../selftests/bpf/progs/test_usdt_multispec.c |   32 +
>  tools/testing/selftests/bpf/sdt-config.h      |    6 +
>  tools/testing/selftests/bpf/sdt.h             |  513 +++++++
>  tools/testing/selftests/bpf/urandom_read.c    |   63 +-
>  .../testing/selftests/bpf/urandom_read_aux.c  |    9 +
>  .../testing/selftests/bpf/urandom_read_lib1.c |   13 +
>  .../testing/selftests/bpf/urandom_read_lib2.c |    8 +
>  19 files changed, 2938 insertions(+), 25 deletions(-)
>  create mode 100644 tools/lib/bpf/usdt.bpf.h
>  create mode 100644 tools/lib/bpf/usdt.c
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/usdt.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_urandom_usdt.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_usdt.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_usdt_multispec.c
>  create mode 100644 tools/testing/selftests/bpf/sdt-config.h
>  create mode 100644 tools/testing/selftests/bpf/sdt.h
>  create mode 100644 tools/testing/selftests/bpf/urandom_read_aux.c
>  create mode 100644 tools/testing/selftests/bpf/urandom_read_lib1.c
>  create mode 100644 tools/testing/selftests/bpf/urandom_read_lib2.c
> 

For the first 5 patches:

Reviewed-by: Hengqi Chen <hengqi.chen@gmail.com>
Tested-by: Hengqi Chen <hengqi.chen@gmail.com>

Hengqi