List comments

GET /api/patches/395/comments/
HTTP 200 OK
Allow: GET, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

[
    {
        "id": 838,
        "web_url": "https://patches.dpdk.org/comment/838/",
        "msgid": "<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": "https://patches.dpdk.org/api/people/32/",
            "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": {
            "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "Cc": "dev@dpdk.org",
            "Content-Type": "text/plain; charset=us-ascii",
            "X-Original-To": "patchwork@dpdk.org",
            "Date": "Wed, 17 Sep 2014 11:35:10 -0400",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "In-Reply-To": "<1410948102-12740-5-git-send-email-bruce.richardson@intel.com>",
            "Precedence": "list",
            "X-BeenThere": "dev@dpdk.org",
            "References": "<1410948102-12740-1-git-send-email-bruce.richardson@intel.com>\n\t<1410948102-12740-5-git-send-email-bruce.richardson@intel.com>",
            "Content-Disposition": "inline",
            "X-Spam-Status": "No",
            "To": "Bruce Richardson <bruce.richardson@intel.com>",
            "User-Agent": "Mutt/1.5.23 (2014-03-12)",
            "Errors-To": "dev-bounces@dpdk.org",
            "MIME-Version": "1.0",
            "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"
            ],
            "From": "Neil Horman <nhorman@tuxdriver.com>",
            "Message-ID": "<20140917153510.GG4213@localhost.localdomain>",
            "List-Post": "<mailto:dev@dpdk.org>",
            "X-Spam-Score": "-2.9 (--)",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "List-Id": "patches and discussions about DPDK <dev.dpdk.org>",
            "Subject": "Re: [dpdk-dev] [PATCH 4/5] mbuf: add userdata pointer field",
            "List-Archive": "<http://dpdk.org/ml/archives/dev/>",
            "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "Return-Path": "<dev-bounces@dpdk.org>",
            "Delivered-To": "patchwork@dpdk.org",
            "X-Mailman-Version": "2.1.15"
        }
    },
    {
        "id": 840,
        "web_url": "https://patches.dpdk.org/comment/840/",
        "msgid": "<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": "https://patches.dpdk.org/api/people/20/",
            "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": {
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "Thread-Index": "AQHP0l6Is28HwrbvRkaWBwsjE0afXZwFZG0AgAAWEJA=",
            "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>",
            "X-MS-Has-Attach": "",
            "X-Original-To": "patchwork@dpdk.org",
            "Date": "Wed, 17 Sep 2014 16:02:07 +0000",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "In-Reply-To": "<20140917153510.GG4213@localhost.localdomain>",
            "Message-ID": "<59AF69C657FD0841A61C55336867B5B0343F2F91@IRSMSX103.ger.corp.intel.com>",
            "X-BeenThere": "dev@dpdk.org",
            "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "Accept-Language": "en-GB, en-US",
            "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>",
            "X-MS-TNEF-Correlator": "",
            "Return-Path": "<dev-bounces@dpdk.org>",
            "x-originating-ip": "[163.33.239.180]",
            "Errors-To": "dev-bounces@dpdk.org",
            "List-Post": "<mailto:dev@dpdk.org>",
            "MIME-Version": "1.0",
            "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"
            ],
            "From": "\"Richardson, Bruce\" <bruce.richardson@intel.com>",
            "Precedence": "list",
            "Delivered-To": "patchwork@dpdk.org",
            "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "Content-Transfer-Encoding": "quoted-printable",
            "Subject": "Re: [dpdk-dev] [PATCH 4/5] mbuf: add userdata pointer field",
            "List-Archive": "<http://dpdk.org/ml/archives/dev/>",
            "To": "Neil Horman <nhorman@tuxdriver.com>",
            "List-Id": "patches and discussions about DPDK <dev.dpdk.org>",
            "Content-Language": "en-US",
            "Content-Type": "text/plain; charset=\"us-ascii\"",
            "X-Mailman-Version": "2.1.15",
            "X-ExtLoop1": "1",
            "Thread-Topic": "[dpdk-dev] [PATCH 4/5] mbuf: add userdata pointer field",
            "X-IronPort-AV": "E=Sophos;i=\"5.04,540,1406617200\"; d=\"scan'208\";a=\"574492978\""
        }
    },
    {
        "id": 843,
        "web_url": "https://patches.dpdk.org/comment/843/",
        "msgid": "<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": "https://patches.dpdk.org/api/people/32/",
            "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": {
            "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>",
            "Content-Type": "text/plain; charset=us-ascii",
            "X-Original-To": "patchwork@dpdk.org",
            "Date": "Wed, 17 Sep 2014 14:29:51 -0400",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "In-Reply-To": "<59AF69C657FD0841A61C55336867B5B0343F2F91@IRSMSX103.ger.corp.intel.com>",
            "Precedence": "list",
            "X-BeenThere": "dev@dpdk.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>",
            "Content-Disposition": "inline",
            "X-Spam-Status": "No",
            "To": "\"Richardson, Bruce\" <bruce.richardson@intel.com>",
            "User-Agent": "Mutt/1.5.23 (2014-03-12)",
            "Errors-To": "dev-bounces@dpdk.org",
            "MIME-Version": "1.0",
            "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"
            ],
            "From": "Neil Horman <nhorman@tuxdriver.com>",
            "Message-ID": "<20140917182951.GB13492@hmsreliant.think-freely.org>",
            "List-Post": "<mailto:dev@dpdk.org>",
            "X-Spam-Score": "-2.9 (--)",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "List-Id": "patches and discussions about DPDK <dev.dpdk.org>",
            "Subject": "Re: [dpdk-dev] [PATCH 4/5] mbuf: add userdata pointer field",
            "List-Archive": "<http://dpdk.org/ml/archives/dev/>",
            "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "Return-Path": "<dev-bounces@dpdk.org>",
            "Delivered-To": "patchwork@dpdk.org",
            "X-Mailman-Version": "2.1.15"
        }
    }
]