diff mbox series

[bpf-next,v1,2/2] selftests/bpf: Add tests for preempt kfuncs

Message ID 20240423061922.2295517-3-memxor@gmail.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series Introduce bpf_preempt_{disable,enable} | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 949 this patch: 949
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 10 maintainers not CCed: kpsingh@kernel.org john.fastabend@gmail.com shuah@kernel.org jolsa@kernel.org song@kernel.org linux-kselftest@vger.kernel.org martin.lau@linux.dev mykolal@fb.com haoluo@google.com sdf@google.com
netdev/build_clang success Errors and warnings before: 955 this patch: 955
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 961 this patch: 961
netdev/checkpatch warning CHECK: No space is necessary after a cast WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-18 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_maps, false, 360) / test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-PR fail merge-conflict

Commit Message

Kumar Kartikeya Dwivedi April 23, 2024, 6:19 a.m. UTC 
Add tests for nested cases, nested count preservation upon different
subprog calls that disable/enable preemption, and test sleepable helper
call in non-preemptible regions.

181/1   preempt_lock/preempt_lock_missing_1:OK
181/2   preempt_lock/preempt_lock_missing_2:OK
181/3   preempt_lock/preempt_lock_missing_3:OK
181/4   preempt_lock/preempt_lock_missing_3_minus_2:OK
181/5   preempt_lock/preempt_lock_missing_1_subprog:OK
181/6   preempt_lock/preempt_lock_missing_2_subprog:OK
181/7   preempt_lock/preempt_lock_missing_2_minus_1_subprog:OK
181/8   preempt_lock/preempt_balance:OK
181/9   preempt_lock/preempt_balance_subprog_test:OK
181/10  preempt_lock/preempt_sleepable_helper:OK
181     preempt_lock:OK
Summary: 1/10 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/prog_tests/preempt_lock.c   |   9 ++
 .../selftests/bpf/progs/preempt_lock.c        | 119 ++++++++++++++++++
 2 files changed, 128 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/preempt_lock.c
 create mode 100644 tools/testing/selftests/bpf/progs/preempt_lock.c

Comments

Jiri Olsa April 23, 2024, 11:17 a.m. UTC | #1 
On Tue, Apr 23, 2024 at 06:19:22AM +0000, Kumar Kartikeya Dwivedi wrote:
> Add tests for nested cases, nested count preservation upon different
> subprog calls that disable/enable preemption, and test sleepable helper
> call in non-preemptible regions.
> 
> 181/1   preempt_lock/preempt_lock_missing_1:OK
> 181/2   preempt_lock/preempt_lock_missing_2:OK
> 181/3   preempt_lock/preempt_lock_missing_3:OK
> 181/4   preempt_lock/preempt_lock_missing_3_minus_2:OK
> 181/5   preempt_lock/preempt_lock_missing_1_subprog:OK
> 181/6   preempt_lock/preempt_lock_missing_2_subprog:OK
> 181/7   preempt_lock/preempt_lock_missing_2_minus_1_subprog:OK
> 181/8   preempt_lock/preempt_balance:OK
> 181/9   preempt_lock/preempt_balance_subprog_test:OK
> 181/10  preempt_lock/preempt_sleepable_helper:OK

should we also check that the global function call is not allowed?

jirka

> 181     preempt_lock:OK
> Summary: 1/10 PASSED, 0 SKIPPED, 0 FAILED
> 
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  .../selftests/bpf/prog_tests/preempt_lock.c   |   9 ++
>  .../selftests/bpf/progs/preempt_lock.c        | 119 ++++++++++++++++++
>  2 files changed, 128 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/preempt_lock.c
>  create mode 100644 tools/testing/selftests/bpf/progs/preempt_lock.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/preempt_lock.c b/tools/testing/selftests/bpf/prog_tests/preempt_lock.c
> new file mode 100644
> index 000000000000..02917c672441
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/preempt_lock.c
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include <network_helpers.h>
> +#include <preempt_lock.skel.h>
> +
> +void test_preempt_lock(void)
> +{
> +	RUN_TESTS(preempt_lock);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/preempt_lock.c b/tools/testing/selftests/bpf/progs/preempt_lock.c
> new file mode 100644
> index 000000000000..53320ea80fa4
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/preempt_lock.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include "bpf_misc.h"
> +
> +void bpf_preempt_disable(void) __ksym;
> +void bpf_preempt_enable(void) __ksym;
> +
> +SEC("?tc")
> +__failure __msg("1 bpf_preempt_enable is missing")
> +int preempt_lock_missing_1(struct __sk_buff *ctx)
> +{
> +	bpf_preempt_disable();
> +	return 0;
> +}
> +
> +SEC("?tc")
> +__failure __msg("2 bpf_preempt_enable(s) are missing")
> +int preempt_lock_missing_2(struct __sk_buff *ctx)
> +{
> +	bpf_preempt_disable();
> +	bpf_preempt_disable();
> +	return 0;
> +}
> +
> +SEC("?tc")
> +__failure __msg("3 bpf_preempt_enable(s) are missing")
> +int preempt_lock_missing_3(struct __sk_buff *ctx)
> +{
> +	bpf_preempt_disable();
> +	bpf_preempt_disable();
> +	bpf_preempt_disable();
> +	return 0;
> +}
> +
> +SEC("?tc")
> +__failure __msg("1 bpf_preempt_enable is missing")
> +int preempt_lock_missing_3_minus_2(struct __sk_buff *ctx)
> +{
> +	bpf_preempt_disable();
> +	bpf_preempt_disable();
> +	bpf_preempt_disable();
> +	bpf_preempt_enable();
> +	bpf_preempt_enable();
> +	return 0;
> +}
> +
> +static __noinline void preempt_disable(void)
> +{
> +	bpf_preempt_disable();
> +}
> +
> +static __noinline void preempt_enable(void)
> +{
> +	bpf_preempt_enable();
> +}
> +
> +SEC("?tc")
> +__failure __msg("1 bpf_preempt_enable is missing")
> +int preempt_lock_missing_1_subprog(struct __sk_buff *ctx)
> +{
> +	preempt_disable();
> +	return 0;
> +}
> +
> +SEC("?tc")
> +__failure __msg("2 bpf_preempt_enable(s) are missing")
> +int preempt_lock_missing_2_subprog(struct __sk_buff *ctx)
> +{
> +	preempt_disable();
> +	preempt_disable();
> +	return 0;
> +}
> +
> +SEC("?tc")
> +__failure __msg("1 bpf_preempt_enable is missing")
> +int preempt_lock_missing_2_minus_1_subprog(struct __sk_buff *ctx)
> +{
> +	preempt_disable();
> +	preempt_disable();
> +	preempt_enable();
> +	return 0;
> +}
> +
> +static __noinline void preempt_balance_subprog(void)
> +{
> +	preempt_disable();
> +	preempt_enable();
> +}
> +
> +SEC("?tc")
> +__success int preempt_balance(struct __sk_buff *ctx)
> +{
> +	bpf_preempt_disable();
> +	bpf_preempt_enable();
> +	return 0;
> +}
> +
> +SEC("?tc")
> +__success int preempt_balance_subprog_test(struct __sk_buff *ctx)
> +{
> +	preempt_balance_subprog();
> +	return 0;
> +}
> +
> +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
> +__failure __msg("sleepable helper bpf_copy_from_user#")
> +int preempt_sleepable_helper(void *ctx)
> +{
> +	u32 data;
> +
> +	bpf_preempt_disable();
> +	bpf_copy_from_user(&data, sizeof(data), NULL);
> +	bpf_preempt_enable();
> +	return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> -- 
> 2.43.0
> 
>
Kumar Kartikeya Dwivedi April 23, 2024, 12:06 p.m. UTC | #2 
On Tue, 23 Apr 2024 at 13:17, Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Tue, Apr 23, 2024 at 06:19:22AM +0000, Kumar Kartikeya Dwivedi wrote:
> > Add tests for nested cases, nested count preservation upon different
> > subprog calls that disable/enable preemption, and test sleepable helper
> > call in non-preemptible regions.
> >
> > 181/1   preempt_lock/preempt_lock_missing_1:OK
> > 181/2   preempt_lock/preempt_lock_missing_2:OK
> > 181/3   preempt_lock/preempt_lock_missing_3:OK
> > 181/4   preempt_lock/preempt_lock_missing_3_minus_2:OK
> > 181/5   preempt_lock/preempt_lock_missing_1_subprog:OK
> > 181/6   preempt_lock/preempt_lock_missing_2_subprog:OK
> > 181/7   preempt_lock/preempt_lock_missing_2_minus_1_subprog:OK
> > 181/8   preempt_lock/preempt_balance:OK
> > 181/9   preempt_lock/preempt_balance_subprog_test:OK
> > 181/10  preempt_lock/preempt_sleepable_helper:OK
>
> should we also check that the global function call is not allowed?
>

Good point, that is missing, I'll wait for more reviews and then
respin with a failure test for this.

> jirka
>
Alexei Starovoitov April 23, 2024, 3:02 p.m. UTC | #3 
On Tue, Apr 23, 2024 at 5:06 AM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Tue, 23 Apr 2024 at 13:17, Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Tue, Apr 23, 2024 at 06:19:22AM +0000, Kumar Kartikeya Dwivedi wrote:
> > > Add tests for nested cases, nested count preservation upon different
> > > subprog calls that disable/enable preemption, and test sleepable helper
> > > call in non-preemptible regions.
> > >
> > > 181/1   preempt_lock/preempt_lock_missing_1:OK
> > > 181/2   preempt_lock/preempt_lock_missing_2:OK
> > > 181/3   preempt_lock/preempt_lock_missing_3:OK
> > > 181/4   preempt_lock/preempt_lock_missing_3_minus_2:OK
> > > 181/5   preempt_lock/preempt_lock_missing_1_subprog:OK
> > > 181/6   preempt_lock/preempt_lock_missing_2_subprog:OK
> > > 181/7   preempt_lock/preempt_lock_missing_2_minus_1_subprog:OK
> > > 181/8   preempt_lock/preempt_balance:OK
> > > 181/9   preempt_lock/preempt_balance_subprog_test:OK
> > > 181/10  preempt_lock/preempt_sleepable_helper:OK
> >
> > should we also check that the global function call is not allowed?
> >
>
> Good point, that is missing, I'll wait for more reviews and then
> respin with a failure test for this.

I couldn't find the check in patch 1 that does:
"Global functions are disallowed from being called".

And I agree that we need to allow global funcs in preempt disabled region.
Sounds like you're planning that in the follow up.
Kumar Kartikeya Dwivedi April 23, 2024, 4:17 p.m. UTC | #4 
On Tue, 23 Apr 2024 at 17:02, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Apr 23, 2024 at 5:06 AM Kumar Kartikeya Dwivedi
> <memxor@gmail.com> wrote:
> >
> > On Tue, 23 Apr 2024 at 13:17, Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Tue, Apr 23, 2024 at 06:19:22AM +0000, Kumar Kartikeya Dwivedi wrote:
> > > > Add tests for nested cases, nested count preservation upon different
> > > > subprog calls that disable/enable preemption, and test sleepable helper
> > > > call in non-preemptible regions.
> > > >
> > > > 181/1   preempt_lock/preempt_lock_missing_1:OK
> > > > 181/2   preempt_lock/preempt_lock_missing_2:OK
> > > > 181/3   preempt_lock/preempt_lock_missing_3:OK
> > > > 181/4   preempt_lock/preempt_lock_missing_3_minus_2:OK
> > > > 181/5   preempt_lock/preempt_lock_missing_1_subprog:OK
> > > > 181/6   preempt_lock/preempt_lock_missing_2_subprog:OK
> > > > 181/7   preempt_lock/preempt_lock_missing_2_minus_1_subprog:OK
> > > > 181/8   preempt_lock/preempt_balance:OK
> > > > 181/9   preempt_lock/preempt_balance_subprog_test:OK
> > > > 181/10  preempt_lock/preempt_sleepable_helper:OK
> > >
> > > should we also check that the global function call is not allowed?
> > >
> >
> > Good point, that is missing, I'll wait for more reviews and then
> > respin with a failure test for this.
>
> I couldn't find the check in patch 1 that does:
> "Global functions are disallowed from being called".

See this part：

+ /* Only global subprogs cannot be called with preemption disabled. */
+ if (env->cur_state->active_preempt_lock) {
+ verbose(env, "global function calls are not allowed with preemption
disabled,\n"
+     "use static function instead\n");
+ return -EINVAL;
+ }

>
> And I agree that we need to allow global funcs in preempt disabled region.
> Sounds like you're planning that in the follow up.

Yeah, but it's not very simple. I noticed a few more problems with
global functions before that can be done.
They need to be marked !sleepable before do_check and only verified
with !sleepable. Otherwise if that is done later in do_check the
global function will be seen in the non-preemptible section but may
have been already verified.
We need some summarization pass for both preempt and rcu_read_lock kfuncs.
Kumar Kartikeya Dwivedi April 23, 2024, 4:20 p.m. UTC | #5 
On Tue, 23 Apr 2024 at 18:17, Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
>
> On Tue, 23 Apr 2024 at 17:02, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Tue, Apr 23, 2024 at 5:06 AM Kumar Kartikeya Dwivedi
> > <memxor@gmail.com> wrote:
> > >
> > > On Tue, 23 Apr 2024 at 13:17, Jiri Olsa <olsajiri@gmail.com> wrote:
> > > >
> > > > On Tue, Apr 23, 2024 at 06:19:22AM +0000, Kumar Kartikeya Dwivedi wrote:
> > > > > Add tests for nested cases, nested count preservation upon different
> > > > > subprog calls that disable/enable preemption, and test sleepable helper
> > > > > call in non-preemptible regions.
> > > > >
> > > > > 181/1   preempt_lock/preempt_lock_missing_1:OK
> > > > > 181/2   preempt_lock/preempt_lock_missing_2:OK
> > > > > 181/3   preempt_lock/preempt_lock_missing_3:OK
> > > > > 181/4   preempt_lock/preempt_lock_missing_3_minus_2:OK
> > > > > 181/5   preempt_lock/preempt_lock_missing_1_subprog:OK
> > > > > 181/6   preempt_lock/preempt_lock_missing_2_subprog:OK
> > > > > 181/7   preempt_lock/preempt_lock_missing_2_minus_1_subprog:OK
> > > > > 181/8   preempt_lock/preempt_balance:OK
> > > > > 181/9   preempt_lock/preempt_balance_subprog_test:OK
> > > > > 181/10  preempt_lock/preempt_sleepable_helper:OK
> > > >
> > > > should we also check that the global function call is not allowed?
> > > >
> > >
> > > Good point, that is missing, I'll wait for more reviews and then
> > > respin with a failure test for this.
> >
> > I couldn't find the check in patch 1 that does:
> > "Global functions are disallowed from being called".
>
> See this part：
>
> + /* Only global subprogs cannot be called with preemption disabled. */
> + if (env->cur_state->active_preempt_lock) {
> + verbose(env, "global function calls are not allowed with preemption
> disabled,\n"
> +     "use static function instead\n");
> + return -EINVAL;
> + }
>
> >
> > And I agree that we need to allow global funcs in preempt disabled region.
> > Sounds like you're planning that in the follow up.
>
> Yeah, but it's not very simple. I noticed a few more problems with
> global functions before that can be done.
> They need to be marked !sleepable before do_check and only verified
> with !sleepable. Otherwise if that is done later in do_check the
> global function will be seen in the non-preemptible section but may
> have been already verified.
> We need some summarization pass for both preempt and rcu_read_lock kfuncs.

E.g. I also have in my TODO list (from hitting this a while ago):

+ * Global functions may clear packet pointers, but data, data_end
remain unclobbered in the caller.
They may call a helper on ctx that invalidates pkt pointers but the
caller may not see that during verification and continue accessing
them.
This needs to be known for the global function before their callers
are verified.

Similarly, the context they are called from is atomic or not
throughput the program needs to be known before their verification is
done.
Otherwise they are verified as sleepable for sleepable programs, while
they maybe called from non-preemptible region.

I will fix the latter first, but a summarization pass for them is
needed and useful to fix the former as well.
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/preempt_lock.c b/tools/testing/selftests/bpf/prog_tests/preempt_lock.c
new file mode 100644
index 000000000000..02917c672441
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/preempt_lock.c
@@ -0,0 +1,9 @@ 
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <network_helpers.h>
+#include <preempt_lock.skel.h>
+
+void test_preempt_lock(void)
+{
+	RUN_TESTS(preempt_lock);
+}
diff --git a/tools/testing/selftests/bpf/progs/preempt_lock.c b/tools/testing/selftests/bpf/progs/preempt_lock.c
new file mode 100644
index 000000000000..53320ea80fa4
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/preempt_lock.c
@@ -0,0 +1,119 @@ 
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+void bpf_preempt_disable(void) __ksym;
+void bpf_preempt_enable(void) __ksym;
+
+SEC("?tc")
+__failure __msg("1 bpf_preempt_enable is missing")
+int preempt_lock_missing_1(struct __sk_buff *ctx)
+{
+	bpf_preempt_disable();
+	return 0;
+}
+
+SEC("?tc")
+__failure __msg("2 bpf_preempt_enable(s) are missing")
+int preempt_lock_missing_2(struct __sk_buff *ctx)
+{
+	bpf_preempt_disable();
+	bpf_preempt_disable();
+	return 0;
+}
+
+SEC("?tc")
+__failure __msg("3 bpf_preempt_enable(s) are missing")
+int preempt_lock_missing_3(struct __sk_buff *ctx)
+{
+	bpf_preempt_disable();
+	bpf_preempt_disable();
+	bpf_preempt_disable();
+	return 0;
+}
+
+SEC("?tc")
+__failure __msg("1 bpf_preempt_enable is missing")
+int preempt_lock_missing_3_minus_2(struct __sk_buff *ctx)
+{
+	bpf_preempt_disable();
+	bpf_preempt_disable();
+	bpf_preempt_disable();
+	bpf_preempt_enable();
+	bpf_preempt_enable();
+	return 0;
+}
+
+static __noinline void preempt_disable(void)
+{
+	bpf_preempt_disable();
+}
+
+static __noinline void preempt_enable(void)
+{
+	bpf_preempt_enable();
+}
+
+SEC("?tc")
+__failure __msg("1 bpf_preempt_enable is missing")
+int preempt_lock_missing_1_subprog(struct __sk_buff *ctx)
+{
+	preempt_disable();
+	return 0;
+}
+
+SEC("?tc")
+__failure __msg("2 bpf_preempt_enable(s) are missing")
+int preempt_lock_missing_2_subprog(struct __sk_buff *ctx)
+{
+	preempt_disable();
+	preempt_disable();
+	return 0;
+}
+
+SEC("?tc")
+__failure __msg("1 bpf_preempt_enable is missing")
+int preempt_lock_missing_2_minus_1_subprog(struct __sk_buff *ctx)
+{
+	preempt_disable();
+	preempt_disable();
+	preempt_enable();
+	return 0;
+}
+
+static __noinline void preempt_balance_subprog(void)
+{
+	preempt_disable();
+	preempt_enable();
+}
+
+SEC("?tc")
+__success int preempt_balance(struct __sk_buff *ctx)
+{
+	bpf_preempt_disable();
+	bpf_preempt_enable();
+	return 0;
+}
+
+SEC("?tc")
+__success int preempt_balance_subprog_test(struct __sk_buff *ctx)
+{
+	preempt_balance_subprog();
+	return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__failure __msg("sleepable helper bpf_copy_from_user#")
+int preempt_sleepable_helper(void *ctx)
+{
+	u32 data;
+
+	bpf_preempt_disable();
+	bpf_copy_from_user(&data, sizeof(data), NULL);
+	bpf_preempt_enable();
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";