From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 9746F68B7 for ; Fri, 12 Sep 2014 22:57:19 +0200 (CEST) Received: from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214] helo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1XSY1o-0002Ip-1E; Fri, 12 Sep 2014 23:05:17 +0200 Message-ID: <54135F63.2090401@6wind.com> Date: Fri, 12 Sep 2014 23:02:27 +0200 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.5.0 MIME-Version: 1.0 To: "Dumitrescu, Cristian" , "Richardson, Bruce" , "dev@dpdk.org" 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> <3EB4FA525960D640B5BDFFD6A3D891262E070D42@IRSMSX108.ger.corp.intel.com> In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D891262E070D42@IRSMSX108.ger.corp.intel.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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 20:57:19 -0000 Hello Cristian, > What is the reason to remove this field? Please explain the > rationale of removing this field. The rationale is explained in http://dpdk.org/ml/archives/dev/2014-September/005232.html "The format of the metadata is up to the application". The type of data the application stores after the mbuf has not to be defined in the mbuf. These macros limits the types of metadata to uint8, uint16, uint32, uint64? What should I do if I need a void*, a struct foo ? Should we add a macro for each possible type? > We previously agreed we need to provide an easy and standard > mechanism for applications to extend the mandatory per buffer > metadata (mbuf) with optional application-dependent > metadata. Defining a structure in the application which does not pollute the rte_mbuf structure is "easy and standard(TM)" too. > This field just provides a clean way for the apps to > know where is the end of the mandatory metadata, i.e. the first > 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 packet data). A zero-size field is the > standard mechanism that DPDK uses extensively in pretty much > every library to access memory immediately after a header > structure. Having the following is clean too: struct metadata { ... }; struct app_mbuf { struct rte_mbuf mbuf; struct metadata metadata; }; There is no need to define anything in the mbuf structure. > > The impact of removing this field is that there is no standard > way to identify where the end of the mandatory metadata is, so > each application will have 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 metadata, > while using this simple standard mechanism would prevent this. So > why remove this? Waow. Five occurences of "standard" until now. Could you give a reference to the standard you're refering to? :) Our application defines private metadata in mbufs in the way described above, we never changed that since we're supporting the dpdk. So I don't understand when you say that each time mbuf is reformatted it breaks the application. > Having applications define their optional meta-data is a real > need. Sure. This patch does not prevent this at all. You can continue to do exactly the same, but in the concerned application, not in the generic mbuf structure. > Please take a look at the Service Chaining IEFT emerging > protocols (https://datatracker.ietf.org/wg/sfc/documents/), which > provide standard mechanisms for applications to define their own Six :) I'm not sure these documents define the way to extend a packet structure with metadata in a C program. Again, Bruce's patch does not prevent to do what you need, it just moves it at the proper place. > packet meta-data and share it between the elements of the > processing pipeline (for Service Chaining, these are typically > virtual machines scattered amongst the data center). > > And, in my opinion, there is no negative impact/cost associated > with keeping this field. To summarize what I think: - this patch does not prevent to do what you want to do - removing the macros help to have a shorter and more comprehensible mbuf structure - the previous approach does not scale because it would require a macro per type > -------------------------------------------------------------- > 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 sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. This is a public mailing list, this disclaimer is invalid. Regards, Olivier