Skip to content

Support associating BPF programs with struct_ops #10089

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
Open
wants to merge 6 commits into
base: bpf-next_base
Choose a base branch
from
Open

Support associating BPF programs with struct_ops #10089

wants to merge 6 commits into from
+753 −2

Conversation

@kernel-patches-daemon-bpf
Copy link

Pull request for series with
subject: Support associating BPF programs with struct_ops
version: 4
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1015659

@ameryhung
bpf: Allow verifier to fixup kernel module kfuncs
b33792e 
Allow verifier to fixup kfuncs in kernel module to support kfuncs with
__prog arguments. Currently, special kfuncs and kfuncs with __prog
arguments are kernel kfuncs. Allowing kernel module kfuncs should not
affect existing kfunc fixup as kernel module kfuncs have BTF IDs greater
than kernel kfuncs' BTF IDs.

Signed-off-by: Amery Hung <[email protected]>
@ameryhung
bpf: Support associating BPF program with struct_ops
abe990a 
Add a new BPF command BPF_PROG_ASSOC_STRUCT_OPS to allow associating
a BPF program with a struct_ops map. This command takes a file
descriptor of a struct_ops map and a BPF program and set
prog->aux->st_ops_assoc to the kdata of the struct_ops map.

The command does not accept a struct_ops program nor a non-struct_ops
map. Programs of a struct_ops map is automatically associated with the
map during map update. If a program is shared between two struct_ops
maps, prog->aux->st_ops_assoc will be poisoned to indicate that the
associated struct_ops is ambiguous. The pointer, once poisoned, cannot
be reset since we have lost track of associated struct_ops. For other
program types, the associated struct_ops map, once set, cannot be
changed later. This restriction may be lifted in the future if there is
a use case.

A kernel helper bpf_prog_get_assoc_struct_ops() can be used to retrieve
the associated struct_ops pointer. The returned pointer, if not NULL, is
guaranteed to be valid and point to a fully updated struct_ops struct.
For struct_ops program reused in multiple struct_ops map, the return
will be NULL. The call must be paired with bpf_struct_ops_put() once the
caller is done with the struct_ops.

To make sure the returned pointer to be valid, the command increases the
refcount of the map for every associated non-struct_ops programs. For
struct_ops programs, since they do not increase the refcount of
struct_ops map, bpf_prog_get_assoc_struct_ops() has to bump the refcount
of the map to prevent a map from being freed while the program runs.
This can happen if a struct_ops program schedules a time callback that
runs after the struct_ops map is freed.

struct_ops implementers should note that the struct_ops returned may or
may not be attached. The struct_ops implementer will be responsible for
tracking and checking the state of the associated struct_ops map if the
use case requires an attached struct_ops.

Signed-off-by: Amery Hung <[email protected]>
@ameryhung
libbpf: Add support for associating BPF program with struct_ops
b08abc2 
Add low-level wrapper and libbpf API for BPF_PROG_ASSOC_STRUCT_OPS
command in the bpf() syscall.

Signed-off-by: Amery Hung <[email protected]>
@ameryhung
selftests/bpf: Test BPF_PROG_ASSOC_STRUCT_OPS command
939d1d1 
Test BPF_PROG_ASSOC_STRUCT_OPS command that associates a BPF program
with a struct_ops. The test follows the same logic in commit
ba7000f ("selftests/bpf: Test multi_st_ops and calling kfuncs from
different programs"), but instead of using map id to identify a specific
struct_ops, this test uses the new BPF command to associate a struct_ops
with a program.

The test consists of two sets of almost identical struct_ops maps and BPF
programs associated with the map. Their only difference is the unique
value returned by bpf_testmod_multi_st_ops::test_1().

The test first loads the programs and associates them with struct_ops
maps. Then, it exercises the BPF programs. They will in turn call kfunc
bpf_kfunc_multi_st_ops_test_1_prog_arg() to trigger test_1() of the
associated struct_ops map, and then check if the right unique value is
returned.

Signed-off-by: Amery Hung <[email protected]>
@ameryhung
selftests/bpf: Test ambiguous associated struct_ops
d9c4312 
Add a test to make sure implicit struct_ops association does not
break backward compatibility nor return incorrect struct_ops.
struct_ops programs should still be allowed to be reused in
different struct_ops map. The associated struct_ops map set implicitly
however will be poisoned. Trying to read it through the helper
bpf_prog_get_assoc_struct_ops() should result in a NULL pointer.

While recursion of test_1() cannot happen due to the associated
struct_ops being ambiguois, explicitly check for it to prevent stack
overflow if the test regresses.

Signed-off-by: Amery Hung <[email protected]>
@ameryhung
selftests/bpf: Test getting associated struct_ops in timer callback
55e52af 
Make sure 1) a timer callback can also reference the associated
struct_ops, and then make sure 2) the timer callback cannot get a
dangled pointer to the struct_ops when the map is freed.

The test schedules a timer callback from a struct_ops program since
struct_ops programs do not pin the map. It is possible for the timer
callback to run after the map is freed. The timer callback calls a
kfunc that runs .test_1() of the associated struct_ops, which should
return MAP_MAGIC when the map is still alive or -1 when the map is
gone.

The first subtest added in this patch schedules the timer callback to
run immediately, while the map is still alive. The second subtest added
schedules the callback to run 500ms after syscall_prog runs and then
frees the map right after syscall_prog runs. Both subtests then wait
until the callback runs to check the return of the kfunc.

Signed-off-by: Amery Hung <[email protected]>
@kernel-patches-daemon-bpf
Copy link
Author

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

@kernel-patches-review-bot
Copy link 

commit b08abc299849ff6fb04ae36ea93a6b06377d5fd2
Author: Amery Hung <[email protected]>

libbpf: Add support for associating BPF program with struct_ops

This commit adds libbpf wrappers for the BPF_PROG_ASSOC_STRUCT_OPS
syscall command, providing both a low-level bpf_prog_assoc_struct_ops()
function and a higher-level bpf_program__assoc_struct_ops() API.

> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 339b19797..885b0f891 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c

[ ... ]

> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index e983a3e40..1f9c28d27 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h

[ ... ]

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b90574f39..483a19052 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -13891,6 +13891,36 @@ int bpf_program__set_attach_target(struct bpf_program *prog,
>  	return 0;
>  }
>
> +int bpf_program__assoc_struct_ops(struct bpf_program *prog, struct bpf_map *map,
> +				  struct bpf_prog_assoc_struct_ops_opts *opts)
> +{
> +	int prog_fd;
> +
> +	prog_fd = bpf_program__fd(prog);
> +	if (prog_fd < 0) {
> +		pr_warn("prog '%s': can't associate BPF program without FD (was it loaded?)\n",
> +			prog->name);

Can prog->name be dereferenced here if prog is NULL? The bpf_program__fd()
function returns -EINVAL when prog is NULL, which triggers this error path.
Looking at bpf_program__set_attach_target() just above, it checks !prog
before any dereference.

> +		return -EINVAL;
> +	}
> +
> +	if (prog->type == BPF_PROG_TYPE_STRUCT_OPS) {
> +		pr_warn("prog '%s': can't associate struct_ops program\n", prog->name);
> +		return -EINVAL;
> +	}
> +
> +	if (map->fd < 0) {
> +		pr_warn("map '%s': can't associate BPF map without FD (was it created?)\n", map->name);

Similarly, can map->fd and map->name be safely accessed if map is NULL?

> +		return -EINVAL;
> +	}
> +
> +	if (!bpf_map__is_struct_ops(map)) {
> +		pr_warn("map '%s': can't associate non-struct_ops map\n", map->name);
> +		return -EINVAL;
> +	}
> +
> +	return bpf_prog_assoc_struct_ops(prog_fd, map->fd, opts);
> +}

> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5118d0a90..45720b7c2 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h

[ ... ]

> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 8ed874990..84fb90a01 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map

[ ... ]

AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: libbpf: Add support for associating BPF program with struct_ops
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18792840393

@kernel-patches-daemon-bpf
Copy link
Author

Forwarding comment 3445040391 via email
In-Reply-To: [email protected]
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

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

bpf-next new V4-ci-fail V4

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ameryhung