Re: [dpdk-dev] [PATCH] virtio: fix the vq size issue

["Xie, Huawei" <huawei.xie@intel.com>]

On 7/2/2015 8:29 AM, Ouyang, Changchun wrote:
> Hi huawei,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Xie, Huawei
>> Sent: Wednesday, July 1, 2015 11:53 PM
>> To: dev@dpdk.org; Thomas Monjalon
>> Subject: Re: [dpdk-dev] [PATCH] virtio: fix the vq size issue
>>
>> On 7/1/2015 3:49 PM, Ouyang Changchun wrote:
>>> This commit breaks virtio basic packets rx functionality:
>>>   d78deadae4dca240e85054bf2d604a801676becc
>>>
>>> The QEMU use 256 as default vring size, also use this default value to
>>> calculate the virtio avail ring base address and used ring base
>>> address, and vhost in the backend use the ring base address to do packet
>> IO.
>>> Virtio spec also says the queue size in PCI configuration is
>>> read-only, so virtio front end can't change it. just need use the
>>> read-only value to allocate space for vring and calculate the avail
>>> and used ring base address. Otherwise, the avail and used ring base
>> address will be different between host and guest, accordingly, packet IO
>> can't work normally.
>> virtio driver could still use the vq_size to initialize avail ring and use ring so
>> that they still have the same base address.
>> The other issue is vhost use  index & (vq->size -1) to index the ring.
> I am not sure what is your clear message here,
> Vhost has no choice but use vq->size -1 to index the ring, 
> It is qemu that always use 256 as the vq size, and set the avail and used ring base address,
> It also tells vhost the vq size is 256.

I mean "the same base address issue" could be resolved, but we still
couldn't stop vhost using idx & vq->size -1 to index the ring.

>>
>> Thomas:
>> This fix works but introduces slight change with original code. Could we just
>> rollback that commit?
> What's your major concern for the slight change here?
> just removing the unnecessary check for nb_desc itself.
> So I think no issue for the slight change.  

No major concern. It is better if this patch just rollbacks that commit
without introduce extra change if not necessary.
The original code set nb_desc to vq_size, though it isn't used later.

>
> Thanks
> Changchun
>
>
>
>