From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 54B31A0351; Fri, 4 Mar 2022 12:54:41 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 33F9F427A9; Fri, 4 Mar 2022 12:54:41 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 59E594013F for ; Fri, 4 Mar 2022 12:54:40 +0100 (CET) X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [RFC] ethdev: introduce protocol type based header split Date: Fri, 4 Mar 2022 12:54:38 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D86F22@smartserver.smartshare.dk> In-Reply-To: X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [RFC] ethdev: introduce protocol type based header split Thread-Index: AQHYLsRaim7ezLmaXEqNPKPgSYjBOqytUDIAgAGouzCAACPD0A== References: <20220303060136.36427-1-xuan.ding@intel.com> <20220303081539.1de139f9@hermes.local> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Zhang, Qi Z" , "Stephen Hemminger" , "Ding, Xuan" Cc: , "Yigit, Ferruh" , , , , "Yu, Ping" , "Wang, YuanX" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > From: Zhang, Qi Z [mailto:qi.z.zhang@intel.com] > Sent: Friday, 4 March 2022 10.58 >=20 > > From: Stephen Hemminger > > Sent: Friday, March 4, 2022 12:16 AM > > > > On Thu, 3 Mar 2022 06:01:36 +0000 > > xuan.ding@intel.com wrote: > > > > > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h > index > > > c2d1f9a972..6743648c22 100644 > > > --- a/lib/ethdev/rte_ethdev.h > > > +++ b/lib/ethdev/rte_ethdev.h > > > @@ -1202,7 +1202,8 @@ struct rte_eth_rxseg_split { > > > struct rte_mempool *mp; /**< Memory pool to allocate segment > from. > > */ > > > uint16_t length; /**< Segment data length, configures split > point. */ > > > uint16_t offset; /**< Data offset from beginning of mbuf data > buffer. */ > > > - uint32_t reserved; /**< Reserved field. */ > > > + uint16_t proto; > > > + uint16_t reserved; /**< Reserved field. */ > > > }; > > > > This feature suffers from a common bad design pattern. > > You can't just start using reserved fields unless the previous > versions enforced > > that the field was a particular value (usually zero). >=20 > Yes, agree, that's a mistake there is no document for the reserved > field in the previous release, and usually, it should be zero, > And I think one of the typical purposes of the reserved field is to > make life easy for new feature adding without breaking ABI. > So, should we just take the risk as I guess it might not be a big deal > in real cases? >=20 In this specific case, I think it can be done with very low risk in real = cases. Assuming that splitting based on fixed length and protocol header = parsing is mutually exclusive, the PMDs can simply ignore the "proto" = field (and log a warning about it) if the length field is non-zero. This = will provide backwards compatibility with applications not zeroing out = the 32 bit "reserved" field. > Thanks > Qi >=20 >=20 >=20 > > > > There is no guarantee that application will initialize these = reserved > fields and > > now using them risks breaking the API/ABI. It looks like > > > > rte_eth_rx_queue_check_split(const struct rte_eth_rxseg_split > *rx_seg, > > > > Would have had to check in previous release. > > > > This probably has to wait until 22.11 next API release.