From 9fd496848b1c4cd04fee5f152bb7bcec11e1b901 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Wed, 5 Apr 2023 14:21:16 +0100 Subject: bpftool: Support inline annotations when dumping the CFG of a program We support dumping the control flow graph of loaded programs to the DOT format with bpftool, but so far this feature wouldn't display the source code lines available through BTF along with the eBPF bytecode. Let's add support for these annotations, to make it easier to read the graph. In prog.c, we move the call to dump_xlated_cfg() in order to pass and use the full struct dump_data, instead of creating a minimal one in draw_bb_node(). We pass the pointer to this struct down to dump_xlated_for_graph() in xlated_dumper.c, where most of the logics is added. We deal with BTF mostly like we do for plain or JSON output, except that we cannot use a "nr_skip" value to skip a given number of linfo records (we don't process the BPF instructions linearly, and apart from the root of the graph we don't know how many records we should skip, so we just store the last linfo and make sure the new one we find is different before printing it). When printing the source instructions to the label of a DOT graph node, there are a few subtleties to address. We want some special newline markers, and there are some characters that we must escape. To deal with them, we introduce a new dedicated function btf_dump_linfo_dotlabel() in btf_dumper.c. We'll reuse this function in a later commit to format the filepath, line, and column references as well. Signed-off-by: Quentin Monnet Link: https://lore.kernel.org/r/20230405132120.59886-4-quentin@isovalent.com Signed-off-by: Alexei Starovoitov --- tools/bpf/bpftool/prog.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'tools/bpf/bpftool/prog.c') diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index afbe3ec342c8..d855118f0d96 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -840,11 +840,6 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode, false)) goto exit_free; } - } else if (visual) { - if (json_output) - jsonw_null(json_wtr); - else - dump_xlated_cfg(buf, member_len); } else { kernel_syms_load(&dd); dd.nr_jited_ksyms = info->nr_jited_ksyms; @@ -854,12 +849,14 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode, dd.finfo_rec_size = info->func_info_rec_size; dd.prog_linfo = prog_linfo; - if (json_output) - dump_xlated_json(&dd, buf, member_len, opcodes, - linum); + if (json_output && visual) + jsonw_null(json_wtr); + else if (json_output) + dump_xlated_json(&dd, buf, member_len, opcodes, linum); + else if (visual) + dump_xlated_cfg(&dd, buf, member_len); else - dump_xlated_plain(&dd, buf, member_len, opcodes, - linum); + dump_xlated_plain(&dd, buf, member_len, opcodes, linum); kernel_syms_destroy(&dd); } -- cgit From 05a06be722896e51f65dbbb6a3610f85a8353d6b Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Wed, 5 Apr 2023 14:21:17 +0100 Subject: bpftool: Return an error on prog dumps if both CFG and JSON are required We do not support JSON output for control flow graphs of programs with bpftool. So far, requiring both the CFG and JSON output would result in producing a null JSON object. It makes more sense to raise an error directly when parsing command line arguments and options, so that users know they won't get any output they might expect. If JSON is required for the graph, we leave it to Graphviz instead: # bpftool prog dump xlated visual | dot -Tjson Suggested-by: Eduard Zingerman Signed-off-by: Quentin Monnet Link: https://lore.kernel.org/r/20230405132120.59886-5-quentin@isovalent.com Signed-off-by: Alexei Starovoitov --- tools/bpf/bpftool/prog.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'tools/bpf/bpftool/prog.c') diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index d855118f0d96..736defc6e5d0 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -849,9 +849,7 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode, dd.finfo_rec_size = info->func_info_rec_size; dd.prog_linfo = prog_linfo; - if (json_output && visual) - jsonw_null(json_wtr); - else if (json_output) + if (json_output) dump_xlated_json(&dd, buf, member_len, opcodes, linum); else if (visual) dump_xlated_cfg(&dd, buf, member_len); @@ -940,6 +938,10 @@ static int do_dump(int argc, char **argv) usage(); goto exit_close; } + if (json_output && visual) { + p_err("'visual' is not compatible with JSON output"); + goto exit_close; + } if (json_output && nb_fds > 1) jsonw_start_array(json_wtr); /* root array */ -- cgit From 9b79f02722bbf24f060b2ab79513ad6e22c8e2f0 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Wed, 5 Apr 2023 14:21:18 +0100 Subject: bpftool: Support "opcodes", "linum", "visual" simultaneously When dumping a program, the keywords "opcodes" (for printing the raw opcodes), "linum" (for displaying the filename, line number, column number along with the source code), and "visual" (for generating the control flow graph for translated programs) are mutually exclusive. But there's no reason why they should be. Let's make it possible to pass several of them at once. The "file FILE" option, which makes bpftool output a binary image to a file, remains incompatible with the others. Signed-off-by: Quentin Monnet Link: https://lore.kernel.org/r/20230405132120.59886-6-quentin@isovalent.com Signed-off-by: Alexei Starovoitov --- tools/bpf/bpftool/prog.c | 61 ++++++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 28 deletions(-) (limited to 'tools/bpf/bpftool/prog.c') diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index 736defc6e5d0..092525a6933a 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -905,37 +905,42 @@ static int do_dump(int argc, char **argv) if (nb_fds < 1) goto exit_free; - if (is_prefix(*argv, "file")) { - NEXT_ARG(); - if (!argc) { - p_err("expected file path"); - goto exit_close; - } - if (nb_fds > 1) { - p_err("several programs matched"); - goto exit_close; - } + while (argc) { + if (is_prefix(*argv, "file")) { + NEXT_ARG(); + if (!argc) { + p_err("expected file path"); + goto exit_close; + } + if (nb_fds > 1) { + p_err("several programs matched"); + goto exit_close; + } - filepath = *argv; - NEXT_ARG(); - } else if (is_prefix(*argv, "opcodes")) { - opcodes = true; - NEXT_ARG(); - } else if (is_prefix(*argv, "visual")) { - if (nb_fds > 1) { - p_err("several programs matched"); + filepath = *argv; + NEXT_ARG(); + } else if (is_prefix(*argv, "opcodes")) { + opcodes = true; + NEXT_ARG(); + } else if (is_prefix(*argv, "visual")) { + if (nb_fds > 1) { + p_err("several programs matched"); + goto exit_close; + } + + visual = true; + NEXT_ARG(); + } else if (is_prefix(*argv, "linum")) { + linum = true; + NEXT_ARG(); + } else { + usage(); goto exit_close; } - - visual = true; - NEXT_ARG(); - } else if (is_prefix(*argv, "linum")) { - linum = true; - NEXT_ARG(); } - if (argc) { - usage(); + if (filepath && (opcodes || visual || linum)) { + p_err("'file' is not compatible with 'opcodes', 'visual', or 'linum'"); goto exit_close; } if (json_output && visual) { @@ -2419,8 +2424,8 @@ static int do_help(int argc, char **argv) fprintf(stderr, "Usage: %1$s %2$s { show | list } [PROG]\n" - " %1$s %2$s dump xlated PROG [{ file FILE | opcodes | visual | linum }]\n" - " %1$s %2$s dump jited PROG [{ file FILE | opcodes | linum }]\n" + " %1$s %2$s dump xlated PROG [{ file FILE | [opcodes] [linum] [visual] }]\n" + " %1$s %2$s dump jited PROG [{ file FILE | [opcodes] [linum] }]\n" " %1$s %2$s pin PROG FILE\n" " %1$s %2$s { load | loadall } OBJ PATH \\\n" " [type TYPE] [dev NAME] \\\n" -- cgit From 7483a7a70a12d7c00c9f80574d533b01689d39a7 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Wed, 5 Apr 2023 14:21:19 +0100 Subject: bpftool: Support printing opcodes and source file references in CFG Add support for displaying opcodes or/and file references (filepath, line and column numbers) when dumping the control flow graphs of loaded BPF programs with bpftool. The filepaths in the records are absolute. To avoid blocks on the graph to get too wide, we truncate them when they get too long (but we always keep the entire file name). In the unlikely case where the resulting file name is ambiguous, it remains possible to get the full path with a regular dump (no CFG). Signed-off-by: Quentin Monnet Link: https://lore.kernel.org/r/20230405132120.59886-7-quentin@isovalent.com Signed-off-by: Alexei Starovoitov --- tools/bpf/bpftool/prog.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/bpf/bpftool/prog.c') diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index 092525a6933a..430f72306409 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -852,7 +852,7 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode, if (json_output) dump_xlated_json(&dd, buf, member_len, opcodes, linum); else if (visual) - dump_xlated_cfg(&dd, buf, member_len); + dump_xlated_cfg(&dd, buf, member_len, opcodes, linum); else dump_xlated_plain(&dd, buf, member_len, opcodes, linum); kernel_syms_destroy(&dd); -- cgit From b24f0b049e706c6fc6e822dfc519ee33c3865092 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Fri, 7 Apr 2023 08:14:26 +0000 Subject: bpftool: Set program type only if it differs from the desired one After commit d6e6286a12e7 ("libbpf: disassociate section handler on explicit bpf_program__set_type() call"), bpf_program__set_type() will force cleanup the program's SEC() definition, this commit fixed the test helper but missed the bpftool, which leads to bpftool prog autoattach broken as follows: $ bpftool prog load spi-xfer-r1v1.o /sys/fs/bpf/test autoattach Program spi_xfer_r1v1 does not support autoattach, falling back to pinning This patch fix bpftool to set program type only if it differs. Fixes: d6e6286a12e7 ("libbpf: disassociate section handler on explicit bpf_program__set_type() call") Signed-off-by: Wei Yongjun Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20230407081427.2621590-1-weiyongjun@huaweicloud.com --- tools/bpf/bpftool/prog.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tools/bpf/bpftool/prog.c') diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index 430f72306409..e5b613a7974c 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -1685,7 +1685,8 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) } bpf_program__set_ifindex(pos, ifindex); - bpf_program__set_type(pos, prog_type); + if (bpf_program__type(pos) != prog_type) + bpf_program__set_type(pos, prog_type); bpf_program__set_expected_attach_type(pos, expected_attach_type); } -- cgit From 0232b788978652571c4f4e57dc26ff8e4837926a Mon Sep 17 00:00:00 2001 From: Kui-Feng Lee Date: Wed, 19 Apr 2023 17:28:21 -0700 Subject: bpftool: Register struct_ops with a link. You can include an optional path after specifying the object name for the 'struct_ops register' subcommand. Since the commit 226bc6ae6405 ("Merge branch 'Transit between BPF TCP congestion controls.'") has been accepted, it is now possible to create a link for a struct_ops. This can be done by defining a struct_ops in SEC(".struct_ops.link") to make libbpf returns a real link. If we don't pin the links before leaving bpftool, they will disappear. To instruct bpftool to pin the links in a directory with the names of the maps, we need to provide the path of that directory. Signed-off-by: Kui-Feng Lee Reviewed-by: Quentin Monnet Link: https://lore.kernel.org/r/20230420002822.345222-1-kuifeng@meta.com Signed-off-by: Alexei Starovoitov --- tools/bpf/bpftool/prog.c | 13 ------------- 1 file changed, 13 deletions(-) (limited to 'tools/bpf/bpftool/prog.c') diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index e5b613a7974c..91b6075b2db3 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -1476,19 +1476,6 @@ auto_attach_program(struct bpf_program *prog, const char *path) return err; } -static int pathname_concat(char *buf, size_t buf_sz, const char *path, const char *name) -{ - int len; - - len = snprintf(buf, buf_sz, "%s/%s", path, name); - if (len < 0) - return -EINVAL; - if ((size_t)len >= buf_sz) - return -ENAMETOOLONG; - - return 0; -} - static int auto_attach_programs(struct bpf_object *obj, const char *path) { -- cgit