Patch Comment List
List patch comments
GET /api/patches/395/comments/?format=api&order=-submitter
[ { "id": 838, "web_url": "http://patches.dpdk.org/comment/838/", "msgid": "<20140917153510.GG4213@localhost.localdomain>", "list_archive_url": "https://inbox.dpdk.org/dev/20140917153510.GG4213@localhost.localdomain", "date": "2014-09-17T15:35:10", "subject": "Re: [dpdk-dev] [PATCH 4/5] mbuf: add userdata pointer field", "submitter": { "id": 32, "url": "http://patches.dpdk.org/api/people/32/?format=api", "name": "Neil Horman", "email": "nhorman@tuxdriver.com" }, "content": "On Wed, Sep 17, 2014 at 11:01:41AM +0100, Bruce Richardson wrote:\n> While some applications may store metadata about packets in the packet\n> mbuf headroom, this is not a workable solution for packet metadata which\n> is either:\n> * larger than the headroom (or headroom is needed for adding pkt headers)\n> * needs to be shared or copied among packets\n> \n> To support these use cases in applications, we reserve a general\n> \"userdata\" pointer field inside the second cache-line of the mbuf. This\n> is better than having the application store the pointer to the external\n> metadata in the packet headroom, as it saves an additional cache-line\n> from being used.\n> \n> Apart from storing metadata, this field also provides a general 8-byte\n> scratch space inside the mbuf for any other application uses that are\n> applicable.\n> \n> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>\n> ---\n> lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h | 3 ++-\n> lib/librte_mbuf/rte_mbuf.h | 3 +++\n> 2 files changed, 5 insertions(+), 1 deletion(-)\n> \n> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h\n> index 25ed672..d27e891 100644\n> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h\n> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h\n> @@ -117,7 +117,8 @@ struct rte_kni_mbuf {\n> \tuint16_t data_len; /**< Amount of data in segment buffer. */\n> \tuint32_t pkt_len; /**< Total pkt len: sum of all segment data_len. */\n> \tchar pad3[8];\n> -\tvoid *pool __attribute__((__aligned__(64)));\n> +\tvoid *pad4 __attribute__((__aligned__(64)));\n> +\tvoid *pool;\nI don't see a comment about this in the changelog, only about the userdata\npointer being added below.\n\n\n> \tvoid *next;\n> };\n> \n> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h\n> index 8e27d2e..b1acfc3 100644\n> --- a/lib/librte_mbuf/rte_mbuf.h\n> +++ b/lib/librte_mbuf/rte_mbuf.h\n> @@ -172,6 +172,9 @@ struct rte_mbuf {\n> \n> \t/* second cache line - fields only used in slow path or on TX */\n> \tMARKER cacheline1 __rte_cache_aligned;\n> +\n> +\tvoid *userdata; /**< Can be used for external metadata */\n> +\nDo you want to make this a void* or a char[8]? I ask because if people are\ngoing to use is as a scratch space (rather than a pointer), they get a suprise\nwhen they build this on 32 bit systems, and their 8 byte scratch space is\nreduced to 4 bytes.\n\nNeil\n\n> \tstruct rte_mempool *pool; /**< Pool from which mbuf was allocated. */\n> \tstruct rte_mbuf *next; /**< Next segment of scattered packet. */\n> \n> -- \n> 1.9.3\n> \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 5E654B3A6;\n\tWed, 17 Sep 2014 17:29:47 +0200 (CEST)", "from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58])\n\tby dpdk.org (Postfix) with ESMTP id 60BB75916\n\tfor <dev@dpdk.org>; Wed, 17 Sep 2014 17:29:44 +0200 (CEST)", "from nat-pool-rdu-u.redhat.com ([66.187.233.203] helo=localhost)\n\tby smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63)\n\t(envelope-from <nhorman@tuxdriver.com>)\n\tid 1XUHGI-0001QY-63; Wed, 17 Sep 2014 11:35:24 -0400" ], "Date": "Wed, 17 Sep 2014 11:35:10 -0400", "From": "Neil Horman <nhorman@tuxdriver.com>", "To": "Bruce Richardson <bruce.richardson@intel.com>", "Message-ID": "<20140917153510.GG4213@localhost.localdomain>", "References": "<1410948102-12740-1-git-send-email-bruce.richardson@intel.com>\n\t<1410948102-12740-5-git-send-email-bruce.richardson@intel.com>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=us-ascii", "Content-Disposition": "inline", "In-Reply-To": "<1410948102-12740-5-git-send-email-bruce.richardson@intel.com>", "User-Agent": "Mutt/1.5.23 (2014-03-12)", "X-Spam-Score": "-2.9 (--)", "X-Spam-Status": "No", "Cc": "dev@dpdk.org", "Subject": "Re: [dpdk-dev] [PATCH 4/5] mbuf: add userdata pointer field", "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": 843, "web_url": "http://patches.dpdk.org/comment/843/", "msgid": "<20140917182951.GB13492@hmsreliant.think-freely.org>", "list_archive_url": "https://inbox.dpdk.org/dev/20140917182951.GB13492@hmsreliant.think-freely.org", "date": "2014-09-17T18:29:51", "subject": "Re: [dpdk-dev] [PATCH 4/5] mbuf: add userdata pointer field", "submitter": { "id": 32, "url": "http://patches.dpdk.org/api/people/32/?format=api", "name": "Neil Horman", "email": "nhorman@tuxdriver.com" }, "content": "On Wed, Sep 17, 2014 at 04:02:07PM +0000, Richardson, Bruce wrote:\n> > -----Original Message-----\n> > From: Neil Horman [mailto:nhorman@tuxdriver.com]\n> > Sent: Wednesday, September 17, 2014 4:35 PM\n> > To: Richardson, Bruce\n> > Cc: dev@dpdk.org\n> > Subject: Re: [dpdk-dev] [PATCH 4/5] mbuf: add userdata pointer field\n> > \n> > On Wed, Sep 17, 2014 at 11:01:41AM +0100, Bruce Richardson wrote:\n> > > While some applications may store metadata about packets in the packet\n> > > mbuf headroom, this is not a workable solution for packet metadata which\n> > > is either:\n> > > * larger than the headroom (or headroom is needed for adding pkt headers)\n> > > * needs to be shared or copied among packets\n> > >\n> > > To support these use cases in applications, we reserve a general\n> > > \"userdata\" pointer field inside the second cache-line of the mbuf. This\n> > > is better than having the application store the pointer to the external\n> > > metadata in the packet headroom, as it saves an additional cache-line\n> > > from being used.\n> > >\n> > > Apart from storing metadata, this field also provides a general 8-byte\n> > > scratch space inside the mbuf for any other application uses that are\n> > > applicable.\n> > >\n> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>\n> > > ---\n> > > lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h | 3 ++-\n> > > lib/librte_mbuf/rte_mbuf.h | 3 +++\n> > > 2 files changed, 5 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h\n> > b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h\n> > > index 25ed672..d27e891 100644\n> > > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h\n> > > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h\n> > > @@ -117,7 +117,8 @@ struct rte_kni_mbuf {\n> > > \tuint16_t data_len; /**< Amount of data in segment buffer. */\n> > > \tuint32_t pkt_len; /**< Total pkt len: sum of all segment data_len. */\n> > > \tchar pad3[8];\n> > > -\tvoid *pool __attribute__((__aligned__(64)));\n> > > +\tvoid *pad4 __attribute__((__aligned__(64)));\n> > > +\tvoid *pool;\n> > I don't see a comment about this in the changelog, only about the userdata\n> > pointer being added below.\n> \n> Yes, this is the userdata pointer - just added as padding here, since it's not actually needed by the kernel-side KNI module.\n> \nAh, then maybe merge it with the pad3 pointer, to make it look a bit better?\n\n> > \n> > \n> > > \tvoid *next;\n> > > };\n> > >\n> > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h\n> > > index 8e27d2e..b1acfc3 100644\n> > > --- a/lib/librte_mbuf/rte_mbuf.h\n> > > +++ b/lib/librte_mbuf/rte_mbuf.h\n> > > @@ -172,6 +172,9 @@ struct rte_mbuf {\n> > >\n> > > \t/* second cache line - fields only used in slow path or on TX */\n> > > \tMARKER cacheline1 __rte_cache_aligned;\n> > > +\n> > > +\tvoid *userdata; /**< Can be used for external metadata */\n> > > +\n> > Do you want to make this a void* or a char[8]? I ask because if people are\n> > going to use is as a scratch space (rather than a pointer), they get a suprise\n> > when they build this on 32 bit systems, and their 8 byte scratch space is\n> > reduced to 4 bytes.\n> \n> I think this is better as a pointer, as that is how it is likely to be used. As for 32-bit, I'm torn between wanting to just update the comment and feeling the need to update the code to actually make the thing 8-byte on 32-bit! Changing the comment to be more accurate is easier, unions are ugly looking in the structure IMHO, so maybe I'll just mark the following field (pool) as always 8-byte aligned....\n> \nOk, however you want to bring it into alignment, as long as its clear that its\neither 8 bytes always, or a variable size based on the arch.\n\nBest\nNeil\n\n> /Bruce\n> \n> > \n> > Neil\n> > \n> > > \tstruct rte_mempool *pool; /**< Pool from which mbuf was allocated.\n> > */\n> > > \tstruct rte_mbuf *next; /**< Next segment of scattered packet. */\n> > >\n> > > --\n> > > 1.9.3\n> > >\n> > >\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 9B344B39E;\n\tWed, 17 Sep 2014 20:24:22 +0200 (CEST)", "from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58])\n\tby dpdk.org (Postfix) with ESMTP id A5FF3B39C\n\tfor <dev@dpdk.org>; Wed, 17 Sep 2014 20:24:21 +0200 (CEST)", "from hmsreliant.think-freely.org\n\t([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost)\n\tby smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63)\n\t(envelope-from <nhorman@tuxdriver.com>)\n\tid 1XUJzD-0002go-Vd; Wed, 17 Sep 2014 14:29:58 -0400" ], "Date": "Wed, 17 Sep 2014 14:29:51 -0400", "From": "Neil Horman <nhorman@tuxdriver.com>", "To": "\"Richardson, Bruce\" <bruce.richardson@intel.com>", "Message-ID": "<20140917182951.GB13492@hmsreliant.think-freely.org>", "References": "<1410948102-12740-1-git-send-email-bruce.richardson@intel.com>\n\t<1410948102-12740-5-git-send-email-bruce.richardson@intel.com>\n\t<20140917153510.GG4213@localhost.localdomain>\n\t<59AF69C657FD0841A61C55336867B5B0343F2F91@IRSMSX103.ger.corp.intel.com>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=us-ascii", "Content-Disposition": "inline", "In-Reply-To": "<59AF69C657FD0841A61C55336867B5B0343F2F91@IRSMSX103.ger.corp.intel.com>", "User-Agent": "Mutt/1.5.23 (2014-03-12)", "X-Spam-Score": "-2.9 (--)", "X-Spam-Status": "No", "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>", "Subject": "Re: [dpdk-dev] [PATCH 4/5] mbuf: add userdata pointer field", "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": 840, "web_url": "http://patches.dpdk.org/comment/840/", "msgid": "<59AF69C657FD0841A61C55336867B5B0343F2F91@IRSMSX103.ger.corp.intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/59AF69C657FD0841A61C55336867B5B0343F2F91@IRSMSX103.ger.corp.intel.com", "date": "2014-09-17T16:02:07", "subject": "Re: [dpdk-dev] [PATCH 4/5] mbuf: add userdata pointer field", "submitter": { "id": 20, "url": "http://patches.dpdk.org/api/people/20/?format=api", "name": "Bruce Richardson", "email": "bruce.richardson@intel.com" }, "content": "> -----Original Message-----\n> From: Neil Horman [mailto:nhorman@tuxdriver.com]\n> Sent: Wednesday, September 17, 2014 4:35 PM\n> To: Richardson, Bruce\n> Cc: dev@dpdk.org\n> Subject: Re: [dpdk-dev] [PATCH 4/5] mbuf: add userdata pointer field\n> \n> On Wed, Sep 17, 2014 at 11:01:41AM +0100, Bruce Richardson wrote:\n> > While some applications may store metadata about packets in the packet\n> > mbuf headroom, this is not a workable solution for packet metadata which\n> > is either:\n> > * larger than the headroom (or headroom is needed for adding pkt headers)\n> > * needs to be shared or copied among packets\n> >\n> > To support these use cases in applications, we reserve a general\n> > \"userdata\" pointer field inside the second cache-line of the mbuf. This\n> > is better than having the application store the pointer to the external\n> > metadata in the packet headroom, as it saves an additional cache-line\n> > from being used.\n> >\n> > Apart from storing metadata, this field also provides a general 8-byte\n> > scratch space inside the mbuf for any other application uses that are\n> > applicable.\n> >\n> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>\n> > ---\n> > lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h | 3 ++-\n> > lib/librte_mbuf/rte_mbuf.h | 3 +++\n> > 2 files changed, 5 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h\n> b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h\n> > index 25ed672..d27e891 100644\n> > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h\n> > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h\n> > @@ -117,7 +117,8 @@ struct rte_kni_mbuf {\n> > \tuint16_t data_len; /**< Amount of data in segment buffer. */\n> > \tuint32_t pkt_len; /**< Total pkt len: sum of all segment data_len. */\n> > \tchar pad3[8];\n> > -\tvoid *pool __attribute__((__aligned__(64)));\n> > +\tvoid *pad4 __attribute__((__aligned__(64)));\n> > +\tvoid *pool;\n> I don't see a comment about this in the changelog, only about the userdata\n> pointer being added below.\n\nYes, this is the userdata pointer - just added as padding here, since it's not actually needed by the kernel-side KNI module.\n\n> \n> \n> > \tvoid *next;\n> > };\n> >\n> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h\n> > index 8e27d2e..b1acfc3 100644\n> > --- a/lib/librte_mbuf/rte_mbuf.h\n> > +++ b/lib/librte_mbuf/rte_mbuf.h\n> > @@ -172,6 +172,9 @@ struct rte_mbuf {\n> >\n> > \t/* second cache line - fields only used in slow path or on TX */\n> > \tMARKER cacheline1 __rte_cache_aligned;\n> > +\n> > +\tvoid *userdata; /**< Can be used for external metadata */\n> > +\n> Do you want to make this a void* or a char[8]? I ask because if people are\n> going to use is as a scratch space (rather than a pointer), they get a suprise\n> when they build this on 32 bit systems, and their 8 byte scratch space is\n> reduced to 4 bytes.\n\nI think this is better as a pointer, as that is how it is likely to be used. As for 32-bit, I'm torn between wanting to just update the comment and feeling the need to update the code to actually make the thing 8-byte on 32-bit! Changing the comment to be more accurate is easier, unions are ugly looking in the structure IMHO, so maybe I'll just mark the following field (pool) as always 8-byte aligned....\n\n/Bruce\n\n> \n> Neil\n> \n> > \tstruct rte_mempool *pool; /**< Pool from which mbuf was allocated.\n> */\n> > \tstruct rte_mbuf *next; /**< Next segment of scattered packet. */\n> >\n> > --\n> > 1.9.3\n> >\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 043EFB3AC;\n\tWed, 17 Sep 2014 17:59:43 +0200 (CEST)", "from mga02.intel.com (mga02.intel.com [134.134.136.20])\n\tby dpdk.org (Postfix) with ESMTP id B07D5B3A5\n\tfor <dev@dpdk.org>; Wed, 17 Sep 2014 17:59:41 +0200 (CEST)", "from orsmga001.jf.intel.com ([10.7.209.18])\n\tby orsmga101.jf.intel.com with ESMTP; 17 Sep 2014 09:03:51 -0700", "from irsmsx103.ger.corp.intel.com ([163.33.3.157])\n\tby orsmga001.jf.intel.com with ESMTP; 17 Sep 2014 09:02:08 -0700", "from irsmsx152.ger.corp.intel.com (163.33.192.66) by\n\tIRSMSX103.ger.corp.intel.com (163.33.3.157) with Microsoft SMTP\n\tServer (TLS) id 14.3.195.1; Wed, 17 Sep 2014 17:02:07 +0100", "from irsmsx103.ger.corp.intel.com ([169.254.3.112]) by\n\tIRSMSX152.ger.corp.intel.com ([169.254.6.48]) with mapi id\n\t14.03.0195.001; Wed, 17 Sep 2014 17:02:08 +0100" ], "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.04,540,1406617200\"; d=\"scan'208\";a=\"574492978\"", "From": "\"Richardson, Bruce\" <bruce.richardson@intel.com>", "To": "Neil Horman <nhorman@tuxdriver.com>", "Thread-Topic": "[dpdk-dev] [PATCH 4/5] mbuf: add userdata pointer field", "Thread-Index": "AQHP0l6Is28HwrbvRkaWBwsjE0afXZwFZG0AgAAWEJA=", "Date": "Wed, 17 Sep 2014 16:02:07 +0000", "Message-ID": "<59AF69C657FD0841A61C55336867B5B0343F2F91@IRSMSX103.ger.corp.intel.com>", "References": "<1410948102-12740-1-git-send-email-bruce.richardson@intel.com>\n\t<1410948102-12740-5-git-send-email-bruce.richardson@intel.com>\n\t<20140917153510.GG4213@localhost.localdomain>", "In-Reply-To": "<20140917153510.GG4213@localhost.localdomain>", "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", "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>", "Subject": "Re: [dpdk-dev] [PATCH 4/5] mbuf: add userdata pointer field", "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 } ]