From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mail.toke.dk (Postfix) with ESMTPS id EF9248A6778 for ; Thu, 2 Sep 2021 08:43:54 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10094"; a="217134450" X-IronPort-AV: E=Sophos;i="5.84,371,1620716400"; d="scan'208";a="217134450" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Sep 2021 23:43:50 -0700 X-IronPort-AV: E=Sophos;i="5.84,371,1620716400"; d="scan'208";a="688875241" Received: from unknown (HELO localhost.localdomain) ([10.102.102.63]) by fmsmga006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Sep 2021 23:43:48 -0700 Date: Wed, 1 Sep 2021 22:49:43 -0400 From: Michal Swiatkowski To: Toke =?iso-8859-1?Q?H=F8iland-J=F8rgensen?= Subject: Re: XDP-hints: Howto support multiple BTF types per packet basis? Message-ID: References: <60b12897d2e3f_1cf820896@john-XPS-13-9370.notmuch> <8735u3dv2l.fsf@toke.dk> <60b6cf5b6505e_38d6d208d8@john-XPS-13-9370.notmuch> <20210602091837.65ec197a@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net> <874kdqqfnm.fsf@toke.dk> <87mtrfmoyh.fsf@toke.dk> <8735snvjp7.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <8735snvjp7.fsf@toke.dk> Message-ID-Hash: HSK3YZ4PFQ5SSBW2MVF7PDWDVEZ3RNF6 X-Message-ID-Hash: HSK3YZ4PFQ5SSBW2MVF7PDWDVEZ3RNF6 X-MailFrom: michal.swiatkowski@linux.intel.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Jakub Kicinski , John Fastabend , Jesper Dangaard Brouer , Andrii Nakryiko , BPF-dev-list , Magnus Karlsson , William Tu , xdp-hints@xdp-project.net, Zaremba Larysa X-Mailman-Version: 3.3.4 Precedence: list List-Id: XDP hardware hints design discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Fri, Jul 09, 2021 at 12:57:08PM +0200, Toke H=F8iland-J=F8rgensen wrote: > Michal Swiatkowski writes: >=20 > >> I would expect that the program would decide ahead-of-time which BTF I= Ds > >> 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). > >>=20 > >> Which would allow you to write code like: > >>=20 > >> if (ctx->has_driver_meta) { > >> /* this should be at a well-known position, like first (or last) in = meta area */ > >> __u32 *meta_btf_id =3D ctx->data_meta; > >> =20 > >> if (*meta_btf_id =3D=3D BTF_ID_MLX5) { > >> struct meta_mlx5 *meta =3D ctx->data_meta; > >> /* do something with meta */ > >> } else if (meta_btf_id =3D=3D BTF_ID_I40E) { > >> struct meta_i40e *meta =3D ctx->data_meta; > >> /* do something with meta */ > >> } /* etc */ > >> } > >>=20 > >> 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? >=20 > 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). >=20 > > Maybe create another /sys/kernel/btf/hints with vmlinux and hints from > > all drivers which support hints? >=20 > 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. >=20 > > 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? >=20 > 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 ' 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: >=20 > bpftool gen_xdp_meta rxhash tstamp > my_meta.h >=20 > which would then produce my_meta.h with content like: >=20 > struct my_meta { /* contains fields specified on the command line */ > u32 rxhash; > u32 tstamp; > } >=20 > struct meta_mlx5 {/*generated from kernel BTF */}; > struct meta_i40e {/*generated from kernel BTF */}; >=20 > 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 me= ta area */ > __u32 *meta_btf_id =3D ctx->data_meta; > =20 > if (*meta_btf_id =3D=3D BTF_ID_MLX5) { > struct meta_mlx5 *meta =3D ctx->data_meta; > my_meta->rxhash =3D meta->rxhash; > my_meta->tstamp =3D meta->tstamp; > return 0; > } else if (meta_btf_id =3D=3D BTF_ID_I40E) { > struct meta_i40e *meta =3D ctx->data_meta; > my_meta->rxhash =3D meta->rxhash; > my_meta->tstamp =3D meta->tstamp; > return 0; > } /* etc */ > } > return -ENOENT; > } According to meta_btf_id. Do You have an idea how driver should fill this field?=20 hints->btf_id =3D 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. >=20 >=20 > -Toke >=20