* [PATCH bpf-next] selftests/bpf: Fix flaky test verif_scale_strobemeta_subprogs
@ 2024-02-06 6:30 Yonghong Song
2024-02-08 0:10 ` Andrii Nakryiko
0 siblings, 1 reply; 3+ messages in thread
From: Yonghong Song @ 2024-02-06 6:30 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau
With latest llvm19, I hit the following selftest failures with
$ ./test_progs -j
libbpf: prog 'on_event': BPF program load failed: Permission denied
libbpf: prog 'on_event': -- BEGIN PROG LOAD LOG --
combined stack size of 4 calls is 544. Too large
verification time 1344153 usec
stack depth 24+440+0+32
processed 51008 insns (limit 1000000) max_states_per_insn 19 total_states 1467 peak_states 303 mark_read 146
-- END PROG LOAD LOG --
libbpf: prog 'on_event': failed to load: -13
libbpf: failed to load object 'strobemeta_subprogs.bpf.o'
scale_test:FAIL:expect_success unexpected error: -13 (errno 13)
#498 verif_scale_strobemeta_subprogs:FAIL
The verifier complains too big of the combined stack size (544 bytes) which
exceeds the maximum stack limit 512. This is a regression from llvm19 ([1]).
In the above error log, the original stack depth is 24+440+0+32.
To satisfy interpreter's need, in verifier the stack depth is adjusted to
32+448+32+32=544 which exceeds 512, hence the error. The same adjusted
stack size is also used for jit case.
But the jitted codes could use smaller stack size.
$ egrep -r stack_depth | grep round_up
arm64/net/bpf_jit_comp.c: ctx->stack_size = round_up(prog->aux->stack_depth, 16);
loongarch/net/bpf_jit.c: bpf_stack_adjust = round_up(ctx->prog->aux->stack_depth, 16);
powerpc/net/bpf_jit_comp.c: cgctx.stack_size = round_up(fp->aux->stack_depth, 16);
riscv/net/bpf_jit_comp32.c: round_up(ctx->prog->aux->stack_depth, STACK_ALIGN);
riscv/net/bpf_jit_comp64.c: bpf_stack_adjust = round_up(ctx->prog->aux->stack_depth, 16);
s390/net/bpf_jit_comp.c: u32 stack_depth = round_up(fp->aux->stack_depth, 8);
sparc/net/bpf_jit_comp_64.c: stack_needed += round_up(stack_depth, 16);
x86/net/bpf_jit_comp.c: EMIT3_off32(0x48, 0x81, 0xEC, round_up(stack_depth, 8));
x86/net/bpf_jit_comp.c: int tcc_off = -4 - round_up(stack_depth, 8);
x86/net/bpf_jit_comp.c: round_up(stack_depth, 8));
x86/net/bpf_jit_comp.c: int tcc_off = -4 - round_up(stack_depth, 8);
x86/net/bpf_jit_comp.c: EMIT3_off32(0x48, 0x81, 0xC4, round_up(stack_depth, 8));
In the above, STACK_ALIGN in riscv/net/bpf_jit_comp32.c is defined as 16.
So stack is aligned in either 8 or 16, x86/s390 having 8-byte stack alignment and
the rest having 16-byte alignment.
This patch calculates total stack depth based on 16-byte alignment if jit is requested.
For the above failing case, the new stack size will be 32+448+0+32=512 and no verification
failure. llvm19 regression will be discussed separately in llvm upstream.
[1] https://lore.kernel.org/bpf/32bde0f0-1881-46c9-931a-673be566c61d@linux.dev/
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
kernel/bpf/verifier.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ddaf09db1175..10e33d49ca21 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5812,6 +5812,17 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
strict);
}
+static int round_up_stack_depth(struct bpf_verifier_env *env, int stack_depth)
+{
+ if (env->prog->jit_requested)
+ return round_up(stack_depth, 16);
+
+ /* round up to 32-bytes, since this is granularity
+ * of interpreter stack size
+ */
+ return round_up(max_t(u32, stack_depth, 1), 32);
+}
+
/* starting from main bpf function walk all instructions of the function
* and recursively walk all callees that given function can call.
* Ignore jump and exit insns.
@@ -5855,10 +5866,8 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx)
depth);
return -EACCES;
}
- /* round up to 32-bytes, since this is granularity
- * of interpreter stack size
- */
- depth += round_up(max_t(u32, subprog[idx].stack_depth, 1), 32);
+
+ depth += round_up_stack_depth(env, subprog[idx].stack_depth);
if (depth > MAX_BPF_STACK) {
verbose(env, "combined stack size of %d calls is %d. Too large\n",
frame + 1, depth);
@@ -5952,7 +5961,7 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx)
*/
if (frame == 0)
return 0;
- depth -= round_up(max_t(u32, subprog[idx].stack_depth, 1), 32);
+ depth -= round_up_stack_depth(env, subprog[idx].stack_depth);
frame--;
i = ret_insn[frame];
idx = ret_prog[frame];
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix flaky test verif_scale_strobemeta_subprogs
2024-02-06 6:30 [PATCH bpf-next] selftests/bpf: Fix flaky test verif_scale_strobemeta_subprogs Yonghong Song
@ 2024-02-08 0:10 ` Andrii Nakryiko
2024-02-08 1:43 ` Yonghong Song
0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2024-02-08 0:10 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Martin KaFai Lau
On Mon, Feb 5, 2024 at 10:30 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
> With latest llvm19, I hit the following selftest failures with
>
> $ ./test_progs -j
> libbpf: prog 'on_event': BPF program load failed: Permission denied
> libbpf: prog 'on_event': -- BEGIN PROG LOAD LOG --
> combined stack size of 4 calls is 544. Too large
> verification time 1344153 usec
> stack depth 24+440+0+32
> processed 51008 insns (limit 1000000) max_states_per_insn 19 total_states 1467 peak_states 303 mark_read 146
> -- END PROG LOAD LOG --
> libbpf: prog 'on_event': failed to load: -13
> libbpf: failed to load object 'strobemeta_subprogs.bpf.o'
> scale_test:FAIL:expect_success unexpected error: -13 (errno 13)
> #498 verif_scale_strobemeta_subprogs:FAIL
>
> The verifier complains too big of the combined stack size (544 bytes) which
> exceeds the maximum stack limit 512. This is a regression from llvm19 ([1]).
>
> In the above error log, the original stack depth is 24+440+0+32.
> To satisfy interpreter's need, in verifier the stack depth is adjusted to
> 32+448+32+32=544 which exceeds 512, hence the error. The same adjusted
> stack size is also used for jit case.
>
> But the jitted codes could use smaller stack size.
>
> $ egrep -r stack_depth | grep round_up
> arm64/net/bpf_jit_comp.c: ctx->stack_size = round_up(prog->aux->stack_depth, 16);
> loongarch/net/bpf_jit.c: bpf_stack_adjust = round_up(ctx->prog->aux->stack_depth, 16);
> powerpc/net/bpf_jit_comp.c: cgctx.stack_size = round_up(fp->aux->stack_depth, 16);
> riscv/net/bpf_jit_comp32.c: round_up(ctx->prog->aux->stack_depth, STACK_ALIGN);
> riscv/net/bpf_jit_comp64.c: bpf_stack_adjust = round_up(ctx->prog->aux->stack_depth, 16);
> s390/net/bpf_jit_comp.c: u32 stack_depth = round_up(fp->aux->stack_depth, 8);
> sparc/net/bpf_jit_comp_64.c: stack_needed += round_up(stack_depth, 16);
> x86/net/bpf_jit_comp.c: EMIT3_off32(0x48, 0x81, 0xEC, round_up(stack_depth, 8));
> x86/net/bpf_jit_comp.c: int tcc_off = -4 - round_up(stack_depth, 8);
> x86/net/bpf_jit_comp.c: round_up(stack_depth, 8));
> x86/net/bpf_jit_comp.c: int tcc_off = -4 - round_up(stack_depth, 8);
> x86/net/bpf_jit_comp.c: EMIT3_off32(0x48, 0x81, 0xC4, round_up(stack_depth, 8));
>
> In the above, STACK_ALIGN in riscv/net/bpf_jit_comp32.c is defined as 16.
> So stack is aligned in either 8 or 16, x86/s390 having 8-byte stack alignment and
> the rest having 16-byte alignment.
>
> This patch calculates total stack depth based on 16-byte alignment if jit is requested.
> For the above failing case, the new stack size will be 32+448+0+32=512 and no verification
> failure. llvm19 regression will be discussed separately in llvm upstream.
>
> [1] https://lore.kernel.org/bpf/32bde0f0-1881-46c9-931a-673be566c61d@linux.dev/
>
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
Seems like a few selftests have to be adjusted, current BPF CI is unhappy ([0])
[0] https://github.com/kernel-patches/bpf/actions/runs/7795686155/job/21259132248
pw-bot: cr
> kernel/bpf/verifier.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index ddaf09db1175..10e33d49ca21 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5812,6 +5812,17 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
> strict);
> }
>
> +static int round_up_stack_depth(struct bpf_verifier_env *env, int stack_depth)
> +{
> + if (env->prog->jit_requested)
> + return round_up(stack_depth, 16);
> +
> + /* round up to 32-bytes, since this is granularity
> + * of interpreter stack size
> + */
> + return round_up(max_t(u32, stack_depth, 1), 32);
> +}
> +
> /* starting from main bpf function walk all instructions of the function
> * and recursively walk all callees that given function can call.
> * Ignore jump and exit insns.
> @@ -5855,10 +5866,8 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx)
> depth);
> return -EACCES;
> }
> - /* round up to 32-bytes, since this is granularity
> - * of interpreter stack size
> - */
> - depth += round_up(max_t(u32, subprog[idx].stack_depth, 1), 32);
> +
> + depth += round_up_stack_depth(env, subprog[idx].stack_depth);
> if (depth > MAX_BPF_STACK) {
> verbose(env, "combined stack size of %d calls is %d. Too large\n",
> frame + 1, depth);
> @@ -5952,7 +5961,7 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx)
> */
> if (frame == 0)
> return 0;
> - depth -= round_up(max_t(u32, subprog[idx].stack_depth, 1), 32);
> + depth -= round_up_stack_depth(env, subprog[idx].stack_depth);
> frame--;
> i = ret_insn[frame];
> idx = ret_prog[frame];
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Fix flaky test verif_scale_strobemeta_subprogs
2024-02-08 0:10 ` Andrii Nakryiko
@ 2024-02-08 1:43 ` Yonghong Song
0 siblings, 0 replies; 3+ messages in thread
From: Yonghong Song @ 2024-02-08 1:43 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Martin KaFai Lau
On 2/7/24 4:10 PM, Andrii Nakryiko wrote:
> On Mon, Feb 5, 2024 at 10:30 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>> With latest llvm19, I hit the following selftest failures with
>>
>> $ ./test_progs -j
>> libbpf: prog 'on_event': BPF program load failed: Permission denied
>> libbpf: prog 'on_event': -- BEGIN PROG LOAD LOG --
>> combined stack size of 4 calls is 544. Too large
>> verification time 1344153 usec
>> stack depth 24+440+0+32
>> processed 51008 insns (limit 1000000) max_states_per_insn 19 total_states 1467 peak_states 303 mark_read 146
>> -- END PROG LOAD LOG --
>> libbpf: prog 'on_event': failed to load: -13
>> libbpf: failed to load object 'strobemeta_subprogs.bpf.o'
>> scale_test:FAIL:expect_success unexpected error: -13 (errno 13)
>> #498 verif_scale_strobemeta_subprogs:FAIL
>>
>> The verifier complains too big of the combined stack size (544 bytes) which
>> exceeds the maximum stack limit 512. This is a regression from llvm19 ([1]).
>>
>> In the above error log, the original stack depth is 24+440+0+32.
>> To satisfy interpreter's need, in verifier the stack depth is adjusted to
>> 32+448+32+32=544 which exceeds 512, hence the error. The same adjusted
>> stack size is also used for jit case.
>>
>> But the jitted codes could use smaller stack size.
>>
>> $ egrep -r stack_depth | grep round_up
>> arm64/net/bpf_jit_comp.c: ctx->stack_size = round_up(prog->aux->stack_depth, 16);
>> loongarch/net/bpf_jit.c: bpf_stack_adjust = round_up(ctx->prog->aux->stack_depth, 16);
>> powerpc/net/bpf_jit_comp.c: cgctx.stack_size = round_up(fp->aux->stack_depth, 16);
>> riscv/net/bpf_jit_comp32.c: round_up(ctx->prog->aux->stack_depth, STACK_ALIGN);
>> riscv/net/bpf_jit_comp64.c: bpf_stack_adjust = round_up(ctx->prog->aux->stack_depth, 16);
>> s390/net/bpf_jit_comp.c: u32 stack_depth = round_up(fp->aux->stack_depth, 8);
>> sparc/net/bpf_jit_comp_64.c: stack_needed += round_up(stack_depth, 16);
>> x86/net/bpf_jit_comp.c: EMIT3_off32(0x48, 0x81, 0xEC, round_up(stack_depth, 8));
>> x86/net/bpf_jit_comp.c: int tcc_off = -4 - round_up(stack_depth, 8);
>> x86/net/bpf_jit_comp.c: round_up(stack_depth, 8));
>> x86/net/bpf_jit_comp.c: int tcc_off = -4 - round_up(stack_depth, 8);
>> x86/net/bpf_jit_comp.c: EMIT3_off32(0x48, 0x81, 0xC4, round_up(stack_depth, 8));
>>
>> In the above, STACK_ALIGN in riscv/net/bpf_jit_comp32.c is defined as 16.
>> So stack is aligned in either 8 or 16, x86/s390 having 8-byte stack alignment and
>> the rest having 16-byte alignment.
>>
>> This patch calculates total stack depth based on 16-byte alignment if jit is requested.
>> For the above failing case, the new stack size will be 32+448+0+32=512 and no verification
>> failure. llvm19 regression will be discussed separately in llvm upstream.
>>
>> [1] https://lore.kernel.org/bpf/32bde0f0-1881-46c9-931a-673be566c61d@linux.dev/
>>
>> Suggested-by: Alexei Starovoitov <ast@kernel.org>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
> Seems like a few selftests have to be adjusted, current BPF CI is unhappy ([0])
>
> [0] https://github.com/kernel-patches/bpf/actions/runs/7795686155/job/21259132248
>
> pw-bot: cr
Thanks for reminder! Will take a look soon.
>
>> kernel/bpf/verifier.c | 19 ++++++++++++++-----
>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index ddaf09db1175..10e33d49ca21 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -5812,6 +5812,17 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
>> strict);
>> }
>>
>> +static int round_up_stack_depth(struct bpf_verifier_env *env, int stack_depth)
>> +{
>> + if (env->prog->jit_requested)
>> + return round_up(stack_depth, 16);
>> +
>> + /* round up to 32-bytes, since this is granularity
>> + * of interpreter stack size
>> + */
>> + return round_up(max_t(u32, stack_depth, 1), 32);
>> +}
>> +
[...]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-08 1:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-06 6:30 [PATCH bpf-next] selftests/bpf: Fix flaky test verif_scale_strobemeta_subprogs Yonghong Song
2024-02-08 0:10 ` Andrii Nakryiko
2024-02-08 1:43 ` Yonghong Song
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox