From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by dpdk.org (Postfix) with ESMTP id 621CF1E9C2 for ; Tue, 12 Jun 2018 13:52:05 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BB72580401A8; Tue, 12 Jun 2018 11:52:04 +0000 (UTC) Received: from [10.36.112.45] (ovpn-112-45.ams2.redhat.com [10.36.112.45]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E2BE3111CB82; Tue, 12 Jun 2018 11:52:03 +0000 (UTC) To: Tiwei Bie Cc: zhihong.wang@intel.com, dev@dpdk.org References: <20180607092616.27720-1-maxime.coquelin@redhat.com> <20180607092616.27720-2-maxime.coquelin@redhat.com> <20180612063514.GA14993@debian> From: Maxime Coquelin Message-ID: Date: Tue, 12 Jun 2018 13:52:02 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180612063514.GA14993@debian> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 12 Jun 2018 11:52:04 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 12 Jun 2018 11:52:04 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'maxime.coquelin@redhat.com' RCPT:'' Subject: Re: [dpdk-dev] [PATCH v3 1/5] net/virtio: forbid simple Tx path by default X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Jun 2018 11:52:05 -0000 On 06/12/2018 08:35 AM, Tiwei Bie wrote: > On Thu, Jun 07, 2018 at 11:26:12AM +0200, Maxime Coquelin wrote: >> Simple Tx path is not compliant with the Virtio specification, >> as it assumes the device will use the descriptors in order. >> >> VIRTIO_F_IN_ORDER feature has been introduced recently, but the >> simple Tx path is not compliant with it as VIRTIO_F_IN_ORDER >> requires that chained descriptors are used sequentially, which >> is not the case in simple Tx path. >> >> This patch introduces 'simple_tx_support' devarg to unlock >> Tx simple path selection. >> >> Reported-by: Tiwei Bie >> Signed-off-by: Maxime Coquelin >> --- >> doc/guides/nics/virtio.rst | 9 +++++ >> drivers/net/virtio/virtio_ethdev.c | 73 +++++++++++++++++++++++++++++++++++++- >> drivers/net/virtio/virtio_pci.h | 1 + >> 3 files changed, 82 insertions(+), 1 deletion(-) >> >> diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst >> index 8922f9c0b..53ce1c12a 100644 >> --- a/doc/guides/nics/virtio.rst >> +++ b/doc/guides/nics/virtio.rst >> @@ -222,6 +222,9 @@ Tx callbacks: >> >> #. ``virtio_xmit_pkts_simple``: >> Vector version fixes the available ring indexes to optimize performance. >> + This implementation does not comply with the Virtio specification, and so >> + is not selectable by default. "simple_tx_support=1" devarg must be passed >> + to unlock it. >> >> >> By default, the non-vector callbacks are used: >> @@ -331,3 +334,9 @@ The user can specify below argument in devargs. >> driver, and works as a HW vhost backend. This argument is used to specify >> a virtio device needs to work in vDPA mode. >> (Default: 0 (disabled)) >> + >> +#. ``simple_tx_support``: >> + >> + This argument enables support for the simple Tx path, which is not >> + compliant with the Virtio specification. >> + (Default: 0 (disabled)) > > I tried this patch on my server. Virtio-user will > fail to probe when simple_tx_support is specified > dues to the check in virtio_user_pmd_probe(): > > PMD: Error parsing device, invalid key > virtio_user_pmd_probe(): error when parsing param > vdev_probe(): failed to initialize virtio_user0 device > EAL: Bus (vdev) probe failed. Thanks for the heads-up, I hadn't tried with virtio-user. I'll post a new version with this fixed soon. > >> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c >> index 5833dad73..052dd056a 100644 >> --- a/drivers/net/virtio/virtio_ethdev.c >> +++ b/drivers/net/virtio/virtio_ethdev.c >> @@ -1331,6 +1331,8 @@ set_rxtx_funcs(struct rte_eth_dev *eth_dev) >> if (hw->use_simple_tx) { >> PMD_INIT_LOG(INFO, "virtio: using simple Tx path on port %u", >> eth_dev->data->port_id); >> + PMD_INIT_LOG(WARNING, >> + "virtio: simple Tx path does not comply with Virtio spec"); >> eth_dev->tx_pkt_burst = virtio_xmit_pkts_simple; >> } else { >> PMD_INIT_LOG(INFO, "virtio: using standard Tx path on port %u", >> @@ -1790,6 +1792,66 @@ rte_virtio_pmd_init(void) >> rte_pci_register(&rte_virtio_pmd); >> } >> >> +#define VIRTIO_SIMPLE_TX_SUPPORT "simple_tx_support" >> + >> +static int virtio_dev_args_check(const char *key, const char *val, >> + void *opaque) >> +{ >> + struct rte_eth_dev *dev = opaque; >> + struct virtio_hw *hw = dev->data->dev_private; >> + unsigned long tmp; >> + int ret = 0; >> + >> + errno = 0; >> + tmp = strtoul(val, NULL, 0); >> + if (errno) { >> + PMD_INIT_LOG(INFO, >> + "%s: \"%s\" is not a valid integer", key, val); >> + return errno; >> + } >> + >> + if (strcmp(VIRTIO_SIMPLE_TX_SUPPORT, key) == 0) >> + hw->support_simple_tx = !!tmp; >> + >> + return ret; >> +} >> + >> +static int >> +virtio_dev_args(struct rte_eth_dev *dev) >> +{ >> + struct rte_kvargs *kvlist; >> + struct rte_devargs *devargs; >> + const char *valid_args[] = { >> + VIRTIO_SIMPLE_TX_SUPPORT, >> + NULL, >> + }; > > checkpatch is complaining about above definition: > > WARNING:STATIC_CONST_CHAR_ARRAY: char * array declaration might be > better as static const > #96: FILE: drivers/net/virtio/virtio_ethdev.c:1824: > + const char *valid_args[] = { Yeah, I missed the warning when running my build scripts and noticed it from the upstream CI mail. It will be fixed in next version. Thanks! Maxime > Best regards, > Tiwei Bie >