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 483C38B1A4B for ; Tue, 7 Sep 2021 12:21:39 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10099"; a="218295234" X-IronPort-AV: E=Sophos;i="5.85,274,1624345200"; d="scan'208";a="218295234" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Sep 2021 03:21:34 -0700 X-IronPort-AV: E=Sophos;i="5.85,274,1624345200"; d="scan'208";a="478621656" Received: from unknown (HELO localhost.localdomain) ([10.102.102.63]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Sep 2021 03:21:31 -0700 Date: Tue, 7 Sep 2021 02:27:29 -0400 From: Michal Swiatkowski To: Jesper Dangaard Brouer Subject: Re: XDP-hints: Howto support multiple BTF types per packet basis? Message-ID: References: <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> <190d8d21-f11d-bb83-58aa-08e86e0006d9@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <190d8d21-f11d-bb83-58aa-08e86e0006d9@redhat.com> Message-ID-Hash: 7UFHVLMYTZVGZQ6Z3WMOGVLORURTXGUM X-Message-ID-Hash: 7UFHVLMYTZVGZQ6Z3WMOGVLORURTXGUM 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: Toke =?iso-8859-1?Q?H=F8iland-J=F8rgensen?= , brouer@redhat.com, Jakub Kicinski , John Fastabend , Andrii Nakryiko , BPF-dev-list , Magnus Karlsson , William Tu , xdp-hints@xdp-project.net, Zaremba Larysa , Jiri Olsa 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 Thu, Sep 02, 2021 at 11:17:43AM +0200, Jesper Dangaard Brouer wrote: >=20 >=20 > On 02/09/2021 04.49, Michal Swiatkowski wrote: > > On Fri, Jul 09, 2021 at 12:57:08PM +0200, Toke H=F8iland-J=F8rgensen wr= ote: > > > Michal Swiatkowski writes: > > >=20 > > > > > 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 me= tadata > > > > > 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 en= d 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 las= t) in meta area */ > > > > > __u32 *meta_btf_id =3D ctx->data_meta; > > > > > 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 struc= ts, > > > > > even removing the code for the ones that don't exist on the runni= ng > > > > > kernel. > > > >=20 > > > > This looks nice. In this case we need defintions of struct meta_mlx= 5 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 defini= tion > > > > 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 inclu= des > > > 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 f= rom > > > > 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 li= bbpf > > > > and proposed creating something like mega xdp hints structure with = all > > > > available fields across all drivers. As I understand this could sol= ve > > > > 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 n= ame > > > > 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 > > > >=20 > > > > When there will be only one driver loaded should libbpf do correct > > > > reallocation of fields? What will happen when both of the drivers a= re > > > > 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 co= de > > > in vmlinux.h and the bpftool skeletons. So maybe we could have a comm= and > > > 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 *me= ta) > > > { > > > 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; > > > 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; > > > } > >=20 > > According to meta_btf_id. >=20 > In BPF-prog (that gets loaded by libbpf), the BTF_ID_MLX5 and BTF_ID_I40E > should be replaced by bpf_core_type_id_kernel() calls. >=20 > I have a code example here[1], where I use the triple-underscore to lookup > btf_id =3D bpf_core_type_id_kernel(struct sk_buff___local). >=20 > AFAIK (Andrii correctly me if I'm wrong) It is libbpf that does the bpf_id > lookup before loading the BPF-prog into the kernel. >=20 > For AF_XDP we need to code our own similar lookup of the btf_id. (In that > process I imagine that userspace tools could/would read the BTF member > offsets and check it against what they expected). >=20 >=20 > [1] https://github.com/xdp-project/bpf-examples/blob/master/ktrace-CO-RE= /ktrace01_kern.c#L34-L57 >=20 Thanks a lot. I tested Your CO-RE example. For defines that are located in vmlinux everything works fine (like for sk_buff). When I tried to get btf id of structures defined in module (loaded module, structure can be find in /sys/kerne/btf/module_name) I always get 0 (not found). Do You know if bpf_core_type_id_kernel() should also support modules btf? Based on: [1] https://lore.kernel.org/bpf/20201110011932.3201430-5-andrii@kernel.org/ I assume that modules btfs also are marked as in-kernel, but I can't make it works with bpf_core_type_id_kernel(). My clang version is 12.0.1, so changes needed by modules btf should be there [2] https://reviews.llvm.org/D91489 > > Do You have an idea how driver should fill this field? >=20 > (Andrii please correctly me as this is likely wrong:) > I imagine that driver will have a pointer to a 'struct btf' object and the > btf_id can be read via btf_obj_id() (that just return btf->id). > As this also allows driver to take refcnt on the btf-object. > Much like Ederson did in [2]. >=20 > Maybe I misunderstood the use of the 'struct btf' object ? >=20 > Maybe it is the wrong approach? As the patchset[2] exports btf_obj_id() a= nd > introduced helper functions that can register/unregister btf objects[3], > which I sense might not be needed today, as modules can get their own BTF > info automatically today. > Maybe this (btf->id) is not the ID we are looking for? >=20 > [2] https://lore.kernel.org/all/20210803010331.39453-11-ederson.desouza@i= ntel.com/ > [3] > https://lore.kernel.org/all/20210803010331.39453-2-ederson.desouza@intel.= com/ >=20 As 'struct btf' object do You mean one module btf with all definitions or specific structure btf object? In case of Your example [1]. If in driver side there will be call to get btf id of sk_buff: id =3D btf_find_by_name_kind(vmlinux_btf, "sk_buff", BTF_KIND_STRUCT); this id will be the same as id from Your ktrace01 program. I think this is id that we are looking for. > > hints->btf_id =3D btf_id_by_name("struct meta_i40e"); /* fill btf id at > > config time */ >=20 > Yes, at config time the btf_id can change (and maybe we want to cache the > btf_obj_id() lookup to avoid a function call). >=20 > > 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 > There is a function named: btf_find_by_name_kind() > Thanks, this is what I needed. > --Jesper > 1