From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-x730.google.com (mail-qk1-x730.google.com [IPv6:2607:f8b0:4864:20::730]) by mail.toke.dk (Postfix) with ESMTPS id 5F39DA1B368 for ; Fri, 21 Jul 2023 00:27:44 +0200 (CEST) Authentication-Results: mail.toke.dk; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20221208 header.b=pIEIzOoU Received: by mail-qk1-x730.google.com with SMTP id af79cd13be357-765942d497fso119563685a.1 for ; Thu, 20 Jul 2023 15:27:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689892062; x=1690496862; h=content-transfer-encoding:mime-version:subject:references :in-reply-to:message-id:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=wcKhesrVeKLtLXd1iNI5/TtHDFxZo4Ec1vI3ofFv2C0=; b=pIEIzOoUyAZJgvfrDsJxPf0zzWbvWiTx6sfMKz6BMJZLMc0Gp1SHbnrUbklDgEDuYs /QHI2sCEAfEgKCWIC/80/q/F3upISVmOFH1oYwjOB4IkLvvx7SUwECw3jho+UEuK8Amt DapPWqruHvFV+75Ol8LoggfTjXyVSeZmOZYo6AE3g32M+I8ZTB5w/YZjMl7jew+ZQIH8 WKeeTg6sDVgLhquaz/K6wA1Tm6OrdFH0QLOaPQsFKPQeof0yCfh4Wj/M3erHrXKCtB99 DyHC5zWp8yF8D2KQ49a+fCQIFIGa8taCen+/d2EON8xkKHwOjOu32Wl5vv6bwqr24dFP jpaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689892062; x=1690496862; h=content-transfer-encoding:mime-version:subject:references :in-reply-to:message-id:cc:to:from:date:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=wcKhesrVeKLtLXd1iNI5/TtHDFxZo4Ec1vI3ofFv2C0=; b=ioYg2qIS2yADQA9TniqOmQRnNCDPvTPNfQn2wlpuSdXAmw3D78ubIjVYlpQoSv9dL/ /82CWZ30Cipz8NL1YEKvySBXcePKtQbqAaSksHQq3biN5XDPQMV6f9B4bLOE9H6+DRvb efFmoy/ErvH1MhM5ptXHvs3UwvEEHFCtlev5euccnUySxW8ls4hNu3+bJHpxVmismXdl cNKdC2ppDx24IUFhwMHjKJXVp/NC8TnHbEKbzj5VaTz88n1raznedJ9atLN+mFU5eipo plXuAkQrOgrJ+06QL73QwlyUXTeta3AMqgyL/wz4cTJKviCJ6VDF8Afn50Eqz1m1Kw/t goqg== X-Gm-Message-State: ABy/qLbSsPoYwM9nSnK+NZUAZ24tnQ1FdUI4CPuOyWd5KuNXHkaOlXvs FUAm1Wgv6McRDVWLTgGkR9HMQXe1RQ8= X-Google-Smtp-Source: APBJJlEwJSrzsxt2v1eBTSo1BMVY1u4FA6mZQIZWPwmOe0ZeRV1ZQwKri/g15PxCSnRq8gm3z6kU2A== X-Received: by 2002:a05:620a:cc1:b0:765:a57a:16fe with SMTP id b1-20020a05620a0cc100b00765a57a16femr48397qkj.76.1689892062290; Thu, 20 Jul 2023 15:27:42 -0700 (PDT) Received: from localhost (172.174.245.35.bc.googleusercontent.com. [35.245.174.172]) by smtp.gmail.com with ESMTPSA id j18-20020a0cf512000000b0063c666dc802sm791422qvm.27.2023.07.20.15.27.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Jul 2023 15:27:41 -0700 (PDT) Date: Thu, 20 Jul 2023 18:27:41 -0400 From: Willem de Bruijn To: "Zaremba, Larysa" , Willem de Bruijn Message-ID: <64b9b4ddae4e7_2c3d502944a@willemb.c.googlers.com.notmuch> In-Reply-To: References: <20230719183734.21681-1-larysa.zaremba@intel.com> <20230719183734.21681-13-larysa.zaremba@intel.com> <64b858ac9edd3_2849c129476@willemb.c.googlers.com.notmuch> <64b93cc46ad9b_2ad92129445@willemb.c.googlers.com.notmuch> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Message-ID-Hash: SMBYXGIWVATDPBRGBSVB6PHI74TDR6PO X-Message-ID-Hash: SMBYXGIWVATDPBRGBSVB6PHI74TDR6PO X-MailFrom: willemdebruijn.kernel@gmail.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: "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" , "sdf@google.com" , "haoluo@google.com" , "jolsa@kernel.org" , David Ahern , Jakub Kicinski , Willem de Bruijn , "Brouer, Jesper" , "Burakov, Anatoly" , "Lobakin, Aleksander" , Magnus Karlsson , "Tahhan, Maryam" , "xdp-hints@xdp-project.net" , "netdev@vger.kernel.org" X-Mailman-Version: 3.3.8 Precedence: list Subject: [xdp-hints] Re: [PATCH bpf-next v3 12/21] xdp: Add checksum hint List-Id: XDP hardware hints design discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Zaremba, Larysa wrote: > On Thu, Jul 20, 2023 at 09:55:16AM -0400, Willem de Bruijn wrote: > > Zaremba, Larysa wrote: > > > On Thu, Jul 20, 2023 at 09:57:05AM +0000, Zaremba, Larysa wrote: > > > > On Wed, Jul 19, 2023 at 05:42:04PM -0400, Willem de Bruijn wrote: > > > > > Larysa Zaremba wrote: > > > > > > Implement functionality that enables drivers to expose to XDP code checksum > > > > > > information that consists of: > > > > > > > > > > > > - Checksum status - bitfield that consists of > > > > > > - number of consecutive validated checksums. This is almost the same as > > > > > > csum_level in skb, but starts with 1. Enum names for those bits still > > > > > > use checksum level concept, so it is less confusing for driver > > > > > > developers. > > > > > > - Is checksum partial? This bit cannot coexist with any other > > > > > > - Is there a complete checksum available? > > > > > > - Additional checksum data, a union of: > > > > > > - checksum start and offset, if checksum is partial > > > > > > - complete checksum, if available > > > > > > > > > > > > Signed-off-by: Larysa Zaremba > > > > > > --- > > > > > > Documentation/networking/xdp-rx-metadata.rst | 3 ++ > > > > > > include/linux/netdevice.h | 3 ++ > > > > > > include/net/xdp.h | 46 ++++++++++++++++++++ > > > > > > kernel/bpf/offload.c | 2 + > > > > > > net/core/xdp.c | 23 ++++++++++ > > > > > > 5 files changed, 77 insertions(+) > > > > > > > > > > > > diff --git a/Documentation/networking/xdp-rx-metadata.rst b/Documentation/networking/xdp-rx-metadata.rst > > > > > > index ea6dd79a21d3..7f056a44f682 100644 > > > > > > --- a/Documentation/networking/xdp-rx-metadata.rst > > > > > > +++ b/Documentation/networking/xdp-rx-metadata.rst > > > > > > @@ -26,6 +26,9 @@ metadata is supported, this set will grow: > > > > > > .. kernel-doc:: net/core/xdp.c > > > > > > :identifiers: bpf_xdp_metadata_rx_vlan_tag > > > > > > > > > > > > +.. kernel-doc:: net/core/xdp.c > > > > > > + :identifiers: bpf_xdp_metadata_rx_csum > > > > > > + > > > > > > An XDP program can use these kfuncs to read the metadata into stack > > > > > > variables for its own consumption. Or, to pass the metadata on to other > > > > > > consumers, an XDP program can store it into the metadata area carried > > > > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > > > > > > index 1749f4f75c64..4f6da36ac123 100644 > > > > > > --- a/include/linux/netdevice.h > > > > > > +++ b/include/linux/netdevice.h > > > > > > @@ -1660,6 +1660,9 @@ struct xdp_metadata_ops { > > > > > > enum xdp_rss_hash_type *rss_type); > > > > > > int (*xmo_rx_vlan_tag)(const struct xdp_md *ctx, u16 *vlan_tci, > > > > > > __be16 *vlan_proto); > > > > > > + int (*xmo_rx_csum)(const struct xdp_md *ctx, > > > > > > + enum xdp_csum_status *csum_status, > > > > > > + union xdp_csum_info *csum_info); > > > > > > }; > > > > > > > > > > > > /** > > > > > > diff --git a/include/net/xdp.h b/include/net/xdp.h > > > > > > index 89c58f56ffc6..2b7a7d678ff4 100644 > > > > > > --- a/include/net/xdp.h > > > > > > +++ b/include/net/xdp.h > > > > > > @@ -391,6 +391,8 @@ void xdp_attachment_setup(struct xdp_attachment_info *info, > > > > > > bpf_xdp_metadata_rx_hash) \ > > > > > > XDP_METADATA_KFUNC(XDP_METADATA_KFUNC_RX_VLAN_TAG, \ > > > > > > bpf_xdp_metadata_rx_vlan_tag) \ > > > > > > + XDP_METADATA_KFUNC(XDP_METADATA_KFUNC_RX_CSUM, \ > > > > > > + bpf_xdp_metadata_rx_csum) \ > > > > > > > > > > > > enum { > > > > > > #define XDP_METADATA_KFUNC(name, _) name, > > > > > > @@ -448,6 +450,50 @@ enum xdp_rss_hash_type { > > > > > > XDP_RSS_TYPE_L4_IPV6_SCTP_EX = XDP_RSS_TYPE_L4_IPV6_SCTP | XDP_RSS_L3_DYNHDR, > > > > > > }; > > > > > > > > > > > > +union xdp_csum_info { > > > > > > + /* Checksum referred to by ``csum_start + csum_offset`` is considered > > > > > > + * valid, but was never calculated, TX device has to do this, > > > > > > + * starting from csum_start packet byte. > > > > > > + * Any preceding checksums are also considered valid. > > > > > > + * Available, if ``status == XDP_CHECKSUM_PARTIAL``. > > > > > > + */ > > > > > > + struct { > > > > > > + u16 csum_start; > > > > > > + u16 csum_offset; > > > > > > + }; > > > > > > + > > > > > > + /* Checksum, calculated over the whole packet. > > > > > > + * Available, if ``status & XDP_CHECKSUM_COMPLETE``. > > > > > > + */ > > > > > > + u32 checksum; > > > > > > +}; > > > > > > + > > > > > > +enum xdp_csum_status { > > > > > > + /* HW had parsed several transport headers and validated their > > > > > > + * checksums, same as ``CHECKSUM_UNNECESSARY`` in ``sk_buff``. > > > > > > + * 3 least significat bytes contain number of consecutive checksum, > > > > > > > > > > typo: significant > > > > > > > > > > (I did not scan for typos, just came across this when trying to understand > > > > > the skb->csum_level + 1 trick. Probably good to run a spell check). > > > > > > > > > > > Oh, and about skb->csum_level + 1, maybe this way it would be more > > > understandable: XDP_CHECKSUM_VALID_LVL0 + skb->csum_level? > > > > Agreed, that would help document the intent. > > > > > Using number of valid checksums (starts with 1) instead of checksum level > > > (starts with 0) is a debatable decision, but I have decided to go with it under > > > 2 assumptions: > > > > > > - the only reason checksum level in skb starts with 0 is to use less bits > > > - checksum number would be more intuitive for XDP/AF_XDP application developers > > > > > > I encourage everyone to share their opinion on that. > > > > I assumed this offset by one was because csum_status zero implicitly > > meant XDP_CHECKSUM_NONE. Is that not correct? That should probably > > get an explicit name. > > > > Well, I was not sure, whether I should add XDP_CHECKSUM_NONE, because it would > be equal to returning -ENODATA from kfunc, but after giving it some thought now, > it is worth to have XDP_CHECKSUM_NONE for packets that have no checksum to > check, like for hash there is XDP_RSS_TYPE_L2. On receive, CHECKSUM_NONE means that the packet has not been checked, not necessarily that it has no checksum. Perhaps the device was unable to parse the protocol. (on transmit, it conveys that a transmit checksum is not required.)