From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id E6D8DA00E6 for ; Thu, 11 Jul 2019 10:04:07 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D6A68397D; Thu, 11 Jul 2019 10:04:06 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 9B2EA3772 for ; Thu, 11 Jul 2019 10:04:04 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jul 2019 01:04:03 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.63,476,1557212400"; d="scan'208";a="168558801" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga003.jf.intel.com with ESMTP; 11 Jul 2019 01:04:03 -0700 Received: from fmsmsx117.amr.corp.intel.com (10.18.116.17) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 11 Jul 2019 01:04:03 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by fmsmsx117.amr.corp.intel.com (10.18.116.17) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 11 Jul 2019 01:04:02 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.134]) by shsmsx102.ccr.corp.intel.com ([169.254.2.3]) with mapi id 14.03.0439.000; Thu, 11 Jul 2019 16:04:00 +0800 From: "Wang, Haiyue" To: Olivier Matz CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [RFC] mbuf: support dynamic fields and flags Thread-Index: AQHVNwH9rBZt164hek6OOjYJj0DjD6bEEJBwgABvLYCAAIwSIA== Date: Thu, 11 Jul 2019 08:04:00 +0000 Message-ID: References: <20190710092907.5565-1-olivier.matz@6wind.com> <20190711072619.fldr3dz7qblyvej5@glumotte.dev.6wind.com> In-Reply-To: <20190711072619.fldr3dz7qblyvej5@glumotte.dev.6wind.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTNkYzNhMDItMmU2Yi00NTZlLThlMjEtMjUzODVhYWEzNzMyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoibDhhcUdyNVRrWVZBM2lkSUV4ZytwRk9CajlTcWZlQUc5ck9OMlB6MkZkV29lcXBMQW9GQ2FoMGIwNGtaWWM2UCJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC] mbuf: support dynamic fields and flags X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Olivier Matz [mailto:olivier.matz@6wind.com] > Sent: Thursday, July 11, 2019 15:26 > To: Wang, Haiyue > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [RFC] mbuf: support dynamic fields and flags >=20 > Hi, >=20 > On Wed, Jul 10, 2019 at 05:14:33PM +0000, Wang, Haiyue wrote: > > Hi, > > > > Sounds cool, just have some questions inline. > > > > > -----Original Message----- > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz > > > Sent: Wednesday, July 10, 2019 17:29 > > > To: dev@dpdk.org > > > Subject: [dpdk-dev] [RFC] mbuf: support dynamic fields and flags > > > > > > Many features require to store data inside the mbuf. As the room in m= buf > > > structure is limited, it is not possible to have a field for each > > > feature. Also, changing fields in the mbuf structure can break the AP= I > > > or ABI. > > > > > > This commit addresses these issues, by enabling the dynamic registrat= ion > > > of fields or flags: > > > > > > - a dynamic field is a named area in the rte_mbuf structure, with a > > > given size (>=3D 1 byte) and alignment constraint. > > > - a dynamic flag is a named bit in the rte_mbuf structure. > > > > > > The typical use case is a PMD that registers space for an offload > > > feature, when the application requests to enable this feature. As > > > the space in mbuf is limited, the space should only be reserved if it > > > is going to be used (i.e when the application explicitly asks for it)= . > > > > > > The registration can be done at any moment, but it is not possible > > > to unregister fields or flags for now. > > > > > > Signed-off-by: Olivier Matz >=20 > (...) >=20 > > > +/** > > > + * @file > > > + * RTE Mbuf dynamic fields and flags > > > + * > > > + * Many features require to store data inside the mbuf. As the room = in > > > + * mbuf structure is limited, it is not possible to have a field for > > > + * each feature. Also, changing fields in the mbuf structure can bre= ak > > > + * the API or ABI. > > > + * > > > + * This module addresses this issue, by enabling the dynamic > > > + * registration of fields or flags: > > > + * > > > + * - a dynamic field is a named area in the rte_mbuf structure, with= a > > > + * given size (>=3D 1 byte) and alignment constraint. > > > + * - a dynamic flag is a named bit in the rte_mbuf structure. > > > + * > > > + * The typical use case is a PMD that registers space for an offload > > > + * feature, when the application requests to enable this feature. A= s > > > + * the space in mbuf is limited, the space should only be reserved i= f it > > > + * is going to be used (i.e when the application explicitly asks for= it). > > > + * > > > + * The registration can be done at any moment, but it is not possibl= e > > > + * to unregister fields or flags for now. > > > + * > > > + * Example of use: > > > + * > > > + * - RTE_MBUF_DYN__(ID|SIZE|ALIGN) are defined in this file > > > > Does it means that all PMDs define their own 'RTE_MBUF_DYN__(I= D|SIZE|ALIGN)' > > here ? In other words, each PMD can expose its private DYN_ he= re for public > > using ? >=20 > For generic fields, I think they should be declared in this file. For > instance, if we decide to replace the current m->timestamp field by a > dynamic field, we should add like this: >=20 > #define RTE_MBUF_DYN_TIMESTAMP_ID "rte_timestamp" > #define RTE_MBUF_DYN_TIMESTAMP_SIZE sizeof(uint64_t) > #define RTE_MBUF_DYN_TIMESTAMP_ALIGN __alignof__(uint64_t) >=20 > If the feature is PMD-specific, the defines could be exposed in a > PMD header. >=20 Now, understand the comments a little : ... must not define identifers pref= ixed with "rte_", which are reserved for standard features. Seems have big plan ? > > How about adding another eth_dev_ops API definitions to show the PMD's = supporting feature > > names, sizes, align in run time for testpmd ? And also another eth_dev_= ops API for showing > > the data saved in rte_mbuf by 'dump_pkt_burst' ? Adding a new command f= or testpmd to set > > the dynamic feature may be good for PMD test. > > > > > + * - If the application asks for the feature, the PMD use > > > > How does the application ask for the feature ? By ' rte_mbuf_dynfield_r= egister()' ? >=20 > No change in this area. If we take again the timestamp example, the > feature is asked by the application through the ethdev layer by passing > DEV_RX_OFFLOAD_TIMESTAMP to port or queue configuration. >=20 > > > > > + * rte_mbuf_dynfield_register() to get the dynamic offset and stor= es > > > + * in a global variable. > > > > In case, the PMD calls 'rte_mbuf_dynfield_register()' for 'dyn_feature'= firstly, this > > means that PMD requests the dynamic feature itself if I understand corr= ectly. Should > > PMD calls 'rte_mbuf_dynfield_lookup' for 'dyn_feature' to query the nam= e exists, the > > size and align are right as expected ? If exists, but size and align ar= e not right, may > > be for PMD change its definition, then PMD can give a warning or error = message. If name > > exists, both size and align are expected, then PMD think that the appli= cation request > > the right dynamic features. >=20 > The PMD should only call rte_mbuf_dynfield_register() if the application > requests the feature (through ethdev, or through another mean if it's a > PMD-specific feature). The goal is to only reserve the area in the mbuf > for features that are actually needed. >=20 > Hope this is clearer now. I think I need to enhance the documentation in > next version ;) >=20 Clearer now, more test code also will be better for fully understanding, th= anks! :) > Thanks for the feedback.