[PATCH] Add BPF Kernel Function bpf_ptrace

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [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