Message ID | 20181017072400.2768484-4-yhs@fb.com |
---|---|
State | Changes Requested, archived |
Delegated to: | BPF Maintainers |
Headers | show |
Series | bpf: add btf func info support | expand |
On 17/10/18 08:24, Yonghong Song wrote: > This patch added support to print function signature > if btf func_info is available. Note that ksym > now uses function name instead of prog_name as > prog_name has a limit of 16 bytes including > ending '\0'. > > The following is a sample output for selftests > test_btf with file test_btf_haskv.o: > > $ bpftool prog dump jited id 1 > int _dummy_tracepoint(struct dummy_tracepoint_args * ): > bpf_prog_b07ccb89267cf242__dummy_tracepoint: > 0: push %rbp > 1: mov %rsp,%rbp > ...... > 3c: add $0x28,%rbp > 40: leaveq > 41: retq > > int test_long_fname_1(struct dummy_tracepoint_args * ): > bpf_prog_2dcecc18072623fc_test_long_fname_1: > 0: push %rbp > 1: mov %rsp,%rbp > ...... > 3a: add $0x28,%rbp > 3e: leaveq > 3f: retq > > int test_long_fname_2(struct dummy_tracepoint_args * ): > bpf_prog_89d64e4abf0f0126_test_long_fname_2: > 0: push %rbp > 1: mov %rsp,%rbp > ...... > 80: add $0x28,%rbp > 84: leaveq > 85: retq > > Signed-off-by: Yonghong Song <yhs@fb.com> > --- > tools/bpf/bpftool/btf_dumper.c | 96 ++++++++++++++++++++++++++++++++++ > tools/bpf/bpftool/main.h | 2 + > tools/bpf/bpftool/prog.c | 54 +++++++++++++++++++ > 3 files changed, 152 insertions(+) > > diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c > index 55bc512a1831..a31df4202335 100644 > --- a/tools/bpf/bpftool/btf_dumper.c > +++ b/tools/bpf/bpftool/btf_dumper.c > @@ -249,3 +249,99 @@ int btf_dumper_type(const struct btf_dumper *d, __u32 type_id, > { > return btf_dumper_do_type(d, type_id, 0, data); > } > + > +#define BTF_PRINT_STRING(str) \ > + { \ > + pos += snprintf(func_sig + pos, size - pos, str); \ > + if (pos >= size) \ > + return -1; \ > + } Usual kernel practice for this sort of macro is to use do { \ } while(0) to ensure correct behaviour if the macro is used within another control flow statement, e.g. if (x) BTF_PRINT_STRING(x); else do_something_else(); will not compile with the bare braces as the else will be detached. > +#define BTF_PRINT_ONE_ARG(fmt, arg) \ > + { \ > + pos += snprintf(func_sig + pos, size - pos, fmt, arg); \ > + if (pos >= size) \ > + return -1; \ > + } Any reason for not just using a variadic macro? > +#define BTF_PRINT_TYPE_ONLY(type) \ > + { \ > + pos = __btf_dumper_type_only(btf, type, func_sig, \ > + pos, size); \ > + if (pos == -1) \ > + return -1; \ > + } > + > +static int __btf_dumper_type_only(struct btf *btf, __u32 type_id, > + char *func_sig, int pos, int size) > +{ > + const struct btf_type *t = btf__type_by_id(btf, type_id); > + const struct btf_array *array; > + int i, vlen; > + > + switch (BTF_INFO_KIND(t->info)) { > + case BTF_KIND_INT: > + BTF_PRINT_ONE_ARG("%s ", > + btf__name_by_offset(btf, t->name_off)); > + break; > + case BTF_KIND_STRUCT: > + BTF_PRINT_ONE_ARG("struct %s ", > + btf__name_by_offset(btf, t->name_off)); > + break; > + case BTF_KIND_UNION: > + BTF_PRINT_ONE_ARG("union %s ", > + btf__name_by_offset(btf, t->name_off)); > + break; > + case BTF_KIND_ENUM: > + BTF_PRINT_ONE_ARG("enum %s ", > + btf__name_by_offset(btf, t->name_off)); > + break; > + case BTF_KIND_ARRAY: > + array = (struct btf_array *)(t + 1); > + BTF_PRINT_TYPE_ONLY(array->type); > + BTF_PRINT_ONE_ARG("[%d]", array->nelems); > + break; > + case BTF_KIND_PTR: > + BTF_PRINT_TYPE_ONLY(t->type); > + BTF_PRINT_STRING("* "); > + break; > + case BTF_KIND_UNKN: > + case BTF_KIND_FWD: > + case BTF_KIND_TYPEDEF: > + return -1; > + case BTF_KIND_VOLATILE: > + BTF_PRINT_STRING("volatile "); > + BTF_PRINT_TYPE_ONLY(t->type); > + break; > + case BTF_KIND_CONST: > + BTF_PRINT_STRING("const "); > + BTF_PRINT_TYPE_ONLY(t->type); > + break; > + case BTF_KIND_RESTRICT: > + BTF_PRINT_STRING("restrict "); > + BTF_PRINT_TYPE_ONLY(t->type); > + break; > + case BTF_KIND_FUNC: > + case BTF_KIND_FUNC_PROTO: > + BTF_PRINT_TYPE_ONLY(t->type); > + BTF_PRINT_ONE_ARG("%s(", btf__name_by_offset(btf, t->name_off)); > + vlen = BTF_INFO_VLEN(t->info); > + for (i = 0; i < vlen; i++) { > + __u32 arg_type = ((__u32 *)(t + 1))[i]; > + > + BTF_PRINT_TYPE_ONLY(arg_type); > + if (i != (vlen - 1)) > + BTF_PRINT_STRING(", "); > + } In this kind of loop I find it cleaner to print the comma before the item; that way the test becomes i != 0. Thus: for (i = 0; i < vlen; i++) { __u32 arg_type = ((__u32 *)(t + 1))[i]; if (i) BTF_PRINT_STRING(", "); BTF_PRINT_TYPE_ONLY(arg_type); } -Ed
On 10/17/18 4:11 AM, Edward Cree wrote: > On 17/10/18 08:24, Yonghong Song wrote: >> This patch added support to print function signature >> if btf func_info is available. Note that ksym >> now uses function name instead of prog_name as >> prog_name has a limit of 16 bytes including >> ending '\0'. >> >> The following is a sample output for selftests >> test_btf with file test_btf_haskv.o: >> >> $ bpftool prog dump jited id 1 >> int _dummy_tracepoint(struct dummy_tracepoint_args * ): >> bpf_prog_b07ccb89267cf242__dummy_tracepoint: >> 0: push %rbp >> 1: mov %rsp,%rbp >> ...... >> 3c: add $0x28,%rbp >> 40: leaveq >> 41: retq >> >> int test_long_fname_1(struct dummy_tracepoint_args * ): >> bpf_prog_2dcecc18072623fc_test_long_fname_1: >> 0: push %rbp >> 1: mov %rsp,%rbp >> ...... >> 3a: add $0x28,%rbp >> 3e: leaveq >> 3f: retq >> >> int test_long_fname_2(struct dummy_tracepoint_args * ): >> bpf_prog_89d64e4abf0f0126_test_long_fname_2: >> 0: push %rbp >> 1: mov %rsp,%rbp >> ...... >> 80: add $0x28,%rbp >> 84: leaveq >> 85: retq >> >> Signed-off-by: Yonghong Song <yhs@fb.com> >> --- >> tools/bpf/bpftool/btf_dumper.c | 96 ++++++++++++++++++++++++++++++++++ >> tools/bpf/bpftool/main.h | 2 + >> tools/bpf/bpftool/prog.c | 54 +++++++++++++++++++ >> 3 files changed, 152 insertions(+) >> >> diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c >> index 55bc512a1831..a31df4202335 100644 >> --- a/tools/bpf/bpftool/btf_dumper.c >> +++ b/tools/bpf/bpftool/btf_dumper.c >> @@ -249,3 +249,99 @@ int btf_dumper_type(const struct btf_dumper *d, __u32 type_id, >> { >> return btf_dumper_do_type(d, type_id, 0, data); >> } >> + >> +#define BTF_PRINT_STRING(str) \ >> + { \ >> + pos += snprintf(func_sig + pos, size - pos, str); \ >> + if (pos >= size) \ >> + return -1; \ >> + } > Usual kernel practice for this sort of macro is to use > do { \ > } while(0) > to ensure correct behaviour if the macro is used within another control > flow statement, e.g. > if (x) > BTF_PRINT_STRING(x); > else > do_something_else(); > will not compile with the bare braces as the else will be detached. Thanks for the review! Will change to use the "do while" format as you suggested. >> +#define BTF_PRINT_ONE_ARG(fmt, arg) \ >> + { \ >> + pos += snprintf(func_sig + pos, size - pos, fmt, arg); \ >> + if (pos >= size) \ >> + return -1; \ >> + } > Any reason for not just using a variadic macro? No particular reason. I will try to use it in the next revision. >> +#define BTF_PRINT_TYPE_ONLY(type) \ >> + { \ >> + pos = __btf_dumper_type_only(btf, type, func_sig, \ >> + pos, size); \ >> + if (pos == -1) \ >> + return -1; \ >> + } >> + >> +static int __btf_dumper_type_only(struct btf *btf, __u32 type_id, >> + char *func_sig, int pos, int size) >> +{ >> + const struct btf_type *t = btf__type_by_id(btf, type_id); >> + const struct btf_array *array; >> + int i, vlen; >> + >> + switch (BTF_INFO_KIND(t->info)) { >> + case BTF_KIND_INT: >> + BTF_PRINT_ONE_ARG("%s ", >> + btf__name_by_offset(btf, t->name_off)); >> + break; >> + case BTF_KIND_STRUCT: >> + BTF_PRINT_ONE_ARG("struct %s ", >> + btf__name_by_offset(btf, t->name_off)); >> + break; >> + case BTF_KIND_UNION: >> + BTF_PRINT_ONE_ARG("union %s ", >> + btf__name_by_offset(btf, t->name_off)); >> + break; >> + case BTF_KIND_ENUM: >> + BTF_PRINT_ONE_ARG("enum %s ", >> + btf__name_by_offset(btf, t->name_off)); >> + break; >> + case BTF_KIND_ARRAY: >> + array = (struct btf_array *)(t + 1); >> + BTF_PRINT_TYPE_ONLY(array->type); >> + BTF_PRINT_ONE_ARG("[%d]", array->nelems); >> + break; >> + case BTF_KIND_PTR: >> + BTF_PRINT_TYPE_ONLY(t->type); >> + BTF_PRINT_STRING("* "); >> + break; >> + case BTF_KIND_UNKN: >> + case BTF_KIND_FWD: >> + case BTF_KIND_TYPEDEF: >> + return -1; >> + case BTF_KIND_VOLATILE: >> + BTF_PRINT_STRING("volatile "); >> + BTF_PRINT_TYPE_ONLY(t->type); >> + break; >> + case BTF_KIND_CONST: >> + BTF_PRINT_STRING("const "); >> + BTF_PRINT_TYPE_ONLY(t->type); >> + break; >> + case BTF_KIND_RESTRICT: >> + BTF_PRINT_STRING("restrict "); >> + BTF_PRINT_TYPE_ONLY(t->type); >> + break; >> + case BTF_KIND_FUNC: >> + case BTF_KIND_FUNC_PROTO: >> + BTF_PRINT_TYPE_ONLY(t->type); >> + BTF_PRINT_ONE_ARG("%s(", btf__name_by_offset(btf, t->name_off)); >> + vlen = BTF_INFO_VLEN(t->info); >> + for (i = 0; i < vlen; i++) { >> + __u32 arg_type = ((__u32 *)(t + 1))[i]; >> + >> + BTF_PRINT_TYPE_ONLY(arg_type); >> + if (i != (vlen - 1)) >> + BTF_PRINT_STRING(", "); >> + } > In this kind of loop I find it cleaner to print the comma before the item; > that way the test becomes i != 0. Thus: > for (i = 0; i < vlen; i++) { > __u32 arg_type = ((__u32 *)(t + 1))[i]; > > if (i) > BTF_PRINT_STRING(", "); > BTF_PRINT_TYPE_ONLY(arg_type); > } Good suggestion. Will make change in the next revision. > > -Ed >
diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c index 55bc512a1831..a31df4202335 100644 --- a/tools/bpf/bpftool/btf_dumper.c +++ b/tools/bpf/bpftool/btf_dumper.c @@ -249,3 +249,99 @@ int btf_dumper_type(const struct btf_dumper *d, __u32 type_id, { return btf_dumper_do_type(d, type_id, 0, data); } + +#define BTF_PRINT_STRING(str) \ + { \ + pos += snprintf(func_sig + pos, size - pos, str); \ + if (pos >= size) \ + return -1; \ + } +#define BTF_PRINT_ONE_ARG(fmt, arg) \ + { \ + pos += snprintf(func_sig + pos, size - pos, fmt, arg); \ + if (pos >= size) \ + return -1; \ + } +#define BTF_PRINT_TYPE_ONLY(type) \ + { \ + pos = __btf_dumper_type_only(btf, type, func_sig, \ + pos, size); \ + if (pos == -1) \ + return -1; \ + } + +static int __btf_dumper_type_only(struct btf *btf, __u32 type_id, + char *func_sig, int pos, int size) +{ + const struct btf_type *t = btf__type_by_id(btf, type_id); + const struct btf_array *array; + int i, vlen; + + switch (BTF_INFO_KIND(t->info)) { + case BTF_KIND_INT: + BTF_PRINT_ONE_ARG("%s ", + btf__name_by_offset(btf, t->name_off)); + break; + case BTF_KIND_STRUCT: + BTF_PRINT_ONE_ARG("struct %s ", + btf__name_by_offset(btf, t->name_off)); + break; + case BTF_KIND_UNION: + BTF_PRINT_ONE_ARG("union %s ", + btf__name_by_offset(btf, t->name_off)); + break; + case BTF_KIND_ENUM: + BTF_PRINT_ONE_ARG("enum %s ", + btf__name_by_offset(btf, t->name_off)); + break; + case BTF_KIND_ARRAY: + array = (struct btf_array *)(t + 1); + BTF_PRINT_TYPE_ONLY(array->type); + BTF_PRINT_ONE_ARG("[%d]", array->nelems); + break; + case BTF_KIND_PTR: + BTF_PRINT_TYPE_ONLY(t->type); + BTF_PRINT_STRING("* "); + break; + case BTF_KIND_UNKN: + case BTF_KIND_FWD: + case BTF_KIND_TYPEDEF: + return -1; + case BTF_KIND_VOLATILE: + BTF_PRINT_STRING("volatile "); + BTF_PRINT_TYPE_ONLY(t->type); + break; + case BTF_KIND_CONST: + BTF_PRINT_STRING("const "); + BTF_PRINT_TYPE_ONLY(t->type); + break; + case BTF_KIND_RESTRICT: + BTF_PRINT_STRING("restrict "); + BTF_PRINT_TYPE_ONLY(t->type); + break; + case BTF_KIND_FUNC: + case BTF_KIND_FUNC_PROTO: + BTF_PRINT_TYPE_ONLY(t->type); + BTF_PRINT_ONE_ARG("%s(", btf__name_by_offset(btf, t->name_off)); + vlen = BTF_INFO_VLEN(t->info); + for (i = 0; i < vlen; i++) { + __u32 arg_type = ((__u32 *)(t + 1))[i]; + + BTF_PRINT_TYPE_ONLY(arg_type); + if (i != (vlen - 1)) + BTF_PRINT_STRING(", "); + } + BTF_PRINT_STRING(")"); + break; + default: + return -1; + } + + return pos; +} + +int btf_dumper_type_only(struct btf *btf, __u32 type_id, char *func_sig, + int size) +{ + return __btf_dumper_type_only(btf, type_id, func_sig, 0, size); +} diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h index 28ee769bd11b..f887564476dc 100644 --- a/tools/bpf/bpftool/main.h +++ b/tools/bpf/bpftool/main.h @@ -168,6 +168,8 @@ struct btf_dumper { */ int btf_dumper_type(const struct btf_dumper *d, __u32 type_id, const void *data); +int btf_dumper_type_only(struct btf *btf, __u32 func_type_id, char *func_only, + int size); struct nlattr; struct ifinfomsg; diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index 335028968dfb..e78632a30ea3 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -47,6 +47,7 @@ #include <linux/err.h> #include <bpf.h> +#include <btf.h> #include <libbpf.h> #include "cfg.h" @@ -447,9 +448,11 @@ static int do_show(int argc, char **argv) static int do_dump(int argc, char **argv) { unsigned long *func_ksyms = NULL; + unsigned int *func_types = NULL; struct bpf_prog_info info = {}; unsigned int *func_lens = NULL; unsigned int nr_func_ksyms; + unsigned int nr_func_types; unsigned int nr_func_lens; struct dump_data dd = {}; __u32 len = sizeof(info); @@ -546,6 +549,16 @@ static int do_dump(int argc, char **argv) } } + nr_func_types = info.nr_jited_func_types; + if (nr_func_types) { + func_types = malloc(nr_func_types * sizeof(__u32)); + if (!func_types) { + p_err("mem alloc failed"); + close(fd); + goto err_free; + } + } + memset(&info, 0, sizeof(info)); *member_ptr = ptr_to_u64(buf); @@ -554,6 +567,8 @@ static int do_dump(int argc, char **argv) info.nr_jited_ksyms = nr_func_ksyms; info.jited_func_lens = ptr_to_u64(func_lens); info.nr_jited_func_lens = nr_func_lens; + info.jited_func_types = ptr_to_u64(func_types); + info.nr_jited_func_types = nr_func_types; err = bpf_obj_get_info_by_fd(fd, &info, &len); close(fd); @@ -577,6 +592,11 @@ static int do_dump(int argc, char **argv) goto err_free; } + if (info.nr_jited_func_types > nr_func_types) { + p_err("too many types returned"); + goto err_free; + } + if ((member_len == &info.jited_prog_len && info.jited_prog_insns == 0) || (member_len == &info.xlated_prog_len && @@ -585,6 +605,12 @@ static int do_dump(int argc, char **argv) goto err_free; } + if (info.btf_id && + info.nr_jited_func_lens != info.nr_jited_func_types) { + p_err("unequal jited func lens and types"); + goto err_free; + } + if (filepath) { fd = open(filepath, O_WRONLY | O_CREAT | O_TRUNC, 0600); if (fd < 0) { @@ -618,7 +644,9 @@ static int do_dump(int argc, char **argv) struct kernel_sym *sym = NULL; char sym_name[SYM_MAX_NAME]; unsigned char *img = buf; + struct btf *btf = NULL; __u64 *ksyms = NULL; + char func_sig[1024]; __u32 *lens; __u32 i; @@ -627,6 +655,14 @@ static int do_dump(int argc, char **argv) ksyms = (__u64 *) info.jited_ksyms; } + if (info.btf_id) { + err = btf_get_from_id(info.btf_id, &btf); + if (err) { + p_err("failed to get btf"); + goto err_free; + } + } + if (json_output) jsonw_start_array(json_wtr); @@ -642,12 +678,28 @@ static int do_dump(int argc, char **argv) strcpy(sym_name, "unknown"); } + func_sig[0] = '\0'; + if (btf) { + err = btf_dumper_type_only(btf, + func_types[i], + func_sig, + sizeof(func_sig)); + if (err < 0) + func_sig[0] = '\0'; + } + if (json_output) { jsonw_start_object(json_wtr); + if (func_sig[0] != '\0') { + jsonw_name(json_wtr, "proto"); + jsonw_string(json_wtr, func_sig); + } jsonw_name(json_wtr, "name"); jsonw_string(json_wtr, sym_name); jsonw_name(json_wtr, "insns"); } else { + if (func_sig[0] != '\0') + printf("%s:\n", func_sig); printf("%s:\n", sym_name); } @@ -685,12 +737,14 @@ static int do_dump(int argc, char **argv) free(buf); free(func_ksyms); free(func_lens); + free(func_types); return 0; err_free: free(buf); free(func_ksyms); free(func_lens); + free(func_types); return -1; }
This patch added support to print function signature if btf func_info is available. Note that ksym now uses function name instead of prog_name as prog_name has a limit of 16 bytes including ending '\0'. The following is a sample output for selftests test_btf with file test_btf_haskv.o: $ bpftool prog dump jited id 1 int _dummy_tracepoint(struct dummy_tracepoint_args * ): bpf_prog_b07ccb89267cf242__dummy_tracepoint: 0: push %rbp 1: mov %rsp,%rbp ...... 3c: add $0x28,%rbp 40: leaveq 41: retq int test_long_fname_1(struct dummy_tracepoint_args * ): bpf_prog_2dcecc18072623fc_test_long_fname_1: 0: push %rbp 1: mov %rsp,%rbp ...... 3a: add $0x28,%rbp 3e: leaveq 3f: retq int test_long_fname_2(struct dummy_tracepoint_args * ): bpf_prog_89d64e4abf0f0126_test_long_fname_2: 0: push %rbp 1: mov %rsp,%rbp ...... 80: add $0x28,%rbp 84: leaveq 85: retq Signed-off-by: Yonghong Song <yhs@fb.com> --- tools/bpf/bpftool/btf_dumper.c | 96 ++++++++++++++++++++++++++++++++++ tools/bpf/bpftool/main.h | 2 + tools/bpf/bpftool/prog.c | 54 +++++++++++++++++++ 3 files changed, 152 insertions(+)