lkml.org 
[lkml]   [2023]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH bpf-next v2 4/4] bpf/selftests: Verify struct_ops prog sleepable behavior
From
On 1/24/23 8:08 AM, David Vernet wrote:
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 50123afab9bf..64034311c5f7 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1474,6 +1474,7 @@ struct bpf_dummy_ops {
> int (*test_1)(struct bpf_dummy_ops_state *cb);
> int (*test_2)(struct bpf_dummy_ops_state *cb, int a1, unsigned short a2,
> char a3, unsigned long a4);
> + int (*test_3)(struct bpf_dummy_ops_state *cb);

nit. May be a self describe name like test_sleepable().

> };
>
> int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,
> diff --git a/net/bpf/bpf_dummy_struct_ops.c b/net/bpf/bpf_dummy_struct_ops.c
> index 1ac4467928a9..46099737d1da 100644
> --- a/net/bpf/bpf_dummy_struct_ops.c
> +++ b/net/bpf/bpf_dummy_struct_ops.c
> @@ -154,6 +154,23 @@ static bool bpf_dummy_ops_is_valid_access(int off, int size,
> return bpf_tracing_btf_ctx_access(off, size, type, prog, info);
> }
>
> +static int bpf_dummy_ops_check_member(const struct btf_type *t,
> + const struct btf_member *member,
> + const struct bpf_prog *prog)
> +{
> + u32 moff = __btf_member_bit_offset(t, member) / 8;
> +
> + switch (moff) {
> + case offsetof(struct bpf_dummy_ops, test_3):
> + break;
> + default:
> + if (prog->aux->sleepable)
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int bpf_dummy_ops_btf_struct_access(struct bpf_verifier_log *log,
> const struct bpf_reg_state *reg,
> int off, int size, enum bpf_access_type atype,
> @@ -208,6 +225,7 @@ static void bpf_dummy_unreg(void *kdata)
> struct bpf_struct_ops bpf_bpf_dummy_ops = {
> .verifier_ops = &bpf_dummy_verifier_ops,
> .init = bpf_dummy_init,
> + .check_member = bpf_dummy_ops_check_member,
> .init_member = bpf_dummy_init_member,
> .reg = bpf_dummy_reg,
> .unreg = bpf_dummy_unreg,
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 8da0d73b368e..33ea57d34c0b 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -730,6 +730,10 @@ noinline void bpf_kfunc_call_test_destructive(void)
> {
> }
>
> +noinline void bpf_kfunc_call_test_sleepable(void)
> +{
> +}
> +
> __diag_pop();
>
> BTF_SET8_START(bpf_test_modify_return_ids)
> @@ -767,6 +771,7 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail1)
> BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail2)
> BTF_ID_FLAGS(func, bpf_kfunc_call_test_ref, KF_TRUSTED_ARGS)
> BTF_ID_FLAGS(func, bpf_kfunc_call_test_destructive, KF_DESTRUCTIVE)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_test_sleepable, KF_SLEEPABLE)

KF_SLEEPABLE kfunc is not specific to the struct_ops prog. I hope a test has
already covered that KF_SLEEPABLE kfunc can only be called from sleepable prog.
eg. there is bpf_fentry_test1.

This new kfunc could then be omitted and make the test simpler. There is no need
to add the test to the DENYLIST.s390x:
https://github.com/kernel-patches/bpf/actions/runs/3998188872/jobs/6861920516

> diff --git a/tools/testing/selftests/bpf/progs/dummy_st_ops_common.h b/tools/testing/selftests/bpf/progs/dummy_st_ops_common.h
> new file mode 100644
> index 000000000000..7d0761594b69
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/dummy_st_ops_common.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
> +
> +#ifndef _DUMMY_ST_OPS_COMMON_H
> +#define _DUMMY_ST_OPS_COMMON_H
> +
> +struct bpf_dummy_ops_state {
> + int val;
> +} __attribute__((preserve_access_index));
> +
> +struct bpf_dummy_ops {
> + int (*test_1)(struct bpf_dummy_ops_state *state);
> + int (*test_2)(struct bpf_dummy_ops_state *state, int a1, unsigned short a2,
> + char a3, unsigned long a4);
> + int (*test_3)(struct bpf_dummy_ops_state *state);
> +};

Instead of adding a new dummy_st_ops_common.h header, try to directly include
vmlinux.h in the dummy_st_ops_{success,fail}.c.

> +
> +void bpf_kfunc_call_test_sleepable(void) __ksym;

\
 
 \ /
  Last update: 2023-03-26 23:54    [W:0.056 / U:0.728 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site