Skip to content
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

bpf: Fix tailcall hierarchy #7008

Closed
wants to merge 6 commits into from
Closed

bpf: Fix tailcall hierarchy #7008

wants to merge 6 commits into from

Conversation

kernel-patches-daemon-bpf[bot]
Copy link

Pull request for series with
subject: bpf: Fix tailcall hierarchy
version: 4
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=851924

@kernel-patches-daemon-bpf
Copy link
Author

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

@kernel-patches-daemon-bpf
Copy link
Author

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

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 0d03a4d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=851924
version: 4

@Asphaltt
bpf, verifier: Correct tail_call_reachable when no tailcall in subprog
c3fb0bd 
When there is tailcall in main prog but there is no tailcall in its
subprogs, prog->aux->tail_call_reachable is incorrect for this case.

In order to correct it, it has to check subprog[0].has_tail_call at the
time when to check subprog[0].tail_call_reachable in
check_max_stack_depth_subprog().

It's for fixing a tailcall issue whose patch relies on
prog->aux->tail_call_reachable.

Signed-off-by: Leon Hwang <hffilwlqm@gmail.com>
@Asphaltt
bpf: Introduce bpf_jit_supports_tail_call_cnt_ptr()
e0c78dc 
In order to store tail_call_cnt on the stack of bpf prog's caller,
introduce bpf_tail_call_run_ctx as a new ctx for bpf prog, which
wraps the original ctx and tail_call_cnt pointer.

To avoid breaking run time, introduce use_tail_call_run_ctx in
prog->aux in order to determine whether to use bpf_tail_call_run_ctx
before calling bpf prog. This flag will be set when
prog->aux->tail_call_reachable and the prog is jited and the arch
supports bpf_jit_supports_tail_call_cnt_ptr() at load time. Thereafter,
the prog's prologue has to cache tail_call_cnt_ptr, and retore the
original ctx meanwhile.

Signed-off-by: Leon Hwang <hffilwlqm@gmail.com>
@kernel-patches-daemon-bpf
Copy link
Author

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

@Asphaltt
bpf, x64: Fix tailcall hierarchy
ef17f5c 
This patch fixes a tailcall issue caused by abusing the tailcall in
bpf2bpf feature.

As we know, tail_call_cnt propagates by rax from caller to callee when
to call subprog in tailcall context. But, like the following example,
MAX_TAIL_CALL_CNT won't work because of missing tail_call_cnt
back-propagation from callee to caller.

\#include <linux/bpf.h>
\#include <bpf/bpf_helpers.h>
\#include "bpf_legacy.h"

struct {
	__uint(type, BPF_MAP_TYPE_PROG_ARRAY);
	__uint(max_entries, 1);
	__uint(key_size, sizeof(__u32));
	__uint(value_size, sizeof(__u32));
} jmp_table SEC(".maps");

int count = 0;

static __noinline
int subprog_tail1(struct __sk_buff *skb)
{
	bpf_tail_call_static(skb, &jmp_table, 0);
	return 0;
}

static __noinline
int subprog_tail2(struct __sk_buff *skb)
{
	bpf_tail_call_static(skb, &jmp_table, 0);
	return 0;
}

SEC("tc")
int entry(struct __sk_buff *skb)
{
	volatile int ret = 1;

	count++;
	subprog_tail1(skb);
	subprog_tail2(skb);

	return ret;
}

char __license[] SEC("license") = "GPL";

At run time, the tail_call_cnt in entry() will be propagated to
subprog_tail1() and subprog_tail2(). But, when the tail_call_cnt in
subprog_tail1() updates when bpf_tail_call_static(), the tail_call_cnt
in entry() won't be updated at the same time. As a result, in entry(),
when tail_call_cnt in entry() is less than MAX_TAIL_CALL_CNT and
subprog_tail1() returns because of MAX_TAIL_CALL_CNT limit,
bpf_tail_call_static() in suprog_tail2() is able to run because the
tail_call_cnt in subprog_tail2() propagated from entry() is less than
MAX_TAIL_CALL_CNT.

So, how many tailcalls are there for this case if no error happens?

From top-down view, does it look like hierarchy layer and layer?

With view, there will be 2+4+8+...+2^33 = 2^34 - 2 = 17,179,869,182
tailcalls for this case.

How about there are N subprog_tail() in entry()? There will be almost
N^34 tailcalls.

Then, in this patch, it resolves this case on x86_64.

In stead of propagating tail_call_cnt from caller to callee, it
propagate its pointer, tail_call_cnt_ptr, tcc_ptr for short.

However, where does it store tail_call_cnt?

It stores tail_call_cnt on the stack of bpf prog's caller by the way in
previous patch "bpf: Introduce bpf_jit_supports_tail_call_cnt_ptr()".
Then, in bpf prog's prologue, it loads tcc_ptr from bpf_tail_call_run_ctx,
and restores the original ctx from bpf_tail_call_run_ctx meanwhile.

Then, when a tailcall runs, it compares tail_call_cnt accessed by
tcc_ptr with MAX_TAIL_CALL_CNT and then increments tail_call_cnt at
tcc_ptr.

Furthermore, when trampoline is the caller of bpf prog, it is required
to prepare tail_call_cnt and tail call run ctx on the stack of the
trampoline.

Finally, enable bpf_jit_supports_tail_call_cnt_ptr() to use
bpf_tail_call_run_ctx in __bpf_prog_run().

Fixes: ebf7d1f ("bpf, x64: rework pro/epilogue and tailcall handling in JIT")
Fixes: e411901 ("bpf: allow for tailcalls in BPF subprograms for x64 JIT")
Signed-off-by: Leon Hwang <hffilwlqm@gmail.com>
@Asphaltt
bpf, arm64: Fix tailcall hierarchy
06589cc 
Like the way of "bpf, x64: Fix tailcall hierarchy", this patch fixes
this issue on arm64.

At prologue, it loads tail_call_cnt_ptr from bpf_tail_call_run_ctx to
TCCNT_PTR register, and restores the original ctx from
bpf_tail_call_run_ctx to X0 register meanwhile.

Then, when a tailcall runs:
1. load tail_call_cnt from tail_call_cnt_ptr
2. compare tail_call_cnt with MAX_TAIL_CALL_CNT
3. increment tail_call_cnt
4. store tail_call_cnt by tail_call_cnt_ptr

Furthermore, when trampoline is the caller of bpf prog, it is required
to prepare tail_call_cnt and tail call run ctx on the stack of the
trampoline.

Finally, enable bpf_jit_supports_tail_call_cnt_ptr() to use
bpf_tail_call_run_ctx in __bpf_prog_run().

Fixes: d4609a5 ("bpf, arm64: Keep tail call count across bpf2bpf calls")
Signed-off-by: Leon Hwang <hffilwlqm@gmail.com>
@Asphaltt
selftests/bpf: Add testcases for tailcall hierarchy fixing
2023ca8 
Add some test cases to confirm the tailcall hierarchy issue has been fixed.

On x64, the selftests result is:

cd tools/testing/selftests/bpf && ./test_progs -t tailcalls
319/18  tailcalls/tailcall_bpf2bpf_hierarchy_1:OK
319/19  tailcalls/tailcall_bpf2bpf_hierarchy_fentry:OK
319/20  tailcalls/tailcall_bpf2bpf_hierarchy_fexit:OK
319/21  tailcalls/tailcall_bpf2bpf_hierarchy_fentry_fexit:OK
319/22  tailcalls/tailcall_bpf2bpf_hierarchy_fentry_entry:OK
319/23  tailcalls/tailcall_bpf2bpf_hierarchy_2:OK
319/24  tailcalls/tailcall_bpf2bpf_hierarchy_3:OK
319     tailcalls:OK
Summary: 1/24 PASSED, 0 SKIPPED, 0 FAILED

On arm64, the selftests result is:

cd tools/testing/selftests/bpf && ./test_progs -t tailcalls
323/18  tailcalls/tailcall_bpf2bpf_hierarchy_1:OK
323/19  tailcalls/tailcall_bpf2bpf_hierarchy_fentry:OK
323/20  tailcalls/tailcall_bpf2bpf_hierarchy_fexit:OK
323/21  tailcalls/tailcall_bpf2bpf_hierarchy_fentry_fexit:OK
323/22  tailcalls/tailcall_bpf2bpf_hierarchy_fentry_entry:OK
323/23  tailcalls/tailcall_bpf2bpf_hierarchy_2:OK
323/24  tailcalls/tailcall_bpf2bpf_hierarchy_3:OK
323     tailcalls:OK
Summary: 1/24 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Leon Hwang <hffilwlqm@gmail.com>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 80c5a07
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=851924
version: 4

Pull request is NOT updated. Failed to apply https://patchwork.kernel.org/project/netdevbpf/list/?series=851924
error message:

Cmd('git') failed due to: exit code(128)
  cmdline: git am --3way
  stdout: 'Applying: bpf, verifier: Correct tail_call_reachable when no tailcall in subprog
Applying: bpf: Introduce bpf_jit_supports_tail_call_cnt_ptr()
Using index info to reconstruct a base tree...
M	include/linux/filter.h
M	kernel/bpf/core.c
Falling back to patching base and 3-way merge...
Auto-merging kernel/bpf/core.c
CONFLICT (content): Merge conflict in kernel/bpf/core.c
Auto-merging include/linux/filter.h
Patch failed at 0002 bpf: Introduce bpf_jit_supports_tail_call_cnt_ptr()
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".'
  stderr: 'error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch'

conflict:

diff --cc kernel/bpf/core.c
index aa59af9f9bd9,3fad4d973b82..000000000000
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@@ -2941,13 -2952,10 +2952,20 @@@ bool __weak bpf_jit_needs_zext(void
  	return false;
  }
  
++<<<<<<< HEAD
 +/* Return true if the JIT inlines the call to the helper corresponding to
 + * the imm.
 + *
 + * The verifier will not patch the insn->imm for the call to the helper if
 + * this returns true.
 + */
 +bool __weak bpf_jit_inlines_helper_call(s32 imm)
++=======
+ /* Return TRUE if the JIT backend supports tail call count pointer in tailcall
+  * context.
+  */
+ bool __weak bpf_jit_supports_tail_call_cnt_ptr(void)
++>>>>>>> bpf: Introduce bpf_jit_supports_tail_call_cnt_ptr()
  {
  	return false;
  }

@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot force-pushed the bpf-next_base branch 5 times, most recently from 78e63a1 to 179d075 Compare May 13, 2024 00:33
@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot force-pushed the bpf-next_base branch 6 times, most recently from c7673c1 to 0b7406d Compare May 18, 2024 17:53
@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot force-pushed the bpf-next_base branch 4 times, most recently from fb42b3d to 169fa82 Compare May 22, 2024 22:08
@kernel-patches-daemon-bpf
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=851924 irrelevant now for [Munch({'archived': False, 'project': 399, 'delegate': 121173})] search patterns

@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot deleted the series/851924=>bpf-next branch May 25, 2024 23:00
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-pass V4
Projects
None yet
Milestone
No milestone
1 participant
@Asphaltt