From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qt1-x82e.google.com (mail-qt1-x82e.google.com [IPv6:2607:f8b0:4864:20::82e]) by mail.toke.dk (Postfix) with ESMTPS id 8A9D3A1B137 for ; Thu, 20 Jul 2023 15:55:18 +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=FVKeE7iI Received: by mail-qt1-x82e.google.com with SMTP id d75a77b69052e-403b30c7377so6667701cf.1 for ; Thu, 20 Jul 2023 06:55:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689861317; x=1690466117; 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=ZmQzphPp6MsTwBmccR9FSyIZcou9tRoCvSUS9C0hUzU=; b=FVKeE7iIzIcTqqTBE32N6Hi81S6a5PvH88h6tsACN+Itpd4kHLEnohSwcVcWpavPWR O0nqaKMsOA/DQl+xnCdmZ2fGSbBW9Tec823RoOwGS+9f7AuIJLj93yAYN5dthy1xZz6V bwrb+5JlF4lSGINVZYdL59H9oUiZb4uDE850YWdDGkmFyPN0M66QqSrqZThd6ftt51c9 42Q6ARjtsWB5AP9kDf9jdi4uTXyzjZANT69seQVfnBZNYVVc3gVV2jPcX9tcvgJWP3JB QpZMxUtjEKuNFi3HSsffAwWhHmncoMQDtfFi/mwox5EkROFb6PWg/xVb1N3bj5uA+Au4 q/7A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689861317; x=1690466117; 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=ZmQzphPp6MsTwBmccR9FSyIZcou9tRoCvSUS9C0hUzU=; b=OM2tTtFjFy7hn4MCqFzbzBMWPydkKlg9yFH0i35wmpfL5OS5latXe5ZRYjeooVXMwi Uqv6ulpwVbKWtFqYRTnHEyxYoUH4zkkJDy+gwSQHBGcLovsizD2OnTmFfg7DEbLkcnSq 28kAJZ00xN8iGLojMdrZd200vgNtXDVbGkaOjNjvWRO3rO+EEu5GTQNkaoJfw5e+XfBb JFAYgnJOp57vUsrEjutLxtm7owKGMim7X0z4xhIdSCI8OcG4did56BP7+ngM62ofL2e1 d0I3BsvN1N1/2HWuS6vcadbZcjD4idR7UCm/EiRHfqRZi0xKOh3ypwMVoov04XK9B9IL I8ow== X-Gm-Message-State: ABy/qLbVn4+xbO7QAVQq6+Cj7Mbf6Oqr7E8qmSsRN6PPZ47VU3iPzb4b YH47f5phTuOP5c2y7AOlzYM= X-Google-Smtp-Source: APBJJlHt8gW9/kRasKkWTHQdzcYBTRPNmDl9FEOLu6xE+ywV6+D7gwx0OeCSgxRgBJcOHOXFSFX/ow== X-Received: by 2002:ac8:5992:0:b0:405:4673:58e0 with SMTP id e18-20020ac85992000000b00405467358e0mr850583qte.63.1689861317098; Thu, 20 Jul 2023 06:55:17 -0700 (PDT) Received: from localhost (172.174.245.35.bc.googleusercontent.com. [35.245.174.172]) by smtp.gmail.com with ESMTPSA id n15-20020ac8674f000000b004033c3948f9sm326503qtp.42.2023.07.20.06.55.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Jul 2023 06:55:16 -0700 (PDT) Date: Thu, 20 Jul 2023 09:55:16 -0400 From: Willem de Bruijn To: "Zaremba, Larysa" , Willem de Bruijn Message-ID: <64b93cc46ad9b_2ad92129445@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> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Message-ID-Hash: 3RBRJZOASCHDWY2CGAL3IUPJC6HJQZIF X-Message-ID-Hash: 3RBRJZOASCHDWY2CGAL3IUPJC6HJQZIF 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: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.