Re: [PATCH bpf-next v9 02/10] bpf: Return false for bpf_prog_check_recur() default case

[Yonghong Song <yonghong.song@xxxxxxxxx>]

On 11/5/24 7:50 AM, Alexei Starovoitov wrote:
> On Mon, Nov 4, 2024 at 10:02 PM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
> > > I also don't understand the point of this patch 2.
> > > The patch 3 can still do:
> > >
> > > + switch (prog->type) {
> > > + case BPF_PROG_TYPE_KPROBE:
> > > + case BPF_PROG_TYPE_TRACEPOINT:
> > > + case BPF_PROG_TYPE_PERF_EVENT:
> > > + case BPF_PROG_TYPE_RAW_TRACEPOINT:
> > > +   return PRIV_STACK_ADAPTIVE;
> > > + default:
> > > +   break;
> > > + }
> > > +
> > > + if (!bpf_prog_check_recur(prog))
> > > +   return NO_PRIV_STACK;
> > >
> > > which would mean that iter, lsm, struct_ops will not be allowed
> > > to use priv stack.
> > One example is e.g. a TC prog. Since bpf_prog_check_recur(prog)
> > will return true (means supporting recursion), and private stack
> > does not really support TC prog, the logic will become more
> > complicated.
> >
> > I am totally okay with removing patch 2 and go back to my
> > previous approach to explicitly list prog types supporting
> > private stack.
> The point of reusing bpf_prog_check_recur() is that we don't
> need to duplicate the logic.
> We can still do something like:
> switch (prog->type) {
>   case BPF_PROG_TYPE_KPROBE:
>   case BPF_PROG_TYPE_TRACEPOINT:
>   case BPF_PROG_TYPE_PERF_EVENT:
>   case BPF_PROG_TYPE_RAW_TRACEPOINT:
>      return PRIV_STACK_ADAPTIVE;
>   case BPF_PROG_TYPE_TRACING:
>   case BPF_PROG_TYPE_LSM:
>   case BPF_PROG_TYPE_STRUCT_OPS:
>      if (bpf_prog_check_recur())
>        return PRIV_STACK_ADAPTIVE;
>      /* fallthrough */
>    default:
>      return NO_PRIV_STACK;
> }

Right. Listing trampoline related prog types explicitly
and using bpf_prog_check_recur() will be safe.

One thing is for BPF_PROG_TYPE_STRUCT_OPS, PRIV_STACK_ALWAYS
will be returned. I will make adjustment like

switch (prog->type) {
 case BPF_PROG_TYPE_KPROBE:
 case BPF_PROG_TYPE_TRACEPOINT:
 case BPF_PROG_TYPE_PERF_EVENT:
 case BPF_PROG_TYPE_RAW_TRACEPOINT:
    return PRIV_STACK_ADAPTIVE;
 case BPF_PROG_TYPE_TRACING:
 case BPF_PROG_TYPE_LSM:
 case BPF_PROG_TYPE_STRUCT_OPS:
    if (bpf_prog_check_recur()) {
      if (prog->type == BPF_PROG_TYPE_STRUCT_OPS)
          return PRIV_STACK_ALWAYS;
      else
          return PRIV_STACK_ADAPTIVE;
    }
    /* fallthrough */
  default:
    return NO_PRIV_STACK;
}