On Thu, Apr 10, 2025 at 12:03 AM Feng Yang <yangfeng59949@xxxxxxx> wrote: > > From: Feng Yang <yangfeng@xxxxxxxxxx> > > Many conditional checks in switch-case are redundant > with bpf_base_func_proto and should be removed. > > Signed-off-by: Feng Yang <yangfeng@xxxxxxxxxx> > Acked-by: Song Liu <song@xxxxxxxxxx> > --- > Changes in v3: > - Only modify patch description information. > - Link to v2: https://lore.kernel.org/all/20250408071151.229329-1-yangfeng59949@xxxxxxx/ > > Changes in v2: > - Only modify patch description information. > - Link to v1: https://lore.kernel.org/all/20250320032258.116156-1-yangfeng59949@xxxxxxx/ > --- > kernel/trace/bpf_trace.c | 72 ---------------------------------------- > 1 file changed, 72 deletions(-) > All this looks good, I checked that those functions indeed are allowed in bpf_base_func_proto. The only (minor) differences are capabilities, bpf_base_func_proto() correctly guards some of the helpers with CAP_BPF and/or CAP_PERFMON checks, while bpf_tracing_func_proto() doesn't seem to bother (which is either a bug or any tracing prog implies CAP_BPF and CAP_PERFMON, I'm not sure, didn't check). But I think we can take it further and remove even more stuff from bpf_tracing_func_proto and/or add more stuff into bpf_base_func_proto (perhaps as a few patches in a series, so it's easier to review and validate). Basically, except for a few custom implementations that depend on tracing program type (like get_stack and others like that), if something is OK to call from a tracing program it should be ok to call from any program type. And as such it can (should?) be added to bpf_base_func_proto, IMO. P.S. I'd name the patch/series as "bpf: streamline allowed helpers between tracing and base sets" or something like that to make the purpose clearer [...] > case BPF_FUNC_get_current_uid_gid: > return &bpf_get_current_uid_gid_proto; > case BPF_FUNC_get_current_comm: > return &bpf_get_current_comm_proto; I'm surprised these two are not part of bpf_base_func_proto, tbh... maybe let's move them there while we are cleaning all this up? pw-bot: cr > - case BPF_FUNC_trace_printk: > - return bpf_get_trace_printk_proto(); > case BPF_FUNC_get_smp_processor_id: > return &bpf_get_smp_processor_id_proto; this one should be cleaned up as well and bpf_get_smp_processor_id_proto removed. All BPF programs either disable CPU preemption or CPU migration, so bpf_base_func_proto's implementation should work just fine (but please do it as a separate patch) > - case BPF_FUNC_get_numa_node_id: > - return &bpf_get_numa_node_id_proto; > case BPF_FUNC_perf_event_read: > return &bpf_perf_event_read_proto; [...]
