| Message ID | 20220816151725.153343-1-weiyongjun1@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] bpftool: Add trace subcommand | expand |
On 16/08/2022 16:17, Wei Yongjun wrote: > Currently, only one command is supported > bpftool trace pin <bpf_prog.o> <path> > > It will pin the trace bpf program in the object file <bpf_prog.o> > to the <path> where <path> should be on a bpffs mount. > > For example, > $ bpftool trace pin ./mtd_mchp23k256.o /sys/fs/bpf/mchp23k256 > > The implementation a BPF based backend for mockup mchp23k256 mtd > SPI device. > > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> Thanks a lot for the patch. However, I don't think it's a good idea to add the new "trace" command just for loading/attaching/pinning tracepoints programs. Instead I see two possible approaches. The first would be to extend support for program attachment. Bpftool is already able to load programs including tracepoints via "bpftool prog load", and it is able to attach some of them via "bpftool prog/net/cgroup attach". We don't support attaching tracing-related programs because at the time, BPF links didn't exist so we couldn't keep the programs running after bpftool exited, and after links were created nobody implemented it. So I would prefer that we extend this, by making bpftool able to attach (and pin the link for) tracing-related programs. Not necessarily just tracepoints by the way, it would be nice to have support for kprobes too. This could be by extending "bpftool prog attach" or creating "bpftool perf attach" ("bpftool perf" already focuses on tracing programs, so no need to add a new "trace" subcommand). Second approach: I realise that the above adds a constraint, because if we attach a program that was already loaded, we can't get the attach point from the section name in the ELF file, we need to pass it on the command line instead. I understand the desire for a one-step load-attach-pin_link, but with your new subcommand it would ignore all the work we've done on "bpftool prog load": support loading multiple programs from an ELF file, for reusing or pinning the maps, etc. So I would rather extend the existing "bpftool prog load(_all)" with a new keyword to tell bpftool to attach and create the link(s), if possible (when all relevant info for attaching is present in the ELF file), after loading the program(s). I'd really have the first approach in bpftool at some point, I haven't found the time to look into it just yet. The second one is probably closer to what you're looking to achieve, and would be nice to have as well. What do you think? Thanks, Quentin
On 2022/8/17 18:14, Quentin Monnet wrote: > On 16/08/2022 16:17, Wei Yongjun wrote: >> Currently, only one command is supported >> bpftool trace pin <bpf_prog.o> <path> >> >> It will pin the trace bpf program in the object file <bpf_prog.o> >> to the <path> where <path> should be on a bpffs mount. >> >> For example, >> $ bpftool trace pin ./mtd_mchp23k256.o /sys/fs/bpf/mchp23k256 >> >> The implementation a BPF based backend for mockup mchp23k256 mtd >> SPI device. >> >> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > Thanks a lot for the patch. > > However, I don't think it's a good idea to add the new "trace" command > just for loading/attaching/pinning tracepoints programs. Instead I see > two possible approaches. > > The first would be to extend support for program attachment. Bpftool is > already able to load programs including tracepoints via "bpftool prog > load", and it is able to attach some of them via "bpftool > prog/net/cgroup attach". We don't support attaching tracing-related > programs because at the time, BPF links didn't exist so we couldn't keep > the programs running after bpftool exited, and after links were created > nobody implemented it. > > So I would prefer that we extend this, by making bpftool able to attach > (and pin the link for) tracing-related programs. Not necessarily just > tracepoints by the way, it would be nice to have support for kprobes > too. This could be by extending "bpftool prog attach" or creating > "bpftool perf attach" ("bpftool perf" already focuses on tracing > programs, so no need to add a new "trace" subcommand). Thanks for your suggestion. I tried to make sense of what you mean. With the first approach , we can attach program with following commands: $ bpftool prog load smbus-xfer-default.o /sys/fs/bpf/testprog $ bpftool prog 302: raw_tracepoint_writable name smbus_xfer_default tag 90c6bea70fef5b14 gpl loaded_at 2022-08-17T14:17:44+0000 uid 0 xlated 816B not jited memlock 4096B map_ids 296,294,297 btf_id 500 $ bpftool perf attach id 302 spi_transfer_writeable /sys/fs/bpf/tplink Note: spi_transfer_writeable is the name of tracepoint > > Second approach: I realise that the above adds a constraint, because if > we attach a program that was already loaded, we can't get the attach > point from the section name in the ELF file, we need to pass it on the > command line instead. I understand the desire for a one-step > load-attach-pin_link, but with your new subcommand it would ignore all > the work we've done on "bpftool prog load": support loading multiple > programs from an ELF file, for reusing or pinning the maps, etc. So I > would rather extend the existing "bpftool prog load(_all)" with a new > keyword to tell bpftool to attach and create the link(s), if possible > (when all relevant info for attaching is present in the ELF file), after > loading the program(s). Or add new keyword like 'pintp' to do one-step load-attach-pin_link: $ bpftool prog load smbus-xfer-default.o /sys/fs/bpf/testprog pintp and $ bpftool prog loadall smbus-xfer-default.o /sys/fs/bpf/testprog pintp > > I'd really have the first approach in bpftool at some point, I haven't > found the time to look into it just yet. The second one is probably > closer to what you're looking to achieve, and would be nice to have as > well. What do you think? I will try to create the "bpftool perf attach" approach, if you agree with it. Thanks, Wei Yongjun
diff --git a/tools/bpf/bpftool/Documentation/bpftool-trace.rst b/tools/bpf/bpftool/Documentation/bpftool-trace.rst new file mode 100644 index 000000000000..d44256f6a021 --- /dev/null +++ b/tools/bpf/bpftool/Documentation/bpftool-trace.rst @@ -0,0 +1,57 @@ +.. SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) + +============ +bpftool-trace +============ +------------------------------------------------------------------------------- +tool to create BPF tracepoints +------------------------------------------------------------------------------- + +:Manual section: 8 + +.. include:: substitutions.rst + +SYNOPSIS +======== + + **bpftool** [*OPTIONS*] **trace** *COMMAND* + + *OPTIONS* := { |COMMON_OPTIONS| } + + *COMMANDS* := { **pin** | **help** } + +ITER COMMANDS +=================== + +| **bpftool** **trace pin** *OBJ* *PATH* +| **bpftool** **trace help** +| +| *OBJ* := /a/file/of/bpf_tp_target.o + +DESCRIPTION +=========== + **bpftool trace pin** *OBJ* *PATH* + A bpf raw tracepoint allows a tracepoint to provide a safe + buffer that can be read or written from a bpf program. + + The *pin* command attaches a bpf raw tracepoint from *OBJ*, + and pin it to *PATH*. The *PATH* should be located + in *bpffs* mount. It must not contain a dot + character ('.'), which is reserved for future extensions + of *bpffs*. + + **bpftool trace help** + Print short help message. + +OPTIONS +======= + .. include:: common_options.rst + +EXAMPLES +======== +**# bpftool trace pin bpf_mtd_chip_mockup.o /sys/fs/bpf/mtd_chip_mockup** + +:: + + Attach to the raw tracepoint from bpf_mtd_chip_mockup.o and pin it + to /sys/fs/bpf/mtd_chip_mockup diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst index 6965c94dfdaf..aae13255a8cb 100644 --- a/tools/bpf/bpftool/Documentation/bpftool.rst +++ b/tools/bpf/bpftool/Documentation/bpftool.rst @@ -21,7 +21,7 @@ SYNOPSIS **bpftool** **version** *OBJECT* := { **map** | **program** | **link** | **cgroup** | **perf** | **net** | **feature** | - **btf** | **gen** | **struct_ops** | **iter** } + **btf** | **gen** | **struct_ops** | **iter** | **trace** } *OPTIONS* := { { **-V** | **--version** } | |COMMON_OPTIONS| } @@ -50,6 +50,8 @@ SYNOPSIS *ITER-COMMANDS* := { **pin** | **help** } + *TRACE-COMMANDS* := { **pin** | **help** } + DESCRIPTION =========== *bpftool* allows for inspection and simple modification of BPF objects diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool index dc1641e3670e..1c0442ed7436 100644 --- a/tools/bpf/bpftool/bash-completion/bpftool +++ b/tools/bpf/bpftool/bash-completion/bpftool @@ -646,6 +646,19 @@ _bpftool() ;; esac ;; + trace) + case $command in + pin) + _filedir + return 0 + ;; + *) + [[ $prev == $object ]] && \ + COMPREPLY=( $( compgen -W 'pin help' \ + -- "$cur" ) ) + ;; + esac + ;; map) local MAP_TYPE='id pinned name' case $command in diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c index ccd7457f92bf..d24373f4e957 100644 --- a/tools/bpf/bpftool/main.c +++ b/tools/bpf/bpftool/main.c @@ -295,6 +295,7 @@ static const struct cmd cmds[] = { { "gen", do_gen }, { "struct_ops", do_struct_ops }, { "iter", do_iter }, + { "trace", do_trace }, { "version", do_version }, { 0 } }; diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h index 5e5060c2ac04..7ff53de4f718 100644 --- a/tools/bpf/bpftool/main.h +++ b/tools/bpf/bpftool/main.h @@ -163,6 +163,7 @@ int do_tracelog(int argc, char **arg) __weak; int do_feature(int argc, char **argv) __weak; int do_struct_ops(int argc, char **argv) __weak; int do_iter(int argc, char **argv) __weak; +int do_trace(int argc, char **arg) __weak; int parse_u32_arg(int *argc, char ***argv, __u32 *val, const char *what); int prog_parse_fd(int *argc, char ***argv); diff --git a/tools/bpf/bpftool/trace.c b/tools/bpf/bpftool/trace.c new file mode 100644 index 000000000000..08dd6e1d8f39 --- /dev/null +++ b/tools/bpf/bpftool/trace.c @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +// Copyright (C) 2022 Huawei Technologies Co., Ltd. + +#include <stdio.h> +#include <unistd.h> +#include <errno.h> +#include <bpf/libbpf.h> + +#include "json_writer.h" +#include "main.h" + +static bool is_trace_program_type(struct bpf_program *prog) +{ + enum bpf_prog_type trace_types[] = { + BPF_PROG_TYPE_RAW_TRACEPOINT, + BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE, + }; + enum bpf_prog_type prog_type; + size_t i; + + prog_type = bpf_program__type(prog); + for (i = 0; i < ARRAY_SIZE(trace_types); i++) { + if (prog_type == trace_types[i]) + return true; + } + + return false; +} + +static int do_pin(int argc, char **argv) +{ + const char *objfile, *path; + struct bpf_program *prog; + struct bpf_object *obj; + struct bpf_link *link; + int err; + + if (!REQ_ARGS(2)) + usage(); + + objfile = GET_ARG(); + path = GET_ARG(); + + obj = bpf_object__open(objfile); + err = libbpf_get_error(obj); + if (err) { + p_err("can't open objfile %s", objfile); + return err; + } + + err = bpf_object__load(obj); + if (err) { + p_err("can't load objfile %s", objfile); + goto close_obj; + } + + prog = bpf_object__next_program(obj, NULL); + if (!prog) { + p_err("can't find bpf program in objfile %s", objfile); + goto close_obj; + } + + if (!is_trace_program_type(prog)) { + p_err("invalid bpf program type"); + err = -EINVAL; + goto close_obj; + } + + link = bpf_program__attach(prog); + err = libbpf_get_error(link); + if (err) { + p_err("can't attach program %s", bpf_program__name(prog)); + goto close_obj; + } + + err = mount_bpffs_for_pin(path); + if (err) + goto close_link; + + err = bpf_link__pin(link, path); + if (err) { + p_err("pin failed for program %s to path %s", + bpf_program__name(prog), path); + goto close_link; + } + +close_link: + bpf_link__destroy(link); +close_obj: + bpf_object__close(obj); + return err; +} + +static int do_help(int argc, char **argv) +{ + if (json_output) { + jsonw_null(json_wtr); + return 0; + } + + fprintf(stderr, + "Usage: %1$s %2$s pin OBJ PATH\n" + " %1$s %2$s help\n" + "\n" + "", + bin_name, "trace"); + + return 0; +} + +static const struct cmd cmds[] = { + { "help", do_help }, + { "pin", do_pin }, + { 0 } +}; + +int do_trace(int argc, char **argv) +{ + return cmd_select(cmds, argc, argv, do_help); +}
Currently, only one command is supported bpftool trace pin <bpf_prog.o> <path> It will pin the trace bpf program in the object file <bpf_prog.o> to the <path> where <path> should be on a bpffs mount. For example, $ bpftool trace pin ./mtd_mchp23k256.o /sys/fs/bpf/mchp23k256 The implementation a BPF based backend for mockup mchp23k256 mtd SPI device. Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> --- .../bpftool/Documentation/bpftool-trace.rst | 57 +++++++++ tools/bpf/bpftool/Documentation/bpftool.rst | 4 +- tools/bpf/bpftool/bash-completion/bpftool | 13 ++ tools/bpf/bpftool/main.c | 1 + tools/bpf/bpftool/main.h | 1 + tools/bpf/bpftool/trace.c | 120 ++++++++++++++++++ 6 files changed, 195 insertions(+), 1 deletion(-)