Re: [PATCHv3 bpf-next 00/26] bpf: Add multi uprobe link

[Jiri Olsa <olsajiri@gmail.com>]

On Wed, Jul 05, 2023 at 02:45:42PM +0200, Daniel Borkmann wrote:
> On 6/30/23 10:33 AM, Jiri Olsa wrote:
> > hi,
> > this patchset is adding support to attach multiple uprobes and usdt probes
> > through new uprobe_multi link.
> > 
> > The current uprobe is attached through the perf event and attaching many
> > uprobes takes a lot of time because of that.
> > 
> > The main reason is that we need to install perf event for each probed function
> > and profile shows perf event installation (perf_install_in_context) as culprit.
> > 
> > The new uprobe_multi link just creates raw uprobes and attaches the bpf
> > program to them without perf event being involved.
> > 
> > In addition to being faster we also save file descriptors. For the current
> > uprobe attach we use extra perf event fd for each probed function. The new
> > link just need one fd that covers all the functions we are attaching to.
> > 
> > v3 changes:
> >    - consolidate attach type checks in bpf_prog_attach_check_attach_type [Andrii]
> >    - remove bpf_prog_active check [Alexei]
> >    - change rcu locking [Andrii]
> >    - allocate ref_ctr_offsets conditionally [Andrii]
> >    - remove ctx check from bpf_uprobe_multi_cookie [Andrii]
> >    - move some elf_* functions in elf.c object [Andrii]
> >    - fix uprobe link detection code [Andrii]
> >    - add usdt.s program section [Andrii]
> >    - rename bpf_program__attach_uprobe_multi_opts to bpf_program__attach_uprobe_multi [Andrii]
> >    - remove extra case from attach_uprobe_multi [Andrii]
> >    - rework usdt_manager_attach_usdt [Andrii]
> >    - rework/rename new elf_find_* functions [Andrii]
> >    - elf iterator fixes [Andrii]
> >      - renames
> >      - elf_sym_iter_next loop restruct
> >      - return directly GElf_Sym in elf_sym
> >      - add elf_sym_offset helper
> >      - add st_type arg elf_sym_iter_new
> >      - get rid of '_' prefixed functions
> >      - simplify offsets handling
> >      - other smaller changes
> >    - added acks
> >    - todo:
> >      - seems like more elf_* helpers could go in elf.c object,
> >        I'll send that as follow up
> >      - will send fill_info support when [3] is merged [Andrii]
> > 
> > 
> > There's support for bpftrace [2] and tetragon [1].
> > 
> > Also available at:
> >    https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> >    uprobe_multi
> > 
> > thanks,
> > jirka
> > 
> > 
> > [1] https://github.com/cilium/tetragon/pull/936
> > [2] https://github.com/iovisor/bpftrace/compare/master...olsajiri:bpftrace:uprobe_multi
> > [3] https://lore.kernel.org/bpf/20230628115329.248450-1-laoar.shao@gmail.com/
> > ---
> > Jiri Olsa (26):
> >        bpf: Add attach_type checks under bpf_prog_attach_check_attach_type
> >        bpf: Add multi uprobe link
> >        bpf: Add cookies support for uprobe_multi link
> >        bpf: Add pid filter support for uprobe_multi link
> >        bpf: Add bpf_get_func_ip helper support for uprobe link
> >        libbpf: Add uprobe_multi attach type and link names
> >        libbpf: Move elf_find_func_offset* functions to elf object
> >        libbpf: Add elf_open/elf_close functions
> >        libbpf: Add elf symbol iterator
> >        libbpf: Add elf_resolve_syms_offsets function
> >        libbpf: Add elf_resolve_pattern_offsets function
> >        libbpf: Add bpf_link_create support for multi uprobes
> >        libbpf: Add bpf_program__attach_uprobe_multi function
> >        libbpf: Add support for u[ret]probe.multi[.s] program sections
> >        libbpf: Add uprobe multi link detection
> >        libbpf: Add uprobe multi link support to bpf_program__attach_usdt
> >        selftests/bpf: Add uprobe_multi skel test
> >        selftests/bpf: Add uprobe_multi api test
> >        selftests/bpf: Add uprobe_multi link test
> >        selftests/bpf: Add uprobe_multi test program
> >        selftests/bpf: Add uprobe_multi bench test
> >        selftests/bpf: Add usdt_multi test program
> >        selftests/bpf: Add usdt_multi bench test
> >        selftests/bpf: Add uprobe_multi cookie test
> >        selftests/bpf: Add uprobe_multi pid filter tests
> >        selftests/bpf: Add extra link to uprobe_multi tests
> > 
> >   include/linux/trace_events.h                               |   6 ++
> >   include/uapi/linux/bpf.h                                   |  16 ++++
> >   kernel/bpf/syscall.c                                       | 122 ++++++++++++-------------
> >   kernel/trace/bpf_trace.c                                   | 346 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >   tools/include/uapi/linux/bpf.h                             |  16 ++++
> >   tools/lib/bpf/Build                                        |   2 +-
> >   tools/lib/bpf/bpf.c                                        |  11 +++
> >   tools/lib/bpf/bpf.h                                        |  11 ++-
> >   tools/lib/bpf/elf.c                                        | 435 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   tools/lib/bpf/libbpf.c                                     | 396 ++++++++++++++++++++++++++++++++++++++++++---------------------------------------
> >   tools/lib/bpf/libbpf.h                                     |  27 ++++++
> >   tools/lib/bpf/libbpf.map                                   |   1 +
> >   tools/lib/bpf/libbpf_elf.h                                 |  24 +++++
> >   tools/lib/bpf/libbpf_internal.h                            |   3 +
> >   tools/lib/bpf/usdt.c                                       | 109 +++++++++++++++--------
> >   tools/testing/selftests/bpf/Makefile                       |  10 +++
> >   tools/testing/selftests/bpf/prog_tests/bpf_cookie.c        |  78 ++++++++++++++++
> >   tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c | 449 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   tools/testing/selftests/bpf/progs/uprobe_multi.c           | 110 +++++++++++++++++++++++
> >   tools/testing/selftests/bpf/progs/uprobe_multi_usdt.c      |  16 ++++
> >   tools/testing/selftests/bpf/uprobe_multi.c                 |  53 +++++++++++
> >   tools/testing/selftests/bpf/usdt_multi.c                   |  24 +++++
> >   22 files changed, 1969 insertions(+), 296 deletions(-)
> >   create mode 100644 tools/lib/bpf/elf.c
> >   create mode 100644 tools/lib/bpf/libbpf_elf.h
> >   create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> >   create mode 100644 tools/testing/selftests/bpf/progs/uprobe_multi.c
> >   create mode 100644 tools/testing/selftests/bpf/progs/uprobe_multi_usdt.c
> >   create mode 100644 tools/testing/selftests/bpf/uprobe_multi.c
> >   create mode 100644 tools/testing/selftests/bpf/usdt_multi.c
> 
> Looks like BPF CI did not trigger because of build error with LLVM:
> 
> https://github.com/kernel-patches/bpf/actions/runs/5423873866/jobs/9908376097

will check, thanks

jirka

> 
>   [...]
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:367:6: error: variable 'attach_end_ns' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_OK(err, "uprobe_multi__attach"))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:381:18: note: uninitialized use occurs here
>           attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
>                           ^~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:367:2: note: remove the 'if' if its condition is always false
>           if (!ASSERT_OK(err, "uprobe_multi__attach"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:361:6: error: variable 'attach_end_ns' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_EQ(err, 0, "uprobe_multi__load"))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:381:18: note: uninitialized use occurs here
>           attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
>                           ^~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:361:2: note: remove the 'if' if its condition is always false
>           if (!ASSERT_EQ(err, 0, "uprobe_multi__load"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:352:6: error: variable 'attach_end_ns' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_OK_PTR(skel, "uprobe_multi__open"))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:381:18: note: uninitialized use occurs here
>           attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
>                           ^~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:352:2: note: remove the 'if' if its condition is always false
>           if (!ASSERT_OK_PTR(skel, "uprobe_multi__open"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:344:37: note: initialize the variable 'attach_end_ns' to silence this warning
>           long attach_start_ns, attach_end_ns;
>                                              ^
>                                               = 0
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:361:6: error: variable 'attach_start_ns' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_EQ(err, 0, "uprobe_multi__load"))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:381:34: note: uninitialized use occurs here
>           attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
>                                           ^~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:361:2: note: remove the 'if' if its condition is always false
>           if (!ASSERT_EQ(err, 0, "uprobe_multi__load"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:352:6: error: variable 'attach_start_ns' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_OK_PTR(skel, "uprobe_multi__open"))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:381:34: note: uninitialized use occurs here
>           attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
>                                           ^~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:352:2: note: remove the 'if' if its condition is always false
>           if (!ASSERT_OK_PTR(skel, "uprobe_multi__open"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:344:22: note: initialize the variable 'attach_start_ns' to silence this warning
>           long attach_start_ns, attach_end_ns;
>                               ^
>                                = 0
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:414:6: error: variable 'attach_end_ns' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_OK_PTR(skel->links.usdt0, "bpf_program__attach_usdt"))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:428:18: note: uninitialized use occurs here
>           attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
>                           ^~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:414:2: note: remove the 'if' if its condition is always false
>           if (!ASSERT_OK_PTR(skel->links.usdt0, "bpf_program__attach_usdt"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:407:6: error: variable 'attach_end_ns' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_EQ(err, 0, "uprobe_multi_usdt__load"))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:428:18: note: uninitialized use occurs here
>           attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
>                           ^~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:407:2: note: remove the 'if' if its condition is always false
>           if (!ASSERT_EQ(err, 0, "uprobe_multi_usdt__load"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:398:6: error: variable 'attach_end_ns' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_OK_PTR(skel, "uprobe_multi__open"))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:428:18: note: uninitialized use occurs here
>           attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
>                           ^~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:398:2: note: remove the 'if' if its condition is always false
>           if (!ASSERT_OK_PTR(skel, "uprobe_multi__open"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:391:37: note: initialize the variable 'attach_end_ns' to silence this warning
>           long attach_start_ns, attach_end_ns;
>                                              ^
>                                               = 0
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:407:6: error: variable 'attach_start_ns' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_EQ(err, 0, "uprobe_multi_usdt__load"))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:428:34: note: uninitialized use occurs here
>           attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
>                                           ^~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:407:2: note: remove the 'if' if its condition is always false
>           if (!ASSERT_EQ(err, 0, "uprobe_multi_usdt__load"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:398:6: error: variable 'attach_start_ns' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>           if (!ASSERT_OK_PTR(skel, "uprobe_multi__open"))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:428:34: note: uninitialized use occurs here
>           attach_delta = (attach_end_ns - attach_start_ns) / 1000000000.0;
>                                           ^~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:398:2: note: remove the 'if' if its condition is always false
>           if (!ASSERT_OK_PTR(skel, "uprobe_multi__open"))
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c:391:22: note: initialize the variable 'attach_start_ns' to silence this warning
>           long attach_start_ns, attach_end_ns;
>                               ^
>                                = 0
>   10 errors generated.
>     TEST-OBJ [test_progs] lookup_key.test.o
>   make: *** [Makefile:578: /tmp/work/bpf/bpf/tools/testing/selftests/bpf/uprobe_multi_test.test.o] Error 1
>   make: *** Waiting for unfinished jobs....
>   make: Leaving directory '/tmp/work/bpf/bpf/tools/testing/selftests/bpf'
>   Error: Process completed with exit code 2.