List comments

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

[
    {
        "id": 82076,
        "web_url": "http://patches.dpdk.org/comment/82076/",
        "msgid": "<20180612063514.GA14993@debian>",
        "date": "2018-06-12T06:35:14",
        "subject": "Re: [dpdk-dev] [PATCH v3 1/5] net/virtio: forbid simple Tx path by\n\tdefault",
        "submitter": {
            "id": 617,
            "url": "http://patches.dpdk.org/api/people/617/",
            "name": "Tiwei Bie",
            "email": "tiwei.bie@intel.com"
        },
        "content": "On Thu, Jun 07, 2018 at 11:26:12AM +0200, Maxime Coquelin wrote:\n> Simple Tx path is not compliant with the Virtio specification,\n> as it assumes the device will use the descriptors in order.\n> \n> VIRTIO_F_IN_ORDER feature has been introduced recently, but the\n> simple Tx path is not compliant with it as VIRTIO_F_IN_ORDER\n> requires that chained descriptors are used sequentially, which\n> is not the case in simple Tx path.\n> \n> This patch introduces 'simple_tx_support' devarg to unlock\n> Tx simple path selection.\n> \n> Reported-by: Tiwei Bie <tiwei.bie@intel.com>\n> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>\n> ---\n>  doc/guides/nics/virtio.rst         |  9 +++++\n>  drivers/net/virtio/virtio_ethdev.c | 73 +++++++++++++++++++++++++++++++++++++-\n>  drivers/net/virtio/virtio_pci.h    |  1 +\n>  3 files changed, 82 insertions(+), 1 deletion(-)\n> \n> diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst\n> index 8922f9c0b..53ce1c12a 100644\n> --- a/doc/guides/nics/virtio.rst\n> +++ b/doc/guides/nics/virtio.rst\n> @@ -222,6 +222,9 @@ Tx callbacks:\n>  \n>  #. ``virtio_xmit_pkts_simple``:\n>     Vector version fixes the available ring indexes to optimize performance.\n> +   This implementation does not comply with the Virtio specification, and so\n> +   is not selectable by default. \"simple_tx_support=1\" devarg must be passed\n> +   to unlock it.\n>  \n>  \n>  By default, the non-vector callbacks are used:\n> @@ -331,3 +334,9 @@ The user can specify below argument in devargs.\n>      driver, and works as a HW vhost backend. This argument is used to specify\n>      a virtio device needs to work in vDPA mode.\n>      (Default: 0 (disabled))\n> +\n> +#.  ``simple_tx_support``:\n> +\n> +    This argument enables support for the simple Tx path, which is not\n> +    compliant with the Virtio specification.\n> +    (Default: 0 (disabled))\n\nI tried this patch on my server. Virtio-user will\nfail to probe when simple_tx_support is specified\ndues to the check in virtio_user_pmd_probe():\n\nPMD: Error parsing device, invalid key <simple_tx_support>\nvirtio_user_pmd_probe(): error when parsing param\nvdev_probe(): failed to initialize virtio_user0 device\nEAL: Bus (vdev) probe failed.\n\n\n> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c\n> index 5833dad73..052dd056a 100644\n> --- a/drivers/net/virtio/virtio_ethdev.c\n> +++ b/drivers/net/virtio/virtio_ethdev.c\n> @@ -1331,6 +1331,8 @@ set_rxtx_funcs(struct rte_eth_dev *eth_dev)\n>  \tif (hw->use_simple_tx) {\n>  \t\tPMD_INIT_LOG(INFO, \"virtio: using simple Tx path on port %u\",\n>  \t\t\teth_dev->data->port_id);\n> +\t\tPMD_INIT_LOG(WARNING,\n> +\t\t\t\t\"virtio: simple Tx path does not comply with Virtio spec\");\n>  \t\teth_dev->tx_pkt_burst = virtio_xmit_pkts_simple;\n>  \t} else {\n>  \t\tPMD_INIT_LOG(INFO, \"virtio: using standard Tx path on port %u\",\n> @@ -1790,6 +1792,66 @@ rte_virtio_pmd_init(void)\n>  \trte_pci_register(&rte_virtio_pmd);\n>  }\n>  \n> +#define VIRTIO_SIMPLE_TX_SUPPORT \"simple_tx_support\"\n> +\n> +static int virtio_dev_args_check(const char *key, const char *val,\n> +\t\tvoid *opaque)\n> +{\n> +\tstruct rte_eth_dev *dev = opaque;\n> +\tstruct virtio_hw *hw = dev->data->dev_private;\n> +\tunsigned long tmp;\n> +\tint ret = 0;\n> +\n> +\terrno = 0;\n> +\ttmp = strtoul(val, NULL, 0);\n> +\tif (errno) {\n> +\t\tPMD_INIT_LOG(INFO,\n> +\t\t\t\t\"%s: \\\"%s\\\" is not a valid integer\", key, val);\n> +\t\treturn errno;\n> +\t}\n> +\n> +\tif (strcmp(VIRTIO_SIMPLE_TX_SUPPORT, key) == 0)\n> +\t\thw->support_simple_tx = !!tmp;\n> +\n> +\treturn ret;\n> +}\n> +\n> +static int\n> +virtio_dev_args(struct rte_eth_dev *dev)\n> +{\n> +\tstruct rte_kvargs *kvlist;\n> +\tstruct rte_devargs *devargs;\n> +\tconst char *valid_args[] = {\n> +\t\tVIRTIO_SIMPLE_TX_SUPPORT,\n> +\t\tNULL,\n> +\t};\n\ncheckpatch is complaining about above definition:\n\nWARNING:STATIC_CONST_CHAR_ARRAY: char * array declaration might be\nbetter as static const\n#96: FILE: drivers/net/virtio/virtio_ethdev.c:1824:\n+       const char *valid_args[] = {\n\nBest regards,\nTiwei Bie",
        "headers": {
            "Return-Path": "<dev-bounces@dpdk.org>",
            "References": "<20180607092616.27720-1-maxime.coquelin@redhat.com>\n\t<20180607092616.27720-2-maxime.coquelin@redhat.com>",
            "X-Mailman-Version": "2.1.15",
            "X-IronPort-AV": "E=Sophos;i=\"5.51,213,1526367600\"; d=\"scan'208\";a=\"56382278\"",
            "From": "Tiwei Bie <tiwei.bie@intel.com>",
            "User-Agent": "Mutt/1.9.5 (2018-04-13)",
            "List-Post": "<mailto:dev@dpdk.org>",
            "Content-Type": "text/plain; charset=utf-8",
            "Delivered-To": "patchwork@dpdk.org",
            "X-Original-To": "patchwork@dpdk.org",
            "Received": [
                "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 6CF241E553;\n\tTue, 12 Jun 2018 08:35:10 +0200 (CEST)",
                "from mga04.intel.com (mga04.intel.com [192.55.52.120])\n\tby dpdk.org (Postfix) with ESMTP id A6BF41E4E1\n\tfor <dev@dpdk.org>; Tue, 12 Jun 2018 08:35:09 +0200 (CEST)",
                "from fmsmga002.fm.intel.com ([10.253.24.26])\n\tby fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t11 Jun 2018 23:35:07 -0700",
                "from debian.sh.intel.com (HELO debian) ([10.67.104.228])\n\tby fmsmga002.fm.intel.com with ESMTP; 11 Jun 2018 23:35:07 -0700"
            ],
            "Subject": "Re: [dpdk-dev] [PATCH v3 1/5] net/virtio: forbid simple Tx path by\n\tdefault",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "X-Amp-File-Uploaded": "False",
            "Message-ID": "<20180612063514.GA14993@debian>",
            "X-Amp-Original-Verdict": "FILE UNKNOWN",
            "X-BeenThere": "dev@dpdk.org",
            "Date": "Tue, 12 Jun 2018 14:35:14 +0800",
            "List-Archive": "<http://dpdk.org/ml/archives/dev/>",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "X-ExtLoop1": "1",
            "List-Subscribe": "<https://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "Cc": "zhihong.wang@intel.com, dev@dpdk.org",
            "List-Id": "DPDK patches and discussions <dev.dpdk.org>",
            "Precedence": "list",
            "In-Reply-To": "<20180607092616.27720-2-maxime.coquelin@redhat.com>",
            "Errors-To": "dev-bounces@dpdk.org",
            "List-Unsubscribe": "<https://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "MIME-Version": "1.0",
            "To": "Maxime Coquelin <maxime.coquelin@redhat.com>",
            "X-Amp-Result": "UNKNOWN",
            "Content-Disposition": "inline"
        }
    },
    {
        "id": 82088,
        "web_url": "http://patches.dpdk.org/comment/82088/",
        "msgid": "<d2c0a9d0-5821-c287-fe99-bf232ce91d10@redhat.com>",
        "date": "2018-06-12T11:52:02",
        "subject": "Re: [dpdk-dev] [PATCH v3 1/5] net/virtio: forbid simple Tx path by\n\tdefault",
        "submitter": {
            "id": 512,
            "url": "http://patches.dpdk.org/api/people/512/",
            "name": "Maxime Coquelin",
            "email": "maxime.coquelin@redhat.com"
        },
        "content": "On 06/12/2018 08:35 AM, Tiwei Bie wrote:\n> On Thu, Jun 07, 2018 at 11:26:12AM +0200, Maxime Coquelin wrote:\n>> Simple Tx path is not compliant with the Virtio specification,\n>> as it assumes the device will use the descriptors in order.\n>>\n>> VIRTIO_F_IN_ORDER feature has been introduced recently, but the\n>> simple Tx path is not compliant with it as VIRTIO_F_IN_ORDER\n>> requires that chained descriptors are used sequentially, which\n>> is not the case in simple Tx path.\n>>\n>> This patch introduces 'simple_tx_support' devarg to unlock\n>> Tx simple path selection.\n>>\n>> Reported-by: Tiwei Bie <tiwei.bie@intel.com>\n>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>\n>> ---\n>>   doc/guides/nics/virtio.rst         |  9 +++++\n>>   drivers/net/virtio/virtio_ethdev.c | 73 +++++++++++++++++++++++++++++++++++++-\n>>   drivers/net/virtio/virtio_pci.h    |  1 +\n>>   3 files changed, 82 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst\n>> index 8922f9c0b..53ce1c12a 100644\n>> --- a/doc/guides/nics/virtio.rst\n>> +++ b/doc/guides/nics/virtio.rst\n>> @@ -222,6 +222,9 @@ Tx callbacks:\n>>   \n>>   #. ``virtio_xmit_pkts_simple``:\n>>      Vector version fixes the available ring indexes to optimize performance.\n>> +   This implementation does not comply with the Virtio specification, and so\n>> +   is not selectable by default. \"simple_tx_support=1\" devarg must be passed\n>> +   to unlock it.\n>>   \n>>   \n>>   By default, the non-vector callbacks are used:\n>> @@ -331,3 +334,9 @@ The user can specify below argument in devargs.\n>>       driver, and works as a HW vhost backend. This argument is used to specify\n>>       a virtio device needs to work in vDPA mode.\n>>       (Default: 0 (disabled))\n>> +\n>> +#.  ``simple_tx_support``:\n>> +\n>> +    This argument enables support for the simple Tx path, which is not\n>> +    compliant with the Virtio specification.\n>> +    (Default: 0 (disabled))\n> \n> I tried this patch on my server. Virtio-user will\n> fail to probe when simple_tx_support is specified\n> dues to the check in virtio_user_pmd_probe():\n> \n> PMD: Error parsing device, invalid key <simple_tx_support>\n> virtio_user_pmd_probe(): error when parsing param\n> vdev_probe(): failed to initialize virtio_user0 device\n> EAL: Bus (vdev) probe failed.\n\n\nThanks for the heads-up, I hadn't tried with virtio-user.\nI'll post a new version with this fixed soon.\n\n> \n>> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c\n>> index 5833dad73..052dd056a 100644\n>> --- a/drivers/net/virtio/virtio_ethdev.c\n>> +++ b/drivers/net/virtio/virtio_ethdev.c\n>> @@ -1331,6 +1331,8 @@ set_rxtx_funcs(struct rte_eth_dev *eth_dev)\n>>   \tif (hw->use_simple_tx) {\n>>   \t\tPMD_INIT_LOG(INFO, \"virtio: using simple Tx path on port %u\",\n>>   \t\t\teth_dev->data->port_id);\n>> +\t\tPMD_INIT_LOG(WARNING,\n>> +\t\t\t\t\"virtio: simple Tx path does not comply with Virtio spec\");\n>>   \t\teth_dev->tx_pkt_burst = virtio_xmit_pkts_simple;\n>>   \t} else {\n>>   \t\tPMD_INIT_LOG(INFO, \"virtio: using standard Tx path on port %u\",\n>> @@ -1790,6 +1792,66 @@ rte_virtio_pmd_init(void)\n>>   \trte_pci_register(&rte_virtio_pmd);\n>>   }\n>>   \n>> +#define VIRTIO_SIMPLE_TX_SUPPORT \"simple_tx_support\"\n>> +\n>> +static int virtio_dev_args_check(const char *key, const char *val,\n>> +\t\tvoid *opaque)\n>> +{\n>> +\tstruct rte_eth_dev *dev = opaque;\n>> +\tstruct virtio_hw *hw = dev->data->dev_private;\n>> +\tunsigned long tmp;\n>> +\tint ret = 0;\n>> +\n>> +\terrno = 0;\n>> +\ttmp = strtoul(val, NULL, 0);\n>> +\tif (errno) {\n>> +\t\tPMD_INIT_LOG(INFO,\n>> +\t\t\t\t\"%s: \\\"%s\\\" is not a valid integer\", key, val);\n>> +\t\treturn errno;\n>> +\t}\n>> +\n>> +\tif (strcmp(VIRTIO_SIMPLE_TX_SUPPORT, key) == 0)\n>> +\t\thw->support_simple_tx = !!tmp;\n>> +\n>> +\treturn ret;\n>> +}\n>> +\n>> +static int\n>> +virtio_dev_args(struct rte_eth_dev *dev)\n>> +{\n>> +\tstruct rte_kvargs *kvlist;\n>> +\tstruct rte_devargs *devargs;\n>> +\tconst char *valid_args[] = {\n>> +\t\tVIRTIO_SIMPLE_TX_SUPPORT,\n>> +\t\tNULL,\n>> +\t};\n> \n> checkpatch is complaining about above definition:\n> \n> WARNING:STATIC_CONST_CHAR_ARRAY: char * array declaration might be\n> better as static const\n> #96: FILE: drivers/net/virtio/virtio_ethdev.c:1824:\n> +       const char *valid_args[] = {\n\nYeah, I missed the warning when running my build scripts and noticed\nit from the upstream CI mail.\n\nIt will be fixed in next version.\n\nThanks!\nMaxime\n\n> Best regards,\n> Tiwei Bie\n>",
        "headers": {
            "Return-Path": "<dev-bounces@dpdk.org>",
            "References": "<20180607092616.27720-1-maxime.coquelin@redhat.com>\n\t<20180607092616.27720-2-maxime.coquelin@redhat.com>\n\t<20180612063514.GA14993@debian>",
            "X-Mailman-Version": "2.1.15",
            "X-Scanned-By": "MIMEDefang 2.78 on 10.11.54.3",
            "Date": "Tue, 12 Jun 2018 13:52:02 +0200",
            "User-Agent": "Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.7.0",
            "List-Post": "<mailto:dev@dpdk.org>",
            "Content-Type": "text/plain; charset=utf-8; format=flowed",
            "X-BeenThere": "dev@dpdk.org",
            "X-Original-To": "patchwork@dpdk.org",
            "X-Greylist": [
                "Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.11.55.8]); Tue, 12 Jun 2018 11:52:04 +0000 (UTC)",
                "inspected by milter-greylist-4.5.16 (mx1.redhat.com\n\t[10.11.55.8]); \n\tTue, 12 Jun 2018 11:52:04 +0000 (UTC) for IP:'10.11.54.3'\n\tDOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com'\n\tHELO:'smtp.corp.redhat.com' FROM:'maxime.coquelin@redhat.com' RCPT:''"
            ],
            "Received": [
                "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id DDD411E9C3;\n\tTue, 12 Jun 2018 13:52:06 +0200 (CEST)",
                "from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73])\n\tby dpdk.org (Postfix) with ESMTP id 621CF1E9C2\n\tfor <dev@dpdk.org>; Tue, 12 Jun 2018 13:52:05 +0200 (CEST)",
                "from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id BB72580401A8;\n\tTue, 12 Jun 2018 11:52:04 +0000 (UTC)",
                "from [10.36.112.45] (ovpn-112-45.ams2.redhat.com [10.36.112.45])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id E2BE3111CB82;\n\tTue, 12 Jun 2018 11:52:03 +0000 (UTC)"
            ],
            "Subject": "Re: [dpdk-dev] [PATCH v3 1/5] net/virtio: forbid simple Tx path by\n\tdefault",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "Content-Language": "en-US",
            "Message-ID": "<d2c0a9d0-5821-c287-fe99-bf232ce91d10@redhat.com>",
            "Precedence": "list",
            "From": "Maxime Coquelin <maxime.coquelin@redhat.com>",
            "List-Archive": "<http://dpdk.org/ml/archives/dev/>",
            "Errors-To": "dev-bounces@dpdk.org",
            "List-Subscribe": "<https://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "Cc": "zhihong.wang@intel.com, dev@dpdk.org",
            "List-Id": "DPDK patches and discussions <dev.dpdk.org>",
            "Delivered-To": "patchwork@dpdk.org",
            "In-Reply-To": "<20180612063514.GA14993@debian>",
            "List-Unsubscribe": "<https://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "MIME-Version": "1.0",
            "Content-Transfer-Encoding": "7bit",
            "To": "Tiwei Bie <tiwei.bie@intel.com>"
        }
    }
]