From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-io1-xd2d.google.com (mail-io1-xd2d.google.com [IPv6:2607:f8b0:4864:20::d2d]) by mail.toke.dk (Postfix) with ESMTPS id 68DE79C95BC for ; Thu, 1 Dec 2022 18:14:23 +0100 (CET) Authentication-Results: mail.toke.dk; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20210112 header.b=gU/Yux4O Received: by mail-io1-xd2d.google.com with SMTP id h184so1327511iof.10 for ; Thu, 01 Dec 2022 09:14:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=fxCmfBah9ZMYfRaa3VChKHviqburNhg5/RwvVPix2pc=; b=gU/Yux4O9DYTa2mEHOvL6QQ1GsOU9kvJDJpVRneXLc+UrZWEe9lmn7TrLhQQEKwssr 5hk782/etlINnAftOyVnywjecVn9OLuRMzHBIfrcsIiIf5tLgFYKQrIgOYo6ksP+aqal cFqq1uvcrjlDSS6r0i8iwRHNhH+si8yhszvrOIvREBPmCRLiN/RIc+zDAZMbwQeEQUTm WSt88h777LdqwmpX0eJDNWCSd8lv169bUMnLPK4aa7lVqgiSp/hkHLKrtyUQXQ84w+DB Hyc9ErsEH9sf/xrk5wHwWY8hJ8sV7MByqgwEnfOMT4vLGcWTzukRMYBtfvb2SsLcCJxo FxLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=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=fxCmfBah9ZMYfRaa3VChKHviqburNhg5/RwvVPix2pc=; b=TNx+nPmAgrHQvryt0Hg4xKGTACwlhwi+eJnwXtnoZEkmNfpZeTnPlOgw5Id64MazWH BnGPacOWKRcDAFU+F0+/EXQR98a6q0hVZSTtWTvbmfjJ3rtxfbgTALd9TCJ1cw3CCFtU 3w/izuAWmxRD7LoszgxU6UeQMKdcJHSdnF0O9HqDyYMXWKS3t9m7XrmvJEMmvCNsyv0o u+ISzwP5P2JCFpVaQdeuB1zl0iNu4gClXToR8dCLW/KHJl/7cHyNK04eGFSOqJ+Waf8w rPdjwPWpMNhzD4QdTaxJqn24EeSedl4XRc95vUXeBviOQLPCfrdLi0WaWcDeEC0X+NMo vJgw== X-Gm-Message-State: ANoB5pkgY0ui9OonA7gP9omK9sGRxFmZ4PgylBP1GVSEz8Tulwq/c+4q /RPjDj8ni8E43lqM4sK3mB8Y24t4SlCtysgRjIRd/w== X-Google-Smtp-Source: AA0mqf7G1v4A40tgXr1NRHstLcTme21GLZJRJyVkeIBRJ8cOOHakVhk+7tqNHxac0XV9O/wu9t/1tPlmxunVkBo6PRs= X-Received: by 2002:a02:ccb5:0:b0:375:bdb9:f1e4 with SMTP id t21-20020a02ccb5000000b00375bdb9f1e4mr23543554jap.67.1669914860723; Thu, 01 Dec 2022 09:14:20 -0800 (PST) MIME-Version: 1.0 References: <20221121182552.2152891-1-sdf@google.com> <20221121182552.2152891-3-sdf@google.com> In-Reply-To: From: Stanislav Fomichev Date: Thu, 1 Dec 2022 09:14:04 -0800 Message-ID: To: Larysa Zaremba Content-Type: text/plain; charset="UTF-8" Message-ID-Hash: ALH5Z2I2HEZTHB6PNH3JG4ZTMPTWCZPD X-Message-ID-Hash: ALH5Z2I2HEZTHB6PNH3JG4ZTMPTWCZPD X-MailFrom: sdf@google.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@redhat.com, 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 X-Mailman-Version: 3.3.7 Precedence: list Subject: [xdp-hints] Re: [PATCH bpf-next v2 2/8] bpf: XDP metadata RX kfuncs List-Id: XDP hardware hints design discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu, Dec 1, 2022 at 6:08 AM Larysa Zaremba wrote: > > On Wed, Nov 30, 2022 at 12:17:39PM -0800, Stanislav Fomichev wrote: > > On Wed, Nov 30, 2022 at 11:06 AM Stanislav Fomichev wrote: > > > > > > On Wed, Nov 30, 2022 at 9:38 AM Larysa Zaremba wrote: > > > > > > > > On Mon, Nov 21, 2022 at 10:25:46AM -0800, Stanislav Fomichev wrote: > > > > > > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > > > > index 9528a066cfa5..315876fa9d30 100644 > > > > > --- a/kernel/bpf/verifier.c > > > > > +++ b/kernel/bpf/verifier.c > > > > > @@ -15171,6 +15171,25 @@ static int fixup_call_args(struct bpf_verifier_env *env) > > > > > return err; > > > > > } > > > > > > > > > > +static int fixup_xdp_kfunc_call(struct bpf_verifier_env *env, u32 func_id) > > > > > +{ > > > > > + struct bpf_prog_aux *aux = env->prog->aux; > > > > > + void *resolved = NULL; > > > > > > > > First I would like to say I really like the kfunc hints impementation. > > > > > > > > I am currently trying to test possible performace benefits of the unrolled > > > > version in the ice driver. I was working on top of the RFC v2, > > > > when I noticed a problem that also persists in this newer version. > > > > > > > > For debugging purposes, I have put the following logs in this place in code. > > > > > > > > printk(KERN_ERR "func_id=%u\n", func_id); > > > > printk(KERN_ERR "XDP_METADATA_KFUNC_RX_TIMESTAMP_SUPPORTED=%u\n", > > > > xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_TIMESTAMP_SUPPORTED)); > > > > printk(KERN_ERR "XDP_METADATA_KFUNC_RX_TIMESTAMP=%u\n", > > > > xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_TIMESTAMP)); > > > > printk(KERN_ERR "XDP_METADATA_KFUNC_RX_HASH_SUPPORTED=%u\n", > > > > xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_HASH_SUPPORTED)); > > > > printk(KERN_ERR "XDP_METADATA_KFUNC_RX_HASH=%u\n", > > > > xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_HASH)); > > > > > > > > Loading the program, which uses bpf_xdp_metadata_rx_timestamp_supported() > > > > and bpf_xdp_metadata_rx_timestamp(), has resulted in such messages: > > > > > > > > [ 412.611888] func_id=108131 > > > > [ 412.611891] XDP_METADATA_KFUNC_RX_TIMESTAMP_SUPPORTED=108126 > > > > [ 412.611892] XDP_METADATA_KFUNC_RX_TIMESTAMP=108128 > > > > [ 412.611892] XDP_METADATA_KFUNC_RX_HASH_SUPPORTED=108130 > > > > [ 412.611893] XDP_METADATA_KFUNC_RX_HASH=108131 > > > > [ 412.611894] func_id=108130 > > > > [ 412.611894] XDP_METADATA_KFUNC_RX_TIMESTAMP_SUPPORTED=108126 > > > > [ 412.611895] XDP_METADATA_KFUNC_RX_TIMESTAMP=108128 > > > > [ 412.611895] XDP_METADATA_KFUNC_RX_HASH_SUPPORTED=108130 > > > > [ 412.611895] XDP_METADATA_KFUNC_RX_HASH=108131 > > > > > > > > As you can see, I've got 108131 and 108130 IDs in program, > > > > while 108126 and 108128 would be more reasonable. > > > > It's hard to proceed with the implementation, when IDs cannot be sustainably > > > > compared. > > > > > > Thanks for the report! > > > Toke has reported a similar issue in [0], have you tried his patch? > > > I've also tried to address it in v3 [1], could you retry on top of it? > > > I'll try to insert your printk in my local build to see what happens > > > with btf ids on my side. Will get back to you.. > > > > > > 0: https://lore.kernel.org/bpf/87mt8e2a69.fsf@toke.dk/ > > > 1: https://lore.kernel.org/bpf/20221129193452.3448944-3-sdf@google.com/T/#u > > > > Nope, even if I go back to v2, I still can't reproduce locally. > > Somehow in my setup they are sorted properly :-/ > > Would appreciate it if you can test the v3 patch and confirm whether > > it's fixed on your side or not. > > > > I've tested v3 and it looks like the isssue was resolved. > Thanks a lot! Great, thank you for verifying! > > > > Furthermore, dumped vmlinux BTF shows the IDs is in the exactly reversed > > > > order: > > > > > > > > [108126] FUNC 'bpf_xdp_metadata_rx_hash' type_id=108125 linkage=static > > > > [108128] FUNC 'bpf_xdp_metadata_rx_hash_supported' type_id=108127 linkage=static > > > > [108130] FUNC 'bpf_xdp_metadata_rx_timestamp' type_id=108129 linkage=static > > > > [108131] FUNC 'bpf_xdp_metadata_rx_timestamp_supported' type_id=108127 linkage=static > > > > > > > > > + > > > > > + if (func_id == xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_TIMESTAMP_SUPPORTED)) > > > > > + resolved = aux->xdp_netdev->netdev_ops->ndo_xdp_rx_timestamp_supported; > > > > > + else if (func_id == xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_TIMESTAMP)) > > > > > + resolved = aux->xdp_netdev->netdev_ops->ndo_xdp_rx_timestamp; > > > > > + else if (func_id == xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_HASH_SUPPORTED)) > > > > > + resolved = aux->xdp_netdev->netdev_ops->ndo_xdp_rx_hash_supported; > > > > > + else if (func_id == xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_HASH)) > > > > > + resolved = aux->xdp_netdev->netdev_ops->ndo_xdp_rx_hash; > > > > > + > > > > > + if (resolved) > > > > > + return BPF_CALL_IMM(resolved); > > > > > + return 0; > > > > > +} > > > > > + > > > > > > > > My working tree (based on this version) is available on github [0]. Situation > > > > is also described in the last commit message. > > > > I would be great, if you could check, whether this behaviour can be reproduced > > > > on your setup. > > > > > > > > [0] https://github.com/walking-machine/linux/tree/hints-v2