[PATCH bpf-next 0/3] Add target-less tracing SEC() definitions

[PATCH bpf-next 0/3] Add target-less tracing SEC() definitions

[Andrii Nakryiko]

Allow specifying "target-less" SEC() definitions for tracing BPF programs,
both non-BTF-backed (kprobes, tracepoints, raw tracepoints) and BTF-backed
(fentry/fexit, iter, lsm, etc).

There are various situations where attach target cannot be known at
compilation time, so libbpf's insistence on specifying something leads to
users having to add random test like SEC("kprobe/whatever") and then
specifying correct target at runtime using APIs like
bpf_program__attach_kprobe().

So this patch set improves ergonomics by allowing simple SEC() definitions
that define BPF program type and nothing else. Such programs won't be
auto-attachable, of course, but they also won't fail skeleton auto-attachment,
just like we do this for uprobes.

Andrii Nakryiko (3):
  libbpf: allow "incomplete" basic tracing SEC() definitions
  libbpf: support target-less SEC() definitions for BTF-backed programs
  selftests/bpf: use target-less SEC() definitions in various tests

 tools/lib/bpf/libbpf.c                        | 118 ++++++++++++------
 .../selftests/bpf/prog_tests/attach_probe.c   |  10 ++
 .../bpf/prog_tests/kprobe_multi_test.c        |  14 +--
 .../selftests/bpf/progs/kprobe_multi.c        |  14 +++
 .../selftests/bpf/progs/test_attach_probe.c   |  23 +++-
 .../selftests/bpf/progs/test_module_attach.c  |   2 +-
 6 files changed, 135 insertions(+), 46 deletions(-)

-- 
2.30.2

[PATCH bpf-next 1/3] libbpf: allow "incomplete" basic tracing SEC() definitions

[Andrii Nakryiko]

In a lot of cases the target of kprobe/kretprobe, tracepoint, raw
tracepoint, etc BPF program might not be known at the compilation time
and will be discovered at runtime. This was always a supported case by
libbpf, with APIs like bpf_program__attach_{kprobe,tracepoint,etc}()
accepting full target definition, regardless of what was defined in
SEC() definition in BPF source code.

Unfortunately, up till now libbpf still enforced users to specify at
least something for the fake target, e.g., SEC("kprobe/whatever"), which
is cumbersome and somewhat misleading.

This patch allows target-less SEC() definitions for basic tracing BPF
program types:
  - kprobe/kretprobe;
  - multi-kprobe/multi-kretprobe;
  - tracepoints;
  - raw tracepoints.

Such target-less SEC() definitions are meant to specify declaratively
proper BPF program type only. Attachment of them will have to be handled
programmatically using correct APIs. As such, skeleton's auto-attachment
of such BPF programs is skipped and generic bpf_program__attach() will
fail, if attempted, due to the lack of enough target information.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c | 69 +++++++++++++++++++++++++++++++-----------
 1 file changed, 51 insertions(+), 18 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 9deb1fc67f19..81911a1e1f3e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8668,22 +8668,22 @@ static const struct bpf_sec_def section_defs[] = {
 	SEC_DEF("socket",		SOCKET_FILTER, 0, SEC_NONE | SEC_SLOPPY_PFX),
 	SEC_DEF("sk_reuseport/migrate",	SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
 	SEC_DEF("sk_reuseport",		SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
-	SEC_DEF("kprobe/",		KPROBE,	0, SEC_NONE, attach_kprobe),
+	SEC_DEF("kprobe+",		KPROBE,	0, SEC_NONE, attach_kprobe),
 	SEC_DEF("uprobe+",		KPROBE,	0, SEC_NONE, attach_uprobe),
-	SEC_DEF("kretprobe/",		KPROBE, 0, SEC_NONE, attach_kprobe),
+	SEC_DEF("kretprobe+",		KPROBE, 0, SEC_NONE, attach_kprobe),
 	SEC_DEF("uretprobe+",		KPROBE, 0, SEC_NONE, attach_uprobe),
-	SEC_DEF("kprobe.multi/",	KPROBE,	BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
-	SEC_DEF("kretprobe.multi/",	KPROBE,	BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
+	SEC_DEF("kprobe.multi+",	KPROBE,	BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
+	SEC_DEF("kretprobe.multi+",	KPROBE,	BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
 	SEC_DEF("usdt+",		KPROBE,	0, SEC_NONE, attach_usdt),
 	SEC_DEF("tc",			SCHED_CLS, 0, SEC_NONE),
 	SEC_DEF("classifier",		SCHED_CLS, 0, SEC_NONE | SEC_SLOPPY_PFX | SEC_DEPRECATED),
 	SEC_DEF("action",		SCHED_ACT, 0, SEC_NONE | SEC_SLOPPY_PFX),
-	SEC_DEF("tracepoint/",		TRACEPOINT, 0, SEC_NONE, attach_tp),
-	SEC_DEF("tp/",			TRACEPOINT, 0, SEC_NONE, attach_tp),
-	SEC_DEF("raw_tracepoint/",	RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
-	SEC_DEF("raw_tp/",		RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
-	SEC_DEF("raw_tracepoint.w/",	RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
-	SEC_DEF("raw_tp.w/",		RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
+	SEC_DEF("tracepoint+",		TRACEPOINT, 0, SEC_NONE, attach_tp),
+	SEC_DEF("tp+",			TRACEPOINT, 0, SEC_NONE, attach_tp),
+	SEC_DEF("raw_tracepoint+",	RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
+	SEC_DEF("raw_tp+",		RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
+	SEC_DEF("raw_tracepoint.w+",	RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
+	SEC_DEF("raw_tp.w+",		RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
 	SEC_DEF("tp_btf/",		TRACING, BPF_TRACE_RAW_TP, SEC_ATTACH_BTF, attach_trace),
 	SEC_DEF("fentry/",		TRACING, BPF_TRACE_FENTRY, SEC_ATTACH_BTF, attach_trace),
 	SEC_DEF("fmod_ret/",		TRACING, BPF_MODIFY_RETURN, SEC_ATTACH_BTF, attach_trace),
@@ -10411,6 +10411,12 @@ static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf
 	char *func;
 	int n;
 
+	*link = NULL;
+
+	/* no auto-attach for SEC("kprobe") and SEC("kretprobe") */
+	if (strcmp(prog->sec_name, "kprobe") == 0 || strcmp(prog->sec_name, "kretprobe") == 0)
+		return 0;
+
 	opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe/");
 	if (opts.retprobe)
 		func_name = prog->sec_name + sizeof("kretprobe/") - 1;
@@ -10441,6 +10447,13 @@ static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, stru
 	char *pattern;
 	int n;
 
+	*link = NULL;
+
+	/* no auto-attach for SEC("kprobe.multi") and SEC("kretprobe.multi") */
+	if (strcmp(prog->sec_name, "kprobe.multi") == 0 ||
+	    strcmp(prog->sec_name, "kretprobe.multi") == 0)
+		return 0;
+
 	opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe.multi/");
 	if (opts.retprobe)
 		spec = prog->sec_name + sizeof("kretprobe.multi/") - 1;
@@ -11145,6 +11158,12 @@ static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_lin
 	if (!sec_name)
 		return -ENOMEM;
 
+	*link = NULL;
+
+	/* no auto-attach for SEC("tp") or SEC("tracepoint") */
+	if (strcmp(prog->sec_name, "tp") == 0 || strcmp(prog->sec_name, "tracepoint") == 0)
+		return 0;
+
 	/* extract "tp/<category>/<name>" or "tracepoint/<category>/<name>" */
 	if (str_has_pfx(prog->sec_name, "tp/"))
 		tp_cat = sec_name + sizeof("tp/") - 1;
@@ -11196,20 +11215,34 @@ struct bpf_link *bpf_program__attach_raw_tracepoint(const struct bpf_program *pr
 static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link)
 {
 	static const char *const prefixes[] = {
-		"raw_tp/",
-		"raw_tracepoint/",
-		"raw_tp.w/",
-		"raw_tracepoint.w/",
+		"raw_tp",
+		"raw_tracepoint",
+		"raw_tp.w",
+		"raw_tracepoint.w",
 	};
 	size_t i;
 	const char *tp_name = NULL;
 
+	*link = NULL;
+
 	for (i = 0; i < ARRAY_SIZE(prefixes); i++) {
-		if (str_has_pfx(prog->sec_name, prefixes[i])) {
-			tp_name = prog->sec_name + strlen(prefixes[i]);
-			break;
-		}
+		size_t pfx_len;
+
+		if (!str_has_pfx(prog->sec_name, prefixes[i]))
+			continue;
+
+		pfx_len = strlen(prefixes[i]);
+		/* no auto-attach case of, e.g., SEC("raw_tp") */
+		if (prog->sec_name[pfx_len] == '\0')
+			return 0;
+
+		if (prog->sec_name[pfx_len] != '/')
+			continue;
+
+		tp_name = prog->sec_name + pfx_len + 1;
+		break;
 	}
+
 	if (!tp_name) {
 		pr_warn("prog '%s': invalid section name '%s'\n",
 			prog->name, prog->sec_name);
-- 
2.30.2

[PATCH bpf-next 2/3] libbpf: support target-less SEC() definitions for BTF-backed programs

[Andrii Nakryiko]

Similar to previous patch, support target-less definitions like
SEC("fentry"), SEC("freplace"), etc. For such BTF-backed program types
it is expected that user will specify BTF target programmatically at
runtime using bpf_program__set_attach_target() *before* load phase. If
not, libbpf will report this as an error.

Aslo use SEC_ATTACH_BTF flag instead of explicitly listing a set of
types that are expected to require attach_btf_id. This was an accidental
omission during custom SEC() support refactoring.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c | 49 +++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 81911a1e1f3e..76c0b3a5cde9 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6628,17 +6628,32 @@ static int libbpf_prepare_prog_load(struct bpf_program *prog,
 	if (prog->type == BPF_PROG_TYPE_XDP && (def & SEC_XDP_FRAGS))
 		opts->prog_flags |= BPF_F_XDP_HAS_FRAGS;
 
-	if (def & SEC_DEPRECATED)
+	if (def & SEC_DEPRECATED) {
 		pr_warn("SEC(\"%s\") is deprecated, please see https://github.com/libbpf/libbpf/wiki/Libbpf-1.0-migration-guide#bpf-program-sec-annotation-deprecations for details\n",
 			prog->sec_name);
+	}
 
-	if ((prog->type == BPF_PROG_TYPE_TRACING ||
-	     prog->type == BPF_PROG_TYPE_LSM ||
-	     prog->type == BPF_PROG_TYPE_EXT) && !prog->attach_btf_id) {
+	if ((def & SEC_ATTACH_BTF) && !prog->attach_btf_id) {
 		int btf_obj_fd = 0, btf_type_id = 0, err;
 		const char *attach_name;
 
-		attach_name = strchr(prog->sec_name, '/') + 1;
+		attach_name = strchr(prog->sec_name, '/');
+		if (!attach_name) {
+			/* if BPF program is annotated with just SEC("fentry")
+			 * (or similar) without declaratively specifying
+			 * target, then it is expected that target will be
+			 * specified with bpf_program__set_attach_target() at
+			 * runtime before BPF object load step. If not, then
+			 * there is nothing to load into the kernel as BPF
+			 * verifier won't be able to validate BPF program
+			 * correctness anyways.
+			 */
+			pr_warn("prog '%s': no BTF-based attach target is specified, use bpf_program__set_attach_target()\n",
+				prog->name);
+			return -EINVAL;
+		}
+		attach_name++; /* skip over / */
+
 		err = libbpf_find_attach_btf_id(prog, attach_name, &btf_obj_fd, &btf_type_id);
 		if (err)
 			return err;
@@ -8684,18 +8699,18 @@ static const struct bpf_sec_def section_defs[] = {
 	SEC_DEF("raw_tp+",		RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
 	SEC_DEF("raw_tracepoint.w+",	RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
 	SEC_DEF("raw_tp.w+",		RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
-	SEC_DEF("tp_btf/",		TRACING, BPF_TRACE_RAW_TP, SEC_ATTACH_BTF, attach_trace),
-	SEC_DEF("fentry/",		TRACING, BPF_TRACE_FENTRY, SEC_ATTACH_BTF, attach_trace),
-	SEC_DEF("fmod_ret/",		TRACING, BPF_MODIFY_RETURN, SEC_ATTACH_BTF, attach_trace),
-	SEC_DEF("fexit/",		TRACING, BPF_TRACE_FEXIT, SEC_ATTACH_BTF, attach_trace),
-	SEC_DEF("fentry.s/",		TRACING, BPF_TRACE_FENTRY, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_trace),
-	SEC_DEF("fmod_ret.s/",		TRACING, BPF_MODIFY_RETURN, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_trace),
-	SEC_DEF("fexit.s/",		TRACING, BPF_TRACE_FEXIT, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_trace),
-	SEC_DEF("freplace/",		EXT, 0, SEC_ATTACH_BTF, attach_trace),
-	SEC_DEF("lsm/",			LSM, BPF_LSM_MAC, SEC_ATTACH_BTF, attach_lsm),
-	SEC_DEF("lsm.s/",		LSM, BPF_LSM_MAC, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_lsm),
-	SEC_DEF("iter/",		TRACING, BPF_TRACE_ITER, SEC_ATTACH_BTF, attach_iter),
-	SEC_DEF("iter.s/",		TRACING, BPF_TRACE_ITER, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_iter),
+	SEC_DEF("tp_btf+",		TRACING, BPF_TRACE_RAW_TP, SEC_ATTACH_BTF, attach_trace),
+	SEC_DEF("fentry+",		TRACING, BPF_TRACE_FENTRY, SEC_ATTACH_BTF, attach_trace),
+	SEC_DEF("fmod_ret+",		TRACING, BPF_MODIFY_RETURN, SEC_ATTACH_BTF, attach_trace),
+	SEC_DEF("fexit+",		TRACING, BPF_TRACE_FEXIT, SEC_ATTACH_BTF, attach_trace),
+	SEC_DEF("fentry.s+",		TRACING, BPF_TRACE_FENTRY, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_trace),
+	SEC_DEF("fmod_ret.s+",		TRACING, BPF_MODIFY_RETURN, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_trace),
+	SEC_DEF("fexit.s+",		TRACING, BPF_TRACE_FEXIT, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_trace),
+	SEC_DEF("freplace+",		EXT, 0, SEC_ATTACH_BTF, attach_trace),
+	SEC_DEF("lsm+",			LSM, BPF_LSM_MAC, SEC_ATTACH_BTF, attach_lsm),
+	SEC_DEF("lsm.s+",		LSM, BPF_LSM_MAC, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_lsm),
+	SEC_DEF("iter+",		TRACING, BPF_TRACE_ITER, SEC_ATTACH_BTF, attach_iter),
+	SEC_DEF("iter.s+",		TRACING, BPF_TRACE_ITER, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_iter),
 	SEC_DEF("syscall",		SYSCALL, 0, SEC_SLEEPABLE),
 	SEC_DEF("xdp.frags/devmap",	XDP, BPF_XDP_DEVMAP, SEC_XDP_FRAGS),
 	SEC_DEF("xdp/devmap",		XDP, BPF_XDP_DEVMAP, SEC_ATTACHABLE),
-- 
2.30.2

[PATCH bpf-next 3/3] selftests/bpf: use target-less SEC() definitions in various tests

[Andrii Nakryiko]

Add new or modify existing SEC() definitions to be target-less and
validate that libbpf handles such program definitions correctly.

For kprobe/kretprobe we also add explicit test that generic
bpf_program__attach() works in cases when kprobe definition contains
proper target. It wasn't previously tested as selftests code always
explicitly specified the target regardless.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 .../selftests/bpf/prog_tests/attach_probe.c   | 10 ++++++++
 .../bpf/prog_tests/kprobe_multi_test.c        | 14 +++++------
 .../selftests/bpf/progs/kprobe_multi.c        | 14 +++++++++++
 .../selftests/bpf/progs/test_attach_probe.c   | 23 ++++++++++++++++---
 .../selftests/bpf/progs/test_module_attach.c  |  2 +-
 5 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
index c0c6d410751d..08c0601b3e84 100644
--- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -55,6 +55,7 @@ void test_attach_probe(void)
 	if (!ASSERT_OK_PTR(skel->bss, "check_bss"))
 		goto cleanup;
 
+	/* manual-attach kprobe/kretprobe */
 	kprobe_link = bpf_program__attach_kprobe(skel->progs.handle_kprobe,
 						 false /* retprobe */,
 						 SYS_NANOSLEEP_KPROBE_NAME);
@@ -69,6 +70,13 @@ void test_attach_probe(void)
 		goto cleanup;
 	skel->links.handle_kretprobe = kretprobe_link;
 
+	/* auto-attachable kprobe and kretprobe */
+	skel->links.handle_kprobe_auto = bpf_program__attach(skel->progs.handle_kprobe_auto);
+	ASSERT_OK_PTR(skel->links.handle_kprobe_auto, "attach_kprobe_auto");
+
+	skel->links.handle_kretprobe_auto = bpf_program__attach(skel->progs.handle_kretprobe_auto);
+	ASSERT_OK_PTR(skel->links.handle_kretprobe_auto, "attach_kretprobe_auto");
+
 	if (!legacy)
 		ASSERT_EQ(uprobe_ref_ctr, 0, "uprobe_ref_ctr_before");
 
@@ -157,7 +165,9 @@ void test_attach_probe(void)
 	trigger_func2();
 
 	ASSERT_EQ(skel->bss->kprobe_res, 1, "check_kprobe_res");
+	ASSERT_EQ(skel->bss->kprobe2_res, 11, "check_kprobe_auto_res");
 	ASSERT_EQ(skel->bss->kretprobe_res, 2, "check_kretprobe_res");
+	ASSERT_EQ(skel->bss->kretprobe2_res, 22, "check_kretprobe_auto_res");
 	ASSERT_EQ(skel->bss->uprobe_res, 3, "check_uprobe_res");
 	ASSERT_EQ(skel->bss->uretprobe_res, 4, "check_uretprobe_res");
 	ASSERT_EQ(skel->bss->uprobe_byname_res, 5, "check_uprobe_byname_res");
diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index b9876b55fc0c..c56db65d4c15 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -140,14 +140,14 @@ test_attach_api(const char *pattern, struct bpf_kprobe_multi_opts *opts)
 		goto cleanup;
 
 	skel->bss->pid = getpid();
-	link1 = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe,
+	link1 = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
 						      pattern, opts);
 	if (!ASSERT_OK_PTR(link1, "bpf_program__attach_kprobe_multi_opts"))
 		goto cleanup;
 
 	if (opts) {
 		opts->retprobe = true;
-		link2 = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kretprobe,
+		link2 = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kretprobe_manual,
 							      pattern, opts);
 		if (!ASSERT_OK_PTR(link2, "bpf_program__attach_kprobe_multi_opts"))
 			goto cleanup;
@@ -232,7 +232,7 @@ static void test_attach_api_fails(void)
 	skel->bss->pid = getpid();
 
 	/* fail_1 - pattern and opts NULL */
-	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe,
+	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
 						     NULL, NULL);
 	if (!ASSERT_ERR_PTR(link, "fail_1"))
 		goto cleanup;
@@ -246,7 +246,7 @@ static void test_attach_api_fails(void)
 	opts.cnt = ARRAY_SIZE(syms);
 	opts.cookies = NULL;
 
-	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe,
+	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
 						     NULL, &opts);
 	if (!ASSERT_ERR_PTR(link, "fail_2"))
 		goto cleanup;
@@ -260,7 +260,7 @@ static void test_attach_api_fails(void)
 	opts.cnt = ARRAY_SIZE(syms);
 	opts.cookies = NULL;
 
-	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe,
+	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
 						     "ksys_*", &opts);
 	if (!ASSERT_ERR_PTR(link, "fail_3"))
 		goto cleanup;
@@ -274,7 +274,7 @@ static void test_attach_api_fails(void)
 	opts.cnt = ARRAY_SIZE(syms);
 	opts.cookies = NULL;
 
-	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe,
+	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
 						     "ksys_*", &opts);
 	if (!ASSERT_ERR_PTR(link, "fail_4"))
 		goto cleanup;
@@ -288,7 +288,7 @@ static void test_attach_api_fails(void)
 	opts.cnt = 0;
 	opts.cookies = cookies;
 
-	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe,
+	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
 						     "ksys_*", &opts);
 	if (!ASSERT_ERR_PTR(link, "fail_5"))
 		goto cleanup;
diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi.c b/tools/testing/selftests/bpf/progs/kprobe_multi.c
index 600be50800f8..93510f4f0f3a 100644
--- a/tools/testing/selftests/bpf/progs/kprobe_multi.c
+++ b/tools/testing/selftests/bpf/progs/kprobe_multi.c
@@ -98,3 +98,17 @@ int test_kretprobe(struct pt_regs *ctx)
 	kprobe_multi_check(ctx, true);
 	return 0;
 }
+
+SEC("kprobe.multi")
+int test_kprobe_manual(struct pt_regs *ctx)
+{
+	kprobe_multi_check(ctx, false);
+	return 0;
+}
+
+SEC("kretprobe.multi")
+int test_kretprobe_manual(struct pt_regs *ctx)
+{
+	kprobe_multi_check(ctx, true);
+	return 0;
+}
diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
index af994d16bb10..ce9acf4db8d2 100644
--- a/tools/testing/selftests/bpf/progs/test_attach_probe.c
+++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
@@ -5,9 +5,12 @@
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
 
 int kprobe_res = 0;
+int kprobe2_res = 0;
 int kretprobe_res = 0;
+int kretprobe2_res = 0;
 int uprobe_res = 0;
 int uretprobe_res = 0;
 int uprobe_byname_res = 0;
@@ -15,20 +18,34 @@ int uretprobe_byname_res = 0;
 int uprobe_byname2_res = 0;
 int uretprobe_byname2_res = 0;
 
-SEC("kprobe/sys_nanosleep")
+SEC("kprobe")
 int handle_kprobe(struct pt_regs *ctx)
 {
 	kprobe_res = 1;
 	return 0;
 }
 
-SEC("kretprobe/sys_nanosleep")
-int BPF_KRETPROBE(handle_kretprobe)
+SEC("kprobe/" SYS_PREFIX "sys_nanosleep")
+int BPF_KPROBE(handle_kprobe_auto)
+{
+	kprobe2_res = 11;
+	return 0;
+}
+
+SEC("kretprobe")
+int handle_kretprobe(struct pt_regs *ctx)
 {
 	kretprobe_res = 2;
 	return 0;
 }
 
+SEC("kretprobe/" SYS_PREFIX "sys_nanosleep")
+int BPF_KRETPROBE(handle_kretprobe_auto)
+{
+	kretprobe2_res = 22;
+	return 0;
+}
+
 SEC("uprobe")
 int handle_uprobe(struct pt_regs *ctx)
 {
diff --git a/tools/testing/selftests/bpf/progs/test_module_attach.c b/tools/testing/selftests/bpf/progs/test_module_attach.c
index 50ce16d02da7..08628afedb77 100644
--- a/tools/testing/selftests/bpf/progs/test_module_attach.c
+++ b/tools/testing/selftests/bpf/progs/test_module_attach.c
@@ -64,7 +64,7 @@ int BPF_PROG(handle_fentry,
 
 __u32 fentry_manual_read_sz = 0;
 
-SEC("fentry/placeholder")
+SEC("fentry")
 int BPF_PROG(handle_fentry_manual,
 	     struct file *file, struct kobject *kobj,
 	     struct bin_attribute *bin_attr, char *buf, loff_t off, size_t len)
-- 
2.30.2

Re: [PATCH bpf-next 1/3] libbpf: allow "incomplete" basic tracing SEC() definitions

[Song Liu]

On Fri, Apr 8, 2022 at 1:34 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> In a lot of cases the target of kprobe/kretprobe, tracepoint, raw
> tracepoint, etc BPF program might not be known at the compilation time
> and will be discovered at runtime. This was always a supported case by
> libbpf, with APIs like bpf_program__attach_{kprobe,tracepoint,etc}()
> accepting full target definition, regardless of what was defined in
> SEC() definition in BPF source code.
>
> Unfortunately, up till now libbpf still enforced users to specify at
> least something for the fake target, e.g., SEC("kprobe/whatever"), which
> is cumbersome and somewhat misleading.
>
> This patch allows target-less SEC() definitions for basic tracing BPF
> program types:
>   - kprobe/kretprobe;
>   - multi-kprobe/multi-kretprobe;
>   - tracepoints;
>   - raw tracepoints.
>
> Such target-less SEC() definitions are meant to specify declaratively
> proper BPF program type only. Attachment of them will have to be handled
> programmatically using correct APIs. As such, skeleton's auto-attachment
> of such BPF programs is skipped and generic bpf_program__attach() will
> fail, if attempted, due to the lack of enough target information.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  tools/lib/bpf/libbpf.c | 69 +++++++++++++++++++++++++++++++-----------
>  1 file changed, 51 insertions(+), 18 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 9deb1fc67f19..81911a1e1f3e 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -8668,22 +8668,22 @@ static const struct bpf_sec_def section_defs[] = {
>         SEC_DEF("socket",               SOCKET_FILTER, 0, SEC_NONE | SEC_SLOPPY_PFX),
>         SEC_DEF("sk_reuseport/migrate", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
>         SEC_DEF("sk_reuseport",         SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
> -       SEC_DEF("kprobe/",              KPROBE, 0, SEC_NONE, attach_kprobe),
> +       SEC_DEF("kprobe+",              KPROBE, 0, SEC_NONE, attach_kprobe),
>         SEC_DEF("uprobe+",              KPROBE, 0, SEC_NONE, attach_uprobe),
> -       SEC_DEF("kretprobe/",           KPROBE, 0, SEC_NONE, attach_kprobe),
> +       SEC_DEF("kretprobe+",           KPROBE, 0, SEC_NONE, attach_kprobe),
>         SEC_DEF("uretprobe+",           KPROBE, 0, SEC_NONE, attach_uprobe),
> -       SEC_DEF("kprobe.multi/",        KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> -       SEC_DEF("kretprobe.multi/",     KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> +       SEC_DEF("kprobe.multi+",        KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> +       SEC_DEF("kretprobe.multi+",     KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
>         SEC_DEF("usdt+",                KPROBE, 0, SEC_NONE, attach_usdt),
>         SEC_DEF("tc",                   SCHED_CLS, 0, SEC_NONE),
>         SEC_DEF("classifier",           SCHED_CLS, 0, SEC_NONE | SEC_SLOPPY_PFX | SEC_DEPRECATED),
>         SEC_DEF("action",               SCHED_ACT, 0, SEC_NONE | SEC_SLOPPY_PFX),
> -       SEC_DEF("tracepoint/",          TRACEPOINT, 0, SEC_NONE, attach_tp),
> -       SEC_DEF("tp/",                  TRACEPOINT, 0, SEC_NONE, attach_tp),
> -       SEC_DEF("raw_tracepoint/",      RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
> -       SEC_DEF("raw_tp/",              RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
> -       SEC_DEF("raw_tracepoint.w/",    RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
> -       SEC_DEF("raw_tp.w/",            RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
> +       SEC_DEF("tracepoint+",          TRACEPOINT, 0, SEC_NONE, attach_tp),
> +       SEC_DEF("tp+",                  TRACEPOINT, 0, SEC_NONE, attach_tp),
> +       SEC_DEF("raw_tracepoint+",      RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
> +       SEC_DEF("raw_tp+",              RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
> +       SEC_DEF("raw_tracepoint.w+",    RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
> +       SEC_DEF("raw_tp.w+",            RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
>         SEC_DEF("tp_btf/",              TRACING, BPF_TRACE_RAW_TP, SEC_ATTACH_BTF, attach_trace),
>         SEC_DEF("fentry/",              TRACING, BPF_TRACE_FENTRY, SEC_ATTACH_BTF, attach_trace),
>         SEC_DEF("fmod_ret/",            TRACING, BPF_MODIFY_RETURN, SEC_ATTACH_BTF, attach_trace),
> @@ -10411,6 +10411,12 @@ static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf
>         char *func;
>         int n;
>
> +       *link = NULL;
> +
> +       /* no auto-attach for SEC("kprobe") and SEC("kretprobe") */
> +       if (strcmp(prog->sec_name, "kprobe") == 0 || strcmp(prog->sec_name, "kretprobe") == 0)
> +               return 0;
> +
>         opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe/");
>         if (opts.retprobe)
>                 func_name = prog->sec_name + sizeof("kretprobe/") - 1;
> @@ -10441,6 +10447,13 @@ static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, stru
>         char *pattern;
>         int n;
>
> +       *link = NULL;
> +
> +       /* no auto-attach for SEC("kprobe.multi") and SEC("kretprobe.multi") */
> +       if (strcmp(prog->sec_name, "kprobe.multi") == 0 ||
> +           strcmp(prog->sec_name, "kretprobe.multi") == 0)
> +               return 0;
> +
>         opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe.multi/");
>         if (opts.retprobe)
>                 spec = prog->sec_name + sizeof("kretprobe.multi/") - 1;
> @@ -11145,6 +11158,12 @@ static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_lin
>         if (!sec_name)
>                 return -ENOMEM;
>
> +       *link = NULL;
> +
> +       /* no auto-attach for SEC("tp") or SEC("tracepoint") */
> +       if (strcmp(prog->sec_name, "tp") == 0 || strcmp(prog->sec_name, "tracepoint") == 0)
> +               return 0;
> +
>         /* extract "tp/<category>/<name>" or "tracepoint/<category>/<name>" */
>         if (str_has_pfx(prog->sec_name, "tp/"))
>                 tp_cat = sec_name + sizeof("tp/") - 1;
> @@ -11196,20 +11215,34 @@ struct bpf_link *bpf_program__attach_raw_tracepoint(const struct bpf_program *pr
>  static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link)
>  {
>         static const char *const prefixes[] = {
> -               "raw_tp/",
> -               "raw_tracepoint/",
> -               "raw_tp.w/",
> -               "raw_tracepoint.w/",
> +               "raw_tp",
> +               "raw_tracepoint",
> +               "raw_tp.w",
> +               "raw_tracepoint.w",
>         };
>         size_t i;
>         const char *tp_name = NULL;
>
> +       *link = NULL;
> +
>         for (i = 0; i < ARRAY_SIZE(prefixes); i++) {
> -               if (str_has_pfx(prog->sec_name, prefixes[i])) {
> -                       tp_name = prog->sec_name + strlen(prefixes[i]);
> -                       break;
> -               }
> +               size_t pfx_len;
> +
> +               if (!str_has_pfx(prog->sec_name, prefixes[i]))
> +                       continue;
> +
> +               pfx_len = strlen(prefixes[i]);
> +               /* no auto-attach case of, e.g., SEC("raw_tp") */
> +               if (prog->sec_name[pfx_len] == '\0')
> +                       return 0;
> +
> +               if (prog->sec_name[pfx_len] != '/')
> +                       continue;

Maybe introduce a sec_has_pfx() function with tri-state return value:
1 for match with tp_name, 0, for match without tp_name, -1 for no match.

> +
> +               tp_name = prog->sec_name + pfx_len + 1;
> +               break;
>         }
> +
>         if (!tp_name) {
>                 pr_warn("prog '%s': invalid section name '%s'\n",
>                         prog->name, prog->sec_name);
> --
> 2.30.2
>

Re: [PATCH bpf-next 3/3] selftests/bpf: use target-less SEC() definitions in various tests

[Song Liu]

On Fri, Apr 8, 2022 at 1:34 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Add new or modify existing SEC() definitions to be target-less and
> validate that libbpf handles such program definitions correctly.
>
> For kprobe/kretprobe we also add explicit test that generic
> bpf_program__attach() works in cases when kprobe definition contains
> proper target. It wasn't previously tested as selftests code always
> explicitly specified the target regardless.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Song Liu <songliubraving@fb.com>

Re: [PATCH bpf-next 2/3] libbpf: support target-less SEC() definitions for BTF-backed programs

[Song Liu]

On Fri, Apr 8, 2022 at 1:34 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Similar to previous patch, support target-less definitions like
> SEC("fentry"), SEC("freplace"), etc. For such BTF-backed program types
> it is expected that user will specify BTF target programmatically at
> runtime using bpf_program__set_attach_target() *before* load phase. If
> not, libbpf will report this as an error.
>
> Aslo use SEC_ATTACH_BTF flag instead of explicitly listing a set of
> types that are expected to require attach_btf_id. This was an accidental
> omission during custom SEC() support refactoring.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Song Liu <songliubraving@fb.com>

Re: [PATCH bpf-next 1/3] libbpf: allow "incomplete" basic tracing SEC() definitions

[Andrii Nakryiko]

On Fri, Apr 8, 2022 at 1:46 PM Song Liu <song@kernel.org> wrote:
>
> On Fri, Apr 8, 2022 at 1:34 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> >
> > In a lot of cases the target of kprobe/kretprobe, tracepoint, raw
> > tracepoint, etc BPF program might not be known at the compilation time
> > and will be discovered at runtime. This was always a supported case by
> > libbpf, with APIs like bpf_program__attach_{kprobe,tracepoint,etc}()
> > accepting full target definition, regardless of what was defined in
> > SEC() definition in BPF source code.
> >
> > Unfortunately, up till now libbpf still enforced users to specify at
> > least something for the fake target, e.g., SEC("kprobe/whatever"), which
> > is cumbersome and somewhat misleading.
> >
> > This patch allows target-less SEC() definitions for basic tracing BPF
> > program types:
> >   - kprobe/kretprobe;
> >   - multi-kprobe/multi-kretprobe;
> >   - tracepoints;
> >   - raw tracepoints.
> >
> > Such target-less SEC() definitions are meant to specify declaratively
> > proper BPF program type only. Attachment of them will have to be handled
> > programmatically using correct APIs. As such, skeleton's auto-attachment
> > of such BPF programs is skipped and generic bpf_program__attach() will
> > fail, if attempted, due to the lack of enough target information.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> >  tools/lib/bpf/libbpf.c | 69 +++++++++++++++++++++++++++++++-----------
> >  1 file changed, 51 insertions(+), 18 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 9deb1fc67f19..81911a1e1f3e 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -8668,22 +8668,22 @@ static const struct bpf_sec_def section_defs[] = {
> >         SEC_DEF("socket",               SOCKET_FILTER, 0, SEC_NONE | SEC_SLOPPY_PFX),
> >         SEC_DEF("sk_reuseport/migrate", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
> >         SEC_DEF("sk_reuseport",         SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
> > -       SEC_DEF("kprobe/",              KPROBE, 0, SEC_NONE, attach_kprobe),
> > +       SEC_DEF("kprobe+",              KPROBE, 0, SEC_NONE, attach_kprobe),
> >         SEC_DEF("uprobe+",              KPROBE, 0, SEC_NONE, attach_uprobe),
> > -       SEC_DEF("kretprobe/",           KPROBE, 0, SEC_NONE, attach_kprobe),
> > +       SEC_DEF("kretprobe+",           KPROBE, 0, SEC_NONE, attach_kprobe),
> >         SEC_DEF("uretprobe+",           KPROBE, 0, SEC_NONE, attach_uprobe),
> > -       SEC_DEF("kprobe.multi/",        KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> > -       SEC_DEF("kretprobe.multi/",     KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> > +       SEC_DEF("kprobe.multi+",        KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> > +       SEC_DEF("kretprobe.multi+",     KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> >         SEC_DEF("usdt+",                KPROBE, 0, SEC_NONE, attach_usdt),
> >         SEC_DEF("tc",                   SCHED_CLS, 0, SEC_NONE),
> >         SEC_DEF("classifier",           SCHED_CLS, 0, SEC_NONE | SEC_SLOPPY_PFX | SEC_DEPRECATED),
> >         SEC_DEF("action",               SCHED_ACT, 0, SEC_NONE | SEC_SLOPPY_PFX),
> > -       SEC_DEF("tracepoint/",          TRACEPOINT, 0, SEC_NONE, attach_tp),
> > -       SEC_DEF("tp/",                  TRACEPOINT, 0, SEC_NONE, attach_tp),
> > -       SEC_DEF("raw_tracepoint/",      RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
> > -       SEC_DEF("raw_tp/",              RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
> > -       SEC_DEF("raw_tracepoint.w/",    RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
> > -       SEC_DEF("raw_tp.w/",            RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
> > +       SEC_DEF("tracepoint+",          TRACEPOINT, 0, SEC_NONE, attach_tp),
> > +       SEC_DEF("tp+",                  TRACEPOINT, 0, SEC_NONE, attach_tp),
> > +       SEC_DEF("raw_tracepoint+",      RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
> > +       SEC_DEF("raw_tp+",              RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
> > +       SEC_DEF("raw_tracepoint.w+",    RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
> > +       SEC_DEF("raw_tp.w+",            RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
> >         SEC_DEF("tp_btf/",              TRACING, BPF_TRACE_RAW_TP, SEC_ATTACH_BTF, attach_trace),
> >         SEC_DEF("fentry/",              TRACING, BPF_TRACE_FENTRY, SEC_ATTACH_BTF, attach_trace),
> >         SEC_DEF("fmod_ret/",            TRACING, BPF_MODIFY_RETURN, SEC_ATTACH_BTF, attach_trace),
> > @@ -10411,6 +10411,12 @@ static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf
> >         char *func;
> >         int n;
> >
> > +       *link = NULL;
> > +
> > +       /* no auto-attach for SEC("kprobe") and SEC("kretprobe") */
> > +       if (strcmp(prog->sec_name, "kprobe") == 0 || strcmp(prog->sec_name, "kretprobe") == 0)
> > +               return 0;
> > +
> >         opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe/");
> >         if (opts.retprobe)
> >                 func_name = prog->sec_name + sizeof("kretprobe/") - 1;
> > @@ -10441,6 +10447,13 @@ static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, stru
> >         char *pattern;
> >         int n;
> >
> > +       *link = NULL;
> > +
> > +       /* no auto-attach for SEC("kprobe.multi") and SEC("kretprobe.multi") */
> > +       if (strcmp(prog->sec_name, "kprobe.multi") == 0 ||
> > +           strcmp(prog->sec_name, "kretprobe.multi") == 0)
> > +               return 0;
> > +
> >         opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe.multi/");
> >         if (opts.retprobe)
> >                 spec = prog->sec_name + sizeof("kretprobe.multi/") - 1;
> > @@ -11145,6 +11158,12 @@ static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_lin
> >         if (!sec_name)
> >                 return -ENOMEM;
> >
> > +       *link = NULL;
> > +
> > +       /* no auto-attach for SEC("tp") or SEC("tracepoint") */
> > +       if (strcmp(prog->sec_name, "tp") == 0 || strcmp(prog->sec_name, "tracepoint") == 0)
> > +               return 0;
> > +
> >         /* extract "tp/<category>/<name>" or "tracepoint/<category>/<name>" */
> >         if (str_has_pfx(prog->sec_name, "tp/"))
> >                 tp_cat = sec_name + sizeof("tp/") - 1;
> > @@ -11196,20 +11215,34 @@ struct bpf_link *bpf_program__attach_raw_tracepoint(const struct bpf_program *pr
> >  static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> >  {
> >         static const char *const prefixes[] = {
> > -               "raw_tp/",
> > -               "raw_tracepoint/",
> > -               "raw_tp.w/",
> > -               "raw_tracepoint.w/",
> > +               "raw_tp",
> > +               "raw_tracepoint",
> > +               "raw_tp.w",
> > +               "raw_tracepoint.w",
> >         };
> >         size_t i;
> >         const char *tp_name = NULL;
> >
> > +       *link = NULL;
> > +
> >         for (i = 0; i < ARRAY_SIZE(prefixes); i++) {
> > -               if (str_has_pfx(prog->sec_name, prefixes[i])) {
> > -                       tp_name = prog->sec_name + strlen(prefixes[i]);
> > -                       break;
> > -               }
> > +               size_t pfx_len;
> > +
> > +               if (!str_has_pfx(prog->sec_name, prefixes[i]))
> > +                       continue;
> > +
> > +               pfx_len = strlen(prefixes[i]);
> > +               /* no auto-attach case of, e.g., SEC("raw_tp") */
> > +               if (prog->sec_name[pfx_len] == '\0')
> > +                       return 0;
> > +
> > +               if (prog->sec_name[pfx_len] != '/')
> > +                       continue;
>
> Maybe introduce a sec_has_pfx() function with tri-state return value:
> 1 for match with tp_name, 0, for match without tp_name, -1 for no match.
>

Hm.. tri-state might be quite confusing, but there might be some clean
ups to be done around this prefix handling for SEC_DEF()s. I'm
planning to do some more work on SEC() handling, I'll do this clean up
as a follow up, if you don't mind. Need to see how to best consolidate
this across all the places where we do this prefix matching.

> > +
> > +               tp_name = prog->sec_name + pfx_len + 1;
> > +               break;
> >         }
> > +
> >         if (!tp_name) {
> >                 pr_warn("prog '%s': invalid section name '%s'\n",
> >                         prog->name, prog->sec_name);
> > --
> > 2.30.2
> >

Re: [PATCH bpf-next 1/3] libbpf: allow "incomplete" basic tracing SEC() definitions

[Song Liu]

> On Apr 8, 2022, at 3:21 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> 
> On Fri, Apr 8, 2022 at 1:46 PM Song Liu <song@kernel.org> wrote:
>> 
>> On Fri, Apr 8, 2022 at 1:34 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>>> 
>>> In a lot of cases the target of kprobe/kretprobe, tracepoint, raw
>>> tracepoint, etc BPF program might not be known at the compilation time
>>> and will be discovered at runtime. This was always a supported case by
>>> libbpf, with APIs like bpf_program__attach_{kprobe,tracepoint,etc}()
>>> accepting full target definition, regardless of what was defined in
>>> SEC() definition in BPF source code.
>>> 
>>> Unfortunately, up till now libbpf still enforced users to specify at
>>> least something for the fake target, e.g., SEC("kprobe/whatever"), which
>>> is cumbersome and somewhat misleading.
>>> 
>>> This patch allows target-less SEC() definitions for basic tracing BPF
>>> program types:
>>>  - kprobe/kretprobe;
>>>  - multi-kprobe/multi-kretprobe;
>>>  - tracepoints;
>>>  - raw tracepoints.
>>> 
>>> Such target-less SEC() definitions are meant to specify declaratively
>>> proper BPF program type only. Attachment of them will have to be handled
>>> programmatically using correct APIs. As such, skeleton's auto-attachment
>>> of such BPF programs is skipped and generic bpf_program__attach() will
>>> fail, if attempted, due to the lack of enough target information.
>>> 
>>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>>> ---
>>> tools/lib/bpf/libbpf.c | 69 +++++++++++++++++++++++++++++++-----------
>>> 1 file changed, 51 insertions(+), 18 deletions(-)
>>> 
>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>> index 9deb1fc67f19..81911a1e1f3e 100644
>>> --- a/tools/lib/bpf/libbpf.c
>>> +++ b/tools/lib/bpf/libbpf.c
>>> @@ -8668,22 +8668,22 @@ static const struct bpf_sec_def section_defs[] = {
>>>        SEC_DEF("socket",               SOCKET_FILTER, 0, SEC_NONE | SEC_SLOPPY_PFX),
>>>        SEC_DEF("sk_reuseport/migrate", SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
>>>        SEC_DEF("sk_reuseport",         SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
>>> -       SEC_DEF("kprobe/",              KPROBE, 0, SEC_NONE, attach_kprobe),
>>> +       SEC_DEF("kprobe+",              KPROBE, 0, SEC_NONE, attach_kprobe),
>>>        SEC_DEF("uprobe+",              KPROBE, 0, SEC_NONE, attach_uprobe),
>>> -       SEC_DEF("kretprobe/",           KPROBE, 0, SEC_NONE, attach_kprobe),
>>> +       SEC_DEF("kretprobe+",           KPROBE, 0, SEC_NONE, attach_kprobe),
>>>        SEC_DEF("uretprobe+",           KPROBE, 0, SEC_NONE, attach_uprobe),
>>> -       SEC_DEF("kprobe.multi/",        KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
>>> -       SEC_DEF("kretprobe.multi/",     KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
>>> +       SEC_DEF("kprobe.multi+",        KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
>>> +       SEC_DEF("kretprobe.multi+",     KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
>>>        SEC_DEF("usdt+",                KPROBE, 0, SEC_NONE, attach_usdt),
>>>        SEC_DEF("tc",                   SCHED_CLS, 0, SEC_NONE),
>>>        SEC_DEF("classifier",           SCHED_CLS, 0, SEC_NONE | SEC_SLOPPY_PFX | SEC_DEPRECATED),
>>>        SEC_DEF("action",               SCHED_ACT, 0, SEC_NONE | SEC_SLOPPY_PFX),
>>> -       SEC_DEF("tracepoint/",          TRACEPOINT, 0, SEC_NONE, attach_tp),
>>> -       SEC_DEF("tp/",                  TRACEPOINT, 0, SEC_NONE, attach_tp),
>>> -       SEC_DEF("raw_tracepoint/",      RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
>>> -       SEC_DEF("raw_tp/",              RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
>>> -       SEC_DEF("raw_tracepoint.w/",    RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
>>> -       SEC_DEF("raw_tp.w/",            RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
>>> +       SEC_DEF("tracepoint+",          TRACEPOINT, 0, SEC_NONE, attach_tp),
>>> +       SEC_DEF("tp+",                  TRACEPOINT, 0, SEC_NONE, attach_tp),
>>> +       SEC_DEF("raw_tracepoint+",      RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
>>> +       SEC_DEF("raw_tp+",              RAW_TRACEPOINT, 0, SEC_NONE, attach_raw_tp),
>>> +       SEC_DEF("raw_tracepoint.w+",    RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
>>> +       SEC_DEF("raw_tp.w+",            RAW_TRACEPOINT_WRITABLE, 0, SEC_NONE, attach_raw_tp),
>>>        SEC_DEF("tp_btf/",              TRACING, BPF_TRACE_RAW_TP, SEC_ATTACH_BTF, attach_trace),
>>>        SEC_DEF("fentry/",              TRACING, BPF_TRACE_FENTRY, SEC_ATTACH_BTF, attach_trace),
>>>        SEC_DEF("fmod_ret/",            TRACING, BPF_MODIFY_RETURN, SEC_ATTACH_BTF, attach_trace),
>>> @@ -10411,6 +10411,12 @@ static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf
>>>        char *func;
>>>        int n;
>>> 
>>> +       *link = NULL;
>>> +
>>> +       /* no auto-attach for SEC("kprobe") and SEC("kretprobe") */
>>> +       if (strcmp(prog->sec_name, "kprobe") == 0 || strcmp(prog->sec_name, "kretprobe") == 0)
>>> +               return 0;
>>> +
>>>        opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe/");
>>>        if (opts.retprobe)
>>>                func_name = prog->sec_name + sizeof("kretprobe/") - 1;
>>> @@ -10441,6 +10447,13 @@ static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, stru
>>>        char *pattern;
>>>        int n;
>>> 
>>> +       *link = NULL;
>>> +
>>> +       /* no auto-attach for SEC("kprobe.multi") and SEC("kretprobe.multi") */
>>> +       if (strcmp(prog->sec_name, "kprobe.multi") == 0 ||
>>> +           strcmp(prog->sec_name, "kretprobe.multi") == 0)
>>> +               return 0;
>>> +
>>>        opts.retprobe = str_has_pfx(prog->sec_name, "kretprobe.multi/");
>>>        if (opts.retprobe)
>>>                spec = prog->sec_name + sizeof("kretprobe.multi/") - 1;
>>> @@ -11145,6 +11158,12 @@ static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_lin
>>>        if (!sec_name)
>>>                return -ENOMEM;
>>> 
>>> +       *link = NULL;
>>> +
>>> +       /* no auto-attach for SEC("tp") or SEC("tracepoint") */
>>> +       if (strcmp(prog->sec_name, "tp") == 0 || strcmp(prog->sec_name, "tracepoint") == 0)
>>> +               return 0;
>>> +
>>>        /* extract "tp/<category>/<name>" or "tracepoint/<category>/<name>" */
>>>        if (str_has_pfx(prog->sec_name, "tp/"))
>>>                tp_cat = sec_name + sizeof("tp/") - 1;
>>> @@ -11196,20 +11215,34 @@ struct bpf_link *bpf_program__attach_raw_tracepoint(const struct bpf_program *pr
>>> static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link)
>>> {
>>>        static const char *const prefixes[] = {
>>> -               "raw_tp/",
>>> -               "raw_tracepoint/",
>>> -               "raw_tp.w/",
>>> -               "raw_tracepoint.w/",
>>> +               "raw_tp",
>>> +               "raw_tracepoint",
>>> +               "raw_tp.w",
>>> +               "raw_tracepoint.w",
>>>        };
>>>        size_t i;
>>>        const char *tp_name = NULL;
>>> 
>>> +       *link = NULL;
>>> +
>>>        for (i = 0; i < ARRAY_SIZE(prefixes); i++) {
>>> -               if (str_has_pfx(prog->sec_name, prefixes[i])) {
>>> -                       tp_name = prog->sec_name + strlen(prefixes[i]);
>>> -                       break;
>>> -               }
>>> +               size_t pfx_len;
>>> +
>>> +               if (!str_has_pfx(prog->sec_name, prefixes[i]))
>>> +                       continue;
>>> +
>>> +               pfx_len = strlen(prefixes[i]);
>>> +               /* no auto-attach case of, e.g., SEC("raw_tp") */
>>> +               if (prog->sec_name[pfx_len] == '\0')
>>> +                       return 0;
>>> +
>>> +               if (prog->sec_name[pfx_len] != '/')
>>> +                       continue;
>> 
>> Maybe introduce a sec_has_pfx() function with tri-state return value:
>> 1 for match with tp_name, 0, for match without tp_name, -1 for no match.
>> 
> 
> Hm.. tri-state might be quite confusing, but there might be some clean
> ups to be done around this prefix handling for SEC_DEF()s. I'm
> planning to do some more work on SEC() handling, I'll do this clean up
> as a follow up, if you don't mind. Need to see how to best consolidate
> this across all the places where we do this prefix matching.

Sounds good to me. 

Acked-by: Song Liu <songliubraving@fb.com>