[RFC] Generic metadata AF XDP

[edersondisouza]

Hi,

This is the AF_XDP side of xdp hints/metadata I've been working on, on top of Michal
metadata work.

Given I'm usually interested in the igc driver (as I have i225 to test on), this PR
has some igc patches that my work is on top of. I've also picked some Quentin Monnet
patches for libbpf that live on bpfnext, so that parsing BTF is a bit easier.

As there's no BTF registration step for the driver, I ended up exposing some
BTF related symbols (as well as providing a new call), so that drivers can
get the BTF id they need to put on the btf_id inside the XDP metadata.

One thing I noticed is that, with no user of xdp_meta_generic inside the kernel,
it is currently exposed on the module BTF (unless module is built-in), so this
PR assumes that is the case - for instance, the driver will get its own btf
(via new btf_get_from_module call), unless it knows is built-in, in which case
it gets the btf id from vmlinux BTF.

Currently, there's no space for a TX timestamp on xdp_meta_generic. Instead
of trying to put it inside it while preserving the 32 bytes size, I decided
to test a custom way. Basically, I defined an igc version of it,
xdp_meta_generic___igc, which contains the fields I currently care about.
Using the BTF information, I can still get the right size and offset
for the field in the AF_XDP application, and IIUC, the same should just
work for BPF programs with CO-RE. Note that while this is a nice exploration
for custom metadata, I welcome ideas on how to add the TX timestamp to
the generic struct - maybe using a union to separate TX and RX relevant fields?

Note that in this series I still have the ethtool way to get how big the
xdp_headroom shall be. I saw that Jesper proposed some way of getting that
information, but I didn't look carefully into it yet.

Another note is that the xsk socket helpers are still added to in kernel
libbpf. I know that those should go towards libxdp - I just kept them here
for now to ease review (I think that everything together is easier to
review than on different projects, but let me know if this doesn't work).

Finally, I also added some (small) fixes to get things compiling along the
way.

I'm eager to get any feedback (critics, comments, suggestions) =D

[mswiatko]

Isn't this btf_id of whole definition for module or vmlinux? Maybe something like that to get id of used structure:
adapter->btf_id = btf_find_by_name_kind(btf, "xdp_meta_generic__igc", BTF_KIND_STRUCT);
What do You think?

[edersondisouza]

I tested here, it doesn't really help. I still need the base (vmlinux) and module btf to parse the btf for the struct. So, i'd need to send two ids instead of one. Following is a snippet of the code I ended up testing with:

    base = btf__parse("/sys/kernel/btf/vmlinux", NULL);
    btf = btf__load_from_kernel_by_id_split(66, base); // "66" is the module btf_id
    t = btf__type_by_id(btf, 133298); // "133298" is the xdp_meta_generic btf_id

[mswiatko]

Can You try btf_get_type_id from this commit 36335fe ?

Why do You need btf? I thought type id will be enough in hints case.

[edersondisouza]

I'll take a look! I needed to also get btf, is because the type is, IIUC, relative to the module. So btf__get_from_id(base) fails.

[edersondisouza]

Err, I mean, btf__type_by_id. I took a look at using btf_get_type_id. It gives me the BTF type ID of the xdp_meta_generic struct. But that alone is not useful on user space - I need the BTF ID for the module as well. Check Andrii's email to Jesper [1].

So it's kinda pointless to include only the BTF type ID. If we include the BTF ID for the module, it's easy to check if any type there has prefix name "xdp_meta_generic". Unless I'm missing a way to get the BTF ID for the module - is there one?

[1]https://www.spinics.net/lists/bpf/msg45746.html

[edersondisouza]

On Tue, 2021-09-07 at 00:29 -0700, mswiatko wrote: @mswiatko commented on this pull request. In drivers/net/ethernet/intel/igc/igc_main.c: > @@ -4206,6 +4236,19 @@ static int igc_init_interrupt_scheme(struct > igc_adapter *adapter, bool msix)   return err;  }   +static void igc_btf_init(struct igc_adapter *adapter) +{ + struct module *owner = THIS_MODULE; + struct btf *btf; + + if (owner) + btf = btf_get_from_module(owner); + else + btf = bpf_get_btf_vmlinux(); + + adapter->btf_id = btf_obj_id(btf); Isn't this btf_id of whole definition for module or vmlinux? Maybe something like that to get id of used structure: adapter->btf_id = btf_find_by_name_kind(btf, "xdp_meta_generic__igc", BTF_KIND_STRUCT); What do You think?

I'll take a look!

…

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[edersondisouza]

v2: Experiment with using a union inside xdp_meta_generic so as to have more space, since RX and TX relevant data are different.

[walking-machine]

Maybe return ERR_PTR(-ENOSYS) or hide function completely if CONFIG_DEBUG_INFO_BTF_MODULES is not defined?

[edersondisouza]

I just followed bpf_try_get_module pattern. Not sure if hiding an exported symbol is a good idea, but returning ERR_PTR(-ENOSYS) sounds fine to me.