From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
To: "Toke Høiland-Jørgensen" <toke@redhat.com>
Cc: Jakub Kicinski <kuba@kernel.org>,
John Fastabend <john.fastabend@gmail.com>,
Jesper Dangaard Brouer <brouer@redhat.com>,
Andrii Nakryiko <andrii.nakryiko@gmail.com>,
BPF-dev-list <bpf@vger.kernel.org>,
Magnus Karlsson <magnus.karlsson@gmail.com>,
William Tu <u9012063@gmail.com>,
xdp-hints@xdp-project.net,
Zaremba Larysa <larysa.zaremba@intel.com>
Subject: Re: XDP-hints: Howto support multiple BTF types per packet basis?
Date: Wed, 1 Sep 2021 22:49:43 -0400 [thread overview]
Message-ID: <YTA7x6BIq85UWrYZ@localhost.localdomain> (raw)
In-Reply-To: <8735snvjp7.fsf@toke.dk>
On Fri, Jul 09, 2021 at 12:57:08PM +0200, Toke Høiland-Jørgensen wrote:
> Michal Swiatkowski <michal.swiatkowski@linux.intel.com> writes:
>
> >> I would expect that the program would decide ahead-of-time which BTF IDs
> >> it supports, by something like including the relevant structs from
> >> vmlinux.h. And then we need the BTF ID encoded into the packet metadata
> >> as well, so that it is possible to check at run-time which driver the
> >> packet came from (since a packet can be redirected, so you may end up
> >> having to deal with multiple formats in the same XDP program).
> >>
> >> Which would allow you to write code like:
> >>
> >> if (ctx->has_driver_meta) {
> >> /* this should be at a well-known position, like first (or last) in meta area */
> >> __u32 *meta_btf_id = ctx->data_meta;
> >>
> >> if (*meta_btf_id == BTF_ID_MLX5) {
> >> struct meta_mlx5 *meta = ctx->data_meta;
> >> /* do something with meta */
> >> } else if (meta_btf_id == BTF_ID_I40E) {
> >> struct meta_i40e *meta = ctx->data_meta;
> >> /* do something with meta */
> >> } /* etc */
> >> }
> >>
> >> and libbpf could do relocations based on the different meta structs,
> >> even removing the code for the ones that don't exist on the running
> >> kernel.
> >
> > This looks nice. In this case we need defintions of struct meta_mlx5 and
> > struct meta_i40e at build time. How are we going to deliver this to bpf
> > core app? This will be available in /sys/kernel/btf/mlx5 and
> > /sys/kernel/btf/i40e (if drivers are loaded). Should we dump this to
> > vmlinux.h? Or a developer of the xdp program should add this definition
> > to his code?
>
> Well, if the driver just defines the struct, the BTF for it will be
> automatically part of the driver module BTF. BPF program developers
> would need to include this in their programs somehow (similar to how
> you'll need to get the type definitions from vmlinux.h today to use
> CO-RE); how they do this is up to them. Since this is a compile-time
> thing it will probably depend on the project (for instance, BCC includes
> a copy of vmlinux.h in their source tree, but you can also just pick out
> the structs you need).
>
> > Maybe create another /sys/kernel/btf/hints with vmlinux and hints from
> > all drivers which support hints?
>
> It may be useful to have a way for the kernel to export all the hints
> currently loaded, so libbpf can just use that when relocating. The
> problem of course being that this will only include drivers that are
> actually loaded, so users need to make sure to load all their network
> drivers before loading any XDP programs. I think it would be better if
> the loader could discover all modules *available* on the system, but I'm
> not sure if there's a good way to do that.
>
> > Previously in this thread someone mentioned this ___ use case in libbpf
> > and proposed creating something like mega xdp hints structure with all
> > available fields across all drivers. As I understand this could solve
> > the problem about defining correct structure at build time. But how will
> > it work when there will be more than one structures with the same name
> > before ___? I mean:
> > struct xdp_hints___mega defined only in core app
> > struct xdp_hints___mlx5 available when mlx5 driver is loaded
> > struct xdp_hints___i40e available when i40e driver is loaded
> >
> > When there will be only one driver loaded should libbpf do correct
> > reallocation of fields? What will happen when both of the drivers are
> > loaded?
>
> I think we definitely need to make this easy for developers so they
> don't have to go and manually track down the driver structs and write
> the disambiguation code etc. I.e., the example code I included above
> that checks the frame BTF ID and does the loading based on it should be
> auto-generated. We already have some precedence for auto-generated code
> in vmlinux.h and the bpftool skeletons. So maybe we could have a command
> like 'bpftool gen_xdp_meta <fields>' which would go and lookup all the
> available driver structs and generate a code helper function that will
> extract the driver structs and generate the loader code? So that if,
> say, you're interested in rxhash and tstamp you could do:
>
> bpftool gen_xdp_meta rxhash tstamp > my_meta.h
>
> which would then produce my_meta.h with content like:
>
> struct my_meta { /* contains fields specified on the command line */
> u32 rxhash;
> u32 tstamp;
> }
>
> struct meta_mlx5 {/*generated from kernel BTF */};
> struct meta_i40e {/*generated from kernel BTF */};
>
> static inline int get_xdp_meta(struct xdp_md *ctx, struct my_meta *meta)
> {
> if (ctx->has_driver_meta) {
> /* this should be at a well-known position, like first (or last) in meta area */
> __u32 *meta_btf_id = ctx->data_meta;
>
> if (*meta_btf_id == BTF_ID_MLX5) {
> struct meta_mlx5 *meta = ctx->data_meta;
> my_meta->rxhash = meta->rxhash;
> my_meta->tstamp = meta->tstamp;
> return 0;
> } else if (meta_btf_id == BTF_ID_I40E) {
> struct meta_i40e *meta = ctx->data_meta;
> my_meta->rxhash = meta->rxhash;
> my_meta->tstamp = meta->tstamp;
> return 0;
> } /* etc */
> }
> return -ENOENT;
> }
According to meta_btf_id. Do You have an idea how driver should fill this
field?
hints->btf_id = btf_id_by_name("struct meta_i40e"); /* fill btf id at
config time */
btf_id_by_name will get module btf (or vmlinux btf) and search for
correct name for each ids. Does this look correct?
Is there any way in kernel to get btf id based on name or we have to
write functions for this? I haven't seen code for this case, but maybe I
missed it.
>
>
> -Toke
>
next prev parent reply other threads:[~2021-09-02 6:43 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20210526125848.1c7adbb0@carbon>
[not found] ` <CAEf4BzYXUDyQaBjZmb_Q5-z3jw1-Uvdgxm+cfcQjSwb9oRoXnQ@mail.gmail.com>
[not found] ` <60aeb01ebcd10_fe49208b8@john-XPS-13-9370.notmuch>
[not found] ` <CAEf4Bza3m5dwZ_d0=zAWR+18f5RUjzv9=1NbhTKAO1uzWg_fzQ@mail.gmail.com>
[not found] ` <60aeeb5252147_19a622085a@john-XPS-13-9370.notmuch>
[not found] ` <CAEf4Bzb1OZHpHYagbVs7s9tMSk4wrbxzGeBCCBHQ-qCOgdu6EQ@mail.gmail.com>
[not found] ` <60b08442b18d5_1cf8208a0@john-XPS-13-9370.notmuch>
2021-05-28 9:16 ` Toke Høiland-Jørgensen
2021-05-28 10:38 ` Alexander Lobakin
2021-05-28 14:35 ` John Fastabend
2021-05-28 15:33 ` Toke Høiland-Jørgensen
2021-05-28 16:02 ` Jesper Dangaard Brouer
2021-05-28 17:29 ` John Fastabend
2021-05-30 3:27 ` Andrii Nakryiko
2021-05-31 11:03 ` Toke Høiland-Jørgensen
2021-05-31 13:17 ` Jesper Dangaard Brouer
2021-06-02 0:22 ` John Fastabend
2021-06-02 16:18 ` Jakub Kicinski
2021-06-22 7:44 ` Michal Swiatkowski
2021-06-22 11:53 ` Toke Høiland-Jørgensen
2021-06-23 8:32 ` Michal Swiatkowski
2021-06-24 12:23 ` Toke Høiland-Jørgensen
2021-06-24 13:07 ` Magnus Karlsson
2021-06-24 14:58 ` Alexei Starovoitov
2021-06-24 15:11 ` Zvi Effron
2021-06-24 16:04 ` Toke Høiland-Jørgensen
2021-06-24 16:32 ` Zvi Effron
2021-06-24 16:45 ` Jesper Dangaard Brouer
2021-07-08 8:32 ` Michal Swiatkowski
2021-07-09 10:57 ` Toke Høiland-Jørgensen
2021-09-02 2:49 ` Michal Swiatkowski [this message]
2021-09-02 9:17 ` Jesper Dangaard Brouer
2021-09-07 6:27 ` Michal Swiatkowski
2021-09-08 13:28 ` Jesper Dangaard Brouer
2021-09-09 18:19 ` Andrii Nakryiko
2021-09-10 11:16 ` Jesper Dangaard Brouer
[not found] ` <20210526222023.44f9b3c6@carbon>
[not found] ` <CAEf4BzZ+VSemxx7WFanw7DfLGN7w42G6ZC4dvOSB1zAsUgRQaw@mail.gmail.com>
2021-05-28 11:16 ` Jesper Dangaard Brouer
2021-05-30 3:24 ` Andrii Nakryiko
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
List information: https://lists.xdp-project.net/postorius/lists/xdp-hints.xdp-project.net/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YTA7x6BIq85UWrYZ@localhost.localdomain \
--to=michal.swiatkowski@linux.intel.com \
--cc=andrii.nakryiko@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=brouer@redhat.com \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=larysa.zaremba@intel.com \
--cc=magnus.karlsson@gmail.com \
--cc=toke@redhat.com \
--cc=u9012063@gmail.com \
--cc=xdp-hints@xdp-project.net \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox