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, x86: allow function arguments up to 12 for TRACING #5184

Closed
wants to merge 4 commits into from
Closed

bpf, x86: allow function arguments up to 12 for TRACING #5184

wants to merge 4 commits into from

Conversation

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

Pull request for series with
subject: bpf, x86: allow function arguments up to 12 for TRACING
version: 3
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=754887

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: aa7881f
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=754887
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=754887 expired. Closing PR.

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 67faabb
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=755641
version: 4

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

Cmd('git') failed due to: exit code(128)
  cmdline: git am --3way
  stdout: 'Applying: bpf, x86: allow function arguments up to 12 for TRACING
Applying: bpf, x86: clean garbage value in the stack of trampoline
Applying: selftests/bpf: add testcase for FENTRY/FEXIT with 6+ arguments
Using index info to reconstruct a base tree...
M	tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
Falling back to patching base and 3-way merge...
Auto-merging tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
CONFLICT (content): Merge conflict in tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
Patch failed at 0003 selftests/bpf: add testcase for FENTRY/FEXIT with 6+ arguments
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 tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index aaf6ef1201c7,66615fdbe3df..000000000000
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@@ -191,6 -191,21 +191,24 @@@ noinline int bpf_testmod_fentry_test3(c
  	return a + b + c;
  }
  
++<<<<<<< HEAD
++=======
+ noinline int bpf_testmod_fentry_test7(u64 a, void *b, short c, int d,
+ 				      void *e, u64 f, u64 g)
+ {
+ 	return a + (long)b + c + d + (long)e + f + g;
+ }
+ 
+ noinline int bpf_testmod_fentry_test12(u64 a, void *b, short c, int d,
+ 				       void *e, u64 f, u64 g, u64 h,
+ 				       u64 i, u64 j, u64 k, u64 l)
+ {
+ 	return a + (long)b + c + d + (long)e + f + g + h + i + j + k + l;
+ }
+ 
+ __diag_pop();
+ 
++>>>>>>> selftests/bpf: add testcase for FENTRY/FEXIT with 6+ arguments
  int bpf_testmod_fentry_ok;
  
  noinline ssize_t

@kernel-patches-daemon-bpf
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=755641 expired. Closing PR.

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 25085b4
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=756560
version: 5

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: c03531e
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=756560
version: 5

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 87e098e
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=764727
version: 9

@kernel-patches-daemon-bpf
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=764727 expired. Closing PR.

Kernel Patches Daemon and others added 4 commits July 12, 2023 14:52
adding ci files
cf3e40a
@menglongdong
bpf, x86: save/restore regs with BPF_DW size
e830e7a 
As we already reserve 8 byte in the stack for each reg, it is ok to
store/restore the regs in BPF_DW size. This will make the code in
save_regs()/restore_regs() simpler.

Signed-off-by: Menglong Dong <imagedong@tencent.com>
Acked-by: Yonghong Song <yhs@fb.com>
@menglongdong
bpf, x86: allow function arguments up to 12 for TRACING
84cc398 
For now, the BPF program of type BPF_PROG_TYPE_TRACING can only be used
on the kernel functions whose arguments count less than or equal to 6, if
not considering '> 8 bytes' struct argument. This is not friendly at all,
as too many functions have arguments count more than 6.

According to the current kernel version, below is a statistics of the
function arguments count:

argument count | function count
7              | 704
8              | 270
9              | 84
10             | 47
11             | 47
12             | 27
13             | 22
14             | 5
15             | 0
16             | 1

Therefore, let's enhance it by increasing the function arguments count
allowed in arch_prepare_bpf_trampoline(), for now, only x86_64.

For the case that we don't need to call origin function, which means
without BPF_TRAMP_F_CALL_ORIG, we need only copy the function arguments
that stored in the frame of the caller to current frame. The 7th and later
arguments are stored in "$rbp + 0x18", and they will be copied to the
stack area following where register values are saved.

For the case with BPF_TRAMP_F_CALL_ORIG, we need prepare the arguments
in stack before call origin function, which means we need alloc extra
"8 * (arg_count - 6)" memory in the top of the stack. Note, there should
not be any data be pushed to the stack before calling the origin function.
So 'rbx' value will be stored on a stack position higher than where stack
arguments are stored for BPF_TRAMP_F_CALL_ORIG.

According to the research of Yonghong, struct members should be all in
register or all on the stack. Meanwhile, the compiler will pass the
argument on regs if the remaining regs can hold the argument. Therefore,
we need save the arguments in order. Otherwise, disorder of the args can
happen. For example:

  struct foo_struct {
      long a;
      int b;
  };
  int foo(char, char, char, char, char, struct foo_struct,
          char);

the arg1-5,arg7 will be passed by regs, and arg6 will by stack. Therefore,
we should save/restore the arguments in the same order with the
declaration of foo(). And the args used as ctx in stack will be like this:

  reg_arg6   -- copy from regs
  stack_arg2 -- copy from stack
  stack_arg1
  reg_arg5   -- copy from regs
  reg_arg4
  reg_arg3
  reg_arg2
  reg_arg1

We use EMIT3_off32() or EMIT4() for "lea" and "sub". The range of the
imm in "lea" and "sub" is [-128, 127] if EMIT4() is used. Therefore,
we use EMIT3_off32() instead if the imm out of the range.

It works well for the FENTRY/FEXIT/MODIFY_RETURN.

Signed-off-by: Menglong Dong <imagedong@tencent.com>
Acked-by: Yonghong Song <yhs@fb.com>
@menglongdong
selftests/bpf: add testcase for TRACING with 6+ arguments
824a02b 
Add fentry_many_args.c and fexit_many_args.c to test the fentry/fexit
with 7/11 arguments. As this feature is not supported by arm64 yet, we
disable these testcases for arm64 in DENYLIST.aarch64. We can combine
them with fentry_test.c/fexit_test.c when arm64 is supported too.

Correspondingly, add bpf_testmod_fentry_test7() and
bpf_testmod_fentry_test11() to bpf_testmod.c

Meanwhile, add bpf_modify_return_test2() to test_run.c to test the
MODIFY_RETURN with 7 arguments.

Add bpf_testmod_test_struct_arg_7/bpf_testmod_test_struct_arg_7 in
bpf_testmod.c to test the struct in the arguments.

And the testcases passed on x86_64:

./test_progs -t fexit
Summary: 5/14 PASSED, 0 SKIPPED, 0 FAILED

./test_progs -t fentry
Summary: 3/2 PASSED, 0 SKIPPED, 0 FAILED

./test_progs -t modify_return
Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED

./test_progs -t tracing_struct
Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Menglong Dong <imagedong@tencent.com>
Acked-by: Yonghong Song <yhs@fb.com>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 0a5550b
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=765105
version: 10

@kernel-patches-daemon-bpf
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=765105 irrelevant now. Closing PR.

@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot deleted the series/752015=>bpf-next branch July 13, 2023 23:13
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
accepted bpf-next V10
Projects
None yet
Milestone
No milestone
1 participant
@menglongdong