Re: [PATCH bpf-next v2 1/5] bpf: make MAX_BPF_FUNC_ARGS 14

[Menglong Dong <menglong8.dong@xxxxxxxxx>]

On Sat, Jun 3, 2023 at 2:17 AM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Fri, Jun 2, 2023 at 12:01 AM <menglong8.dong@xxxxxxxxx> wrote:
> >
> > From: Menglong Dong <imagedong@xxxxxxxxxxx>
> >
> > 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