* [bpf-next v8 0/3] bpftool: Add autoattach for bpf prog load|loadall
@ 2022-10-10 10:59 Wang Yufen
2022-10-10 10:59 ` [bpf-next v8 1/3] " Wang Yufen
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Wang Yufen @ 2022-10-10 10:59 UTC (permalink / raw)
To: quentin, ast, daniel, andrii, martin.lau, song, yhs,
john.fastabend, kpsingh, sdf, haoluo, jolsa, davem, kuba, hawk,
nathan, ndesaulniers, trix
Cc: bpf, netdev, llvm
This patchset add "autoattach" optional for "bpftool prog load(_all)" to support
one-step load-attach-pin_link.
v7 -> v8: for the programs not supporting autoattach, fall back to reguler pinning
instead of skipping
v6 -> v7: add info msg print and update doc for the skip program
v5 -> v6: skip the programs not supporting auto-attach,
and change optional name from "auto_attach" to "autoattach"
v4 -> v5: some formatting nits of doc
v3 -> v4: rename functions, update doc, bash and do_help()
v2 -> v3: switch to extend prog load command instead of extend perf
v2: https://patchwork.kernel.org/project/netdevbpf/patch/20220824033837.458197-1-weiyongjun1@huawei.com/
v1: https://patchwork.kernel.org/project/netdevbpf/patch/20220816151725.153343-1-weiyongjun1@huawei.com/
Wang Yufen (3):
bpftool: Add autoattach for bpf prog load|loadall
bpftool: Update doc (add autoattach to prog load)
bpftool: Update the bash completion(add autoattach to prog load)
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 15 ++++-
tools/bpf/bpftool/bash-completion/bpftool | 1 +
tools/bpf/bpftool/prog.c | 78 +++++++++++++++++++++++-
3 files changed, 90 insertions(+), 4 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [bpf-next v8 1/3] bpftool: Add autoattach for bpf prog load|loadall 2022-10-10 10:59 [bpf-next v8 0/3] bpftool: Add autoattach for bpf prog load|loadall Wang Yufen @ 2022-10-10 10:59 ` Wang Yufen 2022-10-13 19:47 ` Andrii Nakryiko 2022-10-10 11:00 ` [bpf-next v8 2/3] bpftool: Update doc (add autoattach to prog load) Wang Yufen ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Wang Yufen @ 2022-10-10 10:59 UTC (permalink / raw) To: quentin, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa, davem, kuba, hawk, nathan, ndesaulniers, trix Cc: bpf, netdev, llvm Add autoattach optional to support one-step load-attach-pin_link. For example, $ bpftool prog loadall test.o /sys/fs/bpf/test autoattach $ bpftool link 26: tracing name test1 tag f0da7d0058c00236 gpl loaded_at 2022-09-09T21:39:49+0800 uid 0 xlated 88B jited 55B memlock 4096B map_ids 3 btf_id 55 28: kprobe name test3 tag 002ef1bef0723833 gpl loaded_at 2022-09-09T21:39:49+0800 uid 0 xlated 88B jited 56B memlock 4096B map_ids 3 btf_id 55 57: tracepoint name oncpu tag 7aa55dfbdcb78941 gpl loaded_at 2022-09-09T21:41:32+0800 uid 0 xlated 456B jited 265B memlock 4096B map_ids 17,13,14,15 btf_id 82 $ bpftool link 1: tracing prog 26 prog_type tracing attach_type trace_fentry 3: perf_event prog 28 10: perf_event prog 57 The autoattach optional can support tracepoints, k(ret)probes, u(ret)probes. Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> Signed-off-by: Wang Yufen <wangyufen@huawei.com> --- tools/bpf/bpftool/prog.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 2 deletions(-) diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index c81362a..8f3afce 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -1453,6 +1453,69 @@ static int do_run(int argc, char **argv) return ret; } +static int +auto_attach_program(struct bpf_program *prog, const char *path) +{ + struct bpf_link *link; + int err; + + link = bpf_program__attach(prog); + if (!link) { + p_info("Program %s does not support autoattach, falling back to pinning", + bpf_program__name(prog)); + return bpf_obj_pin(bpf_program__fd(prog), path); + } + err = bpf_link__pin(link, path); + if (err) { + bpf_link__destroy(link); + return err; + } + return 0; +} + +static int pathname_concat(const char *path, const char *name, char *buf) +{ + int len; + + len = snprintf(buf, PATH_MAX, "%s/%s", path, name); + if (len < 0) + return -EINVAL; + if (len >= PATH_MAX) + return -ENAMETOOLONG; + + return 0; +} + +static int +auto_attach_programs(struct bpf_object *obj, const char *path) +{ + struct bpf_program *prog; + char buf[PATH_MAX]; + int err; + + bpf_object__for_each_program(prog, obj) { + err = pathname_concat(path, bpf_program__name(prog), buf); + if (err) + goto err_unpin_programs; + + err = auto_attach_program(prog, buf); + if (err) + goto err_unpin_programs; + } + + return 0; + +err_unpin_programs: + while ((prog = bpf_object__prev_program(obj, prog))) { + if (pathname_concat(path, bpf_program__name(prog), buf)) + continue; + + bpf_program__unpin(prog, buf); + } + + return err; +} + static int load_with_options(int argc, char **argv, bool first_prog_only) { enum bpf_prog_type common_prog_type = BPF_PROG_TYPE_UNSPEC; @@ -1464,6 +1527,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) struct bpf_program *prog = NULL, *pos; unsigned int old_map_fds = 0; const char *pinmaps = NULL; + bool auto_attach = false; struct bpf_object *obj; struct bpf_map *map; const char *pinfile; @@ -1583,6 +1647,9 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) goto err_free_reuse_maps; pinmaps = GET_ARG(); + } else if (is_prefix(*argv, "autoattach")) { + auto_attach = true; + NEXT_ARG(); } else { p_err("expected no more arguments, 'type', 'map' or 'dev', got: '%s'?", *argv); @@ -1692,14 +1759,20 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) goto err_close_obj; } - err = bpf_obj_pin(bpf_program__fd(prog), pinfile); + if (auto_attach) + err = auto_attach_program(prog, pinfile); + else + err = bpf_obj_pin(bpf_program__fd(prog), pinfile); if (err) { p_err("failed to pin program %s", bpf_program__section_name(prog)); goto err_close_obj; } } else { - err = bpf_object__pin_programs(obj, pinfile); + if (auto_attach) + err = auto_attach_programs(obj, pinfile); + else + err = bpf_object__pin_programs(obj, pinfile); if (err) { p_err("failed to pin all programs"); goto err_close_obj; @@ -2338,6 +2411,7 @@ static int do_help(int argc, char **argv) " [type TYPE] [dev NAME] \\\n" " [map { idx IDX | name NAME } MAP]\\\n" " [pinmaps MAP_DIR]\n" + " [autoattach]\n" " %1$s %2$s attach PROG ATTACH_TYPE [MAP]\n" " %1$s %2$s detach PROG ATTACH_TYPE [MAP]\n" " %1$s %2$s run PROG \\\n" -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [bpf-next v8 1/3] bpftool: Add autoattach for bpf prog load|loadall 2022-10-10 10:59 ` [bpf-next v8 1/3] " Wang Yufen @ 2022-10-13 19:47 ` Andrii Nakryiko 2022-10-14 2:24 ` wangyufen 0 siblings, 1 reply; 9+ messages in thread From: Andrii Nakryiko @ 2022-10-13 19:47 UTC (permalink / raw) To: Wang Yufen Cc: quentin, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa, davem, kuba, hawk, nathan, ndesaulniers, trix, bpf, netdev, llvm On Mon, Oct 10, 2022 at 3:40 AM Wang Yufen <wangyufen@huawei.com> wrote: > > Add autoattach optional to support one-step load-attach-pin_link. > > For example, > $ bpftool prog loadall test.o /sys/fs/bpf/test autoattach > > $ bpftool link > 26: tracing name test1 tag f0da7d0058c00236 gpl > loaded_at 2022-09-09T21:39:49+0800 uid 0 > xlated 88B jited 55B memlock 4096B map_ids 3 > btf_id 55 > 28: kprobe name test3 tag 002ef1bef0723833 gpl > loaded_at 2022-09-09T21:39:49+0800 uid 0 > xlated 88B jited 56B memlock 4096B map_ids 3 > btf_id 55 > 57: tracepoint name oncpu tag 7aa55dfbdcb78941 gpl > loaded_at 2022-09-09T21:41:32+0800 uid 0 > xlated 456B jited 265B memlock 4096B map_ids 17,13,14,15 > btf_id 82 > > $ bpftool link > 1: tracing prog 26 > prog_type tracing attach_type trace_fentry > 3: perf_event prog 28 > 10: perf_event prog 57 > > The autoattach optional can support tracepoints, k(ret)probes, > u(ret)probes. > > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > Signed-off-by: Wang Yufen <wangyufen@huawei.com> > --- > tools/bpf/bpftool/prog.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 76 insertions(+), 2 deletions(-) > > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c > index c81362a..8f3afce 100644 > --- a/tools/bpf/bpftool/prog.c > +++ b/tools/bpf/bpftool/prog.c > @@ -1453,6 +1453,69 @@ static int do_run(int argc, char **argv) > return ret; > } > > +static int > +auto_attach_program(struct bpf_program *prog, const char *path) > +{ > + struct bpf_link *link; > + int err; > + > + link = bpf_program__attach(prog); > + if (!link) { > + p_info("Program %s does not support autoattach, falling back to pinning", > + bpf_program__name(prog)); > + return bpf_obj_pin(bpf_program__fd(prog), path); > + } > + err = bpf_link__pin(link, path); > + if (err) { > + bpf_link__destroy(link); > + return err; > + } leaking link here, destroy it unconditionally. If pinning succeeded, you don't need to hold link's FD open anymore. > + return 0; > +} > + > +static int pathname_concat(const char *path, const char *name, char *buf) why you didn't do the same as in libbpf? Pass buffer size explicitly instead of assuming PATH_MAX > +{ > + int len; > + > + len = snprintf(buf, PATH_MAX, "%s/%s", path, name); > + if (len < 0) > + return -EINVAL; > + if (len >= PATH_MAX) > + return -ENAMETOOLONG; > + > + return 0; > +} > + > +static int > +auto_attach_programs(struct bpf_object *obj, const char *path) > +{ > + struct bpf_program *prog; > + char buf[PATH_MAX]; > + int err; > + > + bpf_object__for_each_program(prog, obj) { > + err = pathname_concat(path, bpf_program__name(prog), buf); > + if (err) > + goto err_unpin_programs; > + > + err = auto_attach_program(prog, buf); > + if (err) > + goto err_unpin_programs; > + } > + > + return 0; > + > +err_unpin_programs: > + while ((prog = bpf_object__prev_program(obj, prog))) { > + if (pathname_concat(path, bpf_program__name(prog), buf)) > + continue; > + > + bpf_program__unpin(prog, buf); > + } > + > + return err; > +} > + > static int load_with_options(int argc, char **argv, bool first_prog_only) > { > enum bpf_prog_type common_prog_type = BPF_PROG_TYPE_UNSPEC; > @@ -1464,6 +1527,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > struct bpf_program *prog = NULL, *pos; > unsigned int old_map_fds = 0; > const char *pinmaps = NULL; > + bool auto_attach = false; > struct bpf_object *obj; > struct bpf_map *map; > const char *pinfile; > @@ -1583,6 +1647,9 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > goto err_free_reuse_maps; > > pinmaps = GET_ARG(); > + } else if (is_prefix(*argv, "autoattach")) { > + auto_attach = true; > + NEXT_ARG(); > } else { > p_err("expected no more arguments, 'type', 'map' or 'dev', got: '%s'?", > *argv); > @@ -1692,14 +1759,20 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > goto err_close_obj; > } > > - err = bpf_obj_pin(bpf_program__fd(prog), pinfile); > + if (auto_attach) > + err = auto_attach_program(prog, pinfile); > + else > + err = bpf_obj_pin(bpf_program__fd(prog), pinfile); > if (err) { > p_err("failed to pin program %s", > bpf_program__section_name(prog)); > goto err_close_obj; > } > } else { > - err = bpf_object__pin_programs(obj, pinfile); > + if (auto_attach) > + err = auto_attach_programs(obj, pinfile); > + else > + err = bpf_object__pin_programs(obj, pinfile); > if (err) { > p_err("failed to pin all programs"); > goto err_close_obj; > @@ -2338,6 +2411,7 @@ static int do_help(int argc, char **argv) > " [type TYPE] [dev NAME] \\\n" > " [map { idx IDX | name NAME } MAP]\\\n" > " [pinmaps MAP_DIR]\n" > + " [autoattach]\n" > " %1$s %2$s attach PROG ATTACH_TYPE [MAP]\n" > " %1$s %2$s detach PROG ATTACH_TYPE [MAP]\n" > " %1$s %2$s run PROG \\\n" > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [bpf-next v8 1/3] bpftool: Add autoattach for bpf prog load|loadall 2022-10-13 19:47 ` Andrii Nakryiko @ 2022-10-14 2:24 ` wangyufen 2022-10-14 3:43 ` Andrii Nakryiko 0 siblings, 1 reply; 9+ messages in thread From: wangyufen @ 2022-10-14 2:24 UTC (permalink / raw) To: Andrii Nakryiko Cc: quentin, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa, davem, kuba, hawk, nathan, ndesaulniers, trix, bpf, netdev, llvm 在 2022/10/14 3:47, Andrii Nakryiko 写道: > On Mon, Oct 10, 2022 at 3:40 AM Wang Yufen <wangyufen@huawei.com> wrote: >> Add autoattach optional to support one-step load-attach-pin_link. >> >> For example, >> $ bpftool prog loadall test.o /sys/fs/bpf/test autoattach >> >> $ bpftool link >> 26: tracing name test1 tag f0da7d0058c00236 gpl >> loaded_at 2022-09-09T21:39:49+0800 uid 0 >> xlated 88B jited 55B memlock 4096B map_ids 3 >> btf_id 55 >> 28: kprobe name test3 tag 002ef1bef0723833 gpl >> loaded_at 2022-09-09T21:39:49+0800 uid 0 >> xlated 88B jited 56B memlock 4096B map_ids 3 >> btf_id 55 >> 57: tracepoint name oncpu tag 7aa55dfbdcb78941 gpl >> loaded_at 2022-09-09T21:41:32+0800 uid 0 >> xlated 456B jited 265B memlock 4096B map_ids 17,13,14,15 >> btf_id 82 >> >> $ bpftool link >> 1: tracing prog 26 >> prog_type tracing attach_type trace_fentry >> 3: perf_event prog 28 >> 10: perf_event prog 57 >> >> The autoattach optional can support tracepoints, k(ret)probes, >> u(ret)probes. >> >> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> >> Signed-off-by: Wang Yufen <wangyufen@huawei.com> >> --- >> tools/bpf/bpftool/prog.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 76 insertions(+), 2 deletions(-) >> >> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c >> index c81362a..8f3afce 100644 >> --- a/tools/bpf/bpftool/prog.c >> +++ b/tools/bpf/bpftool/prog.c >> @@ -1453,6 +1453,69 @@ static int do_run(int argc, char **argv) >> return ret; >> } >> >> +static int >> +auto_attach_program(struct bpf_program *prog, const char *path) >> +{ >> + struct bpf_link *link; >> + int err; >> + >> + link = bpf_program__attach(prog); >> + if (!link) { >> + p_info("Program %s does not support autoattach, falling back to pinning", >> + bpf_program__name(prog)); >> + return bpf_obj_pin(bpf_program__fd(prog), path); >> + } >> + err = bpf_link__pin(link, path); >> + if (err) { >> + bpf_link__destroy(link); >> + return err; >> + } > leaking link here, destroy it unconditionally. If pinning succeeded, > you don't need to hold link's FD open anymore. I got it, will change. Thanks！ > >> + return 0; >> +} >> + >> +static int pathname_concat(const char *path, const char *name, char *buf) > why you didn't do the same as in libbpf? Pass buffer size explicitly > instead of assuming PATH_MAX The pathname_concat function is invoked only in one place and is not a general function here. So, not modified. Or, can I change the "pathname_concat" of libbpf to "LIBBPF_API int libbpf_pathname_concat" and use the libbpf_pathname_concat directly? > >> +{ >> + int len; >> + >> + len = snprintf(buf, PATH_MAX, "%s/%s", path, name); >> + if (len < 0) >> + return -EINVAL; >> + if (len >= PATH_MAX) >> + return -ENAMETOOLONG; >> + >> + return 0; >> +} >> + >> +static int >> +auto_attach_programs(struct bpf_object *obj, const char *path) >> +{ >> + struct bpf_program *prog; >> + char buf[PATH_MAX]; >> + int err; >> + >> + bpf_object__for_each_program(prog, obj) { >> + err = pathname_concat(path, bpf_program__name(prog), buf); >> + if (err) >> + goto err_unpin_programs; >> + >> + err = auto_attach_program(prog, buf); >> + if (err) >> + goto err_unpin_programs; >> + } >> + >> + return 0; >> + >> +err_unpin_programs: >> + while ((prog = bpf_object__prev_program(obj, prog))) { >> + if (pathname_concat(path, bpf_program__name(prog), buf)) >> + continue; >> + >> + bpf_program__unpin(prog, buf); >> + } >> + >> + return err; >> +} >> + >> static int load_with_options(int argc, char **argv, bool first_prog_only) >> { >> enum bpf_prog_type common_prog_type = BPF_PROG_TYPE_UNSPEC; >> @@ -1464,6 +1527,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) >> struct bpf_program *prog = NULL, *pos; >> unsigned int old_map_fds = 0; >> const char *pinmaps = NULL; >> + bool auto_attach = false; >> struct bpf_object *obj; >> struct bpf_map *map; >> const char *pinfile; >> @@ -1583,6 +1647,9 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) >> goto err_free_reuse_maps; >> >> pinmaps = GET_ARG(); >> + } else if (is_prefix(*argv, "autoattach")) { >> + auto_attach = true; >> + NEXT_ARG(); >> } else { >> p_err("expected no more arguments, 'type', 'map' or 'dev', got: '%s'?", >> *argv); >> @@ -1692,14 +1759,20 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) >> goto err_close_obj; >> } >> >> - err = bpf_obj_pin(bpf_program__fd(prog), pinfile); >> + if (auto_attach) >> + err = auto_attach_program(prog, pinfile); >> + else >> + err = bpf_obj_pin(bpf_program__fd(prog), pinfile); >> if (err) { >> p_err("failed to pin program %s", >> bpf_program__section_name(prog)); >> goto err_close_obj; >> } >> } else { >> - err = bpf_object__pin_programs(obj, pinfile); >> + if (auto_attach) >> + err = auto_attach_programs(obj, pinfile); >> + else >> + err = bpf_object__pin_programs(obj, pinfile); >> if (err) { >> p_err("failed to pin all programs"); >> goto err_close_obj; >> @@ -2338,6 +2411,7 @@ static int do_help(int argc, char **argv) >> " [type TYPE] [dev NAME] \\\n" >> " [map { idx IDX | name NAME } MAP]\\\n" >> " [pinmaps MAP_DIR]\n" >> + " [autoattach]\n" >> " %1$s %2$s attach PROG ATTACH_TYPE [MAP]\n" >> " %1$s %2$s detach PROG ATTACH_TYPE [MAP]\n" >> " %1$s %2$s run PROG \\\n" >> -- >> 1.8.3.1 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [bpf-next v8 1/3] bpftool: Add autoattach for bpf prog load|loadall 2022-10-14 2:24 ` wangyufen @ 2022-10-14 3:43 ` Andrii Nakryiko 2022-10-14 6:38 ` wangyufen 0 siblings, 1 reply; 9+ messages in thread From: Andrii Nakryiko @ 2022-10-14 3:43 UTC (permalink / raw) To: wangyufen Cc: quentin, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa, davem, kuba, hawk, nathan, ndesaulniers, trix, bpf, netdev, llvm On Thu, Oct 13, 2022 at 7:24 PM wangyufen <wangyufen@huawei.com> wrote: > > > 在 2022/10/14 3:47, Andrii Nakryiko 写道: > > On Mon, Oct 10, 2022 at 3:40 AM Wang Yufen <wangyufen@huawei.com> wrote: > >> Add autoattach optional to support one-step load-attach-pin_link. > >> > >> For example, > >> $ bpftool prog loadall test.o /sys/fs/bpf/test autoattach > >> > >> $ bpftool link > >> 26: tracing name test1 tag f0da7d0058c00236 gpl > >> loaded_at 2022-09-09T21:39:49+0800 uid 0 > >> xlated 88B jited 55B memlock 4096B map_ids 3 > >> btf_id 55 > >> 28: kprobe name test3 tag 002ef1bef0723833 gpl > >> loaded_at 2022-09-09T21:39:49+0800 uid 0 > >> xlated 88B jited 56B memlock 4096B map_ids 3 > >> btf_id 55 > >> 57: tracepoint name oncpu tag 7aa55dfbdcb78941 gpl > >> loaded_at 2022-09-09T21:41:32+0800 uid 0 > >> xlated 456B jited 265B memlock 4096B map_ids 17,13,14,15 > >> btf_id 82 > >> > >> $ bpftool link > >> 1: tracing prog 26 > >> prog_type tracing attach_type trace_fentry > >> 3: perf_event prog 28 > >> 10: perf_event prog 57 > >> > >> The autoattach optional can support tracepoints, k(ret)probes, > >> u(ret)probes. > >> > >> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > >> Signed-off-by: Wang Yufen <wangyufen@huawei.com> > >> --- > >> tools/bpf/bpftool/prog.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++-- > >> 1 file changed, 76 insertions(+), 2 deletions(-) > >> > >> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c > >> index c81362a..8f3afce 100644 > >> --- a/tools/bpf/bpftool/prog.c > >> +++ b/tools/bpf/bpftool/prog.c > >> @@ -1453,6 +1453,69 @@ static int do_run(int argc, char **argv) > >> return ret; > >> } > >> > >> +static int > >> +auto_attach_program(struct bpf_program *prog, const char *path) > >> +{ > >> + struct bpf_link *link; > >> + int err; > >> + > >> + link = bpf_program__attach(prog); > >> + if (!link) { > >> + p_info("Program %s does not support autoattach, falling back to pinning", > >> + bpf_program__name(prog)); > >> + return bpf_obj_pin(bpf_program__fd(prog), path); > >> + } > >> + err = bpf_link__pin(link, path); > >> + if (err) { > >> + bpf_link__destroy(link); > >> + return err; > >> + } > > leaking link here, destroy it unconditionally. If pinning succeeded, > > you don't need to hold link's FD open anymore. > > I got it, will change. Thanks！ > > > > >> + return 0; > >> +} > >> + > >> +static int pathname_concat(const char *path, const char *name, char *buf) > > why you didn't do the same as in libbpf? Pass buffer size explicitly > > instead of assuming PATH_MAX > > The pathname_concat function is invoked only in one place and is not a > general function here. So, not modified. Or, can I change the "pathname_concat" > of libbpf to "LIBBPF_API int libbpf_pathname_concat" and use the > libbpf_pathname_concat directly? no need to reuse libbpf's helper, but I do think it's cleaner to specify not just buffer pointer, but also it's size, just like you do with any other string buffer API (like snprintf) > > > > > >> +{ > >> + int len; > >> + > >> + len = snprintf(buf, PATH_MAX, "%s/%s", path, name); > >> + if (len < 0) > >> + return -EINVAL; > >> + if (len >= PATH_MAX) > >> + return -ENAMETOOLONG; > >> + > >> + return 0; > >> +} > >> + > >> +static int > >> +auto_attach_programs(struct bpf_object *obj, const char *path) > >> +{ > >> + struct bpf_program *prog; > >> + char buf[PATH_MAX]; > >> + int err; > >> + > >> + bpf_object__for_each_program(prog, obj) { > >> + err = pathname_concat(path, bpf_program__name(prog), buf); > >> + if (err) > >> + goto err_unpin_programs; > >> + > >> + err = auto_attach_program(prog, buf); > >> + if (err) > >> + goto err_unpin_programs; > >> + } > >> + > >> + return 0; > >> + > >> +err_unpin_programs: > >> + while ((prog = bpf_object__prev_program(obj, prog))) { > >> + if (pathname_concat(path, bpf_program__name(prog), buf)) > >> + continue; > >> + > >> + bpf_program__unpin(prog, buf); > >> + } > >> + > >> + return err; > >> +} > >> + > >> static int load_with_options(int argc, char **argv, bool first_prog_only) > >> { > >> enum bpf_prog_type common_prog_type = BPF_PROG_TYPE_UNSPEC; > >> @@ -1464,6 +1527,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > >> struct bpf_program *prog = NULL, *pos; > >> unsigned int old_map_fds = 0; > >> const char *pinmaps = NULL; > >> + bool auto_attach = false; > >> struct bpf_object *obj; > >> struct bpf_map *map; > >> const char *pinfile; > >> @@ -1583,6 +1647,9 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > >> goto err_free_reuse_maps; > >> > >> pinmaps = GET_ARG(); > >> + } else if (is_prefix(*argv, "autoattach")) { > >> + auto_attach = true; > >> + NEXT_ARG(); > >> } else { > >> p_err("expected no more arguments, 'type', 'map' or 'dev', got: '%s'?", > >> *argv); > >> @@ -1692,14 +1759,20 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) > >> goto err_close_obj; > >> } > >> > >> - err = bpf_obj_pin(bpf_program__fd(prog), pinfile); > >> + if (auto_attach) > >> + err = auto_attach_program(prog, pinfile); > >> + else > >> + err = bpf_obj_pin(bpf_program__fd(prog), pinfile); > >> if (err) { > >> p_err("failed to pin program %s", > >> bpf_program__section_name(prog)); > >> goto err_close_obj; > >> } > >> } else { > >> - err = bpf_object__pin_programs(obj, pinfile); > >> + if (auto_attach) > >> + err = auto_attach_programs(obj, pinfile); > >> + else > >> + err = bpf_object__pin_programs(obj, pinfile); > >> if (err) { > >> p_err("failed to pin all programs"); > >> goto err_close_obj; > >> @@ -2338,6 +2411,7 @@ static int do_help(int argc, char **argv) > >> " [type TYPE] [dev NAME] \\\n" > >> " [map { idx IDX | name NAME } MAP]\\\n" > >> " [pinmaps MAP_DIR]\n" > >> + " [autoattach]\n" > >> " %1$s %2$s attach PROG ATTACH_TYPE [MAP]\n" > >> " %1$s %2$s detach PROG ATTACH_TYPE [MAP]\n" > >> " %1$s %2$s run PROG \\\n" > >> -- > >> 1.8.3.1 > >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [bpf-next v8 1/3] bpftool: Add autoattach for bpf prog load|loadall 2022-10-14 3:43 ` Andrii Nakryiko @ 2022-10-14 6:38 ` wangyufen 0 siblings, 0 replies; 9+ messages in thread From: wangyufen @ 2022-10-14 6:38 UTC (permalink / raw) To: Andrii Nakryiko Cc: quentin, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa, davem, kuba, hawk, nathan, ndesaulniers, trix, bpf, netdev, llvm 在 2022/10/14 11:43, Andrii Nakryiko 写道: > On Thu, Oct 13, 2022 at 7:24 PM wangyufen <wangyufen@huawei.com> wrote: >> >> 在 2022/10/14 3:47, Andrii Nakryiko 写道: >>> On Mon, Oct 10, 2022 at 3:40 AM Wang Yufen <wangyufen@huawei.com> wrote: >>>> Add autoattach optional to support one-step load-attach-pin_link. >>>> >>>> For example, >>>> $ bpftool prog loadall test.o /sys/fs/bpf/test autoattach >>>> >>>> $ bpftool link >>>> 26: tracing name test1 tag f0da7d0058c00236 gpl >>>> loaded_at 2022-09-09T21:39:49+0800 uid 0 >>>> xlated 88B jited 55B memlock 4096B map_ids 3 >>>> btf_id 55 >>>> 28: kprobe name test3 tag 002ef1bef0723833 gpl >>>> loaded_at 2022-09-09T21:39:49+0800 uid 0 >>>> xlated 88B jited 56B memlock 4096B map_ids 3 >>>> btf_id 55 >>>> 57: tracepoint name oncpu tag 7aa55dfbdcb78941 gpl >>>> loaded_at 2022-09-09T21:41:32+0800 uid 0 >>>> xlated 456B jited 265B memlock 4096B map_ids 17,13,14,15 >>>> btf_id 82 >>>> >>>> $ bpftool link >>>> 1: tracing prog 26 >>>> prog_type tracing attach_type trace_fentry >>>> 3: perf_event prog 28 >>>> 10: perf_event prog 57 >>>> >>>> The autoattach optional can support tracepoints, k(ret)probes, >>>> u(ret)probes. >>>> >>>> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> >>>> Signed-off-by: Wang Yufen <wangyufen@huawei.com> >>>> --- >>>> tools/bpf/bpftool/prog.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++-- >>>> 1 file changed, 76 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c >>>> index c81362a..8f3afce 100644 >>>> --- a/tools/bpf/bpftool/prog.c >>>> +++ b/tools/bpf/bpftool/prog.c >>>> @@ -1453,6 +1453,69 @@ static int do_run(int argc, char **argv) >>>> return ret; >>>> } >>>> >>>> +static int >>>> +auto_attach_program(struct bpf_program *prog, const char *path) >>>> +{ >>>> + struct bpf_link *link; >>>> + int err; >>>> + >>>> + link = bpf_program__attach(prog); >>>> + if (!link) { >>>> + p_info("Program %s does not support autoattach, falling back to pinning", >>>> + bpf_program__name(prog)); >>>> + return bpf_obj_pin(bpf_program__fd(prog), path); >>>> + } >>>> + err = bpf_link__pin(link, path); >>>> + if (err) { >>>> + bpf_link__destroy(link); >>>> + return err; >>>> + } >>> leaking link here, destroy it unconditionally. If pinning succeeded, >>> you don't need to hold link's FD open anymore. >> I got it, will change. Thanks！ >> >>>> + return 0; >>>> +} >>>> + >>>> +static int pathname_concat(const char *path, const char *name, char *buf) >>> why you didn't do the same as in libbpf? Pass buffer size explicitly >>> instead of assuming PATH_MAX >> The pathname_concat function is invoked only in one place and is not a >> general function here. So, not modified. Or, can I change the "pathname_concat" >> of libbpf to "LIBBPF_API int libbpf_pathname_concat" and use the >> libbpf_pathname_concat directly? > no need to reuse libbpf's helper, but I do think it's cleaner to > specify not just buffer pointer, but also it's size, just like you do > with any other string buffer API (like snprintf) OK, will change in v9 too. Thanks！ > >> >>>> +{ >>>> + int len; >>>> + >>>> + len = snprintf(buf, PATH_MAX, "%s/%s", path, name); >>>> + if (len < 0) >>>> + return -EINVAL; >>>> + if (len >= PATH_MAX) >>>> + return -ENAMETOOLONG; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int >>>> +auto_attach_programs(struct bpf_object *obj, const char *path) >>>> +{ >>>> + struct bpf_program *prog; >>>> + char buf[PATH_MAX]; >>>> + int err; >>>> + >>>> + bpf_object__for_each_program(prog, obj) { >>>> + err = pathname_concat(path, bpf_program__name(prog), buf); >>>> + if (err) >>>> + goto err_unpin_programs; >>>> + >>>> + err = auto_attach_program(prog, buf); >>>> + if (err) >>>> + goto err_unpin_programs; >>>> + } >>>> + >>>> + return 0; >>>> + >>>> +err_unpin_programs: >>>> + while ((prog = bpf_object__prev_program(obj, prog))) { >>>> + if (pathname_concat(path, bpf_program__name(prog), buf)) >>>> + continue; >>>> + >>>> + bpf_program__unpin(prog, buf); >>>> + } >>>> + >>>> + return err; >>>> +} >>>> + >>>> static int load_with_options(int argc, char **argv, bool first_prog_only) >>>> { >>>> enum bpf_prog_type common_prog_type = BPF_PROG_TYPE_UNSPEC; >>>> @@ -1464,6 +1527,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) >>>> struct bpf_program *prog = NULL, *pos; >>>> unsigned int old_map_fds = 0; >>>> const char *pinmaps = NULL; >>>> + bool auto_attach = false; >>>> struct bpf_object *obj; >>>> struct bpf_map *map; >>>> const char *pinfile; >>>> @@ -1583,6 +1647,9 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) >>>> goto err_free_reuse_maps; >>>> >>>> pinmaps = GET_ARG(); >>>> + } else if (is_prefix(*argv, "autoattach")) { >>>> + auto_attach = true; >>>> + NEXT_ARG(); >>>> } else { >>>> p_err("expected no more arguments, 'type', 'map' or 'dev', got: '%s'?", >>>> *argv); >>>> @@ -1692,14 +1759,20 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) >>>> goto err_close_obj; >>>> } >>>> >>>> - err = bpf_obj_pin(bpf_program__fd(prog), pinfile); >>>> + if (auto_attach) >>>> + err = auto_attach_program(prog, pinfile); >>>> + else >>>> + err = bpf_obj_pin(bpf_program__fd(prog), pinfile); >>>> if (err) { >>>> p_err("failed to pin program %s", >>>> bpf_program__section_name(prog)); >>>> goto err_close_obj; >>>> } >>>> } else { >>>> - err = bpf_object__pin_programs(obj, pinfile); >>>> + if (auto_attach) >>>> + err = auto_attach_programs(obj, pinfile); >>>> + else >>>> + err = bpf_object__pin_programs(obj, pinfile); >>>> if (err) { >>>> p_err("failed to pin all programs"); >>>> goto err_close_obj; >>>> @@ -2338,6 +2411,7 @@ static int do_help(int argc, char **argv) >>>> " [type TYPE] [dev NAME] \\\n" >>>> " [map { idx IDX | name NAME } MAP]\\\n" >>>> " [pinmaps MAP_DIR]\n" >>>> + " [autoattach]\n" >>>> " %1$s %2$s attach PROG ATTACH_TYPE [MAP]\n" >>>> " %1$s %2$s detach PROG ATTACH_TYPE [MAP]\n" >>>> " %1$s %2$s run PROG \\\n" >>>> -- >>>> 1.8.3.1 >>>> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [bpf-next v8 2/3] bpftool: Update doc (add autoattach to prog load) 2022-10-10 10:59 [bpf-next v8 0/3] bpftool: Add autoattach for bpf prog load|loadall Wang Yufen 2022-10-10 10:59 ` [bpf-next v8 1/3] " Wang Yufen @ 2022-10-10 11:00 ` Wang Yufen 2022-10-10 11:00 ` [bpf-next v8 3/3] bpftool: Update the bash completion(add " Wang Yufen 2022-10-11 16:17 ` [bpf-next v8 0/3] bpftool: Add autoattach for bpf prog load|loadall Quentin Monnet 3 siblings, 0 replies; 9+ messages in thread From: Wang Yufen @ 2022-10-10 11:00 UTC (permalink / raw) To: quentin, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa, davem, kuba, hawk, nathan, ndesaulniers, trix Cc: bpf, netdev, llvm Add autoattach optional to prog load|loadall for supporting one-step load-attach-pin_link. Signed-off-by: Wang Yufen <wangyufen@huawei.com> --- tools/bpf/bpftool/Documentation/bpftool-prog.rst | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst index eb1b2a2..14de725 100644 --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst @@ -31,7 +31,7 @@ PROG COMMANDS | **bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes** | **visual** | **linum**}] | **bpftool** **prog dump jited** *PROG* [{**file** *FILE* | **opcodes** | **linum**}] | **bpftool** **prog pin** *PROG* *FILE* -| **bpftool** **prog** { **load** | **loadall** } *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*] +| **bpftool** **prog** { **load** | **loadall** } *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*] [**autoattach**] | **bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*] | **bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*] | **bpftool** **prog tracelog** @@ -131,7 +131,7 @@ DESCRIPTION contain a dot character ('.'), which is reserved for future extensions of *bpffs*. - **bpftool prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*] + **bpftool prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*] [**autoattach**] Load bpf program(s) from binary *OBJ* and pin as *PATH*. **bpftool prog load** pins only the first program from the *OBJ* as *PATH*. **bpftool prog loadall** pins all programs @@ -150,6 +150,17 @@ DESCRIPTION Optional **pinmaps** argument can be provided to pin all maps under *MAP_DIR* directory. + If **autoattach** is specified program will be attached + before pin. In that case, only the link (representing the + program attached to its hook) is pinned, not the program as + such, so the path won't show in **bpftool prog show -f**, + only show in **bpftool link show -f**. Also, this only works + when bpftool (libbpf) is able to infer all necessary + information from the object file, in particular, it's not + supported for all program types. If a program does not + support autoattach, bpftool falls back to regular pinning + for that program instead. + Note: *PATH* must be located in *bpffs* mount. It must not contain a dot character ('.'), which is reserved for future extensions of *bpffs*. -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [bpf-next v8 3/3] bpftool: Update the bash completion(add autoattach to prog load) 2022-10-10 10:59 [bpf-next v8 0/3] bpftool: Add autoattach for bpf prog load|loadall Wang Yufen 2022-10-10 10:59 ` [bpf-next v8 1/3] " Wang Yufen 2022-10-10 11:00 ` [bpf-next v8 2/3] bpftool: Update doc (add autoattach to prog load) Wang Yufen @ 2022-10-10 11:00 ` Wang Yufen 2022-10-11 16:17 ` [bpf-next v8 0/3] bpftool: Add autoattach for bpf prog load|loadall Quentin Monnet 3 siblings, 0 replies; 9+ messages in thread From: Wang Yufen @ 2022-10-10 11:00 UTC (permalink / raw) To: quentin, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa, davem, kuba, hawk, nathan, ndesaulniers, trix Cc: bpf, netdev, llvm Add autoattach optional to prog load|loadall for supporting one-step load-attach-pin_link. Signed-off-by: Wang Yufen <wangyufen@huawei.com> --- tools/bpf/bpftool/bash-completion/bpftool | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool index dc1641e..2957b42 100644 --- a/tools/bpf/bpftool/bash-completion/bpftool +++ b/tools/bpf/bpftool/bash-completion/bpftool @@ -505,6 +505,7 @@ _bpftool() _bpftool_once_attr 'type' _bpftool_once_attr 'dev' _bpftool_once_attr 'pinmaps' + _bpftool_once_attr 'autoattach' return 0 ;; esac -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [bpf-next v8 0/3] bpftool: Add autoattach for bpf prog load|loadall 2022-10-10 10:59 [bpf-next v8 0/3] bpftool: Add autoattach for bpf prog load|loadall Wang Yufen ` (2 preceding siblings ...) 2022-10-10 11:00 ` [bpf-next v8 3/3] bpftool: Update the bash completion(add " Wang Yufen @ 2022-10-11 16:17 ` Quentin Monnet 3 siblings, 0 replies; 9+ messages in thread From: Quentin Monnet @ 2022-10-11 16:17 UTC (permalink / raw) To: Wang Yufen, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa, davem, kuba, hawk, nathan, ndesaulniers, trix Cc: bpf, netdev, llvm 2022-10-10 18:59 UTC+0800 ~ Wang Yufen <wangyufen@huawei.com> > This patchset add "autoattach" optional for "bpftool prog load(_all)" to support > one-step load-attach-pin_link. > > v7 -> v8: for the programs not supporting autoattach, fall back to reguler pinning > instead of skipping > v6 -> v7: add info msg print and update doc for the skip program > v5 -> v6: skip the programs not supporting auto-attach, > and change optional name from "auto_attach" to "autoattach" > v4 -> v5: some formatting nits of doc > v3 -> v4: rename functions, update doc, bash and do_help() > v2 -> v3: switch to extend prog load command instead of extend perf > v2: https://patchwork.kernel.org/project/netdevbpf/patch/20220824033837.458197-1-weiyongjun1@huawei.com/ > v1: https://patchwork.kernel.org/project/netdevbpf/patch/20220816151725.153343-1-weiyongjun1@huawei.com/ > > Wang Yufen (3): > bpftool: Add autoattach for bpf prog load|loadall > bpftool: Update doc (add autoattach to prog load) > bpftool: Update the bash completion(add autoattach to prog load) > > tools/bpf/bpftool/Documentation/bpftool-prog.rst | 15 ++++- > tools/bpf/bpftool/bash-completion/bpftool | 1 + > tools/bpf/bpftool/prog.c | 78 +++++++++++++++++++++++- > 3 files changed, 90 insertions(+), 4 deletions(-) > The series looks good to me, thank you! Reviewed-by: Quentin Monnet <quentin@isovalent.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-10-14 6:39 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-10-10 10:59 [bpf-next v8 0/3] bpftool: Add autoattach for bpf prog load|loadall Wang Yufen 2022-10-10 10:59 ` [bpf-next v8 1/3] " Wang Yufen 2022-10-13 19:47 ` Andrii Nakryiko 2022-10-14 2:24 ` wangyufen 2022-10-14 3:43 ` Andrii Nakryiko 2022-10-14 6:38 ` wangyufen 2022-10-10 11:00 ` [bpf-next v8 2/3] bpftool: Update doc (add autoattach to prog load) Wang Yufen 2022-10-10 11:00 ` [bpf-next v8 3/3] bpftool: Update the bash completion(add " Wang Yufen 2022-10-11 16:17 ` [bpf-next v8 0/3] bpftool: Add autoattach for bpf prog load|loadall Quentin Monnet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).