Hi,
well, given that the HID changes haven't moved a lot in the past
revisions and that I am cc-ing a bunch of people, I have dropped them
while we focus on the last 2 requirements in bpf-core changes.
I'll submit a HID targeted series when we get these in tree, which
would make things a lore more independent.
For reference, the whole reasons for these 2 main changes are at
https://lore.kernel.org/bpf/[email protected]/
Compared to v10 (in addition of dropping the HID changes), I have
changed the selftests so we can test both light skeletons and libbbpf
calls.
Cheers,
Benjamin
Benjamin Tissoires (7):
selftests/bpf: regroup and declare similar kfuncs selftests in an
array
bpf: split btf_check_subprog_arg_match in two
bpf/verifier: allow all functions to read user provided context
selftests/bpf: add test for accessing ctx from syscall program type
bpf/btf: bump BTF_KFUNC_SET_MAX_CNT
bpf/verifier: allow kfunc to return an allocated mem
selftests/bpf: Add tests for kfunc returning a memory pointer
include/linux/bpf.h | 11 +-
include/linux/bpf_verifier.h | 2 +
include/linux/btf.h | 10 +
kernel/bpf/btf.c | 149 ++++++++++--
kernel/bpf/verifier.c | 66 +++--
net/bpf/test_run.c | 37 +++
tools/testing/selftests/bpf/Makefile | 5 +-
.../selftests/bpf/prog_tests/kfunc_call.c | 227 ++++++++++++++++--
.../selftests/bpf/progs/kfunc_call_fail.c | 160 ++++++++++++
.../selftests/bpf/progs/kfunc_call_test.c | 71 ++++++
10 files changed, 678 insertions(+), 60 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/kfunc_call_fail.c
--
2.36.1
Similar to tools/testing/selftests/bpf/prog_tests/dynptr.c:
we declare an array of tests that we run one by one in a for loop.
Followup patches will add more similar-ish tests, so avoid a lot of copy
paste by grouping the declaration in an array.
For light skeletons, we have to rely on the offsetof() macro so we can
statically declare which program we are using.
In the libbpf case, we can rely on bpf_object__find_program_by_name().
So also change the Makefile to generate both light skeletons and normal
ones.
Signed-off-by: Benjamin Tissoires <[email protected]>
---
changes in v11:
- use of both light skeletons and normal libbpf
new in v10
---
tools/testing/selftests/bpf/Makefile | 5 +-
.../selftests/bpf/prog_tests/kfunc_call.c | 81 +++++++++++++++----
2 files changed, 68 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index eecad99f1735..314216d77b8d 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -351,11 +351,12 @@ LINKED_SKELS := test_static_linked.skel.h linked_funcs.skel.h \
test_subskeleton.skel.h test_subskeleton_lib.skel.h \
test_usdt.skel.h
-LSKELS := kfunc_call_test.c fentry_test.c fexit_test.c fexit_sleep.c \
+LSKELS := fentry_test.c fexit_test.c fexit_sleep.c \
test_ringbuf.c atomics.c trace_printk.c trace_vprintk.c \
map_ptr_kern.c core_kern.c core_kern_overflow.c
# Generate both light skeleton and libbpf skeleton for these
-LSKELS_EXTRA := test_ksyms_module.c test_ksyms_weak.c kfunc_call_test_subprog.c
+LSKELS_EXTRA := test_ksyms_module.c test_ksyms_weak.c kfunc_call_test.c \
+ kfunc_call_test_subprog.c
SKEL_BLACKLIST += $$(LSKELS)
test_static_linked.skel.h-deps := test_static_linked1.o test_static_linked2.o
diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
index eede7c304f86..9dfbe5355a2d 100644
--- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
+++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
@@ -2,6 +2,7 @@
/* Copyright (c) 2021 Facebook */
#include <test_progs.h>
#include <network_helpers.h>
+#include "kfunc_call_test.skel.h"
#include "kfunc_call_test.lskel.h"
#include "kfunc_call_test_subprog.skel.h"
#include "kfunc_call_test_subprog.lskel.h"
@@ -9,9 +10,31 @@
#include "cap_helpers.h"
-static void test_main(void)
+struct kfunc_test_params {
+ const char *prog_name;
+ unsigned long lskel_prog_desc_offset;
+ int retval;
+};
+
+#define TC_TEST(name, __retval) \
+ { \
+ .prog_name = #name, \
+ .lskel_prog_desc_offset = offsetof(struct kfunc_call_test_lskel, progs.name), \
+ .retval = __retval, \
+ }
+
+static struct kfunc_test_params kfunc_tests[] = {
+ TC_TEST(kfunc_call_test1, 12),
+ TC_TEST(kfunc_call_test2, 3),
+ TC_TEST(kfunc_call_test_ref_btf_id, 0),
+};
+
+static void verify_success(struct kfunc_test_params *param)
{
- struct kfunc_call_test_lskel *skel;
+ struct kfunc_call_test_lskel *lskel = NULL;
+ struct bpf_prog_desc *lskel_prog;
+ struct kfunc_call_test *skel;
+ struct bpf_program *prog;
int prog_fd, err;
LIBBPF_OPTS(bpf_test_run_opts, topts,
.data_in = &pkt_v4,
@@ -19,26 +42,53 @@ static void test_main(void)
.repeat = 1,
);
- skel = kfunc_call_test_lskel__open_and_load();
+ /* first test with normal libbpf */
+ skel = kfunc_call_test__open_and_load();
if (!ASSERT_OK_PTR(skel, "skel"))
return;
- prog_fd = skel->progs.kfunc_call_test1.prog_fd;
- err = bpf_prog_test_run_opts(prog_fd, &topts);
- ASSERT_OK(err, "bpf_prog_test_run(test1)");
- ASSERT_EQ(topts.retval, 12, "test1-retval");
+ prog = bpf_object__find_program_by_name(skel->obj, param->prog_name);
+ if (!ASSERT_OK_PTR(prog, "bpf_object__find_program_by_name"))
+ goto cleanup;
- prog_fd = skel->progs.kfunc_call_test2.prog_fd;
+ prog_fd = bpf_program__fd(prog);
err = bpf_prog_test_run_opts(prog_fd, &topts);
- ASSERT_OK(err, "bpf_prog_test_run(test2)");
- ASSERT_EQ(topts.retval, 3, "test2-retval");
+ if (!ASSERT_OK(err, param->prog_name))
+ goto cleanup;
+
+ if (!ASSERT_EQ(topts.retval, param->retval, "retval"))
+ goto cleanup;
+
+ /* second test with light skeletons */
+ lskel = kfunc_call_test_lskel__open_and_load();
+ if (!ASSERT_OK_PTR(lskel, "lskel"))
+ goto cleanup;
+
+ lskel_prog = (struct bpf_prog_desc *)((char *)lskel + param->lskel_prog_desc_offset);
- prog_fd = skel->progs.kfunc_call_test_ref_btf_id.prog_fd;
+ prog_fd = lskel_prog->prog_fd;
err = bpf_prog_test_run_opts(prog_fd, &topts);
- ASSERT_OK(err, "bpf_prog_test_run(test_ref_btf_id)");
- ASSERT_EQ(topts.retval, 0, "test_ref_btf_id-retval");
+ if (!ASSERT_OK(err, param->prog_name))
+ goto cleanup;
+
+ ASSERT_EQ(topts.retval, param->retval, "retval");
+
+cleanup:
+ kfunc_call_test__destroy(skel);
+ if (lskel)
+ kfunc_call_test_lskel__destroy(lskel);
+}
+
+static void test_main(void)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(kfunc_tests); i++) {
+ if (!test__start_subtest(kfunc_tests[i].prog_name))
+ continue;
- kfunc_call_test_lskel__destroy(skel);
+ verify_success(&kfunc_tests[i]);
+ }
}
static void test_subprog(void)
@@ -121,8 +171,7 @@ static void test_destructive(void)
void test_kfunc_call(void)
{
- if (test__start_subtest("main"))
- test_main();
+ test_main();
if (test__start_subtest("subprog"))
test_subprog();
--
2.36.1
btf_check_subprog_arg_match() was used twice in verifier.c:
- when checking for the type mismatches between a (sub)prog declaration
and BTF
- when checking the call of a subprog to see if the provided arguments
are correct and valid
This is problematic when we check if the first argument of a program
(pointer to ctx) is correctly accessed:
To be able to ensure we access a valid memory in the ctx, the verifier
assumes the pointer to context is not null.
This has the side effect of marking the program accessing the entire
context, even if the context is never dereferenced.
For example, by checking the context access with the current code, the
following eBPF program would fail with -EINVAL if the ctx is set to null
from the userspace:
```
SEC("syscall")
int prog(struct my_ctx *args) {
return 0;
}
```
In that particular case, we do not want to actually check that the memory
is correct while checking for the BTF validity, but we just want to
ensure that the (sub)prog definition matches the BTF we have.
So split btf_check_subprog_arg_match() in two so we can actually check
for the memory used when in a call, and ignore that part when not.
Note that a further patch is in preparation to disentangled
btf_check_func_arg_match() from these two purposes, and so right now we
just add a new hack around that by adding a boolean to this function.
Signed-off-by: Benjamin Tissoires <[email protected]>
---
no changes in v11
new in v10
---
include/linux/bpf.h | 2 ++
kernel/bpf/btf.c | 54 +++++++++++++++++++++++++++++++++++++++----
kernel/bpf/verifier.c | 2 +-
3 files changed, 52 insertions(+), 6 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 9c1674973e03..c9c72a089579 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1943,6 +1943,8 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
struct bpf_reg_state;
int btf_check_subprog_arg_match(struct bpf_verifier_env *env, int subprog,
struct bpf_reg_state *regs);
+int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog,
+ struct bpf_reg_state *regs);
int btf_check_kfunc_arg_match(struct bpf_verifier_env *env,
const struct btf *btf, u32 func_id,
struct bpf_reg_state *regs,
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 903719b89238..eca9ea78ee5f 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6170,7 +6170,8 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
const struct btf *btf, u32 func_id,
struct bpf_reg_state *regs,
bool ptr_to_mem_ok,
- u32 kfunc_flags)
+ u32 kfunc_flags,
+ bool processing_call)
{
enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
bool rel = false, kptr_get = false, trusted_arg = false;
@@ -6356,7 +6357,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
reg_ref_tname);
return -EINVAL;
}
- } else if (ptr_to_mem_ok) {
+ } else if (ptr_to_mem_ok && processing_call) {
const struct btf_type *resolve_ret;
u32 type_size;
@@ -6431,7 +6432,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
return rel ? ref_regno : 0;
}
-/* Compare BTF of a function with given bpf_reg_state.
+/* Compare BTF of a function declaration with given bpf_reg_state.
* Returns:
* EFAULT - there is a verifier bug. Abort verification.
* EINVAL - there is a type mismatch or BTF is not available.
@@ -6458,7 +6459,50 @@ int btf_check_subprog_arg_match(struct bpf_verifier_env *env, int subprog,
return -EINVAL;
is_global = prog->aux->func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL;
- err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global, 0);
+ err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global, 0, false);
+
+ /* Compiler optimizations can remove arguments from static functions
+ * or mismatched type can be passed into a global function.
+ * In such cases mark the function as unreliable from BTF point of view.
+ */
+ if (err)
+ prog->aux->func_info_aux[subprog].unreliable = true;
+ return err;
+}
+
+/* Compare BTF of a function call with given bpf_reg_state.
+ * Returns:
+ * EFAULT - there is a verifier bug. Abort verification.
+ * EINVAL - there is a type mismatch or BTF is not available.
+ * 0 - BTF matches with what bpf_reg_state expects.
+ * Only PTR_TO_CTX and SCALAR_VALUE states are recognized.
+ *
+ * NOTE: the code is duplicated from btf_check_subprog_arg_match()
+ * because btf_check_func_arg_match() is still doing both. Once that
+ * function is split in 2, we can call from here btf_check_subprog_arg_match()
+ * first, and then treat the calling part in a new code path.
+ */
+int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog,
+ struct bpf_reg_state *regs)
+{
+ struct bpf_prog *prog = env->prog;
+ struct btf *btf = prog->aux->btf;
+ bool is_global;
+ u32 btf_id;
+ int err;
+
+ if (!prog->aux->func_info)
+ return -EINVAL;
+
+ btf_id = prog->aux->func_info[subprog].type_id;
+ if (!btf_id)
+ return -EFAULT;
+
+ if (prog->aux->func_info_aux[subprog].unreliable)
+ return -EINVAL;
+
+ is_global = prog->aux->func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL;
+ err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global, 0, true);
/* Compiler optimizations can remove arguments from static functions
* or mismatched type can be passed into a global function.
@@ -6474,7 +6518,7 @@ int btf_check_kfunc_arg_match(struct bpf_verifier_env *env,
struct bpf_reg_state *regs,
u32 kfunc_flags)
{
- return btf_check_func_arg_match(env, btf, func_id, regs, true, kfunc_flags);
+ return btf_check_func_arg_match(env, btf, func_id, regs, true, kfunc_flags, true);
}
/* Convert BTF of a function into bpf_reg_state if possible
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0194a36d0b36..d27fae3ce949 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6626,7 +6626,7 @@ static int __check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn
func_info_aux = env->prog->aux->func_info_aux;
if (func_info_aux)
is_global = func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL;
- err = btf_check_subprog_arg_match(env, subprog, caller->regs);
+ err = btf_check_subprog_call(env, subprog, caller->regs);
if (err == -EFAULT)
return err;
if (is_global) {
--
2.36.1
On Tue, 6 Sept 2022 at 17:13, Benjamin Tissoires
<[email protected]> wrote:
>
> btf_check_subprog_arg_match() was used twice in verifier.c:
> - when checking for the type mismatches between a (sub)prog declaration
> and BTF
> - when checking the call of a subprog to see if the provided arguments
> are correct and valid
>
> This is problematic when we check if the first argument of a program
> (pointer to ctx) is correctly accessed:
> To be able to ensure we access a valid memory in the ctx, the verifier
> assumes the pointer to context is not null.
> This has the side effect of marking the program accessing the entire
> context, even if the context is never dereferenced.
>
> For example, by checking the context access with the current code, the
> following eBPF program would fail with -EINVAL if the ctx is set to null
> from the userspace:
>
> ```
> SEC("syscall")
> int prog(struct my_ctx *args) {
> return 0;
> }
> ```
>
> In that particular case, we do not want to actually check that the memory
> is correct while checking for the BTF validity, but we just want to
> ensure that the (sub)prog definition matches the BTF we have.
>
> So split btf_check_subprog_arg_match() in two so we can actually check
> for the memory used when in a call, and ignore that part when not.
>
> Note that a further patch is in preparation to disentangled
> btf_check_func_arg_match() from these two purposes, and so right now we
> just add a new hack around that by adding a boolean to this function.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>
>
> ---
>
Given I'll fix it properly in my kfunc rework, LGTM otherwise:
Acked-by: Kumar Kartikeya Dwivedi <[email protected]>
> no changes in v11
>
> new in v10
> ---
> include/linux/bpf.h | 2 ++
> kernel/bpf/btf.c | 54 +++++++++++++++++++++++++++++++++++++++----
> kernel/bpf/verifier.c | 2 +-
> 3 files changed, 52 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 9c1674973e03..c9c72a089579 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1943,6 +1943,8 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
> struct bpf_reg_state;
> int btf_check_subprog_arg_match(struct bpf_verifier_env *env, int subprog,
> struct bpf_reg_state *regs);
> +int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog,
> + struct bpf_reg_state *regs);
> int btf_check_kfunc_arg_match(struct bpf_verifier_env *env,
> const struct btf *btf, u32 func_id,
> struct bpf_reg_state *regs,
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 903719b89238..eca9ea78ee5f 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6170,7 +6170,8 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
> const struct btf *btf, u32 func_id,
> struct bpf_reg_state *regs,
> bool ptr_to_mem_ok,
> - u32 kfunc_flags)
> + u32 kfunc_flags,
> + bool processing_call)
> {
> enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
> bool rel = false, kptr_get = false, trusted_arg = false;
> @@ -6356,7 +6357,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
> reg_ref_tname);
> return -EINVAL;
> }
> - } else if (ptr_to_mem_ok) {
> + } else if (ptr_to_mem_ok && processing_call) {
> const struct btf_type *resolve_ret;
> u32 type_size;
>
> @@ -6431,7 +6432,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
> return rel ? ref_regno : 0;
> }
>
> -/* Compare BTF of a function with given bpf_reg_state.
> +/* Compare BTF of a function declaration with given bpf_reg_state.
> * Returns:
> * EFAULT - there is a verifier bug. Abort verification.
> * EINVAL - there is a type mismatch or BTF is not available.
> @@ -6458,7 +6459,50 @@ int btf_check_subprog_arg_match(struct bpf_verifier_env *env, int subprog,
> return -EINVAL;
>
> is_global = prog->aux->func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL;
> - err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global, 0);
> + err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global, 0, false);
> +
> + /* Compiler optimizations can remove arguments from static functions
> + * or mismatched type can be passed into a global function.
> + * In such cases mark the function as unreliable from BTF point of view.
> + */
> + if (err)
> + prog->aux->func_info_aux[subprog].unreliable = true;
> + return err;
> +}
> +
> +/* Compare BTF of a function call with given bpf_reg_state.
> + * Returns:
> + * EFAULT - there is a verifier bug. Abort verification.
> + * EINVAL - there is a type mismatch or BTF is not available.
> + * 0 - BTF matches with what bpf_reg_state expects.
> + * Only PTR_TO_CTX and SCALAR_VALUE states are recognized.
> + *
> + * NOTE: the code is duplicated from btf_check_subprog_arg_match()
> + * because btf_check_func_arg_match() is still doing both. Once that
> + * function is split in 2, we can call from here btf_check_subprog_arg_match()
> + * first, and then treat the calling part in a new code path.
> + */
> +int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog,
> + struct bpf_reg_state *regs)
> +{
> + struct bpf_prog *prog = env->prog;
> + struct btf *btf = prog->aux->btf;
> + bool is_global;
> + u32 btf_id;
> + int err;
> +
> + if (!prog->aux->func_info)
> + return -EINVAL;
> +
> + btf_id = prog->aux->func_info[subprog].type_id;
> + if (!btf_id)
> + return -EFAULT;
> +
> + if (prog->aux->func_info_aux[subprog].unreliable)
> + return -EINVAL;
> +
> + is_global = prog->aux->func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL;
> + err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global, 0, true);
>
> /* Compiler optimizations can remove arguments from static functions
> * or mismatched type can be passed into a global function.
> @@ -6474,7 +6518,7 @@ int btf_check_kfunc_arg_match(struct bpf_verifier_env *env,
> struct bpf_reg_state *regs,
> u32 kfunc_flags)
> {
> - return btf_check_func_arg_match(env, btf, func_id, regs, true, kfunc_flags);
> + return btf_check_func_arg_match(env, btf, func_id, regs, true, kfunc_flags, true);
> }
>
> /* Convert BTF of a function into bpf_reg_state if possible
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 0194a36d0b36..d27fae3ce949 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -6626,7 +6626,7 @@ static int __check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn
> func_info_aux = env->prog->aux->func_info_aux;
> if (func_info_aux)
> is_global = func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL;
> - err = btf_check_subprog_arg_match(env, subprog, caller->regs);
> + err = btf_check_subprog_call(env, subprog, caller->regs);
> if (err == -EFAULT)
> return err;
> if (is_global) {
> --
> 2.36.1
>
On Tue, 6 Sept 2022 at 17:13, Benjamin Tissoires
<[email protected]> wrote:
>
> Similar to tools/testing/selftests/bpf/prog_tests/dynptr.c:
> we declare an array of tests that we run one by one in a for loop.
>
> Followup patches will add more similar-ish tests, so avoid a lot of copy
> paste by grouping the declaration in an array.
>
> For light skeletons, we have to rely on the offsetof() macro so we can
> statically declare which program we are using.
> In the libbpf case, we can rely on bpf_object__find_program_by_name().
> So also change the Makefile to generate both light skeletons and normal
> ones.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>
>
> ---
Acked-by: Kumar Kartikeya Dwivedi <[email protected]>
>
> changes in v11:
> - use of both light skeletons and normal libbpf
>
> new in v10
> ---
> tools/testing/selftests/bpf/Makefile | 5 +-
> .../selftests/bpf/prog_tests/kfunc_call.c | 81 +++++++++++++++----
> 2 files changed, 68 insertions(+), 18 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index eecad99f1735..314216d77b8d 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -351,11 +351,12 @@ LINKED_SKELS := test_static_linked.skel.h linked_funcs.skel.h \
> test_subskeleton.skel.h test_subskeleton_lib.skel.h \
> test_usdt.skel.h
>
> -LSKELS := kfunc_call_test.c fentry_test.c fexit_test.c fexit_sleep.c \
> +LSKELS := fentry_test.c fexit_test.c fexit_sleep.c \
> test_ringbuf.c atomics.c trace_printk.c trace_vprintk.c \
> map_ptr_kern.c core_kern.c core_kern_overflow.c
> # Generate both light skeleton and libbpf skeleton for these
> -LSKELS_EXTRA := test_ksyms_module.c test_ksyms_weak.c kfunc_call_test_subprog.c
> +LSKELS_EXTRA := test_ksyms_module.c test_ksyms_weak.c kfunc_call_test.c \
> + kfunc_call_test_subprog.c
> SKEL_BLACKLIST += $$(LSKELS)
>
> test_static_linked.skel.h-deps := test_static_linked1.o test_static_linked2.o
> diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
> index eede7c304f86..9dfbe5355a2d 100644
> --- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
> +++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
> @@ -2,6 +2,7 @@
> /* Copyright (c) 2021 Facebook */
> #include <test_progs.h>
> #include <network_helpers.h>
> +#include "kfunc_call_test.skel.h"
> #include "kfunc_call_test.lskel.h"
> #include "kfunc_call_test_subprog.skel.h"
> #include "kfunc_call_test_subprog.lskel.h"
> @@ -9,9 +10,31 @@
>
> #include "cap_helpers.h"
>
> -static void test_main(void)
> +struct kfunc_test_params {
> + const char *prog_name;
> + unsigned long lskel_prog_desc_offset;
> + int retval;
> +};
> +
> +#define TC_TEST(name, __retval) \
> + { \
> + .prog_name = #name, \
> + .lskel_prog_desc_offset = offsetof(struct kfunc_call_test_lskel, progs.name), \
> + .retval = __retval, \
> + }
> +
> +static struct kfunc_test_params kfunc_tests[] = {
> + TC_TEST(kfunc_call_test1, 12),
> + TC_TEST(kfunc_call_test2, 3),
> + TC_TEST(kfunc_call_test_ref_btf_id, 0),
> +};
> +
> +static void verify_success(struct kfunc_test_params *param)
> {
> - struct kfunc_call_test_lskel *skel;
> + struct kfunc_call_test_lskel *lskel = NULL;
> + struct bpf_prog_desc *lskel_prog;
> + struct kfunc_call_test *skel;
> + struct bpf_program *prog;
> int prog_fd, err;
> LIBBPF_OPTS(bpf_test_run_opts, topts,
> .data_in = &pkt_v4,
> @@ -19,26 +42,53 @@ static void test_main(void)
> .repeat = 1,
> );
>
> - skel = kfunc_call_test_lskel__open_and_load();
> + /* first test with normal libbpf */
> + skel = kfunc_call_test__open_and_load();
> if (!ASSERT_OK_PTR(skel, "skel"))
> return;
>
> - prog_fd = skel->progs.kfunc_call_test1.prog_fd;
> - err = bpf_prog_test_run_opts(prog_fd, &topts);
> - ASSERT_OK(err, "bpf_prog_test_run(test1)");
> - ASSERT_EQ(topts.retval, 12, "test1-retval");
> + prog = bpf_object__find_program_by_name(skel->obj, param->prog_name);
> + if (!ASSERT_OK_PTR(prog, "bpf_object__find_program_by_name"))
> + goto cleanup;
>
> - prog_fd = skel->progs.kfunc_call_test2.prog_fd;
> + prog_fd = bpf_program__fd(prog);
> err = bpf_prog_test_run_opts(prog_fd, &topts);
> - ASSERT_OK(err, "bpf_prog_test_run(test2)");
> - ASSERT_EQ(topts.retval, 3, "test2-retval");
> + if (!ASSERT_OK(err, param->prog_name))
> + goto cleanup;
> +
> + if (!ASSERT_EQ(topts.retval, param->retval, "retval"))
> + goto cleanup;
> +
> + /* second test with light skeletons */
> + lskel = kfunc_call_test_lskel__open_and_load();
> + if (!ASSERT_OK_PTR(lskel, "lskel"))
> + goto cleanup;
> +
> + lskel_prog = (struct bpf_prog_desc *)((char *)lskel + param->lskel_prog_desc_offset);
>
> - prog_fd = skel->progs.kfunc_call_test_ref_btf_id.prog_fd;
> + prog_fd = lskel_prog->prog_fd;
> err = bpf_prog_test_run_opts(prog_fd, &topts);
> - ASSERT_OK(err, "bpf_prog_test_run(test_ref_btf_id)");
> - ASSERT_EQ(topts.retval, 0, "test_ref_btf_id-retval");
> + if (!ASSERT_OK(err, param->prog_name))
> + goto cleanup;
> +
> + ASSERT_EQ(topts.retval, param->retval, "retval");
> +
> +cleanup:
> + kfunc_call_test__destroy(skel);
> + if (lskel)
> + kfunc_call_test_lskel__destroy(lskel);
> +}
> +
> +static void test_main(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(kfunc_tests); i++) {
> + if (!test__start_subtest(kfunc_tests[i].prog_name))
> + continue;
>
> - kfunc_call_test_lskel__destroy(skel);
> + verify_success(&kfunc_tests[i]);
> + }
> }
>
> static void test_subprog(void)
> @@ -121,8 +171,7 @@ static void test_destructive(void)
>
> void test_kfunc_call(void)
> {
> - if (test__start_subtest("main"))
> - test_main();
> + test_main();
>
> if (test__start_subtest("subprog"))
> test_subprog();
> --
> 2.36.1
>
Hello:
This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[email protected]>:
On Tue, 6 Sep 2022 17:12:56 +0200 you wrote:
> Hi,
>
> well, given that the HID changes haven't moved a lot in the past
> revisions and that I am cc-ing a bunch of people, I have dropped them
> while we focus on the last 2 requirements in bpf-core changes.
>
> I'll submit a HID targeted series when we get these in tree, which
> would make things a lore more independent.
>
> [...]
Here is the summary with links:
- [bpf-next,v11,1/7] selftests/bpf: regroup and declare similar kfuncs selftests in an array
https://git.kernel.org/bpf/bpf-next/c/012ba1156e4a
- [bpf-next,v11,2/7] bpf: split btf_check_subprog_arg_match in two
https://git.kernel.org/bpf/bpf-next/c/95f2f26f3cac
- [bpf-next,v11,3/7] bpf/verifier: allow all functions to read user provided context
https://git.kernel.org/bpf/bpf-next/c/15baa55ff5b0
- [bpf-next,v11,4/7] selftests/bpf: add test for accessing ctx from syscall program type
https://git.kernel.org/bpf/bpf-next/c/fb66223a244f
- [bpf-next,v11,5/7] bpf/btf: bump BTF_KFUNC_SET_MAX_CNT
https://git.kernel.org/bpf/bpf-next/c/f9b348185f4d
- [bpf-next,v11,6/7] bpf/verifier: allow kfunc to return an allocated mem
https://git.kernel.org/bpf/bpf-next/c/eb1f7f71c126
- [bpf-next,v11,7/7] selftests/bpf: Add tests for kfunc returning a memory pointer
https://git.kernel.org/bpf/bpf-next/c/22ed8d5a4652
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html