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; > > }