Re: [PATCH v2 bpf-next 3/3] veristat: guess and substitue underlying program type for freplace (EXT) progs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 24, 2023 at 04:27:45PM -0700, Andrii Nakryiko wrote:
> +static int guess_prog_type_by_ctx_name(const char *ctx_name,
> +				       enum bpf_prog_type *prog_type,
> +				       enum bpf_attach_type *attach_type)
> +{
> +	/* We need to guess program type based on its declared context type.
> +	 * This guess can't be perfect as many different program types might
> +	 * share the same context type.  So we can only hope to reasonably
> +	 * well guess this and get lucky.
> +	 *
> +	 * Just in case, we support both UAPI-side type names and
> +	 * kernel-internal names.
> +	 */
> +	static struct {
> +		const char *uapi_name;
> +		const char *kern_name;
> +		enum bpf_prog_type prog_type;
> +		enum bpf_attach_type attach_type;
> +	} ctx_map[] = {
> +		/* __sk_buff is most ambiguous, for now we assume cgroup_skb */
> +		{ "__sk_buff", "sk_buff", BPF_PROG_TYPE_CGROUP_SKB, BPF_CGROUP_INET_INGRESS },
> +		{ "bpf_sock", "sock", BPF_PROG_TYPE_CGROUP_SOCK, BPF_CGROUP_INET4_POST_BIND },
> +		{ "bpf_sock_addr", "bpf_sock_addr_kern",  BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_BIND },
> +		{ "bpf_sock_ops", "bpf_sock_ops_kern", BPF_PROG_TYPE_SOCK_OPS, BPF_CGROUP_SOCK_OPS },
> +		{ "sk_msg_md", "sk_msg", BPF_PROG_TYPE_SK_MSG, BPF_SK_MSG_VERDICT },
> +		{ "bpf_cgroup_dev_ctx", "bpf_cgroup_dev_ctx", BPF_PROG_TYPE_CGROUP_DEVICE, BPF_CGROUP_DEVICE },
> +		{ "bpf_sysctl", "bpf_sysctl_kern", BPF_PROG_TYPE_CGROUP_SYSCTL, BPF_CGROUP_SYSCTL },
> +		{ "bpf_sockopt", "bpf_sockopt_kern", BPF_PROG_TYPE_CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT },
> +		{ "sk_reuseport_md", "sk_reuseport_kern", BPF_PROG_TYPE_SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE },
> +		{ "bpf_sk_lookup", "bpf_sk_lookup_kern", BPF_PROG_TYPE_SK_LOOKUP, BPF_SK_LOOKUP },
> +		{ "xdp_md", "xdp_buff", BPF_PROG_TYPE_XDP, BPF_XDP },
> +		/* tracing types with no expected attach type */
> +		{ "bpf_user_pt_regs_t", "pt_regs", BPF_PROG_TYPE_KPROBE },
> +		{ "bpf_perf_event_data", "bpf_perf_event_data_kern", BPF_PROG_TYPE_PERF_EVENT },
> +		{ "bpf_raw_tracepoint_args", NULL, BPF_PROG_TYPE_RAW_TRACEPOINT },
> +	};
> +	int i;
> +
> +	if (!ctx_name)
> +		return -EINVAL;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx_map); i++) {
> +		if (strcmp(ctx_map[i].uapi_name, ctx_name) == 0 ||
> +		    (!ctx_map[i].kern_name || strcmp(ctx_map[i].kern_name, ctx_name) == 0)) {

I'm struggling to understand above A || (B || C) logic.
() are redundant?

> +			*prog_type = ctx_map[i].prog_type;
> +			*attach_type = ctx_map[i].attach_type;
> +			return 0;
> +		}
> +	}
> +
> +	return -ESRCH;

This will never be hit, since "bpf_raw_tracepoint_args", NULL is last and
it will always succeed.

The idea is to always succeed in guessing and default to raw_tp ?
... and there should be only one entry with kern_name == NULL...
But it's not mentioned anywhere in the comments.
A weird way to implement a default.
I'm definitely missing something.



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux