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

[Tan, Jianfeng]

Hi Yuanhan,

On 4/22/2016 6:14 AM, Yuanhan Liu wrote:
> 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_..."?

I was to make align with other virtual devices, like ring, null, etc. 
But like suggested by Thomas and you, I'll rename it.

>
>> -   ==================== = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = =
>> -   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.

Nice catch. I should do it in the first commit when it's defined.

Thank,
Jianfeng



>
> 	--yliu