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.
> 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[] = {
Best regards,
Tiwei Bie
