[dpdk-dev] [PATCH v3 2/2] virtio/vdev: add a new vdev named eth_cvio

[Yuanhan Liu]

On Thu, Apr 21, 2016 at 02:56:36AM +0000, Jianfeng Tan wrote:
> Add a new virtual device named eth_cvio, it can be used just like
> eth_ring, eth_null, etc.
> 
> Configured parameters include:
>   - rx (optional, 1 by default), number of rx, not used for now.
>   - tx (optional, 1 by default), number of tx, not used for now.
>   - cq (optional, 0 by default), not supported for now.
>   - mac (optional), random value will be given if not specified.
>   - queue_size (optional, 256 by default), size of virtqueue.
>   - path (madatory), path of vhost, depends on the file type, vhost
>     user if the given path points to a unix socket; vhost-net if the
>     given path points to a char device.
>   - ifname (optional), specify the name of backend tap device; only
>     valid when backend is vhost-net.
> 
> The major difference with original virtio for vm is that, here we use
> virtual addr instead of physical addr for vhost to calculate relative
> address.
> 
> When enable CONFIG_RTE_VIRTIO_VDEV (enabled by default), the compiled
> library can be used in both VM and container environment.
> 
> Examples:
> path_vhost=/dev/vhost-net # use vhost-net as a backend
> path_vhost=<path_to_vhost_user> # use vhost-user as a backend
> 
> sudo ./examples/l2fwd/build/l2fwd -c 0x100000 -n 4 \
>     --socket-mem 0,1024 --no-pci --file-prefix=l2fwd \
>     --vdev=eth_cvio0,mac=00:01:02:03:04:05,path=$path_vhost -- -p 0x1
> 
> Known issues:
>  - Control queue and multi-queue are not supported yet.
>  - Cannot work with --huge-unlink.
>  - Cannot work with no-huge.
>  - Cannot work when there are more than VHOST_MEMORY_MAX_NREGIONS(8)
>    hugepages.
>  - Root privilege is a must (mainly becase of sorting hugepages according
>    to physical address).
>  - Applications should not use file name like HUGEFILE_FMT ("%smap_%d").
> 
> Signed-off-by: Huawei Xie <huawei.xie at intel.com>
> Signed-off-by: Jianfeng Tan <jianfeng.tan at intel.com>
> Acked-By: Neil Horman <nhorman at tuxdrver.com>
> ---
>  doc/guides/nics/overview.rst             |  58 +++++-----
>  doc/guides/rel_notes/release_16_07.rst   |   4 +
>  drivers/net/virtio/rte_eth_virtio_vdev.c | 188 ++++++++++++++++++++++++++++++-

Why prefixing it with "rte_eth_..."?

> -   ==================== = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = =
> -   Feature              a b b b c e e e i i i i i i i i i i f f f f m m m n n p r s v v v v x
> -                        f n n o x 1 n n 4 4 4 4 g g x x x x m m m m l l p f u c i z h i i m e
> -                        p x x n g 0 a i 0 0 0 0 b b g g g g 1 1 1 1 x x i p l a n e o r r x n
> -                        a 2 2 d b 0   c e e e e   v b b b b 0 0 0 0 4 5 p   l p g d s t t n v
> -                        c x x i e 0       . v v   f e e e e k k k k     e         a t i i e i
> -                        k   v n           . f f       . v v   . v v               t   o o t r
> -                        e   f g           .   .       . f f   . f f               a     . 3 t
> +   ==================== = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = =
> +   Feature              a b b b c e e e i i i i i i i i i i f f f f m m m n n p r s v v v v x c
> +                        f n n o x 1 n n 4 4 4 4 g g x x x x m m m m l l p f u c i z h i i m e v
> +                        p x x n g 0 a i 0 0 0 0 b b g g g g 1 1 1 1 x x i p l a n e o r r x n i
> +                        a 2 2 d b 0   c e e e e   v b b b b 0 0 0 0 4 5 p   l p g d s t t n v r
> +                        c x x i e 0       . v v   f e e e e k k k k     e         a t i i e i t
> +                        k   v n           . f f       . v v   . v v               t   o o t r i
> +                        e   f g           .   .       . f f   . f f               a     . 3 t o
>                          t                 v   v       v   v   v   v               2     v


I would wish we have a diff that could do compare by columns but not by
rows :)


> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index 68097e6..3b47332 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -264,7 +264,7 @@ struct virtio_hw {
>  #define VHOST_KERNEL	0
>  #define VHOST_USER	1
>  	int		type; /* type of backend */
> -	uint32_t	queue_num;
> +	uint32_t	queue_size;

Hmm, this kind of change should not be squeezed here, stealthily. I
would agree that the rename in decreases the stealthily, which is a
good thing. You should submit a standalone patch instead.

	--yliu