| Message ID | 1642004329-23514-1-git-send-email-alan.maguire@oracle.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | libbpf: userspace attach by name | expand |
On Wed, Jan 12, 2022 at 8:19 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > This patch series is a rough attempt to support attach by name for > uprobes and USDT (Userland Static Defined Tracing) probes. > Currently attach for such probes is done by determining the offset > manually, so the aim is to try and mimic the simplicity of kprobe > attach, making use of uprobe opts. > > One restriction applies: uprobe attach supports system-wide probing > by specifying "-1" for the pid. That functionality is not supported, > since we need a running process to determine the base address to > subtract to get the uprobe-friendly offset. There may be a way > to do this without a running process, so any suggestions would > be greatly appreciated. > > There are probably a bunch of subtleties missing here; the aim > is to see if this is useful and if so hopefully we can refine > it to deal with more complex cases. I tried to handle one case > that came to mind - weak library symbols - but there are probably > other issues when determining which address to use I haven't > thought of. > > Alan Maguire (4): > libbpf: support function name-based attach for uprobes > libbpf: support usdt provider/probe name-based attach for uprobes > selftests/bpf: add tests for u[ret]probe attach by name > selftests/bpf: add test for USDT uprobe attach by name > Hey Alan, I've been working on USDT support last year. It's considerably more code than in this RFC, but it handles not just finding a location of USDT probe(s), but also fetching its arguments based on argument location specification and more usability focused BPF-side APIs to work with USDTs. I don't remember how up to date it is, but the last "open source" version of it can be found at [0]. I currently have the latest debugged and tested version internally in the process of being integrated into our profiling solution here at Meta. So far it seems to be working fine and covers our production use cases well. The plan is to open source it as a separate companion library to libbpf some time in the next few months. Hopefully that would work for you. Once it is available, I hope we can also utilize it to convert some more BCC-based tools (that rely on USDT) to libbpf ([1]). [0] https://github.com/anakryiko/linux/commit/d473d042c8058da0a9e6c0353d97aeaf574925c6 [1] https://github.com/iovisor/bcc/tree/master/libbpf-tools > tools/lib/bpf/libbpf.c | 244 +++++++++++++++++++++ > tools/lib/bpf/libbpf.h | 17 +- > tools/testing/selftests/bpf/Makefile | 34 +++ > .../selftests/bpf/prog_tests/attach_probe.c | 74 ++++++- > .../selftests/bpf/progs/test_attach_probe.c | 24 ++ > 5 files changed, 391 insertions(+), 2 deletions(-) > > -- > 1.8.3.1 >
On Wed, 12 Jan 2022, Andrii Nakryiko wrote: > On Wed, Jan 12, 2022 at 8:19 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > > > This patch series is a rough attempt to support attach by name for > > uprobes and USDT (Userland Static Defined Tracing) probes. > > Currently attach for such probes is done by determining the offset > > manually, so the aim is to try and mimic the simplicity of kprobe > > attach, making use of uprobe opts. > > > > One restriction applies: uprobe attach supports system-wide probing > > by specifying "-1" for the pid. That functionality is not supported, > > since we need a running process to determine the base address to > > subtract to get the uprobe-friendly offset. There may be a way > > to do this without a running process, so any suggestions would > > be greatly appreciated. > > > > There are probably a bunch of subtleties missing here; the aim > > is to see if this is useful and if so hopefully we can refine > > it to deal with more complex cases. I tried to handle one case > > that came to mind - weak library symbols - but there are probably > > other issues when determining which address to use I haven't > > thought of. > > > > Alan Maguire (4): > > libbpf: support function name-based attach for uprobes > > libbpf: support usdt provider/probe name-based attach for uprobes > > selftests/bpf: add tests for u[ret]probe attach by name > > selftests/bpf: add test for USDT uprobe attach by name > > > > Hey Alan, > > I've been working on USDT support last year. It's considerably more > code than in this RFC, but it handles not just finding a location of > USDT probe(s), but also fetching its arguments based on argument > location specification and more usability focused BPF-side APIs to > work with USDTs. > > I don't remember how up to date it is, but the last "open source" > version of it can be found at [0]. I currently have the latest > debugged and tested version internally in the process of being > integrated into our profiling solution here at Meta. So far it seems > to be working fine and covers our production use cases well. > This looks great Andrii! I really like the argument access work, and the global tracing part is solved too by using the ELF segment info instead of the process maps to get the relative offset, with (I think?) use of BPF cookies to disambiguate between different user attachments. The one piece that seems to be missing from my perspective - and this may be in more recent versions - is uprobe function attachment by name. Most of the work is already done in libusdt so it's reasonably doable I think - at a minimum it would require an equivalent to the find_elf_func_offset() function in my patch 1. Now the name of the library libusdt suggests its focus is on USDT of course, but I think having userspace function attach by name too would be great. Is that part of your plans for this work? Thanks! Alan
On Thu, Jan 13, 2022 at 2:30 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > On Wed, 12 Jan 2022, Andrii Nakryiko wrote: > > > On Wed, Jan 12, 2022 at 8:19 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > > > > > This patch series is a rough attempt to support attach by name for > > > uprobes and USDT (Userland Static Defined Tracing) probes. > > > Currently attach for such probes is done by determining the offset > > > manually, so the aim is to try and mimic the simplicity of kprobe > > > attach, making use of uprobe opts. > > > > > > One restriction applies: uprobe attach supports system-wide probing > > > by specifying "-1" for the pid. That functionality is not supported, > > > since we need a running process to determine the base address to > > > subtract to get the uprobe-friendly offset. There may be a way > > > to do this without a running process, so any suggestions would > > > be greatly appreciated. > > > > > > There are probably a bunch of subtleties missing here; the aim > > > is to see if this is useful and if so hopefully we can refine > > > it to deal with more complex cases. I tried to handle one case > > > that came to mind - weak library symbols - but there are probably > > > other issues when determining which address to use I haven't > > > thought of. > > > > > > Alan Maguire (4): > > > libbpf: support function name-based attach for uprobes > > > libbpf: support usdt provider/probe name-based attach for uprobes > > > selftests/bpf: add tests for u[ret]probe attach by name > > > selftests/bpf: add test for USDT uprobe attach by name > > > > > > > Hey Alan, > > > > I've been working on USDT support last year. It's considerably more > > code than in this RFC, but it handles not just finding a location of > > USDT probe(s), but also fetching its arguments based on argument > > location specification and more usability focused BPF-side APIs to > > work with USDTs. > > > > I don't remember how up to date it is, but the last "open source" > > version of it can be found at [0]. I currently have the latest > > debugged and tested version internally in the process of being > > integrated into our profiling solution here at Meta. So far it seems > > to be working fine and covers our production use cases well. > > > > This looks great Andrii! I really like the argument access work, and the > global tracing part is solved too by using the ELF segment info instead > of the process maps to get the relative offset, with (I think?) use of > BPF cookies to disambiguate between different user attachments. BPF cookies are mandatory for when attaching to a shared library *and* NOT specifying PID. This is actually the mode that BCC doesn't seem to support. In all other cases BPF cookie shouldn't be mandatory. > > The one piece that seems to be missing from my perspective - and this may > be in more recent versions - is uprobe function attachment by name. Most of > the work is already done in libusdt so it's reasonably doable I think - at a > minimum it would require an equivalent to the find_elf_func_offset() > function in my patch 1. Now the name of the library libusdt suggests its > focus is on USDT of course, but I think having userspace function attach > by name too would be great. Is that part of your plans for this work? True, uprobes don't supprot attaching by function name, which is quite annoying. It's certainly not a focus for libusdt (or whatever it will end up being called when open-sources). But if it's not much code and complexity we should probably just add that to libbpf directly for uprobes. > > Thanks! > > Alan
On Fri, 14 Jan 2022, Andrii Nakryiko wrote: > > The one piece that seems to be missing from my perspective - and this may > > be in more recent versions - is uprobe function attachment by name. Most of > > the work is already done in libusdt so it's reasonably doable I think - at a > > minimum it would require an equivalent to the find_elf_func_offset() > > function in my patch 1. Now the name of the library libusdt suggests its > > focus is on USDT of course, but I think having userspace function attach > > by name too would be great. Is that part of your plans for this work? > > True, uprobes don't supprot attaching by function name, which is quite > annoying. It's certainly not a focus for libusdt (or whatever it will > end up being called when open-sources). But if it's not much code and > complexity we should probably just add that to libbpf directly for > uprobes. > I've been looking at this, and I've got the following cases working: - local symbols in a binary. This involves symbol table lookup and relative offset calcuation. - shared object symbols in a shared object. In this case, the symbol table values suffice, no adjustment needed. The former works using the program headers (instead of /proc/pid/maps for offset computation), so can be run for all processes, lifting the limitation in the RFC which only supported name lookup for a specific process. Around a hundred lines for this makes it worthwhile I think. There is one more case, which is a shared library function in a binary - where I specify "malloc" as the function and /usr/bin/foo as the binary path. In this case, for dynamic symbols we can't just look up the symbol table in the binary, since the associated values are 0. Ideally it would be nice if the user could just specify "malloc" and not need to use libc as the binary path argument, but getting this working is proving to be trickier. I've tried making use of PLT section information but no luck yet (the idea being we try to use the trampoline address of malloc@@PLT instead, but I'm still trying to figure out how to extract that). So I'm wondering if we just fail lookup for that case, assuming the user will specify the shared library path if they want to trace a shared library function. What do you think? Thanks! Alan
On Wed, Jan 19, 2022 at 6:04 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > On Fri, 14 Jan 2022, Andrii Nakryiko wrote: > > > > The one piece that seems to be missing from my perspective - and this may > > > be in more recent versions - is uprobe function attachment by name. Most of > > > the work is already done in libusdt so it's reasonably doable I think - at a > > > minimum it would require an equivalent to the find_elf_func_offset() > > > function in my patch 1. Now the name of the library libusdt suggests its > > > focus is on USDT of course, but I think having userspace function attach > > > by name too would be great. Is that part of your plans for this work? > > > > True, uprobes don't supprot attaching by function name, which is quite > > annoying. It's certainly not a focus for libusdt (or whatever it will > > end up being called when open-sources). But if it's not much code and > > complexity we should probably just add that to libbpf directly for > > uprobes. > > > > I've been looking at this, and I've got the following cases working: > > - local symbols in a binary. This involves symbol table lookup and > relative offset calcuation. > - shared object symbols in a shared object. In this case, the symbol > table values suffice, no adjustment needed. > > The former works using the program headers (instead of /proc/pid/maps for > offset computation), so can be run for all processes, lifting the > limitation in the RFC which only supported name lookup for a specific > process. Around a hundred lines for this makes it worthwhile I think. > > There is one more case, which is a shared library function in a binary - > where I specify "malloc" as the function and /usr/bin/foo as the binary > path. In this case, for dynamic symbols we can't just look up the symbol > table in the binary, since the associated values are 0. Ideally it would > be nice if the user could just specify "malloc" and not need to use libc > as the binary path argument, but getting this working is proving to be > trickier. I've tried making use of PLT section information but no luck > yet (the idea being we try to use the trampoline address of malloc@@PLT > instead, but I'm still trying to figure out how to extract that). > > So I'm wondering if we just fail lookup for that case, assuming the user > will specify the shared library path if they want to trace a shared library > function. What do you think? Thanks! I think it all makes sense (but let's see the code as well ;) ). For the latter, can you please double-check what sort of functionality BCC provides? Also make sure that you support specifying absolute address instead of function name as well (func+0x123 probably as well, just like for kprobes?). The annoying bit is libbpf's convention to use '/' as a separator in SEC() definitions. I think bpftrace/dtrace's ':' makes more sense, but it seems to disruptive to switch it now. Because of this, specifying absolute path to the binary would look weird: SEC("uprobe//usr/bin/bash/readline") or something like that would consistent with current convention, but super weird. Did you run into this issue during your experiments? I can see two improvements, more and less radical (short of switching from / to : completely): 1. less radical is to use "custom" format for uprobe after the "uprobe/" part: SEC("uprobe//usr/bin/bash:readline") 2. a bit more radical (but probably better long term) is to support '/' and ':' interchangeably (but only one of them in any given SEC() definition). For existing definitions, we can say that both forms are supported now: SEC("kprobe/some_func") and SEC("kprobe:some_func") For uprobe I'd probably combine #1 and #2 and say that these two forms are supported: SEC("uprobe//usr/bin/bash:readline") (so function separator is always ':') and SEC("uprobe:/usr/bin/bash:readline") (nicer and more consistent). Thoughts? BTW, as much as I like consistency, the proposal to switch to ':' exclusively in libbpf 1.0 is a no-go, IMO, it's too much of a disruption for tons of users. > > Alan