Patch Comment List
List patch comments
GET /api/patches/306/comments/?format=api&order=submitter
[ { "id": 830, "web_url": "https://patches.dpdk.org/comment/830/", "msgid": "<3536812.LpadffOgB2@xps13>", "list_archive_url": "https://inbox.dpdk.org/dev/3536812.LpadffOgB2@xps13", "date": "2014-09-17T14:01:37", "subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "submitter": { "id": 1, "url": "https://patches.dpdk.org/api/people/1/?format=api", "name": "Thomas Monjalon", "email": "thomas.monjalon@6wind.com" }, "content": "2014-09-17 10:31, Richardson, Bruce:\n> > From: Ramia, Kannan Babu\n> > \n> > I completely agree with Cristian here, instead of leaving to applications\n> > where to place their meta data, we can provide a guidance by having this\n> > field about placement of application meta while maintaining transparency\n> > on the contents of application meta information.\n> \n> My opinion on this is that this is better served via documentation or a\n> comment in the code. The reason is that this approach is not going to be\n> suitable for all applications. The mbuf headroom being used by the metadata\n> is actually designed to be used for any additional headers to be added to\n> the packet - though other things can obviously be stored in it too.\n> Therefore the amount of metadata that can be stored in it will depend from\n> application to application, as any apps doing e.g. tunnelling will need the\n> headroom for tunnelling headers and may only be able to store a small\n> amount of metadata - potentially none. For larger amounts of metadata - I\n> would feel that anything over 64-bytes or so - I have proposed adding in a\n> separate userdata pointer in the mbuf structure so that apps have the\n> option of storing the metadata externally e.g. pointing to a flow table\n> entry or similar. [Please see mbuf rework patch set 3 proposal]. Because of\n> this, I think it's better to put in a comment in the code indicating that\n> metadata can go in the headroom, document this properly - including caveats\n> and limitations - in the documentation, and provide an example of doing\n> such - something we already have in the packet framework.\n\nI agree that replacing these markers by documentation give more accurate\ninformations and (bonus) is simpler.\nWhen documentation will be embedded in the git repository, I'd like to see\na patch to document these mbuf usages.\n\n> All that being said, and while I think this is a good patch, I don't feel\n> too strongly about it. I'm happy enough if this particular patch does not\n> get merged in for 1.8, as it's incidental to the overall mbuf changes.\n\nI think also it's a good patch so I keep it.", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 4DC7AB3A0;\n\tWed, 17 Sep 2014 15:56:08 +0200 (CEST)", "from mail-we0-f169.google.com (mail-we0-f169.google.com\n\t[74.125.82.169]) by dpdk.org (Postfix) with ESMTP id BE032B39C\n\tfor <dev@dpdk.org>; Wed, 17 Sep 2014 15:56:06 +0200 (CEST)", "by mail-we0-f169.google.com with SMTP id w61so1506527wes.0\n\tfor <dev@dpdk.org>; Wed, 17 Sep 2014 07:01:48 -0700 (PDT)", "from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136])\n\tby mx.google.com with ESMTPSA id\n\tpc6sm22264849wjb.43.2014.09.17.07.01.46 for <multiple recipients>\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 17 Sep 2014 07:01:47 -0700 (PDT)" ], "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20130820;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:organization\n\t:user-agent:in-reply-to:references:mime-version\n\t:content-transfer-encoding:content-type;\n\tbh=wb4Kq28WWsIZlZiv98yDujy8grvcnjpP8Ff1fy5Sxmc=;\n\tb=EiVKnnPhPXFH9oev4CoVxqWzcAq3jm++viqTW2YjfcVtWFREhVy5tDVro/LdDn/n9v\n\tXaYTe75c1ZLGEQDXz5sV2EVbKw/m1eA9mOzljqYLUP3AMA6LjCTgn1AKBUHg/lS2VJJv\n\t2h8P4hVzB7hioOKFCm0bYIOmY2VUF0o4iYlOGDyfD3INTnjxX+yw+Txq+/wYeXPPCgIP\n\taVAzdNXv4iFQIbdW5YPGSCbaTPDJTL1zqphjj9fMWffHTvqpHs0iDK9qWtRxvAWLalLh\n\tvt6ZwWaAGjbdmPNneh8aushXGJS5duqK/9rNv4upVX7AXemIbOrUa6OPWAKuGUzAMnNx\n\tukMQ==", "X-Gm-Message-State": "ALoCoQlCsS8+FZA4/YhL+FkUiaYGvqMh9upYbzwGlPnIFXIZmOn9ydLzOS3WSn28oNKIggE2CsZl", "X-Received": "by 10.180.96.161 with SMTP id dt1mr40773707wib.1.1410962507930; \n\tWed, 17 Sep 2014 07:01:47 -0700 (PDT)", "From": "Thomas Monjalon <thomas.monjalon@6wind.com>", "To": "\"Richardson, Bruce\" <bruce.richardson@intel.com>", "Date": "Wed, 17 Sep 2014 16:01:37 +0200", "Message-ID": "<3536812.LpadffOgB2@xps13>", "Organization": "6WIND", "User-Agent": "KMail/4.13.3 (Linux/3.15.8-1-ARCH; KDE/4.13.3; x86_64; ; )", "In-Reply-To": "<59AF69C657FD0841A61C55336867B5B0343F2BD2@IRSMSX103.ger.corp.intel.com>", "References": "<1409759378-10113-1-git-send-email-bruce.richardson@intel.com>\n\t<682698A055A0F44AA47192B20141149711B1FFE6@BGSMSX102.gar.corp.intel.com>\n\t<59AF69C657FD0841A61C55336867B5B0343F2BD2@IRSMSX103.ger.corp.intel.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "7Bit", "Content-Type": "text/plain; charset=\"us-ascii\"", "Cc": "dev@dpdk.org", "Subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 766, "web_url": "https://patches.dpdk.org/comment/766/", "msgid": "<54135F63.2090401@6wind.com>", "list_archive_url": "https://inbox.dpdk.org/dev/54135F63.2090401@6wind.com", "date": "2014-09-12T21:02:27", "subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "submitter": { "id": 8, "url": "https://patches.dpdk.org/api/people/8/?format=api", "name": "Olivier Matz", "email": "olivier.matz@6wind.com" }, "content": "Hello Cristian,\n\n> What is the reason to remove this field? Please explain the\n> rationale of removing this field.\n\nThe rationale is explained in\nhttp://dpdk.org/ml/archives/dev/2014-September/005232.html\n\n\"The format of the metadata is up to the application\".\n\nThe type of data the application stores after the mbuf has not\nto be defined in the mbuf. These macros limits the types of\nmetadata to uint8, uint16, uint32, uint64? What should I do\nif I need a void*, a struct foo ? Should we add a macro for\neach possible type?\n\n> We previously agreed we need to provide an easy and standard\n> mechanism for applications to extend the mandatory per buffer\n> metadata (mbuf) with optional application-dependent\n> metadata.\n\nDefining a structure in the application which does not pollute\nthe rte_mbuf structure is \"easy and standard(TM)\" too.\n\n> This field just provides a clean way for the apps to\n> know where is the end of the mandatory metadata, i.e. the first\n> location in the packet buffer where the app can add its own\n> metadata (of course, the app has to manage the headroom space\n> before the first byte of packet data). A zero-size field is the\n> standard mechanism that DPDK uses extensively in pretty much\n> every library to access memory immediately after a header\n> structure.\n\nHaving the following is clean too:\n\nstruct metadata {\n ...\n};\n\nstruct app_mbuf {\n struct rte_mbuf mbuf;\n struct metadata metadata;\n};\n\nThere is no need to define anything in the mbuf structure.\n\n> \n> The impact of removing this field is that there is no standard\n> way to identify where the end of the mandatory metadata is, so\n> each application will have to reinvent this. With no clear\n> convention, we will end up with a lot of non-standard ways. Every\n> time the format of the mbuf structure is going to be changed,\n> this can potentially break applications that use custom metadata,\n> while using this simple standard mechanism would prevent this. So\n> why remove this?\n\nWaow. Five occurences of \"standard\" until now. Could you give a\nreference to the standard you're refering to? :)\n\nOur application defines private metadata in mbufs in the way described\nabove, we never changed that since we're supporting the dpdk. So\nI don't understand when you say that each time mbuf is reformatted\nit breaks the application.\n\n> Having applications define their optional meta-data is a real\n> need. \n\nSure. This patch does not prevent this at all. You can continue\nto do exactly the same, but in the concerned application, not\nin the generic mbuf structure.\n\n> Please take a look at the Service Chaining IEFT emerging\n> protocols (https://datatracker.ietf.org/wg/sfc/documents/), which\n> provide standard mechanisms for applications to define their own\n\nSix :)\n\nI'm not sure these documents define the way to extend a packet\nstructure with metadata in a C program. Again, Bruce's patch does\nnot prevent to do what you need, it just moves it at the proper\nplace.\n\n> packet meta-data and share it between the elements of the\n> processing pipeline (for Service Chaining, these are typically\n> virtual machines scattered amongst the data center).\n> \n> And, in my opinion, there is no negative impact/cost associated\n> with keeping this field.\n\nTo summarize what I think:\n\n- this patch does not prevent to do what you want to do\n- removing the macros help to have a shorter and more comprehensible\n mbuf structure\n- the previous approach does not scale because it would require\n a macro per type\n\n\n> --------------------------------------------------------------\n> Intel Shannon Limited\n> Registered in Ireland\n> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare\n> Registered Number: 308263\n> Business address: Dromore House, East Park, Shannon, Co. Clare\n> \n> 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.\n\nThis is a public mailing list, this disclaimer is invalid.\n\nRegards,\nOlivier", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 5FCF66AB7;\n\tFri, 12 Sep 2014 22:57:21 +0200 (CEST)", "from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67])\n\tby dpdk.org (Postfix) with ESMTP id 9746F68B7\n\tfor <dev@dpdk.org>; Fri, 12 Sep 2014 22:57:19 +0200 (CEST)", "from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214]\n\thelo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa\n\t(TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128)\n\t(Exim 4.80) (envelope-from <olivier.matz@6wind.com>)\n\tid 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 <olivier.matz@6wind.com>", "User-Agent": "Mozilla/5.0 (X11; Linux x86_64;\n\trv:24.0) Gecko/20100101 Icedove/24.5.0", "MIME-Version": "1.0", "To": "\"Dumitrescu, Cristian\" <cristian.dumitrescu@intel.com>, \n\t\"Richardson, Bruce\" <bruce.richardson@intel.com>,\n\t\"dev@dpdk.org\" <dev@dpdk.org>", "References": "<1409759378-10113-1-git-send-email-bruce.richardson@intel.com>\n\t<1409759378-10113-8-git-send-email-bruce.richardson@intel.com>\n\t<540D9B95.3020504@6wind.com>\n\t<59AF69C657FD0841A61C55336867B5B0343EFAA3@IRSMSX103.ger.corp.intel.com>\n\t<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\n\tmbuf metadata", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 717, "web_url": "https://patches.dpdk.org/comment/717/", "msgid": "<54106EBC.6040206@6wind.com>", "list_archive_url": "https://inbox.dpdk.org/dev/54106EBC.6040206@6wind.com", "date": "2014-09-10T15:31:08", "subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "submitter": { "id": 8, "url": "https://patches.dpdk.org/api/people/8/?format=api", "name": "Olivier Matz", "email": "olivier.matz@6wind.com" }, "content": "Hi Bruce,\n\nOn 09/10/2014 05:09 PM, Bruce Richardson wrote:\n>> Just one question: why not removing RTE_MBUF_METADATA*() macros?\n>> I'd just provide one macro that gives a (void*) to the first byte\n>> after the mbuf structure.\n>>\n>> The format of the metadata is up to the application, that usually\n>> casts (m + 1) into a private structure, making the macros not very\n>> useful. I suggest to move these macros outside rte_mbuf.h, in the\n>> application-specific or library-specific header, what do you think?\n>>\n>\n> Things look to work if I just move the definitions en-masse into\n> rte_port.h. Is that the sort of change you would be thinking of?\n> I was wondering about replacing the typed macros here with a\n> generic one to access just beyond the definition of the mbuf\n> structure, but on further thought, I believe that using the\n> buf_addr pointer in the mbuf data structure is probably enough\n> for most applications. [An alternative to moving the definitions\n> into rte_port.h is to move them into rte_table.h and having\n> port_frag.c use the buf_addr pointer instead of a macro to get at\n> the metadata. All other references to the macros apart from two\n> in that port file, are in the tables or in apps that use the\n> tables lib.]\n> What do you think?\n\nYes, moving the macros in rte_port.h looks good to me, since\nthe libraries using rte_ports are the users of this specific\nmetadata format.\n\nThanks!\nOlivier", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id D15266829;\n\tWed, 10 Sep 2014 17:26:02 +0200 (CEST)", "from mail-wg0-f50.google.com (mail-wg0-f50.google.com\n\t[74.125.82.50]) by dpdk.org (Postfix) with ESMTP id 5ED1632A5\n\tfor <dev@dpdk.org>; Wed, 10 Sep 2014 17:26:01 +0200 (CEST)", "by mail-wg0-f50.google.com with SMTP id x13so6359735wgg.33\n\tfor <dev@dpdk.org>; Wed, 10 Sep 2014 08:31:10 -0700 (PDT)", "from [10.16.0.195] (guy78-3-82-239-227-177.fbx.proxad.net.\n\t[82.239.227.177]) by mx.google.com with ESMTPSA id\n\tbj7sm2452679wjc.33.2014.09.10.08.31.08 for <multiple recipients>\n\t(version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128);\n\tWed, 10 Sep 2014 08:31:09 -0700 (PDT)" ], "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20130820;\n\th=x-gm-message-state:message-id:date:from:user-agent:mime-version:to\n\t:cc:subject:references:in-reply-to:content-type\n\t:content-transfer-encoding;\n\tbh=QCMjc26DWol1CnHeRAfw9izSilWAxYgeI4MhDEM1dGM=;\n\tb=CYcVMed52tSBYHxiH883o0VKUdrWSga34YpRhNQNTLagsjSD7FQlxls3W3VKJZ4yLo\n\tHg2oaD72L4CC2+zlFHGCjinap+Xf6qrJRNX9EXJpwPOJhRM9XQslpRuYdMxrRFbd72+r\n\tAGHLIRFthuhPZmi334RxD8/NDCmJXDhYsq53N3TfQXG6U6OyRuAAbzgdykWILqeVdqoM\n\tK47nnLdmLGg8b95RcitgObs8REjGcoLW2fTA4Ao0s3CAFF9EnmI6vIBv+AdFPfZMn5fk\n\tSZ1g1DK/UulaoMQhQG9x6jAm+ralscFleJ+x0mXE4mRuPWWuAP2v9z/4sMKxJO4QBgGq\n\tHXHQ==", "X-Gm-Message-State": "ALoCoQmKTyjyQBLCMFp6Qbe68QE6cSiCnqcIB3ZmzGmxtJtIad6VuWEicrFLkDpIk97mqrkWly9G", "X-Received": "by 10.180.72.239 with SMTP id g15mr12530729wiv.47.1410363070428; \n\tWed, 10 Sep 2014 08:31:10 -0700 (PDT)", "Message-ID": "<54106EBC.6040206@6wind.com>", "Date": "Wed, 10 Sep 2014 17:31:08 +0200", "From": "Olivier MATZ <olivier.matz@6wind.com>", "User-Agent": "Mozilla/5.0 (X11; Linux x86_64;\n\trv:24.0) Gecko/20100101 Icedove/24.5.0", "MIME-Version": "1.0", "To": "Bruce Richardson <bruce.richardson@intel.com>", "References": "<1409759378-10113-1-git-send-email-bruce.richardson@intel.com>\n\t<1409759378-10113-8-git-send-email-bruce.richardson@intel.com>\n\t<540D9B95.3020504@6wind.com> <20140910150914.GA9656@BRICHA3-MOBL>", "In-Reply-To": "<20140910150914.GA9656@BRICHA3-MOBL>", "Content-Type": "text/plain; charset=ISO-8859-1; format=flowed", "Content-Transfer-Encoding": "7bit", "Cc": "dev@dpdk.org", "Subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 686, "web_url": "https://patches.dpdk.org/comment/686/", "msgid": "<540D9B95.3020504@6wind.com>", "list_archive_url": "https://inbox.dpdk.org/dev/540D9B95.3020504@6wind.com", "date": "2014-09-08T12:05:41", "subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "submitter": { "id": 8, "url": "https://patches.dpdk.org/api/people/8/?format=api", "name": "Olivier Matz", "email": "olivier.matz@6wind.com" }, "content": "Hi Bruce,\n\nOn 09/03/2014 05:49 PM, Bruce Richardson wrote:\n> Removed the explicit zero-sized metadata definition at the end of the\n> mbuf data structure. Updated the metadata macros to take account of this\n> change so that all existing code which uses those macros still works.\n> \n> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>\n> ---\n> lib/librte_mbuf/rte_mbuf.h | 22 ++++++++--------------\n> 1 file changed, 8 insertions(+), 14 deletions(-)\n> \n> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h\n> index 5260001..ca66d9a 100644\n> --- a/lib/librte_mbuf/rte_mbuf.h\n> +++ b/lib/librte_mbuf/rte_mbuf.h\n> @@ -166,31 +166,25 @@ struct rte_mbuf {\n> \tstruct rte_mempool *pool; /**< Pool from which mbuf was allocated. */\n> \tstruct rte_mbuf *next; /**< Next segment of scattered packet. */\n> \n> -\tunion {\n> -\t\tuint8_t metadata[0];\n> -\t\tuint16_t metadata16[0];\n> -\t\tuint32_t metadata32[0];\n> -\t\tuint64_t metadata64[0];\n> -\t} __rte_cache_aligned;\n> } __rte_cache_aligned;\n> \n> #define RTE_MBUF_METADATA_UINT8(mbuf, offset) \\\n> -\t(mbuf->metadata[offset])\n> +\t(((uint8_t *)&(mbuf)[1])[offset])\n> #define RTE_MBUF_METADATA_UINT16(mbuf, offset) \\\n> -\t(mbuf->metadata16[offset/sizeof(uint16_t)])\n> +\t(((uint16_t *)&(mbuf)[1])[offset/sizeof(uint16_t)])\n> #define RTE_MBUF_METADATA_UINT32(mbuf, offset) \\\n> -\t(mbuf->metadata32[offset/sizeof(uint32_t)])\n> +\t(((uint32_t *)&(mbuf)[1])[offset/sizeof(uint32_t)])\n> #define RTE_MBUF_METADATA_UINT64(mbuf, offset) \\\n> -\t(mbuf->metadata64[offset/sizeof(uint64_t)])\n> +\t(((uint64_t *)&(mbuf)[1])[offset/sizeof(uint64_t)])\n> \n> #define RTE_MBUF_METADATA_UINT8_PTR(mbuf, offset) \\\n> -\t(&mbuf->metadata[offset])\n> +\t(&RTE_MBUF_METADATA_UINT8(mbuf, offset))\n> #define RTE_MBUF_METADATA_UINT16_PTR(mbuf, offset) \\\n> -\t(&mbuf->metadata16[offset/sizeof(uint16_t)])\n> +\t(&RTE_MBUF_METADATA_UINT16(mbuf, offset))\n> #define RTE_MBUF_METADATA_UINT32_PTR(mbuf, offset) \\\n> -\t(&mbuf->metadata32[offset/sizeof(uint32_t)])\n> +\t(&RTE_MBUF_METADATA_UINT32(mbuf, offset))\n> #define RTE_MBUF_METADATA_UINT64_PTR(mbuf, offset) \\\n> -\t(&mbuf->metadata64[offset/sizeof(uint64_t)])\n> +\t(&RTE_MBUF_METADATA_UINT64(mbuf, offset))\n> \n> /**\n> * Given the buf_addr returns the pointer to corresponding mbuf.\n> \n\nI think it goes in the good direction. So:\nAcked-by: Olivier Matz <olivier.matz@6wind.com>\n\nJust one question: why not removing RTE_MBUF_METADATA*() macros?\nI'd just provide one macro that gives a (void*) to the first byte\nafter the mbuf structure.\n\nThe format of the metadata is up to the application, that usually\ncasts (m + 1) into a private structure, making the macros not very\nuseful. I suggest to move these macros outside rte_mbuf.h, in the\napplication-specific or library-specific header, what do you think?\n\nRegards,\nOlivier", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id D21EAB39A;\n\tMon, 8 Sep 2014 14:00:54 +0200 (CEST)", "from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67])\n\tby dpdk.org (Postfix) with ESMTP id 34AC3B399\n\tfor <dev@dpdk.org>; Mon, 8 Sep 2014 14:00:53 +0200 (CEST)", "from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214]\n\thelo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa\n\t(TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128)\n\t(Exim 4.80) (envelope-from <olivier.matz@6wind.com>)\n\tid 1XQxk7-0007jZ-8l; Mon, 08 Sep 2014 14:08:27 +0200" ], "Message-ID": "<540D9B95.3020504@6wind.com>", "Date": "Mon, 08 Sep 2014 14:05:41 +0200", "From": "Olivier MATZ <olivier.matz@6wind.com>", "User-Agent": "Mozilla/5.0 (X11; Linux x86_64;\n\trv:24.0) Gecko/20100101 Icedove/24.5.0", "MIME-Version": "1.0", "To": "Bruce Richardson <bruce.richardson@intel.com>, dev@dpdk.org", "References": "<1409759378-10113-1-git-send-email-bruce.richardson@intel.com>\n\t<1409759378-10113-8-git-send-email-bruce.richardson@intel.com>", "In-Reply-To": "<1409759378-10113-8-git-send-email-bruce.richardson@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\n\tmbuf metadata", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 761, "web_url": "https://patches.dpdk.org/comment/761/", "msgid": "<3EB4FA525960D640B5BDFFD6A3D891262E070D42@IRSMSX108.ger.corp.intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/3EB4FA525960D640B5BDFFD6A3D891262E070D42@IRSMSX108.ger.corp.intel.com", "date": "2014-09-12T16:56:59", "subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "submitter": { "id": 19, "url": "https://patches.dpdk.org/api/people/19/?format=api", "name": "Cristian Dumitrescu", "email": "cristian.dumitrescu@intel.com" }, "content": "Bruce, Olivier, \n\nWhat is the reason to remove this field? Please explain the rationale of removing this field.\n\nWe 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. 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.\n\nThe 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?\n\nHaving applications define their optional meta-data is a real need. 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 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).\n\nAnd, in my opinion, there is no negative impact/cost associated with keeping this field.\n\nRegards,\nCristian\n\n\n-----Original Message-----\nFrom: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Richardson, Bruce\nSent: Tuesday, September 9, 2014 10:01 AM\nTo: Olivier MATZ; dev@dpdk.org\nSubject: Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the mbuf metadata\n\n> -----Original Message-----\n> From: Olivier MATZ [mailto:olivier.matz@6wind.com]\n> Sent: Monday, September 08, 2014 1:06 PM\n> To: Richardson, Bruce; dev@dpdk.org\n> Subject: Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n> mbuf metadata\n> \n> Hi Bruce,\n> \n> On 09/03/2014 05:49 PM, Bruce Richardson wrote:\n> > Removed the explicit zero-sized metadata definition at the end of the\n> > mbuf data structure. Updated the metadata macros to take account of this\n> > change so that all existing code which uses those macros still works.\n> >\n> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>\n> > ---\n> > lib/librte_mbuf/rte_mbuf.h | 22 ++++++++--------------\n> > 1 file changed, 8 insertions(+), 14 deletions(-)\n> >\n> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h\n> > index 5260001..ca66d9a 100644\n> > --- a/lib/librte_mbuf/rte_mbuf.h\n> > +++ b/lib/librte_mbuf/rte_mbuf.h\n> > @@ -166,31 +166,25 @@ struct rte_mbuf {\n> > \tstruct rte_mempool *pool; /**< Pool from which mbuf was allocated.\n> */\n> > \tstruct rte_mbuf *next; /**< Next segment of scattered packet. */\n> >\n> > -\tunion {\n> > -\t\tuint8_t metadata[0];\n> > -\t\tuint16_t metadata16[0];\n> > -\t\tuint32_t metadata32[0];\n> > -\t\tuint64_t metadata64[0];\n> > -\t} __rte_cache_aligned;\n> > } __rte_cache_aligned;\n> >\n> > #define RTE_MBUF_METADATA_UINT8(mbuf, offset) \\\n> > -\t(mbuf->metadata[offset])\n> > +\t(((uint8_t *)&(mbuf)[1])[offset])\n> > #define RTE_MBUF_METADATA_UINT16(mbuf, offset) \\\n> > -\t(mbuf->metadata16[offset/sizeof(uint16_t)])\n> > +\t(((uint16_t *)&(mbuf)[1])[offset/sizeof(uint16_t)])\n> > #define RTE_MBUF_METADATA_UINT32(mbuf, offset) \\\n> > -\t(mbuf->metadata32[offset/sizeof(uint32_t)])\n> > +\t(((uint32_t *)&(mbuf)[1])[offset/sizeof(uint32_t)])\n> > #define RTE_MBUF_METADATA_UINT64(mbuf, offset) \\\n> > -\t(mbuf->metadata64[offset/sizeof(uint64_t)])\n> > +\t(((uint64_t *)&(mbuf)[1])[offset/sizeof(uint64_t)])\n> >\n> > #define RTE_MBUF_METADATA_UINT8_PTR(mbuf, offset) \\\n> > -\t(&mbuf->metadata[offset])\n> > +\t(&RTE_MBUF_METADATA_UINT8(mbuf, offset))\n> > #define RTE_MBUF_METADATA_UINT16_PTR(mbuf, offset) \\\n> > -\t(&mbuf->metadata16[offset/sizeof(uint16_t)])\n> > +\t(&RTE_MBUF_METADATA_UINT16(mbuf, offset))\n> > #define RTE_MBUF_METADATA_UINT32_PTR(mbuf, offset) \\\n> > -\t(&mbuf->metadata32[offset/sizeof(uint32_t)])\n> > +\t(&RTE_MBUF_METADATA_UINT32(mbuf, offset))\n> > #define RTE_MBUF_METADATA_UINT64_PTR(mbuf, offset) \\\n> > -\t(&mbuf->metadata64[offset/sizeof(uint64_t)])\n> > +\t(&RTE_MBUF_METADATA_UINT64(mbuf, offset))\n> >\n> > /**\n> > * Given the buf_addr returns the pointer to corresponding mbuf.\n> >\n> \n> I think it goes in the good direction. So:\n> Acked-by: Olivier Matz <olivier.matz@6wind.com>\n> \n> Just one question: why not removing RTE_MBUF_METADATA*() macros?\n> I'd just provide one macro that gives a (void*) to the first byte\n> after the mbuf structure.\n> \n> The format of the metadata is up to the application, that usually\n> casts (m + 1) into a private structure, making the macros not very\n> useful. I suggest to move these macros outside rte_mbuf.h, in the\n> application-specific or library-specific header, what do you think?\n> \n> Regards,\n> Olivier\n> \nYes, I'll look into that.\n\n/Bruce\n--------------------------------------------------------------\nIntel Shannon Limited\nRegistered in Ireland\nRegistered Office: Collinstown Industrial Park, Leixlip, County Kildare\nRegistered Number: 308263\nBusiness address: Dromore House, East Park, Shannon, Co. Clare\n\nThis 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.", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 4E3BAAF7F;\n\tFri, 12 Sep 2014 18:51:54 +0200 (CEST)", "from mga02.intel.com (mga02.intel.com [134.134.136.20])\n\tby dpdk.org (Postfix) with ESMTP id 2538168B7\n\tfor <dev@dpdk.org>; Fri, 12 Sep 2014 18:51:51 +0200 (CEST)", "from azsmga001.ch.intel.com ([10.2.17.19])\n\tby orsmga101.jf.intel.com with ESMTP; 12 Sep 2014 09:57:08 -0700", "from irsmsx102.ger.corp.intel.com ([163.33.3.155])\n\tby azsmga001.ch.intel.com with ESMTP; 12 Sep 2014 09:57:06 -0700", "from irsmsx153.ger.corp.intel.com (163.33.192.75) by\n\tIRSMSX102.ger.corp.intel.com (163.33.3.155) with Microsoft SMTP\n\tServer (TLS) id 14.3.195.1; Fri, 12 Sep 2014 17:57:05 +0100", "from irsmsx108.ger.corp.intel.com ([169.254.11.157]) by\n\tIRSMSX153.ger.corp.intel.com ([169.254.9.160]) with mapi id\n\t14.03.0195.001; Fri, 12 Sep 2014 17:56:59 +0100" ], "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.04,514,1406617200\"; d=\"scan'208\";a=\"477158569\"", "From": "\"Dumitrescu, Cristian\" <cristian.dumitrescu@intel.com>", "To": "\"Richardson, Bruce\" <bruce.richardson@intel.com>, Olivier MATZ\n\t<olivier.matz@6wind.com>, \"dev@dpdk.org\" <dev@dpdk.org>", "Thread-Topic": "[dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf 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>\n\t<1409759378-10113-8-git-send-email-bruce.richardson@intel.com>\n\t<540D9B95.3020504@6wind.com>\n\t<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\n\tmbuf metadata", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 796, "web_url": "https://patches.dpdk.org/comment/796/", "msgid": "<3EB4FA525960D640B5BDFFD6A3D891262E071FE6@IRSMSX108.ger.corp.intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/3EB4FA525960D640B5BDFFD6A3D891262E071FE6@IRSMSX108.ger.corp.intel.com", "date": "2014-09-16T20:07:27", "subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "submitter": { "id": 19, "url": "https://patches.dpdk.org/api/people/19/?format=api", "name": "Cristian Dumitrescu", "email": "cristian.dumitrescu@intel.com" }, "content": "Hi Olivier,\n\nI agree that your suggested approach for application-dependent metadata makes sense, in fact the two approaches work in exactly the same way (packet metadata immediately after the regular mbuf), there is only a subtle difference, which is related to defining consistent DPDK usage guidelines.\n\n1. Advertising the presence of application-dependent meta-data as supported mechanism\nIf we explicitly have a metadata zero-size field at the end of the mbuf, we basically tell people that adding their own application meta-data at the end of the mandatory meta-data (mbuf structure) is a mechanism that DPDK allows and supports, and will continue to do so for the foreseeable future. In other words, we guarantee that an application doing so will continue to build successfully with future releases of DPDK, and we will not introduce changes in DPDK that could potentially break this mechanism. It is also a hint to people of where to put their application dependent meta-data.\n\n2. Defining a standard base address for the application-dependent metadata\n- There are also libraries in DPDK that work with application dependent meta-data, currently these are the Packet Framework libraries: librte_port, librte_table, librte_pipeline. Of course, the library does not have the knowledge of the application dependent meta-data format, so they treat it as opaque array of bytes, with the offset and size of the array given as arguments. In my opinion, it is safer (and more elegant) if these libraries (and others) can rely on an mbuf API to access the application dependent meta-data (in an opaque way) rather than make an assumption about the mbuf (i.e. the location of custom metadata relative to the mbuf) that is not clearly supported/defined by the mbuf library. \n- By having this API, we basically say: we define the custom meta-data base address (first location where custom metadata _could_ be placed) immediately after the mbuf, so libraries and apps accessing custom meta-data should do so by using a relative offset from this base rather than each application defining its own base: immediately after mbuf, or 128 bytes after mbuf, or 64 bytes before the end of the buffer, or other.\n\nMore (minor) comments inline below.\n\nThanks,\nCristian\n\n-----Original Message-----\nFrom: Olivier MATZ [mailto:olivier.matz@6wind.com] \nSent: Friday, September 12, 2014 10:02 PM\nTo: Dumitrescu, Cristian; Richardson, Bruce; dev@dpdk.org\nSubject: Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the mbuf metadata\n\nHello Cristian,\n\n> What is the reason to remove this field? Please explain the\n> rationale of removing this field.\n\nThe rationale is explained in\nhttp://dpdk.org/ml/archives/dev/2014-September/005232.html\n\n\"The format of the metadata is up to the application\".\n\nThe type of data the application stores after the mbuf has not\nto be defined in the mbuf. These macros limits the types of\nmetadata to uint8, uint16, uint32, uint64? What should I do\nif I need a void*, a struct foo ? Should we add a macro for\neach possible type?\n\n[Cristian] Actually, this is not correct, as macros to access metadata through pointers (to void or scalar types) are provided as well. This pointer can be converted by the application to the format is defines.\n\n> We previously agreed we need to provide an easy and standard\n> mechanism for applications to extend the mandatory per buffer\n> metadata (mbuf) with optional application-dependent\n> metadata.\n\nDefining a structure in the application which does not pollute\nthe rte_mbuf structure is \"easy and standard(TM)\" too.\n\n[Cristian] I agree, both approaches work the same really, it is just the difference in advertising the presence of meta-data as supported mechanism and defining a standard base address for it.\n\n> This field just provides a clean way for the apps to\n> know where is the end of the mandatory metadata, i.e. the first\n> location in the packet buffer where the app can add its own\n> metadata (of course, the app has to manage the headroom space\n> before the first byte of packet data). A zero-size field is the\n> standard mechanism that DPDK uses extensively in pretty much\n> every library to access memory immediately after a header\n> structure.\n\nHaving the following is clean too:\n\nstruct metadata {\n ...\n};\n\nstruct app_mbuf {\n struct rte_mbuf mbuf;\n struct metadata metadata;\n};\n\nThere is no need to define anything in the mbuf structure.\n\n[Cristian] I agree, both approaches work the same really, it is just the difference in advertising the presence of meta-data as supported mechanism and defining a standard base address for it.\n\n> \n> The impact of removing this field is that there is no standard\n> way to identify where the end of the mandatory metadata is, so\n> each application will have to reinvent this. With no clear\n> convention, we will end up with a lot of non-standard ways. Every\n> time the format of the mbuf structure is going to be changed,\n> this can potentially break applications that use custom metadata,\n> while using this simple standard mechanism would prevent this. So\n> why remove this?\n\nWaow. Five occurences of \"standard\" until now. \n[Cristian] I am sorry :)\n\nCould you give a\nreference to the standard you're refering to? :)\n\n[Cristian] See the IEFT Service Function Chaining link below, the environment is different (data center pipeline vs. CPU core-level pipeline), but the concepts are very similar.\n\nOur application defines private metadata in mbufs in the way described\nabove, we never changed that since we're supporting the dpdk. So\nI don't understand when you say that each time mbuf is reformatted\nit breaks the application.\n\n> Having applications define their optional meta-data is a real\n> need. \n\nSure. This patch does not prevent this at all. You can continue\nto do exactly the same, but in the concerned application, not\nin the generic mbuf structure.\n\n\n> Please take a look at the Service Chaining IEFT emerging\n> protocols (https://datatracker.ietf.org/wg/sfc/documents/), which\n> provide standard mechanisms for applications to define their own\n\nSix :)\n\nI'm not sure these documents define the way to extend a packet\nstructure with metadata in a C program. Again, Bruce's patch does\nnot prevent to do what you need, it just moves it at the proper\nplace.\n\n> packet meta-data and share it between the elements of the\n> processing pipeline (for Service Chaining, these are typically\n> virtual machines scattered amongst the data center).\n> \n> And, in my opinion, there is no negative impact/cost associated\n> with keeping this field.\n\nTo summarize what I think:\n\n- this patch does not prevent to do what you want to do\n- removing the macros help to have a shorter and more comprehensible\n mbuf structure\n- the previous approach does not scale because it would require\n a macro per type\n\n\n> --------------------------------------------------------------\n> Intel Shannon Limited\n> Registered in Ireland\n> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare\n> Registered Number: 308263\n> Business address: Dromore House, East Park, Shannon, Co. Clare\n> \n> 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.\n\nThis is a public mailing list, this disclaimer is invalid.\n\nRegards,\nOlivier\n\n--------------------------------------------------------------\nIntel Shannon Limited\nRegistered in Ireland\nRegistered Office: Collinstown Industrial Park, Leixlip, County Kildare\nRegistered Number: 308263\nBusiness address: Dromore House, East Park, Shannon, Co. Clare\n\nThis 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.", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 6C290AFD1;\n\tTue, 16 Sep 2014 22:02:05 +0200 (CEST)", "from mga11.intel.com (mga11.intel.com [192.55.52.93])\n\tby dpdk.org (Postfix) with ESMTP id 5B3706A1D\n\tfor <dev@dpdk.org>; Tue, 16 Sep 2014 22:02:03 +0200 (CEST)", "from fmsmga002.fm.intel.com ([10.253.24.26])\n\tby fmsmga102.fm.intel.com with ESMTP; 16 Sep 2014 13:07:30 -0700", "from irsmsx104.ger.corp.intel.com ([163.33.3.159])\n\tby fmsmga002.fm.intel.com with ESMTP; 16 Sep 2014 13:07:29 -0700", "from irsmsx108.ger.corp.intel.com ([169.254.11.157]) by\n\tIRSMSX104.ger.corp.intel.com ([163.33.3.159]) with mapi id\n\t14.03.0195.001; Tue, 16 Sep 2014 21:07:27 +0100" ], "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.04,535,1406617200\"; d=\"scan'208\";a=\"600573034\"", "From": "\"Dumitrescu, Cristian\" <cristian.dumitrescu@intel.com>", "To": "Olivier MATZ <olivier.matz@6wind.com>, \"Richardson, Bruce\"\n\t<bruce.richardson@intel.com>, \"dev@dpdk.org\" <dev@dpdk.org>", "Thread-Topic": "[dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "Thread-Index": "AQHPzszq5sDkvNeJ6UiyApy8tXytDZwEKlCA", "Date": "Tue, 16 Sep 2014 20:07:27 +0000", "Message-ID": "<3EB4FA525960D640B5BDFFD6A3D891262E071FE6@IRSMSX108.ger.corp.intel.com>", "References": "<1409759378-10113-1-git-send-email-bruce.richardson@intel.com>\n\t<1409759378-10113-8-git-send-email-bruce.richardson@intel.com>\n\t<540D9B95.3020504@6wind.com>\n\t<59AF69C657FD0841A61C55336867B5B0343EFAA3@IRSMSX103.ger.corp.intel.com>\n\t<3EB4FA525960D640B5BDFFD6A3D891262E070D42@IRSMSX108.ger.corp.intel.com>\n\t<54135F63.2090401@6wind.com>", "In-Reply-To": "<54135F63.2090401@6wind.com>", "Accept-Language": "en-US", "Content-Language": "en-US", "X-MS-Has-Attach": "", "X-MS-TNEF-Correlator": "", "x-originating-ip": "[163.33.239.180]", "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\n\tmbuf metadata", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 809, "web_url": "https://patches.dpdk.org/comment/809/", "msgid": "<59AF69C657FD0841A61C55336867B5B0343F2BD2@IRSMSX103.ger.corp.intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/59AF69C657FD0841A61C55336867B5B0343F2BD2@IRSMSX103.ger.corp.intel.com", "date": "2014-09-17T10:31:35", "subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "submitter": { "id": 20, "url": "https://patches.dpdk.org/api/people/20/?format=api", "name": "Bruce Richardson", "email": "bruce.richardson@intel.com" }, "content": "> -----Original Message-----\n> From: Ramia, Kannan Babu\n> Sent: Tuesday, September 16, 2014 11:06 PM\n> To: Dumitrescu, Cristian; Olivier MATZ; Richardson, Bruce; dev@dpdk.org\n> Subject: RE: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n> mbuf metadata\n> \n> I completely agree with Cristian here, instead of leaving to applications where to\n> place their meta data, we can provide a guidance by having this field about\n> placement of application meta while maintaining transparency on the contents\n> of application meta information.\n> \nMy opinion on this is that this is better served via documentation or a comment in the code. The reason is that this approach is not going to be suitable for all applications. The mbuf headroom being used by the metadata is actually designed to be used for any additional headers to be added to the packet - though other things can obviously be stored in it too. Therefore the amount of metadata that can be stored in it will depend from application to application, as any apps doing e.g. tunnelling will need the headroom for tunnelling headers and may only be able to store a small amount of metadata - potentially none. For larger amounts of metadata - I would feel that anything over 64-bytes or so - I have proposed adding in a separate userdata pointer in the mbuf structure so that apps have the option of storing the metadata externally e.g. pointing to a flow table entry or similar. [Please see mbuf rework patch set 3 proposal].\nBecause of this, I think it's better to put in a comment in the code indicating that metadata can go in the headroom, document this properly - including caveats and limitations - in the documentation, and provide an example of doing such - something we already have in the packet framework.\n\nAll that being said, and while I think this is a good patch, I don't feel too strongly about it. I'm happy enough if this particular patch does not get merged in for 1.8, as it's incidental to the overall mbuf changes.\n\nRegards,\n/Bruce\n\n\n> Regards\n> Kannan Babu Ramia\n> Sr.System Architect\n> Communication Storage Infrastructure Group\n> DCG\n> -----Original Message-----\n> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dumitrescu, Cristian\n> Sent: Wednesday, September 17, 2014 1:37 AM\n> To: Olivier MATZ; Richardson, Bruce; dev@dpdk.org\n> Subject: Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n> mbuf metadata\n> \n> Hi Olivier,\n> \n> I agree that your suggested approach for application-dependent metadata\n> makes sense, in fact the two approaches work in exactly the same way (packet\n> metadata immediately after the regular mbuf), there is only a subtle difference,\n> which is related to defining consistent DPDK usage guidelines.\n> \n> 1. Advertising the presence of application-dependent meta-data as supported\n> mechanism If we explicitly have a metadata zero-size field at the end of the\n> mbuf, we basically tell people that adding their own application meta-data at\n> the end of the mandatory meta-data (mbuf structure) is a mechanism that DPDK\n> allows and supports, and will continue to do so for the foreseeable future. In\n> other words, we guarantee that an application doing so will continue to build\n> successfully with future releases of DPDK, and we will not introduce changes in\n> DPDK that could potentially break this mechanism. It is also a hint to people of\n> where to put their application dependent meta-data.\n> \n> 2. Defining a standard base address for the application-dependent metadata\n> - There are also libraries in DPDK that work with application dependent meta-\n> data, currently these are the Packet Framework libraries: librte_port,\n> librte_table, librte_pipeline. Of course, the library does not have the knowledge\n> of the application dependent meta-data format, so they treat it as opaque array\n> of bytes, with the offset and size of the array given as arguments. In my opinion,\n> it is safer (and more elegant) if these libraries (and others) can rely on an mbuf\n> API to access the application dependent meta-data (in an opaque way) rather\n> than make an assumption about the mbuf (i.e. the location of custom metadata\n> relative to the mbuf) that is not clearly supported/defined by the mbuf library.\n> - By having this API, we basically say: we define the custom meta-data base\n> address (first location where custom metadata _could_ be placed) immediately\n> after the mbuf, so libraries and apps accessing custom meta-data should do so\n> by using a relative offset from this base rather than each application defining its\n> own base: immediately after mbuf, or 128 bytes after mbuf, or 64 bytes before\n> the end of the buffer, or other.\n> \n> More (minor) comments inline below.\n> \n> Thanks,\n> Cristian\n> \n> -----Original Message-----\n> From: Olivier MATZ [mailto:olivier.matz@6wind.com]\n> Sent: Friday, September 12, 2014 10:02 PM\n> To: Dumitrescu, Cristian; Richardson, Bruce; dev@dpdk.org\n> Subject: Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n> mbuf metadata\n> \n> Hello Cristian,\n> \n> > What is the reason to remove this field? Please explain the rationale\n> > of removing this field.\n> \n> The rationale is explained in\n> http://dpdk.org/ml/archives/dev/2014-September/005232.html\n> \n> \"The format of the metadata is up to the application\".\n> \n> The type of data the application stores after the mbuf has not to be defined in\n> the mbuf. These macros limits the types of metadata to uint8, uint16, uint32,\n> uint64? What should I do if I need a void*, a struct foo ? Should we add a macro\n> for each possible type?\n> \n> [Cristian] Actually, this is not correct, as macros to access metadata through\n> pointers (to void or scalar types) are provided as well. This pointer can be\n> converted by the application to the format is defines.\n> \n> > We previously agreed we need to provide an easy and standard mechanism\n> > for applications to extend the mandatory per buffer metadata (mbuf)\n> > with optional application-dependent metadata.\n> \n> Defining a structure in the application which does not pollute the rte_mbuf\n> structure is \"easy and standard(TM)\" too.\n> \n> [Cristian] I agree, both approaches work the same really, it is just the difference\n> in advertising the presence of meta-data as supported mechanism and defining a\n> standard base address for it.\n> \n> > This field just provides a clean way for the apps to know where is the\n> > end of the mandatory metadata, i.e. the first location in the packet\n> > buffer where the app can add its own metadata (of course, the app has\n> > to manage the headroom space before the first byte of packet data). A\n> > zero-size field is the standard mechanism that DPDK uses extensively\n> > in pretty much every library to access memory immediately after a\n> > header structure.\n> \n> Having the following is clean too:\n> \n> struct metadata {\n> ...\n> };\n> \n> struct app_mbuf {\n> struct rte_mbuf mbuf;\n> struct metadata metadata;\n> };\n> \n> There is no need to define anything in the mbuf structure.\n> \n> [Cristian] I agree, both approaches work the same really, it is just the difference\n> in advertising the presence of meta-data as supported mechanism and defining a\n> standard base address for it.\n> \n> >\n> > The impact of removing this field is that there is no standard way to\n> > identify where the end of the mandatory metadata is, so each\n> > application will have to reinvent this. With no clear convention, we\n> > will end up with a lot of non-standard ways. Every time the format of\n> > the mbuf structure is going to be changed, this can potentially break\n> > applications that use custom metadata, while using this simple\n> > standard mechanism would prevent this. So why remove this?\n> \n> Waow. Five occurences of \"standard\" until now.\n> [Cristian] I am sorry :)\n> \n> Could you give a\n> reference to the standard you're refering to? :)\n> \n> [Cristian] See the IEFT Service Function Chaining link below, the environment is\n> different (data center pipeline vs. CPU core-level pipeline), but the concepts are\n> very similar.\n> \n> Our application defines private metadata in mbufs in the way described above,\n> we never changed that since we're supporting the dpdk. So I don't understand\n> when you say that each time mbuf is reformatted it breaks the application.\n> \n> > Having applications define their optional meta-data is a real need.\n> \n> Sure. This patch does not prevent this at all. You can continue to do exactly the\n> same, but in the concerned application, not in the generic mbuf structure.\n> \n> \n> > Please take a look at the Service Chaining IEFT emerging protocols\n> > (https://datatracker.ietf.org/wg/sfc/documents/), which provide\n> > standard mechanisms for applications to define their own\n> \n> Six :)\n> \n> I'm not sure these documents define the way to extend a packet structure with\n> metadata in a C program. Again, Bruce's patch does not prevent to do what you\n> need, it just moves it at the proper place.\n> \n> > packet meta-data and share it between the elements of the processing\n> > pipeline (for Service Chaining, these are typically virtual machines\n> > scattered amongst the data center).\n> >\n> > And, in my opinion, there is no negative impact/cost associated with\n> > keeping this field.\n> \n> To summarize what I think:\n> \n> - this patch does not prevent to do what you want to do\n> - removing the macros help to have a shorter and more comprehensible\n> mbuf structure\n> - the previous approach does not scale because it would require\n> a macro per type\n> \n> \n> > --------------------------------------------------------------\n> > Intel Shannon Limited\n> > Registered in Ireland\n> > Registered Office: Collinstown Industrial Park, Leixlip, County\n> > Kildare Registered Number: 308263 Business address: Dromore House,\n> > East Park, Shannon, Co. Clare\n> >\n> > This e-mail and any attachments may contain confidential material for the sole\n> use of the intended recipient(s). Any review or distribution by others is strictly\n> prohibited. If you are not the intended recipient, please contact the sender and\n> delete all copies.\n> \n> This is a public mailing list, this disclaimer is invalid.\n> \n> Regards,\n> Olivier\n> \n> --------------------------------------------------------------\n> Intel Shannon Limited\n> Registered in Ireland\n> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered\n> Number: 308263 Business address: Dromore House, East Park, Shannon, Co.\n> Clare\n> \n> This e-mail and any attachments may contain confidential material for the sole\n> use of the intended recipient(s). Any review or distribution by others is strictly\n> prohibited. If you are not the intended recipient, please contact the sender and\n> delete all copies.\n>", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id A23A1B39B;\n\tWed, 17 Sep 2014 12:26:59 +0200 (CEST)", "from mga02.intel.com (mga02.intel.com [134.134.136.20])\n\tby dpdk.org (Postfix) with ESMTP id 6085E333\n\tfor <dev@dpdk.org>; Wed, 17 Sep 2014 12:26:57 +0200 (CEST)", "from azsmga001.ch.intel.com ([10.2.17.19])\n\tby orsmga101.jf.intel.com with ESMTP; 17 Sep 2014 03:32:36 -0700", "from irsmsx102.ger.corp.intel.com ([163.33.3.155])\n\tby azsmga001.ch.intel.com with ESMTP; 17 Sep 2014 03:32:34 -0700", "from irsmsx103.ger.corp.intel.com ([169.254.3.112]) by\n\tIRSMSX102.ger.corp.intel.com ([169.254.2.24]) with mapi id\n\t14.03.0195.001; Wed, 17 Sep 2014 11:31:36 +0100" ], "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.04,540,1406617200\"; d=\"scan'208\";a=\"478095754\"", "From": "\"Richardson, Bruce\" <bruce.richardson@intel.com>", "To": "\"Ramia, Kannan Babu\" <kannan.babu.ramia@intel.com>, \"Dumitrescu,\n\tCristian\"\n\t<cristian.dumitrescu@intel.com>, Olivier MATZ <olivier.matz@6wind.com>,\n\t\"dev@dpdk.org\" <dev@dpdk.org>", "Thread-Topic": "[dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "Thread-Index": "AQHPx46wQcWhgcKKB0epuTO3xpr155v3GoqAgAFviACABTV2wIAAOk6AgAY59YCAACFCgIAAziWw", "Date": "Wed, 17 Sep 2014 10:31:35 +0000", "Message-ID": "<59AF69C657FD0841A61C55336867B5B0343F2BD2@IRSMSX103.ger.corp.intel.com>", "References": "<1409759378-10113-1-git-send-email-bruce.richardson@intel.com>\n\t<1409759378-10113-8-git-send-email-bruce.richardson@intel.com>\n\t<540D9B95.3020504@6wind.com>\n\t<59AF69C657FD0841A61C55336867B5B0343EFAA3@IRSMSX103.ger.corp.intel.com>\n\t<3EB4FA525960D640B5BDFFD6A3D891262E070D42@IRSMSX108.ger.corp.intel.com>\n\t<54135F63.2090401@6wind.com>\n\t<3EB4FA525960D640B5BDFFD6A3D891262E071FE6@IRSMSX108.ger.corp.intel.com>\n\t<682698A055A0F44AA47192B20141149711B1FFE6@BGSMSX102.gar.corp.intel.com>", "In-Reply-To": "<682698A055A0F44AA47192B20141149711B1FFE6@BGSMSX102.gar.corp.intel.com>", "Accept-Language": "en-GB, en-US", "Content-Language": "en-US", "X-MS-Has-Attach": "", "X-MS-TNEF-Correlator": "", "x-originating-ip": "[163.33.239.180]", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "quoted-printable", "MIME-Version": "1.0", "Subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 710, "web_url": "https://patches.dpdk.org/comment/710/", "msgid": "<59AF69C657FD0841A61C55336867B5B0343EFAA3@IRSMSX103.ger.corp.intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/59AF69C657FD0841A61C55336867B5B0343EFAA3@IRSMSX103.ger.corp.intel.com", "date": "2014-09-09T09:01:22", "subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "submitter": { "id": 20, "url": "https://patches.dpdk.org/api/people/20/?format=api", "name": "Bruce Richardson", "email": "bruce.richardson@intel.com" }, "content": "> -----Original Message-----\n> From: Olivier MATZ [mailto:olivier.matz@6wind.com]\n> Sent: Monday, September 08, 2014 1:06 PM\n> To: Richardson, Bruce; dev@dpdk.org\n> Subject: Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n> mbuf metadata\n> \n> Hi Bruce,\n> \n> On 09/03/2014 05:49 PM, Bruce Richardson wrote:\n> > Removed the explicit zero-sized metadata definition at the end of the\n> > mbuf data structure. Updated the metadata macros to take account of this\n> > change so that all existing code which uses those macros still works.\n> >\n> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>\n> > ---\n> > lib/librte_mbuf/rte_mbuf.h | 22 ++++++++--------------\n> > 1 file changed, 8 insertions(+), 14 deletions(-)\n> >\n> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h\n> > index 5260001..ca66d9a 100644\n> > --- a/lib/librte_mbuf/rte_mbuf.h\n> > +++ b/lib/librte_mbuf/rte_mbuf.h\n> > @@ -166,31 +166,25 @@ struct rte_mbuf {\n> > \tstruct rte_mempool *pool; /**< Pool from which mbuf was allocated.\n> */\n> > \tstruct rte_mbuf *next; /**< Next segment of scattered packet. */\n> >\n> > -\tunion {\n> > -\t\tuint8_t metadata[0];\n> > -\t\tuint16_t metadata16[0];\n> > -\t\tuint32_t metadata32[0];\n> > -\t\tuint64_t metadata64[0];\n> > -\t} __rte_cache_aligned;\n> > } __rte_cache_aligned;\n> >\n> > #define RTE_MBUF_METADATA_UINT8(mbuf, offset) \\\n> > -\t(mbuf->metadata[offset])\n> > +\t(((uint8_t *)&(mbuf)[1])[offset])\n> > #define RTE_MBUF_METADATA_UINT16(mbuf, offset) \\\n> > -\t(mbuf->metadata16[offset/sizeof(uint16_t)])\n> > +\t(((uint16_t *)&(mbuf)[1])[offset/sizeof(uint16_t)])\n> > #define RTE_MBUF_METADATA_UINT32(mbuf, offset) \\\n> > -\t(mbuf->metadata32[offset/sizeof(uint32_t)])\n> > +\t(((uint32_t *)&(mbuf)[1])[offset/sizeof(uint32_t)])\n> > #define RTE_MBUF_METADATA_UINT64(mbuf, offset) \\\n> > -\t(mbuf->metadata64[offset/sizeof(uint64_t)])\n> > +\t(((uint64_t *)&(mbuf)[1])[offset/sizeof(uint64_t)])\n> >\n> > #define RTE_MBUF_METADATA_UINT8_PTR(mbuf, offset) \\\n> > -\t(&mbuf->metadata[offset])\n> > +\t(&RTE_MBUF_METADATA_UINT8(mbuf, offset))\n> > #define RTE_MBUF_METADATA_UINT16_PTR(mbuf, offset) \\\n> > -\t(&mbuf->metadata16[offset/sizeof(uint16_t)])\n> > +\t(&RTE_MBUF_METADATA_UINT16(mbuf, offset))\n> > #define RTE_MBUF_METADATA_UINT32_PTR(mbuf, offset) \\\n> > -\t(&mbuf->metadata32[offset/sizeof(uint32_t)])\n> > +\t(&RTE_MBUF_METADATA_UINT32(mbuf, offset))\n> > #define RTE_MBUF_METADATA_UINT64_PTR(mbuf, offset) \\\n> > -\t(&mbuf->metadata64[offset/sizeof(uint64_t)])\n> > +\t(&RTE_MBUF_METADATA_UINT64(mbuf, offset))\n> >\n> > /**\n> > * Given the buf_addr returns the pointer to corresponding mbuf.\n> >\n> \n> I think it goes in the good direction. So:\n> Acked-by: Olivier Matz <olivier.matz@6wind.com>\n> \n> Just one question: why not removing RTE_MBUF_METADATA*() macros?\n> I'd just provide one macro that gives a (void*) to the first byte\n> after the mbuf structure.\n> \n> The format of the metadata is up to the application, that usually\n> casts (m + 1) into a private structure, making the macros not very\n> useful. I suggest to move these macros outside rte_mbuf.h, in the\n> application-specific or library-specific header, what do you think?\n> \n> Regards,\n> Olivier\n> \nYes, I'll look into that.\n\n/Bruce", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id E1F61B3B4;\n\tTue, 9 Sep 2014 10:57:19 +0200 (CEST)", "from mga09.intel.com (mga09.intel.com [134.134.136.24])\n\tby dpdk.org (Postfix) with ESMTP id 90146B3B2\n\tfor <dev@dpdk.org>; Tue, 9 Sep 2014 10:57:18 +0200 (CEST)", "from orsmga001.jf.intel.com ([10.7.209.18])\n\tby orsmga102.jf.intel.com with ESMTP; 09 Sep 2014 01:56:15 -0700", "from irsmsx104.ger.corp.intel.com ([163.33.3.159])\n\tby orsmga001.jf.intel.com with ESMTP; 09 Sep 2014 02:01:44 -0700", "from irsmsx103.ger.corp.intel.com ([169.254.3.112]) by\n\tIRSMSX104.ger.corp.intel.com ([163.33.3.159]) with mapi id\n\t14.03.0195.001; Tue, 9 Sep 2014 10:01:23 +0100" ], "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.04,491,1406617200\"; d=\"scan'208\";a=\"570430656\"", "From": "\"Richardson, Bruce\" <bruce.richardson@intel.com>", "To": "Olivier MATZ <olivier.matz@6wind.com>, \"dev@dpdk.org\" <dev@dpdk.org>", "Thread-Topic": "[dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "Thread-Index": "AQHPx46wQcWhgcKKB0epuTO3xpr155v3GoqAgAFviAA=", "Date": "Tue, 9 Sep 2014 09:01:22 +0000", "Message-ID": "<59AF69C657FD0841A61C55336867B5B0343EFAA3@IRSMSX103.ger.corp.intel.com>", "References": "<1409759378-10113-1-git-send-email-bruce.richardson@intel.com>\n\t<1409759378-10113-8-git-send-email-bruce.richardson@intel.com>\n\t<540D9B95.3020504@6wind.com>", "In-Reply-To": "<540D9B95.3020504@6wind.com>", "Accept-Language": "en-GB, en-US", "Content-Language": "en-US", "X-MS-Has-Attach": "", "X-MS-TNEF-Correlator": "", "x-originating-ip": "[163.33.239.180]", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "quoted-printable", "MIME-Version": "1.0", "Subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 716, "web_url": "https://patches.dpdk.org/comment/716/", "msgid": "<20140910150914.GA9656@BRICHA3-MOBL>", "list_archive_url": "https://inbox.dpdk.org/dev/20140910150914.GA9656@BRICHA3-MOBL", "date": "2014-09-10T15:09:14", "subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "submitter": { "id": 20, "url": "https://patches.dpdk.org/api/people/20/?format=api", "name": "Bruce Richardson", "email": "bruce.richardson@intel.com" }, "content": "On Mon, Sep 08, 2014 at 02:05:41PM +0200, Olivier MATZ wrote:\n> Hi Bruce,\n> \n> On 09/03/2014 05:49 PM, Bruce Richardson wrote:\n> > Removed the explicit zero-sized metadata definition at the end of the\n> > mbuf data structure. Updated the metadata macros to take account of this\n> > change so that all existing code which uses those macros still works.\n> > \n> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>\n> > ---\n> > lib/librte_mbuf/rte_mbuf.h | 22 ++++++++--------------\n> > 1 file changed, 8 insertions(+), 14 deletions(-)\n> > \n> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h\n> > index 5260001..ca66d9a 100644\n> > --- a/lib/librte_mbuf/rte_mbuf.h\n> > +++ b/lib/librte_mbuf/rte_mbuf.h\n> > @@ -166,31 +166,25 @@ struct rte_mbuf {\n> > \tstruct rte_mempool *pool; /**< Pool from which mbuf was allocated. */\n> > \tstruct rte_mbuf *next; /**< Next segment of scattered packet. */\n> > \n> > -\tunion {\n> > -\t\tuint8_t metadata[0];\n> > -\t\tuint16_t metadata16[0];\n> > -\t\tuint32_t metadata32[0];\n> > -\t\tuint64_t metadata64[0];\n> > -\t} __rte_cache_aligned;\n> > } __rte_cache_aligned;\n> > \n> > #define RTE_MBUF_METADATA_UINT8(mbuf, offset) \\\n> > -\t(mbuf->metadata[offset])\n> > +\t(((uint8_t *)&(mbuf)[1])[offset])\n> > #define RTE_MBUF_METADATA_UINT16(mbuf, offset) \\\n> > -\t(mbuf->metadata16[offset/sizeof(uint16_t)])\n> > +\t(((uint16_t *)&(mbuf)[1])[offset/sizeof(uint16_t)])\n> > #define RTE_MBUF_METADATA_UINT32(mbuf, offset) \\\n> > -\t(mbuf->metadata32[offset/sizeof(uint32_t)])\n> > +\t(((uint32_t *)&(mbuf)[1])[offset/sizeof(uint32_t)])\n> > #define RTE_MBUF_METADATA_UINT64(mbuf, offset) \\\n> > -\t(mbuf->metadata64[offset/sizeof(uint64_t)])\n> > +\t(((uint64_t *)&(mbuf)[1])[offset/sizeof(uint64_t)])\n> > \n> > #define RTE_MBUF_METADATA_UINT8_PTR(mbuf, offset) \\\n> > -\t(&mbuf->metadata[offset])\n> > +\t(&RTE_MBUF_METADATA_UINT8(mbuf, offset))\n> > #define RTE_MBUF_METADATA_UINT16_PTR(mbuf, offset) \\\n> > -\t(&mbuf->metadata16[offset/sizeof(uint16_t)])\n> > +\t(&RTE_MBUF_METADATA_UINT16(mbuf, offset))\n> > #define RTE_MBUF_METADATA_UINT32_PTR(mbuf, offset) \\\n> > -\t(&mbuf->metadata32[offset/sizeof(uint32_t)])\n> > +\t(&RTE_MBUF_METADATA_UINT32(mbuf, offset))\n> > #define RTE_MBUF_METADATA_UINT64_PTR(mbuf, offset) \\\n> > -\t(&mbuf->metadata64[offset/sizeof(uint64_t)])\n> > +\t(&RTE_MBUF_METADATA_UINT64(mbuf, offset))\n> > \n> > /**\n> > * Given the buf_addr returns the pointer to corresponding mbuf.\n> > \n> \n> I think it goes in the good direction. So:\n> Acked-by: Olivier Matz <olivier.matz@6wind.com>\n> \n> Just one question: why not removing RTE_MBUF_METADATA*() macros?\n> I'd just provide one macro that gives a (void*) to the first byte\n> after the mbuf structure.\n> \n> The format of the metadata is up to the application, that usually\n> casts (m + 1) into a private structure, making the macros not very\n> useful. I suggest to move these macros outside rte_mbuf.h, in the\n> application-specific or library-specific header, what do you think?\n> \n\nThings look to work if I just move the definitions en-masse into rte_port.h. Is that the sort of change you would be thinking of? I was wondering about replacing the typed macros here with a generic one to access just beyond the definition of the mbuf structure, but on further thought, I believe that using the buf_addr pointer in the mbuf data structure is probably enough for most applications. [An alternative to moving the definitions into rte_port.h is to move them into rte_table.h and having port_frag.c use the buf_addr pointer instead of a macro to get at the metadata. All other references to the macros apart from two in that port file, are in the tables or in apps that use the tables lib.]\nWhat do you think?\n\n/Bruce", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id DD83E6829;\n\tWed, 10 Sep 2014 17:08:00 +0200 (CEST)", "from mga09.intel.com (mga09.intel.com [134.134.136.24])\n\tby dpdk.org (Postfix) with ESMTP id 4C5CD32A5\n\tfor <dev@dpdk.org>; Wed, 10 Sep 2014 17:07:58 +0200 (CEST)", "from orsmga002.jf.intel.com ([10.7.209.21])\n\tby orsmga102.jf.intel.com with ESMTP; 10 Sep 2014 08:03:11 -0700", "from bricha3-mobl.ger.corp.intel.com (HELO\n\tbricha3-mobl.ir.intel.com) ([10.243.20.24])\n\tby orsmga002.jf.intel.com with SMTP; 10 Sep 2014 08:09:15 -0700", "by bricha3-mobl.ir.intel.com (sSMTP sendmail emulation);\n\tWed, 10 Sep 2014 16:09:14 +0001" ], "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.04,499,1406617200\"; d=\"scan'208\";a=\"600889981\"", "Date": "Wed, 10 Sep 2014 16:09:14 +0100", "From": "Bruce Richardson <bruce.richardson@intel.com>", "To": "Olivier MATZ <olivier.matz@6wind.com>", "Message-ID": "<20140910150914.GA9656@BRICHA3-MOBL>", "References": "<1409759378-10113-1-git-send-email-bruce.richardson@intel.com>\n\t<1409759378-10113-8-git-send-email-bruce.richardson@intel.com>\n\t<540D9B95.3020504@6wind.com>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=us-ascii", "Content-Disposition": "inline", "In-Reply-To": "<540D9B95.3020504@6wind.com>", "Organization": "Intel Shannon Limited. Registered in Ireland. Registered\n\tOffice: Collinstown Industrial Park, Leixlip,\n\tCounty Kildare. Registered\n\tNumber: 308263. Business address: Dromore House, East Park, Shannon,\n\tCo. Clare.", "User-Agent": "Mutt/1.5.22 (2013-10-16)", "Cc": "dev@dpdk.org", "Subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 798, "web_url": "https://patches.dpdk.org/comment/798/", "msgid": "<682698A055A0F44AA47192B20141149711B1FFE6@BGSMSX102.gar.corp.intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/682698A055A0F44AA47192B20141149711B1FFE6@BGSMSX102.gar.corp.intel.com", "date": "2014-09-16T22:06:29", "subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "submitter": { "id": 71, "url": "https://patches.dpdk.org/api/people/71/?format=api", "name": "Ramia, Kannan Babu", "email": "kannan.babu.ramia@intel.com" }, "content": "I completely agree with Cristian here, instead of leaving to applications where to place their meta data, we can provide a guidance by having this field about placement of application meta while maintaining transparency on the contents of application meta information. \n\nRegards\nKannan Babu Ramia\nSr.System Architect\nCommunication Storage Infrastructure Group\nDCG\n-----Original Message-----\nFrom: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dumitrescu, Cristian\nSent: Wednesday, September 17, 2014 1:37 AM\nTo: Olivier MATZ; Richardson, Bruce; dev@dpdk.org\nSubject: Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the mbuf metadata\n\nHi Olivier,\n\nI agree that your suggested approach for application-dependent metadata makes sense, in fact the two approaches work in exactly the same way (packet metadata immediately after the regular mbuf), there is only a subtle difference, which is related to defining consistent DPDK usage guidelines.\n\n1. Advertising the presence of application-dependent meta-data as supported mechanism If we explicitly have a metadata zero-size field at the end of the mbuf, we basically tell people that adding their own application meta-data at the end of the mandatory meta-data (mbuf structure) is a mechanism that DPDK allows and supports, and will continue to do so for the foreseeable future. In other words, we guarantee that an application doing so will continue to build successfully with future releases of DPDK, and we will not introduce changes in DPDK that could potentially break this mechanism. It is also a hint to people of where to put their application dependent meta-data.\n\n2. Defining a standard base address for the application-dependent metadata\n- There are also libraries in DPDK that work with application dependent meta-data, currently these are the Packet Framework libraries: librte_port, librte_table, librte_pipeline. Of course, the library does not have the knowledge of the application dependent meta-data format, so they treat it as opaque array of bytes, with the offset and size of the array given as arguments. In my opinion, it is safer (and more elegant) if these libraries (and others) can rely on an mbuf API to access the application dependent meta-data (in an opaque way) rather than make an assumption about the mbuf (i.e. the location of custom metadata relative to the mbuf) that is not clearly supported/defined by the mbuf library. \n- By having this API, we basically say: we define the custom meta-data base address (first location where custom metadata _could_ be placed) immediately after the mbuf, so libraries and apps accessing custom meta-data should do so by using a relative offset from this base rather than each application defining its own base: immediately after mbuf, or 128 bytes after mbuf, or 64 bytes before the end of the buffer, or other.\n\nMore (minor) comments inline below.\n\nThanks,\nCristian\n\n-----Original Message-----\nFrom: Olivier MATZ [mailto:olivier.matz@6wind.com]\nSent: Friday, September 12, 2014 10:02 PM\nTo: Dumitrescu, Cristian; Richardson, Bruce; dev@dpdk.org\nSubject: Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the mbuf metadata\n\nHello Cristian,\n\n> What is the reason to remove this field? Please explain the rationale \n> of removing this field.\n\nThe rationale is explained in\nhttp://dpdk.org/ml/archives/dev/2014-September/005232.html\n\n\"The format of the metadata is up to the application\".\n\nThe 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?\n\n[Cristian] Actually, this is not correct, as macros to access metadata through pointers (to void or scalar types) are provided as well. This pointer can be converted by the application to the format is defines.\n\n> We previously agreed we need to provide an easy and standard mechanism \n> for applications to extend the mandatory per buffer metadata (mbuf) \n> with optional application-dependent metadata.\n\nDefining a structure in the application which does not pollute the rte_mbuf structure is \"easy and standard(TM)\" too.\n\n[Cristian] I agree, both approaches work the same really, it is just the difference in advertising the presence of meta-data as supported mechanism and defining a standard base address for it.\n\n> This field just provides a clean way for the apps to know where is the \n> end of the mandatory metadata, i.e. the first location in the packet \n> buffer where the app can add its own metadata (of course, the app has \n> to manage the headroom space before the first byte of packet data). A \n> zero-size field is the standard mechanism that DPDK uses extensively \n> in pretty much every library to access memory immediately after a \n> header structure.\n\nHaving the following is clean too:\n\nstruct metadata {\n ...\n};\n\nstruct app_mbuf {\n struct rte_mbuf mbuf;\n struct metadata metadata;\n};\n\nThere is no need to define anything in the mbuf structure.\n\n[Cristian] I agree, both approaches work the same really, it is just the difference in advertising the presence of meta-data as supported mechanism and defining a standard base address for it.\n\n> \n> The impact of removing this field is that there is no standard way to \n> identify where the end of the mandatory metadata is, so each \n> application will have to reinvent this. With no clear convention, we \n> will end up with a lot of non-standard ways. Every time the format of \n> the mbuf structure is going to be changed, this can potentially break \n> applications that use custom metadata, while using this simple \n> standard mechanism would prevent this. So why remove this?\n\nWaow. Five occurences of \"standard\" until now. \n[Cristian] I am sorry :)\n\nCould you give a\nreference to the standard you're refering to? :)\n\n[Cristian] See the IEFT Service Function Chaining link below, the environment is different (data center pipeline vs. CPU core-level pipeline), but the concepts are very similar.\n\nOur 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.\n\n> Having applications define their optional meta-data is a real need.\n\nSure. 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.\n\n\n> Please take a look at the Service Chaining IEFT emerging protocols \n> (https://datatracker.ietf.org/wg/sfc/documents/), which provide \n> standard mechanisms for applications to define their own\n\nSix :)\n\nI'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.\n\n> packet meta-data and share it between the elements of the processing \n> pipeline (for Service Chaining, these are typically virtual machines \n> scattered amongst the data center).\n> \n> And, in my opinion, there is no negative impact/cost associated with \n> keeping this field.\n\nTo summarize what I think:\n\n- this patch does not prevent to do what you want to do\n- removing the macros help to have a shorter and more comprehensible\n mbuf structure\n- the previous approach does not scale because it would require\n a macro per type\n\n\n> --------------------------------------------------------------\n> Intel Shannon Limited\n> Registered in Ireland\n> Registered Office: Collinstown Industrial Park, Leixlip, County \n> Kildare Registered Number: 308263 Business address: Dromore House, \n> East Park, Shannon, Co. Clare\n> \n> 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.\n\nThis is a public mailing list, this disclaimer is invalid.\n\nRegards,\nOlivier\n\n--------------------------------------------------------------\nIntel Shannon Limited\nRegistered in Ireland\nRegistered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare\n\nThis 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.", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 9DA1FB372;\n\tWed, 17 Sep 2014 00:01:52 +0200 (CEST)", "from mga02.intel.com (mga02.intel.com [134.134.136.20])\n\tby dpdk.org (Postfix) with ESMTP id 77B7CAFD1\n\tfor <dev@dpdk.org>; Wed, 17 Sep 2014 00:01:50 +0200 (CEST)", "from orsmga002.jf.intel.com ([10.7.209.21])\n\tby orsmga101.jf.intel.com with ESMTP; 16 Sep 2014 15:07:01 -0700", "from fmsmsx104.amr.corp.intel.com ([10.18.124.202])\n\tby orsmga002.jf.intel.com with ESMTP; 16 Sep 2014 15:06:36 -0700", "from fmsmsx113.amr.corp.intel.com (10.18.116.7) by\n\tfmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP\n\tServer (TLS) id 14.3.195.1; Tue, 16 Sep 2014 15:06:33 -0700", "from bgsmsx152.gar.corp.intel.com (10.224.48.50) by\n\tFMSMSX113.amr.corp.intel.com (10.18.116.7) with Microsoft SMTP Server\n\t(TLS) id 14.3.195.1; Tue, 16 Sep 2014 15:06:32 -0700", "from bgsmsx102.gar.corp.intel.com ([169.254.2.43]) by\n\tBGSMSX152.gar.corp.intel.com ([169.254.6.175]) with mapi id\n\t14.03.0195.001; Wed, 17 Sep 2014 03:36:30 +0530" ], "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.04,536,1406617200\"; d=\"scan'208\";a=\"603833023\"", "From": "\"Ramia, Kannan Babu\" <kannan.babu.ramia@intel.com>", "To": "\"Dumitrescu, Cristian\" <cristian.dumitrescu@intel.com>, Olivier MATZ\n\t<olivier.matz@6wind.com>, \"Richardson,\n\tBruce\" <bruce.richardson@intel.com>, \"dev@dpdk.org\" <dev@dpdk.org>", "Thread-Topic": "[dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "Thread-Index": "AQHPy11R3D9dFAO/RUGocjSLSkflH5v4JlIAgAU74YCAAESWgIAGOfWAgAB8WLA=", "Date": "Tue, 16 Sep 2014 22:06:29 +0000", "Message-ID": "<682698A055A0F44AA47192B20141149711B1FFE6@BGSMSX102.gar.corp.intel.com>", "References": "<1409759378-10113-1-git-send-email-bruce.richardson@intel.com>\n\t<1409759378-10113-8-git-send-email-bruce.richardson@intel.com>\n\t<540D9B95.3020504@6wind.com>\n\t<59AF69C657FD0841A61C55336867B5B0343EFAA3@IRSMSX103.ger.corp.intel.com>\n\t<3EB4FA525960D640B5BDFFD6A3D891262E070D42@IRSMSX108.ger.corp.intel.com>\n\t<54135F63.2090401@6wind.com>\n\t<3EB4FA525960D640B5BDFFD6A3D891262E071FE6@IRSMSX108.ger.corp.intel.com>", "In-Reply-To": "<3EB4FA525960D640B5BDFFD6A3D891262E071FE6@IRSMSX108.ger.corp.intel.com>", "Accept-Language": "en-US", "Content-Language": "en-US", "X-MS-Has-Attach": "", "X-MS-TNEF-Correlator": "", "x-originating-ip": "[10.223.10.10]", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "quoted-printable", "MIME-Version": "1.0", "Subject": "Re: [dpdk-dev] [PATCH 07/13] mbuf: use macros only to access the\n\tmbuf metadata", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null } ]