bpf: Remove duplicate judgments

Commit Message

Feng Yang March 20, 2025, 3:22 a.m. UTC

From: Feng Yang <[e-mail]>

Most of the judgments also exist in bpf_base_func_deto, remove them.

Signed-off-by: Feng Yang <[e-mail]>
---
 kernel/trace/bpf_trace.c | 72 ----------------------------------------
 1 file changed, 72 deletions(-)

Comments

Song Liu March 20, 2025, 4:45 p.m. UTC | #1

On Wed, Mar 19, 2025 at 8:23 PM Feng Yang <[e-mail]> wrote:
>
> From: Feng Yang <[e-mail]>
>
> Most of the judgments also exist in bpf_base_func_deto, remove them.

"Most" of them also exist is not enough. Please make sure that this does
not introduce any behavior change. For example, we should not remove
return of bpf_perf_event_read_value_proto.

For future patches, please read Documentation/bpf/bpf_devel_QA.rst
and follow rules for email subject, etc. For example, this patch should
have a subject like "[PATCH bpf-next] xxx".

Thanks,
Song


>
> Signed-off-by: Feng Yang <[e-mail]>
> ---
>  kernel/trace/bpf_trace.c | 72 ----------------------------------------
>  1 file changed, 72 deletions(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 6b07fa7081d9..c89b25344422 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1443,56 +1443,14 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>         const struct bpf_func_proto *func_proto;
>
>         switch (func_id) {
> -       case BPF_FUNC_map_lookup_elem:
> -               return &bpf_map_lookup_elem_proto;
> -       case BPF_FUNC_map_update_elem:
> -               return &bpf_map_update_elem_proto;
> -       case BPF_FUNC_map_delete_elem:
> -               return &bpf_map_delete_elem_proto;
> -       case BPF_FUNC_map_push_elem:
> -               return &bpf_map_push_elem_proto;
> -       case BPF_FUNC_map_pop_elem:
> -               return &bpf_map_pop_elem_proto;
> -       case BPF_FUNC_map_peek_elem:
> -               return &bpf_map_peek_elem_proto;
> -       case BPF_FUNC_map_lookup_percpu_elem:
> -               return &bpf_map_lookup_percpu_elem_proto;
> -       case BPF_FUNC_ktime_get_ns:
> -               return &bpf_ktime_get_ns_proto;
> -       case BPF_FUNC_ktime_get_boot_ns:
> -               return &bpf_ktime_get_boot_ns_proto;
> -       case BPF_FUNC_tail_call:
> -               return &bpf_tail_call_proto;
> -       case BPF_FUNC_get_current_task:
> -               return &bpf_get_current_task_proto;
> -       case BPF_FUNC_get_current_task_btf:
> -               return &bpf_get_current_task_btf_proto;
> -       case BPF_FUNC_task_pt_regs:
> -               return &bpf_task_pt_regs_proto;
>         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;
> -       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;
> -       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;
> -       case BPF_FUNC_get_prandom_u32:
> -               return &bpf_get_prandom_u32_proto;
> -       case BPF_FUNC_probe_read_user:
> -               return &bpf_probe_read_user_proto;
> -       case BPF_FUNC_probe_read_kernel:
> -               return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ?
> -                      NULL : &bpf_probe_read_kernel_proto;
> -       case BPF_FUNC_probe_read_user_str:
> -               return &bpf_probe_read_user_str_proto;
> -       case BPF_FUNC_probe_read_kernel_str:
> -               return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ?
> -                      NULL : &bpf_probe_read_kernel_str_proto;
>  #ifdef CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>         case BPF_FUNC_probe_read:
>                 return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ?
> @@ -1502,10 +1460,6 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>                        NULL : &bpf_probe_read_compat_str_proto;
>  #endif
>  #ifdef CONFIG_CGROUPS
> -       case BPF_FUNC_cgrp_storage_get:
> -               return &bpf_cgrp_storage_get_proto;
> -       case BPF_FUNC_cgrp_storage_delete:
> -               return &bpf_cgrp_storage_delete_proto;
>         case BPF_FUNC_current_task_under_cgroup:
>                 return &bpf_current_task_under_cgroup_proto;
>  #endif
> @@ -1513,20 +1467,6 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>                 return &bpf_send_signal_proto;
>         case BPF_FUNC_send_signal_thread:
>                 return &bpf_send_signal_thread_proto;
> -       case BPF_FUNC_perf_event_read_value:
> -               return &bpf_perf_event_read_value_proto;
> -       case BPF_FUNC_ringbuf_output:
> -               return &bpf_ringbuf_output_proto;
> -       case BPF_FUNC_ringbuf_reserve:
> -               return &bpf_ringbuf_reserve_proto;
> -       case BPF_FUNC_ringbuf_submit:
> -               return &bpf_ringbuf_submit_proto;
> -       case BPF_FUNC_ringbuf_discard:
> -               return &bpf_ringbuf_discard_proto;
> -       case BPF_FUNC_ringbuf_query:
> -               return &bpf_ringbuf_query_proto;
> -       case BPF_FUNC_jiffies64:
> -               return &bpf_jiffies64_proto;
>         case BPF_FUNC_get_task_stack:
>                 return prog->sleepable ? &bpf_get_task_stack_sleepable_proto
>                                        : &bpf_get_task_stack_proto;
> @@ -1534,12 +1474,6 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>                 return &bpf_copy_from_user_proto;
>         case BPF_FUNC_copy_from_user_task:
>                 return &bpf_copy_from_user_task_proto;
> -       case BPF_FUNC_snprintf_btf:
> -               return &bpf_snprintf_btf_proto;
> -       case BPF_FUNC_per_cpu_ptr:
> -               return &bpf_per_cpu_ptr_proto;
> -       case BPF_FUNC_this_cpu_ptr:
> -               return &bpf_this_cpu_ptr_proto;
>         case BPF_FUNC_task_storage_get:
>                 if (bpf_prog_check_recur(prog))
>                         return &bpf_task_storage_get_recur_proto;
> @@ -1548,18 +1482,12 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>                 if (bpf_prog_check_recur(prog))
>                         return &bpf_task_storage_delete_recur_proto;
>                 return &bpf_task_storage_delete_proto;
> -       case BPF_FUNC_for_each_map_elem:
> -               return &bpf_for_each_map_elem_proto;
> -       case BPF_FUNC_snprintf:
> -               return &bpf_snprintf_proto;
>         case BPF_FUNC_get_func_ip:
>                 return &bpf_get_func_ip_proto_tracing;
>         case BPF_FUNC_get_branch_snapshot:
>                 return &bpf_get_branch_snapshot_proto;
>         case BPF_FUNC_find_vma:
>                 return &bpf_find_vma_proto;
> -       case BPF_FUNC_trace_vprintk:
> -               return bpf_get_trace_vprintk_proto();
>         default:
>                 break;
>         }
> --
> 2.43.0
>

Feng Yang March 21, 2025, 4:35 a.m. UTC | #2

On Thu, 20 Mar 2025 09:45:17 -0700 Song Liu <[e-mail]> wrote:
>
> On Wed, Mar 19, 2025 at 8:23 PM Feng Yang <[e-mail]> wrote:
> >
> > From: Feng Yang <[e-mail]>
> >
> > Most of the judgments also exist in bpf_base_func_deto, remove them.
>
> "Most" of them also exist is not enough. Please make sure that this does
> not introduce any behavior change. For example, we should not remove
> return of bpf_perf_event_read_value_proto.

in trace/bpf_trace.c:
const struct bpf_func_proto *bpf_get_perf_event_read_value_proto(void)
{
	return &bpf_perf_event_read_value_proto;
}
in bpf/core.c:
const struct bpf_func_proto * __weak bpf_get_perf_event_read_value_proto(void)
{
	return NULL;
}

And weak symbols will be covered
nm vmlinux | grep bpf_get_perf_event_read_value_proto
ffffffff814b90e0 T bpf_get_perf_event_read_value_proto
ffffffff814b90d0 T __pfx_bpf_get_perf_event_read_value_proto

So the return of bpf_perf_event_read_value_proto can be done through the bpf_base_func_proto function.
bpf_base_func_proto
	......
	case BPF_FUNC_perf_event_read_value:
		return bpf_get_perf_event_read_value_proto();

I think this can be removed.

> For future patches, please read Documentation/bpf/bpf_devel_QA.rst
> and follow rules for email subject, etc. For example, this patch should
> have a subject like "[PATCH bpf-next] xxx".

Thank you very much for your suggestion. I will pay attention to it next time.

> Thanks,
> Song

Song Liu March 21, 2025, 5:23 a.m. UTC | #3

On Thu, Mar 20, 2025 at 9:36 PM Feng Yang <[e-mail]> wrote:
>
[...]
>
> So the return of bpf_perf_event_read_value_proto can be done through the bpf_base_func_proto function.
> bpf_base_func_proto
>         ......
>         case BPF_FUNC_perf_event_read_value:
>                 return bpf_get_perf_event_read_value_proto();
>
> I think this can be removed.

I see. I misunderstood this part because I was reading code before
ae0a457f5d33c336f3c4259a258f8b537531a04b. Now it looks good.

Acked-by: Song Liu <[e-mail]>

> > For future patches, please read Documentation/bpf/bpf_devel_QA.rst
> > and follow rules for email subject, etc. For example, this patch should
> > have a subject like "[PATCH bpf-next] xxx".
>
> Thank you very much for your suggestion. I will pay attention to it next time.
>

Patch

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 6b07fa7081d9..c89b25344422 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1443,56 +1443,14 @@  bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	const struct bpf_func_proto *func_proto;
 
 	switch (func_id) {
-	case BPF_FUNC_map_lookup_elem:
-		return &bpf_map_lookup_elem_proto;
-	case BPF_FUNC_map_update_elem:
-		return &bpf_map_update_elem_proto;
-	case BPF_FUNC_map_delete_elem:
-		return &bpf_map_delete_elem_proto;
-	case BPF_FUNC_map_push_elem:
-		return &bpf_map_push_elem_proto;
-	case BPF_FUNC_map_pop_elem:
-		return &bpf_map_pop_elem_proto;
-	case BPF_FUNC_map_peek_elem:
-		return &bpf_map_peek_elem_proto;
-	case BPF_FUNC_map_lookup_percpu_elem:
-		return &bpf_map_lookup_percpu_elem_proto;
-	case BPF_FUNC_ktime_get_ns:
-		return &bpf_ktime_get_ns_proto;
-	case BPF_FUNC_ktime_get_boot_ns:
-		return &bpf_ktime_get_boot_ns_proto;
-	case BPF_FUNC_tail_call:
-		return &bpf_tail_call_proto;
-	case BPF_FUNC_get_current_task:
-		return &bpf_get_current_task_proto;
-	case BPF_FUNC_get_current_task_btf:
-		return &bpf_get_current_task_btf_proto;
-	case BPF_FUNC_task_pt_regs:
-		return &bpf_task_pt_regs_proto;
 	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;
-	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;
-	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;
-	case BPF_FUNC_get_prandom_u32:
-		return &bpf_get_prandom_u32_proto;
-	case BPF_FUNC_probe_read_user:
-		return &bpf_probe_read_user_proto;
-	case BPF_FUNC_probe_read_kernel:
-		return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ?
-		       NULL : &bpf_probe_read_kernel_proto;
-	case BPF_FUNC_probe_read_user_str:
-		return &bpf_probe_read_user_str_proto;
-	case BPF_FUNC_probe_read_kernel_str:
-		return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ?
-		       NULL : &bpf_probe_read_kernel_str_proto;
 #ifdef CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
 	case BPF_FUNC_probe_read:
 		return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ?
@@ -1502,10 +1460,6 @@  bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		       NULL : &bpf_probe_read_compat_str_proto;
 #endif
 #ifdef CONFIG_CGROUPS
-	case BPF_FUNC_cgrp_storage_get:
-		return &bpf_cgrp_storage_get_proto;
-	case BPF_FUNC_cgrp_storage_delete:
-		return &bpf_cgrp_storage_delete_proto;
 	case BPF_FUNC_current_task_under_cgroup:
 		return &bpf_current_task_under_cgroup_proto;
 #endif
@@ -1513,20 +1467,6 @@  bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_send_signal_proto;
 	case BPF_FUNC_send_signal_thread:
 		return &bpf_send_signal_thread_proto;
-	case BPF_FUNC_perf_event_read_value:
-		return &bpf_perf_event_read_value_proto;
-	case BPF_FUNC_ringbuf_output:
-		return &bpf_ringbuf_output_proto;
-	case BPF_FUNC_ringbuf_reserve:
-		return &bpf_ringbuf_reserve_proto;
-	case BPF_FUNC_ringbuf_submit:
-		return &bpf_ringbuf_submit_proto;
-	case BPF_FUNC_ringbuf_discard:
-		return &bpf_ringbuf_discard_proto;
-	case BPF_FUNC_ringbuf_query:
-		return &bpf_ringbuf_query_proto;
-	case BPF_FUNC_jiffies64:
-		return &bpf_jiffies64_proto;
 	case BPF_FUNC_get_task_stack:
 		return prog->sleepable ? &bpf_get_task_stack_sleepable_proto
 				       : &bpf_get_task_stack_proto;
@@ -1534,12 +1474,6 @@  bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_copy_from_user_proto;
 	case BPF_FUNC_copy_from_user_task:
 		return &bpf_copy_from_user_task_proto;
-	case BPF_FUNC_snprintf_btf:
-		return &bpf_snprintf_btf_proto;
-	case BPF_FUNC_per_cpu_ptr:
-		return &bpf_per_cpu_ptr_proto;
-	case BPF_FUNC_this_cpu_ptr:
-		return &bpf_this_cpu_ptr_proto;
 	case BPF_FUNC_task_storage_get:
 		if (bpf_prog_check_recur(prog))
 			return &bpf_task_storage_get_recur_proto;
@@ -1548,18 +1482,12 @@  bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		if (bpf_prog_check_recur(prog))
 			return &bpf_task_storage_delete_recur_proto;
 		return &bpf_task_storage_delete_proto;
-	case BPF_FUNC_for_each_map_elem:
-		return &bpf_for_each_map_elem_proto;
-	case BPF_FUNC_snprintf:
-		return &bpf_snprintf_proto;
 	case BPF_FUNC_get_func_ip:
 		return &bpf_get_func_ip_proto_tracing;
 	case BPF_FUNC_get_branch_snapshot:
 		return &bpf_get_branch_snapshot_proto;
 	case BPF_FUNC_find_vma:
 		return &bpf_find_vma_proto;
-	case BPF_FUNC_trace_vprintk:
-		return bpf_get_trace_vprintk_proto();
 	default:
 		break;
 	}