Commit a1852ce0 authored by Andrii Nakryiko's avatar Andrii Nakryiko

Merge branch 'add support for writable bare tracepoint'

Hou Tao says:

====================

From: Hou Tao <houtao1@huawei.com>

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. Now we have one use case
in which we add bare tracepoint in VFS layer, and update
file::f_mode for specific files. The reason using bare tracepoint
is that it doesn't form a ABI and we can change it freely. So
add support for it in BPF.

Comments are always welcome.

[1]: https://lore.kernel.org/lkml/20190426184951.21812-1-mmullins@fb.com

Change log:
v5:
 * rebased on bpf-next
 * patch 1: add Acked-by tag
 * patch 2: handle invalid section name, make prefixes array being const

v4: https://www.spinics.net/lists/bpf/msg47021.html
 * rebased on bpf-next
 * update patch 2 to add support for writable raw tracepoint attachment
   in attach_raw_tp().
 * update patch 3 to add Acked-by tag

v3: https://www.spinics.net/lists/bpf/msg46824.html
  * use raw_tp.w instead of raw_tp_writable as section
    name of writable tp
  * use ASSERT_XXX() instead of CHECK()
  * define a common macro for "/sys/kernel/bpf_testmod"

v2: https://www.spinics.net/lists/bpf/msg46356.html
  * rebase on bpf-next tree
  * address comments from Yonghong Song
  * rename bpf_testmode_test_writable_ctx::ret as early_ret to reflect
    its purpose better.

v1: https://www.spinics.net/lists/bpf/msg46221.html
====================
Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
parents 1c8dab7d fa7f17d0
...@@ -93,8 +93,7 @@ __section("__bpf_raw_tp_map") = { \ ...@@ -93,8 +93,7 @@ __section("__bpf_raw_tp_map") = { \
#define FIRST(x, ...) x #define FIRST(x, ...) x
#undef DEFINE_EVENT_WRITABLE #define __CHECK_WRITABLE_BUF_SIZE(call, proto, args, size) \
#define DEFINE_EVENT_WRITABLE(template, call, proto, args, size) \
static inline void bpf_test_buffer_##call(void) \ static inline void bpf_test_buffer_##call(void) \
{ \ { \
/* BUILD_BUG_ON() is ignored if the code is completely eliminated, but \ /* BUILD_BUG_ON() is ignored if the code is completely eliminated, but \
...@@ -103,8 +102,12 @@ static inline void bpf_test_buffer_##call(void) \ ...@@ -103,8 +102,12 @@ static inline void bpf_test_buffer_##call(void) \
*/ \ */ \
FIRST(proto); \ FIRST(proto); \
(void)BUILD_BUG_ON_ZERO(size != sizeof(*FIRST(args))); \ (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 #undef DEFINE_EVENT
#define DEFINE_EVENT(template, call, proto, args) \ #define DEFINE_EVENT(template, call, proto, args) \
...@@ -119,9 +122,17 @@ __DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size) ...@@ -119,9 +122,17 @@ __DEFINE_EVENT(template, call, PARAMS(proto), PARAMS(args), size)
__BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args)) \ __BPF_DECLARE_TRACE(call, PARAMS(proto), PARAMS(args)) \
__DEFINE_EVENT(call, call, PARAMS(proto), PARAMS(args), 0) __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) #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
#undef DECLARE_TRACE_WRITABLE
#undef DEFINE_EVENT_WRITABLE #undef DEFINE_EVENT_WRITABLE
#undef __CHECK_WRITABLE_BUF_SIZE
#undef __DEFINE_EVENT #undef __DEFINE_EVENT
#undef FIRST #undef FIRST
......
...@@ -8077,6 +8077,8 @@ static const struct bpf_sec_def section_defs[] = { ...@@ -8077,6 +8077,8 @@ static const struct bpf_sec_def section_defs[] = {
SEC_DEF("tp/", TRACEPOINT, 0, SEC_NONE, attach_tp), SEC_DEF("tp/", TRACEPOINT, 0, SEC_NONE, attach_tp),
SEC_DEF("raw_tracepoint/", RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp), SEC_DEF("raw_tracepoint/", RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
SEC_DEF("raw_tp/", RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp), SEC_DEF("raw_tp/", RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
SEC_DEF("raw_tracepoint.w/", RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
SEC_DEF("raw_tp.w/", RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
SEC_DEF("tp_btf/", TRACING, BPF_TRACE_RAW_TP, SEC_ATTACH_BTF, attach_trace), SEC_DEF("tp_btf/", TRACING, BPF_TRACE_RAW_TP, SEC_ATTACH_BTF, attach_trace),
SEC_DEF("fentry/", TRACING, BPF_TRACE_FENTRY, SEC_ATTACH_BTF, attach_trace), SEC_DEF("fentry/", TRACING, BPF_TRACE_FENTRY, SEC_ATTACH_BTF, attach_trace),
SEC_DEF("fmod_ret/", TRACING, BPF_MODIFY_RETURN, SEC_ATTACH_BTF, attach_trace), SEC_DEF("fmod_ret/", TRACING, BPF_MODIFY_RETURN, SEC_ATTACH_BTF, attach_trace),
...@@ -9846,12 +9848,26 @@ struct bpf_link *bpf_program__attach_raw_tracepoint(const struct bpf_program *pr ...@@ -9846,12 +9848,26 @@ struct bpf_link *bpf_program__attach_raw_tracepoint(const struct bpf_program *pr
static struct bpf_link *attach_raw_tp(const struct bpf_program *prog, long cookie) static struct bpf_link *attach_raw_tp(const struct bpf_program *prog, long cookie)
{ {
const char *tp_name; static const char *const prefixes[] = {
"raw_tp/",
"raw_tracepoint/",
"raw_tp.w/",
"raw_tracepoint.w/",
};
size_t i;
const char *tp_name = NULL;
if (str_has_pfx(prog->sec_name, "raw_tp/")) for (i = 0; i < ARRAY_SIZE(prefixes); i++) {
tp_name = prog->sec_name + sizeof("raw_tp/") - 1; if (str_has_pfx(prog->sec_name, prefixes[i])) {
else tp_name = prog->sec_name + strlen(prefixes[i]);
tp_name = prog->sec_name + sizeof("raw_tracepoint/") - 1; break;
}
}
if (!tp_name) {
pr_warn("prog '%s': invalid section name '%s'\n",
prog->name, prog->sec_name);
return libbpf_err_ptr(-EINVAL);
}
return bpf_program__attach_raw_tracepoint(prog, tp_name); return bpf_program__attach_raw_tracepoint(prog, tp_name);
} }
......
...@@ -34,6 +34,21 @@ DECLARE_TRACE(bpf_testmod_test_write_bare, ...@@ -34,6 +34,21 @@ DECLARE_TRACE(bpf_testmod_test_write_bare,
TP_ARGS(task, ctx) 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 */ #endif /* _BPF_TESTMOD_EVENTS_H */
#undef TRACE_INCLUDE_PATH #undef TRACE_INCLUDE_PATH
......
...@@ -50,6 +50,16 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj, ...@@ -50,6 +50,16 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
if (bpf_testmod_loop_test(101) > 100) if (bpf_testmod_loop_test(101) > 100)
trace_bpf_testmod_test_read(current, &ctx); trace_bpf_testmod_test_read(current, &ctx);
/* Magic number to enable writable tp */
if (len == 64) {
struct bpf_testmod_test_writable_ctx writable = {
.val = 1024,
};
trace_bpf_testmod_test_writable_bare(&writable);
if (writable.early_ret)
return snprintf(buf, len, "%d\n", writable.val);
}
return -EIO; /* always fail */ return -EIO; /* always fail */
} }
EXPORT_SYMBOL(bpf_testmod_test_read); EXPORT_SYMBOL(bpf_testmod_test_read);
......
...@@ -17,4 +17,9 @@ struct bpf_testmod_test_write_ctx { ...@@ -17,4 +17,9 @@ struct bpf_testmod_test_write_ctx {
size_t len; size_t len;
}; };
struct bpf_testmod_test_writable_ctx {
bool early_ret;
int val;
};
#endif /* _BPF_TESTMOD_H */ #endif /* _BPF_TESTMOD_H */
...@@ -2,10 +2,36 @@ ...@@ -2,10 +2,36 @@
/* Copyright (c) 2020 Facebook */ /* Copyright (c) 2020 Facebook */
#include <test_progs.h> #include <test_progs.h>
#include <stdbool.h>
#include "test_module_attach.skel.h" #include "test_module_attach.skel.h"
static int duration; static int duration;
static int trigger_module_test_writable(int *val)
{
int fd, err;
char buf[65];
ssize_t rd;
fd = open(BPF_TESTMOD_TEST_FILE, O_RDONLY);
err = -errno;
if (!ASSERT_GE(fd, 0, "testmode_file_open"))
return err;
rd = read(fd, buf, sizeof(buf) - 1);
err = -errno;
if (!ASSERT_GT(rd, 0, "testmod_file_rd_val")) {
close(fd);
return err;
}
buf[rd] = '\0';
*val = strtol(buf, NULL, 0);
close(fd);
return 0;
}
static int delete_module(const char *name, int flags) static int delete_module(const char *name, int flags)
{ {
return syscall(__NR_delete_module, name, flags); return syscall(__NR_delete_module, name, flags);
...@@ -19,6 +45,7 @@ void test_module_attach(void) ...@@ -19,6 +45,7 @@ void test_module_attach(void)
struct test_module_attach__bss *bss; struct test_module_attach__bss *bss;
struct bpf_link *link; struct bpf_link *link;
int err; int err;
int writable_val = 0;
skel = test_module_attach__open(); skel = test_module_attach__open();
if (CHECK(!skel, "skel_open", "failed to open skeleton\n")) if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
...@@ -51,6 +78,14 @@ void test_module_attach(void) ...@@ -51,6 +78,14 @@ void test_module_attach(void)
ASSERT_EQ(bss->fexit_ret, -EIO, "fexit_tet"); ASSERT_EQ(bss->fexit_ret, -EIO, "fexit_tet");
ASSERT_EQ(bss->fmod_ret_read_sz, READ_SZ, "fmod_ret"); ASSERT_EQ(bss->fmod_ret_read_sz, READ_SZ, "fmod_ret");
bss->raw_tp_writable_bare_early_ret = true;
bss->raw_tp_writable_bare_out_val = 0xf1f2f3f4;
ASSERT_OK(trigger_module_test_writable(&writable_val),
"trigger_writable");
ASSERT_EQ(bss->raw_tp_writable_bare_in_val, 1024, "writable_test_in");
ASSERT_EQ(bss->raw_tp_writable_bare_out_val, writable_val,
"writable_test_out");
test_module_attach__detach(skel); test_module_attach__detach(skel);
/* attach fentry/fexit and make sure it get's module reference */ /* attach fentry/fexit and make sure it get's module reference */
......
...@@ -27,6 +27,20 @@ int BPF_PROG(handle_raw_tp_bare, ...@@ -27,6 +27,20 @@ int BPF_PROG(handle_raw_tp_bare,
return 0; return 0;
} }
int raw_tp_writable_bare_in_val = 0;
int raw_tp_writable_bare_early_ret = 0;
int raw_tp_writable_bare_out_val = 0;
SEC("raw_tp.w/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->early_ret = raw_tp_writable_bare_early_ret;
writable->val = raw_tp_writable_bare_out_val;
return 0;
}
__u32 tp_btf_read_sz = 0; __u32 tp_btf_read_sz = 0;
SEC("tp_btf/bpf_testmod_test_read") SEC("tp_btf/bpf_testmod_test_read")
......
...@@ -747,7 +747,7 @@ int trigger_module_test_read(int read_sz) ...@@ -747,7 +747,7 @@ int trigger_module_test_read(int read_sz)
{ {
int fd, err; int fd, err;
fd = open("/sys/kernel/bpf_testmod", O_RDONLY); fd = open(BPF_TESTMOD_TEST_FILE, O_RDONLY);
err = -errno; err = -errno;
if (!ASSERT_GE(fd, 0, "testmod_file_open")) if (!ASSERT_GE(fd, 0, "testmod_file_open"))
return err; return err;
...@@ -769,7 +769,7 @@ int trigger_module_test_write(int write_sz) ...@@ -769,7 +769,7 @@ int trigger_module_test_write(int write_sz)
memset(buf, 'a', write_sz); memset(buf, 'a', write_sz);
buf[write_sz-1] = '\0'; buf[write_sz-1] = '\0';
fd = open("/sys/kernel/bpf_testmod", O_WRONLY); fd = open(BPF_TESTMOD_TEST_FILE, O_WRONLY);
err = -errno; err = -errno;
if (!ASSERT_GE(fd, 0, "testmod_file_open")) { if (!ASSERT_GE(fd, 0, "testmod_file_open")) {
free(buf); free(buf);
......
...@@ -301,3 +301,5 @@ int trigger_module_test_write(int write_sz); ...@@ -301,3 +301,5 @@ int trigger_module_test_write(int write_sz);
#else #else
#define SYS_NANOSLEEP_KPROBE_NAME "sys_nanosleep" #define SYS_NANOSLEEP_KPROBE_NAME "sys_nanosleep"
#endif #endif
#define BPF_TESTMOD_TEST_FILE "/sys/kernel/bpf_testmod"
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment