On Sat, 24 Sept 2022 at 01:49, Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Fri, Sep 23, 2022 at 4:38 PM Kumar Kartikeya Dwivedi
> <memxor@xxxxxxxxx> wrote:
> >
> > On Sat, 24 Sept 2022 at 01:20, Andrii Nakryiko
> > <andrii.nakryiko@xxxxxxxxx> wrote:
> > >
> > > On Fri, Sep 23, 2022 at 3:32 PM Kumar Kartikeya Dwivedi
> > > <memxor@xxxxxxxxx> wrote:
> > > >
> > > > Hi,
> > > > For the following example:
> > > >
> > > > kkd@Legion ~/src/linux
> > > > ; cat bpf.c
> > > > #define tag __attribute__((btf_decl_tag("tag")))
> > > >
> > > > int a tag;
> > > > int b tag;
> > > >
> > > > int main() {
> > > > return a + b;
> > > > }
> > > >
> > > > --
> > > >
> > > > When I compile using:
> > > > clang -target bpf -O2 -g -c bpf.c
> > > >
> > > > For the BTF dump, I see:
> > > > [1] FUNC_PROTO '(anon)' ret_type_id=2 vlen=0
> > > > [2] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
> > > > [3] FUNC 'main' type_id=1 linkage=global
> > > > [4] VAR 'a' type_id=2, linkage=global
> > > > [5] DECL_TAG 'tag' type_id=4 component_idx=-1
> > > > [6] VAR 'b' type_id=2, linkage=global
> > > > [7] DECL_TAG 'tag' type_id=6 component_idx=-1
> > > > [8] DATASEC '.bss' size=0 vlen=2
> > > > type_id=4 offset=0 size=4 (VAR 'a')
> > > > type_id=6 offset=0 size=4 (VAR 'b')
> > > >
> > > > There are two issues that I hit:
> > > >
> > > > 1. The component_idx=-1 makes it a little inconvenient to correlate
> > > > the tag applied to a VAR in a DATASEC. In case of structs the index
> > > > can be matched with component_idx, in case of DATASEC we have to match
> > > > VAR's type_id. So the code has to be different. If it also had
> > > > component_idx set it would be possible to make the code same for both
> > > > inside the kernel's field parsing routine.
> > >
> > > This is expected and documented in UAPI:
> > >
> > > "If component_idx == -1, the tag is applied to a struct, union,
> > > variable or function."
> > >
> > > Variable is an independent entity and tag applies to it. I don't
> > > know/remember why we did it this way, but it's probably easier for
> > > Clang to generate it this way. And it probably is easier for BPF
> > > static linker as well. Note that you don't merge two structs together,
> > > while static linker does merge variables between DATASECs.
> > >
> > > In short, DATASEC+VAR is sufficiently different from STRUCT+fields, so
> > > is treated differently.
> > >
> >
> > I see, thanks. In that case I'll add this description as a comment
> > where the code matches the tag to the VAR.
> >
> > > >
> > > > 2. The second issue is that the offset is always 0 for DATASEC VARs.
> > > > That makes it difficult to ensure proper alignment of the variables.
> > >
> > > That's also expected (even if unfortunate) behavior of Clang. Good
> > > news is that libbpf's static linker API is normalizing this. Libbpf
> > > itself also normalizes it internally if passed .bpf.o file straight
> > > from Clang's output.
> > >
> > > In short, if you run `bpftool gen object my_normalized.bpf.o
> > > my_clang.bpf.o` you'll get offsets in my_normalized.bpf.o's BTF.
> > >
> >
> > I see. It would be better if this was fixed up by clang itself, but I
> > don't know enough about the BPF backend to comment whether that would
> > make sense.
> >
> > It's unlikely people will run it through libbpf's linker when they're
> > not statically linking more than one object (or even be using libbpf,
> > but that's a separate thing), and it's very hard to correlate the
> > errors to their cause when failure happens during MAP_CREATE (I guess
> > eventually it should support a log buffer like btf load and prog
> > load), so in the end it would be confusing for users.
> >
> > However we do need to ensure the offsets are correct to detect proper
> > alignment. So adding some checks looks inevitable.
> > I guess for the first VAR, we can assume offset==0 is fine, but from
> > the next VAR, we should return an error if we see offset==0 again?
> > This should catch the not normalized case in case of DATASEC vlen > 1
> > (== 1 should still be ok). wdyt?
> >
>
> It's unclear who's "we", who's loading what, when and where, tbh. So
> please provide a bit of a context. We started from a simple isolated C
> example and two questions about BTF, and now I'm not sure what we are
> talking about at all.
>
> Libbpf patches BTF and assigns correct offsets before loading BTF into
> the kernel. But I assume whatever you are doing doesn't use libbpf?
>
Sorry :), my bad. So this is about the bpf list series.
struct bpf_list_head head __contains(foo, node); // global variable
That 'contains' is a declaration tag.
In https://lore.kernel.org/bpf/20220904204145.3089-14-memxor@xxxxxxxxx
I was calling btf_find_list_head inside btf_find_datasec_var with
index == -1 (third argument) which is eventually matched with
component_idx in btf_find_decl_tag_value.
So that part is something you clarified, but earlier in
btf_find_datasec_var the code also checks the alignment using the
offset of the VAR.
What I was trying to say is that we cannot assume libbpf fixes up the
offsets for the map BTF inside the kernel, a user may load a
non-normalized BTF with all offsets of DATASEC VARs as 0, so we should
probably still reject seeing off == 0 after the first VAR in a
DATASEC, otherwise the off % align check can be bypassed.
Hopefully it's clearer now.
> > > >
> > > > I would like to know if these are expected behaviors or bugs?
> > >
> > > Features ;)
> > >
> > > > Thanks
> > > > --
> > > > ; clang --version
> > > > Ubuntu clang version
> > > > 16.0.0-++20220813052912+eaf0aa1f1fbd-1~exp1~20220813173018.344
