From nobody Fri Nov 05 05:08:30 2021 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id A514B18383C2; Fri, 5 Nov 2021 05:08:44 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4HlpSX345Hz3rCR; Fri, 5 Nov 2021 05:08:44 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.16.1/8.16.1) with ESMTPS id 1A558Upt010453 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Fri, 5 Nov 2021 07:08:33 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua 1A558Upt010453 Received: (from kostik@localhost) by tom.home (8.16.1/8.16.1/Submit) id 1A558UU1010452; Fri, 5 Nov 2021 07:08:30 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 5 Nov 2021 07:08:30 +0200 From: Konstantin Belousov To: Hans Petter Selasky Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 10a62eb109ce - main - Use layer five checksum flags in the mbuf packet header to pass on crypto state. Message-ID: References: <202111041754.1A4HsVkY050121@gitrepo.freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <202111041754.1A4HsVkY050121@gitrepo.freebsd.org> X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.5 X-Spam-Checker-Version: SpamAssassin 3.4.5 (2021-03-20) on tom.home X-Rspamd-Queue-Id: 4HlpSX345Hz3rCR X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-ThisMailContainsUnwantedMimeParts: N On Thu, Nov 04, 2021 at 05:54:31PM +0000, Hans Petter Selasky wrote: > The branch main has been updated by hselasky: > > URL: https://cgit.FreeBSD.org/src/commit/?id=10a62eb109ceafce32aa2b18ec835b3b7285c2dd > > commit 10a62eb109ceafce32aa2b18ec835b3b7285c2dd > Author: Hans Petter Selasky > AuthorDate: 2021-11-04 17:43:24 +0000 > Commit: Hans Petter Selasky > CommitDate: 2021-11-04 17:52:06 +0000 > > Use layer five checksum flags in the mbuf packet header to pass on crypto state. > > The mbuf protocol flags get cleared between layers, and also it was discovered > that M_DECRYPTED conflicts with M_HASFCS when receiving ethernet patckets. > > Add the proper CSUM_TLS_MASK and CSUM_TLS_DECRYPTED defines, and start using > these instead of M_DECRYPTED inside the TCP LRO code. > > This change is needed by coming TLS RX hardware offload support patches. > > Suggested by: kib@ > Reviewed by: jhb@ > MFC after: 1 week > Sponsored by: NVIDIA Networking > --- > sys/netinet/tcp_lro.c | 11 ++++++++++- > sys/sys/mbuf.h | 2 ++ > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/sys/netinet/tcp_lro.c b/sys/netinet/tcp_lro.c > index cb9681559777..ea23ad75994d 100644 > --- a/sys/netinet/tcp_lro.c > +++ b/sys/netinet/tcp_lro.c > @@ -395,7 +395,8 @@ tcp_lro_parser(struct mbuf *m, struct lro_parser *po, struct lro_parser *pi, boo > htons(m->m_pkthdr.ether_vtag) & htons(EVL_VLID_MASK); > } > /* Store decrypted flag, if any. */ > - if (__predict_false(m->m_flags & M_DECRYPTED)) > + if (__predict_false((m->m_pkthdr.csum_flags & > + CSUM_TLS_MASK) == CSUM_TLS_DECRYPTED)) > po->data.lro_flags |= LRO_FLAG_DECRYPTED; > } > > @@ -833,6 +834,8 @@ tcp_flush_out_entry(struct lro_ctrl *lc, struct lro_entry *le) > le->m_head->m_pkthdr.csum_flags = CSUM_DATA_VALID | > CSUM_PSEUDO_HDR | CSUM_IP_CHECKED | CSUM_IP_VALID; > le->m_head->m_pkthdr.csum_data = 0xffff; > + if (__predict_false(le->outer.data.lro_flags & LRO_FLAG_DECRYPTED)) > + le->m_head->m_pkthdr.csum_flags |= CSUM_TLS_DECRYPTED; > break; > case LRO_TYPE_IPV6_TCP: > csum = tcp_lro_update_checksum(&le->inner, le, > @@ -844,6 +847,8 @@ tcp_flush_out_entry(struct lro_ctrl *lc, struct lro_entry *le) > le->m_head->m_pkthdr.csum_flags = CSUM_DATA_VALID | > CSUM_PSEUDO_HDR; > le->m_head->m_pkthdr.csum_data = 0xffff; > + if (__predict_false(le->outer.data.lro_flags & LRO_FLAG_DECRYPTED)) > + le->m_head->m_pkthdr.csum_flags |= CSUM_TLS_DECRYPTED; > break; > case LRO_TYPE_NONE: > switch (le->outer.data.lro_type) { > @@ -854,6 +859,8 @@ tcp_flush_out_entry(struct lro_ctrl *lc, struct lro_entry *le) > le->m_head->m_pkthdr.csum_flags = CSUM_DATA_VALID | > CSUM_PSEUDO_HDR | CSUM_IP_CHECKED | CSUM_IP_VALID; > le->m_head->m_pkthdr.csum_data = 0xffff; > + if (__predict_false(le->outer.data.lro_flags & LRO_FLAG_DECRYPTED)) > + le->m_head->m_pkthdr.csum_flags |= CSUM_TLS_DECRYPTED; > break; > case LRO_TYPE_IPV6_TCP: > csum = tcp_lro_update_checksum(&le->outer, le, > @@ -862,6 +869,8 @@ tcp_flush_out_entry(struct lro_ctrl *lc, struct lro_entry *le) > le->m_head->m_pkthdr.csum_flags = CSUM_DATA_VALID | > CSUM_PSEUDO_HDR; > le->m_head->m_pkthdr.csum_data = 0xffff; > + if (__predict_false(le->outer.data.lro_flags & LRO_FLAG_DECRYPTED)) > + le->m_head->m_pkthdr.csum_flags |= CSUM_TLS_DECRYPTED; > break; > default: > break; > diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h > index 413854cc9a57..d0f90805fa78 100644 > --- a/sys/sys/mbuf.h > +++ b/sys/sys/mbuf.h > @@ -721,6 +721,8 @@ m_epg_pagelen(const struct mbuf *m, int pidx, int pgoff) > #define CSUM_UDP_IPV6 CSUM_IP6_UDP > #define CSUM_TCP_IPV6 CSUM_IP6_TCP > #define CSUM_SCTP_IPV6 CSUM_IP6_SCTP > +#define CSUM_TLS_MASK (CSUM_L5_CALC|CSUM_L5_VALID) > +#define CSUM_TLS_DECRYPTED CSUM_L5_CALC It is worth explaining in a comment that selection of the value for CSUM_TLS_DECRYPTED does not reuse previous value, in the sense that correct use of the CSUM_L5 flags from drivers (are there any?) is still correct. We usurp a value for L5 CSUM flags which should not be returned from driver, to mean something new. It also may be worth discussing do we need L5 flags in its present form, and are there any existing consumers that do not abuse there semi-free flags for something else. If no external consumers exist (*) then perhaps officially re-purposing them is better route. We could define them as something extensible or at least context (inpcb etc)-dependent. (*) it seems that there is no in-tree users > > /* > * mbuf types describing the content of the mbuf (including external storage).