* [PATCH bpf-next 0/3] libbpf: support custom SEC() handlers
@ 2022-02-05 1:27 Andrii Nakryiko
2022-02-05 1:27 ` [PATCH bpf-next 1/3] libbpf: allow BPF program auto-attach handlers to bail out Andrii Nakryiko
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Andrii Nakryiko @ 2022-02-05 1:27 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team, Alan Maguire
Add ability for user applications and libraries to register custom BPF program
SEC() handlers. See patch #2 for examples where this is useful.
Patch #1 does some preliminary refactoring to allow exponsing program
init, preload, and attach callbacks as public API. It also establishes
a protocol to allow optional auto-attach behavior. This will also help the
case of sometimes auto-attachable uprobes.
Cc: Alan Maguire <alan.maguire@oracle.com>
Andrii Nakryiko (3):
libbpf: allow BPF program auto-attach handlers to bail out
libbpf: support custom SEC() handlers
selftests/bpf: add custom SEC() handling selftest
tools/lib/bpf/libbpf.c | 299 ++++++++++++------
tools/lib/bpf/libbpf.h | 81 +++++
tools/lib/bpf/libbpf.map | 2 +
.../bpf/prog_tests/custom_sec_handlers.c | 136 ++++++++
.../bpf/progs/test_custom_sec_handlers.c | 51 +++
5 files changed, 480 insertions(+), 89 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
create mode 100644 tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH bpf-next 1/3] libbpf: allow BPF program auto-attach handlers to bail out
2022-02-05 1:27 [PATCH bpf-next 0/3] libbpf: support custom SEC() handlers Andrii Nakryiko
@ 2022-02-05 1:27 ` Andrii Nakryiko
2022-02-07 12:16 ` Alan Maguire
2022-02-05 1:27 ` [PATCH bpf-next 1/1] selftests/bpf: add custom SEC() handling selftest Andrii Nakryiko
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2022-02-05 1:27 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team, Alan Maguire
Allow some BPF program types to support auto-attach only in subste of
cases. Currently, if some BPF program type specifies attach callback, it
is assumed that during skeleton attach operation all such programs
either successfully attach or entire skeleton attachment fails. If some
program doesn't support auto-attachment from skeleton, such BPF program
types shouldn't have attach callback specified.
This is limiting for cases when, depending on how full the SEC("")
definition is, there could either be enough details to support
auto-attach or there might not be and user has to use some specific API
to provide more details at runtime.
One specific example of such desired behavior might be SEC("uprobe"). If
it's specified as just uprobe auto-attach isn't possible. But if it's
SEC("uprobe/<some_binary>:<some_func>") then there are enough details to
support auto-attach.
Another improvement to the way libbpf is handling SEC()s would be to not
require providing dummy kernel function name for kprobe. Currently,
SEC("kprobe/whatever") is necessary even if actual kernel function is
determined by user at runtime and bpf_program__attach_kprobe() is used
to specify it. With changes in this patch, it's possible to support both
SEC("kprobe") and SEC("kprobe/<actual_kernel_function"), while only in
the latter case auto-attach will be performed. In the former one, such
kprobe will be skipped during skeleton attach operation.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 110 +++++++++++++++++++++++++----------------
1 file changed, 67 insertions(+), 43 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 904cdf83002b..2902534def2c 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -209,11 +209,12 @@ struct reloc_desc {
};
};
-struct bpf_sec_def;
-
-typedef int (*init_fn_t)(struct bpf_program *prog, long cookie);
-typedef int (*preload_fn_t)(struct bpf_program *prog, struct bpf_prog_load_opts *opts, long cookie);
-typedef struct bpf_link *(*attach_fn_t)(const struct bpf_program *prog, long cookie);
+typedef int (*libbpf_prog_init_fn_t)(struct bpf_program *prog, long cookie);
+typedef int (*libbpf_prog_preload_fn_t)(struct bpf_program *prog,
+ struct bpf_prog_load_opts *opts, long cookie);
+/* If auto-attach is not supported, callback should return 0 and set link to NULL */
+typedef int (*libbpf_prog_attach_fn_t)(const struct bpf_program *prog, long cookie,
+ struct bpf_link **link);
/* stored as sec_def->cookie for all libbpf-supported SEC()s */
enum sec_def_flags {
@@ -247,9 +248,9 @@ struct bpf_sec_def {
enum bpf_attach_type expected_attach_type;
long cookie;
- init_fn_t init_fn;
- preload_fn_t preload_fn;
- attach_fn_t attach_fn;
+ libbpf_prog_init_fn_t init_fn;
+ libbpf_prog_preload_fn_t preload_fn;
+ libbpf_prog_attach_fn_t attach_fn;
};
/*
@@ -8589,12 +8590,12 @@ int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log
__VA_ARGS__ \
}
-static struct bpf_link *attach_kprobe(const struct bpf_program *prog, long cookie);
-static struct bpf_link *attach_tp(const struct bpf_program *prog, long cookie);
-static struct bpf_link *attach_raw_tp(const struct bpf_program *prog, long cookie);
-static struct bpf_link *attach_trace(const struct bpf_program *prog, long cookie);
-static struct bpf_link *attach_lsm(const struct bpf_program *prog, long cookie);
-static struct bpf_link *attach_iter(const struct bpf_program *prog, long cookie);
+static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link);
+static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
+static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
+static int attach_trace(const struct bpf_program *prog, long cookie, struct bpf_link **link);
+static int attach_lsm(const struct bpf_program *prog, long cookie, struct bpf_link **link);
+static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_link **link);
static const struct bpf_sec_def section_defs[] = {
SEC_DEF("socket", SOCKET_FILTER, 0, SEC_NONE | SEC_SLOPPY_PFX),
@@ -10101,14 +10102,13 @@ struct bpf_link *bpf_program__attach_kprobe(const struct bpf_program *prog,
return bpf_program__attach_kprobe_opts(prog, func_name, &opts);
}
-static struct bpf_link *attach_kprobe(const struct bpf_program *prog, long cookie)
+static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link)
{
DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts);
unsigned long offset = 0;
- struct bpf_link *link;
const char *func_name;
char *func;
- int n, err;
+ int n;
opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe/");
if (opts.retprobe)
@@ -10118,21 +10118,19 @@ static struct bpf_link *attach_kprobe(const struct bpf_program *prog, long cooki
n = sscanf(func_name, "%m[a-zA-Z0-9_.]+%li", &func, &offset);
if (n < 1) {
- err = -EINVAL;
pr_warn("kprobe name is invalid: %s\n", func_name);
- return libbpf_err_ptr(err);
+ return -EINVAL;
}
if (opts.retprobe && offset != 0) {
free(func);
- err = -EINVAL;
pr_warn("kretprobes do not support offset specification\n");
- return libbpf_err_ptr(err);
+ return -EINVAL;
}
opts.offset = offset;
- link = bpf_program__attach_kprobe_opts(prog, func, &opts);
+ *link = bpf_program__attach_kprobe_opts(prog, func, &opts);
free(func);
- return link;
+ return libbpf_get_error(*link);
}
static void gen_uprobe_legacy_event_name(char *buf, size_t buf_sz,
@@ -10387,14 +10385,13 @@ struct bpf_link *bpf_program__attach_tracepoint(const struct bpf_program *prog,
return bpf_program__attach_tracepoint_opts(prog, tp_category, tp_name, NULL);
}
-static struct bpf_link *attach_tp(const struct bpf_program *prog, long cookie)
+static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link)
{
char *sec_name, *tp_cat, *tp_name;
- struct bpf_link *link;
sec_name = strdup(prog->sec_name);
if (!sec_name)
- return libbpf_err_ptr(-ENOMEM);
+ return -ENOMEM;
/* extract "tp/<category>/<name>" or "tracepoint/<category>/<name>" */
if (str_has_pfx(prog->sec_name, "tp/"))
@@ -10404,14 +10401,14 @@ static struct bpf_link *attach_tp(const struct bpf_program *prog, long cookie)
tp_name = strchr(tp_cat, '/');
if (!tp_name) {
free(sec_name);
- return libbpf_err_ptr(-EINVAL);
+ return -EINVAL;
}
*tp_name = '\0';
tp_name++;
- link = bpf_program__attach_tracepoint(prog, tp_cat, tp_name);
+ *link = bpf_program__attach_tracepoint(prog, tp_cat, tp_name);
free(sec_name);
- return link;
+ return libbpf_get_error(*link);
}
struct bpf_link *bpf_program__attach_raw_tracepoint(const struct bpf_program *prog,
@@ -10444,7 +10441,7 @@ struct bpf_link *bpf_program__attach_raw_tracepoint(const struct bpf_program *pr
return link;
}
-static struct bpf_link *attach_raw_tp(const struct bpf_program *prog, long cookie)
+static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link)
{
static const char *const prefixes[] = {
"raw_tp/",
@@ -10464,10 +10461,11 @@ static struct bpf_link *attach_raw_tp(const struct bpf_program *prog, long cooki
if (!tp_name) {
pr_warn("prog '%s': invalid section name '%s'\n",
prog->name, prog->sec_name);
- return libbpf_err_ptr(-EINVAL);
+ return -EINVAL;
}
- return bpf_program__attach_raw_tracepoint(prog, tp_name);
+ *link = bpf_program__attach_raw_tracepoint(prog, tp_name);
+ return libbpf_get_error(link);
}
/* Common logic for all BPF program types that attach to a btf_id */
@@ -10510,14 +10508,16 @@ struct bpf_link *bpf_program__attach_lsm(const struct bpf_program *prog)
return bpf_program__attach_btf_id(prog);
}
-static struct bpf_link *attach_trace(const struct bpf_program *prog, long cookie)
+static int attach_trace(const struct bpf_program *prog, long cookie, struct bpf_link **link)
{
- return bpf_program__attach_trace(prog);
+ *link = bpf_program__attach_trace(prog);
+ return libbpf_get_error(*link);
}
-static struct bpf_link *attach_lsm(const struct bpf_program *prog, long cookie)
+static int attach_lsm(const struct bpf_program *prog, long cookie, struct bpf_link **link)
{
- return bpf_program__attach_lsm(prog);
+ *link = bpf_program__attach_lsm(prog);
+ return libbpf_get_error(*link);
}
static struct bpf_link *
@@ -10646,17 +10646,31 @@ bpf_program__attach_iter(const struct bpf_program *prog,
return link;
}
-static struct bpf_link *attach_iter(const struct bpf_program *prog, long cookie)
+static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_link **link)
{
- return bpf_program__attach_iter(prog, NULL);
+ *link = bpf_program__attach_iter(prog, NULL);
+ return libbpf_get_error(*link);
}
struct bpf_link *bpf_program__attach(const struct bpf_program *prog)
{
+ struct bpf_link *link;
+ int err;
+
if (!prog->sec_def || !prog->sec_def->attach_fn)
- return libbpf_err_ptr(-ESRCH);
+ return libbpf_err_ptr(-EOPNOTSUPP);
+
+ err = prog->sec_def->attach_fn(prog, prog->sec_def->cookie, &link);
+ if (err)
+ return libbpf_err_ptr(err);
+
+ /* auto-attach support is optional (see also comment in
+ * bpf_object__attach_skeleton()), but when explicitly expected by
+ * user it's an error if it's not */
+ if (!link)
+ return libbpf_err_ptr(-EOPNOTSUPP);
- return prog->sec_def->attach_fn(prog, prog->sec_def->cookie);
+ return link;
}
static int bpf_link__detach_struct_ops(struct bpf_link *link)
@@ -11800,13 +11814,23 @@ int bpf_object__attach_skeleton(struct bpf_object_skeleton *s)
if (!prog->sec_def || !prog->sec_def->attach_fn)
continue;
- *link = bpf_program__attach(prog);
- err = libbpf_get_error(*link);
+ err = prog->sec_def->attach_fn(prog, prog->sec_def->cookie, link);
if (err) {
- pr_warn("failed to auto-attach program '%s': %d\n",
+ pr_warn("prog '%s': failed to auto-attach: %d\n",
bpf_program__name(prog), err);
return libbpf_err(err);
}
+
+ /* It's possible that for some SEC() definitions auto-attach
+ * is supported in some cases (e.g., if definition completely
+ * specifies target information), but is not in other cases.
+ * SEC("uprobe") is one such case. If user specified target
+ * binary and function name, such BPF program can be
+ * auto-attached. But if not, it shouldn't trigger skeleton's
+ * attach to fail. It should just be skipped.
+ * attach_fn signals such case with returning 0 (no error) and
+ * setting link to NULL.
+ */
}
return 0;
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next 1/1] selftests/bpf: add custom SEC() handling selftest
2022-02-05 1:27 [PATCH bpf-next 0/3] libbpf: support custom SEC() handlers Andrii Nakryiko
2022-02-05 1:27 ` [PATCH bpf-next 1/3] libbpf: allow BPF program auto-attach handlers to bail out Andrii Nakryiko
@ 2022-02-05 1:27 ` Andrii Nakryiko
2022-02-05 1:33 ` Andrii Nakryiko
2022-02-05 1:27 ` [PATCH bpf-next 2/3] libbpf: support custom SEC() handlers Andrii Nakryiko
2022-02-05 1:27 ` [PATCH bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest Andrii Nakryiko
3 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2022-02-05 1:27 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team, Alan Maguire
Add a selftest validating various aspects of libbpf's handling of custom
SEC() handlers. It also demonstrates how libraries can ensure very early
callbacks registration and unregistration using
__attribute__((constructor))/__attribute__((destructor)) functions.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
.../bpf/prog_tests/custom_sec_handlers.c | 136 ++++++++++++++++++
.../bpf/progs/test_custom_sec_handlers.c | 51 +++++++
2 files changed, 187 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
create mode 100644 tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
diff --git a/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
new file mode 100644
index 000000000000..8e43c5f21878
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022 Facebook */
+
+#include <test_progs.h>
+#include "test_custom_sec_handlers.skel.h"
+
+#define COOKIE_ABC1 1
+#define COOKIE_ABC2 2
+#define COOKIE_CUSTOM 3
+#define COOKIE_FALLBACK 4
+
+static int custom_init_prog(struct bpf_program *prog, long cookie)
+{
+ if (cookie == COOKIE_ABC1)
+ bpf_program__set_autoload(prog, false);
+
+ return 0;
+}
+
+static int custom_preload_prog(struct bpf_program *prog,
+ struct bpf_prog_load_opts *opts, long cookie)
+{
+ return 0;
+}
+
+static int custom_attach_prog(const struct bpf_program *prog, long cookie,
+ struct bpf_link **link)
+{
+ switch (cookie) {
+ case COOKIE_ABC2:
+ *link = bpf_program__attach_raw_tracepoint(prog, "sys_enter");
+ return libbpf_get_error(*link);
+ case COOKIE_CUSTOM:
+ *link = bpf_program__attach_tracepoint(prog, "syscalls", "sys_enter_nanosleep");
+ return libbpf_get_error(*link);
+ case COOKIE_FALLBACK:
+ /* no auto-attach for SEC("xyz") */
+ *link = NULL;
+ return 0;
+ default:
+ ASSERT_FALSE(true, "unexpected cookie");
+ return -EINVAL;
+ }
+}
+
+static int abc1_id;
+static int abc2_id;
+static int custom_id;
+static int fallback_id;
+
+__attribute__((constructor))
+static void register_sec_handlers(void)
+{
+ abc1_id = libbpf_register_prog_handler("abc",
+ BPF_PROG_TYPE_RAW_TRACEPOINT, 0,
+ custom_init_prog, custom_preload_prog,
+ custom_attach_prog,
+ COOKIE_ABC1, NULL);
+ abc2_id = libbpf_register_prog_handler("abc/",
+ BPF_PROG_TYPE_RAW_TRACEPOINT, 0,
+ custom_init_prog, custom_preload_prog,
+ custom_attach_prog,
+ COOKIE_ABC2, NULL);
+ custom_id = libbpf_register_prog_handler("custom+",
+ BPF_PROG_TYPE_TRACEPOINT, 0,
+ custom_init_prog, custom_preload_prog,
+ custom_attach_prog,
+ COOKIE_CUSTOM, NULL);
+}
+
+__attribute__((destructor))
+static void unregister_sec_handlers(void)
+{
+ libbpf_unregister_prog_handler(abc1_id);
+ libbpf_unregister_prog_handler(abc2_id);
+ libbpf_unregister_prog_handler(custom_id);
+}
+
+void test_custom_sec_handlers(void)
+{
+ struct test_custom_sec_handlers* skel;
+ int err;
+
+ ASSERT_GT(abc1_id, 0, "abc1_id");
+ ASSERT_GT(abc2_id, 0, "abc2_id");
+ ASSERT_GT(custom_id, 0, "custom_id");
+
+ fallback_id = libbpf_register_prog_handler(NULL, /* fallback handler */
+ BPF_PROG_TYPE_KPROBE, 0,
+ custom_init_prog, custom_preload_prog,
+ custom_attach_prog,
+ COOKIE_FALLBACK, NULL);
+ if (!ASSERT_GT(fallback_id, 0, "fallback_id"))
+ return;
+
+ /* open skeleton and validate assumptions */
+ skel = test_custom_sec_handlers__open();
+ if (!ASSERT_OK_PTR(skel, "skel_open"))
+ goto cleanup;
+
+ ASSERT_EQ(bpf_program__type(skel->progs.abc1), BPF_PROG_TYPE_RAW_TRACEPOINT, "abc1_type");
+ ASSERT_FALSE(bpf_program__autoload(skel->progs.abc1), "abc1_autoload");
+
+ ASSERT_EQ(bpf_program__type(skel->progs.abc2), BPF_PROG_TYPE_RAW_TRACEPOINT, "abc2_type");
+ ASSERT_EQ(bpf_program__type(skel->progs.custom1), BPF_PROG_TYPE_TRACEPOINT, "custom1_type");
+ ASSERT_EQ(bpf_program__type(skel->progs.custom2), BPF_PROG_TYPE_TRACEPOINT, "custom2_type");
+ ASSERT_EQ(bpf_program__type(skel->progs.xyz), BPF_PROG_TYPE_KPROBE, "xyz_type");
+
+ skel->rodata->my_pid = getpid();
+
+ /* now attempt to load everything */
+ err = test_custom_sec_handlers__load(skel);
+ if (!ASSERT_OK(err, "skel_load"))
+ goto cleanup;
+
+ /* now try to auto-attach everything */
+ err = test_custom_sec_handlers__attach(skel);
+ if (!ASSERT_OK(err, "skel_attach"))
+ goto cleanup;
+
+ /* trigger programs */
+ usleep(1);
+
+ /* SEC("abc") is set to not auto-loaded */
+ ASSERT_FALSE(skel->bss->abc1_called, "abc1_called");
+ ASSERT_TRUE(skel->bss->abc2_called, "abc2_called");
+ ASSERT_TRUE(skel->bss->custom1_called, "custom1_called");
+ ASSERT_TRUE(skel->bss->custom2_called, "custom2_called");
+ /* SEC("xyz") shouldn't be auto-attached */
+ ASSERT_FALSE(skel->bss->xyz_called, "xyz_called");
+
+cleanup:
+ test_custom_sec_handlers__destroy(skel);
+
+ ASSERT_OK(libbpf_unregister_prog_handler(fallback_id), "unregister_fallback");
+}
diff --git a/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c b/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
new file mode 100644
index 000000000000..2df368783678
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022 Facebook */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+const volatile int my_pid;
+
+bool abc1_called;
+bool abc2_called;
+bool custom1_called;
+bool custom2_called;
+bool xyz_called;
+
+SEC("abc")
+int abc1(void *ctx)
+{
+ abc1_called = true;
+ return 0;
+}
+
+SEC("abc/whatever")
+int abc2(void *ctx)
+{
+ abc2_called = true;
+ return 0;
+}
+
+SEC("custom")
+int custom1(void *ctx)
+{
+ custom1_called = true;
+ return 0;
+}
+
+SEC("custom/something")
+int custom2(void *ctx)
+{
+ custom2_called = true;
+ return 0;
+}
+
+SEC("xyz/blah")
+int xyz(void *ctx)
+{
+ xyz_called = true;
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next 2/3] libbpf: support custom SEC() handlers
2022-02-05 1:27 [PATCH bpf-next 0/3] libbpf: support custom SEC() handlers Andrii Nakryiko
2022-02-05 1:27 ` [PATCH bpf-next 1/3] libbpf: allow BPF program auto-attach handlers to bail out Andrii Nakryiko
2022-02-05 1:27 ` [PATCH bpf-next 1/1] selftests/bpf: add custom SEC() handling selftest Andrii Nakryiko
@ 2022-02-05 1:27 ` Andrii Nakryiko
2022-02-07 13:56 ` Alan Maguire
2022-02-05 1:27 ` [PATCH bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest Andrii Nakryiko
3 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2022-02-05 1:27 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team, Alan Maguire
Allow registering and unregistering custom handlers for BPF program.
This allows user applications and libraries to plug into libbpf's
declarative SEC() definition handling logic. This allows to offload
complex and intricate custom logic into external libraries, but still
provide a great user experience.
One such example is USDT handling library, which has a lot of code and
complexity which doesn't make sense to put into libbpf directly, but it
would be really great for users to be able to specify BPF programs with
something like SEC("usdt/<path-to-binary>:<usdt_provider>:<usdt_name>")
and have correct BPF program type set (BPF_PROGRAM_TYPE_KPROBE, as it is
uprobe) and even support BPF skeleton's auto-attach logic.
In some cases, it might be even good idea to override libbpf's default
handling, like for SEC("perf_event") programs. With custom library, it's
possible to extend logic to support specifying perf event specification
right there in SEC() definition without burdening libbpf with lots of
custom logic or extra library dependecies (e.g., libpfm4). With current
patch it's possible to override libbpf's SEC("perf_event") handling and
specify a completely custom ones.
Further, it's possible to specify a generic fallback handling for any
SEC() that doesn't match any other custom or standard libbpf handlers.
This allows to accommodate whatever legacy use cases there might be, if
necessary.
See doc comments for libbpf_register_prog_handler() and
libbpf_unregister_prog_handler() for detailed semantics.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 201 +++++++++++++++++++++++++++++----------
tools/lib/bpf/libbpf.h | 81 ++++++++++++++++
tools/lib/bpf/libbpf.map | 2 +
3 files changed, 232 insertions(+), 52 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2902534def2c..d78a6365ba74 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -209,13 +209,6 @@ struct reloc_desc {
};
};
-typedef int (*libbpf_prog_init_fn_t)(struct bpf_program *prog, long cookie);
-typedef int (*libbpf_prog_preload_fn_t)(struct bpf_program *prog,
- struct bpf_prog_load_opts *opts, long cookie);
-/* If auto-attach is not supported, callback should return 0 and set link to NULL */
-typedef int (*libbpf_prog_attach_fn_t)(const struct bpf_program *prog, long cookie,
- struct bpf_link **link);
-
/* stored as sec_def->cookie for all libbpf-supported SEC()s */
enum sec_def_flags {
SEC_NONE = 0,
@@ -243,10 +236,11 @@ enum sec_def_flags {
};
struct bpf_sec_def {
- const char *sec;
+ char *sec;
enum bpf_prog_type prog_type;
enum bpf_attach_type expected_attach_type;
long cookie;
+ int handler_id;
libbpf_prog_init_fn_t init_fn;
libbpf_prog_preload_fn_t preload_fn;
@@ -8582,7 +8576,7 @@ int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log
}
#define SEC_DEF(sec_pfx, ptype, atype, flags, ...) { \
- .sec = sec_pfx, \
+ .sec = (char *)sec_pfx, \
.prog_type = BPF_PROG_TYPE_##ptype, \
.expected_attach_type = atype, \
.cookie = (long)(flags), \
@@ -8675,61 +8669,164 @@ static const struct bpf_sec_def section_defs[] = {
SEC_DEF("sk_lookup", SK_LOOKUP, BPF_SK_LOOKUP, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
};
-#define MAX_TYPE_NAME_SIZE 32
+static size_t custom_sec_def_cnt;
+static struct bpf_sec_def *custom_sec_defs;
+static struct bpf_sec_def custom_fallback_def;
+static bool has_custom_fallback_def;
-static const struct bpf_sec_def *find_sec_def(const char *sec_name)
+static int last_custom_sec_def_handler_id;
+
+int libbpf_register_prog_handler(const char *sec,
+ enum bpf_prog_type prog_type,
+ enum bpf_attach_type exp_attach_type,
+ libbpf_prog_init_fn_t prog_init_fn,
+ libbpf_prog_preload_fn_t prog_preload_fn,
+ libbpf_prog_attach_fn_t prog_attach_fn,
+ long cookie,
+ const void *opts)
{
- const struct bpf_sec_def *sec_def;
- enum sec_def_flags sec_flags;
- int i, n = ARRAY_SIZE(section_defs), len;
- bool strict = libbpf_mode & LIBBPF_STRICT_SEC_NAME;
+ struct bpf_sec_def *sec_def;
- for (i = 0; i < n; i++) {
- sec_def = §ion_defs[i];
- sec_flags = sec_def->cookie;
- len = strlen(sec_def->sec);
+ if (opts)
+ return libbpf_err(-EINVAL);
+ if (last_custom_sec_def_handler_id == INT_MAX) /* prevent overflow */
+ return libbpf_err(-E2BIG);
- /* "type/" always has to have proper SEC("type/extras") form */
- if (sec_def->sec[len - 1] == '/') {
- if (str_has_pfx(sec_name, sec_def->sec))
- return sec_def;
- continue;
- }
+ if (sec) {
+ sec_def = libbpf_reallocarray(custom_sec_defs, custom_sec_def_cnt + 1,
+ sizeof(*sec_def));
+ if (!sec_def)
+ return libbpf_err(-ENOMEM);
- /* "type+" means it can be either exact SEC("type") or
- * well-formed SEC("type/extras") with proper '/' separator
- */
- if (sec_def->sec[len - 1] == '+') {
- len--;
- /* not even a prefix */
- if (strncmp(sec_name, sec_def->sec, len) != 0)
- continue;
- /* exact match or has '/' separator */
- if (sec_name[len] == '\0' || sec_name[len] == '/')
- return sec_def;
- continue;
- }
+ custom_sec_defs = sec_def;
+ sec_def = &custom_sec_defs[custom_sec_def_cnt];
+ } else {
+ if (has_custom_fallback_def)
+ return libbpf_err(-EBUSY);
- /* SEC_SLOPPY_PFX definitions are allowed to be just prefix
- * matches, unless strict section name mode
- * (LIBBPF_STRICT_SEC_NAME) is enabled, in which case the
- * match has to be exact.
- */
- if ((sec_flags & SEC_SLOPPY_PFX) && !strict) {
- if (str_has_pfx(sec_name, sec_def->sec))
- return sec_def;
- continue;
- }
+ sec_def = &custom_fallback_def;
+ }
- /* Definitions not marked SEC_SLOPPY_PFX (e.g.,
- * SEC("syscall")) are exact matches in both modes.
- */
- if (strcmp(sec_name, sec_def->sec) == 0)
+ sec_def->sec = sec ? strdup(sec) : NULL;
+ if (sec && !sec_def->sec)
+ return libbpf_err(-ENOMEM);
+
+ sec_def->prog_type = prog_type;
+ sec_def->expected_attach_type = exp_attach_type;
+ sec_def->cookie = cookie;
+
+ sec_def->init_fn = prog_init_fn;
+ sec_def->preload_fn = prog_preload_fn;
+ sec_def->attach_fn = prog_attach_fn;
+
+ sec_def->handler_id = ++last_custom_sec_def_handler_id;
+
+ if (sec)
+ custom_sec_def_cnt++;
+ else
+ has_custom_fallback_def = true;
+
+ return sec_def->handler_id;
+}
+
+int libbpf_unregister_prog_handler(int handler_id)
+{
+ int i;
+
+ if (handler_id <= 0)
+ return libbpf_err(-EINVAL);
+
+ if (has_custom_fallback_def && custom_fallback_def.handler_id == handler_id) {
+ memset(&custom_fallback_def, 0, sizeof(custom_fallback_def));
+ has_custom_fallback_def = false;
+ return 0;
+ }
+
+ for (i = 0; i < custom_sec_def_cnt; i++) {
+ if (custom_sec_defs[i].handler_id == handler_id)
+ break;
+ }
+
+ if (i == custom_sec_def_cnt)
+ return libbpf_err(-ENOENT);
+
+ free(custom_sec_defs[i].sec);
+ for (i = i + 1; i < custom_sec_def_cnt; i++)
+ custom_sec_defs[i - 1] = custom_sec_defs[i];
+ custom_sec_def_cnt--;
+
+ return 0;
+}
+
+static bool sec_def_matches(const struct bpf_sec_def *sec_def, const char *sec_name,
+ bool allow_sloppy)
+{
+ size_t len = strlen(sec_def->sec);
+
+ /* "type/" always has to have proper SEC("type/extras") form */
+ if (sec_def->sec[len - 1] == '/') {
+ if (str_has_pfx(sec_name, sec_def->sec))
+ return true;
+ return false;
+ }
+
+ /* "type+" means it can be either exact SEC("type") or
+ * well-formed SEC("type/extras") with proper '/' separator
+ */
+ if (sec_def->sec[len - 1] == '+') {
+ len--;
+ /* not even a prefix */
+ if (strncmp(sec_name, sec_def->sec, len) != 0)
+ return false;
+ /* exact match or has '/' separator */
+ if (sec_name[len] == '\0' || sec_name[len] == '/')
+ return true;
+ return false;
+ }
+
+ /* SEC_SLOPPY_PFX definitions are allowed to be just prefix
+ * matches, unless strict section name mode
+ * (LIBBPF_STRICT_SEC_NAME) is enabled, in which case the
+ * match has to be exact.
+ */
+ if (allow_sloppy && str_has_pfx(sec_name, sec_def->sec))
+ return true;
+
+ /* Definitions not marked SEC_SLOPPY_PFX (e.g.,
+ * SEC("syscall")) are exact matches in both modes.
+ */
+ return strcmp(sec_name, sec_def->sec) == 0;
+}
+
+static const struct bpf_sec_def *find_sec_def(const char *sec_name)
+{
+ const struct bpf_sec_def *sec_def;
+ int i, n;
+ bool strict = libbpf_mode & LIBBPF_STRICT_SEC_NAME, allow_sloppy;
+
+ n = custom_sec_def_cnt;
+ for (i = 0; i < n; i++) {
+ sec_def = &custom_sec_defs[i];
+ if (sec_def_matches(sec_def, sec_name, false))
return sec_def;
}
+
+ n = ARRAY_SIZE(section_defs);
+ for (i = 0; i < n; i++) {
+ sec_def = §ion_defs[i];
+ allow_sloppy = (sec_def->cookie & SEC_SLOPPY_PFX) && !strict;
+ if (sec_def_matches(sec_def, sec_name, allow_sloppy))
+ return sec_def;
+ }
+
+ if (has_custom_fallback_def)
+ return &custom_fallback_def;
+
return NULL;
}
+#define MAX_TYPE_NAME_SIZE 32
+
static char *libbpf_get_type_names(bool attach_type)
{
int i, len = ARRAY_SIZE(section_defs) * MAX_TYPE_NAME_SIZE;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index c8d8daad212e..6e665c26dcc7 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1328,6 +1328,87 @@ LIBBPF_API int bpf_linker__add_file(struct bpf_linker *linker,
LIBBPF_API int bpf_linker__finalize(struct bpf_linker *linker);
LIBBPF_API void bpf_linker__free(struct bpf_linker *linker);
+/*
+ * Custom handling of BPF program's SEC() definitions
+ */
+
+struct bpf_prog_load_opts; /* defined in bpf.h */
+
+/* Called during bpf_object__open() for each recognized BPF program. Callback
+ * can use various bpf_program__set_*() setters to adjust whatever properties
+ * are necessary.
+ */
+typedef int (*libbpf_prog_init_fn_t)(struct bpf_program *prog, long cookie);
+
+/* Called right before libbpf performs bpf_prog_load() to load BPF program
+ * into the kernel. Callback can adjust opts as necessary.
+ */
+typedef int (*libbpf_prog_preload_fn_t)(struct bpf_program *prog,
+ struct bpf_prog_load_opts *opts, long cookie);
+
+/* Called during skeleton attach or through bpf_program__attach(). If
+ * auto-attach is not supported, callback should return 0 and set link to
+ * NULL (it's not considered an error during skeleton attach, but it will be
+ * an error for bpf_program__attach() calls). On error, error should be
+ * returned directly and link set to NULL. On success, return 0 and set link
+ * to a valid struct bpf_link.
+ */
+typedef int (*libbpf_prog_attach_fn_t)(const struct bpf_program *prog, long cookie,
+ struct bpf_link **link);
+
+/**
+ * @brief **libbpf_register_prog_handler()** registers a custom BPF program
+ * SEC() handler.
+ * @param sec section prefix for which custom handler is registered
+ * @param prog_type BPF program type associated with specified section
+ * @param exp_attach_type Expected BPF attach type associated with specified section
+ * @param prog_init_fn BPF program initialization callback (see libbpf_prog_init_fn_t)
+ * @param prog_preload_fn BPF program loading callback (see libbpf_prog_preload_fn_t)
+ * @param prog_attach_fn BPF program attach callback (see libbpf_prog_attach_fn_t)
+ * @param cookie User-provided cookie passed to each callback
+ * @param opts reserved for future extensibility, should be NULL
+ * @return Non-negative handler ID is returned on success. This handler ID has
+ * to be passed to *libbpf_unregister_prog_handler()* to unregister such
+ * custom handler. Negative error code is returned on error.
+ *
+ * *sec* defines which SEC() definitions are handled by this custom handler
+ * registration. *sec* can have few different forms:
+ * - if *sec* is just a plain string (e.g., "abc"), it will match only
+ * SEC("abc"). If BPF program specifies SEC("abc/whatever") it will result
+ * in an error;
+ * - if *sec* is of the form "abc/", proper SEC() form is
+ * SEC("abc/something"), where acceptable "something" should be checked by
+ * *prog_init_fn* callback, if there are additional restrictions;
+ * - if *sec* is of the form "abc+", it will successfully match both
+ * SEC("abc") and SEC("abc/whatever") forms;
+ * - if *sec* is NULL, custom handler is registered for any BPF program that
+ * doesn't match any of the registered (custom or libbpf's own) SEC()
+ * handlers. There could be only one such generic custom handler registered
+ * at any given time.
+ *
+ * All custom handlers (except the one with *sec* == NULL) are processed
+ * before libbpf's own SEC() handlers. It is allowed to "override" libbpf's
+ * SEC() handlers by registering custom ones for the same section prefix
+ * (i.e., it's possible to have custom SEC("perf_event/LLC-load-misses")
+ * handler).
+ */
+LIBBPF_API int libbpf_register_prog_handler(const char *sec,
+ enum bpf_prog_type prog_type,
+ enum bpf_attach_type exp_attach_type,
+ libbpf_prog_init_fn_t prog_init_fn,
+ libbpf_prog_preload_fn_t prog_preload_fn,
+ libbpf_prog_attach_fn_t prog_attach_fn,
+ long cookie,
+ const void *opts);
+/**
+ * @brief *libbpf_unregister_prog_handler()* unregisters previously registered
+ * custom BPF program SEC() handler.
+ * @param handler_id handler ID returned by *libbpf_register_prog_handler()*
+ * after successful registration
+ * @return 0 on success, negative error code if handler isn't found
+ */
+LIBBPF_API int libbpf_unregister_prog_handler(int handler_id);
+
#ifdef __cplusplus
} /* extern "C" */
#endif
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index aef6253a90c8..4e75f06c1a00 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -438,4 +438,6 @@ LIBBPF_0.7.0 {
libbpf_probe_bpf_map_type;
libbpf_probe_bpf_prog_type;
libbpf_set_memlock_rlim_max;
+ libbpf_register_prog_handler;
+ libbpf_unregister_prog_handler;
};
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-05 1:27 [PATCH bpf-next 0/3] libbpf: support custom SEC() handlers Andrii Nakryiko
` (2 preceding siblings ...)
2022-02-05 1:27 ` [PATCH bpf-next 2/3] libbpf: support custom SEC() handlers Andrii Nakryiko
@ 2022-02-05 1:27 ` Andrii Nakryiko
2022-02-07 14:20 ` Alan Maguire
3 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2022-02-05 1:27 UTC (permalink / raw)
To: bpf, ast, daniel; +Cc: andrii, kernel-team, Alan Maguire
Add a selftest validating various aspects of libbpf's handling of custom
SEC() handlers. It also demonstrates how libraries can ensure very early
callbacks registration and unregistration using
__attribute__((constructor))/__attribute__((destructor)) functions.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
.../bpf/prog_tests/custom_sec_handlers.c | 136 ++++++++++++++++++
.../bpf/progs/test_custom_sec_handlers.c | 51 +++++++
2 files changed, 187 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
create mode 100644 tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
diff --git a/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
new file mode 100644
index 000000000000..8e43c5f21878
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022 Facebook */
+
+#include <test_progs.h>
+#include "test_custom_sec_handlers.skel.h"
+
+#define COOKIE_ABC1 1
+#define COOKIE_ABC2 2
+#define COOKIE_CUSTOM 3
+#define COOKIE_FALLBACK 4
+
+static int custom_init_prog(struct bpf_program *prog, long cookie)
+{
+ if (cookie == COOKIE_ABC1)
+ bpf_program__set_autoload(prog, false);
+
+ return 0;
+}
+
+static int custom_preload_prog(struct bpf_program *prog,
+ struct bpf_prog_load_opts *opts, long cookie)
+{
+ return 0;
+}
+
+static int custom_attach_prog(const struct bpf_program *prog, long cookie,
+ struct bpf_link **link)
+{
+ switch (cookie) {
+ case COOKIE_ABC2:
+ *link = bpf_program__attach_raw_tracepoint(prog, "sys_enter");
+ return libbpf_get_error(*link);
+ case COOKIE_CUSTOM:
+ *link = bpf_program__attach_tracepoint(prog, "syscalls", "sys_enter_nanosleep");
+ return libbpf_get_error(*link);
+ case COOKIE_FALLBACK:
+ /* no auto-attach for SEC("xyz") */
+ *link = NULL;
+ return 0;
+ default:
+ ASSERT_FALSE(true, "unexpected cookie");
+ return -EINVAL;
+ }
+}
+
+static int abc1_id;
+static int abc2_id;
+static int custom_id;
+static int fallback_id;
+
+__attribute__((constructor))
+static void register_sec_handlers(void)
+{
+ abc1_id = libbpf_register_prog_handler("abc",
+ BPF_PROG_TYPE_RAW_TRACEPOINT, 0,
+ custom_init_prog, custom_preload_prog,
+ custom_attach_prog,
+ COOKIE_ABC1, NULL);
+ abc2_id = libbpf_register_prog_handler("abc/",
+ BPF_PROG_TYPE_RAW_TRACEPOINT, 0,
+ custom_init_prog, custom_preload_prog,
+ custom_attach_prog,
+ COOKIE_ABC2, NULL);
+ custom_id = libbpf_register_prog_handler("custom+",
+ BPF_PROG_TYPE_TRACEPOINT, 0,
+ custom_init_prog, custom_preload_prog,
+ custom_attach_prog,
+ COOKIE_CUSTOM, NULL);
+}
+
+__attribute__((destructor))
+static void unregister_sec_handlers(void)
+{
+ libbpf_unregister_prog_handler(abc1_id);
+ libbpf_unregister_prog_handler(abc2_id);
+ libbpf_unregister_prog_handler(custom_id);
+}
+
+void test_custom_sec_handlers(void)
+{
+ struct test_custom_sec_handlers* skel;
+ int err;
+
+ ASSERT_GT(abc1_id, 0, "abc1_id");
+ ASSERT_GT(abc2_id, 0, "abc2_id");
+ ASSERT_GT(custom_id, 0, "custom_id");
+
+ fallback_id = libbpf_register_prog_handler(NULL, /* fallback handler */
+ BPF_PROG_TYPE_KPROBE, 0,
+ custom_init_prog, custom_preload_prog,
+ custom_attach_prog,
+ COOKIE_FALLBACK, NULL);
+ if (!ASSERT_GT(fallback_id, 0, "fallback_id"))
+ return;
+
+ /* open skeleton and validate assumptions */
+ skel = test_custom_sec_handlers__open();
+ if (!ASSERT_OK_PTR(skel, "skel_open"))
+ goto cleanup;
+
+ ASSERT_EQ(bpf_program__type(skel->progs.abc1), BPF_PROG_TYPE_RAW_TRACEPOINT, "abc1_type");
+ ASSERT_FALSE(bpf_program__autoload(skel->progs.abc1), "abc1_autoload");
+
+ ASSERT_EQ(bpf_program__type(skel->progs.abc2), BPF_PROG_TYPE_RAW_TRACEPOINT, "abc2_type");
+ ASSERT_EQ(bpf_program__type(skel->progs.custom1), BPF_PROG_TYPE_TRACEPOINT, "custom1_type");
+ ASSERT_EQ(bpf_program__type(skel->progs.custom2), BPF_PROG_TYPE_TRACEPOINT, "custom2_type");
+ ASSERT_EQ(bpf_program__type(skel->progs.xyz), BPF_PROG_TYPE_KPROBE, "xyz_type");
+
+ skel->rodata->my_pid = getpid();
+
+ /* now attempt to load everything */
+ err = test_custom_sec_handlers__load(skel);
+ if (!ASSERT_OK(err, "skel_load"))
+ goto cleanup;
+
+ /* now try to auto-attach everything */
+ err = test_custom_sec_handlers__attach(skel);
+ if (!ASSERT_OK(err, "skel_attach"))
+ goto cleanup;
+
+ /* trigger programs */
+ usleep(1);
+
+ /* SEC("abc") is set to not auto-loaded */
+ ASSERT_FALSE(skel->bss->abc1_called, "abc1_called");
+ ASSERT_TRUE(skel->bss->abc2_called, "abc2_called");
+ ASSERT_TRUE(skel->bss->custom1_called, "custom1_called");
+ ASSERT_TRUE(skel->bss->custom2_called, "custom2_called");
+ /* SEC("xyz") shouldn't be auto-attached */
+ ASSERT_FALSE(skel->bss->xyz_called, "xyz_called");
+
+cleanup:
+ test_custom_sec_handlers__destroy(skel);
+
+ ASSERT_OK(libbpf_unregister_prog_handler(fallback_id), "unregister_fallback");
+}
diff --git a/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c b/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
new file mode 100644
index 000000000000..2df368783678
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022 Facebook */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+const volatile int my_pid;
+
+bool abc1_called;
+bool abc2_called;
+bool custom1_called;
+bool custom2_called;
+bool xyz_called;
+
+SEC("abc")
+int abc1(void *ctx)
+{
+ abc1_called = true;
+ return 0;
+}
+
+SEC("abc/whatever")
+int abc2(void *ctx)
+{
+ abc2_called = true;
+ return 0;
+}
+
+SEC("custom")
+int custom1(void *ctx)
+{
+ custom1_called = true;
+ return 0;
+}
+
+SEC("custom/something")
+int custom2(void *ctx)
+{
+ custom2_called = true;
+ return 0;
+}
+
+SEC("xyz/blah")
+int xyz(void *ctx)
+{
+ xyz_called = true;
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 1/1] selftests/bpf: add custom SEC() handling selftest
2022-02-05 1:27 ` [PATCH bpf-next 1/1] selftests/bpf: add custom SEC() handling selftest Andrii Nakryiko
@ 2022-02-05 1:33 ` Andrii Nakryiko
0 siblings, 0 replies; 12+ messages in thread
From: Andrii Nakryiko @ 2022-02-05 1:33 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team, Alan Maguire
On Fri, Feb 4, 2022 at 5:27 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Add a selftest validating various aspects of libbpf's handling of custom
> SEC() handlers. It also demonstrates how libraries can ensure very early
> callbacks registration and unregistration using
> __attribute__((constructor))/__attribute__((destructor)) functions.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
I screwed up submission and sent selftest as 1/1 and 1/3 patch. This
confuses CI, I'll resubmit entire series as v2, sorry for the noise.
> .../bpf/prog_tests/custom_sec_handlers.c | 136 ++++++++++++++++++
> .../bpf/progs/test_custom_sec_handlers.c | 51 +++++++
> 2 files changed, 187 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
>
[...]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 1/3] libbpf: allow BPF program auto-attach handlers to bail out
2022-02-05 1:27 ` [PATCH bpf-next 1/3] libbpf: allow BPF program auto-attach handlers to bail out Andrii Nakryiko
@ 2022-02-07 12:16 ` Alan Maguire
2022-02-07 21:32 ` Andrii Nakryiko
0 siblings, 1 reply; 12+ messages in thread
From: Alan Maguire @ 2022-02-07 12:16 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, kernel-team, Alan Maguire
On Sat, 5 Feb 2022, Andrii Nakryiko wrote:
> Allow some BPF program types to support auto-attach only in subste of
> cases. Currently, if some BPF program type specifies attach callback, it
> is assumed that during skeleton attach operation all such programs
> either successfully attach or entire skeleton attachment fails. If some
> program doesn't support auto-attachment from skeleton, such BPF program
> types shouldn't have attach callback specified.
>
This is a great feature! I've had cases where I had to
implement custom section-specific handling before, so this
will make that process much easier!
> This is limiting for cases when, depending on how full the SEC("")
> definition is, there could either be enough details to support
> auto-attach or there might not be and user has to use some specific API
> to provide more details at runtime.
>
> One specific example of such desired behavior might be SEC("uprobe"). If
> it's specified as just uprobe auto-attach isn't possible. But if it's
> SEC("uprobe/<some_binary>:<some_func>") then there are enough details to
> support auto-attach.
Would be good to describe the different handling for explicit
bpf_program__attach() (which fails when auto-attach is supported
but does not return a non-NULL link) vs bpf_object__attach_skeleton()
(which skips the NULL link case) here I think; it's all clarified in
comments below but no harm to reiterate at the top-level I think.
>
> Another improvement to the way libbpf is handling SEC()s would be to not
> require providing dummy kernel function name for kprobe. Currently,
> SEC("kprobe/whatever") is necessary even if actual kernel function is
> determined by user at runtime and bpf_program__attach_kprobe() is used
> to specify it. With changes in this patch, it's possible to support both
> SEC("kprobe") and SEC("kprobe/<actual_kernel_function"), while only in
> the latter case auto-attach will be performed. In the former one, such
> kprobe will be skipped during skeleton attach operation.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
A few nits and suggestions for future, but this looks great!
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> ---
> tools/lib/bpf/libbpf.c | 110 +++++++++++++++++++++++++----------------
> 1 file changed, 67 insertions(+), 43 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 904cdf83002b..2902534def2c 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -209,11 +209,12 @@ struct reloc_desc {
> };
> };
>
> -struct bpf_sec_def;
> -
> -typedef int (*init_fn_t)(struct bpf_program *prog, long cookie);
> -typedef int (*preload_fn_t)(struct bpf_program *prog, struct bpf_prog_load_opts *opts, long cookie);
> -typedef struct bpf_link *(*attach_fn_t)(const struct bpf_program *prog, long cookie);
> +typedef int (*libbpf_prog_init_fn_t)(struct bpf_program *prog, long cookie);
> +typedef int (*libbpf_prog_preload_fn_t)(struct bpf_program *prog,
> + struct bpf_prog_load_opts *opts, long cookie);
> +/* If auto-attach is not supported, callback should return 0 and set link to NULL */
> +typedef int (*libbpf_prog_attach_fn_t)(const struct bpf_program *prog, long cookie,
> + struct bpf_link **link);
>
> /* stored as sec_def->cookie for all libbpf-supported SEC()s */
> enum sec_def_flags {
> @@ -247,9 +248,9 @@ struct bpf_sec_def {
> enum bpf_attach_type expected_attach_type;
> long cookie;
>
> - init_fn_t init_fn;
> - preload_fn_t preload_fn;
> - attach_fn_t attach_fn;
> + libbpf_prog_init_fn_t init_fn;
> + libbpf_prog_preload_fn_t preload_fn;
> + libbpf_prog_attach_fn_t attach_fn;
> };
>
> /*
> @@ -8589,12 +8590,12 @@ int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log
> __VA_ARGS__ \
> }
>
> -static struct bpf_link *attach_kprobe(const struct bpf_program *prog, long cookie);
> -static struct bpf_link *attach_tp(const struct bpf_program *prog, long cookie);
> -static struct bpf_link *attach_raw_tp(const struct bpf_program *prog, long cookie);
> -static struct bpf_link *attach_trace(const struct bpf_program *prog, long cookie);
> -static struct bpf_link *attach_lsm(const struct bpf_program *prog, long cookie);
> -static struct bpf_link *attach_iter(const struct bpf_program *prog, long cookie);
> +static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> +static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> +static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> +static int attach_trace(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> +static int attach_lsm(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> +static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_link **link);
>
One thought here - in the future it might be useful to export
these internal auto-attach functions. The reason I suggest this
is some use-cases of auto-attach might involve pre-processing of
the section name, and once the required info is extracted the
auto-attach function could use the original auto-attach functionality.
That could be done separately to what you're doing here of course.
One concrete example of this: I had a BPF program which consisted
of BPF programs containing required attachments to top-level protocol
module functions along with a set of optional attachments to
transport-specific module functions. Since multiple transports were
possible, it was always possible that module A wouldn't be loaded
while module B was, or vice versa. To deal with this, I used the
"o" prefix (optional) for the associated kprobe/kretprobe section
definitions; an "okprobe" might not attach, but a "kprobe" should.
Using the mechanisms in this patch set, this could be easily
implemented by a custom auto-attach which looked for the "o"
then passed the rest of the section string into the default
auto-attach function for kprobes, handling attach errors for
optional sections while passing them through for required ones.
Tracers might find such pre-processing combined with the default
mechanisms useful too; we could even potentially implement
support for ":" separators this way too (convert instances
of ":" to "/" and then call default auto-attach)!
> static const struct bpf_sec_def section_defs[] = {
> SEC_DEF("socket", SOCKET_FILTER, 0, SEC_NONE | SEC_SLOPPY_PFX),
> @@ -10101,14 +10102,13 @@ struct bpf_link *bpf_program__attach_kprobe(const struct bpf_program *prog,
> return bpf_program__attach_kprobe_opts(prog, func_name, &opts);
> }
>
> -static struct bpf_link *attach_kprobe(const struct bpf_program *prog, long cookie)
> +static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> {
> DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts);
> unsigned long offset = 0;
> - struct bpf_link *link;
> const char *func_name;
> char *func;
> - int n, err;
> + int n;
>
> opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe/");
> if (opts.retprobe)
> @@ -10118,21 +10118,19 @@ static struct bpf_link *attach_kprobe(const struct bpf_program *prog, long cooki
>
> n = sscanf(func_name, "%m[a-zA-Z0-9_.]+%li", &func, &offset);
> if (n < 1) {
> - err = -EINVAL;
> pr_warn("kprobe name is invalid: %s\n", func_name);
> - return libbpf_err_ptr(err);
> + return -EINVAL;
> }
> if (opts.retprobe && offset != 0) {
> free(func);
> - err = -EINVAL;
> pr_warn("kretprobes do not support offset specification\n");
> - return libbpf_err_ptr(err);
> + return -EINVAL;
> }
>
> opts.offset = offset;
> - link = bpf_program__attach_kprobe_opts(prog, func, &opts);
> + *link = bpf_program__attach_kprobe_opts(prog, func, &opts);
> free(func);
> - return link;
> + return libbpf_get_error(*link);
> }
>
> static void gen_uprobe_legacy_event_name(char *buf, size_t buf_sz,
> @@ -10387,14 +10385,13 @@ struct bpf_link *bpf_program__attach_tracepoint(const struct bpf_program *prog,
> return bpf_program__attach_tracepoint_opts(prog, tp_category, tp_name, NULL);
> }
>
> -static struct bpf_link *attach_tp(const struct bpf_program *prog, long cookie)
> +static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> {
> char *sec_name, *tp_cat, *tp_name;
> - struct bpf_link *link;
>
> sec_name = strdup(prog->sec_name);
> if (!sec_name)
> - return libbpf_err_ptr(-ENOMEM);
> + return -ENOMEM;
>
> /* extract "tp/<category>/<name>" or "tracepoint/<category>/<name>" */
> if (str_has_pfx(prog->sec_name, "tp/"))
> @@ -10404,14 +10401,14 @@ static struct bpf_link *attach_tp(const struct bpf_program *prog, long cookie)
> tp_name = strchr(tp_cat, '/');
> if (!tp_name) {
> free(sec_name);
> - return libbpf_err_ptr(-EINVAL);
> + return -EINVAL;
> }
> *tp_name = '\0';
> tp_name++;
>
> - link = bpf_program__attach_tracepoint(prog, tp_cat, tp_name);
> + *link = bpf_program__attach_tracepoint(prog, tp_cat, tp_name);
> free(sec_name);
> - return link;
> + return libbpf_get_error(*link);
> }
>
> struct bpf_link *bpf_program__attach_raw_tracepoint(const struct bpf_program *prog,
> @@ -10444,7 +10441,7 @@ struct bpf_link *bpf_program__attach_raw_tracepoint(const struct bpf_program *pr
> return link;
> }
>
> -static struct bpf_link *attach_raw_tp(const struct bpf_program *prog, long cookie)
> +static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> {
> static const char *const prefixes[] = {
> "raw_tp/",
> @@ -10464,10 +10461,11 @@ static struct bpf_link *attach_raw_tp(const struct bpf_program *prog, long cooki
> if (!tp_name) {
> pr_warn("prog '%s': invalid section name '%s'\n",
> prog->name, prog->sec_name);
> - return libbpf_err_ptr(-EINVAL);
> + return -EINVAL;
> }
>
> - return bpf_program__attach_raw_tracepoint(prog, tp_name);
> + *link = bpf_program__attach_raw_tracepoint(prog, tp_name);
> + return libbpf_get_error(link);
> }
>
> /* Common logic for all BPF program types that attach to a btf_id */
> @@ -10510,14 +10508,16 @@ struct bpf_link *bpf_program__attach_lsm(const struct bpf_program *prog)
> return bpf_program__attach_btf_id(prog);
> }
>
> -static struct bpf_link *attach_trace(const struct bpf_program *prog, long cookie)
> +static int attach_trace(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> {
> - return bpf_program__attach_trace(prog);
> + *link = bpf_program__attach_trace(prog);
> + return libbpf_get_error(*link);
> }
>
> -static struct bpf_link *attach_lsm(const struct bpf_program *prog, long cookie)
> +static int attach_lsm(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> {
> - return bpf_program__attach_lsm(prog);
> + *link = bpf_program__attach_lsm(prog);
> + return libbpf_get_error(*link);
> }
>
> static struct bpf_link *
> @@ -10646,17 +10646,31 @@ bpf_program__attach_iter(const struct bpf_program *prog,
> return link;
> }
>
> -static struct bpf_link *attach_iter(const struct bpf_program *prog, long cookie)
> +static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> {
> - return bpf_program__attach_iter(prog, NULL);
> + *link = bpf_program__attach_iter(prog, NULL);
> + return libbpf_get_error(*link);
> }
>
> struct bpf_link *bpf_program__attach(const struct bpf_program *prog)
> {
> + struct bpf_link *link;
might be no harm to initialize link to NULL; we could imagine
a user-supplied auto-attach function bailing early and not
remembering to set it explicitly.
> + int err;
> +
> if (!prog->sec_def || !prog->sec_def->attach_fn)
> - return libbpf_err_ptr(-ESRCH);
> + return libbpf_err_ptr(-EOPNOTSUPP);
> +
> + err = prog->sec_def->attach_fn(prog, prog->sec_def->cookie, &link);
> + if (err)
> + return libbpf_err_ptr(err);
> +
> + /* auto-attach support is optional (see also comment in
> + * bpf_object__attach_skeleton()), but when explicitly expected by
> + * user it's an error if it's not */
nit: checkpatch wants the closing "*/" on the next line.
Also I think it would be good to clarify along the lines
of "when calling bpf_program__attach() explicitly, auto-attach
support is expected to work, and a NULL link is considered as
an error. See comment in bpf_object__attach_skeleton() which
describes different handling of the 0 return value/NULL link
there."
> + if (!link)
> + return libbpf_err_ptr(-EOPNOTSUPP);
>
> - return prog->sec_def->attach_fn(prog, prog->sec_def->cookie);
> + return link;
> }
>
> static int bpf_link__detach_struct_ops(struct bpf_link *link)
> @@ -11800,13 +11814,23 @@ int bpf_object__attach_skeleton(struct bpf_object_skeleton *s)
> if (!prog->sec_def || !prog->sec_def->attach_fn)
> continue;
>
> - *link = bpf_program__attach(prog);
> - err = libbpf_get_error(*link);
> + err = prog->sec_def->attach_fn(prog, prog->sec_def->cookie, link);
> if (err) {
> - pr_warn("failed to auto-attach program '%s': %d\n",
> + pr_warn("prog '%s': failed to auto-attach: %d\n",
> bpf_program__name(prog), err);
> return libbpf_err(err);
> }
> +
> + /* It's possible that for some SEC() definitions auto-attach
> + * is supported in some cases (e.g., if definition completely
> + * specifies target information), but is not in other cases.
> + * SEC("uprobe") is one such case. If user specified target
> + * binary and function name, such BPF program can be
> + * auto-attached. But if not, it shouldn't trigger skeleton's
> + * attach to fail. It should just be skipped.
> + * attach_fn signals such case with returning 0 (no error) and
> + * setting link to NULL.
> + */
> }
>
> return 0;
> --
> 2.30.2
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/3] libbpf: support custom SEC() handlers
2022-02-05 1:27 ` [PATCH bpf-next 2/3] libbpf: support custom SEC() handlers Andrii Nakryiko
@ 2022-02-07 13:56 ` Alan Maguire
2022-02-07 21:41 ` Andrii Nakryiko
0 siblings, 1 reply; 12+ messages in thread
From: Alan Maguire @ 2022-02-07 13:56 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, kernel-team, Alan Maguire
On Sat, 5 Feb 2022, Andrii Nakryiko wrote:
> Allow registering and unregistering custom handlers for BPF program.
> This allows user applications and libraries to plug into libbpf's
> declarative SEC() definition handling logic. This allows to offload
> complex and intricate custom logic into external libraries, but still
> provide a great user experience.
>
> One such example is USDT handling library, which has a lot of code and
> complexity which doesn't make sense to put into libbpf directly, but it
> would be really great for users to be able to specify BPF programs with
> something like SEC("usdt/<path-to-binary>:<usdt_provider>:<usdt_name>")
> and have correct BPF program type set (BPF_PROGRAM_TYPE_KPROBE, as it is
> uprobe) and even support BPF skeleton's auto-attach logic.
>
> In some cases, it might be even good idea to override libbpf's default
> handling, like for SEC("perf_event") programs. With custom library, it's
> possible to extend logic to support specifying perf event specification
> right there in SEC() definition without burdening libbpf with lots of
> custom logic or extra library dependecies (e.g., libpfm4). With current
> patch it's possible to override libbpf's SEC("perf_event") handling and
> specify a completely custom ones.
>
> Further, it's possible to specify a generic fallback handling for any
> SEC() that doesn't match any other custom or standard libbpf handlers.
> This allows to accommodate whatever legacy use cases there might be, if
> necessary.
>
> See doc comments for libbpf_register_prog_handler() and
> libbpf_unregister_prog_handler() for detailed semantics.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> tools/lib/bpf/libbpf.c | 201 +++++++++++++++++++++++++++++----------
> tools/lib/bpf/libbpf.h | 81 ++++++++++++++++
> tools/lib/bpf/libbpf.map | 2 +
> 3 files changed, 232 insertions(+), 52 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 2902534def2c..d78a6365ba74 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -209,13 +209,6 @@ struct reloc_desc {
> };
> };
>
> -typedef int (*libbpf_prog_init_fn_t)(struct bpf_program *prog, long cookie);
> -typedef int (*libbpf_prog_preload_fn_t)(struct bpf_program *prog,
> - struct bpf_prog_load_opts *opts, long cookie);
> -/* If auto-attach is not supported, callback should return 0 and set link to NULL */
> -typedef int (*libbpf_prog_attach_fn_t)(const struct bpf_program *prog, long cookie,
> - struct bpf_link **link);
> -
> /* stored as sec_def->cookie for all libbpf-supported SEC()s */
> enum sec_def_flags {
> SEC_NONE = 0,
> @@ -243,10 +236,11 @@ enum sec_def_flags {
> };
>
> struct bpf_sec_def {
> - const char *sec;
> + char *sec;
> enum bpf_prog_type prog_type;
> enum bpf_attach_type expected_attach_type;
> long cookie;
> + int handler_id;
>
> libbpf_prog_init_fn_t init_fn;
> libbpf_prog_preload_fn_t preload_fn;
> @@ -8582,7 +8576,7 @@ int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log
> }
>
> #define SEC_DEF(sec_pfx, ptype, atype, flags, ...) { \
> - .sec = sec_pfx, \
> + .sec = (char *)sec_pfx, \
> .prog_type = BPF_PROG_TYPE_##ptype, \
> .expected_attach_type = atype, \
> .cookie = (long)(flags), \
> @@ -8675,61 +8669,164 @@ static const struct bpf_sec_def section_defs[] = {
> SEC_DEF("sk_lookup", SK_LOOKUP, BPF_SK_LOOKUP, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
> };
>
> -#define MAX_TYPE_NAME_SIZE 32
> +static size_t custom_sec_def_cnt;
> +static struct bpf_sec_def *custom_sec_defs;
> +static struct bpf_sec_def custom_fallback_def;
> +static bool has_custom_fallback_def;
>
> -static const struct bpf_sec_def *find_sec_def(const char *sec_name)
> +static int last_custom_sec_def_handler_id;
> +
> +int libbpf_register_prog_handler(const char *sec,
> + enum bpf_prog_type prog_type,
> + enum bpf_attach_type exp_attach_type,
> + libbpf_prog_init_fn_t prog_init_fn,
> + libbpf_prog_preload_fn_t prog_preload_fn,
> + libbpf_prog_attach_fn_t prog_attach_fn,
> + long cookie,
> + const void *opts)
> {
> - const struct bpf_sec_def *sec_def;
> - enum sec_def_flags sec_flags;
> - int i, n = ARRAY_SIZE(section_defs), len;
> - bool strict = libbpf_mode & LIBBPF_STRICT_SEC_NAME;
> + struct bpf_sec_def *sec_def;
>
> - for (i = 0; i < n; i++) {
> - sec_def = §ion_defs[i];
> - sec_flags = sec_def->cookie;
> - len = strlen(sec_def->sec);
> + if (opts)
> + return libbpf_err(-EINVAL);
> + if (last_custom_sec_def_handler_id == INT_MAX) /* prevent overflow */
> + return libbpf_err(-E2BIG);
>
> - /* "type/" always has to have proper SEC("type/extras") form */
> - if (sec_def->sec[len - 1] == '/') {
> - if (str_has_pfx(sec_name, sec_def->sec))
> - return sec_def;
> - continue;
> - }
> + if (sec) {
> + sec_def = libbpf_reallocarray(custom_sec_defs, custom_sec_def_cnt + 1,
> + sizeof(*sec_def));
> + if (!sec_def)
> + return libbpf_err(-ENOMEM);
>
> - /* "type+" means it can be either exact SEC("type") or
> - * well-formed SEC("type/extras") with proper '/' separator
> - */
> - if (sec_def->sec[len - 1] == '+') {
> - len--;
> - /* not even a prefix */
> - if (strncmp(sec_name, sec_def->sec, len) != 0)
> - continue;
> - /* exact match or has '/' separator */
> - if (sec_name[len] == '\0' || sec_name[len] == '/')
> - return sec_def;
> - continue;
> - }
> + custom_sec_defs = sec_def;
> + sec_def = &custom_sec_defs[custom_sec_def_cnt];
> + } else {
> + if (has_custom_fallback_def)
> + return libbpf_err(-EBUSY);
>
> - /* SEC_SLOPPY_PFX definitions are allowed to be just prefix
> - * matches, unless strict section name mode
> - * (LIBBPF_STRICT_SEC_NAME) is enabled, in which case the
> - * match has to be exact.
> - */
> - if ((sec_flags & SEC_SLOPPY_PFX) && !strict) {
> - if (str_has_pfx(sec_name, sec_def->sec))
> - return sec_def;
> - continue;
> - }
> + sec_def = &custom_fallback_def;
> + }
>
> - /* Definitions not marked SEC_SLOPPY_PFX (e.g.,
> - * SEC("syscall")) are exact matches in both modes.
> - */
> - if (strcmp(sec_name, sec_def->sec) == 0)
> + sec_def->sec = sec ? strdup(sec) : NULL;
> + if (sec && !sec_def->sec)
> + return libbpf_err(-ENOMEM);
> +
> + sec_def->prog_type = prog_type;
> + sec_def->expected_attach_type = exp_attach_type;
> + sec_def->cookie = cookie;
> +
> + sec_def->init_fn = prog_init_fn;
> + sec_def->preload_fn = prog_preload_fn;
> + sec_def->attach_fn = prog_attach_fn;
> +
> + sec_def->handler_id = ++last_custom_sec_def_handler_id;
> +
> + if (sec)
> + custom_sec_def_cnt++;
> + else
> + has_custom_fallback_def = true;
> +
should we try and deal with the (unlikely) case that multiple
fallback definitions are supplied, since only the first will
be used? i.e
if (!sec && has_custom_fallback_def)
return -EEXIST;
?
> + return sec_def->handler_id;
> +}
> +
> +int libbpf_unregister_prog_handler(int handler_id)
> +{
> + int i;
> +
> + if (handler_id <= 0)
> + return libbpf_err(-EINVAL);
> +
> + if (has_custom_fallback_def && custom_fallback_def.handler_id == handler_id) {
> + memset(&custom_fallback_def, 0, sizeof(custom_fallback_def));
> + has_custom_fallback_def = false;
> + return 0;
> + }
> +
> + for (i = 0; i < custom_sec_def_cnt; i++) {
> + if (custom_sec_defs[i].handler_id == handler_id)
> + break;
> + }
> +
> + if (i == custom_sec_def_cnt)
> + return libbpf_err(-ENOENT);
> +
> + free(custom_sec_defs[i].sec);
> + for (i = i + 1; i < custom_sec_def_cnt; i++)
> + custom_sec_defs[i - 1] = custom_sec_defs[i];
> + custom_sec_def_cnt--;
We're leaking a custom table entry each time we register/deregister.
We could libbpf_reallocarray() to trim here I think.
> +
> + return 0;
> +}
> +
> +static bool sec_def_matches(const struct bpf_sec_def *sec_def, const char *sec_name,
> + bool allow_sloppy)
> +{
> + size_t len = strlen(sec_def->sec);
> +
> + /* "type/" always has to have proper SEC("type/extras") form */
> + if (sec_def->sec[len - 1] == '/') {
> + if (str_has_pfx(sec_name, sec_def->sec))
> + return true;
> + return false;
> + }
> +
> + /* "type+" means it can be either exact SEC("type") or
> + * well-formed SEC("type/extras") with proper '/' separator
> + */
> + if (sec_def->sec[len - 1] == '+') {
> + len--;
> + /* not even a prefix */
> + if (strncmp(sec_name, sec_def->sec, len) != 0)
> + return false;
> + /* exact match or has '/' separator */
> + if (sec_name[len] == '\0' || sec_name[len] == '/')
> + return true;
> + return false;
> + }
> +
> + /* SEC_SLOPPY_PFX definitions are allowed to be just prefix
> + * matches, unless strict section name mode
> + * (LIBBPF_STRICT_SEC_NAME) is enabled, in which case the
> + * match has to be exact.
> + */
> + if (allow_sloppy && str_has_pfx(sec_name, sec_def->sec))
> + return true;
> +
> + /* Definitions not marked SEC_SLOPPY_PFX (e.g.,
> + * SEC("syscall")) are exact matches in both modes.
> + */
> + return strcmp(sec_name, sec_def->sec) == 0;
> +}
> +
> +static const struct bpf_sec_def *find_sec_def(const char *sec_name)
> +{
> + const struct bpf_sec_def *sec_def;
> + int i, n;
> + bool strict = libbpf_mode & LIBBPF_STRICT_SEC_NAME, allow_sloppy;
> +
> + n = custom_sec_def_cnt;
> + for (i = 0; i < n; i++) {
> + sec_def = &custom_sec_defs[i];
> + if (sec_def_matches(sec_def, sec_name, false))
> return sec_def;
> }
> +
> + n = ARRAY_SIZE(section_defs);
> + for (i = 0; i < n; i++) {
> + sec_def = §ion_defs[i];
> + allow_sloppy = (sec_def->cookie & SEC_SLOPPY_PFX) && !strict;
> + if (sec_def_matches(sec_def, sec_name, allow_sloppy))
> + return sec_def;
> + }
> +
> + if (has_custom_fallback_def)
> + return &custom_fallback_def;
> +
> return NULL;
> }
>
> +#define MAX_TYPE_NAME_SIZE 32
> +
> static char *libbpf_get_type_names(bool attach_type)
> {
> int i, len = ARRAY_SIZE(section_defs) * MAX_TYPE_NAME_SIZE;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index c8d8daad212e..6e665c26dcc7 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -1328,6 +1328,87 @@ LIBBPF_API int bpf_linker__add_file(struct bpf_linker *linker,
> LIBBPF_API int bpf_linker__finalize(struct bpf_linker *linker);
> LIBBPF_API void bpf_linker__free(struct bpf_linker *linker);
>
> +/*
> + * Custom handling of BPF program's SEC() definitions
> + */
> +
> +struct bpf_prog_load_opts; /* defined in bpf.h */
> +
> +/* Called during bpf_object__open() for each recognized BPF program. Callback
> + * can use various bpf_program__set_*() setters to adjust whatever properties
> + * are necessary.
> + */
> +typedef int (*libbpf_prog_init_fn_t)(struct bpf_program *prog, long cookie);
> +
> +/* Called right before libbpf performs bpf_prog_load() to load BPF program
> + * into the kernel. Callback can adjust opts as necessary.
> + */
> +typedef int (*libbpf_prog_preload_fn_t)(struct bpf_program *prog,
> + struct bpf_prog_load_opts *opts, long cookie);
> +
> +/* Called during skeleton attach or through bpf_program__attach(). If
> + * auto-attach is not supported, callback should return 0 and set link to
> + * NULL (it's not considered an error during skeleton attach, but it will be
> + * an error for bpf_program__attach() calls). On error, error should be
> + * returned directly and link set to NULL. On success, return 0 and set link
> + * to a valid struct bpf_link.
> + */
> +typedef int (*libbpf_prog_attach_fn_t)(const struct bpf_program *prog, long cookie,
> + struct bpf_link **link);
> +
> +/**
> + * @brief **libbpf_register_prog_handler()** registers a custom BPF program
> + * SEC() handler.
> + * @param sec section prefix for which custom handler is registered
> + * @param prog_type BPF program type associated with specified section
> + * @param exp_attach_type Expected BPF attach type associated with specified section
> + * @param prog_init_fn BPF program initialization callback (see libbpf_prog_init_fn_t)
> + * @param prog_preload_fn BPF program loading callback (see libbpf_prog_preload_fn_t)
> + * @param prog_attach_fn BPF program attach callback (see libbpf_prog_attach_fn_t)
> + * @param cookie User-provided cookie passed to each callback
> + * @param opts reserved for future extensibility, should be NULL
> + * @return Non-negative handler ID is returned on success. This handler ID has
> + * to be passed to *libbpf_unregister_prog_handler()* to unregister such
> + * custom handler. Negative error code is returned on error.
> + *
> + * *sec* defines which SEC() definitions are handled by this custom handler
> + * registration. *sec* can have few different forms:
> + * - if *sec* is just a plain string (e.g., "abc"), it will match only
> + * SEC("abc"). If BPF program specifies SEC("abc/whatever") it will result
> + * in an error;
> + * - if *sec* is of the form "abc/", proper SEC() form is
> + * SEC("abc/something"), where acceptable "something" should be checked by
> + * *prog_init_fn* callback, if there are additional restrictions;
> + * - if *sec* is of the form "abc+", it will successfully match both
> + * SEC("abc") and SEC("abc/whatever") forms;
> + * - if *sec* is NULL, custom handler is registered for any BPF program that
> + * doesn't match any of the registered (custom or libbpf's own) SEC()
> + * handlers. There could be only one such generic custom handler registered
> + * at any given time.
> + *
> + * All custom handlers (except the one with *sec* == NULL) are processed
> + * before libbpf's own SEC() handlers. It is allowed to "override" libbpf's
> + * SEC() handlers by registering custom ones for the same section prefix
> + * (i.e., it's possible to have custom SEC("perf_event/LLC-load-misses")
> + * handler).
> + */
Nicely documented!
> +LIBBPF_API int libbpf_register_prog_handler(const char *sec,
> + enum bpf_prog_type prog_type,
> + enum bpf_attach_type exp_attach_type,
> + libbpf_prog_init_fn_t prog_init_fn,
> + libbpf_prog_preload_fn_t prog_preload_fn,
> + libbpf_prog_attach_fn_t prog_attach_fn,
Naming nit: a prog_handler sounds less specific; would
"sec_handler" or "prog_sec_handler" be more descriptive perhaps?
Also, would it make sense to pass the functions in as options instead?
They can all be NULL potentially I think, and it's possible we'd
want additional future handlers too..
> + long cookie,
> + const void *opts);
> +/**
> + * @brief *libbpf_unregister_prog_handler()* unregisters previously registered
> + * custom BPF program SEC() handler.
> + * @param handler_id handler ID returned by *libbpf_register_prog_handler()*
> + * after successful registration
> + * @return 0 on success, negative error code if handler isn't found
> + */
> +LIBBPF_API int libbpf_unregister_prog_handler(int handler_id);
> +
> #ifdef __cplusplus
> } /* extern "C" */
> #endif
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index aef6253a90c8..4e75f06c1a00 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -438,4 +438,6 @@ LIBBPF_0.7.0 {
> libbpf_probe_bpf_map_type;
> libbpf_probe_bpf_prog_type;
> libbpf_set_memlock_rlim_max;
> + libbpf_register_prog_handler;
> + libbpf_unregister_prog_handler;
> };
> --
> 2.30.2
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-05 1:27 ` [PATCH bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest Andrii Nakryiko
@ 2022-02-07 14:20 ` Alan Maguire
2022-02-07 21:43 ` Andrii Nakryiko
0 siblings, 1 reply; 12+ messages in thread
From: Alan Maguire @ 2022-02-07 14:20 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, kernel-team, Alan Maguire
On Sat, 5 Feb 2022, Andrii Nakryiko wrote:
> Add a selftest validating various aspects of libbpf's handling of custom
> SEC() handlers. It also demonstrates how libraries can ensure very early
> callbacks registration and unregistration using
> __attribute__((constructor))/__attribute__((destructor)) functions.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
A few suggestions here for additional tests, but
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Should we override a default attach method to demonstrate that
custom handlers can do that? Or would that break parallel
testing mode?
Also might be good to have a test that captured the difference
in auto-attach behaviour between a skeleton attach and an
explicit bpf_prog__attach(); running the bpf_prog__attach on the
SEC("xyz") should result in -EOPNOTSUPP.
> ---
> .../bpf/prog_tests/custom_sec_handlers.c | 136 ++++++++++++++++++
> .../bpf/progs/test_custom_sec_handlers.c | 51 +++++++
> 2 files changed, 187 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
> new file mode 100644
> index 000000000000..8e43c5f21878
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
> @@ -0,0 +1,136 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2022 Facebook */
> +
> +#include <test_progs.h>
> +#include "test_custom_sec_handlers.skel.h"
> +
> +#define COOKIE_ABC1 1
> +#define COOKIE_ABC2 2
> +#define COOKIE_CUSTOM 3
> +#define COOKIE_FALLBACK 4
> +
> +static int custom_init_prog(struct bpf_program *prog, long cookie)
> +{
> + if (cookie == COOKIE_ABC1)
> + bpf_program__set_autoload(prog, false);
> +
> + return 0;
> +}
> +
> +static int custom_preload_prog(struct bpf_program *prog,
> + struct bpf_prog_load_opts *opts, long cookie)
> +{
> + return 0;
> +}
> +
> +static int custom_attach_prog(const struct bpf_program *prog, long cookie,
> + struct bpf_link **link)
> +{
> + switch (cookie) {
> + case COOKIE_ABC2:
> + *link = bpf_program__attach_raw_tracepoint(prog, "sys_enter");
> + return libbpf_get_error(*link);
> + case COOKIE_CUSTOM:
> + *link = bpf_program__attach_tracepoint(prog, "syscalls", "sys_enter_nanosleep");
> + return libbpf_get_error(*link);
> + case COOKIE_FALLBACK:
> + /* no auto-attach for SEC("xyz") */
> + *link = NULL;
> + return 0;
> + default:
> + ASSERT_FALSE(true, "unexpected cookie");
> + return -EINVAL;
> + }
> +}
> +
> +static int abc1_id;
> +static int abc2_id;
> +static int custom_id;
> +static int fallback_id;
> +
> +__attribute__((constructor))
> +static void register_sec_handlers(void)
> +{
> + abc1_id = libbpf_register_prog_handler("abc",
> + BPF_PROG_TYPE_RAW_TRACEPOINT, 0,
> + custom_init_prog, custom_preload_prog,
> + custom_attach_prog,
> + COOKIE_ABC1, NULL);
> + abc2_id = libbpf_register_prog_handler("abc/",
> + BPF_PROG_TYPE_RAW_TRACEPOINT, 0,
> + custom_init_prog, custom_preload_prog,
> + custom_attach_prog,
> + COOKIE_ABC2, NULL);
> + custom_id = libbpf_register_prog_handler("custom+",
> + BPF_PROG_TYPE_TRACEPOINT, 0,
> + custom_init_prog, custom_preload_prog,
> + custom_attach_prog,
> + COOKIE_CUSTOM, NULL);
> +}
> +
> +__attribute__((destructor))
> +static void unregister_sec_handlers(void)
> +{
> + libbpf_unregister_prog_handler(abc1_id);
> + libbpf_unregister_prog_handler(abc2_id);
> + libbpf_unregister_prog_handler(custom_id);
> +}
> +
> +void test_custom_sec_handlers(void)
> +{
> + struct test_custom_sec_handlers* skel;
> + int err;
> +
> + ASSERT_GT(abc1_id, 0, "abc1_id");
> + ASSERT_GT(abc2_id, 0, "abc2_id");
> + ASSERT_GT(custom_id, 0, "custom_id");
> +
> + fallback_id = libbpf_register_prog_handler(NULL, /* fallback handler */
> + BPF_PROG_TYPE_KPROBE, 0,
> + custom_init_prog, custom_preload_prog,
> + custom_attach_prog,
> + COOKIE_FALLBACK, NULL);
> + if (!ASSERT_GT(fallback_id, 0, "fallback_id"))
> + return;
> +
> + /* open skeleton and validate assumptions */
> + skel = test_custom_sec_handlers__open();
> + if (!ASSERT_OK_PTR(skel, "skel_open"))
> + goto cleanup;
> +
> + ASSERT_EQ(bpf_program__type(skel->progs.abc1), BPF_PROG_TYPE_RAW_TRACEPOINT, "abc1_type");
> + ASSERT_FALSE(bpf_program__autoload(skel->progs.abc1), "abc1_autoload");
> +
> + ASSERT_EQ(bpf_program__type(skel->progs.abc2), BPF_PROG_TYPE_RAW_TRACEPOINT, "abc2_type");
> + ASSERT_EQ(bpf_program__type(skel->progs.custom1), BPF_PROG_TYPE_TRACEPOINT, "custom1_type");
> + ASSERT_EQ(bpf_program__type(skel->progs.custom2), BPF_PROG_TYPE_TRACEPOINT, "custom2_type");
> + ASSERT_EQ(bpf_program__type(skel->progs.xyz), BPF_PROG_TYPE_KPROBE, "xyz_type");
> +
> + skel->rodata->my_pid = getpid();
> +
> + /* now attempt to load everything */
> + err = test_custom_sec_handlers__load(skel);
> + if (!ASSERT_OK(err, "skel_load"))
> + goto cleanup;
> +
> + /* now try to auto-attach everything */
> + err = test_custom_sec_handlers__attach(skel);
> + if (!ASSERT_OK(err, "skel_attach"))
> + goto cleanup;
> +
> + /* trigger programs */
> + usleep(1);
> +
> + /* SEC("abc") is set to not auto-loaded */
> + ASSERT_FALSE(skel->bss->abc1_called, "abc1_called");
> + ASSERT_TRUE(skel->bss->abc2_called, "abc2_called");
> + ASSERT_TRUE(skel->bss->custom1_called, "custom1_called");
> + ASSERT_TRUE(skel->bss->custom2_called, "custom2_called");
> + /* SEC("xyz") shouldn't be auto-attached */
> + ASSERT_FALSE(skel->bss->xyz_called, "xyz_called");
> +
> +cleanup:
> + test_custom_sec_handlers__destroy(skel);
> +
> + ASSERT_OK(libbpf_unregister_prog_handler(fallback_id), "unregister_fallback");
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c b/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
> new file mode 100644
> index 000000000000..2df368783678
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2022 Facebook */
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +const volatile int my_pid;
> +
> +bool abc1_called;
> +bool abc2_called;
> +bool custom1_called;
> +bool custom2_called;
> +bool xyz_called;
> +
> +SEC("abc")
> +int abc1(void *ctx)
> +{
> + abc1_called = true;
> + return 0;
> +}
> +
> +SEC("abc/whatever")
> +int abc2(void *ctx)
> +{
> + abc2_called = true;
> + return 0;
> +}
> +
> +SEC("custom")
> +int custom1(void *ctx)
> +{
> + custom1_called = true;
> + return 0;
> +}
> +
> +SEC("custom/something")
> +int custom2(void *ctx)
> +{
> + custom2_called = true;
> + return 0;
> +}
> +
> +SEC("xyz/blah")
> +int xyz(void *ctx)
> +{
> + xyz_called = true;
> + return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.30.2
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 1/3] libbpf: allow BPF program auto-attach handlers to bail out
2022-02-07 12:16 ` Alan Maguire
@ 2022-02-07 21:32 ` Andrii Nakryiko
0 siblings, 0 replies; 12+ messages in thread
From: Andrii Nakryiko @ 2022-02-07 21:32 UTC (permalink / raw)
To: Alan Maguire
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
On Mon, Feb 7, 2022 at 4:17 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On Sat, 5 Feb 2022, Andrii Nakryiko wrote:
>
> > Allow some BPF program types to support auto-attach only in subste of
> > cases. Currently, if some BPF program type specifies attach callback, it
> > is assumed that during skeleton attach operation all such programs
> > either successfully attach or entire skeleton attachment fails. If some
> > program doesn't support auto-attachment from skeleton, such BPF program
> > types shouldn't have attach callback specified.
> >
>
> This is a great feature! I've had cases where I had to
> implement custom section-specific handling before, so this
> will make that process much easier!
>
> > This is limiting for cases when, depending on how full the SEC("")
> > definition is, there could either be enough details to support
> > auto-attach or there might not be and user has to use some specific API
> > to provide more details at runtime.
> >
> > One specific example of such desired behavior might be SEC("uprobe"). If
> > it's specified as just uprobe auto-attach isn't possible. But if it's
> > SEC("uprobe/<some_binary>:<some_func>") then there are enough details to
> > support auto-attach.
>
> Would be good to describe the different handling for explicit
> bpf_program__attach() (which fails when auto-attach is supported
> but does not return a non-NULL link) vs bpf_object__attach_skeleton()
> (which skips the NULL link case) here I think; it's all clarified in
> comments below but no harm to reiterate at the top-level I think.
Ok, I'll mention the difference here as well.
>
> >
> > Another improvement to the way libbpf is handling SEC()s would be to not
> > require providing dummy kernel function name for kprobe. Currently,
> > SEC("kprobe/whatever") is necessary even if actual kernel function is
> > determined by user at runtime and bpf_program__attach_kprobe() is used
> > to specify it. With changes in this patch, it's possible to support both
> > SEC("kprobe") and SEC("kprobe/<actual_kernel_function"), while only in
> > the latter case auto-attach will be performed. In the former one, such
> > kprobe will be skipped during skeleton attach operation.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
> A few nits and suggestions for future, but this looks great!
Thanks for review!
>
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
>
> > ---
> > tools/lib/bpf/libbpf.c | 110 +++++++++++++++++++++++++----------------
> > 1 file changed, 67 insertions(+), 43 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 904cdf83002b..2902534def2c 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -209,11 +209,12 @@ struct reloc_desc {
> > };
> > };
> >
> > -struct bpf_sec_def;
> > -
> > -typedef int (*init_fn_t)(struct bpf_program *prog, long cookie);
> > -typedef int (*preload_fn_t)(struct bpf_program *prog, struct bpf_prog_load_opts *opts, long cookie);
> > -typedef struct bpf_link *(*attach_fn_t)(const struct bpf_program *prog, long cookie);
> > +typedef int (*libbpf_prog_init_fn_t)(struct bpf_program *prog, long cookie);
> > +typedef int (*libbpf_prog_preload_fn_t)(struct bpf_program *prog,
> > + struct bpf_prog_load_opts *opts, long cookie);
> > +/* If auto-attach is not supported, callback should return 0 and set link to NULL */
> > +typedef int (*libbpf_prog_attach_fn_t)(const struct bpf_program *prog, long cookie,
> > + struct bpf_link **link);
> >
> > /* stored as sec_def->cookie for all libbpf-supported SEC()s */
> > enum sec_def_flags {
> > @@ -247,9 +248,9 @@ struct bpf_sec_def {
> > enum bpf_attach_type expected_attach_type;
> > long cookie;
> >
> > - init_fn_t init_fn;
> > - preload_fn_t preload_fn;
> > - attach_fn_t attach_fn;
> > + libbpf_prog_init_fn_t init_fn;
> > + libbpf_prog_preload_fn_t preload_fn;
> > + libbpf_prog_attach_fn_t attach_fn;
> > };
> >
> > /*
> > @@ -8589,12 +8590,12 @@ int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log
> > __VA_ARGS__ \
> > }
> >
> > -static struct bpf_link *attach_kprobe(const struct bpf_program *prog, long cookie);
> > -static struct bpf_link *attach_tp(const struct bpf_program *prog, long cookie);
> > -static struct bpf_link *attach_raw_tp(const struct bpf_program *prog, long cookie);
> > -static struct bpf_link *attach_trace(const struct bpf_program *prog, long cookie);
> > -static struct bpf_link *attach_lsm(const struct bpf_program *prog, long cookie);
> > -static struct bpf_link *attach_iter(const struct bpf_program *prog, long cookie);
> > +static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> > +static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> > +static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> > +static int attach_trace(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> > +static int attach_lsm(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> > +static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> >
>
> One thought here - in the future it might be useful to export
> these internal auto-attach functions. The reason I suggest this
> is some use-cases of auto-attach might involve pre-processing of
> the section name, and once the required info is extracted the
> auto-attach function could use the original auto-attach functionality.
> That could be done separately to what you're doing here of course.
>
> One concrete example of this: I had a BPF program which consisted
> of BPF programs containing required attachments to top-level protocol
> module functions along with a set of optional attachments to
> transport-specific module functions. Since multiple transports were
> possible, it was always possible that module A wouldn't be loaded
> while module B was, or vice versa. To deal with this, I used the
> "o" prefix (optional) for the associated kprobe/kretprobe section
> definitions; an "okprobe" might not attach, but a "kprobe" should.
>
> Using the mechanisms in this patch set, this could be easily
> implemented by a custom auto-attach which looked for the "o"
> then passed the rest of the section string into the default
> auto-attach function for kprobes, handling attach errors for
> optional sections while passing them through for required ones.
> Tracers might find such pre-processing combined with the default
> mechanisms useful too; we could even potentially implement
> support for ":" separators this way too (convert instances
> of ":" to "/" and then call default auto-attach)!
I'd really like to avoid exposing internal libbpf code as much as
possible. If someone has some complicated use case, they should be
ready to redo some of libbpf parsing, if necessary. Gutting libbpf
internals more than necessary for the convenience of few advanced
users isn't the right trade off from my POV. In the end, there are all
the bpf_program__attach_xxx() APIs, so it's mostly about parsing
"kprobe/<func>", which usually is not big of a deal.
>
> > static const struct bpf_sec_def section_defs[] = {
> > SEC_DEF("socket", SOCKET_FILTER, 0, SEC_NONE | SEC_SLOPPY_PFX),
> > @@ -10101,14 +10102,13 @@ struct bpf_link *bpf_program__attach_kprobe(const struct bpf_program *prog,
> > return bpf_program__attach_kprobe_opts(prog, func_name, &opts);
> > }
> >
> > -static struct bpf_link *attach_kprobe(const struct bpf_program *prog, long cookie)
> > +static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> > {
> > DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts);
> > unsigned long offset = 0;
> > - struct bpf_link *link;
> > const char *func_name;
> > char *func;
> > - int n, err;
> > + int n;
> >
> > opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe/");
> > if (opts.retprobe)
[...]
> > @@ -10646,17 +10646,31 @@ bpf_program__attach_iter(const struct bpf_program *prog,
> > return link;
> > }
> >
> > -static struct bpf_link *attach_iter(const struct bpf_program *prog, long cookie)
> > +static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> > {
> > - return bpf_program__attach_iter(prog, NULL);
> > + *link = bpf_program__attach_iter(prog, NULL);
> > + return libbpf_get_error(*link);
> > }
> >
> > struct bpf_link *bpf_program__attach(const struct bpf_program *prog)
> > {
> > + struct bpf_link *link;
>
> might be no harm to initialize link to NULL; we could imagine
> a user-supplied auto-attach function bailing early and not
> remembering to set it explicitly.
Sure, will do in v2.
>
> > + int err;
> > +
> > if (!prog->sec_def || !prog->sec_def->attach_fn)
> > - return libbpf_err_ptr(-ESRCH);
> > + return libbpf_err_ptr(-EOPNOTSUPP);
> > +
> > + err = prog->sec_def->attach_fn(prog, prog->sec_def->cookie, &link);
> > + if (err)
> > + return libbpf_err_ptr(err);
> > +
> > + /* auto-attach support is optional (see also comment in
> > + * bpf_object__attach_skeleton()), but when explicitly expected by
> > + * user it's an error if it's not */
>
> nit: checkpatch wants the closing "*/" on the next line.
yep, missed during reformatting
> Also I think it would be good to clarify along the lines
> of "when calling bpf_program__attach() explicitly, auto-attach
> support is expected to work, and a NULL link is considered as
> an error. See comment in bpf_object__attach_skeleton() which
> describes different handling of the 0 return value/NULL link
> there."
Ok, I'll adjust the comment.
>
> > + if (!link)
> > + return libbpf_err_ptr(-EOPNOTSUPP);
> >
> > - return prog->sec_def->attach_fn(prog, prog->sec_def->cookie);
> > + return link;
> > }
> >
> > static int bpf_link__detach_struct_ops(struct bpf_link *link)
> > @@ -11800,13 +11814,23 @@ int bpf_object__attach_skeleton(struct bpf_object_skeleton *s)
> > if (!prog->sec_def || !prog->sec_def->attach_fn)
> > continue;
> >
> > - *link = bpf_program__attach(prog);
> > - err = libbpf_get_error(*link);
> > + err = prog->sec_def->attach_fn(prog, prog->sec_def->cookie, link);
> > if (err) {
> > - pr_warn("failed to auto-attach program '%s': %d\n",
> > + pr_warn("prog '%s': failed to auto-attach: %d\n",
> > bpf_program__name(prog), err);
> > return libbpf_err(err);
> > }
> > +
> > + /* It's possible that for some SEC() definitions auto-attach
> > + * is supported in some cases (e.g., if definition completely
> > + * specifies target information), but is not in other cases.
> > + * SEC("uprobe") is one such case. If user specified target
> > + * binary and function name, such BPF program can be
> > + * auto-attached. But if not, it shouldn't trigger skeleton's
> > + * attach to fail. It should just be skipped.
> > + * attach_fn signals such case with returning 0 (no error) and
> > + * setting link to NULL.
> > + */
> > }
> >
> > return 0;
> > --
> > 2.30.2
> >
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/3] libbpf: support custom SEC() handlers
2022-02-07 13:56 ` Alan Maguire
@ 2022-02-07 21:41 ` Andrii Nakryiko
0 siblings, 0 replies; 12+ messages in thread
From: Andrii Nakryiko @ 2022-02-07 21:41 UTC (permalink / raw)
To: Alan Maguire
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
On Mon, Feb 7, 2022 at 5:57 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On Sat, 5 Feb 2022, Andrii Nakryiko wrote:
>
> > Allow registering and unregistering custom handlers for BPF program.
> > This allows user applications and libraries to plug into libbpf's
> > declarative SEC() definition handling logic. This allows to offload
> > complex and intricate custom logic into external libraries, but still
> > provide a great user experience.
> >
> > One such example is USDT handling library, which has a lot of code and
> > complexity which doesn't make sense to put into libbpf directly, but it
> > would be really great for users to be able to specify BPF programs with
> > something like SEC("usdt/<path-to-binary>:<usdt_provider>:<usdt_name>")
> > and have correct BPF program type set (BPF_PROGRAM_TYPE_KPROBE, as it is
> > uprobe) and even support BPF skeleton's auto-attach logic.
> >
> > In some cases, it might be even good idea to override libbpf's default
> > handling, like for SEC("perf_event") programs. With custom library, it's
> > possible to extend logic to support specifying perf event specification
> > right there in SEC() definition without burdening libbpf with lots of
> > custom logic or extra library dependecies (e.g., libpfm4). With current
> > patch it's possible to override libbpf's SEC("perf_event") handling and
> > specify a completely custom ones.
> >
> > Further, it's possible to specify a generic fallback handling for any
> > SEC() that doesn't match any other custom or standard libbpf handlers.
> > This allows to accommodate whatever legacy use cases there might be, if
> > necessary.
> >
> > See doc comments for libbpf_register_prog_handler() and
> > libbpf_unregister_prog_handler() for detailed semantics.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> > tools/lib/bpf/libbpf.c | 201 +++++++++++++++++++++++++++++----------
> > tools/lib/bpf/libbpf.h | 81 ++++++++++++++++
> > tools/lib/bpf/libbpf.map | 2 +
> > 3 files changed, 232 insertions(+), 52 deletions(-)
> >
[...]
> > - /* "type+" means it can be either exact SEC("type") or
> > - * well-formed SEC("type/extras") with proper '/' separator
> > - */
> > - if (sec_def->sec[len - 1] == '+') {
> > - len--;
> > - /* not even a prefix */
> > - if (strncmp(sec_name, sec_def->sec, len) != 0)
> > - continue;
> > - /* exact match or has '/' separator */
> > - if (sec_name[len] == '\0' || sec_name[len] == '/')
> > - return sec_def;
> > - continue;
> > - }
> > + custom_sec_defs = sec_def;
> > + sec_def = &custom_sec_defs[custom_sec_def_cnt];
> > + } else {
> > + if (has_custom_fallback_def)
> > + return libbpf_err(-EBUSY);
this disallows two fallback handlers
> >
> > - /* SEC_SLOPPY_PFX definitions are allowed to be just prefix
> > - * matches, unless strict section name mode
> > - * (LIBBPF_STRICT_SEC_NAME) is enabled, in which case the
> > - * match has to be exact.
> > - */
> > - if ((sec_flags & SEC_SLOPPY_PFX) && !strict) {
> > - if (str_has_pfx(sec_name, sec_def->sec))
> > - return sec_def;
> > - continue;
> > - }
> > + sec_def = &custom_fallback_def;
> > + }
> >
> > - /* Definitions not marked SEC_SLOPPY_PFX (e.g.,
> > - * SEC("syscall")) are exact matches in both modes.
> > - */
> > - if (strcmp(sec_name, sec_def->sec) == 0)
> > + sec_def->sec = sec ? strdup(sec) : NULL;
> > + if (sec && !sec_def->sec)
> > + return libbpf_err(-ENOMEM);
> > +
> > + sec_def->prog_type = prog_type;
> > + sec_def->expected_attach_type = exp_attach_type;
> > + sec_def->cookie = cookie;
> > +
> > + sec_def->init_fn = prog_init_fn;
> > + sec_def->preload_fn = prog_preload_fn;
> > + sec_def->attach_fn = prog_attach_fn;
> > +
> > + sec_def->handler_id = ++last_custom_sec_def_handler_id;
> > +
> > + if (sec)
> > + custom_sec_def_cnt++;
> > + else
> > + has_custom_fallback_def = true;
> > +
>
> should we try and deal with the (unlikely) case that multiple
> fallback definitions are supplied, since only the first will
> be used? i.e
>
> if (!sec && has_custom_fallback_def)
> return -EEXIST;
>
> ?
>
I do that slightly earlier, see comment above
> > + return sec_def->handler_id;
> > +}
> > +
> > +int libbpf_unregister_prog_handler(int handler_id)
> > +{
> > + int i;
> > +
> > + if (handler_id <= 0)
> > + return libbpf_err(-EINVAL);
> > +
> > + if (has_custom_fallback_def && custom_fallback_def.handler_id == handler_id) {
> > + memset(&custom_fallback_def, 0, sizeof(custom_fallback_def));
> > + has_custom_fallback_def = false;
> > + return 0;
> > + }
> > +
> > + for (i = 0; i < custom_sec_def_cnt; i++) {
> > + if (custom_sec_defs[i].handler_id == handler_id)
> > + break;
> > + }
> > +
> > + if (i == custom_sec_def_cnt)
> > + return libbpf_err(-ENOENT);
> > +
> > + free(custom_sec_defs[i].sec);
> > + for (i = i + 1; i < custom_sec_def_cnt; i++)
> > + custom_sec_defs[i - 1] = custom_sec_defs[i];
> > + custom_sec_def_cnt--;
>
> We're leaking a custom table entry each time we register/deregister.
> We could libbpf_reallocarray() to trim here I think.
It's not leaking, we just don't downsize the array. If there are
subsequent registrations we'll just reuse those slots, so it's not a
leak. It doesn't seem likely that applications will add thousands of
custom handlers, then unregister all of them and never add any new
ones (and even in that case we are talking about a few kilobytes of
memory). So it felt unnecessary to try to trim anything.
But if you feel it's important, I can add libbpf_reallocarray() as
well, no big deal.
>
> > +
> > + return 0;
> > +}
> > +
[...]
> > +/**
> > + * @brief **libbpf_register_prog_handler()** registers a custom BPF program
> > + * SEC() handler.
> > + * @param sec section prefix for which custom handler is registered
> > + * @param prog_type BPF program type associated with specified section
> > + * @param exp_attach_type Expected BPF attach type associated with specified section
> > + * @param prog_init_fn BPF program initialization callback (see libbpf_prog_init_fn_t)
> > + * @param prog_preload_fn BPF program loading callback (see libbpf_prog_preload_fn_t)
> > + * @param prog_attach_fn BPF program attach callback (see libbpf_prog_attach_fn_t)
> > + * @param cookie User-provided cookie passed to each callback
> > + * @param opts reserved for future extensibility, should be NULL
> > + * @return Non-negative handler ID is returned on success. This handler ID has
> > + * to be passed to *libbpf_unregister_prog_handler()* to unregister such
> > + * custom handler. Negative error code is returned on error.
> > + *
> > + * *sec* defines which SEC() definitions are handled by this custom handler
> > + * registration. *sec* can have few different forms:
> > + * - if *sec* is just a plain string (e.g., "abc"), it will match only
> > + * SEC("abc"). If BPF program specifies SEC("abc/whatever") it will result
> > + * in an error;
> > + * - if *sec* is of the form "abc/", proper SEC() form is
> > + * SEC("abc/something"), where acceptable "something" should be checked by
> > + * *prog_init_fn* callback, if there are additional restrictions;
> > + * - if *sec* is of the form "abc+", it will successfully match both
> > + * SEC("abc") and SEC("abc/whatever") forms;
> > + * - if *sec* is NULL, custom handler is registered for any BPF program that
> > + * doesn't match any of the registered (custom or libbpf's own) SEC()
> > + * handlers. There could be only one such generic custom handler registered
> > + * at any given time.
> > + *
> > + * All custom handlers (except the one with *sec* == NULL) are processed
> > + * before libbpf's own SEC() handlers. It is allowed to "override" libbpf's
> > + * SEC() handlers by registering custom ones for the same section prefix
> > + * (i.e., it's possible to have custom SEC("perf_event/LLC-load-misses")
> > + * handler).
> > + */
>
> Nicely documented!
>
Thanks!
> > +LIBBPF_API int libbpf_register_prog_handler(const char *sec,
> > + enum bpf_prog_type prog_type,
> > + enum bpf_attach_type exp_attach_type,
> > + libbpf_prog_init_fn_t prog_init_fn,
> > + libbpf_prog_preload_fn_t prog_preload_fn,
> > + libbpf_prog_attach_fn_t prog_attach_fn,
>
> Naming nit: a prog_handler sounds less specific; would
> "sec_handler" or "prog_sec_handler" be more descriptive perhaps?
Well, SEC() is used not just for BPF programs (but maps and variables
as well), so just "sec_handler" isn't even accurate.
"prog_sec_handler" also felt a bit off, as we are handling not really
a section, but BPF program in some SEC() itself.
>
> Also, would it make sense to pass the functions in as options instead?
> They can all be NULL potentially I think, and it's possible we'd
> want additional future handlers too..
Good point, I'll add opts struct and move cookie and callback into it.
>
> > + long cookie,
> > + const void *opts);
> > +/**
> > + * @brief *libbpf_unregister_prog_handler()* unregisters previously registered
> > + * custom BPF program SEC() handler.
> > + * @param handler_id handler ID returned by *libbpf_register_prog_handler()*
> > + * after successful registration
> > + * @return 0 on success, negative error code if handler isn't found
> > + */
> > +LIBBPF_API int libbpf_unregister_prog_handler(int handler_id);
> > +
> > #ifdef __cplusplus
> > } /* extern "C" */
> > #endif
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index aef6253a90c8..4e75f06c1a00 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -438,4 +438,6 @@ LIBBPF_0.7.0 {
> > libbpf_probe_bpf_map_type;
> > libbpf_probe_bpf_prog_type;
> > libbpf_set_memlock_rlim_max;
> > + libbpf_register_prog_handler;
> > + libbpf_unregister_prog_handler;
> > };
> > --
> > 2.30.2
> >
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-07 14:20 ` Alan Maguire
@ 2022-02-07 21:43 ` Andrii Nakryiko
0 siblings, 0 replies; 12+ messages in thread
From: Andrii Nakryiko @ 2022-02-07 21:43 UTC (permalink / raw)
To: Alan Maguire
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
On Mon, Feb 7, 2022 at 6:21 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On Sat, 5 Feb 2022, Andrii Nakryiko wrote:
>
> > Add a selftest validating various aspects of libbpf's handling of custom
> > SEC() handlers. It also demonstrates how libraries can ensure very early
> > callbacks registration and unregistration using
> > __attribute__((constructor))/__attribute__((destructor)) functions.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
> A few suggestions here for additional tests, but
>
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
>
> Should we override a default attach method to demonstrate that
> custom handlers can do that? Or would that break parallel
> testing mode?
Yep, I should. I was a bit lazy and wanted some feedback before adding
more tests. I'll add some kprobe overload to auto-attach to sys_enter
or something like that. If I do that during the test (not in
constructor/destructor), it won't interfere with parallel tests (they
each run in a different process).
>
> Also might be good to have a test that captured the difference
> in auto-attach behaviour between a skeleton attach and an
> explicit bpf_prog__attach(); running the bpf_prog__attach on the
> SEC("xyz") should result in -EOPNOTSUPP.
Sure, can add that as well.
>
>
> > ---
> > .../bpf/prog_tests/custom_sec_handlers.c | 136 ++++++++++++++++++
> > .../bpf/progs/test_custom_sec_handlers.c | 51 +++++++
> > 2 files changed, 187 insertions(+)
> > create mode 100644 tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
> > create mode 100644 tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
> >
[...]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-02-07 21:43 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-05 1:27 [PATCH bpf-next 0/3] libbpf: support custom SEC() handlers Andrii Nakryiko
2022-02-05 1:27 ` [PATCH bpf-next 1/3] libbpf: allow BPF program auto-attach handlers to bail out Andrii Nakryiko
2022-02-07 12:16 ` Alan Maguire
2022-02-07 21:32 ` Andrii Nakryiko
2022-02-05 1:27 ` [PATCH bpf-next 1/1] selftests/bpf: add custom SEC() handling selftest Andrii Nakryiko
2022-02-05 1:33 ` Andrii Nakryiko
2022-02-05 1:27 ` [PATCH bpf-next 2/3] libbpf: support custom SEC() handlers Andrii Nakryiko
2022-02-07 13:56 ` Alan Maguire
2022-02-07 21:41 ` Andrii Nakryiko
2022-02-05 1:27 ` [PATCH bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest Andrii Nakryiko
2022-02-07 14:20 ` Alan Maguire
2022-02-07 21:43 ` Andrii Nakryiko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.