From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BFE1EC43217 for ; Mon, 28 Nov 2022 22:26:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231278AbiK1W0Z (ORCPT ); Mon, 28 Nov 2022 17:26:25 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:32914 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232208AbiK1W0X (ORCPT ); Mon, 28 Nov 2022 17:26:23 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A63BA15A0C for ; Mon, 28 Nov 2022 14:25:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1669674328; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=esifUvS4rY25Js6P1Az3k3jokEJ6OjEKTAKMI9a6KbM=; b=QBLveBDSR0COGGlaG10rCLLi0xLFLG13dTJsOFNNifHT/ZGcabhrz4kn9zOfRytJyE4pmL xbz1A7Q1xKD2sRK+kXFoygaiC/28UrPMtLT3S3UmbAokGwTXKfLaIml7r5IDvMy+CeH8j9 ogRiNrpvxoU0QXEKztAr7uNBLUOG90g= Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-369-pCS1fu3jMHG2D054-LFibQ-1; Mon, 28 Nov 2022 17:25:27 -0500 X-MC-Unique: pCS1fu3jMHG2D054-LFibQ-1 Received: by mail-ej1-f71.google.com with SMTP id sd31-20020a1709076e1f00b007ae63b8d66aso5110674ejc.3 for ; Mon, 28 Nov 2022 14:25:26 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=esifUvS4rY25Js6P1Az3k3jokEJ6OjEKTAKMI9a6KbM=; b=C3old1NQlx3aU/kNpjMeQhlZSTZtKQ6IbGIsqrVgqGfzwdNaorudNACqaXBaTs5zpJ 4i5O3m3MfbN1uJvCQtN4gj5Pn/luudTmbJHWp9kq6StR0nB9MFPSaccbugvPV9t9JHIv FlyYvvx8NzWU0zabPLgnZc7NzuowbNFLGEUOIJ0UJXWlWx2NCvmhYx+eHNsd9BD4OyxD Vg67BT+mvI4IVeLP8aybEJnp2m5mvqRd2r6O2c/O0w1nItfRPJokulT9fORZHep5UyDK c2sCgknhyuguuNsfCSNDA95tT3c/QUKES1cYboDyLSdSY/Ppsq0d3vkEzVyTUdqFG4mz GBYw== X-Gm-Message-State: ANoB5pldZE4fAqAMeOxMIhqe+W5OIdOMknA8fY/zDm6JIY9yv4iPaful zKfIwmkmqxYCP6+mOtqF/MEScGPuOh6iJ4lp+dIdiB3enQfIwvr0/S58SHRrrg71rLXeDPEPGJQ bw4PISbEVLWZ9 X-Received: by 2002:a17:906:6892:b0:78d:ab48:bc84 with SMTP id n18-20020a170906689200b0078dab48bc84mr9183527ejr.22.1669674325028; Mon, 28 Nov 2022 14:25:25 -0800 (PST) X-Google-Smtp-Source: AA0mqf6OCJ/Pgejoyv/p1k+ImioFvemvLHuM7p17Mrwjj7a0VNSDVjrd/Xmy9cF9wmodsOZsxMEB0g== X-Received: by 2002:a17:906:6892:b0:78d:ab48:bc84 with SMTP id n18-20020a170906689200b0078dab48bc84mr9183466ejr.22.1669674323990; Mon, 28 Nov 2022 14:25:23 -0800 (PST) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id n17-20020a05640206d100b00459f4974128sm5581663edy.50.2022.11.28.14.25.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 28 Nov 2022 14:25:23 -0800 (PST) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 04F0D7EBEA3; Mon, 28 Nov 2022 23:25:23 +0100 (CET) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: Stanislav Fomichev Cc: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, martin.lau@linux.dev, song@kernel.org, yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org, haoluo@google.com, jolsa@kernel.org, David Ahern , Jakub Kicinski , Willem de Bruijn , Jesper Dangaard Brouer , Anatoly Burakov , Alexander Lobakin , Magnus Karlsson , Maryam Tahhan , xdp-hints@xdp-project.net, netdev@vger.kernel.org Subject: Re: [xdp-hints] [PATCH bpf-next v2 2/8] bpf: XDP metadata RX kfuncs In-Reply-To: References: <20221121182552.2152891-1-sdf@google.com> <20221121182552.2152891-3-sdf@google.com> <87mt8e2a69.fsf@toke.dk> X-Clacks-Overhead: GNU Terry Pratchett Date: Mon, 28 Nov 2022 23:25:22 +0100 Message-ID: <87y1ruzpgt.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org Stanislav Fomichev writes: > On Mon, Nov 28, 2022 at 10:53 AM Stanislav Fomichev wrot= e: >> >> s >> >> On Fri, Nov 25, 2022 at 9:53 AM Toke H=C3=B8iland-J=C3=B8rgensen wrote: >> > >> > Stanislav Fomichev writes: >> > >> > > There is an ndo handler per kfunc, the verifier replaces a call to t= he >> > > generic kfunc with a call to the per-device one. >> > > >> > > For XDP, we define a new kfunc set (xdp_metadata_kfunc_ids) which >> > > implements all possible metatada kfuncs. Not all devices have to >> > > implement them. If kfunc is not supported by the target device, >> > > the default implementation is called instead. >> > >> > BTW, this "the default implementation is called instead" bit is not >> > included in this version... :) >> >> fixup_xdp_kfunc_call should return 0 when the device doesn't have a >> kfunc defined and should fallback to the default kfunc implementation, >> right? >> Or am I missing something? >> >> > [...] >> > >> > > +#ifdef CONFIG_DEBUG_INFO_BTF >> > > +BTF_SET8_START(xdp_metadata_kfunc_ids) >> > > +#define XDP_METADATA_KFUNC(name, str) BTF_ID_FLAGS(func, str, 0) >> > > +XDP_METADATA_KFUNC_xxx >> > > +#undef XDP_METADATA_KFUNC >> > > +BTF_SET8_END(xdp_metadata_kfunc_ids) >> > > + >> > > +static const struct btf_kfunc_id_set xdp_metadata_kfunc_set =3D { >> > > + .owner =3D THIS_MODULE, >> > > + .set =3D &xdp_metadata_kfunc_ids, >> > > +}; >> > > + >> > > +u32 xdp_metadata_kfunc_id(int id) >> > > +{ >> > > + return xdp_metadata_kfunc_ids.pairs[id].id; >> > > +} >> > > +EXPORT_SYMBOL_GPL(xdp_metadata_kfunc_id); >> > >> > So I was getting some really weird values when testing (always getting= a >> > timestamp value of '1'), and it turns out to be because this way of >> > looking up the ID doesn't work: The set is always sorted by the BTF ID, >> > not the order it was defined. Which meant that the mapping code got the >> > functions mixed up, and would call a different one instead (so the >> > timestamp value I was getting was really the return value of >> > rx_hash_enabled()). >> > >> > I fixed it by building a secondary lookup table as below; feel free to >> > incorporate that (or if you can come up with a better way, go ahead!). >> >> Interesting, will take a closer look. I took this pattern from >> BTF_SOCK_TYPE_xxx, which means that 'sorting by btf-id' is something >> BTF_SET8_START specific... >> But if it's sorted, probably easier to do a bsearch over this table >> than to build another one? > > Ah, I see, there is no place to store an index :-( Maybe the following > is easier still? > > diff --git a/net/core/xdp.c b/net/core/xdp.c > index e43f7d4ef4cf..8240805bfdb7 100644 > --- a/net/core/xdp.c > +++ b/net/core/xdp.c > @@ -743,9 +743,15 @@ static const struct btf_kfunc_id_set > xdp_metadata_kfunc_set =3D { > .set =3D &xdp_metadata_kfunc_ids, > }; > > +BTF_ID_LIST(xdp_metadata_kfunc_ids_unsorted) > +#define XDP_METADATA_KFUNC(name, str) BTF_ID(func, str) > +XDP_METADATA_KFUNC_xxx > +#undef XDP_METADATA_KFUNC > + > u32 xdp_metadata_kfunc_id(int id) > { > - return xdp_metadata_kfunc_ids.pairs[id].id; > + /* xdp_metadata_kfunc_ids is sorted and can't be used */ > + return xdp_metadata_kfunc_ids_unsorted[id]; > } > EXPORT_SYMBOL_GPL(xdp_metadata_kfunc_id); Right, as long as having that extra list isn't problematic (does it make things show up twice somewhere or something like that? not really sure how that works), that is certainly simpler than what I came up with :) -Toke