From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 2538168B7 for ; Fri, 12 Sep 2014 18:51:51 +0200 (CEST) Received: from azsmga001.ch.intel.com ([10.2.17.19]) by orsmga101.jf.intel.com with ESMTP; 12 Sep 2014 09:57:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,514,1406617200"; d="scan'208";a="477158569" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by azsmga001.ch.intel.com with ESMTP; 12 Sep 2014 09:57:06 -0700 Received: from irsmsx153.ger.corp.intel.com (163.33.192.75) by IRSMSX102.ger.corp.intel.com (163.33.3.155) with Microsoft SMTP Server (TLS) id 14.3.195.1; Fri, 12 Sep 2014 17:57:05 +0100 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.157]) by IRSMSX153.ger.corp.intel.com ([169.254.9.160]) with mapi id 14.03.0195.001; Fri, 12 Sep 2014 17:56:59 +0100 From: "Dumitrescu, Cristian" To: "Richardson, Bruce" , Olivier MATZ , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the mbuf metadata Thread-Index: AQHPx46wQcWhgcKKB0epuTO3xpr155v3GoqAgAFviACABTV2wA== Date: Fri, 12 Sep 2014 16:56:59 +0000 Message-ID: <3EB4FA525960D640B5BDFFD6A3D891262E070D42@IRSMSX108.ger.corp.intel.com> References: <1409759378-10113-1-git-send-email-bruce.richardson@intel.com> <1409759378-10113-8-git-send-email-bruce.richardson@intel.com> <540D9B95.3020504@6wind.com> <59AF69C657FD0841A61C55336867B5B0343EFAA3@IRSMSX103.ger.corp.intel.com> In-Reply-To: <59AF69C657FD0841A61C55336867B5B0343EFAA3@IRSMSX103.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the mbuf metadata X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Sep 2014 16:51:53 -0000 Bruce, Olivier, = What is the reason to remove this field? Please explain the rationale of re= moving this field. We previously agreed we need to provide an easy and standard mechanism for = applications to extend the mandatory per buffer metadata (mbuf) with option= al application-dependent metadata. This field just provides a clean way for= the apps to know where is the end of the mandatory metadata, i.e. the firs= t location in the packet buffer where the app can add its own metadata (of = course, the app has to manage the headroom space before the first byte of p= acket data). A zero-size field is the standard mechanism that DPDK uses ext= ensively in pretty much every library to access memory immediately after a = header structure. The impact of removing this field is that there is no standard way to ident= ify where the end of the mandatory metadata is, so each application will ha= ve to reinvent this. With no clear convention, we will end up with a lot of= non-standard ways. Every time the format of the mbuf structure is going to= be changed, this can potentially break applications that use custom metada= ta, while using this simple standard mechanism would prevent this. So why r= emove this? Having applications define their optional meta-data is a real need. Please = take a look at the Service Chaining IEFT emerging protocols (https://datatr= acker.ietf.org/wg/sfc/documents/), which provide standard mechanisms for ap= plications to define their own packet meta-data and share it between the el= ements of the processing pipeline (for Service Chaining, these are typicall= y virtual machines scattered amongst the data center). And, in my opinion, there is no negative impact/cost associated with keepin= g this field. Regards, Cristian -----Original Message----- From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Richardson, Bruce Sent: Tuesday, September 9, 2014 10:01 AM To: Olivier MATZ; dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the m= buf metadata > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Monday, September 08, 2014 1:06 PM > To: Richardson, Bruce; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the > mbuf metadata > = > Hi Bruce, > = > On 09/03/2014 05:49 PM, Bruce Richardson wrote: > > Removed the explicit zero-sized metadata definition at the end of the > > mbuf data structure. Updated the metadata macros to take account of this > > change so that all existing code which uses those macros still works. > > > > Signed-off-by: Bruce Richardson > > --- > > lib/librte_mbuf/rte_mbuf.h | 22 ++++++++-------------- > > 1 file changed, 8 insertions(+), 14 deletions(-) > > > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > > index 5260001..ca66d9a 100644 > > --- a/lib/librte_mbuf/rte_mbuf.h > > +++ b/lib/librte_mbuf/rte_mbuf.h > > @@ -166,31 +166,25 @@ struct rte_mbuf { > > struct rte_mempool *pool; /**< Pool from which mbuf was allocated. > */ > > struct rte_mbuf *next; /**< Next segment of scattered packet. */ > > > > - union { > > - uint8_t metadata[0]; > > - uint16_t metadata16[0]; > > - uint32_t metadata32[0]; > > - uint64_t metadata64[0]; > > - } __rte_cache_aligned; > > } __rte_cache_aligned; > > > > #define RTE_MBUF_METADATA_UINT8(mbuf, offset) \ > > - (mbuf->metadata[offset]) > > + (((uint8_t *)&(mbuf)[1])[offset]) > > #define RTE_MBUF_METADATA_UINT16(mbuf, offset) \ > > - (mbuf->metadata16[offset/sizeof(uint16_t)]) > > + (((uint16_t *)&(mbuf)[1])[offset/sizeof(uint16_t)]) > > #define RTE_MBUF_METADATA_UINT32(mbuf, offset) \ > > - (mbuf->metadata32[offset/sizeof(uint32_t)]) > > + (((uint32_t *)&(mbuf)[1])[offset/sizeof(uint32_t)]) > > #define RTE_MBUF_METADATA_UINT64(mbuf, offset) \ > > - (mbuf->metadata64[offset/sizeof(uint64_t)]) > > + (((uint64_t *)&(mbuf)[1])[offset/sizeof(uint64_t)]) > > > > #define RTE_MBUF_METADATA_UINT8_PTR(mbuf, offset) \ > > - (&mbuf->metadata[offset]) > > + (&RTE_MBUF_METADATA_UINT8(mbuf, offset)) > > #define RTE_MBUF_METADATA_UINT16_PTR(mbuf, offset) \ > > - (&mbuf->metadata16[offset/sizeof(uint16_t)]) > > + (&RTE_MBUF_METADATA_UINT16(mbuf, offset)) > > #define RTE_MBUF_METADATA_UINT32_PTR(mbuf, offset) \ > > - (&mbuf->metadata32[offset/sizeof(uint32_t)]) > > + (&RTE_MBUF_METADATA_UINT32(mbuf, offset)) > > #define RTE_MBUF_METADATA_UINT64_PTR(mbuf, offset) \ > > - (&mbuf->metadata64[offset/sizeof(uint64_t)]) > > + (&RTE_MBUF_METADATA_UINT64(mbuf, offset)) > > > > /** > > * Given the buf_addr returns the pointer to corresponding mbuf. > > > = > I think it goes in the good direction. So: > Acked-by: Olivier Matz > = > Just one question: why not removing RTE_MBUF_METADATA*() macros? > I'd just provide one macro that gives a (void*) to the first byte > after the mbuf structure. > = > The format of the metadata is up to the application, that usually > casts (m + 1) into a private structure, making the macros not very > useful. I suggest to move these macros outside rte_mbuf.h, in the > application-specific or library-specific header, what do you think? > = > Regards, > Olivier > = Yes, I'll look into that. /Bruce -------------------------------------------------------------- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the s= ole use of the intended recipient(s). Any review or distribution by others = is strictly prohibited. If you are not the intended recipient, please conta= ct the sender and delete all copies.