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 AED8AC4332F for ; Fri, 9 Dec 2022 17:46:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229675AbiLIRqf (ORCPT ); Fri, 9 Dec 2022 12:46:35 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56468 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229628AbiLIRqe (ORCPT ); Fri, 9 Dec 2022 12:46:34 -0500 Received: from mail-pj1-x1036.google.com (mail-pj1-x1036.google.com [IPv6:2607:f8b0:4864:20::1036]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7FB061D64A for ; Fri, 9 Dec 2022 09:46:33 -0800 (PST) Received: by mail-pj1-x1036.google.com with SMTP id t11-20020a17090a024b00b0021932afece4so8868296pje.5 for ; Fri, 09 Dec 2022 09:46:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=HQ9dSEXwYBxA4pKSv/9YF2c2JcAm8IxKG1zQ6cKX/dc=; b=dYXP6fDzQN2V/qlc3SQr8N0jvIQ2duJ/fjb7ifX4vUlGuJLJ2+SspX1BrypjxiLwX9 3CJ8ivbUM/t5qTU7Jm8mepJlGzlcVOMEzoJI2fgIapmCEuqj74it1aGl4jKTaMj4zQKU hXWqk1uuV+7fgfEeC4+7GWzoEF4MCI1sAJ/81KLLaFv5di9kdCBAYoZDdXkKwMyq7HB9 oawG4p9VDqLGaGRka74ejvh9F+J3E+rNL7Hdp8ZqBwXD6+uJEbFSLoRW8U9ZM6WYIwCF 4Y2JcN0wsDF5tCIHVO3mlnck3Kp4VlAvyQnQKn/VTDrU41nwROvzrhD0xnov5LTOUDzo RcxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=HQ9dSEXwYBxA4pKSv/9YF2c2JcAm8IxKG1zQ6cKX/dc=; b=4EGlv26IcvzSIGaitsTmgqiLCLN7Sw1E47nFPpuA7gpd/qmZtRGmzSpP+qij79ytD9 HlDr+kRxuewvRdHVxpNTefEje/Hp0oDZ39tg3jz24liTH2CFi+EVIoockHhsVm0NtRQY HHH+XRIHDek3anvKyDQIYkeL8SOU8q6HzNd5RaNuCOAAxw0/O1KEO4qemEOgBRRwwVIL 7fjTzwJLajm8UUITdrN0Lk60DhMaXwOJnjdXZQHfNWzS4SJACMqz4b4Hbzun961lA0i7 a/aY4xi9nqZf4AO0R16O7gh8Zx+NzLHVZLs2UFJgmLRX+VmEArGktBnlvGNf+PpyGMkc tasQ== X-Gm-Message-State: ANoB5pmt+tcwN0nuo8L94/gfp7N7Nd1PSyycI0C59DqZvNtNXkw7/+S0 Halm1OWctHSxnZBnFvdoUlVRJsMgO2guUvmzZJaRAA== X-Google-Smtp-Source: AA0mqf6J7OlXOVrlXIrOK83k51H3KpK5FYzmh8bzgATvgK+PzcS10dPMUBOH9FWm1W7UuPK/JhLJi793PsvGKg1gLfo= X-Received: by 2002:a17:90a:6382:b0:219:fbc:a088 with SMTP id f2-20020a17090a638200b002190fbca088mr65443415pjj.162.1670607992729; Fri, 09 Dec 2022 09:46:32 -0800 (PST) MIME-Version: 1.0 References: <20221206024554.3826186-1-sdf@google.com> <20221206024554.3826186-12-sdf@google.com> <875yellcx6.fsf@toke.dk> <87359pl9zy.fsf@toke.dk> <87tu25ju77.fsf@toke.dk> <87o7sdjt20.fsf@toke.dk> <87cz8sk59e.fsf@toke.dk> <20221209084524.01c09d9c@kernel.org> In-Reply-To: <20221209084524.01c09d9c@kernel.org> From: Stanislav Fomichev Date: Fri, 9 Dec 2022 09:46:20 -0800 Message-ID: Subject: Re: [xdp-hints] Re: [PATCH bpf-next v3 11/12] mlx5: Support RX XDP metadata To: Jakub Kicinski Cc: =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= , Alexei Starovoitov , bpf , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , Hao Luo , Jiri Olsa , Saeed Mahameed , David Ahern , Willem de Bruijn , Jesper Dangaard Brouer , Anatoly Burakov , Alexander Lobakin , Magnus Karlsson , Maryam Tahhan , xdp-hints@xdp-project.net, Network Development Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On Fri, Dec 9, 2022 at 8:45 AM Jakub Kicinski wrote: > > On Fri, 09 Dec 2022 15:42:37 +0100 Toke H=C3=B8iland-J=C3=B8rgensen wrote= : > > If we expect the program to do out of band probing, we could just get > > rid of the _supported() functions entirely? > > > > I mean, to me, the whole point of having the separate _supported() > > function for each item was to have a lower-overhead way of checking if > > the metadata item was supported. But if the overhead is not actually > > lower (because both incur a function call), why have them at all? Then > > we could just change the implementation from this: > > > > bool mlx5e_xdp_rx_hash_supported(const struct xdp_md *ctx) > > { > > const struct mlx5_xdp_buff *_ctx =3D (void *)ctx; > > > > return _ctx->xdp.rxq->dev->features & NETIF_F_RXHASH; > > } > > > > u32 mlx5e_xdp_rx_hash(const struct xdp_md *ctx) > > { > > const struct mlx5_xdp_buff *_ctx =3D (void *)ctx; > > > > return be32_to_cpu(_ctx->cqe->rss_hash_result); > > } > > > > to this: > > > > u32 mlx5e_xdp_rx_hash(const struct xdp_md *ctx) > > { > > const struct mlx5_xdp_buff *_ctx =3D (void *)ctx; > > > > if (!(_ctx->xdp.rxq->dev->features & NETIF_F_RXHASH)) > > return 0; > > > > return be32_to_cpu(_ctx->cqe->rss_hash_result); > > } > > Are there no corner cases? E.g. in case of an L2 frame you'd then > expect a hash of 0? Rather than no hash? > > If I understand we went for the _supported() thing to make inlining > the check easier than inlining the actual read of the field. > But we're told inlining is a bit of a wait.. so isn't the motivation > for the _supported() pretty much gone? And we should we go back to > returning an error from the actual read? Seems fair, we can always bring those _supported() calls back in the future when the inlining is available and having those separate calls shows clear benefit. Then let's go back to a more conventional form below? int mlx5e_xdp_rx_hash(const struct xdp_md *ctx, u32 *timestamp) { const struct mlx5_xdp_buff *_ctx =3D (void *)ctx; if (!(_ctx->xdp.rxq->dev->features & NETIF_F_RXHASH)) return -EOPNOTSUPP; *timestamp =3D be32_to_cpu(_ctx->cqe->rss_hash_result); return 0; } > Is partial inlining hard? (inline just the check and generate a full > call for the read, ending up with the same code as with _supported()) I'm assuming you're suggesting to do this partial inlining manually (as in, writing the code to output this bytecode)? This probably also falls into the "manual bpf asm generation tech debt" buc= ket? LMK if I missed your point.