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.

Sign up for GitHub

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

Jump to bottom

libbpf: type suffixes and autocreate flag for struct_ops maps #6508

Closed
wants to merge 16 commits into from
Closed

libbpf: type suffixes and autocreate flag for struct_ops maps #6508

wants to merge 16 commits into from

Conversation

kernel-patches-daemon-bpf[bot]
Copy link

Pull request for series with
subject: libbpf: type suffixes and autocreate flag for struct_ops maps
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=831743

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 0270d69
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=831743
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 4b2765a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=831743
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 4e73e1b
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=831743
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 2e0405f
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=831743
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 25703ad
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=831743
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 8f79870
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=831743
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 01031fd
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=831743
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 8f50d5c
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=831743
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 8f50d5c
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=832283
version: 3

@eddyz87
libbpf: allow version suffixes (___smth) for struct_ops types
f9a0739 
E.g. allow the following struct_ops definitions:

    struct bpf_testmod_ops___v1 { int (*test)(void); };
    struct bpf_testmod_ops___v2 { int (*test)(void); };

    SEC(".struct_ops.link")
    struct bpf_testmod_ops___v1 a = { .test = ... }
    SEC(".struct_ops.link")
    struct bpf_testmod_ops___v2 b = { .test = ... }

Where both bpf_testmod_ops__v1 and bpf_testmod_ops__v2 would be
resolved as 'struct bpf_testmod_ops' from kernel BTF.

Acked-by: David Vernet <void@manifault.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
libbpf: tie struct_ops programs to kernel BTF ids, not to local ids
bc231c2 
Enforce the following existing limitation on struct_ops programs based
on kernel BTF id instead of program-local BTF id:

    struct_ops BPF prog can be re-used between multiple .struct_ops &
    .struct_ops.link as long as it's the same struct_ops struct
    definition and the same function pointer field

This allows reusing same BPF program for versioned struct_ops map
definitions, e.g.:

    SEC("struct_ops/test")
    int BPF_PROG(foo) { ... }

    struct some_ops___v1 { int (*test)(void); };
    struct some_ops___v2 { int (*test)(void); };

    SEC(".struct_ops.link") struct some_ops___v1 a = { .test = foo }
    SEC(".struct_ops.link") struct some_ops___v2 b = { .test = foo }

Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
libbpf: honor autocreate flag for struct_ops maps
82eadf4 
Skip load steps for struct_ops maps not marked for automatic creation.
This should allow to load bpf object in situations like below:

    SEC("struct_ops/foo") int BPF_PROG(foo) { ... }
    SEC("struct_ops/bar") int BPF_PROG(bar) { ... }

    struct test_ops___v1 {
    	int (*foo)(void);
    };

    struct test_ops___v2 {
    	int (*foo)(void);
    	int (*does_not_exist)(void);
    };

    SEC(".struct_ops.link")
    struct test_ops___v1 map_for_old = {
    	.test_1 = (void *)foo
    };

    SEC(".struct_ops.link")
    struct test_ops___v2 map_for_new = {
    	.test_1 = (void *)foo,
    	.does_not_exist = (void *)bar
    };

Suppose program is loaded on old kernel that does not have definition
for 'does_not_exist' struct_ops member. After this commit it would be
possible to load such object file after the following tweaks:

    bpf_program__set_autoload(skel->progs.bar, false);
    bpf_map__set_autocreate(skel->maps.map_for_new, false);

Acked-by: David Vernet <void@manifault.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
selftests/bpf: test struct_ops map definition with type suffix
7c8ad61 
Extend struct_ops_module test case to check if it is possible to use
'___' suffixes for struct_ops type specification.

Acked-by: David Vernet <void@manifault.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
selftests/bpf: utility functions to capture libbpf log in test_progs
bbbf120 
Several test_progs tests already capture libbpf log in order to check
for some expected output, e.g bpf_tcp_ca.c, kfunc_dynptr_param.c,
log_buf.c and a few others.

This commit provides a, hopefully, simple API to capture libbpf log
w/o necessity to define new print callback in each test:

    /* Creates a global memstream capturing INFO and WARN level output
     * passed to libbpf_print_fn.
     * Returns 0 on success, negative value on failure.
     * On failure the description is printed using PRINT_FAIL and
     * current test case is marked as fail.
     */
    int start_libbpf_log_capture(void)

    /* Destroys global memstream created by start_libbpf_log_capture().
     * Returns a pointer to captured data which has to be freed.
     * Returned buffer is null terminated.
     */
    char *stop_libbpf_log_capture(void)

The intended usage is as follows:

    if (start_libbpf_log_capture())
            return;
    use_libbpf();
    char *log = stop_libbpf_log_capture();
    ASSERT_HAS_SUBSTR(log, "... expected ...", "expected some message");
    free(log);

As a safety measure, free(start_libbpf_log_capture()) is invoked in the
epilogue of the test_progs.c:run_one_test().

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
selftests/bpf: bad_struct_ops test
7b89dd5 
When loading struct_ops programs kernel requires BTF id of the
struct_ops type and member index for attachment point inside that
type. This makes impossible to use same BPF program in several
struct_ops maps that have different struct_ops type.
Check if libbpf rejects such BPF objects files.

Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
selftests/bpf: test autocreate behavior for struct_ops maps
d63906d 
Check that bpf_map__set_autocreate() can be used to disable automatic
creation for struct_ops maps.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
libbpf: sync progs autoload with maps autocreate for struct_ops maps
2f886fe 
Automatically select which struct_ops programs to load depending on
which struct_ops maps are selected for automatic creation.
E.g. for the BPF code below:

    SEC("struct_ops/test_1") int BPF_PROG(foo) { ... }
    SEC("struct_ops/test_2") int BPF_PROG(bar) { ... }

    SEC(".struct_ops.link")
    struct test_ops___v1 A = {
        .foo = (void *)foo
    };

    SEC(".struct_ops.link")
    struct test_ops___v2 B = {
        .foo = (void *)foo,
        .bar = (void *)bar,
    };

And the following libbpf API calls:

    bpf_map__set_autocreate(skel->maps.A, true);
    bpf_map__set_autocreate(skel->maps.B, false);

The autoload would be enabled for program 'foo' and disabled for
program 'bar'.

During load, for each struct_ops program P, referenced from some
struct_ops map M:
- set P.autoload = true if M.autocreate is true for some M;
- set P.autoload = false if M.autocreate is false for all M;
- don't change P.autoload, if P is not referenced from any map.

Do this after bpf_object__init_kern_struct_ops_maps()
to make sure that shadow vars assignment is done.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
selftests/bpf: verify struct_ops autoload/autocreate sync
a05c77d 
Check that autocreate flags of struct_ops map cause autoload of
struct_ops corresponding programs:
- when struct_ops program is referenced only from a map for which
  autocreate is set to false, that program should not be loaded;
- when struct_ops program with autoload == false is set to be used
  from a map with autocreate == true using shadow var,
  that program should be loaded;
- when struct_ops program is not referenced from any map object load
  should fail.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
libbpf: replace elf_state->st_ops_* fields with SEC_ST_OPS sec_type
4cd8ecb 
The next patch would add two new section names for struct_ops maps.
To make working with multiple struct_ops sections more convenient:
- remove fields like elf_state->st_ops_{shndx,link_shndx};
- mark section descriptions hosting struct_ops as
  elf_sec_desc->sec_type == SEC_ST_OPS;

After these changes struct_ops sections could be processed uniformly
by iterating bpf_object->efile.secs entries.

Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
libbpf: struct_ops in SEC("?.struct_ops") / SEC("?.struct_ops.link")
f011077 
Allow using two new section names for struct_ops maps:
- SEC("?.struct_ops")
- SEC("?.struct_ops.link")

To specify maps that have bpf_map->autocreate == false after open.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
libbpf: rewrite btf datasec names starting from '?'
66bcde6 
Optional struct_ops maps are defined using question mark at the start
of the section name, e.g.:

    SEC("?.struct_ops")
    struct test_ops optional_map = { ... };

This commit teaches libbpf to detect if kernel allows '?' prefix
in datasec names, and if it doesn't then to rewrite such names
by replacing '?' with '_', e.g.:

    DATASEC ?.struct_ops -> DATASEC _.struct_ops

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
selftests/bpf: test case for SEC("?.struct_ops")
305958b 
Check that "?.struct_ops" and "?.struct_ops.link" section names define
struct_ops maps with autocreate == false after open.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
bpf: allow all printable characters in BTF DATASEC names
6b33c76 
The intent is to allow libbpf to use SEC("?.struct_ops") to identify
struct_ops maps that are optional, e.g. like in the following BPF code:

    SEC("?.struct_ops")
    struct test_ops optional_map = { ... };

Which yields the following BTF:

    ...
    [13] DATASEC '?.struct_ops' size=0 vlen=...
    ...

To load such BTF libbpf rewrites DATASEC name before load.
After this patch the rewrite won't be necessary.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: a74f509
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=832914
version: 4

@eddyz87
selftests/bpf: test cases for '?' in BTF names
c493e92 
Two test cases to verify that '?' and other printable characters are
allowed in BTF DATASEC names:
- DATASEC with name "?.foo bar:buz" should be accepted;
- type with name "?foo" should be rejected.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 7d763bc
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=832914
version: 4

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

Cmd('git') failed due to: exit code(128)
  cmdline: git am --3way
  stdout: 'Applying: libbpf: allow version suffixes (___smth) for struct_ops types
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
No changes -- Patch already applied.
Applying: libbpf: tie struct_ops programs to kernel BTF ids, not to local ids
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
No changes -- Patch already applied.
Applying: libbpf: honor autocreate flag for struct_ops maps
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
No changes -- Patch already applied.
Applying: selftests/bpf: test struct_ops map definition with type suffix
Using index info to reconstruct a base tree...
M	tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
M	tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
M	tools/testing/selftests/bpf/progs/struct_ops_module.c
Falling back to patching base and 3-way merge...
Auto-merging tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
No changes -- Patch already applied.
Applying: selftests/bpf: utility functions to capture libbpf log in test_progs
Using index info to reconstruct a base tree...
M	tools/testing/selftests/bpf/test_progs.c
M	tools/testing/selftests/bpf/test_progs.h
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
Applying: selftests/bpf: bad_struct_ops test
Using index info to reconstruct a base tree...
M	tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
M	tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
Falling back to patching base and 3-way merge...
CONFLICT (add/add): Merge conflict in tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
Auto-merging tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
Patch failed at 0006 selftests/bpf: bad_struct_ops test
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/bad_struct_ops.c
index 6a707213e46b,9f5dbefa0dd9..000000000000
--- a/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
+++ b/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
@@@ -2,7 -2,6 +2,10 @@@
  
  #include <test_progs.h>
  #include "bad_struct_ops.skel.h"
++<<<<<<< HEAD
 +#include "bad_struct_ops2.skel.h"
++=======
++>>>>>>> selftests/bpf: bad_struct_ops test
  
  static void invalid_prog_reuse(void)
  {
@@@ -29,39 -28,8 +32,45 @@@ cleanup
  	bad_struct_ops__destroy(skel);
  }
  
++<<<<<<< HEAD
 +static void unused_program(void)
 +{
 +	struct bad_struct_ops2 *skel;
 +	char *log = NULL;
 +	int err;
 +
 +	skel = bad_struct_ops2__open();
 +	if (!ASSERT_OK_PTR(skel, "bad_struct_ops2__open"))
 +		return;
 +
 +	/* struct_ops programs not referenced from any maps are open
 +	 * with autoload set to true.
 +	 */
 +	ASSERT_TRUE(bpf_program__autoload(skel->progs.foo), "foo autoload == true");
 +
 +	if (start_libbpf_log_capture())
 +		goto cleanup;
 +
 +	err = bad_struct_ops2__load(skel);
 +	ASSERT_ERR(err, "bad_struct_ops2__load should fail");
 +	log = stop_libbpf_log_capture();
 +	ASSERT_HAS_SUBSTR(log, "prog 'foo': failed to load",
 +			  "message about 'foo' failing to load");
 +
 +cleanup:
 +	free(log);
 +	bad_struct_ops2__destroy(skel);
 +}
 +
++=======
++>>>>>>> selftests/bpf: bad_struct_ops test
  void test_bad_struct_ops(void)
  {
  	if (test__start_subtest("invalid_prog_reuse"))
  		invalid_prog_reuse();
++<<<<<<< HEAD
 +	if (test__start_subtest("unused_program"))
 +		unused_program();
++=======
++>>>>>>> selftests/bpf: bad_struct_ops test
  }

@kernel-patches-daemon-bpf
Copy link
Author

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

@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot deleted the series/830422=>bpf-next branch March 6, 2024 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
accepted bpf-next V4-ci-pass V4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@eddyz87