[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250924080634.GA14633@j66a10360.sqa.eu95>
Date: Wed, 24 Sep 2025 16:06:34 +0800
From: "D. Wythe" <alibuda@...ux.alibaba.com >
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: "D. Wythe" <alibuda@...ux.alibaba.com>, ast@...nel.org,
daniel@...earbox.net, andrii@...nel.org, martin.lau@...ux.dev,
pabeni@...hat.com, song@...nel.org, sdf@...gle.com,
haoluo@...gle.com, yhs@...com, edumazet@...gle.com,
john.fastabend@...il.com, kpsingh@...nel.org, jolsa@...nel.org,
mjambigi@...ux.ibm.com, wenjia@...ux.ibm.com, wintera@...ux.ibm.com,
dust.li@...ux.alibaba.com, tonylu@...ux.alibaba.com,
guwen@...ux.alibaba.com, bpf@...r.kernel.org, davem@...emloft.net,
kuba@...nel.org, netdev@...r.kernel.org, sidraya@...ux.ibm.com,
jaka@...ux.ibm.com
Subject: Re: [PATCH bpf-next v2 3/4] libbpf: fix error when st-prefix_ops and
ops from differ btf
On Fri, Sep 19, 2025 at 03:22:41PM -0700, Andrii Nakryiko wrote:
> On Thu, Sep 18, 2025 at 1:03 AM D. Wythe <alibuda@...ux.alibaba.com> wrote:
> >
> >
> > At present, cases 0, 1, and 3 can be correctly identified, because
> > st_ops_xxx_ops is searched from the same btf with xxx_ops. In order to
> > handle case 2 correctly without affecting other cases, we cannot simply
> > change the search method for st_ops_xxx_ops from find_btf_by_prefix_kind()
> > to find_ksym_btf_id(), because in this way, case 1 will not be
> > recognized anymore.
> >
> > To address the issue, we always look for st_ops_xxx_ops first,
> > figure out the btf, and then look for xxx_ops with the very btf to avoid
>
> What's "very btf"? Commit message would benefit from a little bit of
> proof-reading, if you can. It's a bit hard to follow, even if it's
> more or less clear at the end what problem you are trying to solve.
>
> Also, I'd suggest to send this fix as a separate patch and not block
> it on the overall patch set, which probably will take longer. This fix
> is independent, so we can land it much faster.
>
Thanks a lot for your suggestion. I'll improve the commit message.
It's fine to send this patch separately, I'm concerned it won't be
verifiable in isolation without the other patches. But if you feel
it's okay to proceed, I'm happy to do so.
> > such issue.
> >
> > Fixes: 590a00888250 ("bpf: libbpf: Add STRUCT_OPS support")
> > Signed-off-by: D. Wythe <alibuda@...ux.alibaba.com>
> > Acked-by: Andrii Nakryiko <andrii@...nel.org>
> > ---
> > tools/lib/bpf/libbpf.c | 37 ++++++++++++++++++-------------------
> > 1 file changed, 18 insertions(+), 19 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index fe4fc5438678..50ca13833511 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -1013,35 +1013,34 @@ find_struct_ops_kern_types(struct bpf_object *obj, const char *tname_raw,
> > const struct btf_member *kern_data_member;
> > struct btf *btf = NULL;
> > __s32 kern_vtype_id, kern_type_id;
> > - char tname[256];
> > + char tname[256], stname[256];
> > __u32 i;
> >
> > snprintf(tname, sizeof(tname), "%.*s",
> > (int)bpf_core_essential_name_len(tname_raw), tname_raw);
> >
> > - kern_type_id = find_ksym_btf_id(obj, tname, BTF_KIND_STRUCT,
> > - &btf, mod_btf);
> > - if (kern_type_id < 0) {
> > - pr_warn("struct_ops init_kern: struct %s is not found in kernel BTF\n",
> > - tname);
> > - return kern_type_id;
> > - }
> > - kern_type = btf__type_by_id(btf, kern_type_id);
> > + snprintf(stname, sizeof(stname), "%s%.*s", STRUCT_OPS_VALUE_PREFIX,
> > + (int)strlen(tname), tname);
> >
> > - /* 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.
> > + /* Look for the corresponding "map_value" type that will be used
> > + * in map_update(BPF_MAP_TYPE_STRUCT_OPS) first, figure out the btf
> > + * and the mod_btf.
> > + * For example, find "struct bpf_struct_ops_tcp_congestion_ops".
> > */
> > - kern_vtype_id = find_btf_by_prefix_kind(btf, STRUCT_OPS_VALUE_PREFIX,
> > - tname, BTF_KIND_STRUCT);
> > + 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%s is not found in kernel BTF\n",
> > - STRUCT_OPS_VALUE_PREFIX, tname);
> > + 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);
> >
> > + kern_type_id = btf__find_by_name_kind(btf, tname, BTF_KIND_STRUCT);
> > + if (kern_type_id < 0) {
> > + pr_warn("struct_ops init_kern: struct %s is not found in kernel BTF\n", tname);
> > + return kern_type_id;
> > + }
> > + kern_type = btf__type_by_id(btf, kern_type_id);
> > +
> > /* Find "struct tcp_congestion_ops" from
> > * struct bpf_struct_ops_tcp_congestion_ops {
> > * [ ... ]
> > @@ -1054,8 +1053,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;
> > }
> >
> > --
> > 2.45.0
> >
Powered by blists - more mailing lists