Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libbpf: stricter BPF program section name handling #1791

Closed
wants to merge 11 commits into from

Conversation

kernel-patches-bot
Copy link

Pull request for series with
subject: libbpf: stricter BPF program section name handling
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=548623

@kernel-patches-bot
Copy link
Author

Master branch: 3365627
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=548623
version: 1

Pull request is NOT updated. Failed to apply https://patchwork.kernel.org/project/netdevbpf/list/?series=548623
error message:

Cmd('git') failed due to: exit code(128)
  cmdline: git am -3
  stdout: 'Applying: selftests/bpf: normalize XDP section names in selftests
Applying: selftests/bpf: normalize SEC("classifier") usage
Applying: selftests/bpf: normalize all the rest SEC() uses
Applying: libbpf: refactor internal sec_def handling to enable pluggability
Using index info to reconstruct a base tree...
M	tools/lib/bpf/libbpf.c
Falling back to patching base and 3-way merge...
Auto-merging tools/lib/bpf/libbpf.c
CONFLICT (content): Merge conflict in tools/lib/bpf/libbpf.c
Patch failed at 0004 libbpf: refactor internal sec_def handling to enable pluggability
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".'
  stderr: 'error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch'

conflict:

diff --cc tools/lib/bpf/libbpf.c
index 62a43c408d73,8ab2edbf7a3b..000000000000
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@@ -220,7 -220,9 +220,13 @@@ struct reloc_desc 
  
  struct bpf_sec_def;
  
++<<<<<<< HEAD
 +typedef struct bpf_link *(*attach_fn_t)(struct bpf_program *prog);
++=======
+ typedef int (*init_fn_t)(struct bpf_program *prog, long cookie);
+ typedef int (*preload_fn_t)(struct bpf_program *prog, struct bpf_prog_load_params *attr, long cookie);
+ typedef struct bpf_link *(*attach_fn_t)(const struct bpf_program *prog, long cookie);
++>>>>>>> libbpf: refactor internal sec_def handling to enable pluggability
  
  struct bpf_sec_def {
  	const char *sec;
@@@ -6418,6 -6449,21 +6453,22 @@@ static int bpf_object_init_progs(struc
  		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);
++<<<<<<< HEAD
++=======
+ #pragma GCC diagnostic pop
+ 
+ 		/* sec_def can have custom callback which should be called
+ 		 * after bpf_program is initialized to adjust its properties
+ 		 */
+ 		if (prog->sec_def->init_fn) {
+ 			err = prog->sec_def->init_fn(prog, prog->sec_def->cookie);
+ 			if (err < 0) {
+ 				pr_warn("prog '%s': failed to initialize: %d\n",
+ 					prog->name, err);
+ 				return err;
+ 			}
+ 		}
++>>>>>>> libbpf: refactor internal sec_def handling to enable pluggability
  	}
  
  	return 0;
@@@ -7944,12 -7992,12 +7997,21 @@@ void bpf_program__set_expected_attach_t
  	__VA_ARGS__							    \
  }
  
++<<<<<<< HEAD
 +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, long cookie);
+ static struct bpf_link *attach_tp(const struct bpf_program *prog, long cookie);
+ static struct bpf_link *attach_raw_tp(const struct bpf_program *prog, long cookie);
+ static struct bpf_link *attach_trace(const struct bpf_program *prog, long cookie);
+ static struct bpf_link *attach_lsm(const struct bpf_program *prog, long cookie);
+ static struct bpf_link *attach_iter(const struct bpf_program *prog, long cookie);
++>>>>>>> libbpf: refactor internal sec_def handling to enable pluggability
  
  static const struct bpf_sec_def section_defs[] = {
  	BPF_PROG_SEC("socket",			BPF_PROG_TYPE_SOCKET_FILTER),
@@@ -9401,7 -9445,7 +9463,11 @@@ struct bpf_link *bpf_program__attach_kp
  	return bpf_program__attach_kprobe_opts(prog, func_name, &opts);
  }
  
++<<<<<<< HEAD
 +static struct bpf_link *attach_kprobe(struct bpf_program *prog)
++=======
+ static struct bpf_link *attach_kprobe(const struct bpf_program *prog, long cookie)
++>>>>>>> libbpf: refactor internal sec_def handling to enable pluggability
  {
  	DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts);
  	unsigned long offset = 0;
@@@ -9574,7 -9618,7 +9640,11 @@@ struct bpf_link *bpf_program__attach_tr
  	return bpf_program__attach_tracepoint_opts(prog, tp_category, tp_name, NULL);
  }
  
++<<<<<<< HEAD
 +static struct bpf_link *attach_tp(struct bpf_program *prog)
++=======
+ static struct bpf_link *attach_tp(const struct bpf_program *prog, long cookie)
++>>>>>>> libbpf: refactor internal sec_def handling to enable pluggability
  {
  	char *sec_name, *tp_cat, *tp_name;
  	struct bpf_link *link;
@@@ -9628,7 -9672,7 +9698,11 @@@ struct bpf_link *bpf_program__attach_ra
  	return link;
  }
  
++<<<<<<< HEAD
 +static struct bpf_link *attach_raw_tp(struct bpf_program *prog)
++=======
+ static struct bpf_link *attach_raw_tp(const struct bpf_program *prog, long cookie)
++>>>>>>> libbpf: refactor internal sec_def handling to enable pluggability
  {
  	const char *tp_name = prog->sec_name + prog->sec_def->len;
  
@@@ -9675,12 -9719,12 +9749,20 @@@ struct bpf_link *bpf_program__attach_ls
  	return bpf_program__attach_btf_id(prog);
  }
  
++<<<<<<< HEAD
 +static struct bpf_link *attach_trace(struct bpf_program *prog)
++=======
+ static struct bpf_link *attach_trace(const struct bpf_program *prog, long cookie)
++>>>>>>> libbpf: refactor internal sec_def handling to enable pluggability
  {
  	return bpf_program__attach_trace(prog);
  }
  
++<<<<<<< HEAD
 +static struct bpf_link *attach_lsm(struct bpf_program *prog)
++=======
+ static struct bpf_link *attach_lsm(const struct bpf_program *prog, long cookie)
++>>>>>>> libbpf: refactor internal sec_def handling to enable pluggability
  {
  	return bpf_program__attach_lsm(prog);
  }
@@@ -9811,7 -9855,7 +9893,11 @@@ bpf_program__attach_iter(struct bpf_pro
  	return link;
  }
  
++<<<<<<< HEAD
 +static struct bpf_link *attach_iter(struct bpf_program *prog)
++=======
+ static struct bpf_link *attach_iter(const struct bpf_program *prog, long cookie)
++>>>>>>> libbpf: refactor internal sec_def handling to enable pluggability
  {
  	return bpf_program__attach_iter(prog, NULL);
  }

@kernel-patches-bot
Copy link
Author

Master branch: f706f6c
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=548623
version: 1

@kernel-patches-bot
Copy link
Author

Master branch: ca21a3e
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=548623
version: 1

@kernel-patches-bot
Copy link
Author

Master branch: af54faa
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=548623
version: 1

@kernel-patches-bot
Copy link
Author

Master branch: e57f52b
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=548623
version: 1

Pull request is NOT updated. Failed to apply https://patchwork.kernel.org/project/netdevbpf/list/?series=548623
error message:

Cmd('git') failed due to: exit code(128)
  cmdline: git am -3
  stdout: 'Applying: selftests/bpf: normalize XDP section names in selftests
Applying: selftests/bpf: normalize SEC("classifier") usage
Using index info to reconstruct a base tree...
M	tools/testing/selftests/bpf/prog_tests/reference_tracking.c
Falling back to patching base and 3-way merge...
Auto-merging tools/testing/selftests/bpf/prog_tests/reference_tracking.c
CONFLICT (content): Merge conflict in tools/testing/selftests/bpf/prog_tests/reference_tracking.c
Patch failed at 0002 selftests/bpf: normalize SEC("classifier") usage
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".'
  stderr: 'error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch'

conflict:

diff --cc tools/testing/selftests/bpf/prog_tests/reference_tracking.c
index ded2dc8ddd79,e7bb757343ab..000000000000
--- a/tools/testing/selftests/bpf/prog_tests/reference_tracking.c
+++ b/tools/testing/selftests/bpf/prog_tests/reference_tracking.c
@@@ -29,44 -14,48 +29,69 @@@ void test_reference_tracking(void
  	__u32 duration = 0;
  	int err = 0;
  
 -	obj = bpf_object__open_file(file, &open_opts);
 -	if (!ASSERT_OK_PTR(obj, "obj_open_file"))
 +	obj_iter = bpf_object__open_file(file, &open_opts);
 +	if (!ASSERT_OK_PTR(obj_iter, "obj_iter_open_file"))
  		return;
  
 -	if (CHECK(strcmp(bpf_object__name(obj), obj_name), "obj_name",
 +	if (CHECK(strcmp(bpf_object__name(obj_iter), obj_name), "obj_name",
  		  "wrong obj name '%s', expected '%s'\n",
 -		  bpf_object__name(obj), obj_name))
 +		  bpf_object__name(obj_iter), obj_name))
  		goto cleanup;
  
++<<<<<<< HEAD
 +	bpf_object__for_each_program(prog, obj_iter) {
 +		const char *title;
++=======
+ 	bpf_object__for_each_program(prog, obj) {
+ 		const char *name;
+ 		struct bpf_object *test_obj;
+ 		struct bpf_program *test_prog;
++>>>>>>> selftests/bpf: normalize SEC("classifier") usage
  
  		/* Ignore .text sections */
- 		title = bpf_program__section_name(prog);
- 		if (strstr(title, ".text") != NULL)
+ 		name = bpf_program__name(prog);
+ 		if (!test__start_subtest(name))
  			continue;
  
- 		if (!test__start_subtest(title))
- 			continue;
+ 		test_obj = bpf_object__open_file(file, &open_opts);
+ 		if (!ASSERT_OK_PTR(test_obj, "test_obj"))
+ 			goto cleanup;
+ 
+ 		/* disable all but the chosen program */
+ 		bpf_object__for_each_program(test_prog, test_obj) {
+ 			if (strcmp(bpf_program__name(test_prog), name))
+ 				bpf_program__set_autoload(test_prog, false);
+ 		}
  
 +		obj = bpf_object__open_file(file, &open_opts);
 +		if (!ASSERT_OK_PTR(obj, "obj_open_file"))
 +			goto cleanup;
 +
 +		toggle_object_autoload_progs(obj, title);
  		/* Expect verifier failure if test name has 'err' */
- 		if (strstr(title, "err_") != NULL) {
+ 		if (strncmp(name, "err_", sizeof("err_") - 1) == 0) {
  			libbpf_print_fn_t old_print_fn;
  
  			old_print_fn = libbpf_set_print(NULL);
++<<<<<<< HEAD
 +			err = !bpf_object__load(obj);
 +			libbpf_set_print(old_print_fn);
 +		} else {
 +			err = bpf_object__load(obj);
 +		}
 +		CHECK(err, title, "\n");
 +		bpf_object__close(obj);
 +		obj = NULL;
++=======
+ 			err = !bpf_object__load(test_obj);
+ 			libbpf_set_print(old_print_fn);
+ 		} else {
+ 			err = bpf_object__load(test_obj);
+ 		}
+ 		ASSERT_OK(err, name);
+ 
+ 		bpf_object__close(test_obj);
++>>>>>>> selftests/bpf: normalize SEC("classifier") usage
  	}
  
  cleanup:

@kernel-patches-bot
Copy link
Author

Master branch: e57f52b
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=550091
version: 2

@kernel-patches-bot
Copy link
Author

Master branch: 97c140d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=550091
version: 2

@kernel-patches-bot
Copy link
Author

Master branch: cf8980a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=550091
version: 2

@kernel-patches-bot
Copy link
Author

Master branch: a3d697f
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=550091
version: 2

@kernel-patches-bot
Copy link
Author

Master branch: 17b52c2
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=550091
version: 2

@kernel-patches-bot
Copy link
Author

Master branch: c86216b
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=550091
version: 2

@kernel-patches-bot
Copy link
Author

Master branch: c86216b
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=551319
version: 3

@kernel-patches-bot
Copy link
Author

Master branch: e7d5184
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=551319
version: 3

@kernel-patches-bot
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=551319 expired. Closing PR.

Nobody and others added 11 commits September 28, 2021 10:09
As argued in [0], add "tc" ELF section definition for SCHED_CLS BPF
program type. "classifier" is a misleading terminology and should be
migrated away from.

  [0] https://lore.kernel.org/bpf/270e27b1-e5be-5b1c-b343-51bd644d0747@iogearbox.net/

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Convert almost all SEC("xdp_blah") uses to strict SEC("xdp") to comply
with strict libbpf 1.0 logic of exact section name match for XDP program
types. There is only one exception, which is only tested through
iproute2 and defines multiple XDP programs within the same BPF object.
Given iproute2 still works in non-strict libbpf mode and it doesn't have
means to specify XDP programs by its name (not section name/title),
leave that single file alone for now until iproute2 gains lookup by
function/program name.

Acked-by: Dave Marchevsky <davemarchevsky@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Convert all SEC("classifier*") uses to a new and strict SEC("tc")
section name. In reference_tracking selftests switch from ambiguous
searching by program title (section name) to non-ambiguous searching by
name in some selftests, getting closer to completely removing
bpf_object__find_program_by_title().

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Normalize all the other non-conforming SEC() usages across all
selftests. This is in preparation for libbpf to start to enforce
stricter SEC() rules in libbpf 1.0 mode.

Acked-by: Dave Marchevsky <davemarchevsky@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Refactor internals of libbpf to allow adding custom SEC() handling logic
easily from outside of libbpf. To that effect, each SEC()-handling
registration sets mandatory program type/expected attach type for
a given prefix and can provide three callbacks called at different
points of BPF program lifetime:

  - init callback for right after bpf_program is initialized and
  prog_type/expected_attach_type is set. This happens during
  bpf_object__open() step, close to the very end of constructing
  bpf_object, so all the libbpf APIs for querying and updating
  bpf_program properties should be available;

  - pre-load callback is called right before BPF_PROG_LOAD command is
  called in the kernel. This callbacks has ability to set both
  bpf_program properties, as well as program load attributes, overriding
  and augmenting the standard libbpf handling of them;

  - optional auto-attach callback, which makes a given SEC() handler
  support auto-attachment of a BPF program through bpf_program__attach()
  API and/or BPF skeletons <skel>__attach() method.

Each callbacks gets a `long cookie` parameter passed in, which is
specified during SEC() handling. This can be used by callbacks to lookup
whatever additional information is necessary.

This is not yet completely ready to be exposed to the outside world,
mainly due to non-public nature of struct bpf_prog_load_params. Instead
of making it part of public API, we'll wait until the planned low-level
libbpf API improvements for BPF_PROG_LOAD and other typical bpf()
syscall APIs, at which point we'll have a public, probably OPTS-based,
way to fully specify BPF program load parameters, which will be used as
an interface for custom pre-load callbacks.

But this change itself is already a good first step to unify the BPF
program hanling logic even within the libbpf itself. As one example, all
the extra per-program type handling (sleepable bit, attach_btf_id
resolution, unsetting optional expected attach type) is now more obvious
and is gathered in one place.

Acked-by: Dave Marchevsky <davemarchevsky@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Move closer to not relying on bpf_sec_def internals that won't be part
of public API, when pluggable SEC() handlers will be allowed. Drop
pre-calculated prefix length, and in various helpers don't rely on this
prefix length availability. Also minimize reliance on knowing
bpf_sec_def's prefix for few places where section prefix shortcuts are
supported (e.g., tp vs tracepoint, raw_tp vs raw_tracepoint).

Given checking some string for having a given string-constant prefix is
such a common operation and so annoying to be done with pure C code, add
a small macro helper, str_has_pfx(), and reuse it throughout libbpf.c
where prefix comparison is performed. With __builtin_constant_p() it's
possible to have a convenient helper that checks some string for having
a given prefix, where prefix is either string literal (or compile-time
known string due to compiler optimization) or just a runtime string
pointer, which is quite convenient and saves a lot of typing and string
literal duplication.

Acked-by: Dave Marchevsky <davemarchevsky@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Refactor ELF section handler definitions table to use a set of flags and
unified SEC_DEF() macro. This allows for more succinct and table-like
set of definitions, and allows to more easily extend the logic without
adding more verbosity (this is utilized in later patches in the series).

This approach is also making libbpf-internal program pre-load callback
not rely on bpf_sec_def definition, which demonstrates that future
pluggable ELF section handlers will be able to achieve similar level of
integration without libbpf having to expose extra types and APIs.

For starters, update SEC_DEF() definitions and make them more succinct.
Also convert BPF_PROG_SEC() and BPF_APROG_COMPAT() definitions to
a common SEC_DEF() use.

Acked-by: Dave Marchevsky <davemarchevsky@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
…_SEC

Complete SEC() table refactoring towards unified form by rewriting
BPF_APROG_SEC and BPF_EAPROG_SEC definitions with
SEC_DEF(SEC_ATTACHABLE_OPT) (for optional expected_attach_type) and
SEC_DEF(SEC_ATTACHABLE) (mandatory expected_attach_type), respectively.
Drop BPF_APROG_SEC, BPF_EAPROG_SEC, and BPF_PROG_SEC_IMPL macros after
that, leaving SEC_DEF() macro as the only one used.

Acked-by: Dave Marchevsky <davemarchevsky@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Implement strict ELF section name handling for BPF programs. It utilizes
`libbpf_set_strict_mode()` framework and adds new flag: LIBBPF_STRICT_SEC_NAME.

If this flag is set, libbpf will enforce exact section name matching for
a lot of program types that previously allowed just partial prefix
match. E.g., if previously SEC("xdp_whatever_i_want") was allowed, now
in strict mode only SEC("xdp") will be accepted, which makes SEC("")
definitions cleaner and more structured. SEC() now won't be used as yet
another way to uniquely encode BPF program identifier (for that
C function name is better and is guaranteed to be unique within
bpf_object). Now SEC() is strictly BPF program type and, depending on
program type, extra load/attach parameter specification.

Libbpf completely supports multiple BPF programs in the same ELF
section, so multiple BPF programs of the same type/specification easily
co-exist together within the same bpf_object scope.

Additionally, a new (for now internal) convention is introduced: section
name that can be a stand-alone exact BPF program type specificator, but
also could have extra parameters after '/' delimiter. An example of such
section is "struct_ops", which can be specified by itself, but also
allows to specify the intended operation to be attached to, e.g.,
"struct_ops/dctcp_init". Note, that "struct_ops_some_op" is not allowed.
Such section definition is specified as "struct_ops+".

This change is part of libbpf 1.0 effort ([0], [1]).

  [0] Closes: libbpf/libbpf#271
  [1] https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#stricter-and-more-uniform-bpf-program-section-name-sec-handling

Acked-by: Dave Marchevsky <davemarchevsky@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Update "sk_lookup/" definition to be a stand-alone type specifier,
with backwards-compatible prefix match logic in non-libbpf-1.0 mode.

Currently in selftests all the "sk_lookup/<whatever>" uses just use
<whatever> for duplicated unique name encoding, which is redundant as
BPF program's name (C function name) uniquely and descriptively
identifies the intended use for such BPF programs.

With libbpf's SEC_DEF("sk_lookup") definition updated, switch existing
sk_lookup programs to use "unqualified" SEC("sk_lookup") section names,
with no random text after it.

Acked-by: Dave Marchevsky <davemarchevsky@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
@kernel-patches-bot
Copy link
Author

Master branch: 29eef85
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=554365
version: 4

@kernel-patches-bot
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=554365 irrelevant now. Closing PR.

@kernel-patches-bot kernel-patches-bot deleted the series/548623=>bpf-next branch September 28, 2021 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants