From: Menglong Dong <[email protected]>
For now, the BPF program of type BPF_PROG_TYPE_TRACING can only be used
on the kernel functions whose arguments count less than 6. This is not
friendly at all, as too many functions have arguments count more than 6.
Therefore, let's enhance it by increasing the function arguments count
allowed in arch_prepare_bpf_trampoline(), for now, only x86_64.
In the 1th patch, we make MAX_BPF_FUNC_ARGS 14, according to our
statistics.
In the 2th patch, we make arch_prepare_bpf_trampoline() support to copy
function arguments in stack for x86 arch. Therefore, the maximum
arguments can be up to MAX_BPF_FUNC_ARGS for FENTRY and FEXIT.
And the 3-5th patches are for the testcases of the 2th patch.
Changes since v1:
- change the maximun function arguments to 14 from 12
- add testcases (Jiri Olsa)
- instead EMIT4 with EMIT3_off32 for "lea" to prevent overflow
Menglong Dong (5):
bpf: make MAX_BPF_FUNC_ARGS 14
bpf, x86: allow function arguments up to 14 for TRACING
libbpf: make BPF_PROG support 15 function arguments
selftests/bpf: rename bpf_fentry_test{7,8,9} to bpf_fentry_test_ptr*
selftests/bpf: add testcase for FENTRY/FEXIT with 6+ arguments
arch/x86/net/bpf_jit_comp.c | 96 ++++++++++++++++---
include/linux/bpf.h | 9 +-
net/bpf/test_run.c | 40 ++++++--
tools/lib/bpf/bpf_helpers.h | 9 +-
tools/lib/bpf/bpf_tracing.h | 10 +-
.../selftests/bpf/prog_tests/bpf_cookie.c | 24 ++---
.../bpf/prog_tests/kprobe_multi_test.c | 16 ++--
.../testing/selftests/bpf/progs/fentry_test.c | 50 ++++++++--
.../testing/selftests/bpf/progs/fexit_test.c | 51 ++++++++--
.../selftests/bpf/progs/get_func_ip_test.c | 2 +-
.../selftests/bpf/progs/kprobe_multi.c | 12 +--
.../bpf/progs/verifier_btf_ctx_access.c | 2 +-
.../selftests/bpf/verifier/atomic_fetch_add.c | 4 +-
13 files changed, 249 insertions(+), 76 deletions(-)
--
2.40.1
From: Menglong Dong <[email protected]>
According to the current kernel version, below is a statistics of the
function arguments count:
argument count | FUNC_PROTO count
7 | 367
8 | 196
9 | 71
10 | 43
11 | 22
12 | 10
13 | 15
14 | 4
15 | 0
16 | 1
It's hard to statisics the function count, so I use FUNC_PROTO in the btf
of vmlinux instead. The function with 16 arguments is ZSTD_buildCTable(),
which I think can be ignored.
Therefore, let's make the maximum of function arguments count 14. It used
to be 12, but it seems that there is no harm to make it big enough.
Reviewed-by: Jiang Biao <[email protected]>
Signed-off-by: Menglong Dong <[email protected]>
---
include/linux/bpf.h | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index f58895830ada..8b997779faf7 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -961,10 +961,10 @@ enum bpf_cgroup_storage_type {
#define MAX_BPF_CGROUP_STORAGE_TYPE __BPF_CGROUP_STORAGE_MAX
-/* The longest tracepoint has 12 args.
- * See include/trace/bpf_probe.h
+/* The maximun number of the kernel function arguments.
+ * Let's make it 14, for now.
*/
-#define MAX_BPF_FUNC_ARGS 12
+#define MAX_BPF_FUNC_ARGS 14
/* The maximum number of arguments passed through registers
* a single function may have.
@@ -2273,7 +2273,8 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
static inline bool bpf_tracing_ctx_access(int off, int size,
enum bpf_access_type type)
{
- if (off < 0 || off >= sizeof(__u64) * MAX_BPF_FUNC_ARGS)
+ /* "+1" here is for FEXIT return value. */
+ if (off < 0 || off >= sizeof(__u64) * (MAX_BPF_FUNC_ARGS + 1))
return false;
if (type != BPF_READ)
return false;
--
2.40.1
On Fri, Jun 2, 2023 at 12:01 AM <[email protected]> wrote:
>
> From: Menglong Dong <[email protected]>
>
> According to the current kernel version, below is a statistics of the
> function arguments count:
>
> argument count | FUNC_PROTO count
> 7 | 367
> 8 | 196
> 9 | 71
> 10 | 43
> 11 | 22
> 12 | 10
> 13 | 15
> 14 | 4
> 15 | 0
> 16 | 1
>
> It's hard to statisics the function count, so I use FUNC_PROTO in the btf
> of vmlinux instead. The function with 16 arguments is ZSTD_buildCTable(),
> which I think can be ignored.
>
> Therefore, let's make the maximum of function arguments count 14. It used
> to be 12, but it seems that there is no harm to make it big enough.
I think we're just fine at 12.
People need to fix their code. ZSTD_buildCTable should be first in line.
Passing arguments on the stack is not efficient from performance pov.
On Fri, Jun 02, 2023 at 02:59:54PM +0800, [email protected] wrote:
> From: Menglong Dong <[email protected]>
>
> According to the current kernel version, below is a statistics of the
> function arguments count:
>
> argument count | FUNC_PROTO count
> 7 | 367
> 8 | 196
> 9 | 71
> 10 | 43
> 11 | 22
> 12 | 10
> 13 | 15
> 14 | 4
> 15 | 0
> 16 | 1
>
> It's hard to statisics the function count, so I use FUNC_PROTO in the btf
> of vmlinux instead. The function with 16 arguments is ZSTD_buildCTable(),
> which I think can be ignored.
>
> Therefore, let's make the maximum of function arguments count 14. It used
> to be 12, but it seems that there is no harm to make it big enough.
>
> Reviewed-by: Jiang Biao <[email protected]>
> Signed-off-by: Menglong Dong <[email protected]>
> ---
> include/linux/bpf.h | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index f58895830ada..8b997779faf7 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -961,10 +961,10 @@ enum bpf_cgroup_storage_type {
>
> #define MAX_BPF_CGROUP_STORAGE_TYPE __BPF_CGROUP_STORAGE_MAX
>
> -/* The longest tracepoint has 12 args.
> - * See include/trace/bpf_probe.h
> +/* The maximun number of the kernel function arguments.
Hi Menglong Dong,
as it looks like there will be a v3 anyway, please
consider correcting the spelling of maximum.
...
On Sat, Jun 3, 2023 at 2:17 AM Alexei Starovoitov
<[email protected]> wrote:
>
> On Fri, Jun 2, 2023 at 12:01 AM <[email protected]> wrote:
> >
> > From: Menglong Dong <[email protected]>
> >
> > According to the current kernel version, below is a statistics of the
> > function arguments count:
> >
> > argument count | FUNC_PROTO count
> > 7 | 367
> > 8 | 196
> > 9 | 71
> > 10 | 43
> > 11 | 22
> > 12 | 10
> > 13 | 15
> > 14 | 4
> > 15 | 0
> > 16 | 1
> >
> > It's hard to statisics the function count, so I use FUNC_PROTO in the btf
> > of vmlinux instead. The function with 16 arguments is ZSTD_buildCTable(),
> > which I think can be ignored.
> >
> > Therefore, let's make the maximum of function arguments count 14. It used
> > to be 12, but it seems that there is no harm to make it big enough.
>
> I think we're just fine at 12.
> People need to fix their code. ZSTD_buildCTable should be first in line.
> Passing arguments on the stack is not efficient from performance pov.
But we still need to keep this part:
@@ -2273,7 +2273,8 @@ bool btf_ctx_access(int off, int size, enum
bpf_access_type type,
static inline bool bpf_tracing_ctx_access(int off, int size,
enum bpf_access_type type)
{
- if (off < 0 || off >= sizeof(__u64) * MAX_BPF_FUNC_ARGS)
+ /* "+1" here is for FEXIT return value. */
+ if (off < 0 || off >= sizeof(__u64) * (MAX_BPF_FUNC_ARGS + 1))
return false;
if (type != BPF_READ)
return false;
Isn't it? Otherwise, it will make that the maximum arguments
is 12 for FENTRY, but 11 for FEXIT, as FEXIT needs to store
the return value in ctx.
How about that we change bpf_tracing_ctx_access() into:
static inline bool bpf_tracing_ctx_access(int off, int size,
enum bpf_access_type type,
int max_args)
And the caller can pass MAX_BPF_FUNC_ARGS to
it on no-FEXIT, and 'MAX_BPF_FUNC_ARGS + 1'
on FEXIT.
What do you think?
Thanks!
Menglong Dong
On Sat, Jun 3, 2023 at 10:13 PM Simon Horman <[email protected]> wrote:
>
> On Fri, Jun 02, 2023 at 02:59:54PM +0800, [email protected] wrote:
> > From: Menglong Dong <[email protected]>
> >
> > According to the current kernel version, below is a statistics of the
> > function arguments count:
> >
> > argument count | FUNC_PROTO count
> > 7 | 367
> > 8 | 196
> > 9 | 71
> > 10 | 43
> > 11 | 22
> > 12 | 10
> > 13 | 15
> > 14 | 4
> > 15 | 0
> > 16 | 1
> >
> > It's hard to statisics the function count, so I use FUNC_PROTO in the btf
> > of vmlinux instead. The function with 16 arguments is ZSTD_buildCTable(),
> > which I think can be ignored.
> >
> > Therefore, let's make the maximum of function arguments count 14. It used
> > to be 12, but it seems that there is no harm to make it big enough.
> >
> > Reviewed-by: Jiang Biao <[email protected]>
> > Signed-off-by: Menglong Dong <[email protected]>
> > ---
> > include/linux/bpf.h | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index f58895830ada..8b997779faf7 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -961,10 +961,10 @@ enum bpf_cgroup_storage_type {
> >
> > #define MAX_BPF_CGROUP_STORAGE_TYPE __BPF_CGROUP_STORAGE_MAX
> >
> > -/* The longest tracepoint has 12 args.
> > - * See include/trace/bpf_probe.h
> > +/* The maximun number of the kernel function arguments.
>
> Hi Menglong Dong,
>
> as it looks like there will be a v3 anyway, please
> consider correcting the spelling of maximum.
>
According to the advice of Alexei Starovoitov, it seems
we don't need to modify it here anymore. Anyway,
Thank you for reminding me of this spelling mistake :)
> ...