Re: [PATCH bpf-next v3 4/5] libbpf: fix error when st-prefix_ops and ops from differ btf

["D. Wythe" <alibuda@linux.alibaba.com >]

On Thu, Dec 19, 2024 at 02:43:30PM -0800, Martin KaFai Lau wrote:
> On 12/17/24 6:44 PM, D. Wythe wrote:
> >Here are four possible case:
> >
> >+--------+-------------+-------------+---------------------------------+
> >|        | st_opx_xxx  | xxx         |                                 |
> >+--------+-------------+-------------+---------------------------------+
> >| case 0 | btf_vmlinux | bft_vmlinux | be used and reg only in vmlinux |
> >+--------+-------------+-------------+---------------------------------+
> >| case 1 | btf_vmlinux | bpf_mod     | INVALID                         |
> >+--------+-------------+-------------+---------------------------------+
> >| case 2 | btf_mod     | btf_vmlinux | reg in mod but be used both in  |
> >|        |             |             | vmlinux and mod.                |
> >+--------+-------------+-------------+---------------------------------+
> >| case 3 | btf_mod     | btf_mod     | be used and reg only in mod     |
> >+--------+-------------+-------------+---------------------------------+
> >
> >At present, cases 0, 1, and 3 can be correctly identified, because
> >st_ops_xxx is searched from the same btf with xxx. In order to
> >handle case 2 correctly without affecting other cases, we cannot simply
> >change the search method for st_ops_xxx from find_btf_by_prefix_kind()
> >to find_ksym_btf_id(), because in this way, case 1 will not be
> >recognized anymore.
> >  	snprintf(tname, sizeof(tname), "%.*s",
> >@@ -1020,17 +1021,25 @@ find_struct_ops_kern_types(struct bpf_object *obj, const char *tname_raw,
> >  	}
> >  	kern_type = btf__type_by_id(btf, kern_type_id);
> >+	ret = snprintf(stname, sizeof(stname), "%s%s", STRUCT_OPS_VALUE_PREFIX, tname);
> 
> How about always look for "struct bpf_struct_ops_smc_ops" first,
> figure out the btf, and then look for "struct smc_ops", would it
> work?

I think this might not work, as the core issue lies in the fact that
bpf_struct_ops_smc_ops and smc_ops are located on different btf.
Searching for one fisrt cannot lead to the inference of the other.

> 
> If CONFIG_SMC=y instead of =m, this change cannot be tested?
> 

That is indeed a problem, but currently there is no better solution
unless the CI can add a step to run 'make modules_install'.


Best wishes,
D. Wythe

> >+	if (ret < 0 || ret >= sizeof(stname))
> >+		return -ENAMETOOLONG;
> >+
> >  	/* Find the corresponding "map_value" type that will be used
> >  	 * in map_update(BPF_MAP_TYPE_STRUCT_OPS).  For example,
> >  	 * find "struct bpf_struct_ops_tcp_congestion_ops" from the
> >  	 * btf_vmlinux.
> >  	 */
> >-	kern_vtype_id = find_btf_by_prefix_kind(btf, STRUCT_OPS_VALUE_PREFIX,
> >-						tname, BTF_KIND_STRUCT);
> >+	kern_vtype_id = btf__find_by_name_kind(btf, stname, BTF_KIND_STRUCT);
> >  	if (kern_vtype_id < 0) {
> >-		pr_warn("struct_ops init_kern: struct %s%s is not found in kernel BTF\n",
> >-			STRUCT_OPS_VALUE_PREFIX, tname);
> >-		return kern_vtype_id;
> >+		if (kern_vtype_id == -ENOENT && !*mod_btf)
> >+			kern_vtype_id = find_ksym_btf_id(obj, stname, BTF_KIND_STRUCT,
> >+							 &btf, mod_btf);
> >+		if (kern_vtype_id < 0) {
> >+			pr_warn("struct_ops init_kern: struct %s is not found in kernel BTF\n",
> >+				stname);
> >+			return kern_vtype_id;
> >+		}
> >  	}
> >  	kern_vtype = btf__type_by_id(btf, kern_vtype_id);
> >@@ -1046,8 +1055,8 @@ find_struct_ops_kern_types(struct bpf_object *obj, const char *tname_raw,
> >  			break;
> >  	}
> >  	if (i == btf_vlen(kern_vtype)) {
> >-		pr_warn("struct_ops init_kern: struct %s data is not found in struct %s%s\n",
> >-			tname, STRUCT_OPS_VALUE_PREFIX, tname);
> >+		pr_warn("struct_ops init_kern: struct %s data is not found in struct %s\n",
> >+			tname, stname);
> >  		return -EINVAL;
> >  	}