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....@intel.com>
Signed-off-by: Maxime Coquelin <maxime.coque...@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
