lkml.org 
[lkml]   [2022]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 08/10] libbpf: Add bpf_program__attach_kprobe_opts support for multi kprobes
    On Sun, Mar 6, 2022 at 9:29 AM Jiri Olsa <olsajiri@gmail.com> wrote:
    >
    > On Fri, Mar 04, 2022 at 03:11:19PM -0800, Andrii Nakryiko wrote:
    > > On Tue, Feb 22, 2022 at 9:07 AM Jiri Olsa <jolsa@kernel.org> wrote:
    > > >
    > > > Adding support to bpf_program__attach_kprobe_opts to attach kprobes
    > > > to multiple functions.
    > > >
    > > > If the kprobe program has BPF_TRACE_KPROBE_MULTI as expected_attach_type
    > > > it will use the new kprobe_multi link to attach the program. In this case
    > > > it will use 'func_name' as pattern for functions to attach.
    > > >
    > > > Adding also new section types 'kprobe.multi' and kretprobe.multi'
    > > > that allows to specify wildcards (*?) for functions, like:
    > > >
    > > > SEC("kprobe.multi/bpf_fentry_test*")
    > > > SEC("kretprobe.multi/bpf_fentry_test?")
    > > >
    > > > This will set kprobe's expected_attach_type to BPF_TRACE_KPROBE_MULTI,
    > > > and attach it to functions provided by the function pattern.
    > > >
    > > > Using glob_match from selftests/bpf/test_progs.c and adding support to
    > > > match '?' based on original perf code.
    > > >
    > > > Cc: Masami Hiramatsu <mhiramat@redhat.com>
    > > > Cc: Yucong Sun <fallentree@fb.com>
    > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
    > > > ---
    > > > tools/lib/bpf/libbpf.c | 130 +++++++++++++++++++++++++++++++++++++++--
    > > > 1 file changed, 125 insertions(+), 5 deletions(-)
    > > >
    > >
    > > [...]
    > >
    > > > +static struct bpf_link *
    > > > +attach_kprobe_multi_opts(const struct bpf_program *prog,
    > > > + const char *func_pattern,
    > > > + const struct bpf_kprobe_opts *kopts)
    > > > +{
    > > > + DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
    > >
    > > nit: just LIBBPF_OPTS
    >
    > ok
    >
    > >
    > >
    > > > + struct kprobe_multi_resolve res = {
    > > > + .name = func_pattern,
    > > > + };
    > > > + struct bpf_link *link = NULL;
    > > > + char errmsg[STRERR_BUFSIZE];
    > > > + int err, link_fd, prog_fd;
    > > > + bool retprobe;
    > > > +
    > > > + err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
    > >
    > > hm... I think as a generic API we should support three modes of
    > > specifying attachment target:
    > >
    > >
    > > 1. glob-based (very convenient, I agree)
    > > 2. array of function names (very convenient when I know specific set
    > > of functions)
    > > 3. array of addresses (advanced use case, so probably will be rarely used).
    > >
    > >
    > >
    > > So I wonder if it's better to have a separate
    > > bpf_program__attach_kprobe_multi() API for this, instead of doing both
    > > inside bpf_program__attach_kprobe()...
    > >
    > > In such case bpf_program__attach_kprobe() could either fail if
    > > expected attach type is BPF_TRACE_KPROBE_MULTI or it can redirect to
    > > attach_kprobe_multi with func_name as a pattern or just single
    > > function (let's think which one makes more sense)
    > >
    > > Let's at least think about this
    >
    > I think it would make the code more clear, how about this:
    >
    > struct bpf_kprobe_multi_opts {
    > /* size of this struct, for forward/backward compatiblity */
    > size_t sz;
    >
    > const char **funcs;

    naming nit: func_names (to oppose it to "func_pattern")? Or just
    "names" to be in line with "addrs" (but then "pattern" instead of
    "func_pattern"? with kprobe it's always about functions, so this
    "func_" everywhere is a bit redundant)

    > const unsigned long *addrs;
    > const u64 *cookies;
    > int cnt;

    nit: let's use size_t


    > bool retprobe;
    > size_t :0;
    > };
    >
    > bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
    > const char *pattern,
    > const struct bpf_kprobe_multi_opts *opts);
    >
    >
    > if pattern is NULL we'd use opts data:
    >
    > bpf_program__attach_kprobe_multi_opts(prog, "ksys_*", NULL);
    > bpf_program__attach_kprobe_multi_opts(prog, NULL, &opts);
    >
    > to have '2. array of function names' as direct function argument,
    > we'd need to add 'cnt' as well, so I think it's better to have it
    > in opts, and have just pattern for quick/convenient call without opts
    >

    yeah, naming pattern as direct argument for common use case makes
    sense. Let's go with this scheme


    [...]

    \
     
     \ /
      Last update: 2022-03-08 02:29    [W:2.546 / U:0.512 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site