* [PATCH] Add BPF Kernel Function bpf_ptrace_vprintk
@ 2024-09-25 10:02 Eric Yan
2024-09-26 3:34 ` kernel test robot
0 siblings, 1 reply; 6+ messages in thread
From: Eric Yan @ 2024-09-25 10:02 UTC (permalink / raw)
To: ast, daniel, andrii, martin.lau, yonghong.song, song,
john.fastabend, kpsingh, sdf, haoluo, jolsa
Cc: bpf, linux-kernel, eric.yan
add a kfunc 'bpf_ptrace_vprintk' printing bpf msg with trace_marker
format requirement so that these msgs can be retrieved by android
perfetto by default and well represented in perfetto UI.
[testing prog]
const volatile bool ptrace_enabled = true;
extern int bpf_ptrace_vprintk(char *fmt, u32 fmt_size, const void *args, u32 args__sz) __ksym;
({ \
if (!ptrace_enabled) { \
bpf_printk(fmt, __VA_ARGS__); \
} else { \
char __fmt[] = fmt; \
_Pragma("GCC diagnostic push") \
_Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
u64 __params[] = { __VA_ARGS__ }; \
_Pragma("GCC diagnostic pop") \
bpf_ptrace_vprintk(__fmt, sizeof(__fmt), __params, sizeof(__params)); \
} \
})
SEC("perf_event")
int do_sample(struct bpf_perf_event_data *ctx)
{
u64 ip = PT_REGS_IP(&ctx->regs);
u64 id = bpf_get_current_pid_tgid();
s32 pid = id >> 32;
s32 tid = id;
debug_printk("N|%d|BPRF-%d|BPRF:%llx", pid, tid, ip);
return 0;
}
[output]:
app-3151 [000] d.h1. 6059.904239: tracing_mark_write: N|2491|BPRF-3151|BPRF:58750d0eec
Signed-off-by: Eric Yan <eric.yan@oppo.com>
---
kernel/bpf/helpers.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 1a43d06eab28..e3d17d3b17c5 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2521,6 +2521,39 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid)
return p;
}
+static noinline void tracing_mark_write(char *buf)
+{
+ trace_printk(buf);
+}
+
+/**
+ * same as bpf_trace_vprintk, except for a trace_marker format requirement
+ */
+__bpf_kfunc int bpf_ptrace_vprintk(char *fmt, u32 fmt_size, const void *args, u32 args__sz)
+{
+ struct bpf_bprintf_data data = {
+ .get_bin_args = true,
+ .get_buf = true,
+ };
+ int ret, num_args;
+
+ if (args__sz & 7 || args__sz > MAX_BPRINTF_VARARGS * 8 || (args__sz && !args))
+ return -EINVAL;
+ num_args = args__sz / 8;
+
+ ret = bpf_bprintf_prepare(fmt, fmt_size, args, num_args, &data);
+ if (ret < 0)
+ return ret;
+
+ ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
+
+ tracing_mark_write(data.buf);
+
+ bpf_bprintf_cleanup(&data);
+
+ return ret;
+}
+
/**
* bpf_dynptr_slice() - Obtain a read-only pointer to the dynptr data.
* @p: The dynptr whose data slice to retrieve
@@ -3090,6 +3123,7 @@ BTF_ID_FLAGS(func, bpf_iter_bits_new, KF_ITER_NEW)
BTF_ID_FLAGS(func, bpf_iter_bits_next, KF_ITER_NEXT | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_iter_bits_destroy, KF_ITER_DESTROY)
BTF_ID_FLAGS(func, bpf_copy_from_user_str, KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_ptrace_vprintk)
BTF_KFUNCS_END(common_btf_ids)
static const struct btf_kfunc_id_set common_kfunc_set = {
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Add BPF Kernel Function bpf_ptrace_vprintk
2024-09-25 10:02 [PATCH] Add BPF Kernel Function bpf_ptrace_vprintk Eric Yan
@ 2024-09-26 3:34 ` kernel test robot
2024-09-26 7:27 ` [PATCH v2] " Eric Yan
0 siblings, 1 reply; 6+ messages in thread
From: kernel test robot @ 2024-09-26 3:34 UTC (permalink / raw)
To: Eric Yan, ast, daniel, andrii, martin.lau, yonghong.song, song,
john.fastabend, kpsingh, sdf, haoluo, jolsa
Cc: oe-kbuild-all, bpf, linux-kernel, eric.yan
Hi Eric,
kernel test robot noticed the following build warnings:
[auto build test WARNING on bpf-next/master]
[also build test WARNING on bpf/master linus/master next-20240925]
[cannot apply to v6.11]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Eric-Yan/Add-BPF-Kernel-Function-bpf_ptrace_vprintk/20240925-180530
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20240925100254.436-1-eric.yan%40oppo.com
patch subject: [PATCH] Add BPF Kernel Function bpf_ptrace_vprintk
config: x86_64-buildonly-randconfig-003-20240926 (https://download.01.org/0day-ci/archive/20240926/202409261116.risxWG3M-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240926/202409261116.risxWG3M-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409261116.risxWG3M-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> kernel/bpf/helpers.c:2530: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* same as bpf_trace_vprintk, except for a trace_marker format requirement
vim +2530 kernel/bpf/helpers.c
2528
2529 /**
> 2530 * same as bpf_trace_vprintk, except for a trace_marker format requirement
2531 */
2532 __bpf_kfunc int bpf_ptrace_vprintk(char *fmt, u32 fmt_size, const void *args, u32 args__sz)
2533 {
2534 struct bpf_bprintf_data data = {
2535 .get_bin_args = true,
2536 .get_buf = true,
2537 };
2538 int ret, num_args;
2539
2540 if (args__sz & 7 || args__sz > MAX_BPRINTF_VARARGS * 8 || (args__sz && !args))
2541 return -EINVAL;
2542 num_args = args__sz / 8;
2543
2544 ret = bpf_bprintf_prepare(fmt, fmt_size, args, num_args, &data);
2545 if (ret < 0)
2546 return ret;
2547
2548 ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
2549
2550 tracing_mark_write(data.buf);
2551
2552 bpf_bprintf_cleanup(&data);
2553
2554 return ret;
2555 }
2556
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] Add BPF Kernel Function bpf_ptrace_vprintk
2024-09-26 3:34 ` kernel test robot
@ 2024-09-26 7:27 ` Eric Yan
2024-09-29 17:10 ` Alexei Starovoitov
0 siblings, 1 reply; 6+ messages in thread
From: Eric Yan @ 2024-09-26 7:27 UTC (permalink / raw)
To: lkp
Cc: andrii, ast, bpf, daniel, eric.yan, haoluo, john.fastabend, jolsa,
kpsingh, linux-kernel, martin.lau, oe-kbuild-all, sdf, song,
yonghong.song
add a kfunc 'bpf_ptrace_vprintk' printing bpf msg with trace_marker
format requirement so that these msgs can be retrieved by android
perfetto by default and well represented in perfetto UI.
[testing prog]
const volatile bool ptrace_enabled = true;
extern int bpf_ptrace_vprintk(char *fmt, u32 fmt_size, const void *args, u32 args__sz) __ksym;
({ \
if (!ptrace_enabled) { \
bpf_printk(fmt, __VA_ARGS__); \
} else { \
char __fmt[] = fmt; \
_Pragma("GCC diagnostic push") \
_Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
u64 __params[] = { __VA_ARGS__ }; \
_Pragma("GCC diagnostic pop") \
bpf_ptrace_vprintk(__fmt, sizeof(__fmt), __params, sizeof(__params)); \
} \
})
SEC("perf_event")
int do_sample(struct bpf_perf_event_data *ctx)
{
u64 ip = PT_REGS_IP(&ctx->regs);
u64 id = bpf_get_current_pid_tgid();
s32 pid = id >> 32;
s32 tid = id;
debug_printk("N|%d|BPRF-%d|BPRF:%llx", pid, tid, ip);
return 0;
}
[output]:
app-3151 [000] d.h1. 6059.904239: tracing_mark_write: N|2491|BPRF-3151|BPRF:58750d0eec
Signed-off-by: Eric Yan <eric.yan@oppo.com>
---
kernel/bpf/helpers.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 1a43d06eab28..1e37dae74ca6 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2521,6 +2521,39 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid)
return p;
}
+static noinline void tracing_mark_write(char *buf)
+{
+ trace_printk(buf);
+}
+
+/* same as bpf_trace_vprintk, only with a trace_marker format requirement
+ * @fmt: Format string, e.g. <B|E|C|N>|<%d:pid>|<%s:TAG>...
+ */
+__bpf_kfunc int bpf_ptrace_vprintk(char *fmt, u32 fmt_size, const void *args, u32 args__sz)
+{
+ struct bpf_bprintf_data data = {
+ .get_bin_args = true,
+ .get_buf = true,
+ };
+ int ret, num_args;
+
+ if (args__sz & 7 || args__sz > MAX_BPRINTF_VARARGS * 8 || (args__sz && !args))
+ return -EINVAL;
+ num_args = args__sz / 8;
+
+ ret = bpf_bprintf_prepare(fmt, fmt_size, args, num_args, &data);
+ if (ret < 0)
+ return ret;
+
+ ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
+
+ tracing_mark_write(data.buf);
+
+ bpf_bprintf_cleanup(&data);
+
+ return ret;
+}
+
/**
* bpf_dynptr_slice() - Obtain a read-only pointer to the dynptr data.
* @p: The dynptr whose data slice to retrieve
@@ -3090,6 +3123,7 @@ BTF_ID_FLAGS(func, bpf_iter_bits_new, KF_ITER_NEW)
BTF_ID_FLAGS(func, bpf_iter_bits_next, KF_ITER_NEXT | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_iter_bits_destroy, KF_ITER_DESTROY)
BTF_ID_FLAGS(func, bpf_copy_from_user_str, KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_ptrace_vprintk)
BTF_KFUNCS_END(common_btf_ids)
static const struct btf_kfunc_id_set common_kfunc_set = {
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Add BPF Kernel Function bpf_ptrace_vprintk
2024-09-26 7:27 ` [PATCH v2] " Eric Yan
@ 2024-09-29 17:10 ` Alexei Starovoitov
2024-09-30 8:29 ` 答复: " 燕青洲(Eric Yan)
0 siblings, 1 reply; 6+ messages in thread
From: Alexei Starovoitov @ 2024-09-29 17:10 UTC (permalink / raw)
To: Eric Yan
Cc: kbuild test robot, Andrii Nakryiko, Alexei Starovoitov, bpf,
Daniel Borkmann, Hao Luo, John Fastabend, Jiri Olsa, KP Singh,
LKML, Martin KaFai Lau, oe-kbuild-all, Stanislav Fomichev,
Song Liu, Yonghong Song
On Thu, Sep 26, 2024 at 12:28 AM Eric Yan <eric.yan@oppo.com> wrote:
>
> add a kfunc 'bpf_ptrace_vprintk' printing bpf msg with trace_marker
> format requirement so that these msgs can be retrieved by android
> perfetto by default and well represented in perfetto UI.
>
> [testing prog]
> const volatile bool ptrace_enabled = true;
> extern int bpf_ptrace_vprintk(char *fmt, u32 fmt_size, const void *args, u32 args__sz) __ksym;
>
> ({ \
> if (!ptrace_enabled) { \
> bpf_printk(fmt, __VA_ARGS__); \
> } else { \
> char __fmt[] = fmt; \
> _Pragma("GCC diagnostic push") \
> _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
> u64 __params[] = { __VA_ARGS__ }; \
> _Pragma("GCC diagnostic pop") \
> bpf_ptrace_vprintk(__fmt, sizeof(__fmt), __params, sizeof(__params)); \
> } \
> })
>
> SEC("perf_event")
> int do_sample(struct bpf_perf_event_data *ctx)
> {
> u64 ip = PT_REGS_IP(&ctx->regs);
> u64 id = bpf_get_current_pid_tgid();
> s32 pid = id >> 32;
> s32 tid = id;
> debug_printk("N|%d|BPRF-%d|BPRF:%llx", pid, tid, ip);
> return 0;
> }
>
> [output]:
> app-3151 [000] d.h1. 6059.904239: tracing_mark_write: N|2491|BPRF-3151|BPRF:58750d0eec
>
> Signed-off-by: Eric Yan <eric.yan@oppo.com>
> ---
> kernel/bpf/helpers.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 1a43d06eab28..1e37dae74ca6 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2521,6 +2521,39 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid)
> return p;
> }
>
> +static noinline void tracing_mark_write(char *buf)
> +{
> + trace_printk(buf);
> +}
> +
> +/* same as bpf_trace_vprintk, only with a trace_marker format requirement
> + * @fmt: Format string, e.g. <B|E|C|N>|<%d:pid>|<%s:TAG>...
> + */
> +__bpf_kfunc int bpf_ptrace_vprintk(char *fmt, u32 fmt_size, const void *args, u32 args__sz)
> +{
> + struct bpf_bprintf_data data = {
> + .get_bin_args = true,
> + .get_buf = true,
> + };
> + int ret, num_args;
> +
> + if (args__sz & 7 || args__sz > MAX_BPRINTF_VARARGS * 8 || (args__sz && !args))
> + return -EINVAL;
> + num_args = args__sz / 8;
> +
> + ret = bpf_bprintf_prepare(fmt, fmt_size, args, num_args, &data);
> + if (ret < 0)
> + return ret;
> +
> + ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
> +
> + tracing_mark_write(data.buf);
> +
> + bpf_bprintf_cleanup(&data);
> +
> + return ret;
> +}
> +
> /**
> * bpf_dynptr_slice() - Obtain a read-only pointer to the dynptr data.
> * @p: The dynptr whose data slice to retrieve
> @@ -3090,6 +3123,7 @@ BTF_ID_FLAGS(func, bpf_iter_bits_new, KF_ITER_NEW)
> BTF_ID_FLAGS(func, bpf_iter_bits_next, KF_ITER_NEXT | KF_RET_NULL)
> BTF_ID_FLAGS(func, bpf_iter_bits_destroy, KF_ITER_DESTROY)
> BTF_ID_FLAGS(func, bpf_copy_from_user_str, KF_SLEEPABLE)
> +BTF_ID_FLAGS(func, bpf_ptrace_vprintk)
> BTF_KFUNCS_END(common_btf_ids)
Why new kfunc?
Use bpf_snprintf() and follow with bpf_trace_printk() ?
^ permalink raw reply [flat|nested] 6+ messages in thread
* 答复: [PATCH v2] Add BPF Kernel Function bpf_ptrace_vprintk
2024-09-29 17:10 ` Alexei Starovoitov
@ 2024-09-30 8:29 ` 燕青洲(Eric Yan)
2024-09-30 23:17 ` Andrii Nakryiko
0 siblings, 1 reply; 6+ messages in thread
From: 燕青洲(Eric Yan) @ 2024-09-30 8:29 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: kbuild test robot, Andrii Nakryiko, Alexei Starovoitov, bpf,
Daniel Borkmann, Hao Luo, John Fastabend, Jiri Olsa, KP Singh,
LKML, Martin KaFai Lau, oe-kbuild-all@lists.linux.dev,
Stanislav Fomichev, Song Liu, Yonghong Song
This patch is mainly considered based on the Android Perfetto (A powerful trace collection and analysis tool, support ftrace data source).
The output of bpf_trace_printk and bpf_vtrace_printk in ftrace is like:
app-12345 [001] d... 654321.1970001: bpf_trace_printk: blabla..
FUNCTION field of this kind of message is 'bpf_trace_printk', and there's no standard syntax format for it.
Currently, Perfetto doesn't collect 'bpf_trace/bpf_trace_printk' trace event by default, but does support
'tracing_mark_write' function style by default, such as:
app-3151 [000] d.h1. 6059.904239: tracing_mark_write: B|2491|BPRF-3151|TracingFunc
app-3151 [000] d.h1. 6059.904239: tracing_mark_write: E|2491
Therefore, it's considered to add this kfunc to output formatted BPF messages to ftrace like trace_marker,
allowing perfetto to collect and parse 'tracing_mark_write' events by default and eventually visualize them in the perfetto UI.
-----邮件原件-----
发件人: Alexei Starovoitov <alexei.starovoitov@gmail.com>
发送时间: 2024年9月30日 1:10
收件人: 燕青洲(Eric Yan) <eric.yan@oppo.com>
抄送: kbuild test robot <lkp@intel.com>; Andrii Nakryiko <andrii@kernel.org>; Alexei Starovoitov <ast@kernel.org>; bpf <bpf@vger.kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Hao Luo <haoluo@google.com>; John Fastabend <john.fastabend@gmail.com>; Jiri Olsa <jolsa@kernel.org>; KP Singh <kpsingh@kernel.org>; LKML <linux-kernel@vger.kernel.org>; Martin KaFai Lau <martin.lau@linux.dev>; oe-kbuild-all@lists.linux.dev; Stanislav Fomichev <sdf@fomichev.me>; Song Liu <song@kernel.org>; Yonghong Song <yonghong.song@linux.dev>
主题: Re: [PATCH v2] Add BPF Kernel Function bpf_ptrace_vprintk
On Thu, Sep 26, 2024 at 12:28 AM Eric Yan <eric.yan@oppo.com> wrote:
>
> add a kfunc 'bpf_ptrace_vprintk' printing bpf msg with trace_marker
> format requirement so that these msgs can be retrieved by android
> perfetto by default and well represented in perfetto UI.
>
> [testing prog]
> const volatile bool ptrace_enabled = true; extern int
> bpf_ptrace_vprintk(char *fmt, u32 fmt_size, const void *args, u32
> args__sz) __ksym;
>
> ({ \
> if (!ptrace_enabled) { \
> bpf_printk(fmt, __VA_ARGS__); \
> } else { \
> char __fmt[] = fmt; \
> _Pragma("GCC diagnostic push") \
> _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
> u64 __params[] = { __VA_ARGS__ }; \
> _Pragma("GCC diagnostic pop") \
> bpf_ptrace_vprintk(__fmt, sizeof(__fmt), __params, sizeof(__params)); \
> } \
> })
>
> SEC("perf_event")
> int do_sample(struct bpf_perf_event_data *ctx) {
> u64 ip = PT_REGS_IP(&ctx->regs);
> u64 id = bpf_get_current_pid_tgid();
> s32 pid = id >> 32;
> s32 tid = id;
> debug_printk("N|%d|BPRF-%d|BPRF:%llx", pid, tid, ip);
> return 0;
> }
>
> [output]:
> app-3151 [000] d.h1. 6059.904239: tracing_mark_write: N|2491|BPRF-3151|BPRF:58750d0eec
>
> Signed-off-by: Eric Yan <eric.yan@oppo.com>
> ---
> kernel/bpf/helpers.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index
> 1a43d06eab28..1e37dae74ca6 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2521,6 +2521,39 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid)
> return p;
> }
>
> +static noinline void tracing_mark_write(char *buf) {
> + trace_printk(buf);
> +}
> +
> +/* same as bpf_trace_vprintk, only with a trace_marker format
> +requirement
> + * @fmt: Format string, e.g. <B|E|C|N>|<%d:pid>|<%s:TAG>...
> + */
> +__bpf_kfunc int bpf_ptrace_vprintk(char *fmt, u32 fmt_size, const
> +void *args, u32 args__sz) {
> + struct bpf_bprintf_data data = {
> + .get_bin_args = true,
> + .get_buf = true,
> + };
> + int ret, num_args;
> +
> + if (args__sz & 7 || args__sz > MAX_BPRINTF_VARARGS * 8 || (args__sz && !args))
> + return -EINVAL;
> + num_args = args__sz / 8;
> +
> + ret = bpf_bprintf_prepare(fmt, fmt_size, args, num_args, &data);
> + if (ret < 0)
> + return ret;
> +
> + ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt,
> + data.bin_args);
> +
> + tracing_mark_write(data.buf);
> +
> + bpf_bprintf_cleanup(&data);
> +
> + return ret;
> +}
> +
> /**
> * bpf_dynptr_slice() - Obtain a read-only pointer to the dynptr data.
> * @p: The dynptr whose data slice to retrieve @@ -3090,6 +3123,7 @@
> BTF_ID_FLAGS(func, bpf_iter_bits_new, KF_ITER_NEW) BTF_ID_FLAGS(func,
> bpf_iter_bits_next, KF_ITER_NEXT | KF_RET_NULL) BTF_ID_FLAGS(func,
> bpf_iter_bits_destroy, KF_ITER_DESTROY) BTF_ID_FLAGS(func,
> bpf_copy_from_user_str, KF_SLEEPABLE)
> +BTF_ID_FLAGS(func, bpf_ptrace_vprintk)
> BTF_KFUNCS_END(common_btf_ids)
Why new kfunc?
Use bpf_snprintf() and follow with bpf_trace_printk() ?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Add BPF Kernel Function bpf_ptrace_vprintk
2024-09-30 8:29 ` 答复: " 燕青洲(Eric Yan)
@ 2024-09-30 23:17 ` Andrii Nakryiko
0 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2024-09-30 23:17 UTC (permalink / raw)
To: 燕青洲(Eric Yan)
Cc: Alexei Starovoitov, kbuild test robot, Andrii Nakryiko,
Alexei Starovoitov, bpf, Daniel Borkmann, Hao Luo, John Fastabend,
Jiri Olsa, KP Singh, LKML, Martin KaFai Lau,
oe-kbuild-all@lists.linux.dev, Stanislav Fomichev, Song Liu,
Yonghong Song
On Mon, Sep 30, 2024 at 1:29 AM 燕青洲(Eric Yan) <eric.yan@oppo.com> wrote:
>
> This patch is mainly considered based on the Android Perfetto (A powerful trace collection and analysis tool, support ftrace data source).
> The output of bpf_trace_printk and bpf_vtrace_printk in ftrace is like:
> app-12345 [001] d... 654321.1970001: bpf_trace_printk: blabla..
>
> FUNCTION field of this kind of message is 'bpf_trace_printk', and there's no standard syntax format for it.
> Currently, Perfetto doesn't collect 'bpf_trace/bpf_trace_printk' trace event by default, but does support
> 'tracing_mark_write' function style by default, such as:
> app-3151 [000] d.h1. 6059.904239: tracing_mark_write: B|2491|BPRF-3151|TracingFunc
> app-3151 [000] d.h1. 6059.904239: tracing_mark_write: E|2491
>
> Therefore, it's considered to add this kfunc to output formatted BPF messages to ftrace like trace_marker,
> allowing perfetto to collect and parse 'tracing_mark_write' events by default and eventually visualize them in the perfetto UI.
This does seem like a bit of an overkill to add a new kfunc just to
have "tracing_mark_write" instead of "bpf_trace_printk". Is there any
chance that perfetto can be changed to also track bpf_trace_printk,
perhaps with some pre-agreed upon prefix or something? E.g,
app-3151 [000] d.h1. 6059.904239: bpf_trace_printk:
!B|2491|BPRF-3151|TracingFunc
app-3151 [000] d.h1. 6059.904239: bpf_trace_printk: !E|2491
Generally speaking, bpf_trace_printk() shouldn't be used in production
setup (much), so perhaps parsing everything from bpf_trace_printk() is
OK (assuming it follows this vertical bar syntax)?
>
> -----邮件原件-----
> 发件人: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> 发送时间: 2024年9月30日 1:10
> 收件人: 燕青洲(Eric Yan) <eric.yan@oppo.com>
> 抄送: kbuild test robot <lkp@intel.com>; Andrii Nakryiko <andrii@kernel.org>; Alexei Starovoitov <ast@kernel.org>; bpf <bpf@vger.kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Hao Luo <haoluo@google.com>; John Fastabend <john.fastabend@gmail.com>; Jiri Olsa <jolsa@kernel.org>; KP Singh <kpsingh@kernel.org>; LKML <linux-kernel@vger.kernel.org>; Martin KaFai Lau <martin.lau@linux.dev>; oe-kbuild-all@lists.linux.dev; Stanislav Fomichev <sdf@fomichev.me>; Song Liu <song@kernel.org>; Yonghong Song <yonghong.song@linux.dev>
> 主题: Re: [PATCH v2] Add BPF Kernel Function bpf_ptrace_vprintk
>
> On Thu, Sep 26, 2024 at 12:28 AM Eric Yan <eric.yan@oppo.com> wrote:
> >
> > add a kfunc 'bpf_ptrace_vprintk' printing bpf msg with trace_marker
> > format requirement so that these msgs can be retrieved by android
> > perfetto by default and well represented in perfetto UI.
> >
> > [testing prog]
> > const volatile bool ptrace_enabled = true; extern int
> > bpf_ptrace_vprintk(char *fmt, u32 fmt_size, const void *args, u32
> > args__sz) __ksym;
> >
> > ({ \
> > if (!ptrace_enabled) { \
> > bpf_printk(fmt, __VA_ARGS__); \
> > } else { \
> > char __fmt[] = fmt; \
> > _Pragma("GCC diagnostic push") \
> > _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
> > u64 __params[] = { __VA_ARGS__ }; \
> > _Pragma("GCC diagnostic pop") \
> > bpf_ptrace_vprintk(__fmt, sizeof(__fmt), __params, sizeof(__params)); \
> > } \
> > })
> >
> > SEC("perf_event")
> > int do_sample(struct bpf_perf_event_data *ctx) {
> > u64 ip = PT_REGS_IP(&ctx->regs);
> > u64 id = bpf_get_current_pid_tgid();
> > s32 pid = id >> 32;
> > s32 tid = id;
> > debug_printk("N|%d|BPRF-%d|BPRF:%llx", pid, tid, ip);
> > return 0;
> > }
> >
> > [output]:
> > app-3151 [000] d.h1. 6059.904239: tracing_mark_write: N|2491|BPRF-3151|BPRF:58750d0eec
> >
> > Signed-off-by: Eric Yan <eric.yan@oppo.com>
> > ---
> > kernel/bpf/helpers.c | 34 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index
> > 1a43d06eab28..1e37dae74ca6 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -2521,6 +2521,39 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid)
> > return p;
> > }
> >
> > +static noinline void tracing_mark_write(char *buf) {
> > + trace_printk(buf);
> > +}
> > +
> > +/* same as bpf_trace_vprintk, only with a trace_marker format
> > +requirement
> > + * @fmt: Format string, e.g. <B|E|C|N>|<%d:pid>|<%s:TAG>...
> > + */
> > +__bpf_kfunc int bpf_ptrace_vprintk(char *fmt, u32 fmt_size, const
> > +void *args, u32 args__sz) {
> > + struct bpf_bprintf_data data = {
> > + .get_bin_args = true,
> > + .get_buf = true,
> > + };
> > + int ret, num_args;
> > +
> > + if (args__sz & 7 || args__sz > MAX_BPRINTF_VARARGS * 8 || (args__sz && !args))
> > + return -EINVAL;
> > + num_args = args__sz / 8;
> > +
> > + ret = bpf_bprintf_prepare(fmt, fmt_size, args, num_args, &data);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt,
> > + data.bin_args);
> > +
> > + tracing_mark_write(data.buf);
> > +
> > + bpf_bprintf_cleanup(&data);
> > +
> > + return ret;
> > +}
> > +
> > /**
> > * bpf_dynptr_slice() - Obtain a read-only pointer to the dynptr data.
> > * @p: The dynptr whose data slice to retrieve @@ -3090,6 +3123,7 @@
> > BTF_ID_FLAGS(func, bpf_iter_bits_new, KF_ITER_NEW) BTF_ID_FLAGS(func,
> > bpf_iter_bits_next, KF_ITER_NEXT | KF_RET_NULL) BTF_ID_FLAGS(func,
> > bpf_iter_bits_destroy, KF_ITER_DESTROY) BTF_ID_FLAGS(func,
> > bpf_copy_from_user_str, KF_SLEEPABLE)
> > +BTF_ID_FLAGS(func, bpf_ptrace_vprintk)
> > BTF_KFUNCS_END(common_btf_ids)
>
> Why new kfunc?
> Use bpf_snprintf() and follow with bpf_trace_printk() ?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-30 23:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-25 10:02 [PATCH] Add BPF Kernel Function bpf_ptrace_vprintk Eric Yan
2024-09-26 3:34 ` kernel test robot
2024-09-26 7:27 ` [PATCH v2] " Eric Yan
2024-09-29 17:10 ` Alexei Starovoitov
2024-09-30 8:29 ` 答复: " 燕青洲(Eric Yan)
2024-09-30 23:17 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox