From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Qais Yousef <qais.yousef@arm.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
open list <linux-kernel@vger.kernel.org>,
Arnaldo Carvalho de Melo <acme@kernel.org>
Subject: Re: BTFIDS: FAILED unresolved symbol udp6_sock
Date: Mon, 4 Jan 2021 12:54:39 -0800 [thread overview]
Message-ID: <CAEf4BzbEoVHNw71XifTHm=dFU8ix+_+MdKBi+sT65+jj-mZQFA@mail.gmail.com> (raw)
In-Reply-To: <20210102230654.GA732432@krava>
On Sat, Jan 2, 2021 at 3:07 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Sat, Jan 02, 2021 at 02:25:34PM -0800, Andrii Nakryiko wrote:
>
> SNIP
>
> > >
> > > so your .config has
> > > CONFIG_CRYPTO_DEV_BCM_SPU=y
> > >
> > > and that defines 'struct device_private' which
> > > clashes with the same struct defined in drivers/base/base.h
> > >
> > > so several networking structs will be doubled, like net_device:
> > >
> > > $ bpftool btf dump file ../vmlinux.config | grep net_device\' | grep STRUCT
> > > [2731] STRUCT 'net_device' size=2240 vlen=133
> > > [113981] STRUCT 'net_device' size=2240 vlen=133
> > >
> > > each is using different 'struct device_private' when it's unwinded
> > >
> > > and that will confuse BTFIDS logic, becase we have multiple structs
> > > with the same name, and we can't be sure which one to pick
> > >
> > > perhaps we should check on this in pahole and warn earlier with
> > > better error message.. I'll check, but I'm not sure if pahole can
> > > survive another hastab ;-)
> > >
> > > Andrii, any ideas on this? ;-)
> >
> > It's both unavoidable and correct from the C type system's
> > perspective, so there is nothing for pahole to warn about. We used to
> > have (for a long time) a similar clash with two completely different
> > ring_buffer structs. Eventually they just got renamed to avoid
> > duplication of related structs (task_struct and tons of other). But
> > both BTF dedup and CO-RE relocation algorithms are designed to handle
> > this correctly, ...
>
> AFAIU it's all correctly dedulicated, but still all structs that
> contain (at some point) 'struct device_private' will appear twice
> in BTF data.. each with different 'struct device_private'
it's correct from the type system perspective, right. Those two
duplicates of struct device_private are parts of two different
hierarchies of types. However inconvenient it is, C allows it,
unfortunately :(
>
> > ... so perhaps BTFIDS should be able to handle this as
> > well?
>
> hm, BTFIDS sees BTF data with two same struct names and has no
> way to tell which one to use
>
> unless we have some annotation data for BTF types I don't
> see a way to handle this correctly.. but I think we can
> detect this directly in BTFIDS and print more accurate error
> message
>
> as long as we dont see this on daily basis, I think that better
> error message + following struct rename is good solution
Perhaps warning and handling this gracefully is a bit better way to
handle this. Renaming is definitely good, but shouldn't block the
kernel build process. I don't remember the exact details for why
duplicate struct would cause troubles for resolve_btfids, but maybe
just picking the struct with minimal ID (out of 2+ duplicates) would
be ok in practice most of the time. In any case, that's what most
users (including libbpf) will do, when searching for the type by name.
>
> >
> > >
> > > easy fix is the patch below that renames the bcm's structs,
> > > it makes the kernel to compile.. but of course the new name
> > > is probably wrong and we should push this through that code
> > > authors
> >
> > In this case, I think renaming generic device_private name is a good
> > thing regardless.
>
> ok, I'll send the change
>
great, thanks
> jirka
>
prev parent reply other threads:[~2021-01-04 23:59 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-29 15:13 BTFIDS: FAILED unresolved symbol udp6_sock Qais Yousef
2020-12-29 17:34 ` Jiri Olsa
2020-12-29 23:28 ` Qais Yousef
2020-12-30 9:03 ` Jiri Olsa
2020-12-30 13:27 ` Jiri Olsa
2020-12-30 13:28 ` Jiri Olsa
2020-12-30 14:19 ` Arnaldo Carvalho de Melo
2020-12-30 15:04 ` Jiri Olsa
2020-12-30 15:29 ` Qais Yousef
2021-01-02 22:25 ` Andrii Nakryiko
2021-01-02 23:06 ` Jiri Olsa
2021-01-04 20:54 ` Andrii Nakryiko [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAEf4BzbEoVHNw71XifTHm=dFU8ix+_+MdKBi+sT65+jj-mZQFA@mail.gmail.com' \
--to=andrii.nakryiko@gmail.com \
--cc=acme@kernel.org \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=jolsa@kernel.org \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=qais.yousef@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.