[PATCH bpf-next 0/7] Improve set_attach_target() and deprecate open_opts.attach_prog_fd

[PATCH bpf-next 0/7] Improve set_attach_target() and deprecate open_opts.attach_prog_fd

[Andrii Nakryiko]

This patch set deprecates bpf_object_open_opts.attach_prog_fd (in libbpf 0.7+)
by extending bpf_program__set_attach_target() to support some more flexible
scenarios. Existing fexit_bpf2bpf selftest is updated accordingly to not use
deprecated APIs.

While at it, also deprecate no-op relaxed_core_relocs option (they are always
"relaxed").

Last patch also const-ifies all high-level libbpf attach APIs, as there is no
reason for them to assume bpf_program/bpf_map modifications.

Patch #1 also removes one more unneeded use of find_sec_def(), relying on
prog->sec_def that's set during bpf_object__open() operation, simplifying
upcoming refactoring a little bit more.

All these changes are preparatory patches before SEC() handling refactoring
that will come next.

Andrii Nakryiko (7):
  libbpf: use pre-setup sec_def in libbpf_find_attach_btf_id()
  selftests/bpf: stop using relaxed_core_relocs which has no effect
  libbpf: deprecated bpf_object_open_opts.relaxed_core_relocs
  libbpf: allow skipping attach_func_name in
    bpf_program__set_attach_target()
  selftests/bpf: switch fexit_bpf2bpf selftest to set_attach_target()
    API
  libbpf: schedule open_opts.attach_prog_fd deprecation since v0.7
  libbpf: constify all high-level program attach APIs

 tools/lib/bpf/libbpf.c                        | 98 ++++++++++---------
 tools/lib/bpf/libbpf.h                        | 39 ++++----
 tools/lib/bpf/libbpf_common.h                 |  5 +
 .../selftests/bpf/prog_tests/core_reloc.c     |  3 +-
 .../selftests/bpf/prog_tests/fexit_bpf2bpf.c  | 43 ++++----
 5 files changed, 107 insertions(+), 81 deletions(-)

-- 
2.30.2

[PATCH bpf-next 1/7] libbpf: use pre-setup sec_def in libbpf_find_attach_btf_id()

[Andrii Nakryiko]

Don't perform another search for sec_def inside
libbpf_find_attach_btf_id(), as each recognized bpf_program already has
prog->sec_def set.

Also remove unnecessary NULL check for prog->sec_name, as it can never
be NULL.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 62a43c408d73..5ba11b249e9b 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8461,19 +8461,15 @@ static int libbpf_find_attach_btf_id(struct bpf_program *prog, int *btf_obj_fd,
 {
 	enum bpf_attach_type attach_type = prog->expected_attach_type;
 	__u32 attach_prog_fd = prog->attach_prog_fd;
-	const char *name = prog->sec_name, *attach_name;
-	const struct bpf_sec_def *sec = NULL;
+	const char *attach_name;
 	int err = 0;
 
-	if (!name)
-		return -EINVAL;
-
-	sec = find_sec_def(name);
-	if (!sec || !sec->is_attach_btf) {
-		pr_warn("failed to identify BTF ID based on ELF section name '%s'\n", name);
+	if (!prog->sec_def || !prog->sec_def->is_attach_btf) {
+		pr_warn("failed to identify BTF ID based on ELF section name '%s'\n",
+			prog->sec_name);
 		return -ESRCH;
 	}
-	attach_name = name + sec->len;
+	attach_name = prog->sec_name + prog->sec_def->len;
 
 	/* BPF program's BTF ID */
 	if (attach_prog_fd) {
-- 
2.30.2

[PATCH bpf-next 2/7] selftests/bpf: stop using relaxed_core_relocs which has no effect

[Andrii Nakryiko]

relaxed_core_relocs option hasn't had any effect for a while now, stop
specifying it. Next patch marks it as deprecated.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/testing/selftests/bpf/prog_tests/core_reloc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index 15d355af8d1d..763302e63a29 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -249,8 +249,7 @@ static int duration = 0;
 #define SIZE_CASE_COMMON(name)						\
 	.case_name = #name,						\
 	.bpf_obj_file = "test_core_reloc_size.o",			\
-	.btf_src_file = "btf__core_reloc_" #name ".o",			\
-	.relaxed_core_relocs = true
+	.btf_src_file = "btf__core_reloc_" #name ".o"
 
 #define SIZE_OUTPUT_DATA(type)						\
 	STRUCT_TO_CHAR_PTR(core_reloc_size_output) {			\
-- 
2.30.2

[PATCH bpf-next 3/7] libbpf: deprecated bpf_object_open_opts.relaxed_core_relocs

[Andrii Nakryiko]

It's relevant and hasn't been doing anything for a long while now.
Deprecated it.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 2f6f0e15d1e7..7111e8d651de 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -83,6 +83,7 @@ struct bpf_object_open_opts {
 	 * Non-relocatable instructions are replaced with invalid ones to
 	 * prevent accidental errors.
 	 * */
+	LIBBPF_DEPRECATED_SINCE(0, 6, "field has no effect")
 	bool relaxed_core_relocs;
 	/* maps that set the 'pinning' attribute in their definition will have
 	 * their pin_path attribute set to a file in this directory, and be
-- 
2.30.2

[PATCH bpf-next 4/7] libbpf: allow skipping attach_func_name in bpf_program__set_attach_target()

[Andrii Nakryiko]

Allow to use bpf_program__set_attach_target to only set target attach
program FD, while letting libbpf to use target attach function name from
SEC() definition. This might be useful for some scenarios where
bpf_object contains multiple related freplace BPF programs intended to
replace different sub-programs in target BPF program. In such case all
programs will have the same attach_prog_fd, but different
attach_func_name. It's conveninent to specify such target function names
declaratively in SEC() definitions, but attach_prog_fd is a dynamic
runtime setting.

To simplify such scenario, allow bpf_program__set_attach_target() to
delay BTF ID resolution till the BPF program load time by providing NULL
attach_func_name. In that case the behavior will be similar to using
bpf_object_open_opts.attach_prog_fd (which is marked deprecated since
v0.7), but has the benefit of allowing more control by user in what is
attached to what. Such setup allows having BPF programs attached to
different target attach_prog_fd with target funtions still declaratively
recorded in BPF source code in SEC() definitions.

Selftests changes in the next patch should make this more obvious.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5ba11b249e9b..552d05a85cbb 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10643,18 +10643,29 @@ int bpf_program__set_attach_target(struct bpf_program *prog,
 {
 	int btf_obj_fd = 0, btf_id = 0, err;
 
-	if (!prog || attach_prog_fd < 0 || !attach_func_name)
+	if (!prog || attach_prog_fd < 0)
 		return libbpf_err(-EINVAL);
 
 	if (prog->obj->loaded)
 		return libbpf_err(-EINVAL);
 
+	if (attach_prog_fd && !attach_func_name) {
+		/* remember attach_prog_fd and let bpf_program__load() find
+		 * BTF ID during the program load
+		 */
+		prog->attach_prog_fd = attach_prog_fd;
+		return 0;
+	}
+
 	if (attach_prog_fd) {
 		btf_id = libbpf_find_prog_btf_id(attach_func_name,
 						 attach_prog_fd);
 		if (btf_id < 0)
 			return libbpf_err(btf_id);
 	} else {
+		if (!attach_func_name)
+			return libbpf_err(-EINVAL);
+
 		/* load btf_vmlinux, if not yet */
 		err = bpf_object__load_vmlinux_btf(prog->obj, true);
 		if (err)
-- 
2.30.2

[PATCH bpf-next 5/7] selftests/bpf: switch fexit_bpf2bpf selftest to set_attach_target() API

[Andrii Nakryiko]

Switch fexit_bpf2bpf selftest to bpf_program__set_attach_target()
instead of using bpf_object_open_opts.attach_prog_fd, which is going to
be deprecated. These changes also demonstrate the new mode of
set_attach_target() in which it allows NULL when the target is BPF
program (attach_prog_fd != 0).

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 .../selftests/bpf/prog_tests/fexit_bpf2bpf.c  | 43 +++++++++++--------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
index 73b4c76e6b86..c7c1816899bf 100644
--- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
@@ -60,7 +60,7 @@ static void test_fexit_bpf2bpf_common(const char *obj_file,
 	struct bpf_object *obj = NULL, *tgt_obj;
 	__u32 retval, tgt_prog_id, info_len;
 	struct bpf_prog_info prog_info = {};
-	struct bpf_program **prog = NULL;
+	struct bpf_program **prog = NULL, *p;
 	struct bpf_link **link = NULL;
 	int err, tgt_fd, i;
 	struct btf *btf;
@@ -69,9 +69,6 @@ static void test_fexit_bpf2bpf_common(const char *obj_file,
 			    &tgt_obj, &tgt_fd);
 	if (!ASSERT_OK(err, "tgt_prog_load"))
 		return;
-	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts,
-			    .attach_prog_fd = tgt_fd,
-			   );
 
 	info_len = sizeof(prog_info);
 	err = bpf_obj_get_info_by_fd(tgt_fd, &prog_info, &info_len);
@@ -89,10 +86,15 @@ static void test_fexit_bpf2bpf_common(const char *obj_file,
 	if (!ASSERT_OK_PTR(prog, "prog_ptr"))
 		goto close_prog;
 
-	obj = bpf_object__open_file(obj_file, &opts);
+	obj = bpf_object__open_file(obj_file, NULL);
 	if (!ASSERT_OK_PTR(obj, "obj_open"))
 		goto close_prog;
 
+	bpf_object__for_each_program(p, obj) {
+		err = bpf_program__set_attach_target(p, tgt_fd, NULL);
+		ASSERT_OK(err, "set_attach_target");
+	}
+
 	err = bpf_object__load(obj);
 	if (!ASSERT_OK(err, "obj_load"))
 		goto close_prog;
@@ -270,7 +272,7 @@ static void test_fmod_ret_freplace(void)
 	struct bpf_link *freplace_link = NULL;
 	struct bpf_program *prog;
 	__u32 duration = 0;
-	int err, pkt_fd;
+	int err, pkt_fd, attach_prog_fd;
 
 	err = bpf_prog_load(tgt_name, BPF_PROG_TYPE_UNSPEC,
 			    &pkt_obj, &pkt_fd);
@@ -278,26 +280,32 @@ static void test_fmod_ret_freplace(void)
 	if (CHECK(err, "tgt_prog_load", "file %s err %d errno %d\n",
 		  tgt_name, err, errno))
 		return;
-	opts.attach_prog_fd = pkt_fd;
 
-	freplace_obj = bpf_object__open_file(freplace_name, &opts);
+	freplace_obj = bpf_object__open_file(freplace_name, NULL);
 	if (!ASSERT_OK_PTR(freplace_obj, "freplace_obj_open"))
 		goto out;
 
+	prog = bpf_program__next(NULL, freplace_obj);
+	err = bpf_program__set_attach_target(prog, pkt_fd, NULL);
+	ASSERT_OK(err, "freplace__set_attach_target");
+
 	err = bpf_object__load(freplace_obj);
 	if (CHECK(err, "freplace_obj_load", "err %d\n", err))
 		goto out;
 
-	prog = bpf_program__next(NULL, freplace_obj);
 	freplace_link = bpf_program__attach_trace(prog);
 	if (!ASSERT_OK_PTR(freplace_link, "freplace_attach_trace"))
 		goto out;
 
-	opts.attach_prog_fd = bpf_program__fd(prog);
-	fmod_obj = bpf_object__open_file(fmod_ret_name, &opts);
+	fmod_obj = bpf_object__open_file(fmod_ret_name, NULL);
 	if (!ASSERT_OK_PTR(fmod_obj, "fmod_obj_open"))
 		goto out;
 
+	attach_prog_fd = bpf_program__fd(prog);
+	prog = bpf_program__next(NULL, fmod_obj);
+	err = bpf_program__set_attach_target(prog, attach_prog_fd, NULL);
+	ASSERT_OK(err, "fmod_ret_set_attach_target");
+
 	err = bpf_object__load(fmod_obj);
 	if (CHECK(!err, "fmod_obj_load", "loading fmod_ret should fail\n"))
 		goto out;
@@ -322,14 +330,14 @@ static void test_func_sockmap_update(void)
 }
 
 static void test_obj_load_failure_common(const char *obj_file,
-					  const char *target_obj_file)
-
+					 const char *target_obj_file)
 {
 	/*
 	 * standalone test that asserts failure to load freplace prog
 	 * because of invalid return code.
 	 */
 	struct bpf_object *obj = NULL, *pkt_obj;
+	struct bpf_program *prog;
 	int err, pkt_fd;
 	__u32 duration = 0;
 
@@ -339,14 +347,15 @@ static void test_obj_load_failure_common(const char *obj_file,
 	if (CHECK(err, "tgt_prog_load", "file %s err %d errno %d\n",
 		  target_obj_file, err, errno))
 		return;
-	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts,
-			    .attach_prog_fd = pkt_fd,
-			   );
 
-	obj = bpf_object__open_file(obj_file, &opts);
+	obj = bpf_object__open_file(obj_file, NULL);
 	if (!ASSERT_OK_PTR(obj, "obj_open"))
 		goto close_prog;
 
+	prog = bpf_program__next(NULL, obj);
+	err = bpf_program__set_attach_target(prog, pkt_fd, NULL);
+	ASSERT_OK(err, "set_attach_target");
+
 	/* It should fail to load the program */
 	err = bpf_object__load(obj);
 	if (CHECK(!err, "bpf_obj_load should fail", "err %d\n", err))
-- 
2.30.2

[PATCH bpf-next 6/7] libbpf: schedule open_opts.attach_prog_fd deprecation since v0.7

[Andrii Nakryiko]

bpf_object_open_opts.attach_prog_fd makes a pretty strong assumption
that bpf_object contains either only single freplace BPF program or all
of BPF programs in BPF object are freplaces intended to replace
different subprograms of the same target BPF program. This seems both
a bit confusing, too assuming, and limiting.

We've had bpf_program__set_attach_target() API which allows more
fine-grained control over this, on a per-program level. As such, mark
open_opts.attach_prog_fd as deprecated starting from v0.7, so that we
have one more universal way of setting freplace targets. With previous
change to allow NULL attach_func_name argument, and especially combined
with BPF skeleton, arguable bpf_program__set_attach_target() is a more
convenient and explicit API as well.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c        | 3 +++
 tools/lib/bpf/libbpf.h        | 2 ++
 tools/lib/bpf/libbpf_common.h | 5 +++++
 3 files changed, 10 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 552d05a85cbb..6aeeb0e82acc 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6415,9 +6415,12 @@ static int bpf_object_init_progs(struct bpf_object *obj, const struct bpf_object
 		bpf_program__set_type(prog, prog->sec_def->prog_type);
 		bpf_program__set_expected_attach_type(prog, prog->sec_def->expected_attach_type);
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 		if (prog->sec_def->prog_type == BPF_PROG_TYPE_TRACING ||
 		    prog->sec_def->prog_type == BPF_PROG_TYPE_EXT)
 			prog->attach_prog_fd = OPTS_GET(opts, attach_prog_fd, 0);
+#pragma GCC diagnostic pop
 	}
 
 	return 0;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 7111e8d651de..52b7ee090037 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -90,6 +90,8 @@ struct bpf_object_open_opts {
 	 * auto-pinned to that path on load; defaults to "/sys/fs/bpf".
 	 */
 	const char *pin_root_path;
+
+	LIBBPF_DEPRECATED_SINCE(0, 7, "use bpf_program__set_attach_target() on each individual bpf_program")
 	__u32 attach_prog_fd;
 	/* Additional kernel config content that augments and overrides
 	 * system Kconfig for CONFIG_xxx externs.
diff --git a/tools/lib/bpf/libbpf_common.h b/tools/lib/bpf/libbpf_common.h
index 36ac77f2bea2..aaa1efbf6f51 100644
--- a/tools/lib/bpf/libbpf_common.h
+++ b/tools/lib/bpf/libbpf_common.h
@@ -35,6 +35,11 @@
 #else
 #define __LIBBPF_MARK_DEPRECATED_0_6(X)
 #endif
+#if __LIBBPF_CURRENT_VERSION_GEQ(0, 7)
+#define __LIBBPF_MARK_DEPRECATED_0_7(X) X
+#else
+#define __LIBBPF_MARK_DEPRECATED_0_7(X)
+#endif
 
 /* Helper macro to declare and initialize libbpf options struct
  *
-- 
2.30.2

[PATCH bpf-next 7/7] libbpf: constify all high-level program attach APIs

[Andrii Nakryiko]

Attach APIs shouldn't need to modify bpf_program/bpf_map structs, so
change all struct bpf_program and struct bpf_map pointers to const
pointers. This is completely backwards compatible with no functional
change.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c | 68 +++++++++++++++++++++---------------------
 tools/lib/bpf/libbpf.h | 36 +++++++++++-----------
 2 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6aeeb0e82acc..da65a1666a5e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -220,7 +220,7 @@ struct reloc_desc {
 
 struct bpf_sec_def;
 
-typedef struct bpf_link *(*attach_fn_t)(struct bpf_program *prog);
+typedef struct bpf_link *(*attach_fn_t)(const struct bpf_program *prog);
 
 struct bpf_sec_def {
 	const char *sec;
@@ -7947,12 +7947,12 @@ void bpf_program__set_expected_attach_type(struct bpf_program *prog,
 	__VA_ARGS__							    \
 }
 
-static struct bpf_link *attach_kprobe(struct bpf_program *prog);
-static struct bpf_link *attach_tp(struct bpf_program *prog);
-static struct bpf_link *attach_raw_tp(struct bpf_program *prog);
-static struct bpf_link *attach_trace(struct bpf_program *prog);
-static struct bpf_link *attach_lsm(struct bpf_program *prog);
-static struct bpf_link *attach_iter(struct bpf_program *prog);
+static struct bpf_link *attach_kprobe(const struct bpf_program *prog);
+static struct bpf_link *attach_tp(const struct bpf_program *prog);
+static struct bpf_link *attach_raw_tp(const struct bpf_program *prog);
+static struct bpf_link *attach_trace(const struct bpf_program *prog);
+static struct bpf_link *attach_lsm(const struct bpf_program *prog);
+static struct bpf_link *attach_iter(const struct bpf_program *prog);
 
 static const struct bpf_sec_def section_defs[] = {
 	BPF_PROG_SEC("socket",			BPF_PROG_TYPE_SOCKET_FILTER),
@@ -9092,7 +9092,7 @@ static void bpf_link_perf_dealloc(struct bpf_link *link)
 	free(perf_link);
 }
 
-struct bpf_link *bpf_program__attach_perf_event_opts(struct bpf_program *prog, int pfd,
+struct bpf_link *bpf_program__attach_perf_event_opts(const struct bpf_program *prog, int pfd,
 						     const struct bpf_perf_event_opts *opts)
 {
 	char errmsg[STRERR_BUFSIZE];
@@ -9167,7 +9167,7 @@ struct bpf_link *bpf_program__attach_perf_event_opts(struct bpf_program *prog, i
 	return libbpf_err_ptr(err);
 }
 
-struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog, int pfd)
+struct bpf_link *bpf_program__attach_perf_event(const struct bpf_program *prog, int pfd)
 {
 	return bpf_program__attach_perf_event_opts(prog, pfd, NULL);
 }
@@ -9332,7 +9332,7 @@ static int perf_event_kprobe_open_legacy(bool retprobe, const char *name, uint64
 }
 
 struct bpf_link *
-bpf_program__attach_kprobe_opts(struct bpf_program *prog,
+bpf_program__attach_kprobe_opts(const struct bpf_program *prog,
 				const char *func_name,
 				const struct bpf_kprobe_opts *opts)
 {
@@ -9389,7 +9389,7 @@ bpf_program__attach_kprobe_opts(struct bpf_program *prog,
 	return link;
 }
 
-struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
+struct bpf_link *bpf_program__attach_kprobe(const struct bpf_program *prog,
 					    bool retprobe,
 					    const char *func_name)
 {
@@ -9400,7 +9400,7 @@ struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
 	return bpf_program__attach_kprobe_opts(prog, func_name, &opts);
 }
 
-static struct bpf_link *attach_kprobe(struct bpf_program *prog)
+static struct bpf_link *attach_kprobe(const struct bpf_program *prog)
 {
 	DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts);
 	unsigned long offset = 0;
@@ -9432,7 +9432,7 @@ static struct bpf_link *attach_kprobe(struct bpf_program *prog)
 }
 
 LIBBPF_API struct bpf_link *
-bpf_program__attach_uprobe_opts(struct bpf_program *prog, pid_t pid,
+bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
 				const char *binary_path, size_t func_offset,
 				const struct bpf_uprobe_opts *opts)
 {
@@ -9472,7 +9472,7 @@ bpf_program__attach_uprobe_opts(struct bpf_program *prog, pid_t pid,
 	return link;
 }
 
-struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
+struct bpf_link *bpf_program__attach_uprobe(const struct bpf_program *prog,
 					    bool retprobe, pid_t pid,
 					    const char *binary_path,
 					    size_t func_offset)
@@ -9532,7 +9532,7 @@ static int perf_event_open_tracepoint(const char *tp_category,
 	return pfd;
 }
 
-struct bpf_link *bpf_program__attach_tracepoint_opts(struct bpf_program *prog,
+struct bpf_link *bpf_program__attach_tracepoint_opts(const struct bpf_program *prog,
 						     const char *tp_category,
 						     const char *tp_name,
 						     const struct bpf_tracepoint_opts *opts)
@@ -9566,14 +9566,14 @@ struct bpf_link *bpf_program__attach_tracepoint_opts(struct bpf_program *prog,
 	return link;
 }
 
-struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
+struct bpf_link *bpf_program__attach_tracepoint(const struct bpf_program *prog,
 						const char *tp_category,
 						const char *tp_name)
 {
 	return bpf_program__attach_tracepoint_opts(prog, tp_category, tp_name, NULL);
 }
 
-static struct bpf_link *attach_tp(struct bpf_program *prog)
+static struct bpf_link *attach_tp(const struct bpf_program *prog)
 {
 	char *sec_name, *tp_cat, *tp_name;
 	struct bpf_link *link;
@@ -9597,7 +9597,7 @@ static struct bpf_link *attach_tp(struct bpf_program *prog)
 	return link;
 }
 
-struct bpf_link *bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
+struct bpf_link *bpf_program__attach_raw_tracepoint(const struct bpf_program *prog,
 						    const char *tp_name)
 {
 	char errmsg[STRERR_BUFSIZE];
@@ -9627,7 +9627,7 @@ struct bpf_link *bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
 	return link;
 }
 
-static struct bpf_link *attach_raw_tp(struct bpf_program *prog)
+static struct bpf_link *attach_raw_tp(const struct bpf_program *prog)
 {
 	const char *tp_name = prog->sec_name + prog->sec_def->len;
 
@@ -9635,7 +9635,7 @@ static struct bpf_link *attach_raw_tp(struct bpf_program *prog)
 }
 
 /* Common logic for all BPF program types that attach to a btf_id */
-static struct bpf_link *bpf_program__attach_btf_id(struct bpf_program *prog)
+static struct bpf_link *bpf_program__attach_btf_id(const struct bpf_program *prog)
 {
 	char errmsg[STRERR_BUFSIZE];
 	struct bpf_link *link;
@@ -9664,28 +9664,28 @@ static struct bpf_link *bpf_program__attach_btf_id(struct bpf_program *prog)
 	return (struct bpf_link *)link;
 }
 
-struct bpf_link *bpf_program__attach_trace(struct bpf_program *prog)
+struct bpf_link *bpf_program__attach_trace(const struct bpf_program *prog)
 {
 	return bpf_program__attach_btf_id(prog);
 }
 
-struct bpf_link *bpf_program__attach_lsm(struct bpf_program *prog)
+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(struct bpf_program *prog)
+static struct bpf_link *attach_trace(const struct bpf_program *prog)
 {
 	return bpf_program__attach_trace(prog);
 }
 
-static struct bpf_link *attach_lsm(struct bpf_program *prog)
+static struct bpf_link *attach_lsm(const struct bpf_program *prog)
 {
 	return bpf_program__attach_lsm(prog);
 }
 
 static struct bpf_link *
-bpf_program__attach_fd(struct bpf_program *prog, int target_fd, int btf_id,
+bpf_program__attach_fd(const struct bpf_program *prog, int target_fd, int btf_id,
 		       const char *target_name)
 {
 	DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts,
@@ -9721,24 +9721,24 @@ bpf_program__attach_fd(struct bpf_program *prog, int target_fd, int btf_id,
 }
 
 struct bpf_link *
-bpf_program__attach_cgroup(struct bpf_program *prog, int cgroup_fd)
+bpf_program__attach_cgroup(const struct bpf_program *prog, int cgroup_fd)
 {
 	return bpf_program__attach_fd(prog, cgroup_fd, 0, "cgroup");
 }
 
 struct bpf_link *
-bpf_program__attach_netns(struct bpf_program *prog, int netns_fd)
+bpf_program__attach_netns(const struct bpf_program *prog, int netns_fd)
 {
 	return bpf_program__attach_fd(prog, netns_fd, 0, "netns");
 }
 
-struct bpf_link *bpf_program__attach_xdp(struct bpf_program *prog, int ifindex)
+struct bpf_link *bpf_program__attach_xdp(const struct bpf_program *prog, int ifindex)
 {
 	/* target_fd/target_ifindex use the same field in LINK_CREATE */
 	return bpf_program__attach_fd(prog, ifindex, 0, "xdp");
 }
 
-struct bpf_link *bpf_program__attach_freplace(struct bpf_program *prog,
+struct bpf_link *bpf_program__attach_freplace(const struct bpf_program *prog,
 					      int target_fd,
 					      const char *attach_func_name)
 {
@@ -9771,7 +9771,7 @@ struct bpf_link *bpf_program__attach_freplace(struct bpf_program *prog,
 }
 
 struct bpf_link *
-bpf_program__attach_iter(struct bpf_program *prog,
+bpf_program__attach_iter(const struct bpf_program *prog,
 			 const struct bpf_iter_attach_opts *opts)
 {
 	DECLARE_LIBBPF_OPTS(bpf_link_create_opts, link_create_opts);
@@ -9810,12 +9810,12 @@ bpf_program__attach_iter(struct bpf_program *prog,
 	return link;
 }
 
-static struct bpf_link *attach_iter(struct bpf_program *prog)
+static struct bpf_link *attach_iter(const struct bpf_program *prog)
 {
 	return bpf_program__attach_iter(prog, NULL);
 }
 
-struct bpf_link *bpf_program__attach(struct bpf_program *prog)
+struct bpf_link *bpf_program__attach(const struct bpf_program *prog)
 {
 	if (!prog->sec_def || !prog->sec_def->attach_fn)
 		return libbpf_err_ptr(-ESRCH);
@@ -9833,7 +9833,7 @@ static int bpf_link__detach_struct_ops(struct bpf_link *link)
 	return 0;
 }
 
-struct bpf_link *bpf_map__attach_struct_ops(struct bpf_map *map)
+struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map)
 {
 	struct bpf_struct_ops *st_ops;
 	struct bpf_link *link;
@@ -10918,7 +10918,7 @@ int bpf_object__attach_skeleton(struct bpf_object_skeleton *s)
 		if (!prog->sec_def || !prog->sec_def->attach_fn)
 			continue;
 
-		*link = prog->sec_def->attach_fn(prog);
+		*link = bpf_program__attach(prog);
 		err = libbpf_get_error(*link);
 		if (err) {
 			pr_warn("failed to auto-attach program '%s': %d\n",
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 52b7ee090037..c90e3d79e72c 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -246,7 +246,7 @@ LIBBPF_API int bpf_link__detach(struct bpf_link *link);
 LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
 
 LIBBPF_API struct bpf_link *
-bpf_program__attach(struct bpf_program *prog);
+bpf_program__attach(const struct bpf_program *prog);
 
 struct bpf_perf_event_opts {
 	/* size of this struct, for forward/backward compatiblity */
@@ -257,10 +257,10 @@ struct bpf_perf_event_opts {
 #define bpf_perf_event_opts__last_field bpf_cookie
 
 LIBBPF_API struct bpf_link *
-bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
+bpf_program__attach_perf_event(const struct bpf_program *prog, int pfd);
 
 LIBBPF_API struct bpf_link *
-bpf_program__attach_perf_event_opts(struct bpf_program *prog, int pfd,
+bpf_program__attach_perf_event_opts(const struct bpf_program *prog, int pfd,
 				    const struct bpf_perf_event_opts *opts);
 
 struct bpf_kprobe_opts {
@@ -277,10 +277,10 @@ struct bpf_kprobe_opts {
 #define bpf_kprobe_opts__last_field retprobe
 
 LIBBPF_API struct bpf_link *
-bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
+bpf_program__attach_kprobe(const struct bpf_program *prog, bool retprobe,
 			   const char *func_name);
 LIBBPF_API struct bpf_link *
-bpf_program__attach_kprobe_opts(struct bpf_program *prog,
+bpf_program__attach_kprobe_opts(const struct bpf_program *prog,
                                 const char *func_name,
                                 const struct bpf_kprobe_opts *opts);
 
@@ -300,11 +300,11 @@ struct bpf_uprobe_opts {
 #define bpf_uprobe_opts__last_field retprobe
 
 LIBBPF_API struct bpf_link *
-bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
+bpf_program__attach_uprobe(const struct bpf_program *prog, bool retprobe,
 			   pid_t pid, const char *binary_path,
 			   size_t func_offset);
 LIBBPF_API struct bpf_link *
-bpf_program__attach_uprobe_opts(struct bpf_program *prog, pid_t pid,
+bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
 				const char *binary_path, size_t func_offset,
 				const struct bpf_uprobe_opts *opts);
 
@@ -317,35 +317,35 @@ struct bpf_tracepoint_opts {
 #define bpf_tracepoint_opts__last_field bpf_cookie
 
 LIBBPF_API struct bpf_link *
-bpf_program__attach_tracepoint(struct bpf_program *prog,
+bpf_program__attach_tracepoint(const struct bpf_program *prog,
 			       const char *tp_category,
 			       const char *tp_name);
 LIBBPF_API struct bpf_link *
-bpf_program__attach_tracepoint_opts(struct bpf_program *prog,
+bpf_program__attach_tracepoint_opts(const struct bpf_program *prog,
 				    const char *tp_category,
 				    const char *tp_name,
 				    const struct bpf_tracepoint_opts *opts);
 
 LIBBPF_API struct bpf_link *
-bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
+bpf_program__attach_raw_tracepoint(const struct bpf_program *prog,
 				   const char *tp_name);
 LIBBPF_API struct bpf_link *
-bpf_program__attach_trace(struct bpf_program *prog);
+bpf_program__attach_trace(const struct bpf_program *prog);
 LIBBPF_API struct bpf_link *
-bpf_program__attach_lsm(struct bpf_program *prog);
+bpf_program__attach_lsm(const struct bpf_program *prog);
 LIBBPF_API struct bpf_link *
-bpf_program__attach_cgroup(struct bpf_program *prog, int cgroup_fd);
+bpf_program__attach_cgroup(const struct bpf_program *prog, int cgroup_fd);
 LIBBPF_API struct bpf_link *
-bpf_program__attach_netns(struct bpf_program *prog, int netns_fd);
+bpf_program__attach_netns(const struct bpf_program *prog, int netns_fd);
 LIBBPF_API struct bpf_link *
-bpf_program__attach_xdp(struct bpf_program *prog, int ifindex);
+bpf_program__attach_xdp(const struct bpf_program *prog, int ifindex);
 LIBBPF_API struct bpf_link *
-bpf_program__attach_freplace(struct bpf_program *prog,
+bpf_program__attach_freplace(const struct bpf_program *prog,
 			     int target_fd, const char *attach_func_name);
 
 struct bpf_map;
 
-LIBBPF_API struct bpf_link *bpf_map__attach_struct_ops(struct bpf_map *map);
+LIBBPF_API struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map);
 
 struct bpf_iter_attach_opts {
 	size_t sz; /* size of this struct for forward/backward compatibility */
@@ -355,7 +355,7 @@ struct bpf_iter_attach_opts {
 #define bpf_iter_attach_opts__last_field link_info_len
 
 LIBBPF_API struct bpf_link *
-bpf_program__attach_iter(struct bpf_program *prog,
+bpf_program__attach_iter(const struct bpf_program *prog,
 			 const struct bpf_iter_attach_opts *opts);
 
 struct bpf_insn;
-- 
2.30.2

Re: [PATCH bpf-next 1/7] libbpf: use pre-setup sec_def in libbpf_find_attach_btf_id()

[Yonghong Song]

On 9/15/21 6:58 PM, Andrii Nakryiko wrote:
> Don't perform another search for sec_def inside
> libbpf_find_attach_btf_id(), as each recognized bpf_program already has
> prog->sec_def set.
> 
> Also remove unnecessary NULL check for prog->sec_name, as it can never
> be NULL.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Yonghong Song <yhs@fb.com>

Re: [PATCH bpf-next 2/7] selftests/bpf: stop using relaxed_core_relocs which has no effect

[Yonghong Song]

On 9/15/21 6:58 PM, Andrii Nakryiko wrote:
> relaxed_core_relocs option hasn't had any effect for a while now, stop
> specifying it. Next patch marks it as deprecated.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Yonghong Song <yhs@fb.com>

Re: [PATCH bpf-next 3/7] libbpf: deprecated bpf_object_open_opts.relaxed_core_relocs

[Yonghong Song]

On 9/15/21 6:58 PM, Andrii Nakryiko wrote:
> It's relevant and hasn't been doing anything for a long while now.
> Deprecated it.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Yonghong Song <yhs@fb.com>

Re: [PATCH bpf-next 4/7] libbpf: allow skipping attach_func_name in bpf_program__set_attach_target()

[Yonghong Song]

On 9/15/21 6:58 PM, Andrii Nakryiko wrote:
> Allow to use bpf_program__set_attach_target to only set target attach
> program FD, while letting libbpf to use target attach function name from
> SEC() definition. This might be useful for some scenarios where
> bpf_object contains multiple related freplace BPF programs intended to
> replace different sub-programs in target BPF program. In such case all
> programs will have the same attach_prog_fd, but different
> attach_func_name. It's conveninent to specify such target function names

typo: conveninent => convenient

> declaratively in SEC() definitions, but attach_prog_fd is a dynamic
> runtime setting.
> 
> To simplify such scenario, allow bpf_program__set_attach_target() to
> delay BTF ID resolution till the BPF program load time by providing NULL
> attach_func_name. In that case the behavior will be similar to using
> bpf_object_open_opts.attach_prog_fd (which is marked deprecated since
> v0.7), but has the benefit of allowing more control by user in what is
> attached to what. Such setup allows having BPF programs attached to
> different target attach_prog_fd with target funtions still declaratively
> recorded in BPF source code in SEC() definitions.
> 
> Selftests changes in the next patch should make this more obvious.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Yonghong Song <yhs@fb.com>

Re: [PATCH bpf-next 5/7] selftests/bpf: switch fexit_bpf2bpf selftest to set_attach_target() API

[Yonghong Song]

On 9/15/21 6:58 PM, Andrii Nakryiko wrote:
> Switch fexit_bpf2bpf selftest to bpf_program__set_attach_target()
> instead of using bpf_object_open_opts.attach_prog_fd, which is going to
> be deprecated. These changes also demonstrate the new mode of
> set_attach_target() in which it allows NULL when the target is BPF
> program (attach_prog_fd != 0).
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Ack with a minor nit below.
Acked-by: Yonghong Song <yhs@fb.com>

> ---
>   .../selftests/bpf/prog_tests/fexit_bpf2bpf.c  | 43 +++++++++++--------
>   1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> index 73b4c76e6b86..c7c1816899bf 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> @@ -60,7 +60,7 @@ static void test_fexit_bpf2bpf_common(const char *obj_file,
>   	struct bpf_object *obj = NULL, *tgt_obj;
>   	__u32 retval, tgt_prog_id, info_len;
>   	struct bpf_prog_info prog_info = {};
> -	struct bpf_program **prog = NULL;
> +	struct bpf_program **prog = NULL, *p;
>   	struct bpf_link **link = NULL;
>   	int err, tgt_fd, i;
>   	struct btf *btf;
> @@ -69,9 +69,6 @@ static void test_fexit_bpf2bpf_common(const char *obj_file,
>   			    &tgt_obj, &tgt_fd);
>   	if (!ASSERT_OK(err, "tgt_prog_load"))
>   		return;
> -	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts,
> -			    .attach_prog_fd = tgt_fd,
> -			   );
>   
>   	info_len = sizeof(prog_info);
>   	err = bpf_obj_get_info_by_fd(tgt_fd, &prog_info, &info_len);
> @@ -89,10 +86,15 @@ static void test_fexit_bpf2bpf_common(const char *obj_file,
>   	if (!ASSERT_OK_PTR(prog, "prog_ptr"))
>   		goto close_prog;
>   
> -	obj = bpf_object__open_file(obj_file, &opts);
> +	obj = bpf_object__open_file(obj_file, NULL);
>   	if (!ASSERT_OK_PTR(obj, "obj_open"))
>   		goto close_prog;
>   
> +	bpf_object__for_each_program(p, obj) {
> +		err = bpf_program__set_attach_target(p, tgt_fd, NULL);
> +		ASSERT_OK(err, "set_attach_target");
> +	}
> +
>   	err = bpf_object__load(obj);
>   	if (!ASSERT_OK(err, "obj_load"))
>   		goto close_prog;
> @@ -270,7 +272,7 @@ static void test_fmod_ret_freplace(void)
>   	struct bpf_link *freplace_link = NULL;
>   	struct bpf_program *prog;
>   	__u32 duration = 0;
> -	int err, pkt_fd;
> +	int err, pkt_fd, attach_prog_fd;
>   
>   	err = bpf_prog_load(tgt_name, BPF_PROG_TYPE_UNSPEC,
>   			    &pkt_obj, &pkt_fd);
> @@ -278,26 +280,32 @@ static void test_fmod_ret_freplace(void)
>   	if (CHECK(err, "tgt_prog_load", "file %s err %d errno %d\n",
>   		  tgt_name, err, errno))
>   		return;
> -	opts.attach_prog_fd = pkt_fd;
>   
> -	freplace_obj = bpf_object__open_file(freplace_name, &opts);
> +	freplace_obj = bpf_object__open_file(freplace_name, NULL);
>   	if (!ASSERT_OK_PTR(freplace_obj, "freplace_obj_open"))
>   		goto out;
>   
> +	prog = bpf_program__next(NULL, freplace_obj);
> +	err = bpf_program__set_attach_target(prog, pkt_fd, NULL);
> +	ASSERT_OK(err, "freplace__set_attach_target");

The above pattern appears 3 times. Maybe it is worthwhile to
have a small helper. But the current code is also fine,
so I won't insist.

> +
>   	err = bpf_object__load(freplace_obj);
>   	if (CHECK(err, "freplace_obj_load", "err %d\n", err))
>   		goto out;
>   
> -	prog = bpf_program__next(NULL, freplace_obj);
>   	freplace_link = bpf_program__attach_trace(prog);
>   	if (!ASSERT_OK_PTR(freplace_link, "freplace_attach_trace"))
>   		goto out;
>   
> -	opts.attach_prog_fd = bpf_program__fd(prog);
> -	fmod_obj = bpf_object__open_file(fmod_ret_name, &opts);
> +	fmod_obj = bpf_object__open_file(fmod_ret_name, NULL);
>   	if (!ASSERT_OK_PTR(fmod_obj, "fmod_obj_open"))
>   		goto out;
>   
> +	attach_prog_fd = bpf_program__fd(prog);
> +	prog = bpf_program__next(NULL, fmod_obj);
> +	err = bpf_program__set_attach_target(prog, attach_prog_fd, NULL);
> +	ASSERT_OK(err, "fmod_ret_set_attach_target");
> +
>   	err = bpf_object__load(fmod_obj);
>   	if (CHECK(!err, "fmod_obj_load", "loading fmod_ret should fail\n"))
>   		goto out;
> @@ -322,14 +330,14 @@ static void test_func_sockmap_update(void)
>   }
>   
>   static void test_obj_load_failure_common(const char *obj_file,
> -					  const char *target_obj_file)
> -
> +					 const char *target_obj_file)
>   {
>   	/*
>   	 * standalone test that asserts failure to load freplace prog
>   	 * because of invalid return code.
>   	 */
>   	struct bpf_object *obj = NULL, *pkt_obj;
> +	struct bpf_program *prog;
>   	int err, pkt_fd;
>   	__u32 duration = 0;
>   
> @@ -339,14 +347,15 @@ static void test_obj_load_failure_common(const char *obj_file,
>   	if (CHECK(err, "tgt_prog_load", "file %s err %d errno %d\n",
>   		  target_obj_file, err, errno))
>   		return;
> -	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts,
> -			    .attach_prog_fd = pkt_fd,
> -			   );
>   
> -	obj = bpf_object__open_file(obj_file, &opts);
> +	obj = bpf_object__open_file(obj_file, NULL);
>   	if (!ASSERT_OK_PTR(obj, "obj_open"))
>   		goto close_prog;
>   
> +	prog = bpf_program__next(NULL, obj);
> +	err = bpf_program__set_attach_target(prog, pkt_fd, NULL);
> +	ASSERT_OK(err, "set_attach_target");
> +
>   	/* It should fail to load the program */
>   	err = bpf_object__load(obj);
>   	if (CHECK(!err, "bpf_obj_load should fail", "err %d\n", err))
> 

Re: [PATCH bpf-next 6/7] libbpf: schedule open_opts.attach_prog_fd deprecation since v0.7

[Yonghong Song]

On 9/15/21 6:58 PM, Andrii Nakryiko wrote:
> bpf_object_open_opts.attach_prog_fd makes a pretty strong assumption
> that bpf_object contains either only single freplace BPF program or all
> of BPF programs in BPF object are freplaces intended to replace
> different subprograms of the same target BPF program. This seems both
> a bit confusing, too assuming, and limiting.
> 
> We've had bpf_program__set_attach_target() API which allows more
> fine-grained control over this, on a per-program level. As such, mark
> open_opts.attach_prog_fd as deprecated starting from v0.7, so that we
> have one more universal way of setting freplace targets. With previous
> change to allow NULL attach_func_name argument, and especially combined
> with BPF skeleton, arguable bpf_program__set_attach_target() is a more
> convenient and explicit API as well.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Yonghong Song <yhs@fb.com>

Re: [PATCH bpf-next 7/7] libbpf: constify all high-level program attach APIs

[Yonghong Song]

On 9/15/21 6:58 PM, Andrii Nakryiko wrote:
> Attach APIs shouldn't need to modify bpf_program/bpf_map structs, so
> change all struct bpf_program and struct bpf_map pointers to const
> pointers. This is completely backwards compatible with no functional
> change.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Yonghong Song <yhs@fb.com>

Re: [PATCH bpf-next 5/7] selftests/bpf: switch fexit_bpf2bpf selftest to set_attach_target() API

[Andrii Nakryiko]

On Wed, Sep 15, 2021 at 9:24 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 9/15/21 6:58 PM, Andrii Nakryiko wrote:
> > Switch fexit_bpf2bpf selftest to bpf_program__set_attach_target()
> > instead of using bpf_object_open_opts.attach_prog_fd, which is going to
> > be deprecated. These changes also demonstrate the new mode of
> > set_attach_target() in which it allows NULL when the target is BPF
> > program (attach_prog_fd != 0).
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
> Ack with a minor nit below.
> Acked-by: Yonghong Song <yhs@fb.com>
>
> > ---
> >   .../selftests/bpf/prog_tests/fexit_bpf2bpf.c  | 43 +++++++++++--------
> >   1 file changed, 26 insertions(+), 17 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> > index 73b4c76e6b86..c7c1816899bf 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> > @@ -60,7 +60,7 @@ static void test_fexit_bpf2bpf_common(const char *obj_file,
> >       struct bpf_object *obj = NULL, *tgt_obj;
> >       __u32 retval, tgt_prog_id, info_len;
> >       struct bpf_prog_info prog_info = {};
> > -     struct bpf_program **prog = NULL;
> > +     struct bpf_program **prog = NULL, *p;
> >       struct bpf_link **link = NULL;
> >       int err, tgt_fd, i;
> >       struct btf *btf;
> > @@ -69,9 +69,6 @@ static void test_fexit_bpf2bpf_common(const char *obj_file,
> >                           &tgt_obj, &tgt_fd);
> >       if (!ASSERT_OK(err, "tgt_prog_load"))
> >               return;
> > -     DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts,
> > -                         .attach_prog_fd = tgt_fd,
> > -                        );
> >
> >       info_len = sizeof(prog_info);
> >       err = bpf_obj_get_info_by_fd(tgt_fd, &prog_info, &info_len);
> > @@ -89,10 +86,15 @@ static void test_fexit_bpf2bpf_common(const char *obj_file,
> >       if (!ASSERT_OK_PTR(prog, "prog_ptr"))
> >               goto close_prog;
> >
> > -     obj = bpf_object__open_file(obj_file, &opts);
> > +     obj = bpf_object__open_file(obj_file, NULL);
> >       if (!ASSERT_OK_PTR(obj, "obj_open"))
> >               goto close_prog;
> >
> > +     bpf_object__for_each_program(p, obj) {
> > +             err = bpf_program__set_attach_target(p, tgt_fd, NULL);
> > +             ASSERT_OK(err, "set_attach_target");
> > +     }
> > +
> >       err = bpf_object__load(obj);
> >       if (!ASSERT_OK(err, "obj_load"))
> >               goto close_prog;
> > @@ -270,7 +272,7 @@ static void test_fmod_ret_freplace(void)
> >       struct bpf_link *freplace_link = NULL;
> >       struct bpf_program *prog;
> >       __u32 duration = 0;
> > -     int err, pkt_fd;
> > +     int err, pkt_fd, attach_prog_fd;
> >
> >       err = bpf_prog_load(tgt_name, BPF_PROG_TYPE_UNSPEC,
> >                           &pkt_obj, &pkt_fd);
> > @@ -278,26 +280,32 @@ static void test_fmod_ret_freplace(void)
> >       if (CHECK(err, "tgt_prog_load", "file %s err %d errno %d\n",
> >                 tgt_name, err, errno))
> >               return;
> > -     opts.attach_prog_fd = pkt_fd;
> >
> > -     freplace_obj = bpf_object__open_file(freplace_name, &opts);
> > +     freplace_obj = bpf_object__open_file(freplace_name, NULL);
> >       if (!ASSERT_OK_PTR(freplace_obj, "freplace_obj_open"))
> >               goto out;
> >
> > +     prog = bpf_program__next(NULL, freplace_obj);
> > +     err = bpf_program__set_attach_target(prog, pkt_fd, NULL);
> > +     ASSERT_OK(err, "freplace__set_attach_target");
>
> The above pattern appears 3 times. Maybe it is worthwhile to
> have a small helper. But the current code is also fine,
> so I won't insist.

I think it's acceptable for test code. It actually makes it easier to
follow, because every extra helper function is a bit of distraction
and context switch when reading the code.

My hope that with time we'll clean this up to use BPF skeleton and the
code will be clean without unnecessary repetition.

>
> > +
> >       err = bpf_object__load(freplace_obj);
> >       if (CHECK(err, "freplace_obj_load", "err %d\n", err))
> >               goto out;
> >
> > -     prog = bpf_program__next(NULL, freplace_obj);
> >       freplace_link = bpf_program__attach_trace(prog);
> >       if (!ASSERT_OK_PTR(freplace_link, "freplace_attach_trace"))
> >               goto out;
> >
> > -     opts.attach_prog_fd = bpf_program__fd(prog);
> > -     fmod_obj = bpf_object__open_file(fmod_ret_name, &opts);
> > +     fmod_obj = bpf_object__open_file(fmod_ret_name, NULL);
> >       if (!ASSERT_OK_PTR(fmod_obj, "fmod_obj_open"))
> >               goto out;
> >
> > +     attach_prog_fd = bpf_program__fd(prog);
> > +     prog = bpf_program__next(NULL, fmod_obj);
> > +     err = bpf_program__set_attach_target(prog, attach_prog_fd, NULL);
> > +     ASSERT_OK(err, "fmod_ret_set_attach_target");
> > +
> >       err = bpf_object__load(fmod_obj);
> >       if (CHECK(!err, "fmod_obj_load", "loading fmod_ret should fail\n"))
> >               goto out;
> > @@ -322,14 +330,14 @@ static void test_func_sockmap_update(void)
> >   }
> >
> >   static void test_obj_load_failure_common(const char *obj_file,
> > -                                       const char *target_obj_file)
> > -
> > +                                      const char *target_obj_file)
> >   {
> >       /*
> >        * standalone test that asserts failure to load freplace prog
> >        * because of invalid return code.
> >        */
> >       struct bpf_object *obj = NULL, *pkt_obj;
> > +     struct bpf_program *prog;
> >       int err, pkt_fd;
> >       __u32 duration = 0;
> >
> > @@ -339,14 +347,15 @@ static void test_obj_load_failure_common(const char *obj_file,
> >       if (CHECK(err, "tgt_prog_load", "file %s err %d errno %d\n",
> >                 target_obj_file, err, errno))
> >               return;
> > -     DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts,
> > -                         .attach_prog_fd = pkt_fd,
> > -                        );
> >
> > -     obj = bpf_object__open_file(obj_file, &opts);
> > +     obj = bpf_object__open_file(obj_file, NULL);
> >       if (!ASSERT_OK_PTR(obj, "obj_open"))
> >               goto close_prog;
> >
> > +     prog = bpf_program__next(NULL, obj);
> > +     err = bpf_program__set_attach_target(prog, pkt_fd, NULL);
> > +     ASSERT_OK(err, "set_attach_target");
> > +
> >       /* It should fail to load the program */
> >       err = bpf_object__load(obj);
> >       if (CHECK(!err, "bpf_obj_load should fail", "err %d\n", err))
> >

Re: [PATCH bpf-next 4/7] libbpf: allow skipping attach_func_name in bpf_program__set_attach_target()

[Alexei Starovoitov]

On Wed, Sep 15, 2021 at 9:17 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 9/15/21 6:58 PM, Andrii Nakryiko wrote:
> > Allow to use bpf_program__set_attach_target to only set target attach
> > program FD, while letting libbpf to use target attach function name from
> > SEC() definition. This might be useful for some scenarios where
> > bpf_object contains multiple related freplace BPF programs intended to
> > replace different sub-programs in target BPF program. In such case all
> > programs will have the same attach_prog_fd, but different
> > attach_func_name. It's conveninent to specify such target function names
>
> typo: conveninent => convenient
>
> > declaratively in SEC() definitions, but attach_prog_fd is a dynamic
> > runtime setting.
> >
> > To simplify such scenario, allow bpf_program__set_attach_target() to
> > delay BTF ID resolution till the BPF program load time by providing NULL
> > attach_func_name. In that case the behavior will be similar to using
> > bpf_object_open_opts.attach_prog_fd (which is marked deprecated since
> > v0.7), but has the benefit of allowing more control by user in what is
> > attached to what. Such setup allows having BPF programs attached to
> > different target attach_prog_fd with target funtions still declaratively

Applied with "conveninent" and "funtions" typos fixed.

Re: [PATCH bpf-next 0/7] Improve set_attach_target() and deprecate open_opts.attach_prog_fd

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (refs/heads/master):

On Wed, 15 Sep 2021 18:58:29 -0700 you wrote:
> This patch set deprecates bpf_object_open_opts.attach_prog_fd (in libbpf 0.7+)
> by extending bpf_program__set_attach_target() to support some more flexible
> scenarios. Existing fexit_bpf2bpf selftest is updated accordingly to not use
> deprecated APIs.
> 
> While at it, also deprecate no-op relaxed_core_relocs option (they are always
> "relaxed").
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/7] libbpf: use pre-setup sec_def in libbpf_find_attach_btf_id()
    https://git.kernel.org/bpf/bpf-next/c/f11f86a3931b
  - [bpf-next,2/7] selftests/bpf: stop using relaxed_core_relocs which has no effect
    https://git.kernel.org/bpf/bpf-next/c/23a7baaa9388
  - [bpf-next,3/7] libbpf: deprecated bpf_object_open_opts.relaxed_core_relocs
    https://git.kernel.org/bpf/bpf-next/c/277641859e83
  - [bpf-next,4/7] libbpf: allow skipping attach_func_name in bpf_program__set_attach_target()
    https://git.kernel.org/bpf/bpf-next/c/2d5ec1c66e25
  - [bpf-next,5/7] selftests/bpf: switch fexit_bpf2bpf selftest to set_attach_target() API
    https://git.kernel.org/bpf/bpf-next/c/60aed22076b0
  - [bpf-next,6/7] libbpf: schedule open_opts.attach_prog_fd deprecation since v0.7
    https://git.kernel.org/bpf/bpf-next/c/91b555d73e53
  - [bpf-next,7/7] libbpf: constify all high-level program attach APIs
    https://git.kernel.org/bpf/bpf-next/c/942025c9f37e

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH bpf-next 4/7] libbpf: allow skipping attach_func_name in bpf_program__set_attach_target()

[Andrii Nakryiko]

On Fri, Sep 17, 2021 at 9:10 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Sep 15, 2021 at 9:17 PM Yonghong Song <yhs@fb.com> wrote:
> >
> >
> >
> > On 9/15/21 6:58 PM, Andrii Nakryiko wrote:
> > > Allow to use bpf_program__set_attach_target to only set target attach
> > > program FD, while letting libbpf to use target attach function name from
> > > SEC() definition. This might be useful for some scenarios where
> > > bpf_object contains multiple related freplace BPF programs intended to
> > > replace different sub-programs in target BPF program. In such case all
> > > programs will have the same attach_prog_fd, but different
> > > attach_func_name. It's conveninent to specify such target function names
> >
> > typo: conveninent => convenient
> >
> > > declaratively in SEC() definitions, but attach_prog_fd is a dynamic
> > > runtime setting.
> > >
> > > To simplify such scenario, allow bpf_program__set_attach_target() to
> > > delay BTF ID resolution till the BPF program load time by providing NULL
> > > attach_func_name. In that case the behavior will be similar to using
> > > bpf_object_open_opts.attach_prog_fd (which is marked deprecated since
> > > v0.7), but has the benefit of allowing more control by user in what is
> > > attached to what. Such setup allows having BPF programs attached to
> > > different target attach_prog_fd with target funtions still declaratively
>
> Applied with "conveninent" and "funtions" typos fixed.

Thanks!