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

[Yonghong Song <yonghong.song@linux.dev>]

On 11/5/24 7:50 AM, Alexei Starovoitov wrote:
> On Mon, Nov 4, 2024 at 10:02 PM Yonghong Song <yonghong.song@linux.dev> 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;
}