Hi Steve,
Here I share my testing patch of the BTF for kprobe events.
Currently this only allow user to specify '$$args' for
tracing all arguments of the function. This is only
avaialbe if
- the probe point is on the function entry
- the kernel is compiled with BTF (CONFIG_DEBUG_INFO_BTF)
- the kernel is enables BPF (CONFIG_BPF_SYSCALL)
And Special thanks to Sven! Most of BTF handling part of
this patch comes from his patch [1]
[1] https://stackframe.org/0001-ftrace-arg-hack.patch
What I thought while coding this were;
- kernel/bpf/btf.c can be moved under lib/ so that
the other subsystems can reuse it, independent
from BPF. (Also, this should depends on CONFIG_DEBUG_INFO_BTF)
- some more utility functions can be exposed.
e.g. I copied btf_type_int() from btf.c
- If there are more comments for the BTF APIs, it will
be more useful...
- Overall, the BTF is easy to understand for who
already understand DWARF. Great work!
- I think I need 'ptr' and 'bool' types for fetcharg types.
Anyway, this is just for testing. I have to add some
more cleanup, features and documentations, etc.
Thank you,
---
Masami Hiramatsu (1):
tracing/kprobe: Support $$args for function entry
kernel/trace/trace_kprobe.c | 60 ++++++++++++++++++++++++-
kernel/trace/trace_probe.c | 105 +++++++++++++++++++++++++++++++++++++++++++
kernel/trace/trace_probe.h | 5 ++
3 files changed, 168 insertions(+), 2 deletions(-)
--
Masami Hiramatsu (Linaro) <[email protected]>
Support $$args fetch arg for function entry. This uses
BTF for finding the function argument. Thus it depends
on CONFIG_BPF_SYSCALL.
/sys/kernel/tracing # echo 'p vfs_read $$args' >> kprobe_events
/sys/kernel/tracing # cat kprobe_events
p:kprobes/p_vfs_read_0 vfs_read file=$arg1:x64 buf=$arg2:x64 count=$arg3:u64 pos=$arg4:x64
Note that $$args must be used without argument name.
Signed-off-by: Masami Hiramatsu <[email protected]>
---
kernel/trace/trace_kprobe.c | 60 ++++++++++++++++++++++++-
kernel/trace/trace_probe.c | 105 +++++++++++++++++++++++++++++++++++++++++++
kernel/trace/trace_probe.h | 5 ++
3 files changed, 168 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 3dd4fb719aa3..fe88ee8c8cd8 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -712,6 +712,58 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
return NOTIFY_DONE;
}
+#ifdef CONFIG_BPF_SYSCALL
+
+static int trace_kprobe_parse_btf_args(struct trace_kprobe *tk, int i,
+ const char *arg, unsigned int flags)
+{
+ struct trace_probe *tp = &tk->tp;
+ static struct btf *btf;
+ const struct btf_type *t;
+ const struct btf_param *args;
+ s32 id, nargs;
+ int ret;
+
+ if (!(flags & TPARG_FL_FENTRY))
+ return -EINVAL;
+ if (!tk->symbol)
+ return -EINVAL;
+
+ if (!btf)
+ btf = btf_parse_vmlinux();
+
+ id = btf_find_by_name_kind(btf, tk->symbol, BTF_KIND_FUNC);
+ if (id <= 0)
+ return -ENOENT;
+
+ /* Get BTF_KIND_FUNC type */
+ t = btf_type_by_id(btf, id);
+ if (!btf_type_is_func(t))
+ return -ENOENT;
+
+ /* The type of BTF_KIND_FUNC is BTF_KIND_FUNC_PROTO */
+ t = btf_type_by_id(btf, t->type);
+ if (!btf_type_is_func_proto(t))
+ return -ENOENT;
+
+ args = (const struct btf_param *)(t + 1);
+ nargs = btf_type_vlen(t);
+ for (i = 0; i < nargs; i++) {
+ ret = traceprobe_parse_btf_arg(tp, i, btf, &args[i]);
+ if (ret < 0)
+ break;
+ }
+
+ return ret;
+}
+#else
+static int trace_kprobe_parse_btf_args(struct trace_kprobe *tk, int i,
+ const char *arg, unsigned int flags)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+
static struct notifier_block trace_kprobe_module_nb = {
.notifier_call = trace_kprobe_module_callback,
.priority = 1 /* Invoked after kprobe module callback */
@@ -733,12 +785,13 @@ static int __trace_kprobe_create(int argc, const char *argv[])
* $stack : fetch stack address
* $stackN : fetch Nth of stack (N:0-)
* $comm : fetch current task comm
+ * $$args : fetch parameters using BTF
* @ADDR : fetch memory at ADDR (ADDR should be in kernel)
* @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data symbol)
* %REG : fetch register REG
* Dereferencing memory fetch:
* +|-offs(ARG) : fetch memory at ARG +|- offs address.
- * Alias name of args:
+ * Alias name of args (except for $$args) :
* NAME=FETCHARG : set NAME as alias of FETCHARG.
* Type of args:
* FETCHARG:TYPE : use TYPE instead of unsigned long.
@@ -877,7 +930,10 @@ static int __trace_kprobe_create(int argc, const char *argv[])
/* parse arguments */
for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
trace_probe_log_set_index(i + 2);
- ret = traceprobe_parse_probe_arg(&tk->tp, i, argv[i], flags);
+ if (strcmp(argv[i], "$$args") == 0)
+ ret = trace_kprobe_parse_btf_args(tk, i, argv[i], flags);
+ else
+ ret = traceprobe_parse_probe_arg(&tk->tp, i, argv[i], flags);
if (ret)
goto error; /* This can be -ENOMEM */
}
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 2fe104109525..bbac261b1688 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -765,6 +765,111 @@ static int traceprobe_conflict_field_name(const char *name,
return 0;
}
+#ifdef CONFIG_BPF_SYSCALL
+
+static u32 btf_type_int(const struct btf_type *t)
+{
+ return *(u32 *)(t + 1);
+}
+
+static const char *traceprobe_type_from_btf(struct btf *btf, s32 id)
+{
+ const struct btf_type *t;
+ u32 intdata;
+ s32 tid;
+
+ /* TODO: const char * could be converted as a string */
+ t = btf_type_skip_modifiers(btf, id, &tid);
+
+ switch (BTF_INFO_KIND(t->info)) {
+ case BTF_KIND_ENUM:
+ /* enum is "int", so convert to "s32" */
+ return "s32";
+ case BTF_KIND_PTR:
+ /* pointer will be converted to "x??" */
+ if (IS_ENABLED(CONFIG_64BIT))
+ return "x64";
+ else
+ return "x32";
+ case BTF_KIND_INT:
+ intdata = btf_type_int(t);
+ if (BTF_INT_ENCODING(intdata) & BTF_INT_SIGNED) {
+ switch (BTF_INT_BITS(intdata)) {
+ case 8:
+ return "s8";
+ case 16:
+ return "s16";
+ case 32:
+ return "s32";
+ case 64:
+ return "s64";
+ }
+ } else { /* unsigned */
+ switch (BTF_INT_BITS(intdata)) {
+ case 8:
+ return "u8";
+ case 16:
+ return "u16";
+ case 32:
+ return "u32";
+ case 64:
+ return "u64";
+ }
+ }
+ }
+
+ /* Default type */
+ if (IS_ENABLED(CONFIG_64BIT))
+ return "x64";
+ else
+ return "x32";
+}
+
+int traceprobe_parse_btf_arg(struct trace_probe *tp, int i, struct btf *btf,
+ const struct btf_param *arg)
+{
+ struct probe_arg *parg = &tp->args[i];
+ const char *name, *tname;
+ char *body;
+ int ret;
+
+ tp->nr_args++;
+ name = btf_name_by_offset(btf, arg->name_off);
+ parg->name = kstrdup(name, GFP_KERNEL);
+ if (!parg->name)
+ return -ENOMEM;
+
+ if (!is_good_name(parg->name)) {
+ trace_probe_log_err(0, BAD_ARG_NAME);
+ return -EINVAL;
+ }
+ if (traceprobe_conflict_field_name(parg->name, tp->args, i)) {
+ trace_probe_log_err(0, USED_ARG_NAME);
+ return -EINVAL;
+ }
+
+ /*
+ * Since probe event needs an appropriate command for dyn_event interface,
+ * convert BTF type to corresponding fetch-type string.
+ */
+ tname = traceprobe_type_from_btf(btf, arg->type);
+ if (tname)
+ body = kasprintf(GFP_KERNEL, "$arg%d:%s", i + 1, tname);
+ else
+ body = kasprintf(GFP_KERNEL, "$arg%d", i + 1);
+
+ if (!body)
+ return -ENOMEM;
+ /* Parse fetch argument */
+ ret = traceprobe_parse_probe_arg_body(body, &tp->size, parg,
+ TPARG_FL_KERNEL | TPARG_FL_FENTRY, 0);
+
+ kfree(body);
+
+ return ret;
+}
+#endif
+
int traceprobe_parse_probe_arg(struct trace_probe *tp, int i, const char *arg,
unsigned int flags)
{
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 355c78a930f8..857b946afe29 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -23,6 +23,7 @@
#include <linux/limits.h>
#include <linux/uaccess.h>
#include <linux/bitops.h>
+#include <linux/btf.h>
#include <asm/bitsperlong.h>
#include "trace.h"
@@ -359,6 +360,10 @@ int trace_probe_create(const char *raw_command, int (*createfn)(int, const char
extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
const char *argv, unsigned int flags);
+#ifdef CONFIG_BPF_SYSCALL
+int traceprobe_parse_btf_arg(struct trace_probe *tp, int i, struct btf *btf,
+ const struct btf_param *arg);
+#endif
extern int traceprobe_update_arg(struct probe_arg *arg);
extern void traceprobe_free_probe_arg(struct probe_arg *arg);
On 23/09/2021 13:39, Masami Hiramatsu wrote:
> Support $$args fetch arg for function entry. This uses
> BTF for finding the function argument. Thus it depends
> on CONFIG_BPF_SYSCALL.
>
> /sys/kernel/tracing # echo 'p vfs_read $$args' >> kprobe_events
> /sys/kernel/tracing # cat kprobe_events
> p:kprobes/p_vfs_read_0 vfs_read file=$arg1:x64 buf=$arg2:x64 count=$arg3:u64 pos=$arg4:x64
>
> Note that $$args must be used without argument name.
This looks great! Can I ask which tree you're building on
top of so I can play around with this a bit?
I also wonder if we could rework btf_show_name() to help
render full type info for the args? in kernel/bpf/btf.c:
/*
* Populate show->state.name with type name information.
* Format of type name is
*
* [.member_name = ] (type_name)
*/
The (type_name) part is what we'd want from here; no reason
we can't refactor that function to make the type name available
as a cast. It would rework the output to be something like
p:kprobes/p_vfs_read_0 vfs_read struct file *file=$arg1 , char *buf=$arg2 , size_t count=$arg3 , loff_t pos=$arg4
...if that's wanted of course (not sure what the constraints on format are here)? Thanks for pushing this along!
Alan
> Signed-off-by: Masami Hiramatsu <[email protected]>
> ---
> kernel/trace/trace_kprobe.c | 60 ++++++++++++++++++++++++-
> kernel/trace/trace_probe.c | 105 +++++++++++++++++++++++++++++++++++++++++++
> kernel/trace/trace_probe.h | 5 ++
> 3 files changed, 168 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 3dd4fb719aa3..fe88ee8c8cd8 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -712,6 +712,58 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
> return NOTIFY_DONE;
> }
>
> +#ifdef CONFIG_BPF_SYSCALL
> +
> +static int trace_kprobe_parse_btf_args(struct trace_kprobe *tk, int i,
> + const char *arg, unsigned int flags)
> +{
> + struct trace_probe *tp = &tk->tp;
> + static struct btf *btf;
> + const struct btf_type *t;
> + const struct btf_param *args;
> + s32 id, nargs;
> + int ret;
> +
> + if (!(flags & TPARG_FL_FENTRY))
> + return -EINVAL;
> + if (!tk->symbol)
> + return -EINVAL;
> +
> + if (!btf)
> + btf = btf_parse_vmlinux();
> +
> + id = btf_find_by_name_kind(btf, tk->symbol, BTF_KIND_FUNC);
> + if (id <= 0)
> + return -ENOENT;
> +
> + /* Get BTF_KIND_FUNC type */
> + t = btf_type_by_id(btf, id);
> + if (!btf_type_is_func(t))
> + return -ENOENT;
> +
> + /* The type of BTF_KIND_FUNC is BTF_KIND_FUNC_PROTO */
> + t = btf_type_by_id(btf, t->type);
> + if (!btf_type_is_func_proto(t))
> + return -ENOENT;
> +
> + args = (const struct btf_param *)(t + 1);
> + nargs = btf_type_vlen(t);
> + for (i = 0; i < nargs; i++) {
> + ret = traceprobe_parse_btf_arg(tp, i, btf, &args[i]);
> + if (ret < 0)
> + break;
> + }
> +
> + return ret;
> +}
> +#else
> +static int trace_kprobe_parse_btf_args(struct trace_kprobe *tk, int i,
> + const char *arg, unsigned int flags)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif
> +
> static struct notifier_block trace_kprobe_module_nb = {
> .notifier_call = trace_kprobe_module_callback,
> .priority = 1 /* Invoked after kprobe module callback */
> @@ -733,12 +785,13 @@ static int __trace_kprobe_create(int argc, const char *argv[])
> * $stack : fetch stack address
> * $stackN : fetch Nth of stack (N:0-)
> * $comm : fetch current task comm
> + * $$args : fetch parameters using BTF
> * @ADDR : fetch memory at ADDR (ADDR should be in kernel)
> * @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data symbol)
> * %REG : fetch register REG
> * Dereferencing memory fetch:
> * +|-offs(ARG) : fetch memory at ARG +|- offs address.
> - * Alias name of args:
> + * Alias name of args (except for $$args) :
> * NAME=FETCHARG : set NAME as alias of FETCHARG.
> * Type of args:
> * FETCHARG:TYPE : use TYPE instead of unsigned long.
> @@ -877,7 +930,10 @@ static int __trace_kprobe_create(int argc, const char *argv[])
> /* parse arguments */
> for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
> trace_probe_log_set_index(i + 2);
> - ret = traceprobe_parse_probe_arg(&tk->tp, i, argv[i], flags);
> + if (strcmp(argv[i], "$$args") == 0)
> + ret = trace_kprobe_parse_btf_args(tk, i, argv[i], flags);
> + else
> + ret = traceprobe_parse_probe_arg(&tk->tp, i, argv[i], flags);
> if (ret)
> goto error; /* This can be -ENOMEM */
> }
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 2fe104109525..bbac261b1688 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -765,6 +765,111 @@ static int traceprobe_conflict_field_name(const char *name,
> return 0;
> }
>
> +#ifdef CONFIG_BPF_SYSCALL
> +
> +static u32 btf_type_int(const struct btf_type *t)
> +{
> + return *(u32 *)(t + 1);
> +}
> +
> +static const char *traceprobe_type_from_btf(struct btf *btf, s32 id)
> +{
> + const struct btf_type *t;
> + u32 intdata;
> + s32 tid;
> +
> + /* TODO: const char * could be converted as a string */
> + t = btf_type_skip_modifiers(btf, id, &tid);
> +
> + switch (BTF_INFO_KIND(t->info)) {
> + case BTF_KIND_ENUM:
> + /* enum is "int", so convert to "s32" */
> + return "s32";
> + case BTF_KIND_PTR:
> + /* pointer will be converted to "x??" */
> + if (IS_ENABLED(CONFIG_64BIT))
> + return "x64";
> + else
> + return "x32";
> + case BTF_KIND_INT:
> + intdata = btf_type_int(t);
> + if (BTF_INT_ENCODING(intdata) & BTF_INT_SIGNED) {
> + switch (BTF_INT_BITS(intdata)) {
> + case 8:
> + return "s8";
> + case 16:
> + return "s16";
> + case 32:
> + return "s32";
> + case 64:
> + return "s64";
> + }
> + } else { /* unsigned */
> + switch (BTF_INT_BITS(intdata)) {
> + case 8:
> + return "u8";
> + case 16:
> + return "u16";
> + case 32:
> + return "u32";
> + case 64:
> + return "u64";
> + }
> + }
> + }
> +
> + /* Default type */
> + if (IS_ENABLED(CONFIG_64BIT))
> + return "x64";
> + else
> + return "x32";
> +}
> +
> +int traceprobe_parse_btf_arg(struct trace_probe *tp, int i, struct btf *btf,
> + const struct btf_param *arg)
> +{
> + struct probe_arg *parg = &tp->args[i];
> + const char *name, *tname;
> + char *body;
> + int ret;
> +
> + tp->nr_args++;
> + name = btf_name_by_offset(btf, arg->name_off);
> + parg->name = kstrdup(name, GFP_KERNEL);
> + if (!parg->name)
> + return -ENOMEM;
> +
> + if (!is_good_name(parg->name)) {
> + trace_probe_log_err(0, BAD_ARG_NAME);
> + return -EINVAL;
> + }
> + if (traceprobe_conflict_field_name(parg->name, tp->args, i)) {
> + trace_probe_log_err(0, USED_ARG_NAME);
> + return -EINVAL;
> + }
> +
> + /*
> + * Since probe event needs an appropriate command for dyn_event interface,
> + * convert BTF type to corresponding fetch-type string.
> + */
> + tname = traceprobe_type_from_btf(btf, arg->type);
> + if (tname)
> + body = kasprintf(GFP_KERNEL, "$arg%d:%s", i + 1, tname);
> + else
> + body = kasprintf(GFP_KERNEL, "$arg%d", i + 1);
> +
> + if (!body)
> + return -ENOMEM;
> + /* Parse fetch argument */
> + ret = traceprobe_parse_probe_arg_body(body, &tp->size, parg,
> + TPARG_FL_KERNEL | TPARG_FL_FENTRY, 0);
> +
> + kfree(body);
> +
> + return ret;
> +}
> +#endif
> +
> int traceprobe_parse_probe_arg(struct trace_probe *tp, int i, const char *arg,
> unsigned int flags)
> {
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 355c78a930f8..857b946afe29 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -23,6 +23,7 @@
> #include <linux/limits.h>
> #include <linux/uaccess.h>
> #include <linux/bitops.h>
> +#include <linux/btf.h>
> #include <asm/bitsperlong.h>
>
> #include "trace.h"
> @@ -359,6 +360,10 @@ int trace_probe_create(const char *raw_command, int (*createfn)(int, const char
>
> extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
> const char *argv, unsigned int flags);
> +#ifdef CONFIG_BPF_SYSCALL
> +int traceprobe_parse_btf_arg(struct trace_probe *tp, int i, struct btf *btf,
> + const struct btf_param *arg);
> +#endif
>
> extern int traceprobe_update_arg(struct probe_arg *arg);
> extern void traceprobe_free_probe_arg(struct probe_arg *arg);
>
On Fri, 24 Sep 2021 18:51:04 +0100
Alan Maguire <[email protected]> wrote:
> On 23/09/2021 13:39, Masami Hiramatsu wrote:
>
> > Support $$args fetch arg for function entry. This uses
> > BTF for finding the function argument. Thus it depends
> > on CONFIG_BPF_SYSCALL.
> >
> > /sys/kernel/tracing # echo 'p vfs_read $$args' >> kprobe_events
> > /sys/kernel/tracing # cat kprobe_events
> > p:kprobes/p_vfs_read_0 vfs_read file=$arg1:x64 buf=$arg2:x64 count=$arg3:u64 pos=$arg4:x64
> >
> > Note that $$args must be used without argument name.
>
> This looks great! Can I ask which tree you're building on
>
> top of so I can play around with this a bit?
This can be applied on top of Steve's tracing tree, ftrace/core branch.
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
>
>
> I also wonder if we could rework btf_show_name() to help
>
> render full type info for the args? in kernel/bpf/btf.c:
>
>
> /*
> * Populate show->state.name with type name information.
> * Format of type name is
> *
> * [.member_name = ] (type_name)
> */
>
> The (type_name) part is what we'd want from here; no reason
>
> we can't refactor that function to make the type name available
>
> as a cast. It would rework the output to be something like
>
>
> p:kprobes/p_vfs_read_0 vfs_read struct file *file=$arg1 , char *buf=$arg2 , size_t count=$arg3 , loff_t pos=$arg4
>
> ...if that's wanted of course (not sure what the constraints on format are here)? Thanks for pushing this along!
Sorry, the kprobe_events interface syntax doesn't accept such cast info.
And I want such BTF type query interface to be independent from
this kprobe-events interface.
Can we have a query interface like /sys/kernel/btf/query?
Thank you,
>
>
> Alan
>
> > Signed-off-by: Masami Hiramatsu <[email protected]>
> > ---
> > kernel/trace/trace_kprobe.c | 60 ++++++++++++++++++++++++-
> > kernel/trace/trace_probe.c | 105 +++++++++++++++++++++++++++++++++++++++++++
> > kernel/trace/trace_probe.h | 5 ++
> > 3 files changed, 168 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index 3dd4fb719aa3..fe88ee8c8cd8 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -712,6 +712,58 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
> > return NOTIFY_DONE;
> > }
> >
> > +#ifdef CONFIG_BPF_SYSCALL
> > +
> > +static int trace_kprobe_parse_btf_args(struct trace_kprobe *tk, int i,
> > + const char *arg, unsigned int flags)
> > +{
> > + struct trace_probe *tp = &tk->tp;
> > + static struct btf *btf;
> > + const struct btf_type *t;
> > + const struct btf_param *args;
> > + s32 id, nargs;
> > + int ret;
> > +
> > + if (!(flags & TPARG_FL_FENTRY))
> > + return -EINVAL;
> > + if (!tk->symbol)
> > + return -EINVAL;
> > +
> > + if (!btf)
> > + btf = btf_parse_vmlinux();
> > +
> > + id = btf_find_by_name_kind(btf, tk->symbol, BTF_KIND_FUNC);
> > + if (id <= 0)
> > + return -ENOENT;
> > +
> > + /* Get BTF_KIND_FUNC type */
> > + t = btf_type_by_id(btf, id);
> > + if (!btf_type_is_func(t))
> > + return -ENOENT;
> > +
> > + /* The type of BTF_KIND_FUNC is BTF_KIND_FUNC_PROTO */
> > + t = btf_type_by_id(btf, t->type);
> > + if (!btf_type_is_func_proto(t))
> > + return -ENOENT;
> > +
> > + args = (const struct btf_param *)(t + 1);
> > + nargs = btf_type_vlen(t);
> > + for (i = 0; i < nargs; i++) {
> > + ret = traceprobe_parse_btf_arg(tp, i, btf, &args[i]);
> > + if (ret < 0)
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +#else
> > +static int trace_kprobe_parse_btf_args(struct trace_kprobe *tk, int i,
> > + const char *arg, unsigned int flags)
> > +{
> > + return -EOPNOTSUPP;
> > +}
> > +#endif
> > +
> > static struct notifier_block trace_kprobe_module_nb = {
> > .notifier_call = trace_kprobe_module_callback,
> > .priority = 1 /* Invoked after kprobe module callback */
> > @@ -733,12 +785,13 @@ static int __trace_kprobe_create(int argc, const char *argv[])
> > * $stack : fetch stack address
> > * $stackN : fetch Nth of stack (N:0-)
> > * $comm : fetch current task comm
> > + * $$args : fetch parameters using BTF
> > * @ADDR : fetch memory at ADDR (ADDR should be in kernel)
> > * @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data symbol)
> > * %REG : fetch register REG
> > * Dereferencing memory fetch:
> > * +|-offs(ARG) : fetch memory at ARG +|- offs address.
> > - * Alias name of args:
> > + * Alias name of args (except for $$args) :
> > * NAME=FETCHARG : set NAME as alias of FETCHARG.
> > * Type of args:
> > * FETCHARG:TYPE : use TYPE instead of unsigned long.
> > @@ -877,7 +930,10 @@ static int __trace_kprobe_create(int argc, const char *argv[])
> > /* parse arguments */
> > for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
> > trace_probe_log_set_index(i + 2);
> > - ret = traceprobe_parse_probe_arg(&tk->tp, i, argv[i], flags);
> > + if (strcmp(argv[i], "$$args") == 0)
> > + ret = trace_kprobe_parse_btf_args(tk, i, argv[i], flags);
> > + else
> > + ret = traceprobe_parse_probe_arg(&tk->tp, i, argv[i], flags);
> > if (ret)
> > goto error; /* This can be -ENOMEM */
> > }
> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index 2fe104109525..bbac261b1688 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -765,6 +765,111 @@ static int traceprobe_conflict_field_name(const char *name,
> > return 0;
> > }
> >
> > +#ifdef CONFIG_BPF_SYSCALL
> > +
> > +static u32 btf_type_int(const struct btf_type *t)
> > +{
> > + return *(u32 *)(t + 1);
> > +}
> > +
> > +static const char *traceprobe_type_from_btf(struct btf *btf, s32 id)
> > +{
> > + const struct btf_type *t;
> > + u32 intdata;
> > + s32 tid;
> > +
> > + /* TODO: const char * could be converted as a string */
> > + t = btf_type_skip_modifiers(btf, id, &tid);
> > +
> > + switch (BTF_INFO_KIND(t->info)) {
> > + case BTF_KIND_ENUM:
> > + /* enum is "int", so convert to "s32" */
> > + return "s32";
> > + case BTF_KIND_PTR:
> > + /* pointer will be converted to "x??" */
> > + if (IS_ENABLED(CONFIG_64BIT))
> > + return "x64";
> > + else
> > + return "x32";
> > + case BTF_KIND_INT:
> > + intdata = btf_type_int(t);
> > + if (BTF_INT_ENCODING(intdata) & BTF_INT_SIGNED) {
> > + switch (BTF_INT_BITS(intdata)) {
> > + case 8:
> > + return "s8";
> > + case 16:
> > + return "s16";
> > + case 32:
> > + return "s32";
> > + case 64:
> > + return "s64";
> > + }
> > + } else { /* unsigned */
> > + switch (BTF_INT_BITS(intdata)) {
> > + case 8:
> > + return "u8";
> > + case 16:
> > + return "u16";
> > + case 32:
> > + return "u32";
> > + case 64:
> > + return "u64";
> > + }
> > + }
> > + }
> > +
> > + /* Default type */
> > + if (IS_ENABLED(CONFIG_64BIT))
> > + return "x64";
> > + else
> > + return "x32";
> > +}
> > +
> > +int traceprobe_parse_btf_arg(struct trace_probe *tp, int i, struct btf *btf,
> > + const struct btf_param *arg)
> > +{
> > + struct probe_arg *parg = &tp->args[i];
> > + const char *name, *tname;
> > + char *body;
> > + int ret;
> > +
> > + tp->nr_args++;
> > + name = btf_name_by_offset(btf, arg->name_off);
> > + parg->name = kstrdup(name, GFP_KERNEL);
> > + if (!parg->name)
> > + return -ENOMEM;
> > +
> > + if (!is_good_name(parg->name)) {
> > + trace_probe_log_err(0, BAD_ARG_NAME);
> > + return -EINVAL;
> > + }
> > + if (traceprobe_conflict_field_name(parg->name, tp->args, i)) {
> > + trace_probe_log_err(0, USED_ARG_NAME);
> > + return -EINVAL;
> > + }
> > +
> > + /*
> > + * Since probe event needs an appropriate command for dyn_event interface,
> > + * convert BTF type to corresponding fetch-type string.
> > + */
> > + tname = traceprobe_type_from_btf(btf, arg->type);
> > + if (tname)
> > + body = kasprintf(GFP_KERNEL, "$arg%d:%s", i + 1, tname);
> > + else
> > + body = kasprintf(GFP_KERNEL, "$arg%d", i + 1);
> > +
> > + if (!body)
> > + return -ENOMEM;
> > + /* Parse fetch argument */
> > + ret = traceprobe_parse_probe_arg_body(body, &tp->size, parg,
> > + TPARG_FL_KERNEL | TPARG_FL_FENTRY, 0);
> > +
> > + kfree(body);
> > +
> > + return ret;
> > +}
> > +#endif
> > +
> > int traceprobe_parse_probe_arg(struct trace_probe *tp, int i, const char *arg,
> > unsigned int flags)
> > {
> > diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> > index 355c78a930f8..857b946afe29 100644
> > --- a/kernel/trace/trace_probe.h
> > +++ b/kernel/trace/trace_probe.h
> > @@ -23,6 +23,7 @@
> > #include <linux/limits.h>
> > #include <linux/uaccess.h>
> > #include <linux/bitops.h>
> > +#include <linux/btf.h>
> > #include <asm/bitsperlong.h>
> >
> > #include "trace.h"
> > @@ -359,6 +360,10 @@ int trace_probe_create(const char *raw_command, int (*createfn)(int, const char
> >
> > extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
> > const char *argv, unsigned int flags);
> > +#ifdef CONFIG_BPF_SYSCALL
> > +int traceprobe_parse_btf_arg(struct trace_probe *tp, int i, struct btf *btf,
> > + const struct btf_param *arg);
> > +#endif
> >
> > extern int traceprobe_update_arg(struct probe_arg *arg);
> > extern void traceprobe_free_probe_arg(struct probe_arg *arg);
> >
--
Masami Hiramatsu <[email protected]>
On Thu, 23 Sep 2021 21:39:43 +0900
Masami Hiramatsu <[email protected]> wrote:
> Hi Steve,
>
Hi Masami,
Sorry for the late reply, but Plumbers followed by OSS put me way behind,
and I just got to this email :-/
> Here I share my testing patch of the BTF for kprobe events.
> Currently this only allow user to specify '$$args' for
> tracing all arguments of the function. This is only
> avaialbe if
> - the probe point is on the function entry
> - the kernel is compiled with BTF (CONFIG_DEBUG_INFO_BTF)
> - the kernel is enables BPF (CONFIG_BPF_SYSCALL)
>
> And Special thanks to Sven! Most of BTF handling part of
> this patch comes from his patch [1]
>
> [1] https://stackframe.org/0001-ftrace-arg-hack.patch
Which is newer than this patch because he sent a v2, and that's a couple
patches down in my queue. I'll be looking at that one shortly as well.
>
> What I thought while coding this were;
> - kernel/bpf/btf.c can be moved under lib/ so that
> the other subsystems can reuse it, independent
> from BPF. (Also, this should depends on CONFIG_DEBUG_INFO_BTF)
Makes sense.
> - some more utility functions can be exposed.
> e.g. I copied btf_type_int() from btf.c
Agreed.
> - If there are more comments for the BTF APIs, it will
> be more useful...
> - Overall, the BTF is easy to understand for who
> already understand DWARF. Great work!
Great to hear.
> - I think I need 'ptr' and 'bool' types for fetcharg types.
>
> Anyway, this is just for testing. I have to add some
> more cleanup, features and documentations, etc.
This is awesome, and something to look at for a generic ftrace args point
of view too.
One issue is how do we handle multiple register values? Like a u64 type on
32 bit? As $arg1 is just a register that is in $arg1, for a u64 parameter
on 32 bit, that is usually handled with two registers.
Have thoughts on that?
I'll play with your patch today.
-- Steve
On Mon, 11 Oct 2021 18:23:34 -0400
Steven Rostedt <[email protected]> wrote:
> On Thu, 23 Sep 2021 21:39:43 +0900
> Masami Hiramatsu <[email protected]> wrote:
>
> > Hi Steve,
> >
>
> Hi Masami,
>
> Sorry for the late reply, but Plumbers followed by OSS put me way behind,
> and I just got to this email :-/
>
> > Here I share my testing patch of the BTF for kprobe events.
> > Currently this only allow user to specify '$$args' for
> > tracing all arguments of the function. This is only
> > avaialbe if
> > - the probe point is on the function entry
> > - the kernel is compiled with BTF (CONFIG_DEBUG_INFO_BTF)
> > - the kernel is enables BPF (CONFIG_BPF_SYSCALL)
> >
> > And Special thanks to Sven! Most of BTF handling part of
> > this patch comes from his patch [1]
> >
> > [1] https://stackframe.org/0001-ftrace-arg-hack.patch
>
> Which is newer than this patch because he sent a v2, and that's a couple
> patches down in my queue. I'll be looking at that one shortly as well.
Did he send his BTF hack patch to you ?
I didn't notice that.
> > What I thought while coding this were;
> > - kernel/bpf/btf.c can be moved under lib/ so that
> > the other subsystems can reuse it, independent
> > from BPF. (Also, this should depends on CONFIG_DEBUG_INFO_BTF)
>
> Makes sense.
>
> > - some more utility functions can be exposed.
> > e.g. I copied btf_type_int() from btf.c
>
> Agreed.
>
> > - If there are more comments for the BTF APIs, it will
> > be more useful...
> > - Overall, the BTF is easy to understand for who
> > already understand DWARF. Great work!
>
> Great to hear.
>
> > - I think I need 'ptr' and 'bool' types for fetcharg types.
> >
> > Anyway, this is just for testing. I have to add some
> > more cleanup, features and documentations, etc.
>
> This is awesome, and something to look at for a generic ftrace args point
> of view too.
>
> One issue is how do we handle multiple register values? Like a u64 type on
> 32 bit? As $arg1 is just a register that is in $arg1, for a u64 parameter
> on 32 bit, that is usually handled with two registers.
>
> Have thoughts on that?
Oh, that's a good point! The probe event supports such case, since I expected
the user will use 2 arguments to record it. But indeed, using BTF means we need
such extension.
OK, let me consider how to extend fetchargs to support it.
Thank you!
>
> I'll play with your patch today.
>
> -- Steve
--
Masami Hiramatsu <[email protected]>