* [RFC] libbpf,selftests: Question on btf_dump__emit_type_decl for BTF_KIND_FUNC
@ 2020-03-03 14:08 Jiri Olsa
2020-03-03 17:09 ` Andrii Nakryiko
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2020-03-03 14:08 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Yonghong Song, Martin KaFai Lau, Jakub Kicinski,
David Miller, John Fastabend, Jesper Dangaard Brouer
hi,
for bpftrace I'd like to print BTF functions (BTF_KIND_FUNC)
declarations together with their names.
I saw we have btf_dump__emit_type_decl and added BTF_KIND_FUNC,
where it seemed to be missing, so it prints out something now
(not sure it's the right fix though).
Anyway, would you be ok with adding some flag/bool to struct
btf_dump_emit_type_decl_opts, so I could get output like:
kfunc:ksys_readahead(int fd, long long int offset, long unsigned int count) = ssize_t
kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
... to be able to the arguments and return type separated,
so I could easily get to something like above?
Current interface is just vfprintf callback and I'm not sure
I can rely that it will allywas be called with same arguments,
like having separated calls for parsed atoms like 'return type',
'(', ')', '(', 'arg type', 'arg name', ...
I'm open to any suggestion ;-)
thanks,
jirka
---
tools/lib/bpf/btf_dump.c | 4 ++++
.../selftests/bpf/prog_tests/btf_dump.c | 21 +++++++++++++++++++
.../bpf/progs/btf_dump_test_case_bitfields.c | 10 +++++++++
.../bpf/progs/btf_dump_test_case_multidim.c | 3 +++
.../progs/btf_dump_test_case_namespacing.c | 19 +++++++++++++++++
.../bpf/progs/btf_dump_test_case_ordering.c | 3 +++
.../bpf/progs/btf_dump_test_case_packing.c | 16 ++++++++++++++
.../bpf/progs/btf_dump_test_case_padding.c | 15 +++++++++++++
.../bpf/progs/btf_dump_test_case_syntax.c | 3 +++
9 files changed, 94 insertions(+)
diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index bd09ed1710f1..40c7491424eb 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -1068,6 +1068,7 @@ static void btf_dump_emit_type_decl(struct btf_dump *d, __u32 id,
case BTF_KIND_CONST:
case BTF_KIND_RESTRICT:
case BTF_KIND_FUNC_PROTO:
+ case BTF_KIND_FUNC:
id = t->type;
break;
case BTF_KIND_ARRAY:
@@ -1307,6 +1308,9 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
btf_dump_printf(d, ")");
return;
}
+ case BTF_KIND_FUNC:
+ /* All work is done via BTF_KIND_FUNC_PROTO already. */
+ break;
default:
pr_warn("unexpected type in decl chain, kind:%u, id:[%u]\n",
kind, id);
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
index 7390d3061065..adcd0abcec5c 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
@@ -26,6 +26,9 @@ static struct btf_dump_test_case {
static int btf_dump_all_types(const struct btf *btf,
const struct btf_dump_opts *opts)
{
+ DECLARE_LIBBPF_OPTS(btf_dump_emit_type_decl_opts, decl_opts,
+ .field_name = "",
+ );
size_t type_cnt = btf__get_nr_types(btf);
struct btf_dump *d;
int err = 0, id;
@@ -35,9 +38,27 @@ static int btf_dump_all_types(const struct btf *btf,
return PTR_ERR(d);
for (id = 1; id <= type_cnt; id++) {
+ const struct btf_type *type;
+
err = btf_dump__dump_type(d, id);
if (err)
goto done;
+
+ type = btf__type_by_id(btf, id);
+
+ if (BTF_INFO_KIND(type->info) != BTF_KIND_FUNC)
+ continue;
+
+ err = btf_dump__emit_type_decl(d, id, &decl_opts);
+ if (err)
+ goto done;
+
+ /*
+ * There's no newline at the end of the declaration dumped
+ * by btf_dump__emit_type_decl, so doing an extra *one, so
+ * we can have 'expected' counter part with newline.
+ */
+ fprintf(opts->ctx, "\n");
}
done:
diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_bitfields.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_bitfields.c
index 8f44767a75fa..4d911cab7012 100644
--- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_bitfields.c
+++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_bitfields.c
@@ -82,6 +82,16 @@ struct bitfield_flushed {
long b: 16;
};
+/* ----- START-EXPECTED-OUTPUT ----- */
+/*
+ *int ()(struct {
+ * struct bitfields_only_mixed_types _1;
+ * struct bitfield_mixed_with_others _2;
+ * struct bitfield_flushed _3;
+ *} *_)
+ */
+/* ------ END-EXPECTED-OUTPUT ------ */
+
int f(struct {
struct bitfields_only_mixed_types _1;
struct bitfield_mixed_with_others _2;
diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_multidim.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_multidim.c
index ba97165bdb28..97e189e8246a 100644
--- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_multidim.c
+++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_multidim.c
@@ -27,6 +27,9 @@ struct root_struct {
fn_ptr_multiarr_t _6;
};
+/*
+ *int ()(struct root_struct *s)
+ */
/* ------ END-EXPECTED-OUTPUT ------ */
int f(struct root_struct *s)
diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_namespacing.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_namespacing.c
index 92a4ad428710..ac4141a611bf 100644
--- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_namespacing.c
+++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_namespacing.c
@@ -49,6 +49,25 @@ typedef int Y;
typedef int Z;
+/*
+ *int ()(struct {
+ * struct S _1;
+ * S _2;
+ * union U _3;
+ * U _4;
+ * enum E _5;
+ * E _6;
+ * struct A a;
+ * union B b;
+ * enum C c;
+ * struct X x;
+ * union Y y;
+ * enum Z *z;
+ * X xx;
+ * Y yy;
+ * Z zz;
+ *} *_)
+ */
/*------ END-EXPECTED-OUTPUT ------ */
int f(struct {
diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_ordering.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_ordering.c
index 7c95702ee4cb..2687ca94025d 100644
--- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_ordering.c
+++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_ordering.c
@@ -55,6 +55,9 @@ struct root_struct {
struct callback_head cb;
};
+/*
+ *int ()(struct root_struct *root)
+ */
/*------ END-EXPECTED-OUTPUT ------ */
int f(struct root_struct *root)
diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_packing.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_packing.c
index 1cef3bec1dc7..88bae49bdbbb 100644
--- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_packing.c
+++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_packing.c
@@ -58,6 +58,22 @@ union jump_code_union {
} __attribute__((packed));
};
+/*
+ *int ()(struct {
+ * struct packed_trailing_space _1;
+ * short: 16;
+ * struct non_packed_trailing_space _2;
+ * struct packed_fields _3;
+ * short: 16;
+ * struct non_packed_fields _4;
+ * struct nested_packed _5;
+ * short: 16;
+ * union union_is_never_packed _6;
+ * union union_does_not_need_packing _7;
+ * union jump_code_union _8;
+ * int: 24;
+ *} __attribute__((packed)) *_)
+ */
/*------ END-EXPECTED-OUTPUT ------ */
int f(struct {
diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c
index 35c512818a56..581349bb0c2f 100644
--- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c
+++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c
@@ -102,6 +102,21 @@ struct zone {
struct zone_padding __pad__;
};
+/* ----- START-EXPECTED-OUTPUT ----- */
+/*
+ *int ()(struct {
+ * struct padded_implicitly _1;
+ * struct padded_explicitly _2;
+ * struct padded_a_lot _3;
+ * struct padded_cache_line _4;
+ * struct zone _5;
+ * long: 64;
+ * long: 64;
+ * long: 64;
+ *} *_)
+ */
+/* ------ END-EXPECTED-OUTPUT ------ */
+
int f(struct {
struct padded_implicitly _1;
struct padded_explicitly _2;
diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c
index d4a02fe44a12..b110eea7ffd2 100644
--- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c
+++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c
@@ -221,6 +221,9 @@ struct root_struct {
struct struct_with_embedded_stuff _14;
};
+/*
+ *int ()(struct root_struct *s)
+ */
/* ------ END-EXPECTED-OUTPUT ------ */
int f(struct root_struct *s)
--
2.24.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC] libbpf,selftests: Question on btf_dump__emit_type_decl for BTF_KIND_FUNC
2020-03-03 14:08 [RFC] libbpf,selftests: Question on btf_dump__emit_type_decl for BTF_KIND_FUNC Jiri Olsa
@ 2020-03-03 17:09 ` Andrii Nakryiko
2020-03-03 17:33 ` Jiri Olsa
0 siblings, 1 reply; 5+ messages in thread
From: Andrii Nakryiko @ 2020-03-03 17:09 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking,
bpf, Yonghong Song, Martin KaFai Lau, Jakub Kicinski,
David Miller, John Fastabend, Jesper Dangaard Brouer
On Tue, Mar 3, 2020 at 6:12 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> hi,
> for bpftrace I'd like to print BTF functions (BTF_KIND_FUNC)
> declarations together with their names.
>
> I saw we have btf_dump__emit_type_decl and added BTF_KIND_FUNC,
> where it seemed to be missing, so it prints out something now
> (not sure it's the right fix though).
>
> Anyway, would you be ok with adding some flag/bool to struct
> btf_dump_emit_type_decl_opts, so I could get output like:
>
> kfunc:ksys_readahead(int fd, long long int offset, long unsigned int count) = ssize_t
> kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
>
> ... to be able to the arguments and return type separated,
> so I could easily get to something like above?
>
> Current interface is just vfprintf callback and I'm not sure
> I can rely that it will allywas be called with same arguments,
> like having separated calls for parsed atoms like 'return type',
> '(', ')', '(', 'arg type', 'arg name', ...
>
> I'm open to any suggestion ;-)
Hey Jiri!
Can you please elaborate on the use case and problem you are trying to solve?
I think we can (and probably even should) add such option and support
to dump functions, but whatever we do it should be a valid C syntax
and should be compilable.
Example above:
kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
Is this really the syntax you need to get? I think btf_dump, when
(optionally) emitting function declaration, will have to emit that
particular one as:
size_t ksys_read(unsigned int fd, char buf, long unsigned int count);
But I'd like to hear the use case before we add this. Thanks!
>
> thanks,
> jirka
>
>
> ---
> tools/lib/bpf/btf_dump.c | 4 ++++
> .../selftests/bpf/prog_tests/btf_dump.c | 21 +++++++++++++++++++
> .../bpf/progs/btf_dump_test_case_bitfields.c | 10 +++++++++
> .../bpf/progs/btf_dump_test_case_multidim.c | 3 +++
> .../progs/btf_dump_test_case_namespacing.c | 19 +++++++++++++++++
> .../bpf/progs/btf_dump_test_case_ordering.c | 3 +++
> .../bpf/progs/btf_dump_test_case_packing.c | 16 ++++++++++++++
> .../bpf/progs/btf_dump_test_case_padding.c | 15 +++++++++++++
> .../bpf/progs/btf_dump_test_case_syntax.c | 3 +++
> 9 files changed, 94 insertions(+)
>
[...]
>
> +/*
> + *int ()(struct {
> + * struct packed_trailing_space _1;
> + * short: 16;
> + * struct non_packed_trailing_space _2;
> + * struct packed_fields _3;
> + * short: 16;
> + * struct non_packed_fields _4;
> + * struct nested_packed _5;
> + * short: 16;
> + * union union_is_never_packed _6;
> + * union union_does_not_need_packing _7;
> + * union jump_code_union _8;
> + * int: 24;
> + *} __attribute__((packed)) *_)
> + */
This clearly isn't compilable, right?
> /*------ END-EXPECTED-OUTPUT ------ */
>
> int f(struct {
> diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c
> index 35c512818a56..581349bb0c2f 100644
> --- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c
> +++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c
> @@ -102,6 +102,21 @@ struct zone {
> struct zone_padding __pad__;
> };
>
[...]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] libbpf,selftests: Question on btf_dump__emit_type_decl for BTF_KIND_FUNC
2020-03-03 17:09 ` Andrii Nakryiko
@ 2020-03-03 17:33 ` Jiri Olsa
2020-03-03 18:00 ` Andrii Nakryiko
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2020-03-03 17:33 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Networking, bpf, Yonghong Song, Martin KaFai Lau, Jakub Kicinski,
David Miller, John Fastabend, Jesper Dangaard Brouer
On Tue, Mar 03, 2020 at 09:09:38AM -0800, Andrii Nakryiko wrote:
> On Tue, Mar 3, 2020 at 6:12 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > hi,
> > for bpftrace I'd like to print BTF functions (BTF_KIND_FUNC)
> > declarations together with their names.
> >
> > I saw we have btf_dump__emit_type_decl and added BTF_KIND_FUNC,
> > where it seemed to be missing, so it prints out something now
> > (not sure it's the right fix though).
> >
> > Anyway, would you be ok with adding some flag/bool to struct
> > btf_dump_emit_type_decl_opts, so I could get output like:
> >
> > kfunc:ksys_readahead(int fd, long long int offset, long unsigned int count) = ssize_t
> > kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
> >
> > ... to be able to the arguments and return type separated,
> > so I could easily get to something like above?
> >
> > Current interface is just vfprintf callback and I'm not sure
> > I can rely that it will allywas be called with same arguments,
> > like having separated calls for parsed atoms like 'return type',
> > '(', ')', '(', 'arg type', 'arg name', ...
> >
> > I'm open to any suggestion ;-)
>
> Hey Jiri!
>
> Can you please elaborate on the use case and problem you are trying to solve?
>
> I think we can (and probably even should) add such option and support
> to dump functions, but whatever we do it should be a valid C syntax
> and should be compilable.
> Example above:
>
> kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
>
> Is this really the syntax you need to get? I think btf_dump, when
> (optionally) emitting function declaration, will have to emit that
> particular one as:
>
> size_t ksys_read(unsigned int fd, char buf, long unsigned int count);
>
> But I'd like to hear the use case before we add this. Thanks!
the use case is just for the 'bpftrace -l' output, which displays
the probe names that could be used.. for kernel BTF kernel functions
it's 'kfunc:function(args)'
software:task-clock:
hardware:backend-stalls:
hardware:branch-instructions:
...
tracepoint:kvmmmu:kvm_mmu_pagetable_walk
tracepoint:kvmmmu:kvm_mmu_paging_element
...
kprobe:console_on_rootfs
kprobe:trace_initcall_start_cb
kprobe:run_init_process
kprobe:try_to_run_init_process
...
kfunc:x86_reserve_hardware
kfunc:hw_perf_lbr_event_destroy
kfunc:x86_perf_event_update
I dont want to print the return type as is in C, because it would
mess up the whole output, hence the '= <return type>'
kfunc:ksys_readahead(int fd, long long int offset, long unsigned int count) = ssize_t
kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
also possible only in verbose mode ;-)
the final shape of the format will be decided in a bpftrace review,
but in any case I think I'll need some way to get these bits:
<args> <return type>
thanks,
jirka
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] libbpf,selftests: Question on btf_dump__emit_type_decl for BTF_KIND_FUNC
2020-03-03 17:33 ` Jiri Olsa
@ 2020-03-03 18:00 ` Andrii Nakryiko
2020-03-03 20:05 ` Jiri Olsa
0 siblings, 1 reply; 5+ messages in thread
From: Andrii Nakryiko @ 2020-03-03 18:00 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Networking, bpf, Yonghong Song, Martin KaFai Lau, Jakub Kicinski,
David Miller, John Fastabend, Jesper Dangaard Brouer
On Tue, Mar 3, 2020 at 9:33 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Tue, Mar 03, 2020 at 09:09:38AM -0800, Andrii Nakryiko wrote:
> > On Tue, Mar 3, 2020 at 6:12 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > hi,
> > > for bpftrace I'd like to print BTF functions (BTF_KIND_FUNC)
> > > declarations together with their names.
> > >
> > > I saw we have btf_dump__emit_type_decl and added BTF_KIND_FUNC,
> > > where it seemed to be missing, so it prints out something now
> > > (not sure it's the right fix though).
> > >
> > > Anyway, would you be ok with adding some flag/bool to struct
> > > btf_dump_emit_type_decl_opts, so I could get output like:
> > >
> > > kfunc:ksys_readahead(int fd, long long int offset, long unsigned int count) = ssize_t
> > > kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
> > >
> > > ... to be able to the arguments and return type separated,
> > > so I could easily get to something like above?
> > >
> > > Current interface is just vfprintf callback and I'm not sure
> > > I can rely that it will allywas be called with same arguments,
> > > like having separated calls for parsed atoms like 'return type',
> > > '(', ')', '(', 'arg type', 'arg name', ...
> > >
> > > I'm open to any suggestion ;-)
> >
> > Hey Jiri!
> >
> > Can you please elaborate on the use case and problem you are trying to solve?
> >
> > I think we can (and probably even should) add such option and support
> > to dump functions, but whatever we do it should be a valid C syntax
> > and should be compilable.
> > Example above:
> >
> > kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
> >
> > Is this really the syntax you need to get? I think btf_dump, when
> > (optionally) emitting function declaration, will have to emit that
> > particular one as:
> >
> > size_t ksys_read(unsigned int fd, char buf, long unsigned int count);
> >
> > But I'd like to hear the use case before we add this. Thanks!
>
> the use case is just for the 'bpftrace -l' output, which displays
> the probe names that could be used.. for kernel BTF kernel functions
> it's 'kfunc:function(args)'
>
> software:task-clock:
> hardware:backend-stalls:
> hardware:branch-instructions:
> ...
> tracepoint:kvmmmu:kvm_mmu_pagetable_walk
> tracepoint:kvmmmu:kvm_mmu_paging_element
> ...
> kprobe:console_on_rootfs
> kprobe:trace_initcall_start_cb
> kprobe:run_init_process
> kprobe:try_to_run_init_process
> ...
> kfunc:x86_reserve_hardware
> kfunc:hw_perf_lbr_event_destroy
> kfunc:x86_perf_event_update
>
> I dont want to print the return type as is in C, because it would
> mess up the whole output, hence the '= <return type>'
>
> kfunc:ksys_readahead(int fd, long long int offset, long unsigned int count) = ssize_t
> kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
>
> also possible only in verbose mode ;-)
>
> the final shape of the format will be decided in a bpftrace review,
> but in any case I think I'll need some way to get these bits:
> <args> <return type>
>
Ok, I think for your use case it's better for you to implement it
customly, I don't think this fits btf_dump() C output as is. But you
have all the right high-level APIs anyways. There is nothing irregular
about function declarations, thankfully. Pointers to functions are way
more involved, syntactically, which is already abstracted from you in
btf_dump__emit_type_decl(). Here's the code:
static int dump_funcs(const struct btf *btf, struct btf_dump *d)
{
int err = 0, i, j, cnt = btf__get_nr_types(btf);
const struct btf_type *t;
const struct btf_param *p;
const char *name;
for (i = 1; i <= cnt; i++) {
t = btf__type_by_id(btf, i);
if (!btf_is_func(t))
continue;
name = btf__name_by_offset(btf, t->name_off);
t = btf__type_by_id(btf, t->type);
if (!btf_is_func_proto(t))
return -EINVAL;
printf("kfunc:%s(", name);
for (j = 0, p = btf_params(t); j < btf_vlen(t); j++, p++) {
err = btf_dump__emit_type_decl(d, p->type, NULL);
if (err)
return err;
}
printf(") = ");
err = btf_dump__emit_type_decl(d, t->type, NULL);
if (err)
return err;
printf(";\n");
}
return 0;
}
Beware, this will crash right now due to NULL field_name, but I'm
fixing that with a tiny patch in just a second.
Also beware, there are no argument names captures for func_protos...
So with the above (and btf_dump__emit_type_decl() fix for NULL
field_name), this will produce output:
kfunc:num_digits(int) = int;
kfunc:copy_from_user_nmi(void *const void *long unsigned int) = long
unsigned int;
kfunc:arch_wb_cache_pmem(void *size_t) = void;
kfunc:__clear_user(void *long unsigned int) = long unsigned int;
>
> thanks,
> jirka
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] libbpf,selftests: Question on btf_dump__emit_type_decl for BTF_KIND_FUNC
2020-03-03 18:00 ` Andrii Nakryiko
@ 2020-03-03 20:05 ` Jiri Olsa
0 siblings, 0 replies; 5+ messages in thread
From: Jiri Olsa @ 2020-03-03 20:05 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Networking, bpf, Yonghong Song, Martin KaFai Lau, Jakub Kicinski,
David Miller, John Fastabend, Jesper Dangaard Brouer
On Tue, Mar 03, 2020 at 10:00:02AM -0800, Andrii Nakryiko wrote:
> On Tue, Mar 3, 2020 at 9:33 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Tue, Mar 03, 2020 at 09:09:38AM -0800, Andrii Nakryiko wrote:
> > > On Tue, Mar 3, 2020 at 6:12 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > >
> > > > hi,
> > > > for bpftrace I'd like to print BTF functions (BTF_KIND_FUNC)
> > > > declarations together with their names.
> > > >
> > > > I saw we have btf_dump__emit_type_decl and added BTF_KIND_FUNC,
> > > > where it seemed to be missing, so it prints out something now
> > > > (not sure it's the right fix though).
> > > >
> > > > Anyway, would you be ok with adding some flag/bool to struct
> > > > btf_dump_emit_type_decl_opts, so I could get output like:
> > > >
> > > > kfunc:ksys_readahead(int fd, long long int offset, long unsigned int count) = ssize_t
> > > > kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
> > > >
> > > > ... to be able to the arguments and return type separated,
> > > > so I could easily get to something like above?
> > > >
> > > > Current interface is just vfprintf callback and I'm not sure
> > > > I can rely that it will allywas be called with same arguments,
> > > > like having separated calls for parsed atoms like 'return type',
> > > > '(', ')', '(', 'arg type', 'arg name', ...
> > > >
> > > > I'm open to any suggestion ;-)
> > >
> > > Hey Jiri!
> > >
> > > Can you please elaborate on the use case and problem you are trying to solve?
> > >
> > > I think we can (and probably even should) add such option and support
> > > to dump functions, but whatever we do it should be a valid C syntax
> > > and should be compilable.
> > > Example above:
> > >
> > > kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
> > >
> > > Is this really the syntax you need to get? I think btf_dump, when
> > > (optionally) emitting function declaration, will have to emit that
> > > particular one as:
> > >
> > > size_t ksys_read(unsigned int fd, char buf, long unsigned int count);
> > >
> > > But I'd like to hear the use case before we add this. Thanks!
> >
> > the use case is just for the 'bpftrace -l' output, which displays
> > the probe names that could be used.. for kernel BTF kernel functions
> > it's 'kfunc:function(args)'
> >
> > software:task-clock:
> > hardware:backend-stalls:
> > hardware:branch-instructions:
> > ...
> > tracepoint:kvmmmu:kvm_mmu_pagetable_walk
> > tracepoint:kvmmmu:kvm_mmu_paging_element
> > ...
> > kprobe:console_on_rootfs
> > kprobe:trace_initcall_start_cb
> > kprobe:run_init_process
> > kprobe:try_to_run_init_process
> > ...
> > kfunc:x86_reserve_hardware
> > kfunc:hw_perf_lbr_event_destroy
> > kfunc:x86_perf_event_update
> >
> > I dont want to print the return type as is in C, because it would
> > mess up the whole output, hence the '= <return type>'
> >
> > kfunc:ksys_readahead(int fd, long long int offset, long unsigned int count) = ssize_t
> > kfunc:ksys_read(unsigned int fd, char buf, long unsigned int count) = size_t
> >
> > also possible only in verbose mode ;-)
> >
> > the final shape of the format will be decided in a bpftrace review,
> > but in any case I think I'll need some way to get these bits:
> > <args> <return type>
> >
>
> Ok, I think for your use case it's better for you to implement it
> customly, I don't think this fits btf_dump() C output as is. But you
> have all the right high-level APIs anyways. There is nothing irregular
> about function declarations, thankfully. Pointers to functions are way
> more involved, syntactically, which is already abstracted from you in
> btf_dump__emit_type_decl(). Here's the code:
>
> static int dump_funcs(const struct btf *btf, struct btf_dump *d)
> {
> int err = 0, i, j, cnt = btf__get_nr_types(btf);
> const struct btf_type *t;
> const struct btf_param *p;
> const char *name;
>
> for (i = 1; i <= cnt; i++) {
> t = btf__type_by_id(btf, i);
> if (!btf_is_func(t))
> continue;
>
> name = btf__name_by_offset(btf, t->name_off);
> t = btf__type_by_id(btf, t->type);
> if (!btf_is_func_proto(t))
> return -EINVAL;
>
> printf("kfunc:%s(", name);
> for (j = 0, p = btf_params(t); j < btf_vlen(t); j++, p++) {
> err = btf_dump__emit_type_decl(d, p->type, NULL);
> if (err)
> return err;
> }
> printf(") = ");
>
> err = btf_dump__emit_type_decl(d, t->type, NULL);
> if (err)
> return err;
aaah right, we could move it one level down ;-) ok, that will do
>
> printf(";\n");
> }
> return 0;
> }
>
> Beware, this will crash right now due to NULL field_name, but I'm
> fixing that with a tiny patch in just a second.
>
> Also beware, there are no argument names captures for func_protos...
>
> So with the above (and btf_dump__emit_type_decl() fix for NULL
> field_name), this will produce output:
>
> kfunc:num_digits(int) = int;
> kfunc:copy_from_user_nmi(void *const void *long unsigned int) = long
> unsigned int;
> kfunc:arch_wb_cache_pmem(void *size_t) = void;
> kfunc:__clear_user(void *long unsigned int) = long unsigned int;
thanks, I'll use that
jirka
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-03 20:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-03 14:08 [RFC] libbpf,selftests: Question on btf_dump__emit_type_decl for BTF_KIND_FUNC Jiri Olsa
2020-03-03 17:09 ` Andrii Nakryiko
2020-03-03 17:33 ` Jiri Olsa
2020-03-03 18:00 ` Andrii Nakryiko
2020-03-03 20:05 ` Jiri Olsa
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).