| Message ID | 20210714094400.396467-7-jolsa@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf, x86: Add bpf_get_func_ip helper | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 2 maintainers not CCed: andrii@kernel.org kpsingh@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: From:/Signed-off-by: email address mismatch: 'From: Jiri Olsa <jolsa@redhat.com>' != 'Signed-off-by: Jiri Olsa <jolsa@kernel.org>' WARNING: line length of 82 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Wed, Jul 14, 2021 at 2:45 AM Jiri Olsa <jolsa@redhat.com> wrote: > > Adding bpf_program__attach_kprobe_opts that does the same > as bpf_program__attach_kprobe, but takes opts argument. > > Currently opts struct holds just retprobe bool, but we will > add new field in following patch. > > The function is not exported, so there's no need to add > size to the struct bpf_program_attach_kprobe_opts for now. Why not exported? Please use a proper _opts struct just like others (e.g., bpf_object_open_opts) and add is as a public API, it's a useful addition. We are going to have a similar structure for attach_uprobe, btw. Please send a follow up patch. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/lib/bpf/libbpf.c | 34 +++++++++++++++++++++++++--------- > 1 file changed, 25 insertions(+), 9 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 88b99401040c..d93a6f9408d1 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -10346,19 +10346,24 @@ static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name, > return pfd; > } > > -struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog, > - bool retprobe, > - const char *func_name) > +struct bpf_program_attach_kprobe_opts { when you make it part of libbpf API, let's call it something shorter, like bpf_kprobe_opts, maybe? And later we'll have bpf_uprobe_opts for uprobes. Short and unambiguous. > + bool retprobe; > +}; > + > +static struct bpf_link* > +bpf_program__attach_kprobe_opts(struct bpf_program *prog, > + const char *func_name, > + struct bpf_program_attach_kprobe_opts *opts) > { > char errmsg[STRERR_BUFSIZE]; > struct bpf_link *link; > int pfd, err; > [...]
On Fri, Jul 16, 2021 at 06:41:59PM -0700, Andrii Nakryiko wrote: > On Wed, Jul 14, 2021 at 2:45 AM Jiri Olsa <jolsa@redhat.com> wrote: > > > > Adding bpf_program__attach_kprobe_opts that does the same > > as bpf_program__attach_kprobe, but takes opts argument. > > > > Currently opts struct holds just retprobe bool, but we will > > add new field in following patch. > > > > The function is not exported, so there's no need to add > > size to the struct bpf_program_attach_kprobe_opts for now. > > Why not exported? Please use a proper _opts struct just like others > (e.g., bpf_object_open_opts) and add is as a public API, it's a useful > addition. We are going to have a similar structure for attach_uprobe, > btw. Please send a follow up patch. there's no outside user.. ok > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > tools/lib/bpf/libbpf.c | 34 +++++++++++++++++++++++++--------- > > 1 file changed, 25 insertions(+), 9 deletions(-) > > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > index 88b99401040c..d93a6f9408d1 100644 > > --- a/tools/lib/bpf/libbpf.c > > +++ b/tools/lib/bpf/libbpf.c > > @@ -10346,19 +10346,24 @@ static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name, > > return pfd; > > } > > > > -struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog, > > - bool retprobe, > > - const char *func_name) > > +struct bpf_program_attach_kprobe_opts { > > when you make it part of libbpf API, let's call it something shorter, > like bpf_kprobe_opts, maybe? And later we'll have bpf_uprobe_opts for > uprobes. Short and unambiguous. ok jirka > > > + bool retprobe; > > +}; > > + > > +static struct bpf_link* > > +bpf_program__attach_kprobe_opts(struct bpf_program *prog, > > + const char *func_name, > > + struct bpf_program_attach_kprobe_opts *opts) > > { > > char errmsg[STRERR_BUFSIZE]; > > struct bpf_link *link; > > int pfd, err; > > > > [...] >
On Sun, Jul 18, 2021 at 12:32 PM Jiri Olsa <jolsa@redhat.com> wrote: > > On Fri, Jul 16, 2021 at 06:41:59PM -0700, Andrii Nakryiko wrote: > > On Wed, Jul 14, 2021 at 2:45 AM Jiri Olsa <jolsa@redhat.com> wrote: > > > > > > Adding bpf_program__attach_kprobe_opts that does the same > > > as bpf_program__attach_kprobe, but takes opts argument. > > > > > > Currently opts struct holds just retprobe bool, but we will > > > add new field in following patch. > > > > > > The function is not exported, so there's no need to add > > > size to the struct bpf_program_attach_kprobe_opts for now. > > > > Why not exported? Please use a proper _opts struct just like others > > (e.g., bpf_object_open_opts) and add is as a public API, it's a useful > > addition. We are going to have a similar structure for attach_uprobe, > > btw. Please send a follow up patch. > > there's no outside user.. ok because there is no API :) I've seen people asking about the ability to attach to kprobe+offset in some PRs. > > > > > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > --- > > > tools/lib/bpf/libbpf.c | 34 +++++++++++++++++++++++++--------- > > > 1 file changed, 25 insertions(+), 9 deletions(-) > > > > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > > index 88b99401040c..d93a6f9408d1 100644 > > > --- a/tools/lib/bpf/libbpf.c > > > +++ b/tools/lib/bpf/libbpf.c > > > @@ -10346,19 +10346,24 @@ static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name, > > > return pfd; > > > } > > > > > > -struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog, > > > - bool retprobe, > > > - const char *func_name) > > > +struct bpf_program_attach_kprobe_opts { > > > > when you make it part of libbpf API, let's call it something shorter, > > like bpf_kprobe_opts, maybe? And later we'll have bpf_uprobe_opts for > > uprobes. Short and unambiguous. > > ok > > jirka > > > > > > + bool retprobe; > > > +}; > > > + > > > +static struct bpf_link* > > > +bpf_program__attach_kprobe_opts(struct bpf_program *prog, > > > + const char *func_name, > > > + struct bpf_program_attach_kprobe_opts *opts) > > > { > > > char errmsg[STRERR_BUFSIZE]; > > > struct bpf_link *link; > > > int pfd, err; > > > > > > > [...] > > >
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 88b99401040c..d93a6f9408d1 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -10346,19 +10346,24 @@ static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name, return pfd; } -struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog, - bool retprobe, - const char *func_name) +struct bpf_program_attach_kprobe_opts { + bool retprobe; +}; + +static struct bpf_link* +bpf_program__attach_kprobe_opts(struct bpf_program *prog, + const char *func_name, + struct bpf_program_attach_kprobe_opts *opts) { char errmsg[STRERR_BUFSIZE]; struct bpf_link *link; int pfd, err; - pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name, + pfd = perf_event_open_probe(false /* uprobe */, opts->retprobe, func_name, 0 /* offset */, -1 /* pid */); if (pfd < 0) { pr_warn("prog '%s': failed to create %s '%s' perf event: %s\n", - prog->name, retprobe ? "kretprobe" : "kprobe", func_name, + prog->name, opts->retprobe ? "kretprobe" : "kprobe", func_name, libbpf_strerror_r(pfd, errmsg, sizeof(errmsg))); return libbpf_err_ptr(pfd); } @@ -10367,23 +10372,34 @@ struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog, if (err) { close(pfd); pr_warn("prog '%s': failed to attach to %s '%s': %s\n", - prog->name, retprobe ? "kretprobe" : "kprobe", func_name, + prog->name, opts->retprobe ? "kretprobe" : "kprobe", func_name, libbpf_strerror_r(err, errmsg, sizeof(errmsg))); return libbpf_err_ptr(err); } return link; } +struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog, + bool retprobe, + const char *func_name) +{ + struct bpf_program_attach_kprobe_opts opts = { + .retprobe = retprobe, + }; + + return bpf_program__attach_kprobe_opts(prog, func_name, &opts); +} + static struct bpf_link *attach_kprobe(const struct bpf_sec_def *sec, struct bpf_program *prog) { + struct bpf_program_attach_kprobe_opts opts; const char *func_name; - bool retprobe; func_name = prog->sec_name + sec->len; - retprobe = strcmp(sec->sec, "kretprobe/") == 0; + opts.retprobe = strcmp(sec->sec, "kretprobe/") == 0; - return bpf_program__attach_kprobe(prog, retprobe, func_name); + return bpf_program__attach_kprobe_opts(prog, func_name, &opts); } struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
Adding bpf_program__attach_kprobe_opts that does the same as bpf_program__attach_kprobe, but takes opts argument. Currently opts struct holds just retprobe bool, but we will add new field in following patch. The function is not exported, so there's no need to add size to the struct bpf_program_attach_kprobe_opts for now. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/lib/bpf/libbpf.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)