* [PATCH 0/3] add support for writable bare tracepoint
@ 2021-09-16 13:55 Hou Tao
2021-09-16 13:55 ` [PATCH 1/3] bpf: support writable context for " Hou Tao
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Hou Tao @ 2021-09-16 13:55 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
bpf, netdev, houtao1
Hi,
The patchset series supports writable context for bare tracepoint.
The main idea comes from patchset "writable contexts for bpf
raw tracepoints" [1], but it only supports normal tracepoint
with associated trace event under tracefs. Bare tracepoint
is more light-weight and doesn't have the ABI burden of
normal tracepoint, so add support for it in BPF.
Any comments are welcome.
[1]: https://lore.kernel.org/lkml/20190426184951.21812-1-mmullins@fb.com
Hou Tao (3):
bpf: support writable context for bare tracepoint
libbpf: support detecting and attaching of writable tracepoint program
bpf/selftests: add test for writable bare tracepoint
include/trace/bpf_probe.h | 19 +++++++--
tools/lib/bpf/libbpf.c | 4 ++
.../bpf/bpf_testmod/bpf_testmod-events.h | 15 +++++++
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 10 +++++
.../selftests/bpf/bpf_testmod/bpf_testmod.h | 5 +++
.../selftests/bpf/prog_tests/module_attach.c | 40 ++++++++++++++++++-
.../selftests/bpf/progs/test_module_attach.c | 14 +++++++
7 files changed, 101 insertions(+), 6 deletions(-)
--
2.29.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] bpf: support writable context for bare tracepoint
2021-09-16 13:55 [PATCH 0/3] add support for writable bare tracepoint Hou Tao
@ 2021-09-16 13:55 ` Hou Tao
2021-09-16 23:16 ` Yonghong Song
2021-09-16 13:55 ` [PATCH 2/3] libbpf: support detecting and attaching of writable tracepoint program Hou Tao
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Hou Tao @ 2021-09-16 13:55 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
bpf, netdev, houtao1
Commit 9df1c28bb752 ("bpf: add writable context for raw tracepoints")
supports writable context for tracepoint, but it misses the support
for bare tracepoint which has no associated trace event.
Bare tracepoint is defined by DECLARE_TRACE(), so adding a corresponding
DECLARE_TRACE_WRITABLE() macro to generate a definition in __bpf_raw_tp_map
section for bare tracepoint in a similar way to DEFINE_TRACE_WRITABLE().
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
include/trace/bpf_probe.h | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
index a23be89119aa..d08ee1060d82 100644
--- a/include/trace/bpf_probe.h
+++ b/include/trace/bpf_probe.h
@@ -93,8 +93,7 @@ __section("__bpf_raw_tp_map") = { \
#define FIRST(x, ...) x
-#undef DEFINE_EVENT_WRITABLE
-#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size) \
+#define __CHECK_WRITABLE_BUF_SIZE(call, proto, args, size) \
static inline void bpf_test_buffer_##call(void) \
{ \
/* BUILD_BUG_ON() is ignored if the code is completely eliminated, but \
@@ -103,8 +102,12 @@ static inline void bpf_test_buffer_##call(void) \
*/ \
FIRST(proto); \
(void)BUILD_BUG_ON_ZERO(size != sizeof(*FIRST(args))); \
-} \
-__DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
+}
+
+#undef DEFINE_EVENT_WRITABLE
+#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size) \
+ __CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \
+ __DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
#undef DEFINE_EVENT
#define DEFINE_EVENT(template, call, proto, args) \
@@ -119,10 +122,18 @@ __DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
__BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args)) \
__DEFINE_EVENT(call, call, PARAMS(proto), PARAMS(args), 0)
+#undef DECLARE_TRACE_WRITABLE
+#define DECLARE_TRACE_WRITABLE(call, proto, args, size) \
+ __CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \
+ __BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args)) \
+ __DEFINE_EVENT(call, call, PARAMS(proto), PARAMS(args), size)
+
#include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
#undef DEFINE_EVENT_WRITABLE
+#undef DECLARE_TRACE_WRITABLE
#undef __DEFINE_EVENT
+#undef __CHECK_WRITABLE_BUF_SIZE
#undef FIRST
#endif /* CONFIG_BPF_EVENTS */
--
2.29.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] libbpf: support detecting and attaching of writable tracepoint program
2021-09-16 13:55 [PATCH 0/3] add support for writable bare tracepoint Hou Tao
2021-09-16 13:55 ` [PATCH 1/3] bpf: support writable context for " Hou Tao
@ 2021-09-16 13:55 ` Hou Tao
2021-09-16 23:35 ` Yonghong Song
2021-09-16 13:55 ` [PATCH 3/3] bpf/selftests: add test for writable bare tracepoint Hou Tao
2021-09-16 22:58 ` [PATCH 0/3] add support " Yonghong Song
3 siblings, 1 reply; 11+ messages in thread
From: Hou Tao @ 2021-09-16 13:55 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
bpf, netdev, houtao1
Program on writable tracepoint is BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE,
but its attachment is the same as BPF_PROG_TYPE_RAW_TRACEPOINT.
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
tools/lib/bpf/libbpf.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 88d8825fc6f6..e6a1d552040c 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -7942,6 +7942,10 @@ static const struct bpf_sec_def section_defs[] = {
.attach_fn = attach_raw_tp),
SEC_DEF("raw_tp/", RAW_TRACEPOINT,
.attach_fn = attach_raw_tp),
+ SEC_DEF("raw_tracepoint_writable/", RAW_TRACEPOINT_WRITABLE,
+ .attach_fn = attach_raw_tp),
+ SEC_DEF("raw_tp_writable/", RAW_TRACEPOINT_WRITABLE,
+ .attach_fn = attach_raw_tp),
SEC_DEF("tp_btf/", TRACING,
.expected_attach_type = BPF_TRACE_RAW_TP,
.is_attach_btf = true,
--
2.29.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] bpf/selftests: add test for writable bare tracepoint
2021-09-16 13:55 [PATCH 0/3] add support for writable bare tracepoint Hou Tao
2021-09-16 13:55 ` [PATCH 1/3] bpf: support writable context for " Hou Tao
2021-09-16 13:55 ` [PATCH 2/3] libbpf: support detecting and attaching of writable tracepoint program Hou Tao
@ 2021-09-16 13:55 ` Hou Tao
2021-09-16 23:46 ` Yonghong Song
2021-09-16 22:58 ` [PATCH 0/3] add support " Yonghong Song
3 siblings, 1 reply; 11+ messages in thread
From: Hou Tao @ 2021-09-16 13:55 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
bpf, netdev, houtao1
Add a writable bare tracepoint in bpf_testmod module, and
trigger its calling when reading /sys/kernel/bpf_testmod
with a specific buffer length.
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
.../bpf/bpf_testmod/bpf_testmod-events.h | 15 +++++++
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 10 +++++
.../selftests/bpf/bpf_testmod/bpf_testmod.h | 5 +++
.../selftests/bpf/prog_tests/module_attach.c | 40 ++++++++++++++++++-
.../selftests/bpf/progs/test_module_attach.c | 14 +++++++
5 files changed, 82 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
index 89c6d58e5dd6..11ee801e75e7 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
@@ -34,6 +34,21 @@ DECLARE_TRACE(bpf_testmod_test_write_bare,
TP_ARGS(task, ctx)
);
+#undef BPF_TESTMOD_DECLARE_TRACE
+#ifdef DECLARE_TRACE_WRITABLE
+#define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \
+ DECLARE_TRACE_WRITABLE(call, PARAMS(proto), PARAMS(args), size)
+#else
+#define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \
+ DECLARE_TRACE(call, PARAMS(proto), PARAMS(args))
+#endif
+
+BPF_TESTMOD_DECLARE_TRACE(bpf_testmod_test_writable_bare,
+ TP_PROTO(struct bpf_testmod_test_writable_ctx *ctx),
+ TP_ARGS(ctx),
+ sizeof(struct bpf_testmod_test_writable_ctx)
+);
+
#endif /* _BPF_TESTMOD_EVENTS_H */
#undef TRACE_INCLUDE_PATH
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index 141d8da687d2..3d3fb16eaf8c 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -26,6 +26,16 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
trace_bpf_testmod_test_read(current, &ctx);
+ /* Magic number to enable writable tp */
+ if (len == 1024) {
+ struct bpf_testmod_test_writable_ctx writable = {
+ .val = 1024,
+ };
+ trace_bpf_testmod_test_writable_bare(&writable);
+ if (writable.ret)
+ return snprintf(buf, len, "%d\n", writable.val);
+ }
+
return -EIO; /* always fail */
}
EXPORT_SYMBOL(bpf_testmod_test_read);
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
index b3892dc40111..553d94214aa6 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
@@ -17,4 +17,9 @@ struct bpf_testmod_test_write_ctx {
size_t len;
};
+struct bpf_testmod_test_writable_ctx {
+ bool ret;
+ int val;
+};
+
#endif /* _BPF_TESTMOD_H */
diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c b/tools/testing/selftests/bpf/prog_tests/module_attach.c
index d85a69b7ce44..5565bcab1531 100644
--- a/tools/testing/selftests/bpf/prog_tests/module_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c
@@ -6,11 +6,39 @@
static int duration;
+#define BPF_TESTMOD_PATH "/sys/kernel/bpf_testmod"
+
+static int trigger_module_test_writable(int *val)
+{
+ int fd, err;
+ char buf[1025];
+ ssize_t rd;
+
+ fd = open(BPF_TESTMOD_PATH, O_RDONLY);
+ err = -errno;
+ if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err))
+ return err;
+
+ rd = read(fd, buf, sizeof(buf) - 1);
+ err = rd < 0 ? -errno : -EIO;
+ if (CHECK(rd <= 0, "testmod_file_read", "failed: %d\n", err)) {
+ close(fd);
+ return err;
+ }
+ buf[rd] = '\0';
+
+ *val = strtol(buf, NULL, 0);
+
+ close(fd);
+
+ return 0;
+}
+
static int trigger_module_test_read(int read_sz)
{
int fd, err;
- fd = open("/sys/kernel/bpf_testmod", O_RDONLY);
+ fd = open(BPF_TESTMOD_PATH, O_RDONLY);
err = -errno;
if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err))
return err;
@@ -32,7 +60,7 @@ static int trigger_module_test_write(int write_sz)
memset(buf, 'a', write_sz);
buf[write_sz-1] = '\0';
- fd = open("/sys/kernel/bpf_testmod", O_WRONLY);
+ fd = open(BPF_TESTMOD_PATH, O_WRONLY);
err = -errno;
if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err)) {
free(buf);
@@ -58,6 +86,7 @@ void test_module_attach(void)
struct test_module_attach__bss *bss;
struct bpf_link *link;
int err;
+ int writable_val;
skel = test_module_attach__open();
if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
@@ -90,6 +119,13 @@ void test_module_attach(void)
ASSERT_EQ(bss->fexit_ret, -EIO, "fexit_tet");
ASSERT_EQ(bss->fmod_ret_read_sz, READ_SZ, "fmod_ret");
+ bss->raw_tp_writable_bare_ret = 1;
+ bss->raw_tp_writable_bare_val = 511;
+ writable_val = 0;
+ ASSERT_OK(trigger_module_test_writable(&writable_val), "trigger_writable");
+ ASSERT_EQ(bss->raw_tp_writable_bare_in_val, 1024, "writable_test");
+ ASSERT_EQ(bss->raw_tp_writable_bare_val, writable_val, "writable_test");
+
test_module_attach__detach(skel);
/* attach fentry/fexit and make sure it get's module reference */
diff --git a/tools/testing/selftests/bpf/progs/test_module_attach.c b/tools/testing/selftests/bpf/progs/test_module_attach.c
index bd37ceec5587..4f5c780fcd21 100644
--- a/tools/testing/selftests/bpf/progs/test_module_attach.c
+++ b/tools/testing/selftests/bpf/progs/test_module_attach.c
@@ -27,6 +27,20 @@ int BPF_PROG(handle_raw_tp_bare,
return 0;
}
+int raw_tp_writable_bare_in_val = 0;
+int raw_tp_writable_bare_ret = 0;
+int raw_tp_writable_bare_val = 0;
+
+SEC("raw_tp_writable/bpf_testmod_test_writable_bare")
+int BPF_PROG(handle_raw_tp_writable_bare,
+ struct bpf_testmod_test_writable_ctx *writable)
+{
+ raw_tp_writable_bare_in_val = writable->val;
+ writable->ret = raw_tp_writable_bare_ret;
+ writable->val = raw_tp_writable_bare_val;
+ return 0;
+}
+
__u32 tp_btf_read_sz = 0;
SEC("tp_btf/bpf_testmod_test_read")
--
2.29.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] add support for writable bare tracepoint
2021-09-16 13:55 [PATCH 0/3] add support for writable bare tracepoint Hou Tao
` (2 preceding siblings ...)
2021-09-16 13:55 ` [PATCH 3/3] bpf/selftests: add test for writable bare tracepoint Hou Tao
@ 2021-09-16 22:58 ` Yonghong Song
3 siblings, 0 replies; 11+ messages in thread
From: Yonghong Song @ 2021-09-16 22:58 UTC (permalink / raw)
To: Hou Tao, Alexei Starovoitov
Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
bpf, netdev
On 9/16/21 6:55 AM, Hou Tao wrote:
> Hi,
>
> The patchset series supports writable context for bare tracepoint.
>
> The main idea comes from patchset "writable contexts for bpf
> raw tracepoints" [1], but it only supports normal tracepoint
> with associated trace event under tracefs. Bare tracepoint
> is more light-weight and doesn't have the ABI burden of
> normal tracepoint, so add support for it in BPF.
Just curious that do you have an actual use case for this?
If that is the case, it would be good that you stated in
the cover letter and relevant commit message.
Also, please add bpf-next to the tag like [PATCH bpf-next x/x]
to make it clear that the patch set is for bpf-next.
>
> Any comments are welcome.
>
> [1]: https://lore.kernel.org/lkml/20190426184951.21812-1-mmullins@fb.com
>
> Hou Tao (3):
> bpf: support writable context for bare tracepoint
> libbpf: support detecting and attaching of writable tracepoint program
> bpf/selftests: add test for writable bare tracepoint
>
> include/trace/bpf_probe.h | 19 +++++++--
> tools/lib/bpf/libbpf.c | 4 ++
> .../bpf/bpf_testmod/bpf_testmod-events.h | 15 +++++++
> .../selftests/bpf/bpf_testmod/bpf_testmod.c | 10 +++++
> .../selftests/bpf/bpf_testmod/bpf_testmod.h | 5 +++
> .../selftests/bpf/prog_tests/module_attach.c | 40 ++++++++++++++++++-
> .../selftests/bpf/progs/test_module_attach.c | 14 +++++++
> 7 files changed, 101 insertions(+), 6 deletions(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] bpf: support writable context for bare tracepoint
2021-09-16 13:55 ` [PATCH 1/3] bpf: support writable context for " Hou Tao
@ 2021-09-16 23:16 ` Yonghong Song
2021-09-17 13:45 ` Hou Tao
0 siblings, 1 reply; 11+ messages in thread
From: Yonghong Song @ 2021-09-16 23:16 UTC (permalink / raw)
To: Hou Tao, Alexei Starovoitov
Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
bpf, netdev
On 9/16/21 6:55 AM, Hou Tao wrote:
> Commit 9df1c28bb752 ("bpf: add writable context for raw tracepoints")
> supports writable context for tracepoint, but it misses the support
> for bare tracepoint which has no associated trace event.
>
> Bare tracepoint is defined by DECLARE_TRACE(), so adding a corresponding
> DECLARE_TRACE_WRITABLE() macro to generate a definition in __bpf_raw_tp_map
> section for bare tracepoint in a similar way to DEFINE_TRACE_WRITABLE().
>
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> include/trace/bpf_probe.h | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
> index a23be89119aa..d08ee1060d82 100644
> --- a/include/trace/bpf_probe.h
> +++ b/include/trace/bpf_probe.h
> @@ -93,8 +93,7 @@ __section("__bpf_raw_tp_map") = { \
>
> #define FIRST(x, ...) x
>
> -#undef DEFINE_EVENT_WRITABLE
> -#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size) \
> +#define __CHECK_WRITABLE_BUF_SIZE(call, proto, args, size) \
> static inline void bpf_test_buffer_##call(void) \
> { \
> /* BUILD_BUG_ON() is ignored if the code is completely eliminated, but \
> @@ -103,8 +102,12 @@ static inline void bpf_test_buffer_##call(void) \
> */ \
> FIRST(proto); \
> (void)BUILD_BUG_ON_ZERO(size != sizeof(*FIRST(args))); \
> -} \
> -__DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
> +}
> +
> +#undef DEFINE_EVENT_WRITABLE
> +#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size) \
> + __CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \
> + __DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
>
> #undef DEFINE_EVENT
> #define DEFINE_EVENT(template, call, proto, args) \
> @@ -119,10 +122,18 @@ __DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
> __BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args)) \
> __DEFINE_EVENT(call, call, PARAMS(proto), PARAMS(args), 0)
>
> +#undef DECLARE_TRACE_WRITABLE
> +#define DECLARE_TRACE_WRITABLE(call, proto, args, size) \
> + __CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \
> + __BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args)) \
> + __DEFINE_EVENT(call, call, PARAMS(proto), PARAMS(args), size)
> +
> #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
>
> #undef DEFINE_EVENT_WRITABLE
> +#undef DECLARE_TRACE_WRITABLE
> #undef __DEFINE_EVENT
> +#undef __CHECK_WRITABLE_BUF_SIZE
Put "#undef __CHECK_WRITABLE_BUF_SIZE" right after "#undef
DECLARE_TRACE_WRITABLE" since they are related to each other
and also they are in correct reverse order w.r.t. __DEFINE_EVENT?
> #undef FIRST
>
> #endif /* CONFIG_BPF_EVENTS */
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] libbpf: support detecting and attaching of writable tracepoint program
2021-09-16 13:55 ` [PATCH 2/3] libbpf: support detecting and attaching of writable tracepoint program Hou Tao
@ 2021-09-16 23:35 ` Yonghong Song
0 siblings, 0 replies; 11+ messages in thread
From: Yonghong Song @ 2021-09-16 23:35 UTC (permalink / raw)
To: Hou Tao, Alexei Starovoitov
Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
bpf, netdev
On 9/16/21 6:55 AM, Hou Tao wrote:
> Program on writable tracepoint is BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE,
> but its attachment is the same as BPF_PROG_TYPE_RAW_TRACEPOINT.
>
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> tools/lib/bpf/libbpf.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 88d8825fc6f6..e6a1d552040c 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -7942,6 +7942,10 @@ static const struct bpf_sec_def section_defs[] = {
> .attach_fn = attach_raw_tp),
> SEC_DEF("raw_tp/", RAW_TRACEPOINT,
> .attach_fn = attach_raw_tp),
> + SEC_DEF("raw_tracepoint_writable/", RAW_TRACEPOINT_WRITABLE,
> + .attach_fn = attach_raw_tp),
> + SEC_DEF("raw_tp_writable/", RAW_TRACEPOINT_WRITABLE,
> + .attach_fn = attach_raw_tp),
Looks like initially ([1]) we don't have C bpf program test case for
RAW_TRACEPOINT_WRITABLE, so the above sec definition is missing.
[1] e950e843367d7 selftests: bpf: test writable buffers in raw tps
> SEC_DEF("tp_btf/", TRACING,
> .expected_attach_type = BPF_TRACE_RAW_TP,
> .is_attach_btf = true,
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] bpf/selftests: add test for writable bare tracepoint
2021-09-16 13:55 ` [PATCH 3/3] bpf/selftests: add test for writable bare tracepoint Hou Tao
@ 2021-09-16 23:46 ` Yonghong Song
2021-09-17 14:03 ` Hou Tao
0 siblings, 1 reply; 11+ messages in thread
From: Yonghong Song @ 2021-09-16 23:46 UTC (permalink / raw)
To: Hou Tao, Alexei Starovoitov
Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
bpf, netdev
On 9/16/21 6:55 AM, Hou Tao wrote:
> Add a writable bare tracepoint in bpf_testmod module, and
> trigger its calling when reading /sys/kernel/bpf_testmod
> with a specific buffer length.
The patch cannot be applied cleanly with bpf-next tree.
Please rebase and resubmit.
>
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> .../bpf/bpf_testmod/bpf_testmod-events.h | 15 +++++++
> .../selftests/bpf/bpf_testmod/bpf_testmod.c | 10 +++++
> .../selftests/bpf/bpf_testmod/bpf_testmod.h | 5 +++
> .../selftests/bpf/prog_tests/module_attach.c | 40 ++++++++++++++++++-
> .../selftests/bpf/progs/test_module_attach.c | 14 +++++++
> 5 files changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
> index 89c6d58e5dd6..11ee801e75e7 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
> @@ -34,6 +34,21 @@ DECLARE_TRACE(bpf_testmod_test_write_bare,
> TP_ARGS(task, ctx)
> );
>
> +#undef BPF_TESTMOD_DECLARE_TRACE
> +#ifdef DECLARE_TRACE_WRITABLE
> +#define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \
> + DECLARE_TRACE_WRITABLE(call, PARAMS(proto), PARAMS(args), size)
> +#else
> +#define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \
> + DECLARE_TRACE(call, PARAMS(proto), PARAMS(args))
> +#endif
> +
> +BPF_TESTMOD_DECLARE_TRACE(bpf_testmod_test_writable_bare,
> + TP_PROTO(struct bpf_testmod_test_writable_ctx *ctx),
> + TP_ARGS(ctx),
> + sizeof(struct bpf_testmod_test_writable_ctx)
> +);
> +
> #endif /* _BPF_TESTMOD_EVENTS_H */
>
> #undef TRACE_INCLUDE_PATH
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> index 141d8da687d2..3d3fb16eaf8c 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> @@ -26,6 +26,16 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
>
> trace_bpf_testmod_test_read(current, &ctx);
>
> + /* Magic number to enable writable tp */
> + if (len == 1024) {
> + struct bpf_testmod_test_writable_ctx writable = {
> + .val = 1024,
> + };
> + trace_bpf_testmod_test_writable_bare(&writable);
> + if (writable.ret)
> + return snprintf(buf, len, "%d\n", writable.val);
> + }
> +
> return -EIO; /* always fail */
> }
> EXPORT_SYMBOL(bpf_testmod_test_read);
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
> index b3892dc40111..553d94214aa6 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
> @@ -17,4 +17,9 @@ struct bpf_testmod_test_write_ctx {
> size_t len;
> };
>
> +struct bpf_testmod_test_writable_ctx {
> + bool ret;
> + int val;
> +};
> +
> #endif /* _BPF_TESTMOD_H */
> diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c b/tools/testing/selftests/bpf/prog_tests/module_attach.c
> index d85a69b7ce44..5565bcab1531 100644
> --- a/tools/testing/selftests/bpf/prog_tests/module_attach.c
> +++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c
> @@ -6,11 +6,39 @@
>
> static int duration;
>
> +#define BPF_TESTMOD_PATH "/sys/kernel/bpf_testmod"
> +
> +static int trigger_module_test_writable(int *val)
> +{
> + int fd, err;
> + char buf[1025];
Not critical, but do you need such a big stack size?
Maybe smaller one?
> + ssize_t rd;
> +
> + fd = open(BPF_TESTMOD_PATH, O_RDONLY);
> + err = -errno;
> + if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err))
> + return err;
> +
> + rd = read(fd, buf, sizeof(buf) - 1);
> + err = rd < 0 ? -errno : -EIO;
> + if (CHECK(rd <= 0, "testmod_file_read", "failed: %d\n", err)) {
Please use ASSERT_* macros. You can take a look at other self tests.
> + close(fd);
> + return err;
> + }
Put one blank line here and remove the following three blank lines.
> + buf[rd] = '\0';
> +
> + *val = strtol(buf, NULL, 0);
> +
> + close(fd);
> +
> + return 0;
> +}
> +
> static int trigger_module_test_read(int read_sz)
> {
> int fd, err;
>
> - fd = open("/sys/kernel/bpf_testmod", O_RDONLY);
> + fd = open(BPF_TESTMOD_PATH, O_RDONLY);
> err = -errno;
> if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err))
> return err;
> @@ -32,7 +60,7 @@ static int trigger_module_test_write(int write_sz)
> memset(buf, 'a', write_sz);
> buf[write_sz-1] = '\0';
>
> - fd = open("/sys/kernel/bpf_testmod", O_WRONLY);
> + fd = open(BPF_TESTMOD_PATH, O_WRONLY);
> err = -errno;
> if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err)) {
> free(buf);
> @@ -58,6 +86,7 @@ void test_module_attach(void)
> struct test_module_attach__bss *bss;
> struct bpf_link *link;
> int err;
> + int writable_val;
>
> skel = test_module_attach__open();
> if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
> @@ -90,6 +119,13 @@ void test_module_attach(void)
> ASSERT_EQ(bss->fexit_ret, -EIO, "fexit_tet");
> ASSERT_EQ(bss->fmod_ret_read_sz, READ_SZ, "fmod_ret");
>
> + bss->raw_tp_writable_bare_ret = 1;
> + bss->raw_tp_writable_bare_val = 511;
> + writable_val = 0;
> + ASSERT_OK(trigger_module_test_writable(&writable_val), "trigger_writable");
> + ASSERT_EQ(bss->raw_tp_writable_bare_in_val, 1024, "writable_test");
> + ASSERT_EQ(bss->raw_tp_writable_bare_val, writable_val, "writable_test");
> +
> test_module_attach__detach(skel);
>
> /* attach fentry/fexit and make sure it get's module reference */
> diff --git a/tools/testing/selftests/bpf/progs/test_module_attach.c b/tools/testing/selftests/bpf/progs/test_module_attach.c
> index bd37ceec5587..4f5c780fcd21 100644
> --- a/tools/testing/selftests/bpf/progs/test_module_attach.c
> +++ b/tools/testing/selftests/bpf/progs/test_module_attach.c
> @@ -27,6 +27,20 @@ int BPF_PROG(handle_raw_tp_bare,
> return 0;
> }
>
> +int raw_tp_writable_bare_in_val = 0;
> +int raw_tp_writable_bare_ret = 0;
> +int raw_tp_writable_bare_val = 0;
> +
> +SEC("raw_tp_writable/bpf_testmod_test_writable_bare")
> +int BPF_PROG(handle_raw_tp_writable_bare,
> + struct bpf_testmod_test_writable_ctx *writable)
> +{
> + raw_tp_writable_bare_in_val = writable->val;
> + writable->ret = raw_tp_writable_bare_ret;
> + writable->val = raw_tp_writable_bare_val;
> + return 0;
> +}
> +
> __u32 tp_btf_read_sz = 0;
>
> SEC("tp_btf/bpf_testmod_test_read")
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] bpf: support writable context for bare tracepoint
2021-09-16 23:16 ` Yonghong Song
@ 2021-09-17 13:45 ` Hou Tao
2021-09-17 14:48 ` Yonghong Song
0 siblings, 1 reply; 11+ messages in thread
From: Hou Tao @ 2021-09-17 13:45 UTC (permalink / raw)
To: Yonghong Song, Alexei Starovoitov
Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
bpf, netdev
Hi,
On 9/17/2021 7:16 AM, Yonghong Song wrote:
>
>
> On 9/16/21 6:55 AM, Hou Tao wrote:
>> Commit 9df1c28bb752 ("bpf: add writable context for raw tracepoints")
>> supports writable context for tracepoint, but it misses the support
>> for bare tracepoint which has no associated trace event.
>>
>> Bare tracepoint is defined by DECLARE_TRACE(), so adding a corresponding
>> DECLARE_TRACE_WRITABLE() macro to generate a definition in __bpf_raw_tp_map
>> section for bare tracepoint in a similar way to DEFINE_TRACE_WRITABLE().
>>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>> include/trace/bpf_probe.h | 19 +++++++++++++++----
>> 1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
>> index a23be89119aa..d08ee1060d82 100644
>> --- a/include/trace/bpf_probe.h
>> +++ b/include/trace/bpf_probe.h
>> @@ -93,8 +93,7 @@ __section("__bpf_raw_tp_map") = { \
>> #define FIRST(x, ...) x
>> -#undef DEFINE_EVENT_WRITABLE
>> -#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size) \
>> +#define __CHECK_WRITABLE_BUF_SIZE(call, proto, args, size) \
>> static inline void bpf_test_buffer_##call(void) \
>> { \
>> /* BUILD_BUG_ON() is ignored if the code is completely eliminated, but \
>> @@ -103,8 +102,12 @@ static inline void
>> bpf_test_buffer_##call(void) \
>> */ \
>> FIRST(proto); \
>> (void)BUILD_BUG_ON_ZERO(size != sizeof(*FIRST(args))); \
>> -} \
>> -__DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
>> +}
>> +
>> +#undef DEFINE_EVENT_WRITABLE
>> +#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size) \
>> + __CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \
>> + __DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
>> #undef DEFINE_EVENT
>> #define DEFINE_EVENT(template, call, proto, args) \
>> @@ -119,10 +122,18 @@ __DEFINE_EVENT(template, call, PARAMS(proto),
>> PARAMS(args), size)
>> __BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args)) \
>> __DEFINE_EVENT(call, call, PARAMS(proto), PARAMS(args), 0)
>> +#undef DECLARE_TRACE_WRITABLE
>> +#define DECLARE_TRACE_WRITABLE(call, proto, args, size) \
>> + __CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \
>> + __BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args)) \
>> + __DEFINE_EVENT(call, call, PARAMS(proto), PARAMS(args), size)
>> +
>> #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
>> #undef DEFINE_EVENT_WRITABLE
>> +#undef DECLARE_TRACE_WRITABLE
>> #undef __DEFINE_EVENT
>> +#undef __CHECK_WRITABLE_BUF_SIZE
>
> Put "#undef __CHECK_WRITABLE_BUF_SIZE" right after "#undef
> DECLARE_TRACE_WRITABLE" since they are related to each other
> and also they are in correct reverse order w.r.t. __DEFINE_EVENT?
If considering __CHECK_WRITABLE_BUF_SIZE is used in both DECLARE_TRACE_WRITABLE and
DEFINE_EVENT_WRITABLE and the order of definitions, is the following order better ?
#undef DECLARE_TRACE_WRITABLE
#undef DEFINE_EVENT_WRITABLE
#undef __CHECK_WRITABLE_BUF_SIZE
>
>> #undef FIRST
>> #endif /* CONFIG_BPF_EVENTS */
>>
> .
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] bpf/selftests: add test for writable bare tracepoint
2021-09-16 23:46 ` Yonghong Song
@ 2021-09-17 14:03 ` Hou Tao
0 siblings, 0 replies; 11+ messages in thread
From: Hou Tao @ 2021-09-17 14:03 UTC (permalink / raw)
To: Yonghong Song, Alexei Starovoitov
Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
bpf, netdev
Hi,
On 9/17/2021 7:46 AM, Yonghong Song wrote:
>
>
> On 9/16/21 6:55 AM, Hou Tao wrote:
>> Add a writable bare tracepoint in bpf_testmod module, and
>> trigger its calling when reading /sys/kernel/bpf_testmod
>> with a specific buffer length.
>
> The patch cannot be applied cleanly with bpf-next tree.
> Please rebase and resubmit.
Will do
>
>>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>> .../bpf/bpf_testmod/bpf_testmod-events.h | 15 +++++++
>> .../selftests/bpf/bpf_testmod/bpf_testmod.c | 10 +++++
>> .../selftests/bpf/bpf_testmod/bpf_testmod.h | 5 +++
>> .../selftests/bpf/prog_tests/module_attach.c | 40 ++++++++++++++++++-
>> .../selftests/bpf/progs/test_module_attach.c | 14 +++++++
>> 5 files changed, 82 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
>> b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
>> index 89c6d58e5dd6..11ee801e75e7 100644
>> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
>> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
>> @@ -34,6 +34,21 @@ DECLARE_TRACE(bpf_testmod_test_write_bare,
>> TP_ARGS(task, ctx)
>> );
>> +#undef BPF_TESTMOD_DECLARE_TRACE
>> +#ifdef DECLARE_TRACE_WRITABLE
>> +#define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \
>> + DECLARE_TRACE_WRITABLE(call, PARAMS(proto), PARAMS(args), size)
>> +#else
>> +#define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \
>> + DECLARE_TRACE(call, PARAMS(proto), PARAMS(args))
>> +#endif
>> +
>> +BPF_TESTMOD_DECLARE_TRACE(bpf_testmod_test_writable_bare,
>> + TP_PROTO(struct bpf_testmod_test_writable_ctx *ctx),
>> + TP_ARGS(ctx),
>> + sizeof(struct bpf_testmod_test_writable_ctx)
>> +);
>> +
>> #endif /* _BPF_TESTMOD_EVENTS_H */
>> #undef TRACE_INCLUDE_PATH
>> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
>> b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
>> index 141d8da687d2..3d3fb16eaf8c 100644
>> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
>> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
>> @@ -26,6 +26,16 @@ bpf_testmod_test_read(struct file *file, struct kobject
>> *kobj,
>> trace_bpf_testmod_test_read(current, &ctx);
>> + /* Magic number to enable writable tp */
>> + if (len == 1024) {
>> + struct bpf_testmod_test_writable_ctx writable = {
>> + .val = 1024,
>> + };
>> + trace_bpf_testmod_test_writable_bare(&writable);
>> + if (writable.ret)
>> + return snprintf(buf, len, "%d\n", writable.val);
>> + }
>> +
>> return -EIO; /* always fail */
>> }
>> EXPORT_SYMBOL(bpf_testmod_test_read);
>> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
>> b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
>> index b3892dc40111..553d94214aa6 100644
>> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
>> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
>> @@ -17,4 +17,9 @@ struct bpf_testmod_test_write_ctx {
>> size_t len;
>> };
>> +struct bpf_testmod_test_writable_ctx {
>> + bool ret;
>> + int val;
>> +};
>> +
>> #endif /* _BPF_TESTMOD_H */
>> diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c
>> b/tools/testing/selftests/bpf/prog_tests/module_attach.c
>> index d85a69b7ce44..5565bcab1531 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/module_attach.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c
>> @@ -6,11 +6,39 @@
>> static int duration;
>> +#define BPF_TESTMOD_PATH "/sys/kernel/bpf_testmod"
>> +
>> +static int trigger_module_test_writable(int *val)
>> +{
>> + int fd, err;
>> + char buf[1025];
>
> Not critical, but do you need such a big stack size?
> Maybe smaller one?
65 is also fine.
>
>> + ssize_t rd;
>> +
>> + fd = open(BPF_TESTMOD_PATH, O_RDONLY);
>> + err = -errno;
>> + if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err))
>> + return err;
>> +
>> + rd = read(fd, buf, sizeof(buf) - 1);
>> + err = rd < 0 ? -errno : -EIO;
>> + if (CHECK(rd <= 0, "testmod_file_read", "failed: %d\n", err)) {
>
> Please use ASSERT_* macros. You can take a look at other self tests.
The reason using CHECK instead of ASSERT is we can output the errno
if read() fails.
>> + close(fd);
>> + return err;
>> + }
>
> Put one blank line here and remove the following three blank lines.
Will do.
>
>> + buf[rd] = '\0';
>> +
>> + *val = strtol(buf, NULL, 0);
>> +
>> + close(fd);
>> +
>> + return 0;
>> +}
>> +
>> static int trigger_module_test_read(int read_sz)
>> {
>> int fd, err;
>> - fd = open("/sys/kernel/bpf_testmod", O_RDONLY);
>> + fd = open(BPF_TESTMOD_PATH, O_RDONLY);
>> err = -errno;
>> if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err))
>> return err;
>> @@ -32,7 +60,7 @@ static int trigger_module_test_write(int write_sz)
>> memset(buf, 'a', write_sz);
>> buf[write_sz-1] = '\0';
>> - fd = open("/sys/kernel/bpf_testmod", O_WRONLY);
>> + fd = open(BPF_TESTMOD_PATH, O_WRONLY);
>> err = -errno;
>> if (CHECK(fd < 0, "testmod_file_open", "failed: %d\n", err)) {
>> free(buf);
>> @@ -58,6 +86,7 @@ void test_module_attach(void)
>> struct test_module_attach__bss *bss;
>> struct bpf_link *link;
>> int err;
>> + int writable_val;
>> skel = test_module_attach__open();
>> if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
>> @@ -90,6 +119,13 @@ void test_module_attach(void)
>> ASSERT_EQ(bss->fexit_ret, -EIO, "fexit_tet");
>> ASSERT_EQ(bss->fmod_ret_read_sz, READ_SZ, "fmod_ret");
>> + bss->raw_tp_writable_bare_ret = 1;
>> + bss->raw_tp_writable_bare_val = 511;
>> + writable_val = 0;
>> + ASSERT_OK(trigger_module_test_writable(&writable_val), "trigger_writable");
>> + ASSERT_EQ(bss->raw_tp_writable_bare_in_val, 1024, "writable_test");
>> + ASSERT_EQ(bss->raw_tp_writable_bare_val, writable_val, "writable_test");
>> +
>> test_module_attach__detach(skel);
>> /* attach fentry/fexit and make sure it get's module reference */
>> diff --git a/tools/testing/selftests/bpf/progs/test_module_attach.c
>> b/tools/testing/selftests/bpf/progs/test_module_attach.c
>> index bd37ceec5587..4f5c780fcd21 100644
>> --- a/tools/testing/selftests/bpf/progs/test_module_attach.c
>> +++ b/tools/testing/selftests/bpf/progs/test_module_attach.c
>> @@ -27,6 +27,20 @@ int BPF_PROG(handle_raw_tp_bare,
>> return 0;
>> }
>> +int raw_tp_writable_bare_in_val = 0;
>> +int raw_tp_writable_bare_ret = 0;
>> +int raw_tp_writable_bare_val = 0;
>> +
>> +SEC("raw_tp_writable/bpf_testmod_test_writable_bare")
>> +int BPF_PROG(handle_raw_tp_writable_bare,
>> + struct bpf_testmod_test_writable_ctx *writable)
>> +{
>> + raw_tp_writable_bare_in_val = writable->val;
>> + writable->ret = raw_tp_writable_bare_ret;
>> + writable->val = raw_tp_writable_bare_val;
>> + return 0;
>> +}
>> +
>> __u32 tp_btf_read_sz = 0;
>> SEC("tp_btf/bpf_testmod_test_read")
>>
> .
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] bpf: support writable context for bare tracepoint
2021-09-17 13:45 ` Hou Tao
@ 2021-09-17 14:48 ` Yonghong Song
0 siblings, 0 replies; 11+ messages in thread
From: Yonghong Song @ 2021-09-17 14:48 UTC (permalink / raw)
To: Hou Tao, Alexei Starovoitov
Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
bpf, netdev
On 9/17/21 6:45 AM, Hou Tao wrote:
> Hi,
>
> On 9/17/2021 7:16 AM, Yonghong Song wrote:
>>
>>
>> On 9/16/21 6:55 AM, Hou Tao wrote:
>>> Commit 9df1c28bb752 ("bpf: add writable context for raw tracepoints")
>>> supports writable context for tracepoint, but it misses the support
>>> for bare tracepoint which has no associated trace event.
>>>
>>> Bare tracepoint is defined by DECLARE_TRACE(), so adding a corresponding
>>> DECLARE_TRACE_WRITABLE() macro to generate a definition in __bpf_raw_tp_map
>>> section for bare tracepoint in a similar way to DEFINE_TRACE_WRITABLE().
>>>
>>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>>> ---
>>> include/trace/bpf_probe.h | 19 +++++++++++++++----
>>> 1 file changed, 15 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
>>> index a23be89119aa..d08ee1060d82 100644
>>> --- a/include/trace/bpf_probe.h
>>> +++ b/include/trace/bpf_probe.h
>>> @@ -93,8 +93,7 @@ __section("__bpf_raw_tp_map") = { \
>>> #define FIRST(x, ...) x
>>> -#undef DEFINE_EVENT_WRITABLE
>>> -#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size) \
>>> +#define __CHECK_WRITABLE_BUF_SIZE(call, proto, args, size) \
>>> static inline void bpf_test_buffer_##call(void) \
>>> { \
>>> /* BUILD_BUG_ON() is ignored if the code is completely eliminated, but \
>>> @@ -103,8 +102,12 @@ static inline void
>>> bpf_test_buffer_##call(void) \
>>> */ \
>>> FIRST(proto); \
>>> (void)BUILD_BUG_ON_ZERO(size != sizeof(*FIRST(args))); \
>>> -} \
>>> -__DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
>>> +}
>>> +
>>> +#undef DEFINE_EVENT_WRITABLE
>>> +#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size) \
>>> + __CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \
>>> + __DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
>>> #undef DEFINE_EVENT
>>> #define DEFINE_EVENT(template, call, proto, args) \
>>> @@ -119,10 +122,18 @@ __DEFINE_EVENT(template, call, PARAMS(proto),
>>> PARAMS(args), size)
>>> __BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args)) \
>>> __DEFINE_EVENT(call, call, PARAMS(proto), PARAMS(args), 0)
>>> +#undef DECLARE_TRACE_WRITABLE
>>> +#define DECLARE_TRACE_WRITABLE(call, proto, args, size) \
>>> + __CHECK_WRITABLE_BUF_SIZE(call, PARAMS(proto), PARAMS(args), size) \
>>> + __BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args)) \
>>> + __DEFINE_EVENT(call, call, PARAMS(proto), PARAMS(args), size)
>>> +
>>> #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
>>> #undef DEFINE_EVENT_WRITABLE
>>> +#undef DECLARE_TRACE_WRITABLE
>>> #undef __DEFINE_EVENT
>>> +#undef __CHECK_WRITABLE_BUF_SIZE
>>
>> Put "#undef __CHECK_WRITABLE_BUF_SIZE" right after "#undef
>> DECLARE_TRACE_WRITABLE" since they are related to each other
>> and also they are in correct reverse order w.r.t. __DEFINE_EVENT?
> If considering __CHECK_WRITABLE_BUF_SIZE is used in both DECLARE_TRACE_WRITABLE and
> DEFINE_EVENT_WRITABLE and the order of definitions, is the following order better ?
>
> #undef DECLARE_TRACE_WRITABLE
> #undef DEFINE_EVENT_WRITABLE
> #undef __CHECK_WRITABLE_BUF_SIZE
This should be okay.
>
>>
>>> #undef FIRST
>>> #endif /* CONFIG_BPF_EVENTS */
>>>
>> .
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-09-17 14:49 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-16 13:55 [PATCH 0/3] add support for writable bare tracepoint Hou Tao
2021-09-16 13:55 ` [PATCH 1/3] bpf: support writable context for " Hou Tao
2021-09-16 23:16 ` Yonghong Song
2021-09-17 13:45 ` Hou Tao
2021-09-17 14:48 ` Yonghong Song
2021-09-16 13:55 ` [PATCH 2/3] libbpf: support detecting and attaching of writable tracepoint program Hou Tao
2021-09-16 23:35 ` Yonghong Song
2021-09-16 13:55 ` [PATCH 3/3] bpf/selftests: add test for writable bare tracepoint Hou Tao
2021-09-16 23:46 ` Yonghong Song
2021-09-17 14:03 ` Hou Tao
2021-09-16 22:58 ` [PATCH 0/3] add support " Yonghong Song
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).