From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: Tiwei Bie <tiwei.bie@intel.com>
Cc: zhihong.wang@intel.com, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v3 1/5] net/virtio: forbid simple Tx path by default
Date: Tue, 12 Jun 2018 13:52:02 +0200 [thread overview]
Message-ID: <d2c0a9d0-5821-c287-fe99-bf232ce91d10@redhat.com> (raw)
In-Reply-To: <20180612063514.GA14993@debian>
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 <tiwei.bie@intel.com>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>> 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 <simple_tx_support>
> 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
>
next prev parent reply other threads:[~2018-06-12 11:52 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-07 9:26 [dpdk-dev] [PATCH v3 0/5] net/virtio: Tx path selection and offload improvements Maxime Coquelin
2018-06-07 9:26 ` [dpdk-dev] [PATCH v3 1/5] net/virtio: forbid simple Tx path by default Maxime Coquelin
2018-06-12 6:35 ` Tiwei Bie
2018-06-12 11:52 ` Maxime Coquelin [this message]
2018-06-07 9:26 ` [dpdk-dev] [PATCH v3 2/5] net/virtio: use simple path for Tx even if Rx mergeable Maxime Coquelin
2018-06-12 6:18 ` Tiwei Bie
2018-06-07 9:26 ` [dpdk-dev] [PATCH v3 3/5] net/vhost: improve Tx path selection Maxime Coquelin
2018-06-07 9:26 ` [dpdk-dev] [PATCH v3 4/5] net/virtio: don't use simple Rx if TCP LRO or VLAN strip requested Maxime Coquelin
2018-06-07 9:26 ` [dpdk-dev] [PATCH v3 5/5] net/virtio: improve offload check performance Maxime Coquelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d2c0a9d0-5821-c287-fe99-bf232ce91d10@redhat.com \
--to=maxime.coquelin@redhat.com \
--cc=dev@dpdk.org \
--cc=tiwei.bie@intel.com \
--cc=zhihong.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).