List comments

GET /api/patches/313/comments/?order=submitter
HTTP 200 OK
Allow: GET, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

[
    {
        "id": 1360,
        "web_url": "https://patches.dpdk.org/comment/1360/",
        "msgid": "<542BEA28.3030300@6wind.com>",
        "date": "2014-10-01T11:48:56",
        "subject": "Re: [dpdk-dev] [PATCH] virtio: Fix vring entry number issue",
        "submitter": {
            "id": 8,
            "url": "https://patches.dpdk.org/api/people/8/",
            "name": "Olivier Matz",
            "email": "olivier.matz@6wind.com"
        },
        "content": "Hello,\n\nOn 09/04/2014 08:34 AM, Ouyang Changchun wrote:\n> Fix one issue in virtio TX: it needs one more vring entry to hold\n> the virtio header when transmitting packets, it is used later to\n> determine whether to free more entries from used vring.\n>\n> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>\n> Reviewed-by: Huawei Xie <huawei.xie@intel.com>\n> Tested-by: Jingguo Fu <jingguox.fu@intel.com>\n> ---\n>   lib/librte_pmd_virtio/virtio_rxtx.c | 3 ++-\n>   1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/lib/librte_pmd_virtio/virtio_rxtx.c b/lib/librte_pmd_virtio/virtio_rxtx.c\n> index 0b10108..b1c189c 100644\n> --- a/lib/librte_pmd_virtio/virtio_rxtx.c\n> +++ b/lib/librte_pmd_virtio/virtio_rxtx.c\n> @@ -699,7 +699,8 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)\n>   \tnum = (uint16_t)(likely(nb_used < VIRTIO_MBUF_BURST_SZ) ? nb_used : VIRTIO_MBUF_BURST_SZ);\n>\n>   \twhile (nb_tx < nb_pkts) {\n> -\t\tint need = tx_pkts[nb_tx]->pkt.nb_segs - txvq->vq_free_cnt;\n> +\t\t/* Need one more entry for virtio header. */\n> +\t\tint need = tx_pkts[nb_tx]->pkt.nb_segs - txvq->vq_free_cnt + 1;\n>   \t\tint deq_cnt = RTE_MIN(need, (int)num);\n>\n>   \t\tnum -= (deq_cnt > 0) ? deq_cnt : 0;\n>\n\n\nIn the commit log, you talk about vring entries, but to me it seems that\nit is about virtio descriptors.\n\nI think it could be useful to explain what was the consequence of this\nissue. Is it a performance issue? In my understanding, the problem is\nthat we won't dequeue mbufs from the \"used\" vring, resulting in a\npossible \"blocking\" of the queue. Is it correct? Also, I think it would\nhelp the review to explain in which conditions the problem is triggered\nand how the fix was tested.\n\nLast comment, I'm wondering if the next test should also be modified:\n\n\tif (tx_pkts[nb_tx]->nb_segs <= txvq->vq_free_cnt) {\n\nInto:\n\n\tif (tx_pkts[nb_tx]->nb_segs <= txvq->vq_free_cnt + 1) {\n\n\t(or maybe using the \"need\" variable)\n\nDo you agree?\n\nRegards,\nOlivier",
        "headers": {
            "X-Mailman-Version": "2.1.15",
            "Date": "Wed, 01 Oct 2014 13:48:56 +0200",
            "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "In-Reply-To": "<1409812499-15656-1-git-send-email-changchun.ouyang@intel.com>",
            "Errors-To": "dev-bounces@dpdk.org",
            "Received": [
                "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id DC988212;\n\tWed,  1 Oct 2014 13:42:13 +0200 (CEST)",
                "from mail-wi0-f180.google.com (mail-wi0-f180.google.com\n\t[209.85.212.180]) by dpdk.org (Postfix) with ESMTP id 966EC71\n\tfor <dev@dpdk.org>; Wed,  1 Oct 2014 13:42:12 +0200 (CEST)",
                "by mail-wi0-f180.google.com with SMTP id em10so286811wid.7\n\tfor <dev@dpdk.org>; Wed, 01 Oct 2014 04:48:57 -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\tt1sm18317840wiy.8.2014.10.01.04.48.56 for <multiple recipients>\n\t(version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128);\n\tWed, 01 Oct 2014 04:48:57 -0700 (PDT)"
            ],
            "References": "<1409812499-15656-1-git-send-email-changchun.ouyang@intel.com>",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "MIME-Version": "1.0",
            "X-Received": "by 10.194.103.200 with SMTP id fy8mr4189534wjb.123.1412164137491;\n\tWed, 01 Oct 2014 04:48:57 -0700 (PDT)",
            "Message-ID": "<542BEA28.3030300@6wind.com>",
            "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:subject:references:in-reply-to:content-type\n\t:content-transfer-encoding;\n\tbh=YI6yrXyx3LkxI/FFNBUG5/wjRyRsBRKNNLUFw8hbfbE=;\n\tb=I7CSSy4aNG+rgE9JL1PCBljB/N7xHOlO8jpj7UDSRFNqN3Z2w7IwdD7gGiXs2fCYmz\n\tD2ZJyG1NsY6lB3EqXHzXXboIq+kNqFhdOqY1Dbt9+Yd9Uyxy9jmIalcmCzmWS0IpDnME\n\tnztmJQsfilQCzOYJXmfV4W0t8v5cgDFVTpxve49+ZQARtGuW8Tue0196+zcxjFVGwlaX\n\trXaCOK8NSy9Q/lgHkG7sK4VHCWMZAzYeARPLJ3RHLLbiWj+IKmW2eIkPjlsLK4q3d4zo\n\tnuyzvajWlRGHxphUvNr5LI6SEC4OS8Kdb2Lo31n13WUpQCJbWLQ4HNqBjCR2kxnmNTPa\n\tWRRQ==",
            "Delivered-To": "patchwork@dpdk.org",
            "Precedence": "list",
            "From": "Olivier MATZ <olivier.matz@6wind.com>",
            "X-Original-To": "patchwork@dpdk.org",
            "Content-Type": "text/plain; charset=ISO-8859-1; format=flowed",
            "List-Post": "<mailto:dev@dpdk.org>",
            "Return-Path": "<dev-bounces@dpdk.org>",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "User-Agent": "Mozilla/5.0 (X11; Linux x86_64;\n\trv:24.0) Gecko/20100101 Icedove/24.5.0",
            "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "To": "Ouyang Changchun <changchun.ouyang@intel.com>, dev@dpdk.org",
            "List-Id": "patches and discussions about DPDK <dev.dpdk.org>",
            "X-Gm-Message-State": "ALoCoQlwqb5SdX5U+eeUZRZ6VK/LOOWNx3LRjCpGd7KyIKExIgU1Cxy0Pfq2Ka6NJ3Lw6AAa48wv",
            "X-BeenThere": "dev@dpdk.org",
            "Content-Transfer-Encoding": "7bit",
            "List-Archive": "<http://dpdk.org/ml/archives/dev/>",
            "Subject": "Re: [dpdk-dev] [PATCH] virtio: Fix vring entry number issue"
        }
    },
    {
        "id": 1576,
        "web_url": "https://patches.dpdk.org/comment/1576/",
        "msgid": "<F52918179C57134FAEC9EA62FA2F962511860C0B@shsmsx102.ccr.corp.intel.com>",
        "date": "2014-10-11T08:22:07",
        "subject": "Re: [dpdk-dev] [PATCH] virtio: Fix vring entry number issue",
        "submitter": {
            "id": 31,
            "url": "https://patches.dpdk.org/api/people/31/",
            "name": "Ouyang Changchun",
            "email": "changchun.ouyang@intel.com"
        },
        "content": "Hi Olivier,\n\nI have v2 patch according to your comments.\nPlease Ack it if you can.\n\nThanks,\nChangchun\n\n> -----Original Message-----\n> From: Olivier MATZ [mailto:olivier.matz@6wind.com]\n> Sent: Wednesday, October 1, 2014 7:49 PM\n> To: Ouyang, Changchun; dev@dpdk.org\n> Subject: Re: [dpdk-dev] [PATCH] virtio: Fix vring entry number issue\n> \n> Hello,\n> \n> On 09/04/2014 08:34 AM, Ouyang Changchun wrote:\n> > Fix one issue in virtio TX: it needs one more vring entry to hold the\n> > virtio header when transmitting packets, it is used later to determine\n> > whether to free more entries from used vring.\n> >\n> > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>\n> > Reviewed-by: Huawei Xie <huawei.xie@intel.com>\n> > Tested-by: Jingguo Fu <jingguox.fu@intel.com>\n> > ---\n> >   lib/librte_pmd_virtio/virtio_rxtx.c | 3 ++-\n> >   1 file changed, 2 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/lib/librte_pmd_virtio/virtio_rxtx.c\n> > b/lib/librte_pmd_virtio/virtio_rxtx.c\n> > index 0b10108..b1c189c 100644\n> > --- a/lib/librte_pmd_virtio/virtio_rxtx.c\n> > +++ b/lib/librte_pmd_virtio/virtio_rxtx.c\n> > @@ -699,7 +699,8 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf\n> **tx_pkts, uint16_t nb_pkts)\n> >   \tnum = (uint16_t)(likely(nb_used < VIRTIO_MBUF_BURST_SZ) ?\n> nb_used :\n> > VIRTIO_MBUF_BURST_SZ);\n> >\n> >   \twhile (nb_tx < nb_pkts) {\n> > -\t\tint need = tx_pkts[nb_tx]->pkt.nb_segs - txvq-\n> >vq_free_cnt;\n> > +\t\t/* Need one more entry for virtio header. */\n> > +\t\tint need = tx_pkts[nb_tx]->pkt.nb_segs - txvq->vq_free_cnt\n> + 1;\n> >   \t\tint deq_cnt = RTE_MIN(need, (int)num);\n> >\n> >   \t\tnum -= (deq_cnt > 0) ? deq_cnt : 0;\n> >\n> \n> \n> In the commit log, you talk about vring entries, but to me it seems that it is\n> about virtio descriptors.\n> \nAgree, but in current virtio pmd, one entry has only one descriptor, so both are ok.\nTo be more accurate, in v2 patch I adopt your suggestion. :-)\n\n> I think it could be useful to explain what was the consequence of this issue. Is\n> it a performance issue? In my understanding, the problem is that we won't\n\nNot only performance issue,  as I state in the v2 patch description, \nIn circumstance of only 1 descriptor in free list, the packet with only 1 segment can't be transmitted\nBecause the transmitting need 2 descriptors(one for packet itself, the other for virtio header), but only 1 freed descriptor available.\nAnd due to the false test condition, no more descriptors will be freed into free list.\n\n> dequeue mbufs from the \"used\" vring, resulting in a possible \"blocking\" of\n> the queue. Is it correct? Also, I think it would help the review to explain in\n> which conditions the problem is triggered and how the fix was tested.\n> \n> Last comment, I'm wondering if the next test should also be modified:\n> \n> \tif (tx_pkts[nb_tx]->nb_segs <= txvq->vq_free_cnt) {\n> \n> Into:\n> \n> \tif (tx_pkts[nb_tx]->nb_segs <= txvq->vq_free_cnt + 1) {\n> \n> \t(or maybe using the \"need\" variable)\n> \n> Do you agree?\n\nAgree, change is made in v2 patch.\n\n> \n> Regards,\n> Olivier",
        "headers": {
            "X-Mailman-Version": "2.1.15",
            "X-MS-TNEF-Correlator": "",
            "X-ExtLoop1": "1",
            "Thread-Topic": "[dpdk-dev] [PATCH] virtio: Fix vring entry number issue",
            "x-originating-ip": "[10.239.127.40]",
            "Received": [
                "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 59AC881B4;\n\tSat, 11 Oct 2014 10:14:42 +0200 (CEST)",
                "from mga01.intel.com (mga01.intel.com [192.55.52.88])\n\tby dpdk.org (Postfix) with ESMTP id D1B3081B3\n\tfor <dev@dpdk.org>; Sat, 11 Oct 2014 10:14:40 +0200 (CEST)",
                "from fmsmga003.fm.intel.com ([10.253.24.29])\n\tby fmsmga101.fm.intel.com with ESMTP; 11 Oct 2014 01:22:09 -0700",
                "from fmsmsx108.amr.corp.intel.com ([10.18.124.206])\n\tby FMSMGA003.fm.intel.com with ESMTP; 11 Oct 2014 01:15:11 -0700",
                "from shsmsx101.ccr.corp.intel.com (10.239.4.153) by\n\tFMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP\n\tServer (TLS) id 14.3.195.1; Sat, 11 Oct 2014 01:22:09 -0700",
                "from shsmsx102.ccr.corp.intel.com ([169.254.2.192]) by\n\tSHSMSX101.ccr.corp.intel.com ([169.254.1.203]) with mapi id\n\t14.03.0195.001; Sat, 11 Oct 2014 16:22:07 +0800"
            ],
            "References": "<1409812499-15656-1-git-send-email-changchun.ouyang@intel.com>\n\t<542BEA28.3030300@6wind.com>",
            "List-Id": "patches and discussions about DPDK <dev.dpdk.org>",
            "MIME-Version": "1.0",
            "Message-ID": "<F52918179C57134FAEC9EA62FA2F962511860C0B@shsmsx102.ccr.corp.intel.com>",
            "Accept-Language": "zh-CN, en-US",
            "Thread-Index": "AQHPyApm8Q/qzFS1ukufcJCf6OrUnpwaxSsAgBABGMA=",
            "X-IronPort-AV": "E=Sophos;i=\"4.97,862,1389772800\"; d=\"scan'208\";a=\"398642468\"",
            "Content-Language": "en-US",
            "Content-Transfer-Encoding": "quoted-printable",
            "From": "\"Ouyang, Changchun\" <changchun.ouyang@intel.com>",
            "X-Original-To": "patchwork@dpdk.org",
            "Content-Type": "text/plain; charset=\"us-ascii\"",
            "List-Post": "<mailto:dev@dpdk.org>",
            "Return-Path": "<dev-bounces@dpdk.org>",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "X-MS-Has-Attach": "",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "In-Reply-To": "<542BEA28.3030300@6wind.com>",
            "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "To": "Olivier MATZ <olivier.matz@6wind.com>, \"dev@dpdk.org\" <dev@dpdk.org>",
            "Precedence": "list",
            "Delivered-To": "patchwork@dpdk.org",
            "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "Errors-To": "dev-bounces@dpdk.org",
            "X-BeenThere": "dev@dpdk.org",
            "Subject": "Re: [dpdk-dev] [PATCH] virtio: Fix vring entry number issue",
            "List-Archive": "<http://dpdk.org/ml/archives/dev/>",
            "Date": "Sat, 11 Oct 2014 08:22:07 +0000"
        }
    }
]