[xdp-hints] Re: [PATCH RFC bpf-next 5/9] xdp: controlling XDP-hints from BPF-prog via helper

[Larysa Zaremba <larysa.zaremba@intel.com>]

On Mon, Jul 04, 2022 at 08:26:15PM +0200, Jesper Dangaard Brouer wrote:
> 
> 
> On 04/07/2022 13.00, Zaremba, Larysa wrote:
> > Toke Høiland-Jørgensen <toke@redhat.com> writes:
> > > 
> > > Jesper Dangaard Brouer <jbrouer@redhat.com> writes:
> > > 
> > > > On 29/06/2022 16.20, Toke Høiland-Jørgensen wrote:
> > > > > Jesper Dangaard Brouer <brouer@redhat.com> writes:
> > > > > 
> > > > > > XDP BPF-prog's need a way to interact with the XDP-hints. This
> > > > > > patch introduces a BPF-helper function, that allow XDP BPF-prog's
> > > > > > to interact with the XDP-hints.
> > > > > > 
> > > > > > BPF-prog can query if any XDP-hints have been setup and if this is
> > > > > > compatible with the xdp_hints_common struct. If XDP-hints are
> > > > > > available the BPF "origin" is returned (see enum
> > > > > > xdp_hints_btf_origin) as BTF can come from different sources or
> > > > > > origins e.g. vmlinux, module or local.
> > > > > 
> > > > > I'm not sure I quite understand what this origin is supposed to be
> > > > > good for?
> > > > 
> > > > Some background info on BTF is needed here: BTF_ID numbers are not
> > > > globally unique identifiers, thus we need to know where it originate
> > > > from, to make it unique (as we store this BTF_ID in XDP-hints).
> > > > 
> > > > There is a connection between origin "vmlinux" and "module", which
> > > > is that vmlinux will start at ID=1 and end at a max ID number.
> > > > Modules refer to ID's in "vmlinux", and for this to work, they will
> > > > shift their own numbering to start after ID=max-vmlinux-id.
> > > > 
> > > > Origin "local" is for BTF information stored in the BPF-ELF object file.
> > > > Their numbering starts at ID=1.  The use-case is that a BPF-prog
> > > > want to extend the kernel drivers BTF-layout, and e.g. add a
> > > > RX-timestamp like [1].  Then BPF-prog can check if it knows module's
> > > > BTF_ID and then extend via bpf_xdp_adjust_meta, and update BTF_ID in
> > > > XDP-hints and call the helper (I introduced) marking this as origin
> > > > "local" for kernel to know this is no-longer origin "module".
> > > 
> > > Right, I realise that :)
> > > 
> > > My point was that just knowing "this is a BTF ID coming from a module"
> > > is not terribly useful; you could already figure that out by just
> > > looking at the ID and seeing if it's larger than the maximum ID in vmlinux BTF.
> > > 
> > > Rather, what we need is a way to identify *which* module the BTF ID
> > > comes from; and luckily, the kernel assigns a unique ID to every BTF
> > > *object* as well as to each type ID within that object. These can be
> > > dumped by bpftool:
> > > 
> > > # bpftool btf
> > > bpftool btf
> > > [sudo] password for alrua:
> > > 1: name [vmlinux]  size 4800187B
> > > 2: name [serio]  size 2588B
> > > 3: name [i8042]  size 11786B
> > > 4: name [rng_core]  size 8184B
> > > [...]
> > > 2062: name <anon>  size 36965B
> > > 	pids bpftool(547298)
> > > 
> > > IDs 2-4 are module BTF objects, and that last one is the ID of a BTF
> > > object loaded along with a BPF program by bpftool itself... So we *do*
> > > in fact have a unique ID, by combining the BTF object ID with the type
> > > ID; this is what Alexander is proposing to put into the xdp-hints
> > > struct as well (combining the two IDs into a single u64).
> 
> Thanks for the explanation. I think I understand it now, and I agree
> that we should extend/combining the two IDs into a single u64.
> 
> To Andrii, what is the right terminology when talking about these two
> different BTF-ID's:
> 
> - BTF object ID and BTF type ID?
> 
> - Where BTF *object* ID are the IDs we see above from 'bpftool btf',
>   where vmlinux=1 and module's IDs will start after 1.
> 
> - Where BTF *type* ID are the IDs the individual data "types" within a
>   BTF "object" (e.g. struct xdp_hints_common that BPF-prog's can get
>   via calling bpf_core_type_id_kernel()).
> 

AFAIK, that's the most correct way of distinguish one from another in 
conversation.

Would be still great, if Andrii could confirm that.

I should mention that out patch makes bpf_core_type_id_kernel() return 
u64 (BTF obj ID + BTF type ID), but your statement is true for current 
libbpf version.

> 
> > That's correct, concept was previously discussed [1]. The ID of BTF object wasn't
> > exposed in CO-RE allocations though, we've changed it in the first 4 patches.
> > The main logic is in "libbpf: factor out BTF loading from load_module_btfs()"
> > and "libbpf: patch module BTF ID into BPF insns".
> > 
> > We have a sample that wasn't included eventually, but can possibly
> > give a general understanding of our approach [2].
> > 
> > [1] https://lore.kernel.org/all/CAEf4BzZO=7MKWfx2OCwEc+sKkfPZYzaELuobi4q5p1bOKk4AQQ@mail.gmail.com/
> > [2] https://github.com/alobakin/linux/pull/16/files#diff-c5983904cbe0c280453d59e8a1eefb56c67018c38d5da0c1122abc86225fc7c9
> > 
> (appreciate the links)
> 
> I wonder how these BTF object IDs gets resolved for my "local" category?
> (Origin "local" is for BTF information stored in the BPF-ELF object file)
> 
> Note: For "local" BTF type IDs BPF-prog resolve these via
> bpf_core_type_id_local() (why I choose the term "local").
> 

Every program during CO-RE relocs sees a single local BTF obj, in which 
BTF type IDs start from 1 and correspond to all data types used in 
program. So local BTF obj and type IDs inside are valid only in single 
program, therefore u32 type ID returned by bpf_core_type_id_local() is 
enough.

Local IDs are not resolved, they are just assigned during compilation. 
After program load with CO-RE each local type gets a resolved
vmlinux/module BTF obj pointer and an ID of a type inside this BTF obj 
that is similar enough.

Both local and target type IDs are mainly needed just for comfortable 
iteration inside libbpf, so they are just a side product that is only 
patched in, if we use bpf_core_type_id_local/target() inside a program 
for testing purposes.

> --Jesper
> 
> p.s. For unknown reasons lore.kernel.org did match Larysa's reply with the
> patchset thread here[3].
> 
>  [3] https://lore.kernel.org/bpf/165643378969.449467.13237011812569188299.stgit@firesoul/#r
> 
> 

- Larysa