* [PATCH v3 bpf-next 0/3] libbpf: support custom SEC() handlers
@ 2022-02-11 21:14 Andrii Nakryiko
2022-02-11 21:14 ` [PATCH v3 bpf-next 1/3] libbpf: allow BPF program auto-attach handlers to bail out Andrii Nakryiko
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Andrii Nakryiko @ 2022-02-11 21:14 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.
v1->v2:
- moved callbacks and cookie into OPTS struct (Alan);
- added more test scenarios (Alan);
- address most of Alan's feedback, but kept API name.
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 | 308 +++++++++++++-----
tools/lib/bpf/libbpf.h | 87 +++++
tools/lib/bpf/libbpf.map | 6 +
tools/lib/bpf/libbpf_version.h | 2 +-
.../bpf/prog_tests/custom_sec_handlers.c | 176 ++++++++++
.../bpf/progs/test_custom_sec_handlers.c | 63 ++++
6 files changed, 552 insertions(+), 90 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] 16+ messages in thread
* [PATCH v3 bpf-next 1/3] libbpf: allow BPF program auto-attach handlers to bail out
2022-02-11 21:14 [PATCH v3 bpf-next 0/3] libbpf: support custom SEC() handlers Andrii Nakryiko
@ 2022-02-11 21:14 ` Andrii Nakryiko
2022-02-11 21:14 ` [PATCH v3 bpf-next 2/3] libbpf: support custom SEC() handlers Andrii Nakryiko
2022-02-11 21:14 ` [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest Andrii Nakryiko
2 siblings, 0 replies; 16+ messages in thread
From: Andrii Nakryiko @ 2022-02-11 21:14 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. Note that there is a somewhat subtle difference
between auto-attach behavior of BPF skeleton and using "generic"
bpf_program__attach(prog) (which uses the same attach handlers under the
cover). Skeleton allow some programs within bpf_object to not have
auto-attach implemented and doesn't treat that as an error. Instead such
BPF programs are just skipped during skeleton's (optional) attach step.
bpf_program__attach(), on the other hand, is called when user *expects*
auto-attach to work, so if specified program doesn't implement or
doesn't support auto-attach functionality, that will be treated as an
error.
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.
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 116 ++++++++++++++++++++++++++---------------
1 file changed, 73 insertions(+), 43 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2262bcdfee92..c4acb72d3c7e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -201,11 +201,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 {
@@ -239,9 +240,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;
};
/*
@@ -8581,12 +8582,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),
@@ -10093,14 +10094,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)
@@ -10110,21 +10110,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,
@@ -10379,14 +10377,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/"))
@@ -10396,14 +10393,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,
@@ -10436,7 +10433,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/",
@@ -10456,10 +10453,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 */
@@ -10502,14 +10500,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 *
@@ -10638,17 +10638,33 @@ 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 = NULL;
+ int err;
+
if (!prog->sec_def || !prog->sec_def->attach_fn)
- return libbpf_err_ptr(-ESRCH);
+ return libbpf_err_ptr(-EOPNOTSUPP);
- return prog->sec_def->attach_fn(prog, prog->sec_def->cookie);
+ err = prog->sec_def->attach_fn(prog, prog->sec_def->cookie, &link);
+ if (err)
+ return libbpf_err_ptr(err);
+
+ /* When calling bpf_program__attach() explicitly, auto-attach support
+ * is expected to work, so NULL returned link is considered an error.
+ * This is different for skeleton's attach, see comment in
+ * bpf_object__attach_skeleton().
+ */
+ if (!link)
+ return libbpf_err_ptr(-EOPNOTSUPP);
+
+ return link;
}
static int bpf_link__detach_struct_ops(struct bpf_link *link)
@@ -11792,13 +11808,27 @@ 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);
+ /* if user already set the link manually, don't attempt auto-attach */
+ if (*link)
+ continue;
+
+ 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] 16+ messages in thread
* [PATCH v3 bpf-next 2/3] libbpf: support custom SEC() handlers
2022-02-11 21:14 [PATCH v3 bpf-next 0/3] libbpf: support custom SEC() handlers Andrii Nakryiko
2022-02-11 21:14 ` [PATCH v3 bpf-next 1/3] libbpf: allow BPF program auto-attach handlers to bail out Andrii Nakryiko
@ 2022-02-11 21:14 ` Andrii Nakryiko
2022-02-14 10:34 ` Alan Maguire
2022-02-11 21:14 ` [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest Andrii Nakryiko
2 siblings, 1 reply; 16+ messages in thread
From: Andrii Nakryiko @ 2022-02-11 21:14 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.
This patch also bumps libbpf development version to v0.8 and adds new
APIs there.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 204 ++++++++++++++++++++++++---------
tools/lib/bpf/libbpf.h | 87 ++++++++++++++
tools/lib/bpf/libbpf.map | 6 +
tools/lib/bpf/libbpf_version.h | 2 +-
4 files changed, 246 insertions(+), 53 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index c4acb72d3c7e..2849017aec07 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -201,13 +201,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,
@@ -235,10 +228,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;
@@ -8574,7 +8568,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), \
@@ -8667,61 +8661,167 @@ 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,
+ const struct libbpf_prog_handler_opts *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_VALID(opts, libbpf_prog_handler_opts))
+ return libbpf_err(-EINVAL);
- /* "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 (last_custom_sec_def_handler_id == INT_MAX) /* prevent overflow */
+ return libbpf_err(-E2BIG);
- /* "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;
- }
+ if (sec) {
+ sec_def = libbpf_reallocarray(custom_sec_defs, custom_sec_def_cnt + 1,
+ sizeof(*sec_def));
+ if (!sec_def)
+ return libbpf_err(-ENOMEM);
- /* 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;
- }
+ custom_sec_defs = sec_def;
+ sec_def = &custom_sec_defs[custom_sec_def_cnt];
+ } else {
+ if (has_custom_fallback_def)
+ return libbpf_err(-EBUSY);
- /* 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 = &custom_fallback_def;
+ }
+
+ 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 = OPTS_GET(opts, cookie, 0);
+
+ sec_def->init_fn = OPTS_GET(opts, init_fn, NULL);
+ sec_def->preload_fn = OPTS_GET(opts, preload_fn, NULL);
+ sec_def->attach_fn = OPTS_GET(opts, attach_fn, NULL);
+
+ 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)
+{
+ struct bpf_sec_def *sec_defs;
+ 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--;
+
+ /* try to shrink the array, but it's ok if we couldn't */
+ sec_defs = libbpf_reallocarray(custom_sec_defs, custom_sec_def_cnt, sizeof(*sec_defs));
+ if (sec_defs)
+ custom_sec_defs = sec_defs;
+
+ 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..b7b5aa1b30dd 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1328,6 +1328,93 @@ 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);
+
+struct libbpf_prog_handler_opts {
+ /* size of this struct, for forward/backward compatiblity */
+ size_t sz;
+ /* User-provided cookie passed to each callback */
+ long cookie;
+ /* BPF program initialization callback (see libbpf_prog_init_fn_t) */
+ libbpf_prog_init_fn_t init_fn;
+ /* BPF program loading callback (see libbpf_prog_preload_fn_t) */
+ libbpf_prog_preload_fn_t preload_fn;
+ /* BPF program attach callback (see libbpf_prog_attach_fn_t) */
+ libbpf_prog_attach_fn_t attach_fn;
+};
+#define libbpf_prog_handler_opts__last_field attach_fn
+
+/**
+ * @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 opts optional cookie, callbacks, and other extra options
+ * @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,
+ const struct libbpf_prog_handler_opts *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 47e70c9058d9..df1b947792c8 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -439,3 +439,9 @@ LIBBPF_0.7.0 {
libbpf_probe_bpf_prog_type;
libbpf_set_memlock_rlim_max;
} LIBBPF_0.6.0;
+
+LIBBPF_0.8.0 {
+ global:
+ libbpf_register_prog_handler;
+ libbpf_unregister_prog_handler;
+} LIBBPF_0.7.0;
diff --git a/tools/lib/bpf/libbpf_version.h b/tools/lib/bpf/libbpf_version.h
index 0fefefc3500b..61f2039404b6 100644
--- a/tools/lib/bpf/libbpf_version.h
+++ b/tools/lib/bpf/libbpf_version.h
@@ -4,6 +4,6 @@
#define __LIBBPF_VERSION_H
#define LIBBPF_MAJOR_VERSION 0
-#define LIBBPF_MINOR_VERSION 7
+#define LIBBPF_MINOR_VERSION 8
#endif /* __LIBBPF_VERSION_H */
--
2.30.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-11 21:14 [PATCH v3 bpf-next 0/3] libbpf: support custom SEC() handlers Andrii Nakryiko
2022-02-11 21:14 ` [PATCH v3 bpf-next 1/3] libbpf: allow BPF program auto-attach handlers to bail out Andrii Nakryiko
2022-02-11 21:14 ` [PATCH v3 bpf-next 2/3] libbpf: support custom SEC() handlers Andrii Nakryiko
@ 2022-02-11 21:14 ` Andrii Nakryiko
2022-02-11 23:13 ` Alexei Starovoitov
2022-02-14 10:36 ` Alan Maguire
2 siblings, 2 replies; 16+ messages in thread
From: Andrii Nakryiko @ 2022-02-11 21:14 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.
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
.../bpf/prog_tests/custom_sec_handlers.c | 176 ++++++++++++++++++
.../bpf/progs/test_custom_sec_handlers.c | 63 +++++++
2 files changed, 239 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..28264528280d
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
@@ -0,0 +1,176 @@
+// 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
+#define COOKIE_KPROBE 5
+
+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)
+{
+ if (cookie == COOKIE_FALLBACK)
+ opts->prog_flags |= BPF_F_SLEEPABLE;
+ else if (cookie == COOKIE_ABC1)
+ ASSERT_FALSE(true, "unexpected preload for abc");
+
+ 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_KPROBE:
+ case COOKIE_FALLBACK:
+ /* no auto-attach for SEC("xyz") and SEC("kprobe") */
+ *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;
+static int kprobe_id;
+
+__attribute__((constructor))
+static void register_sec_handlers(void)
+{
+ LIBBPF_OPTS(libbpf_prog_handler_opts, abc1_opts,
+ .cookie = COOKIE_ABC1,
+ .init_fn = custom_init_prog,
+ .preload_fn = custom_preload_prog,
+ .attach_fn = NULL,
+ );
+ LIBBPF_OPTS(libbpf_prog_handler_opts, abc2_opts,
+ .cookie = COOKIE_ABC2,
+ .init_fn = custom_init_prog,
+ .preload_fn = custom_preload_prog,
+ .attach_fn = custom_attach_prog,
+ );
+ LIBBPF_OPTS(libbpf_prog_handler_opts, custom_opts,
+ .cookie = COOKIE_CUSTOM,
+ .init_fn = NULL,
+ .preload_fn = NULL,
+ .attach_fn = custom_attach_prog,
+ );
+
+ abc1_id = libbpf_register_prog_handler("abc", BPF_PROG_TYPE_RAW_TRACEPOINT, 0, &abc1_opts);
+ abc2_id = libbpf_register_prog_handler("abc/", BPF_PROG_TYPE_RAW_TRACEPOINT, 0, &abc2_opts);
+ custom_id = libbpf_register_prog_handler("custom+", BPF_PROG_TYPE_TRACEPOINT, 0, &custom_opts);
+}
+
+__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)
+{
+ LIBBPF_OPTS(libbpf_prog_handler_opts, opts,
+ .init_fn = custom_init_prog,
+ .preload_fn = custom_preload_prog,
+ .attach_fn = custom_attach_prog,
+ );
+ 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");
+
+ /* override libbpf's handle of SEC("kprobe/...") but also allow pure
+ * SEC("kprobe") due to "kprobe+" specifier. Register it as
+ * TRACEPOINT, just for fun.
+ */
+ opts.cookie = COOKIE_KPROBE;
+ kprobe_id = libbpf_register_prog_handler("kprobe+", BPF_PROG_TYPE_TRACEPOINT, 0, &opts);
+ /* fallback treats everything as BPF_PROG_TYPE_SYSCALL program to test
+ * setting custom BPF_F_SLEEPABLE bit in preload handler
+ */
+ opts.cookie = COOKIE_FALLBACK;
+ fallback_id = libbpf_register_prog_handler(NULL, BPF_PROG_TYPE_SYSCALL, 0, &opts);
+
+ if (!ASSERT_GT(fallback_id, 0, "fallback_id") /* || !ASSERT_GT(kprobe_id, 0, "kprobe_id")*/) {
+ if (fallback_id > 0)
+ libbpf_unregister_prog_handler(fallback_id);
+ if (kprobe_id > 0)
+ libbpf_unregister_prog_handler(kprobe_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.kprobe1), BPF_PROG_TYPE_TRACEPOINT, "kprobe1_type");
+ ASSERT_EQ(bpf_program__type(skel->progs.xyz), BPF_PROG_TYPE_SYSCALL, "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;
+
+ skel->links.xyz = bpf_program__attach(skel->progs.kprobe1);
+ ASSERT_EQ(errno, EOPNOTSUPP, "xyz_attach_err");
+ ASSERT_ERR_PTR(skel->links.xyz, "xyz_attach");
+
+ /* 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("kprobe") shouldn't be auto-attached */
+ ASSERT_FALSE(skel->bss->kprobe1_called, "kprobe1_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");
+ ASSERT_OK(libbpf_unregister_prog_handler(kprobe_id), "unregister_kprobe");
+}
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..4061f701ca50
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
@@ -0,0 +1,63 @@
+// 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 kprobe1_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("kprobe")
+int kprobe1(void *ctx)
+{
+ kprobe1_called = true;
+ return 0;
+}
+
+SEC("xyz/blah")
+int xyz(void *ctx)
+{
+ int whatever;
+
+ /* use sleepable helper, custom handler should set sleepable flag */
+ bpf_copy_from_user(&whatever, sizeof(whatever), NULL);
+ xyz_called = true;
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.30.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-11 21:14 ` [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest Andrii Nakryiko
@ 2022-02-11 23:13 ` Alexei Starovoitov
2022-02-11 23:31 ` Andrii Nakryiko
2022-02-14 10:36 ` Alan Maguire
1 sibling, 1 reply; 16+ messages in thread
From: Alexei Starovoitov @ 2022-02-11 23:13 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, kernel-team, Alan Maguire
On Fri, Feb 11, 2022 at 01:14:50PM -0800, 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.
>
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> .../bpf/prog_tests/custom_sec_handlers.c | 176 ++++++++++++++++++
> .../bpf/progs/test_custom_sec_handlers.c | 63 +++++++
> 2 files changed, 239 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..28264528280d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
> @@ -0,0 +1,176 @@
> +// 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
> +#define COOKIE_KPROBE 5
> +
> +static int custom_init_prog(struct bpf_program *prog, long cookie)
> +{
> + if (cookie == COOKIE_ABC1)
> + bpf_program__set_autoload(prog, false);
> +
> + return 0;
> +}
What is the value of init_fn callback?
afaict it was and still unused in libbpf.
The above example doesn't make a compelling case, since set_autoload
can be done out of preload callback.
Maybe drop init_fn for now and extend libbpf_prog_handler_opts
when there is actual need for it?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-11 23:13 ` Alexei Starovoitov
@ 2022-02-11 23:31 ` Andrii Nakryiko
2022-02-12 0:18 ` Alexei Starovoitov
0 siblings, 1 reply; 16+ messages in thread
From: Andrii Nakryiko @ 2022-02-11 23:31 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, Alan Maguire
On Fri, Feb 11, 2022 at 3:13 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Feb 11, 2022 at 01:14:50PM -0800, 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.
> >
> > Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> > .../bpf/prog_tests/custom_sec_handlers.c | 176 ++++++++++++++++++
> > .../bpf/progs/test_custom_sec_handlers.c | 63 +++++++
> > 2 files changed, 239 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..28264528280d
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
> > @@ -0,0 +1,176 @@
> > +// 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
> > +#define COOKIE_KPROBE 5
> > +
> > +static int custom_init_prog(struct bpf_program *prog, long cookie)
> > +{
> > + if (cookie == COOKIE_ABC1)
> > + bpf_program__set_autoload(prog, false);
> > +
> > + return 0;
> > +}
>
> What is the value of init_fn callback?
> afaict it was and still unused in libbpf.
> The above example doesn't make a compelling case, since set_autoload
> can be done out of preload callback.
> Maybe drop init_fn for now and extend libbpf_prog_handler_opts
> when there is actual need for it?
Great question, but no, you can't set_autoload() in the preload
handler, because once preload is called, loading of the program is
inevitable.
We might need to adjust the obj->loaded flag so that set_autoload()
returns an error when called from the preload() callback, but that's a
bit orthogonal. I suspect there will be few more adjustments like this
as we get actual users of this API in the wild.
It's not used by libbpf because we do all the initialization outside
of the callback, as it is the same for all programs and serves as
"default" behavior that custom handlers can override.
Also, keep in mind that this is the very beginning of v0.8 dev cycle,
we'll have time to adjust interfaces and callback contracts in the
next 2-3 months, if necessary. USDT library open-sourcing will almost
100% happen during this time frame (though I think USDT library is a
pretty simple use case, so isn't a great "stress test"). But it also
seems like perf might need to use fallback handler for their crazy
SEC() conventions, that will be a good test as well.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-11 23:31 ` Andrii Nakryiko
@ 2022-02-12 0:18 ` Alexei Starovoitov
2022-02-12 1:16 ` Andrii Nakryiko
0 siblings, 1 reply; 16+ messages in thread
From: Alexei Starovoitov @ 2022-02-12 0:18 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, Alan Maguire
On Fri, Feb 11, 2022 at 03:31:56PM -0800, Andrii Nakryiko wrote:
> On Fri, Feb 11, 2022 at 3:13 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Feb 11, 2022 at 01:14:50PM -0800, 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.
> > >
> > > Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > > ---
> > > .../bpf/prog_tests/custom_sec_handlers.c | 176 ++++++++++++++++++
> > > .../bpf/progs/test_custom_sec_handlers.c | 63 +++++++
> > > 2 files changed, 239 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..28264528280d
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
> > > @@ -0,0 +1,176 @@
> > > +// 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
> > > +#define COOKIE_KPROBE 5
> > > +
> > > +static int custom_init_prog(struct bpf_program *prog, long cookie)
> > > +{
> > > + if (cookie == COOKIE_ABC1)
> > > + bpf_program__set_autoload(prog, false);
> > > +
> > > + return 0;
> > > +}
> >
> > What is the value of init_fn callback?
> > afaict it was and still unused in libbpf.
> > The above example doesn't make a compelling case, since set_autoload
> > can be done out of preload callback.
> > Maybe drop init_fn for now and extend libbpf_prog_handler_opts
> > when there is actual need for it?
>
> Great question, but no, you can't set_autoload() in the preload
> handler, because once preload is called, loading of the program is
> inevitable.
Ahh!, but we can add 'if (prog->load)' in bpf_object_load_prog_instance()
after preload_fn() was called.
Surely the libbpf would waste some time preping the prog with relos,
but that's not a big deal.
Another option is to move preload_fn earlier.
Especially since currently it's only setting attach types.
Calling the callback 'preload' when it cannot affect the load is odd too.
> We might need to adjust the obj->loaded flag so that set_autoload()
> returns an error when called from the preload() callback, but that's a
> bit orthogonal. I suspect there will be few more adjustments like this
> as we get actual users of this API in the wild.
>
> It's not used by libbpf because we do all the initialization outside
> of the callback, as it is the same for all programs and serves as
> "default" behavior that custom handlers can override.
>
> Also, keep in mind that this is the very beginning of v0.8 dev cycle,
> we'll have time to adjust interfaces and callback contracts in the
> next 2-3 months, if necessary. USDT library open-sourcing will almost
> 100% happen during this time frame (though I think USDT library is a
> pretty simple use case, so isn't a great "stress test"). But it also
> seems like perf might need to use fallback handler for their crazy
> SEC() conventions, that will be a good test as well.
It would be much easier to take your word if there was an actual example
(like libusdt) that demonstrates the use of callbacks.
"We will have time to fix things before release" isn't very comforting
in the case of big api extension like this one.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-12 0:18 ` Alexei Starovoitov
@ 2022-02-12 1:16 ` Andrii Nakryiko
2022-02-14 11:13 ` Alan Maguire
2022-02-14 17:27 ` Alexei Starovoitov
0 siblings, 2 replies; 16+ messages in thread
From: Andrii Nakryiko @ 2022-02-12 1:16 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, Alan Maguire
On Fri, Feb 11, 2022 at 4:18 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Feb 11, 2022 at 03:31:56PM -0800, Andrii Nakryiko wrote:
> > On Fri, Feb 11, 2022 at 3:13 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Fri, Feb 11, 2022 at 01:14:50PM -0800, 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.
> > > >
> > > > Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > > > ---
> > > > .../bpf/prog_tests/custom_sec_handlers.c | 176 ++++++++++++++++++
> > > > .../bpf/progs/test_custom_sec_handlers.c | 63 +++++++
> > > > 2 files changed, 239 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..28264528280d
> > > > --- /dev/null
> > > > +++ b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
> > > > @@ -0,0 +1,176 @@
> > > > +// 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
> > > > +#define COOKIE_KPROBE 5
> > > > +
> > > > +static int custom_init_prog(struct bpf_program *prog, long cookie)
> > > > +{
> > > > + if (cookie == COOKIE_ABC1)
> > > > + bpf_program__set_autoload(prog, false);
> > > > +
> > > > + return 0;
> > > > +}
> > >
> > > What is the value of init_fn callback?
> > > afaict it was and still unused in libbpf.
> > > The above example doesn't make a compelling case, since set_autoload
> > > can be done out of preload callback.
> > > Maybe drop init_fn for now and extend libbpf_prog_handler_opts
> > > when there is actual need for it?
> >
> > Great question, but no, you can't set_autoload() in the preload
> > handler, because once preload is called, loading of the program is
> > inevitable.
>
> Ahh!, but we can add 'if (prog->load)' in bpf_object_load_prog_instance()
> after preload_fn() was called.
Yes we can and solve this *one specific* scenario. But there is a
bunch of preparatory stuff that's happening for bpf_program before we
get to actually loading finalized instructions. All the relocations,
marking whether we need vmlinux BTF, etc. All that is skipped if
!prog->load.
I don't want to go and analyze every single possible scenario (and
probably still miss a bunch of subtle ones) to understand if it's
always equivalent. Libbpf's contract is that
bpf_program__set_autoload() is called before bpf_object__load(). You
are asking me to redesign this contract to move it much deeper into
bpf_object__load() (and potentially break a bunch of subtle things)
just to avoid init_fn callback. Hard sell :)
Basically, init_fn is allowed to do everything that normal user code
is allowed to do between bpf_object__open() and bpf_object__load().
preload_fn() doesn't have this luxury, but gets access to
bpf_prog_load opts that normal user code doesn't have access, but it's
not free to do all the stuff that user is free to do before
bpf_object__load(). They are not interchangeable.
> Surely the libbpf would waste some time preping the prog with relos,
> but that's not a big deal.
> Another option is to move preload_fn earlier.
> Especially since currently it's only setting attach types.
It should be able to affect logging and all the attach parameter. I
didn't want to design new OPTS struct just for this callback, so I'm
trying to reuse bpf_prog_load_opts as a contract. That means I can't
easily change prog_type (but that's trivial to handle in init_fn) and
insns (but I can hardly see how that can be done safely at all), but
otherwise those opts give the full power of low-level bpf_prog_load.
I keep a possibility open to change preload_fn contract to actually
execute bpf_prog_load() on its own and return prog fd, but I'm
hesitant because all the libbpf log handling and retries, and other
niceties will be lost, making trivial things like adding extra
BPF_F_SLEEPABLE flag not trivial at all. But here's the thing, we can
later add "advanced" load callback that will be mutually exclusive
with preload_fn and would be able to handle more advanced cases. But
that can be done as an extra extension without changing anything about
current interface.
>
> Calling the callback 'preload' when it cannot affect the load is odd too.
It's what happening before loading, I never had intention to prevent
load... Would "prepare_load_fn" be a better name?
>
> > We might need to adjust the obj->loaded flag so that set_autoload()
> > returns an error when called from the preload() callback, but that's a
> > bit orthogonal. I suspect there will be few more adjustments like this
> > as we get actual users of this API in the wild.
> >
> > It's not used by libbpf because we do all the initialization outside
> > of the callback, as it is the same for all programs and serves as
> > "default" behavior that custom handlers can override.
> >
> > Also, keep in mind that this is the very beginning of v0.8 dev cycle,
> > we'll have time to adjust interfaces and callback contracts in the
> > next 2-3 months, if necessary. USDT library open-sourcing will almost
> > 100% happen during this time frame (though I think USDT library is a
> > pretty simple use case, so isn't a great "stress test"). But it also
> > seems like perf might need to use fallback handler for their crazy
> > SEC() conventions, that will be a good test as well.
>
> It would be much easier to take your word if there was an actual example
> (like libusdt) that demonstrates the use of callbacks.
> "We will have time to fix things before release" isn't very comforting
> in the case of big api extension like this one.
Hmm. For libusdt it would be literally:
libbpf_register_prog_handler("usdt", BPF_PROG_TYPE_KPROBE, 0, NULL);
Done.
There is no way (at least currently) to support auto-attach through
skeleton__attach() or bpf_program__attach(), because single USDT
attachment is actually multiple program attachments (due to inlining).
So until libbpf provides APIs to construct "composite" bpf_link from a
single link, there won't be auto-attach. We might add it later, but I
don't want to design the entire world in one patch set :)
USDT is too simple a use case, perhaps. I'm trying to also take into
consideration perf's custom SEC("lock_page=__lock_page page->flags")
use case, hypothetical SEC("perf_event/cpu_cycles:1000") case, and
just thinking from the "first principles" what some advanced library
might what to be able to do with this. Alan's uprobe attach by
function name would be implementable through these APIs outside of
libbpf as well (except then we won't be able to add func_name into
bpf_uprobe_opts, which would be a pity).
I can postpone this whole patch set until later as well, don't care
all that much. I hate callback APIs anyways :)
We can do USDT library without all this and the user experience won't
change all that much, actually.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 bpf-next 2/3] libbpf: support custom SEC() handlers
2022-02-11 21:14 ` [PATCH v3 bpf-next 2/3] libbpf: support custom SEC() handlers Andrii Nakryiko
@ 2022-02-14 10:34 ` Alan Maguire
0 siblings, 0 replies; 16+ messages in thread
From: Alan Maguire @ 2022-02-14 10:34 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, kernel-team, Alan Maguire
On Fri, 11 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.
>
> This patch also bumps libbpf development version to v0.8 and adds new
> APIs there.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Looks great, thanks!
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> ---
> tools/lib/bpf/libbpf.c | 204 ++++++++++++++++++++++++---------
> tools/lib/bpf/libbpf.h | 87 ++++++++++++++
> tools/lib/bpf/libbpf.map | 6 +
> tools/lib/bpf/libbpf_version.h | 2 +-
> 4 files changed, 246 insertions(+), 53 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index c4acb72d3c7e..2849017aec07 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -201,13 +201,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,
> @@ -235,10 +228,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;
> @@ -8574,7 +8568,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), \
> @@ -8667,61 +8661,167 @@ 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,
> + const struct libbpf_prog_handler_opts *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_VALID(opts, libbpf_prog_handler_opts))
> + return libbpf_err(-EINVAL);
>
> - /* "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 (last_custom_sec_def_handler_id == INT_MAX) /* prevent overflow */
> + return libbpf_err(-E2BIG);
>
> - /* "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;
> - }
> + if (sec) {
> + sec_def = libbpf_reallocarray(custom_sec_defs, custom_sec_def_cnt + 1,
> + sizeof(*sec_def));
> + if (!sec_def)
> + return libbpf_err(-ENOMEM);
>
> - /* 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;
> - }
> + custom_sec_defs = sec_def;
> + sec_def = &custom_sec_defs[custom_sec_def_cnt];
> + } else {
> + if (has_custom_fallback_def)
> + return libbpf_err(-EBUSY);
>
> - /* 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 = &custom_fallback_def;
> + }
> +
> + 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 = OPTS_GET(opts, cookie, 0);
> +
> + sec_def->init_fn = OPTS_GET(opts, init_fn, NULL);
> + sec_def->preload_fn = OPTS_GET(opts, preload_fn, NULL);
> + sec_def->attach_fn = OPTS_GET(opts, attach_fn, NULL);
> +
> + 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)
> +{
> + struct bpf_sec_def *sec_defs;
> + 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--;
> +
> + /* try to shrink the array, but it's ok if we couldn't */
> + sec_defs = libbpf_reallocarray(custom_sec_defs, custom_sec_def_cnt, sizeof(*sec_defs));
> + if (sec_defs)
> + custom_sec_defs = sec_defs;
> +
> + 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..b7b5aa1b30dd 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -1328,6 +1328,93 @@ 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);
> +
> +struct libbpf_prog_handler_opts {
> + /* size of this struct, for forward/backward compatiblity */
> + size_t sz;
> + /* User-provided cookie passed to each callback */
> + long cookie;
> + /* BPF program initialization callback (see libbpf_prog_init_fn_t) */
> + libbpf_prog_init_fn_t init_fn;
> + /* BPF program loading callback (see libbpf_prog_preload_fn_t) */
> + libbpf_prog_preload_fn_t preload_fn;
> + /* BPF program attach callback (see libbpf_prog_attach_fn_t) */
> + libbpf_prog_attach_fn_t attach_fn;
> +};
> +#define libbpf_prog_handler_opts__last_field attach_fn
> +
> +/**
> + * @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 opts optional cookie, callbacks, and other extra options
> + * @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,
> + const struct libbpf_prog_handler_opts *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 47e70c9058d9..df1b947792c8 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -439,3 +439,9 @@ LIBBPF_0.7.0 {
> libbpf_probe_bpf_prog_type;
> libbpf_set_memlock_rlim_max;
> } LIBBPF_0.6.0;
> +
> +LIBBPF_0.8.0 {
> + global:
> + libbpf_register_prog_handler;
> + libbpf_unregister_prog_handler;
> +} LIBBPF_0.7.0;
> diff --git a/tools/lib/bpf/libbpf_version.h b/tools/lib/bpf/libbpf_version.h
> index 0fefefc3500b..61f2039404b6 100644
> --- a/tools/lib/bpf/libbpf_version.h
> +++ b/tools/lib/bpf/libbpf_version.h
> @@ -4,6 +4,6 @@
> #define __LIBBPF_VERSION_H
>
> #define LIBBPF_MAJOR_VERSION 0
> -#define LIBBPF_MINOR_VERSION 7
> +#define LIBBPF_MINOR_VERSION 8
>
> #endif /* __LIBBPF_VERSION_H */
> --
> 2.30.2
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-11 21:14 ` [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest Andrii Nakryiko
2022-02-11 23:13 ` Alexei Starovoitov
@ 2022-02-14 10:36 ` Alan Maguire
1 sibling, 0 replies; 16+ messages in thread
From: Alan Maguire @ 2022-02-14 10:36 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, kernel-team, Alan Maguire
On Fri, 11 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.
>
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Thanks for adding the additional tests, looks great!
> ---
> .../bpf/prog_tests/custom_sec_handlers.c | 176 ++++++++++++++++++
> .../bpf/progs/test_custom_sec_handlers.c | 63 +++++++
> 2 files changed, 239 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..28264528280d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
> @@ -0,0 +1,176 @@
> +// 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
> +#define COOKIE_KPROBE 5
> +
> +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)
> +{
> + if (cookie == COOKIE_FALLBACK)
> + opts->prog_flags |= BPF_F_SLEEPABLE;
> + else if (cookie == COOKIE_ABC1)
> + ASSERT_FALSE(true, "unexpected preload for abc");
> +
> + 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_KPROBE:
> + case COOKIE_FALLBACK:
> + /* no auto-attach for SEC("xyz") and SEC("kprobe") */
> + *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;
> +static int kprobe_id;
> +
> +__attribute__((constructor))
> +static void register_sec_handlers(void)
> +{
> + LIBBPF_OPTS(libbpf_prog_handler_opts, abc1_opts,
> + .cookie = COOKIE_ABC1,
> + .init_fn = custom_init_prog,
> + .preload_fn = custom_preload_prog,
> + .attach_fn = NULL,
> + );
> + LIBBPF_OPTS(libbpf_prog_handler_opts, abc2_opts,
> + .cookie = COOKIE_ABC2,
> + .init_fn = custom_init_prog,
> + .preload_fn = custom_preload_prog,
> + .attach_fn = custom_attach_prog,
> + );
> + LIBBPF_OPTS(libbpf_prog_handler_opts, custom_opts,
> + .cookie = COOKIE_CUSTOM,
> + .init_fn = NULL,
> + .preload_fn = NULL,
> + .attach_fn = custom_attach_prog,
> + );
> +
> + abc1_id = libbpf_register_prog_handler("abc", BPF_PROG_TYPE_RAW_TRACEPOINT, 0, &abc1_opts);
> + abc2_id = libbpf_register_prog_handler("abc/", BPF_PROG_TYPE_RAW_TRACEPOINT, 0, &abc2_opts);
> + custom_id = libbpf_register_prog_handler("custom+", BPF_PROG_TYPE_TRACEPOINT, 0, &custom_opts);
> +}
> +
> +__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)
> +{
> + LIBBPF_OPTS(libbpf_prog_handler_opts, opts,
> + .init_fn = custom_init_prog,
> + .preload_fn = custom_preload_prog,
> + .attach_fn = custom_attach_prog,
> + );
> + 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");
> +
> + /* override libbpf's handle of SEC("kprobe/...") but also allow pure
> + * SEC("kprobe") due to "kprobe+" specifier. Register it as
> + * TRACEPOINT, just for fun.
> + */
> + opts.cookie = COOKIE_KPROBE;
> + kprobe_id = libbpf_register_prog_handler("kprobe+", BPF_PROG_TYPE_TRACEPOINT, 0, &opts);
> + /* fallback treats everything as BPF_PROG_TYPE_SYSCALL program to test
> + * setting custom BPF_F_SLEEPABLE bit in preload handler
> + */
> + opts.cookie = COOKIE_FALLBACK;
> + fallback_id = libbpf_register_prog_handler(NULL, BPF_PROG_TYPE_SYSCALL, 0, &opts);
> +
> + if (!ASSERT_GT(fallback_id, 0, "fallback_id") /* || !ASSERT_GT(kprobe_id, 0, "kprobe_id")*/) {
> + if (fallback_id > 0)
> + libbpf_unregister_prog_handler(fallback_id);
> + if (kprobe_id > 0)
> + libbpf_unregister_prog_handler(kprobe_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.kprobe1), BPF_PROG_TYPE_TRACEPOINT, "kprobe1_type");
> + ASSERT_EQ(bpf_program__type(skel->progs.xyz), BPF_PROG_TYPE_SYSCALL, "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;
> +
> + skel->links.xyz = bpf_program__attach(skel->progs.kprobe1);
> + ASSERT_EQ(errno, EOPNOTSUPP, "xyz_attach_err");
> + ASSERT_ERR_PTR(skel->links.xyz, "xyz_attach");
> +
> + /* 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("kprobe") shouldn't be auto-attached */
> + ASSERT_FALSE(skel->bss->kprobe1_called, "kprobe1_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");
> + ASSERT_OK(libbpf_unregister_prog_handler(kprobe_id), "unregister_kprobe");
> +}
> 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..4061f701ca50
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
> @@ -0,0 +1,63 @@
> +// 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 kprobe1_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("kprobe")
> +int kprobe1(void *ctx)
> +{
> + kprobe1_called = true;
> + return 0;
> +}
> +
> +SEC("xyz/blah")
> +int xyz(void *ctx)
> +{
> + int whatever;
> +
> + /* use sleepable helper, custom handler should set sleepable flag */
> + bpf_copy_from_user(&whatever, sizeof(whatever), NULL);
> + xyz_called = true;
> + return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.30.2
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-12 1:16 ` Andrii Nakryiko
@ 2022-02-14 11:13 ` Alan Maguire
2022-02-14 17:27 ` Alexei Starovoitov
1 sibling, 0 replies; 16+ messages in thread
From: Alan Maguire @ 2022-02-14 11:13 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Alexei Starovoitov,
Daniel Borkmann, Kernel Team, Alan Maguire
On Sat, 12 Feb 2022, Andrii Nakryiko wrote:
> On Fri, Feb 11, 2022 at 4:18 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Feb 11, 2022 at 03:31:56PM -0800, Andrii Nakryiko wrote:
> > > On Fri, Feb 11, 2022 at 3:13 PM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Fri, Feb 11, 2022 at 01:14:50PM -0800, 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.
> > > > >
> > > > > Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > > > > ---
> > > > > .../bpf/prog_tests/custom_sec_handlers.c | 176 ++++++++++++++++++
> > > > > .../bpf/progs/test_custom_sec_handlers.c | 63 +++++++
> > > > > 2 files changed, 239 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..28264528280d
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
> > > > > @@ -0,0 +1,176 @@
> > > > > +// 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
> > > > > +#define COOKIE_KPROBE 5
> > > > > +
> > > > > +static int custom_init_prog(struct bpf_program *prog, long cookie)
> > > > > +{
> > > > > + if (cookie == COOKIE_ABC1)
> > > > > + bpf_program__set_autoload(prog, false);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > >
> > > > What is the value of init_fn callback?
> > > > afaict it was and still unused in libbpf.
> > > > The above example doesn't make a compelling case, since set_autoload
> > > > can be done out of preload callback.
> > > > Maybe drop init_fn for now and extend libbpf_prog_handler_opts
> > > > when there is actual need for it?
> > >
> > > Great question, but no, you can't set_autoload() in the preload
> > > handler, because once preload is called, loading of the program is
> > > inevitable.
> >
> > Ahh!, but we can add 'if (prog->load)' in bpf_object_load_prog_instance()
> > after preload_fn() was called.
>
> Yes we can and solve this *one specific* scenario. But there is a
> bunch of preparatory stuff that's happening for bpf_program before we
> get to actually loading finalized instructions. All the relocations,
> marking whether we need vmlinux BTF, etc. All that is skipped if
> !prog->load.
>
> I don't want to go and analyze every single possible scenario (and
> probably still miss a bunch of subtle ones) to understand if it's
> always equivalent. Libbpf's contract is that
> bpf_program__set_autoload() is called before bpf_object__load(). You
> are asking me to redesign this contract to move it much deeper into
> bpf_object__load() (and potentially break a bunch of subtle things)
> just to avoid init_fn callback. Hard sell :)
>
> Basically, init_fn is allowed to do everything that normal user code
> is allowed to do between bpf_object__open() and bpf_object__load().
> preload_fn() doesn't have this luxury, but gets access to
> bpf_prog_load opts that normal user code doesn't have access, but it's
> not free to do all the stuff that user is free to do before
> bpf_object__load(). They are not interchangeable.
>
> > Surely the libbpf would waste some time preping the prog with relos,
> > but that's not a big deal.
> > Another option is to move preload_fn earlier.
> > Especially since currently it's only setting attach types.
>
> It should be able to affect logging and all the attach parameter. I
> didn't want to design new OPTS struct just for this callback, so I'm
> trying to reuse bpf_prog_load_opts as a contract. That means I can't
> easily change prog_type (but that's trivial to handle in init_fn) and
> insns (but I can hardly see how that can be done safely at all), but
> otherwise those opts give the full power of low-level bpf_prog_load.
>
> I keep a possibility open to change preload_fn contract to actually
> execute bpf_prog_load() on its own and return prog fd, but I'm
> hesitant because all the libbpf log handling and retries, and other
> niceties will be lost, making trivial things like adding extra
> BPF_F_SLEEPABLE flag not trivial at all. But here's the thing, we can
> later add "advanced" load callback that will be mutually exclusive
> with preload_fn and would be able to handle more advanced cases. But
> that can be done as an extra extension without changing anything about
> current interface.
>
> >
> > Calling the callback 'preload' when it cannot affect the load is odd too.
>
> It's what happening before loading, I never had intention to prevent
> load... Would "prepare_load_fn" be a better name?
>
> >
> > > We might need to adjust the obj->loaded flag so that set_autoload()
> > > returns an error when called from the preload() callback, but that's a
> > > bit orthogonal. I suspect there will be few more adjustments like this
> > > as we get actual users of this API in the wild.
> > >
> > > It's not used by libbpf because we do all the initialization outside
> > > of the callback, as it is the same for all programs and serves as
> > > "default" behavior that custom handlers can override.
> > >
> > > Also, keep in mind that this is the very beginning of v0.8 dev cycle,
> > > we'll have time to adjust interfaces and callback contracts in the
> > > next 2-3 months, if necessary. USDT library open-sourcing will almost
> > > 100% happen during this time frame (though I think USDT library is a
> > > pretty simple use case, so isn't a great "stress test"). But it also
> > > seems like perf might need to use fallback handler for their crazy
> > > SEC() conventions, that will be a good test as well.
> >
> > It would be much easier to take your word if there was an actual example
> > (like libusdt) that demonstrates the use of callbacks.
> > "We will have time to fix things before release" isn't very comforting
> > in the case of big api extension like this one.
>
> Hmm. For libusdt it would be literally:
>
> libbpf_register_prog_handler("usdt", BPF_PROG_TYPE_KPROBE, 0, NULL);
>
> Done.
>
> There is no way (at least currently) to support auto-attach through
> skeleton__attach() or bpf_program__attach(), because single USDT
> attachment is actually multiple program attachments (due to inlining).
> So until libbpf provides APIs to construct "composite" bpf_link from a
> single link, there won't be auto-attach. We might add it later, but I
> don't want to design the entire world in one patch set :)
>
> USDT is too simple a use case, perhaps. I'm trying to also take into
> consideration perf's custom SEC("lock_page=__lock_page page->flags")
> use case, hypothetical SEC("perf_event/cpu_cycles:1000") case, and
> just thinking from the "first principles" what some advanced library
> might what to be able to do with this. Alan's uprobe attach by
> function name would be implementable through these APIs outside of
> libbpf as well (except then we won't be able to add func_name into
> bpf_uprobe_opts, which would be a pity).
>
> I can postpone this whole patch set until later as well, don't care
> all that much. I hate callback APIs anyways :)
>
> We can do USDT library without all this and the user experience won't
> change all that much, actually.
>
Here's one case I had to implement which would be made easier with
this support:
- optional attach using an "o" prefix ("okprobe", "okretprobe"),
where attach failure is not fatal. Used for cases where multiple
possible candidate functions in modules are traced, but we
have no guarantees which module is loaded, and hence which
function is available. Could be implemented by overriding
attach and returning 0 with a NULL link for cases where attach
fails with ENOENT, and overriding preload to set program type
to KPROBE. Having optional support like this in pluggable
section handling makes skeleton interactions simpler where we
mix and match required kprobes and optional ones.
Having custom section handling is nice because the above was
required in a bunch of different tools, and with custom section
handling, skeleton interactions are a lot cleaner.
I would expect tracers would find this functionality useful
too, for example supporting per-pid conventions for uprobe
tracing like "uprobe1234/prog:func".
Alan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-12 1:16 ` Andrii Nakryiko
2022-02-14 11:13 ` Alan Maguire
@ 2022-02-14 17:27 ` Alexei Starovoitov
2022-02-14 19:55 ` Andrii Nakryiko
2022-02-14 23:13 ` Alan Maguire
1 sibling, 2 replies; 16+ messages in thread
From: Alexei Starovoitov @ 2022-02-14 17:27 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, Alan Maguire
On Fri, Feb 11, 2022 at 05:16:25PM -0800, Andrii Nakryiko wrote:
> >
> > Calling the callback 'preload' when it cannot affect the load is odd too.
>
> It's what happening before loading, I never had intention to prevent
> load... Would "prepare_load_fn" be a better name?
prepare_load_fn would be more accurate name for sure.
If we're not planning to change place where init_fn is called too
then can we rename it to something that would accurately describe it?
It seems it's called after ELF is fully parsed except relos and progs
are ready to be tweaked.
Should 'prog' be in the name? Like prog_setup_fn ? or prog_init_fn ?
Then ability to set prog autoload would flow naturally from such name.
What else can be done there? Or what is a recommended use of this cb?
> might what to be able to do with this. Alan's uprobe attach by
> function name would be implementable through these APIs outside of
> libbpf as well (except then we won't be able to add func_name into
> bpf_uprobe_opts, which would be a pity).
Alan,
can you demo your "okprobe" feature based on this api?
Any rough patches would do.
The "o" handling will be done in which callback?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-14 17:27 ` Alexei Starovoitov
@ 2022-02-14 19:55 ` Andrii Nakryiko
2022-02-14 23:13 ` Alan Maguire
1 sibling, 0 replies; 16+ messages in thread
From: Andrii Nakryiko @ 2022-02-14 19:55 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, Alan Maguire
On Mon, Feb 14, 2022 at 9:27 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Feb 11, 2022 at 05:16:25PM -0800, Andrii Nakryiko wrote:
> > >
> > > Calling the callback 'preload' when it cannot affect the load is odd too.
> >
> > It's what happening before loading, I never had intention to prevent
> > load... Would "prepare_load_fn" be a better name?
>
> prepare_load_fn would be more accurate name for sure.
SGTM, I'll rename
>
> If we're not planning to change place where init_fn is called too
> then can we rename it to something that would accurately describe it?
> It seems it's called after ELF is fully parsed except relos and progs
> are ready to be tweaked.
> Should 'prog' be in the name? Like prog_setup_fn ? or prog_init_fn ?
> Then ability to set prog autoload would flow naturally from such name.
> What else can be done there? Or what is a recommended use of this cb?
I like prog_setup_fn name, it matches semantics very closely. It is
supposed to be able to do everything that user can do though all the
bpf_program getters/setters before bpf_object__load() step. If some of
those setters doesn't work from prog_setup_fn(), we'll fix that, but
otherwise I think it will always be a "post-bpf_object__open()"
callback to adjust whatever libbpf does by default.
>
> > might what to be able to do with this. Alan's uprobe attach by
> > function name would be implementable through these APIs outside of
> > libbpf as well (except then we won't be able to add func_name into
> > bpf_uprobe_opts, which would be a pity).
>
> Alan,
> can you demo your "okprobe" feature based on this api?
> Any rough patches would do.
> The "o" handling will be done in which callback?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-14 17:27 ` Alexei Starovoitov
2022-02-14 19:55 ` Andrii Nakryiko
@ 2022-02-14 23:13 ` Alan Maguire
2022-02-15 0:29 ` Alexei Starovoitov
1 sibling, 1 reply; 16+ messages in thread
From: Alan Maguire @ 2022-02-14 23:13 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andrii Nakryiko, Andrii Nakryiko, bpf, Alexei Starovoitov,
Daniel Borkmann, Kernel Team, Alan Maguire
On Mon, 14 Feb 2022, Alexei Starovoitov wrote:
> Alan,
> can you demo your "okprobe" feature based on this api?
> Any rough patches would do.
sure; see below. Requires Andrii's v3 patches to be applied first,
and demonstrates okprobe handling for a kprobe function that exists
and one that doesn't - the important thing is skeleton attach
can succeed even when a function is missing (as it would be if
the associated module wasn't loaded).
> The "o" handling will be done in which callback?
>
We set program type at init and do custom attach using the function
name (specified in the program section after the "okprobe" prefix).
However we make sure to catch -ENOENT attach failures and return 0
with a NULL link so skeleton attach can proceed.
From 9bbd615b71f8f59ff743608bc86d7a2a346da2a8 Mon Sep 17 00:00:00 2001
From: Alan Maguire <alan.maguire@oracle.com>
Date: Mon, 14 Feb 2022 22:57:56 +0000
Subject: [PATCH bpf-next] selftests/bpf: demonstrate further use of custom
SEC() handling
Register and use SEC() handling for "okprobe/" kprobe programs
(Optional kprobe) which should be attached as kprobes but
critically should not stop skeleton loading if attach fails
due to non-existence of the to-be-probed function. This mode
of SEC() handling is useful for tracing module functions
where the module might not be loaded.
Note - this patch is based on the v3 of Andrii's section
handling patches [1] and these need to be applied for it to
apply cleanly.
[1] https://lore.kernel.org/bpf/20220211211450.2224877-1-andrii@kernel.org/
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
.../selftests/bpf/prog_tests/custom_sec_handlers.c | 33 ++++++++++++++++++++++
.../selftests/bpf/progs/test_custom_sec_handlers.c | 17 +++++++++++
2 files changed, 50 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
index 2826452..5da1375 100644
--- a/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
+++ b/tools/testing/selftests/bpf/prog_tests/custom_sec_handlers.c
@@ -9,11 +9,14 @@
#define COOKIE_CUSTOM 3
#define COOKIE_FALLBACK 4
#define COOKIE_KPROBE 5
+#define COOKIE_OKPROBE 6
static int custom_init_prog(struct bpf_program *prog, long cookie)
{
if (cookie == COOKIE_ABC1)
bpf_program__set_autoload(prog, false);
+ else if (cookie == COOKIE_OKPROBE)
+ bpf_program__set_type(prog, BPF_PROG_TYPE_KPROBE);
return 0;
}
@@ -32,6 +35,8 @@ static int custom_preload_prog(struct bpf_program *prog,
static int custom_attach_prog(const struct bpf_program *prog, long cookie,
struct bpf_link **link)
{
+ const char *func_name;
+
switch (cookie) {
case COOKIE_ABC2:
*link = bpf_program__attach_raw_tracepoint(prog, "sys_enter");
@@ -39,6 +44,15 @@ static int custom_attach_prog(const struct bpf_program *prog, long cookie,
case COOKIE_CUSTOM:
*link = bpf_program__attach_tracepoint(prog, "syscalls", "sys_enter_nanosleep");
return libbpf_get_error(*link);
+ case COOKIE_OKPROBE:
+ func_name = bpf_program__section_name(prog) + strlen("okprobe/");
+ *link = bpf_program__attach_kprobe(prog, false, func_name);
+ /* it's ok if func doesn't exist. */
+ if (libbpf_get_error(*link) == -ENOENT) {
+ *link = NULL;
+ return 0;
+ }
+ return libbpf_get_error(*link);
case COOKIE_KPROBE:
case COOKIE_FALLBACK:
/* no auto-attach for SEC("xyz") and SEC("kprobe") */
@@ -55,6 +69,7 @@ static int custom_attach_prog(const struct bpf_program *prog, long cookie,
static int custom_id;
static int fallback_id;
static int kprobe_id;
+static int okprobe_id;
__attribute__((constructor))
static void register_sec_handlers(void)
@@ -77,10 +92,18 @@ static void register_sec_handlers(void)
.preload_fn = NULL,
.attach_fn = custom_attach_prog,
);
+ LIBBPF_OPTS(libbpf_prog_handler_opts, okprobe_opts,
+ .cookie = COOKIE_OKPROBE,
+ .init_fn = custom_init_prog,
+ .preload_fn = NULL,
+ .attach_fn = custom_attach_prog,
+ );
+
abc1_id = libbpf_register_prog_handler("abc", BPF_PROG_TYPE_RAW_TRACEPOINT, 0, &abc1_opts);
abc2_id = libbpf_register_prog_handler("abc/", BPF_PROG_TYPE_RAW_TRACEPOINT, 0, &abc2_opts);
custom_id = libbpf_register_prog_handler("custom+", BPF_PROG_TYPE_TRACEPOINT, 0, &custom_opts);
+ okprobe_id = libbpf_register_prog_handler("okprobe/", BPF_PROG_TYPE_KPROBE, 0, &okprobe_opts);
}
__attribute__((destructor))
@@ -89,6 +112,7 @@ static void unregister_sec_handlers(void)
libbpf_unregister_prog_handler(abc1_id);
libbpf_unregister_prog_handler(abc2_id);
libbpf_unregister_prog_handler(custom_id);
+ libbpf_unregister_prog_handler(okprobe_id);
}
void test_custom_sec_handlers(void)
@@ -104,6 +128,7 @@ void test_custom_sec_handlers(void)
ASSERT_GT(abc1_id, 0, "abc1_id");
ASSERT_GT(abc2_id, 0, "abc2_id");
ASSERT_GT(custom_id, 0, "custom_id");
+ ASSERT_GT(okprobe_id, 0, "okprobe_id");
/* override libbpf's handle of SEC("kprobe/...") but also allow pure
* SEC("kprobe") due to "kprobe+" specifier. Register it as
@@ -138,6 +163,8 @@ void test_custom_sec_handlers(void)
ASSERT_EQ(bpf_program__type(skel->progs.custom2), BPF_PROG_TYPE_TRACEPOINT, "custom2_type");
ASSERT_EQ(bpf_program__type(skel->progs.kprobe1), BPF_PROG_TYPE_TRACEPOINT, "kprobe1_type");
ASSERT_EQ(bpf_program__type(skel->progs.xyz), BPF_PROG_TYPE_SYSCALL, "xyz_type");
+ ASSERT_EQ(bpf_program__type(skel->progs.kprobe2), BPF_PROG_TYPE_KPROBE, "kprobe2_type");
+ ASSERT_EQ(bpf_program__type(skel->progs.kprobe3), BPF_PROG_TYPE_KPROBE, "kprobe3_type");
skel->rodata->my_pid = getpid();
@@ -167,6 +194,12 @@ void test_custom_sec_handlers(void)
ASSERT_FALSE(skel->bss->kprobe1_called, "kprobe1_called");
/* SEC("xyz") shouldn't be auto-attached */
ASSERT_FALSE(skel->bss->xyz_called, "xyz_called");
+ /* SEC("okprobe/sys_nanosleep") should be auto-attached */
+ ASSERT_TRUE(skel->bss->kprobe2_called, "kprobe2_called");
+ /* SEC("okprobe/nonexistent_function") shouldn't attach, but
+ * this shouldn't prevent overall skeleton attach.
+ */
+ ASSERT_FALSE(skel->bss->kprobe3_called, "kprobe3_called");
cleanup:
test_custom_sec_handlers__destroy(skel);
diff --git a/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c b/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
index 4061f70..6e9e051f 100644
--- a/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
+++ b/tools/testing/selftests/bpf/progs/test_custom_sec_handlers.c
@@ -4,6 +4,7 @@
#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
const volatile int my_pid;
@@ -12,6 +13,8 @@
bool custom1_called;
bool custom2_called;
bool kprobe1_called;
+bool kprobe2_called;
+bool kprobe3_called;
bool xyz_called;
SEC("abc")
@@ -60,4 +63,18 @@ int xyz(void *ctx)
return 0;
}
+SEC("okprobe/" SYS_PREFIX "sys_nanosleep")
+int kprobe2(void *ctx)
+{
+ kprobe2_called = true;
+ return 0;
+}
+
+SEC("okprobe/nonexistent_function")
+int kprobe3(void *ctx)
+{
+ kprobe3_called = true;
+ return 0;
+}
+
char _license[] SEC("license") = "GPL";
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-14 23:13 ` Alan Maguire
@ 2022-02-15 0:29 ` Alexei Starovoitov
2022-02-15 5:22 ` Andrii Nakryiko
0 siblings, 1 reply; 16+ messages in thread
From: Alexei Starovoitov @ 2022-02-15 0:29 UTC (permalink / raw)
To: Alan Maguire
Cc: Andrii Nakryiko, Andrii Nakryiko, bpf, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
On Mon, Feb 14, 2022 at 3:14 PM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On Mon, 14 Feb 2022, Alexei Starovoitov wrote:
>
> > Alan,
> > can you demo your "okprobe" feature based on this api?
> > Any rough patches would do.
>
> sure; see below. Requires Andrii's v3 patches to be applied first,
> and demonstrates okprobe handling for a kprobe function that exists
> and one that doesn't - the important thing is skeleton attach
> can succeed even when a function is missing (as it would be if
> the associated module wasn't loaded).
>
> > The "o" handling will be done in which callback?
> >
>
> We set program type at init and do custom attach using the function
> name (specified in the program section after the "okprobe" prefix).
> However we make sure to catch -ENOENT attach failures and return 0
> with a NULL link so skeleton attach can proceed.
>
> From 9bbd615b71f8f59ff743608bc86d7a2a346da2a8 Mon Sep 17 00:00:00 2001
> From: Alan Maguire <alan.maguire@oracle.com>
> Date: Mon, 14 Feb 2022 22:57:56 +0000
> Subject: [PATCH bpf-next] selftests/bpf: demonstrate further use of custom
> SEC() handling
>
> Register and use SEC() handling for "okprobe/" kprobe programs
> (Optional kprobe) which should be attached as kprobes but
> critically should not stop skeleton loading if attach fails
> due to non-existence of the to-be-probed function. This mode
> of SEC() handling is useful for tracing module functions
> where the module might not be loaded.
>
> Note - this patch is based on the v3 of Andrii's section
> handling patches [1] and these need to be applied for it to
> apply cleanly.
>
> [1] https://lore.kernel.org/bpf/20220211211450.2224877-1-andrii@kernel.org/
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
Thanks. The patch certainly helps to understand the api usage.
> static int custom_init_prog(struct bpf_program *prog, long cookie)
> {
> if (cookie == COOKIE_ABC1)
> bpf_program__set_autoload(prog, false);
> + else if (cookie == COOKIE_OKPROBE)
> + bpf_program__set_type(prog, BPF_PROG_TYPE_KPROBE);
I think bpf_program__set_type() would have worked
from the prepare_load_fn callback as well.
What would be a recommended way of setting it?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest
2022-02-15 0:29 ` Alexei Starovoitov
@ 2022-02-15 5:22 ` Andrii Nakryiko
0 siblings, 0 replies; 16+ messages in thread
From: Andrii Nakryiko @ 2022-02-15 5:22 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alan Maguire, Andrii Nakryiko, bpf, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
On Mon, Feb 14, 2022 at 4:29 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Feb 14, 2022 at 3:14 PM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > On Mon, 14 Feb 2022, Alexei Starovoitov wrote:
> >
> > > Alan,
> > > can you demo your "okprobe" feature based on this api?
> > > Any rough patches would do.
> >
> > sure; see below. Requires Andrii's v3 patches to be applied first,
> > and demonstrates okprobe handling for a kprobe function that exists
> > and one that doesn't - the important thing is skeleton attach
> > can succeed even when a function is missing (as it would be if
> > the associated module wasn't loaded).
> >
> > > The "o" handling will be done in which callback?
> > >
> >
> > We set program type at init and do custom attach using the function
> > name (specified in the program section after the "okprobe" prefix).
> > However we make sure to catch -ENOENT attach failures and return 0
> > with a NULL link so skeleton attach can proceed.
> >
> > From 9bbd615b71f8f59ff743608bc86d7a2a346da2a8 Mon Sep 17 00:00:00 2001
> > From: Alan Maguire <alan.maguire@oracle.com>
> > Date: Mon, 14 Feb 2022 22:57:56 +0000
> > Subject: [PATCH bpf-next] selftests/bpf: demonstrate further use of custom
> > SEC() handling
> >
> > Register and use SEC() handling for "okprobe/" kprobe programs
> > (Optional kprobe) which should be attached as kprobes but
> > critically should not stop skeleton loading if attach fails
> > due to non-existence of the to-be-probed function. This mode
> > of SEC() handling is useful for tracing module functions
> > where the module might not be loaded.
> >
> > Note - this patch is based on the v3 of Andrii's section
> > handling patches [1] and these need to be applied for it to
> > apply cleanly.
> >
> > [1] https://lore.kernel.org/bpf/20220211211450.2224877-1-andrii@kernel.org/
> >
> > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>
> Thanks. The patch certainly helps to understand the api usage.
>
> > static int custom_init_prog(struct bpf_program *prog, long cookie)
> > {
> > if (cookie == COOKIE_ABC1)
> > bpf_program__set_autoload(prog, false);
> > + else if (cookie == COOKIE_OKPROBE)
> > + bpf_program__set_type(prog, BPF_PROG_TYPE_KPROBE);
>
> I think bpf_program__set_type() would have worked
> from the prepare_load_fn callback as well.
>
> What would be a recommended way of setting it?
Currently, bpf_program__set_type() could be done only from init
callback, as by the time preload_fn is called, libbpf already captured
prog_type. This can be adjusted, but I think it's cleaner to do it in
init callback, so that user code, if it chooses to iterate programs
with bpf_object__for_each_program() and such, would get correct
information before bpf_object__load().
I also just checked bpf_program__set_type(), it seems like it's void
function, but it should certainly fail, if called after
bpf_object__load() phase. Not sure if it's an ABI-breaking change to
change it to int, but would be good to adjust this. Same for
bpf_program__set_expected_attach_type().
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2022-02-15 5:22 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-11 21:14 [PATCH v3 bpf-next 0/3] libbpf: support custom SEC() handlers Andrii Nakryiko
2022-02-11 21:14 ` [PATCH v3 bpf-next 1/3] libbpf: allow BPF program auto-attach handlers to bail out Andrii Nakryiko
2022-02-11 21:14 ` [PATCH v3 bpf-next 2/3] libbpf: support custom SEC() handlers Andrii Nakryiko
2022-02-14 10:34 ` Alan Maguire
2022-02-11 21:14 ` [PATCH v3 bpf-next 3/3] selftests/bpf: add custom SEC() handling selftest Andrii Nakryiko
2022-02-11 23:13 ` Alexei Starovoitov
2022-02-11 23:31 ` Andrii Nakryiko
2022-02-12 0:18 ` Alexei Starovoitov
2022-02-12 1:16 ` Andrii Nakryiko
2022-02-14 11:13 ` Alan Maguire
2022-02-14 17:27 ` Alexei Starovoitov
2022-02-14 19:55 ` Andrii Nakryiko
2022-02-14 23:13 ` Alan Maguire
2022-02-15 0:29 ` Alexei Starovoitov
2022-02-15 5:22 ` Andrii Nakryiko
2022-02-14 10:36 ` Alan Maguire
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.