[Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Longpeng(Mike)]

Hi guys,

We got a BUG report from our testers yesterday, the testing scenario was
migrating a VM (Windows guest, *4 vcpus*, 4GB, vhost-user net: *7 queues*).

We found the cause reason, and we'll report the BUG or send a fix patch
to upstream if necessary( we haven't test the upstream yet, sorry... ).

We want to know why the vhost_net_start() must start *total queues* ( in our
VM there're 7 queues ) but not *the queues that current used* ( in our VM, guest
only uses the first 4 queues because it's limited by the number of vcpus) ?

Looking forward to your help, thx :)

-- 
Regards,
Longpeng

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Jason Wang]

On 2017年11月15日 22:55, Longpeng(Mike) wrote:
> Hi guys,
>
> We got a BUG report from our testers yesterday, the testing scenario was
> migrating a VM (Windows guest, *4 vcpus*, 4GB, vhost-user net: *7 queues*).
>
> We found the cause reason, and we'll report the BUG or send a fix patch
> to upstream if necessary( we haven't test the upstream yet, sorry... ).

Could you explain this a little bit more?

>
> We want to know why the vhost_net_start() must start *total queues* ( in our
> VM there're 7 queues ) but not *the queues that current used* ( in our VM, guest
> only uses the first 4 queues because it's limited by the number of vcpus) ?
>
> Looking forward to your help, thx :)

Since the codes have been there for years and works well for kernel 
datapath. You should really explain what's wrong.

Thanks

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Longpeng(Mike)]

2017-11-15 23:05 GMT+08:00 Jason Wang <jasowang@redhat.com>:
>
>
> On 2017年11月15日 22:55, Longpeng(Mike) wrote:
>>
>> Hi guys,
>>
>> We got a BUG report from our testers yesterday, the testing scenario was
>> migrating a VM (Windows guest, *4 vcpus*, 4GB, vhost-user net: *7
>> queues*).
>>
>> We found the cause reason, and we'll report the BUG or send a fix patch
>> to upstream if necessary( we haven't test the upstream yet, sorry... ).
>
>
> Could you explain this a little bit more?
>
>>
>> We want to know why the vhost_net_start() must start *total queues* ( in
>> our
>> VM there're 7 queues ) but not *the queues that current used* ( in our VM,
>> guest
>> only uses the first 4 queues because it's limited by the number of vcpus)
>> ?
>>
>> Looking forward to your help, thx :)
>
>
> Since the codes have been there for years and works well for kernel
> datapath. You should really explain what's wrong.
>

OK. :)

In our scenario,  the Windows's virtio-net driver only use the first 4
queues and it
*only set desc/avail/used table for the first 4 queues*, so in QEMU
the desc/avail/
used of the last 3 queues are ZERO,  but unfortunately...
'''
vhost_net_start
  for (i = 0; i < total_queues; i++)
    vhost_net_start_one
      vhost_dev_start
        vhost_virtqueue_start
'''
In vhost_virtqueue_start(), it will calculate the HVA of
desc/avail/used table, so for last
3 queues, it will use ZERO as the GPA to calculate the HVA, and then
send the results
to the user-mode backend ( we use *vhost-user* ) by vhost_virtqueue_set_addr().

When the EVS get these address, it will update a *idx* which will be
treated as  vq's
last_avail_idx when virtio-net stop ( pls see vhost_virtqueue_stop() ).

So we get the following result after virtio-net stop:
  the desc/avail/used of the last 3 queues's vqs are all ZERO, but these vqs's
  last_avail_idx is NOT ZERO.

At last, virtio_load() reports an error:
'''
  if (!vdev->vq[i].vring.desc && vdev->vq[i].last_avail_idx) { // <--
will be TRUE
      error_report("VQ %d address 0x0 "
                         "inconsistent with Host index 0x%x",
                         i, vdev->vq[i].last_avail_idx);
            return -1;
   }
'''

BTW, the problem won't appear if use Linux guest, because the Linux virtio-net
driver will set all 7 queues's desc/avail/used tables. And the problem
won't appear
if the VM use vhost-net, because vhost-net won't update *idx* in SET_ADDR ioctl.

Sorry for my pool English, Maybe I could describe the problem in Chinese for you
in private if necessary.


> Thanks


-- 
Regards,
Longpeng

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Longpeng (Mike)]

Hi Jason & Michael,

Do you have any idea about this problem ?

-- 
Regards,
Longpeng(Mike)

On 2017/11/15 23:54, Longpeng(Mike) wrote:

> 2017-11-15 23:05 GMT+08:00 Jason Wang <jasowang@redhat.com>:
>>
>>
>> On 2017年11月15日 22:55, Longpeng(Mike) wrote:
>>>
>>> Hi guys,
>>>
>>> We got a BUG report from our testers yesterday, the testing scenario was
>>> migrating a VM (Windows guest, *4 vcpus*, 4GB, vhost-user net: *7
>>> queues*).
>>>
>>> We found the cause reason, and we'll report the BUG or send a fix patch
>>> to upstream if necessary( we haven't test the upstream yet, sorry... ).
>>
>>
>> Could you explain this a little bit more?
>>
>>>
>>> We want to know why the vhost_net_start() must start *total queues* ( in
>>> our
>>> VM there're 7 queues ) but not *the queues that current used* ( in our VM,
>>> guest
>>> only uses the first 4 queues because it's limited by the number of vcpus)
>>> ?
>>>
>>> Looking forward to your help, thx :)
>>
>>
>> Since the codes have been there for years and works well for kernel
>> datapath. You should really explain what's wrong.
>>
> 
> OK. :)
> 
> In our scenario,  the Windows's virtio-net driver only use the first 4
> queues and it
> *only set desc/avail/used table for the first 4 queues*, so in QEMU
> the desc/avail/
> used of the last 3 queues are ZERO,  but unfortunately...
> '''
> vhost_net_start
>   for (i = 0; i < total_queues; i++)
>     vhost_net_start_one
>       vhost_dev_start
>         vhost_virtqueue_start
> '''
> In vhost_virtqueue_start(), it will calculate the HVA of
> desc/avail/used table, so for last
> 3 queues, it will use ZERO as the GPA to calculate the HVA, and then
> send the results
> to the user-mode backend ( we use *vhost-user* ) by vhost_virtqueue_set_addr().
> 
> When the EVS get these address, it will update a *idx* which will be
> treated as  vq's
> last_avail_idx when virtio-net stop ( pls see vhost_virtqueue_stop() ).
> 
> So we get the following result after virtio-net stop:
>   the desc/avail/used of the last 3 queues's vqs are all ZERO, but these vqs's
>   last_avail_idx is NOT ZERO.
> 
> At last, virtio_load() reports an error:
> '''
>   if (!vdev->vq[i].vring.desc && vdev->vq[i].last_avail_idx) { // <--
> will be TRUE
>       error_report("VQ %d address 0x0 "
>                          "inconsistent with Host index 0x%x",
>                          i, vdev->vq[i].last_avail_idx);
>             return -1;
>    }
> '''
> 
> BTW, the problem won't appear if use Linux guest, because the Linux virtio-net
> driver will set all 7 queues's desc/avail/used tables. And the problem
> won't appear
> if the VM use vhost-net, because vhost-net won't update *idx* in SET_ADDR ioctl.
> 
> Sorry for my pool English, Maybe I could describe the problem in Chinese for you
> in private if necessary.
> 
> 
>> Thanks
> 
> 


-- 
Regards,
Longpeng(Mike)

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Yan Vugenfirer]

Hi Jason,

Windows driver will initialise only the amount of queue based on the amount of available vCPUs. So if there will be more queues in the device than we have vCPUs on the guest, the driver will not initialise “excessive” queues. This is tied to the way RSS on Windows should be implemented.
Exactly as in described scenario (7 queues, but only 4 vCPUs).

Best regards,
Yan.

> On 16 Nov 2017, at 07:53, Longpeng (Mike) <longpeng2@huawei.com> wrote:
> 
> Hi Jason & Michael,
> 
> Do you have any idea about this problem ?
> 
> -- 
> Regards,
> Longpeng(Mike)
> 
> On 2017/11/15 23:54, Longpeng(Mike) wrote:
> 
>> 2017-11-15 23:05 GMT+08:00 Jason Wang <jasowang@redhat.com>:
>>> 
>>> 
>>> On 2017年11月15日 22:55, Longpeng(Mike) wrote:
>>>> 
>>>> Hi guys,
>>>> 
>>>> We got a BUG report from our testers yesterday, the testing scenario was
>>>> migrating a VM (Windows guest, *4 vcpus*, 4GB, vhost-user net: *7
>>>> queues*).
>>>> 
>>>> We found the cause reason, and we'll report the BUG or send a fix patch
>>>> to upstream if necessary( we haven't test the upstream yet, sorry... ).
>>> 
>>> 
>>> Could you explain this a little bit more?
>>> 
>>>> 
>>>> We want to know why the vhost_net_start() must start *total queues* ( in
>>>> our
>>>> VM there're 7 queues ) but not *the queues that current used* ( in our VM,
>>>> guest
>>>> only uses the first 4 queues because it's limited by the number of vcpus)
>>>> ?
>>>> 
>>>> Looking forward to your help, thx :)
>>> 
>>> 
>>> Since the codes have been there for years and works well for kernel
>>> datapath. You should really explain what's wrong.
>>> 
>> 
>> OK. :)
>> 
>> In our scenario,  the Windows's virtio-net driver only use the first 4
>> queues and it
>> *only set desc/avail/used table for the first 4 queues*, so in QEMU
>> the desc/avail/
>> used of the last 3 queues are ZERO,  but unfortunately...
>> '''
>> vhost_net_start
>>  for (i = 0; i < total_queues; i++)
>>    vhost_net_start_one
>>      vhost_dev_start
>>        vhost_virtqueue_start
>> '''
>> In vhost_virtqueue_start(), it will calculate the HVA of
>> desc/avail/used table, so for last
>> 3 queues, it will use ZERO as the GPA to calculate the HVA, and then
>> send the results
>> to the user-mode backend ( we use *vhost-user* ) by vhost_virtqueue_set_addr().
>> 
>> When the EVS get these address, it will update a *idx* which will be
>> treated as  vq's
>> last_avail_idx when virtio-net stop ( pls see vhost_virtqueue_stop() ).
>> 
>> So we get the following result after virtio-net stop:
>>  the desc/avail/used of the last 3 queues's vqs are all ZERO, but these vqs's
>>  last_avail_idx is NOT ZERO.
>> 
>> At last, virtio_load() reports an error:
>> '''
>>  if (!vdev->vq[i].vring.desc && vdev->vq[i].last_avail_idx) { // <--
>> will be TRUE
>>      error_report("VQ %d address 0x0 "
>>                         "inconsistent with Host index 0x%x",
>>                         i, vdev->vq[i].last_avail_idx);
>>            return -1;
>>   }
>> '''
>> 
>> BTW, the problem won't appear if use Linux guest, because the Linux virtio-net
>> driver will set all 7 queues's desc/avail/used tables. And the problem
>> won't appear
>> if the VM use vhost-net, because vhost-net won't update *idx* in SET_ADDR ioctl.
>> 
>> Sorry for my pool English, Maybe I could describe the problem in Chinese for you
>> in private if necessary.
>> 
>> 
>>> Thanks
>> 
>> 
> 
> 
> -- 
> Regards,
> Longpeng(Mike)

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Jason Wang]

On 2017年11月16日 16:11, Yan Vugenfirer wrote:
> Hi Jason,
>
> Windows driver will initialise only the amount of queue based on the amount of available vCPUs. So if there will be more queues in the device than we have vCPUs on the guest, the driver will not initialise “excessive” queues. This is tied to the way RSS on Windows should be implemented.
> Exactly as in described scenario (7 queues, but only 4 vCPUs).

I see but is there any chance that driver can use the left 3 queues (e 
.g cpu hotplug)?

Thanks

>
> Best regards,
> Yan.
>
>> On 16 Nov 2017, at 07:53, Longpeng (Mike) <longpeng2@huawei.com> wrote:
>>
>> Hi Jason & Michael,
>>
>> Do you have any idea about this problem ?
>>
>> -- 
>> Regards,
>> Longpeng(Mike)
>>
>> On 2017/11/15 23:54, Longpeng(Mike) wrote:
>>
>>> 2017-11-15 23:05 GMT+08:00 Jason Wang <jasowang@redhat.com>:
>>>>
>>>> On 2017年11月15日 22:55, Longpeng(Mike) wrote:
>>>>> Hi guys,
>>>>>
>>>>> We got a BUG report from our testers yesterday, the testing scenario was
>>>>> migrating a VM (Windows guest, *4 vcpus*, 4GB, vhost-user net: *7
>>>>> queues*).
>>>>>
>>>>> We found the cause reason, and we'll report the BUG or send a fix patch
>>>>> to upstream if necessary( we haven't test the upstream yet, sorry... ).
>>>>
>>>> Could you explain this a little bit more?
>>>>
>>>>> We want to know why the vhost_net_start() must start *total queues* ( in
>>>>> our
>>>>> VM there're 7 queues ) but not *the queues that current used* ( in our VM,
>>>>> guest
>>>>> only uses the first 4 queues because it's limited by the number of vcpus)
>>>>> ?
>>>>>
>>>>> Looking forward to your help, thx :)
>>>>
>>>> Since the codes have been there for years and works well for kernel
>>>> datapath. You should really explain what's wrong.
>>>>
>>> OK. :)
>>>
>>> In our scenario,  the Windows's virtio-net driver only use the first 4
>>> queues and it
>>> *only set desc/avail/used table for the first 4 queues*, so in QEMU
>>> the desc/avail/
>>> used of the last 3 queues are ZERO,  but unfortunately...
>>> '''
>>> vhost_net_start
>>>   for (i = 0; i < total_queues; i++)
>>>     vhost_net_start_one
>>>       vhost_dev_start
>>>         vhost_virtqueue_start
>>> '''
>>> In vhost_virtqueue_start(), it will calculate the HVA of
>>> desc/avail/used table, so for last
>>> 3 queues, it will use ZERO as the GPA to calculate the HVA, and then
>>> send the results
>>> to the user-mode backend ( we use *vhost-user* ) by vhost_virtqueue_set_addr().
>>>
>>> When the EVS get these address, it will update a *idx* which will be
>>> treated as  vq's
>>> last_avail_idx when virtio-net stop ( pls see vhost_virtqueue_stop() ).
>>>
>>> So we get the following result after virtio-net stop:
>>>   the desc/avail/used of the last 3 queues's vqs are all ZERO, but these vqs's
>>>   last_avail_idx is NOT ZERO.
>>>
>>> At last, virtio_load() reports an error:
>>> '''
>>>   if (!vdev->vq[i].vring.desc && vdev->vq[i].last_avail_idx) { // <--
>>> will be TRUE
>>>       error_report("VQ %d address 0x0 "
>>>                          "inconsistent with Host index 0x%x",
>>>                          i, vdev->vq[i].last_avail_idx);
>>>             return -1;
>>>    }
>>> '''
>>>
>>> BTW, the problem won't appear if use Linux guest, because the Linux virtio-net
>>> driver will set all 7 queues's desc/avail/used tables. And the problem
>>> won't appear
>>> if the VM use vhost-net, because vhost-net won't update *idx* in SET_ADDR ioctl.
>>>
>>> Sorry for my pool English, Maybe I could describe the problem in Chinese for you
>>> in private if necessary.
>>>
>>>
>>>> Thanks
>>>
>>
>> -- 
>> Regards,
>> Longpeng(Mike)

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Jason Wang]

On 2017年11月16日 13:53, Longpeng (Mike) wrote:
> On 2017/11/15 23:54, Longpeng(Mike) wrote:
>> 2017-11-15 23:05 GMT+08:00 Jason Wang<jasowang@redhat.com>:
>>> On 2017年11月15日 22:55, Longpeng(Mike) wrote:
>>>> Hi guys,
>>>>
>>>> We got a BUG report from our testers yesterday, the testing scenario was
>>>> migrating a VM (Windows guest, *4 vcpus*, 4GB, vhost-user net: *7
>>>> queues*).
>>>>
>>>> We found the cause reason, and we'll report the BUG or send a fix patch
>>>> to upstream if necessary( we haven't test the upstream yet, sorry... ).
>>> Could you explain this a little bit more?
>>>
>>>> We want to know why the vhost_net_start() must start*total queues*  ( in
>>>> our
>>>> VM there're 7 queues ) but not*the queues that current used*  ( in our VM,
>>>> guest
>>>> only uses the first 4 queues because it's limited by the number of vcpus)
>>>> ?
>>>>
>>>> Looking forward to your help, thx:)
>>> Since the codes have been there for years and works well for kernel
>>> datapath. You should really explain what's wrong.
>>>
>> OK.:)
>>
>> In our scenario,  the Windows's virtio-net driver only use the first 4
>> queues and it
>> *only set desc/avail/used table for the first 4 queues*, so in QEMU
>> the desc/avail/
>> used of the last 3 queues are ZERO,  but unfortunately...
>> '''
>> vhost_net_start
>>    for (i = 0; i < total_queues; i++)
>>      vhost_net_start_one
>>        vhost_dev_start
>>          vhost_virtqueue_start
>> '''
>> In vhost_virtqueue_start(), it will calculate the HVA of
>> desc/avail/used table, so for last
>> 3 queues, it will use ZERO as the GPA to calculate the HVA, and then
>> send the results
>> to the user-mode backend ( we use*vhost-user*  ) by vhost_virtqueue_set_addr().
>>
>> When the EVS get these address, it will update a*idx*  which will be
>> treated as  vq's
>> last_avail_idx when virtio-net stop ( pls see vhost_virtqueue_stop() ).
>>
>> So we get the following result after virtio-net stop:
>>    the desc/avail/used of the last 3 queues's vqs are all ZERO, but these vqs's
>>    last_avail_idx is NOT ZERO.
>>
>> At last, virtio_load() reports an error:
>> '''
>>    if (!vdev->vq[i].vring.desc && vdev->vq[i].last_avail_idx) { // <--
>> will be TRUE
>>        error_report("VQ %d address 0x0 "
>>                           "inconsistent with Host index 0x%x",
>>                           i, vdev->vq[i].last_avail_idx);
>>              return -1;
>>     }
>> '''
>>
>> BTW, the problem won't appear if use Linux guest, because the Linux virtio-net
>> driver will set all 7 queues's desc/avail/used tables. And the problem
>> won't appear
>> if the VM use vhost-net, because vhost-net won't update*idx*  in SET_ADDR ioctl.

Just to make sure I understand here, I thought Windows guest + vhost_net 
hit this issue?

Thanks

>>
>> Sorry for my pool English, Maybe I could describe the problem in Chinese for you
>> in private if necessary.
>>
>>
>>> Thanks
> -- Regards, Longpeng(Mike)

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Gonglei (Arei)]

> -----Original Message-----
> From: Jason Wang [mailto:jasowang@redhat.com]
> Sent: Thursday, November 16, 2017 4:55 PM
> To: longpeng; mst@redhat.com
> Cc: Longpeng(Mike); qemu-devel@nongnu.org; Gonglei (Arei); Wangjing (King,
> Euler); Huangweidong (C); stefanha@redhat.com
> Subject: Re: [Question] why need to start all queues in vhost_net_start
> 
> 
> 
> On 2017年11月16日 13:53, Longpeng (Mike) wrote:
> > On 2017/11/15 23:54, Longpeng(Mike) wrote:
> >> 2017-11-15 23:05 GMT+08:00 Jason Wang<jasowang@redhat.com>:
> >>> On 2017年11月15日 22:55, Longpeng(Mike) wrote:
> >>>> Hi guys,
> >>>>
> >>>> We got a BUG report from our testers yesterday, the testing scenario was
> >>>> migrating a VM (Windows guest, *4 vcpus*, 4GB, vhost-user net: *7
> >>>> queues*).
> >>>>
> >>>> We found the cause reason, and we'll report the BUG or send a fix patch
> >>>> to upstream if necessary( we haven't test the upstream yet, sorry... ).
> >>> Could you explain this a little bit more?
> >>>
> >>>> We want to know why the vhost_net_start() must start*total queues*
> ( in
> >>>> our
> >>>> VM there're 7 queues ) but not*the queues that current used*  ( in our
> VM,
> >>>> guest
> >>>> only uses the first 4 queues because it's limited by the number of vcpus)
> >>>> ?
> >>>>
> >>>> Looking forward to your help, thx:)
> >>> Since the codes have been there for years and works well for kernel
> >>> datapath. You should really explain what's wrong.
> >>>
> >> OK.:)
> >>
> >> In our scenario,  the Windows's virtio-net driver only use the first 4
> >> queues and it
> >> *only set desc/avail/used table for the first 4 queues*, so in QEMU
> >> the desc/avail/
> >> used of the last 3 queues are ZERO,  but unfortunately...
> >> '''
> >> vhost_net_start
> >>    for (i = 0; i < total_queues; i++)
> >>      vhost_net_start_one
> >>        vhost_dev_start
> >>          vhost_virtqueue_start
> >> '''
> >> In vhost_virtqueue_start(), it will calculate the HVA of
> >> desc/avail/used table, so for last
> >> 3 queues, it will use ZERO as the GPA to calculate the HVA, and then
> >> send the results
> >> to the user-mode backend ( we use*vhost-user*  ) by
> vhost_virtqueue_set_addr().
> >>
> >> When the EVS get these address, it will update a*idx*  which will be
> >> treated as  vq's
> >> last_avail_idx when virtio-net stop ( pls see vhost_virtqueue_stop() ).
> >>
> >> So we get the following result after virtio-net stop:
> >>    the desc/avail/used of the last 3 queues's vqs are all ZERO, but these
> vqs's
> >>    last_avail_idx is NOT ZERO.
> >>
> >> At last, virtio_load() reports an error:
> >> '''
> >>    if (!vdev->vq[i].vring.desc && vdev->vq[i].last_avail_idx) { // <--
> >> will be TRUE
> >>        error_report("VQ %d address 0x0 "
> >>                           "inconsistent with Host index 0x%x",
> >>                           i, vdev->vq[i].last_avail_idx);
> >>              return -1;
> >>     }
> >> '''
> >>
> >> BTW, the problem won't appear if use Linux guest, because the Linux
> virtio-net
> >> driver will set all 7 queues's desc/avail/used tables. And the problem
> >> won't appear
> >> if the VM use vhost-net, because vhost-net won't update*idx*  in
> SET_ADDR ioctl.
> 
> Just to make sure I understand here, I thought Windows guest + vhost_net
> hit this issue?
> 
No, Windows guest + vhost-user/DPDK.

BTW pls see virtio spec in :

"If VIRTIO_NET_F_MQ is negotiated, each of receiveq1. . .receiveqN that will be used SHOULD be populated
with receive buffers."

It is not mandatory that all queues must be initialized.

Thanks,
-Gonglei

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Jason Wang]

On 2017年11月16日 17:01, Gonglei (Arei) wrote:
>
>> -----Original Message-----
>> From: Jason Wang [mailto:jasowang@redhat.com]
>> Sent: Thursday, November 16, 2017 4:55 PM
>> To: longpeng; mst@redhat.com
>> Cc: Longpeng(Mike); qemu-devel@nongnu.org; Gonglei (Arei); Wangjing (King,
>> Euler); Huangweidong (C); stefanha@redhat.com
>> Subject: Re: [Question] why need to start all queues in vhost_net_start
>>
>>
>>
>> On 2017年11月16日 13:53, Longpeng (Mike) wrote:
>>> On 2017/11/15 23:54, Longpeng(Mike) wrote:
>>>> 2017-11-15 23:05 GMT+08:00 Jason Wang<jasowang@redhat.com>:
>>>>> On 2017年11月15日 22:55, Longpeng(Mike) wrote:
>>>>>> Hi guys,
>>>>>>
>>>>>> We got a BUG report from our testers yesterday, the testing scenario was
>>>>>> migrating a VM (Windows guest, *4 vcpus*, 4GB, vhost-user net: *7
>>>>>> queues*).
>>>>>>
>>>>>> We found the cause reason, and we'll report the BUG or send a fix patch
>>>>>> to upstream if necessary( we haven't test the upstream yet, sorry... ).
>>>>> Could you explain this a little bit more?
>>>>>
>>>>>> We want to know why the vhost_net_start() must start*total queues*
>> ( in
>>>>>> our
>>>>>> VM there're 7 queues ) but not*the queues that current used*  ( in our
>> VM,
>>>>>> guest
>>>>>> only uses the first 4 queues because it's limited by the number of vcpus)
>>>>>> ?
>>>>>>
>>>>>> Looking forward to your help, thx:)
>>>>> Since the codes have been there for years and works well for kernel
>>>>> datapath. You should really explain what's wrong.
>>>>>
>>>> OK.:)
>>>>
>>>> In our scenario,  the Windows's virtio-net driver only use the first 4
>>>> queues and it
>>>> *only set desc/avail/used table for the first 4 queues*, so in QEMU
>>>> the desc/avail/
>>>> used of the last 3 queues are ZERO,  but unfortunately...
>>>> '''
>>>> vhost_net_start
>>>>     for (i = 0; i < total_queues; i++)
>>>>       vhost_net_start_one
>>>>         vhost_dev_start
>>>>           vhost_virtqueue_start
>>>> '''
>>>> In vhost_virtqueue_start(), it will calculate the HVA of
>>>> desc/avail/used table, so for last
>>>> 3 queues, it will use ZERO as the GPA to calculate the HVA, and then
>>>> send the results
>>>> to the user-mode backend ( we use*vhost-user*  ) by
>> vhost_virtqueue_set_addr().
>>>> When the EVS get these address, it will update a*idx*  which will be
>>>> treated as  vq's
>>>> last_avail_idx when virtio-net stop ( pls see vhost_virtqueue_stop() ).
>>>>
>>>> So we get the following result after virtio-net stop:
>>>>     the desc/avail/used of the last 3 queues's vqs are all ZERO, but these
>> vqs's
>>>>     last_avail_idx is NOT ZERO.
>>>>
>>>> At last, virtio_load() reports an error:
>>>> '''
>>>>     if (!vdev->vq[i].vring.desc && vdev->vq[i].last_avail_idx) { // <--
>>>> will be TRUE
>>>>         error_report("VQ %d address 0x0 "
>>>>                            "inconsistent with Host index 0x%x",
>>>>                            i, vdev->vq[i].last_avail_idx);
>>>>               return -1;
>>>>      }
>>>> '''
>>>>
>>>> BTW, the problem won't appear if use Linux guest, because the Linux
>> virtio-net
>>>> driver will set all 7 queues's desc/avail/used tables. And the problem
>>>> won't appear
>>>> if the VM use vhost-net, because vhost-net won't update*idx*  in
>> SET_ADDR ioctl.
>>
>> Just to make sure I understand here, I thought Windows guest + vhost_net
>> hit this issue?
>>
> No, Windows guest + vhost-user/DPDK.
>
> BTW pls see virtio spec in :
>
> "If VIRTIO_NET_F_MQ is negotiated, each of receiveq1. . .receiveqN that will be used SHOULD be populated
> with receive buffers."
>
> It is not mandatory that all queues must be initialized.
>
> Thanks,
> -Gonglei
>

Interesting, vhost_net will set last_avail_idx to vq.num during 
SET_VRING_BASE. So I thought it should hit this.

Btw, maybe we should relax the check to:

if (!vdev->vq[i].vring.desc && (vdev->vq[i].last_avail_idx != 
vdev->vq[i].vring.num)) {

Thanks

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Jason Wang]

On 2017年11月16日 17:01, Gonglei (Arei) wrote:
> No, Windows guest + vhost-user/DPDK.
>
> BTW pls see virtio spec in :
>
> "If VIRTIO_NET_F_MQ is negotiated, each of receiveq1. . .receiveqN that will be used SHOULD be populated
> with receive buffers."
>
> It is not mandatory that all queues must be initialized.

I think not, since it said we should fill receive buffers for each queue 
which means we should initialize all queues. May Michael can clarify on 
this.

Thanks

>
> Thanks,
> -Gonglei
>

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Longpeng (Mike)]

Hi Jason,

On 2017/11/16 17:13, Jason Wang wrote:

> 
> 
> On 2017年11月16日 17:01, Gonglei (Arei) wrote:
>> No, Windows guest + vhost-user/DPDK.
>>
>> BTW pls see virtio spec in :
>>
>> "If VIRTIO_NET_F_MQ is negotiated, each of receiveq1. . .receiveqN that will
>> be used SHOULD be populated
>> with receive buffers."
>>
>> It is not mandatory that all queues must be initialized.
> 
> I think not, since it said we should fill receive buffers for each queue which
> means we should initialize all queues. May Michael can clarify on this.
> 


I think this doesn't matter, but QEMU should consider this scenario...

For example, if one queues isn't initialized (Windows guest), the vring.avail=0,
so vq->desc_phys=0, then vq->desc='a avail HVA'(which is the start addr of pc.ram).

    vq->desc_size = s = l = virtio_queue_get_desc_size(vdev, idx);
    vq->desc_phys = a = virtio_queue_get_desc_addr(vdev, idx);
    vq->desc = vhost_memory_map(dev, a, &l, 0);
    if (!vq->desc || l != s) {
        r = -ENOMEM;
        goto fail_alloc_desc;
    }
    .....
    r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
    if (r < 0) {
        r = -errno;
        goto fail_alloc;
    }

Then the HVA is send to the vhost-user.

I think this is wrong, because the '0' here means guest driver doesn't init this
queues, it should not be used to calculate the HVA for this vq.

> Thanks
> 
>>
>> Thanks,
>> -Gonglei
>>
> 
> 
> 
> 
> .
> 


-- 
Regards,
Longpeng(Mike)

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Jason Wang]

On 2017年11月16日 17:32, Longpeng (Mike) wrote:
> Hi Jason,
>
> On 2017/11/16 17:13, Jason Wang wrote:
>
>>
>> On 2017年11月16日 17:01, Gonglei (Arei) wrote:
>>> No, Windows guest + vhost-user/DPDK.
>>>
>>> BTW pls see virtio spec in :
>>>
>>> "If VIRTIO_NET_F_MQ is negotiated, each of receiveq1. . .receiveqN that will
>>> be used SHOULD be populated
>>> with receive buffers."
>>>
>>> It is not mandatory that all queues must be initialized.
>> I think not, since it said we should fill receive buffers for each queue which
>> means we should initialize all queues. May Michael can clarify on this.
>>
>
> I think this doesn't matter, but QEMU should consider this scenario...
>
> For example, if one queues isn't initialized (Windows guest), the vring.avail=0,
> so vq->desc_phys=0, then vq->desc='a avail HVA'(which is the start addr of pc.ram).
>
>      vq->desc_size = s = l = virtio_queue_get_desc_size(vdev, idx);
>      vq->desc_phys = a = virtio_queue_get_desc_addr(vdev, idx);
>      vq->desc = vhost_memory_map(dev, a, &l, 0);
>      if (!vq->desc || l != s) {
>          r = -ENOMEM;
>          goto fail_alloc_desc;
>      }
>      .....
>      r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
>      if (r < 0) {
>          r = -errno;
>          goto fail_alloc;
>      }
>
> Then the HVA is send to the vhost-user.
>
> I think this is wrong, because the '0' here means guest driver doesn't init this
> queues, it should not be used to calculate the HVA for this vq.

Yes, workaround is not hard if windows driver won't use the left 3 
queues any more. But we should have a complete solution. The main 
problem is when vhost need to be started. For legacy device, there's no 
easy way to detect whether or not a specific virtqueue is ready to be 
used. For modern device, we can probably do this through queue_enable 
(but this is not implemented in current code).

Thanks

>
>> Thanks
>>
>>> Thanks,
>>> -Gonglei
>>>
>>
>>
>>
>> .
>>
>

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Longpeng (Mike)]

On 2017/11/16 20:04, Jason Wang wrote:

> 
> 
> On 2017年11月16日 17:32, Longpeng (Mike) wrote:
>> Hi Jason,
>>
>> On 2017/11/16 17:13, Jason Wang wrote:
>>
>>>
>>> On 2017年11月16日 17:01, Gonglei (Arei) wrote:
>>>> No, Windows guest + vhost-user/DPDK.
>>>>
>>>> BTW pls see virtio spec in :
>>>>
>>>> "If VIRTIO_NET_F_MQ is negotiated, each of receiveq1. . .receiveqN that will
>>>> be used SHOULD be populated
>>>> with receive buffers."
>>>>
>>>> It is not mandatory that all queues must be initialized.
>>> I think not, since it said we should fill receive buffers for each queue which
>>> means we should initialize all queues. May Michael can clarify on this.
>>>
>>
>> I think this doesn't matter, but QEMU should consider this scenario...
>>
>> For example, if one queues isn't initialized (Windows guest), the vring.avail=0,
>> so vq->desc_phys=0, then vq->desc='a avail HVA'(which is the start addr of
>> pc.ram).
>>
>>      vq->desc_size = s = l = virtio_queue_get_desc_size(vdev, idx);
>>      vq->desc_phys = a = virtio_queue_get_desc_addr(vdev, idx);
>>      vq->desc = vhost_memory_map(dev, a, &l, 0);
>>      if (!vq->desc || l != s) {
>>          r = -ENOMEM;
>>          goto fail_alloc_desc;
>>      }
>>      .....
>>      r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
>>      if (r < 0) {
>>          r = -errno;
>>          goto fail_alloc;
>>      }
>>
>> Then the HVA is send to the vhost-user.
>>
>> I think this is wrong, because the '0' here means guest driver doesn't init this
>> queues, it should not be used to calculate the HVA for this vq.
> 
> Yes, workaround is not hard if windows driver won't use the left 3 queues any
> more. But we should have a complete solution. The main problem is when vhost
> need to be started. For legacy device, there's no easy way to detect whether or
> not a specific virtqueue is ready to be used. For modern device, we can probably
> do this through queue_enable (but this is not implemented in current code).
> 


Can we initialize the vring.desc/vring.avail/vring.used to -1, then we can
detect whether or not a specific vq is ready to be used by check the addr of its
tables is -1 or not ?

Initialize to 0 is not suitable, because we don't know whether the guest set GPA
0 or the guest not initialize this vq.

> Thanks
> 
>>
>>> Thanks
>>>
>>>> Thanks,
>>>> -Gonglei
>>>>
>>>
>>>
>>>
>>> .
>>>
>>
> 
> 
> .
> 


-- 
Regards,
Longpeng(Mike)

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Jason Wang]

On 2017年11月17日 10:01, Longpeng (Mike) wrote:
> On 2017/11/16 20:04, Jason Wang wrote:
>
>> On 2017年11月16日 17:32, Longpeng (Mike) wrote:
>>> Hi Jason,
>>>
>>> On 2017/11/16 17:13, Jason Wang wrote:
>>>
>>>> On 2017年11月16日 17:01, Gonglei (Arei) wrote:
>>>>> No, Windows guest + vhost-user/DPDK.
>>>>>
>>>>> BTW pls see virtio spec in :
>>>>>
>>>>> "If VIRTIO_NET_F_MQ is negotiated, each of receiveq1. . .receiveqN that will
>>>>> be used SHOULD be populated
>>>>> with receive buffers."
>>>>>
>>>>> It is not mandatory that all queues must be initialized.
>>>> I think not, since it said we should fill receive buffers for each queue which
>>>> means we should initialize all queues. May Michael can clarify on this.
>>>>
>>> I think this doesn't matter, but QEMU should consider this scenario...
>>>
>>> For example, if one queues isn't initialized (Windows guest), the vring.avail=0,
>>> so vq->desc_phys=0, then vq->desc='a avail HVA'(which is the start addr of
>>> pc.ram).
>>>
>>>       vq->desc_size = s = l = virtio_queue_get_desc_size(vdev, idx);
>>>       vq->desc_phys = a = virtio_queue_get_desc_addr(vdev, idx);
>>>       vq->desc = vhost_memory_map(dev, a, &l, 0);
>>>       if (!vq->desc || l != s) {
>>>           r = -ENOMEM;
>>>           goto fail_alloc_desc;
>>>       }
>>>       .....
>>>       r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
>>>       if (r < 0) {
>>>           r = -errno;
>>>           goto fail_alloc;
>>>       }
>>>
>>> Then the HVA is send to the vhost-user.
>>>
>>> I think this is wrong, because the '0' here means guest driver doesn't init this
>>> queues, it should not be used to calculate the HVA for this vq.
>> Yes, workaround is not hard if windows driver won't use the left 3 queues any
>> more. But we should have a complete solution. The main problem is when vhost
>> need to be started. For legacy device, there's no easy way to detect whether or
>> not a specific virtqueue is ready to be used. For modern device, we can probably
>> do this through queue_enable (but this is not implemented in current code).
>>
> Can we initialize the vring.desc/vring.avail/vring.used to -1, then we can
> detect whether or not a specific vq is ready to be used by check the addr of its
> tables is -1 or not ?
>
> Initialize to 0 is not suitable, because we don't know whether the guest set GPA
> 0 or the guest not initialize this vq.
>

Unfortunately, it's too late to do this in spec for legacy device. (If 
you look at the spec, legacy MMIO transport treat 0x0 as an illegal 
value.) For modern device, we could detect this through queue_enable.

Let's wait for Yan to reply whether or not windows driver can use the 
left three queues. If not, I will try to post a fix(workaround) for this 
and let's consider a complete solution.

Thanks

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Michael S. Tsirkin]

On Thu, Nov 16, 2017 at 08:04:34PM +0800, Jason Wang wrote:
> 
> 
> On 2017年11月16日 17:32, Longpeng (Mike) wrote:
> > Hi Jason,
> > 
> > On 2017/11/16 17:13, Jason Wang wrote:
> > 
> > > 
> > > On 2017年11月16日 17:01, Gonglei (Arei) wrote:
> > > > No, Windows guest + vhost-user/DPDK.
> > > > 
> > > > BTW pls see virtio spec in :
> > > > 
> > > > "If VIRTIO_NET_F_MQ is negotiated, each of receiveq1. . .receiveqN that will
> > > > be used SHOULD be populated
> > > > with receive buffers."
> > > > 
> > > > It is not mandatory that all queues must be initialized.
> > > I think not, since it said we should fill receive buffers for each queue which
> > > means we should initialize all queues. May Michael can clarify on this.
> > > 
> > 
> > I think this doesn't matter, but QEMU should consider this scenario...
> > 
> > For example, if one queues isn't initialized (Windows guest), the vring.avail=0,
> > so vq->desc_phys=0, then vq->desc='a avail HVA'(which is the start addr of pc.ram).
> > 
> >      vq->desc_size = s = l = virtio_queue_get_desc_size(vdev, idx);
> >      vq->desc_phys = a = virtio_queue_get_desc_addr(vdev, idx);
> >      vq->desc = vhost_memory_map(dev, a, &l, 0);
> >      if (!vq->desc || l != s) {
> >          r = -ENOMEM;
> >          goto fail_alloc_desc;
> >      }
> >      .....
> >      r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
> >      if (r < 0) {
> >          r = -errno;
> >          goto fail_alloc;
> >      }
> > 
> > Then the HVA is send to the vhost-user.
> > 
> > I think this is wrong, because the '0' here means guest driver doesn't init this
> > queues, it should not be used to calculate the HVA for this vq.
> 
> Yes, workaround is not hard if windows driver won't use the left 3 queues
> any more. But we should have a complete solution. The main problem is when
> vhost need to be started. For legacy device, there's no easy way to detect
> whether or not a specific virtqueue is ready to be used. For modern device,
> we can probably do this through queue_enable (but this is not implemented in
> current code).
> 
> Thanks

What isn't implemented?

Spec is quite explicit:


Client must only process each ring when it is started.

Client must only pass data between the ring and the
backend, when the ring is enabled.

and later:


Client must start ring upon receiving a kick (that is, detecting that file
descriptor is readable) on the descriptor specified by
VHOST_USER_SET_VRING_KICK, and stop ring upon receiving
VHOST_USER_GET_VRING_BASE.


Does someone kick unused rings? What entity does this?




> > 
> > > Thanks
> > > 
> > > > Thanks,
> > > > -Gonglei
> > > > 
> > > 
> > > 
> > > 
> > > .
> > > 
> > 

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Longpeng (Mike)]

On 2017/11/17 12:32, Michael S. Tsirkin wrote:

> On Thu, Nov 16, 2017 at 08:04:34PM +0800, Jason Wang wrote:
>>
>>
>> On 2017年11月16日 17:32, Longpeng (Mike) wrote:
>>> Hi Jason,
>>>
>>> On 2017/11/16 17:13, Jason Wang wrote:
>>>
>>>>
>>>> On 2017年11月16日 17:01, Gonglei (Arei) wrote:
>>>>> No, Windows guest + vhost-user/DPDK.
>>>>>
>>>>> BTW pls see virtio spec in :
>>>>>
>>>>> "If VIRTIO_NET_F_MQ is negotiated, each of receiveq1. . .receiveqN that will
>>>>> be used SHOULD be populated
>>>>> with receive buffers."
>>>>>
>>>>> It is not mandatory that all queues must be initialized.
>>>> I think not, since it said we should fill receive buffers for each queue which
>>>> means we should initialize all queues. May Michael can clarify on this.
>>>>
>>>
>>> I think this doesn't matter, but QEMU should consider this scenario...
>>>
>>> For example, if one queues isn't initialized (Windows guest), the vring.avail=0,
>>> so vq->desc_phys=0, then vq->desc='a avail HVA'(which is the start addr of pc.ram).
>>>
>>>      vq->desc_size = s = l = virtio_queue_get_desc_size(vdev, idx);
>>>      vq->desc_phys = a = virtio_queue_get_desc_addr(vdev, idx);
>>>      vq->desc = vhost_memory_map(dev, a, &l, 0);
>>>      if (!vq->desc || l != s) {
>>>          r = -ENOMEM;
>>>          goto fail_alloc_desc;
>>>      }
>>>      .....
>>>      r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
>>>      if (r < 0) {
>>>          r = -errno;
>>>          goto fail_alloc;
>>>      }
>>>
>>> Then the HVA is send to the vhost-user.
>>>
>>> I think this is wrong, because the '0' here means guest driver doesn't init this
>>> queues, it should not be used to calculate the HVA for this vq.
>>
>> Yes, workaround is not hard if windows driver won't use the left 3 queues
>> any more. But we should have a complete solution. The main problem is when
>> vhost need to be started. For legacy device, there's no easy way to detect
>> whether or not a specific virtqueue is ready to be used. For modern device,
>> we can probably do this through queue_enable (but this is not implemented in
>> current code).
>>
>> Thanks
> 
> What isn't implemented?
> 
> Spec is quite explicit:
> 
> 
> Client must only process each ring when it is started.
> 
> Client must only pass data between the ring and the
> backend, when the ring is enabled.
> 
> and later:
> 
> 
> Client must start ring upon receiving a kick (that is, detecting that file
> descriptor is readable) on the descriptor specified by
> VHOST_USER_SET_VRING_KICK, and stop ring upon receiving
> VHOST_USER_GET_VRING_BASE.
> 
> 
> Does someone kick unused rings? What entity does this?
> 


Hi Michael,

Maybe you don't get our point, so let me be clear. :)

Suppose there is a VM with 4 vcpus and 7 queues (vhost-user backend), the
Windows guest will only initialize the first 4 queues, so in QEMU the addresses
of the desc/avail/used tables of last 3 queues are all ZERO (initial value).

However QEMU will start all 7 queues, please look at the codes below:
'''
vhost_net_start
  for (i = 0; i < total_queues; i++) // start all 7 queues
    vhost_net_start_one
      vhost_dev_start
        vhost_virtqueue_start
'''

In vhost_virtqueue_start(), it will use the ZERO to calculate a corresponding
HVA for the last 3 queues and then send to the backend ( by
vhost_virtqueue_set_addr() ).

So our issue here is: the ZERO here means guest doesn't initialize the last 3
queues, so it needn't to calculate the HVA for them and send to then backend.

But, there is no issue on Linux guest, because Linux driver will initialize the
desc/avail/used tables for all 7 queues (even if it only uses the first 4 queues).

Jason has already get our point, now we need to obtain more information about
Windows virtio-net driver from Yan.

> 
> 
> 
>>>
>>>> Thanks
>>>>
>>>>> Thanks,
>>>>> -Gonglei
>>>>>
>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
> 
> .
> 


-- 
Regards,
Longpeng(Mike)

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Jason Wang]

On 2017年11月17日 12:32, Michael S. Tsirkin wrote:
> On Thu, Nov 16, 2017 at 08:04:34PM +0800, Jason Wang wrote:
>>
>> On 2017年11月16日 17:32, Longpeng (Mike) wrote:
>>> Hi Jason,
>>>
>>> On 2017/11/16 17:13, Jason Wang wrote:
>>>
>>>> On 2017年11月16日 17:01, Gonglei (Arei) wrote:
>>>>> No, Windows guest + vhost-user/DPDK.
>>>>>
>>>>> BTW pls see virtio spec in :
>>>>>
>>>>> "If VIRTIO_NET_F_MQ is negotiated, each of receiveq1. . .receiveqN that will
>>>>> be used SHOULD be populated
>>>>> with receive buffers."
>>>>>
>>>>> It is not mandatory that all queues must be initialized.
>>>> I think not, since it said we should fill receive buffers for each queue which
>>>> means we should initialize all queues. May Michael can clarify on this.
>>>>
>>> I think this doesn't matter, but QEMU should consider this scenario...
>>>
>>> For example, if one queues isn't initialized (Windows guest), the vring.avail=0,
>>> so vq->desc_phys=0, then vq->desc='a avail HVA'(which is the start addr of pc.ram).
>>>
>>>       vq->desc_size = s = l = virtio_queue_get_desc_size(vdev, idx);
>>>       vq->desc_phys = a = virtio_queue_get_desc_addr(vdev, idx);
>>>       vq->desc = vhost_memory_map(dev, a, &l, 0);
>>>       if (!vq->desc || l != s) {
>>>           r = -ENOMEM;
>>>           goto fail_alloc_desc;
>>>       }
>>>       .....
>>>       r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
>>>       if (r < 0) {
>>>           r = -errno;
>>>           goto fail_alloc;
>>>       }
>>>
>>> Then the HVA is send to the vhost-user.
>>>
>>> I think this is wrong, because the '0' here means guest driver doesn't init this
>>> queues, it should not be used to calculate the HVA for this vq.
>> Yes, workaround is not hard if windows driver won't use the left 3 queues
>> any more. But we should have a complete solution. The main problem is when
>> vhost need to be started. For legacy device, there's no easy way to detect
>> whether or not a specific virtqueue is ready to be used. For modern device,
>> we can probably do this through queue_enable (but this is not implemented in
>> current code).
>>
>> Thanks
> What isn't implemented?

I mean queue_enable. Virtio spec said:

queue_enable
    The driver uses this to selectively prevent the device from
    executing requests from this virtqueue. 1 - enabled; 0 - disabled. 


But we have:

     case VIRTIO_PCI_COMMON_Q_ENABLE:
         virtio_queue_set_num(vdev, vdev->queue_sel,
                              proxy->vqs[vdev->queue_sel].num);
         virtio_queue_set_rings(vdev, vdev->queue_sel,
((uint64_t)proxy->vqs[vdev->queue_sel].desc[1]) << 32 |
                        proxy->vqs[vdev->queue_sel].desc[0],
((uint64_t)proxy->vqs[vdev->queue_sel].avail[1]) << 32 |
                        proxy->vqs[vdev->queue_sel].avail[0],
((uint64_t)proxy->vqs[vdev->queue_sel].used[1]) << 32 |
                        proxy->vqs[vdev->queue_sel].used[0]);
         proxy->vqs[vdev->queue_sel].enabled = 1;
         break;

So it looks to me that we need:

- Not assume the value is 1
- Start or stop vhost virtqueue depends on value

Thanks

>
> Spec is quite explicit:
>
>
> Client must only process each ring when it is started.
>
> Client must only pass data between the ring and the
> backend, when the ring is enabled.
>
> and later:
>
>
> Client must start ring upon receiving a kick (that is, detecting that file
> descriptor is readable) on the descriptor specified by
> VHOST_USER_SET_VRING_KICK, and stop ring upon receiving
> VHOST_USER_GET_VRING_BASE.
>
>
> Does someone kick unused rings? What entity does this?
>
>
>
>
>>>> Thanks
>>>>
>>>>> Thanks,
>>>>> -Gonglei
>>>>>
>>>>
>>>>
>>>> .
>>>>

Re: [Qemu-devel] [Question] why need to start all queues in vhost_net_start

[Longpeng (Mike)]

On 2017/11/16 16:54, Jason Wang wrote:

> 
> 
> On 2017年11月16日 13:53, Longpeng (Mike) wrote:
>> On 2017/11/15 23:54, Longpeng(Mike) wrote:
>>> 2017-11-15 23:05 GMT+08:00 Jason Wang<jasowang@redhat.com>:
>>>> On 2017年11月15日 22:55, Longpeng(Mike) wrote:
>>>>> Hi guys,
>>>>>
>>>>> We got a BUG report from our testers yesterday, the testing scenario was
>>>>> migrating a VM (Windows guest, *4 vcpus*, 4GB, vhost-user net: *7
>>>>> queues*).
>>>>>
>>>>> We found the cause reason, and we'll report the BUG or send a fix patch
>>>>> to upstream if necessary( we haven't test the upstream yet, sorry... ).
>>>> Could you explain this a little bit more?
>>>>
>>>>> We want to know why the vhost_net_start() must start*total queues*  ( in
>>>>> our
>>>>> VM there're 7 queues ) but not*the queues that current used*  ( in our VM,
>>>>> guest
>>>>> only uses the first 4 queues because it's limited by the number of vcpus)
>>>>> ?
>>>>>
>>>>> Looking forward to your help, thx:)
>>>> Since the codes have been there for years and works well for kernel
>>>> datapath. You should really explain what's wrong.
>>>>
>>> OK.:)
>>>
>>> In our scenario,  the Windows's virtio-net driver only use the first 4
>>> queues and it
>>> *only set desc/avail/used table for the first 4 queues*, so in QEMU
>>> the desc/avail/
>>> used of the last 3 queues are ZERO,  but unfortunately...
>>> '''
>>> vhost_net_start
>>>    for (i = 0; i < total_queues; i++)
>>>      vhost_net_start_one
>>>        vhost_dev_start
>>>          vhost_virtqueue_start
>>> '''
>>> In vhost_virtqueue_start(), it will calculate the HVA of
>>> desc/avail/used table, so for last
>>> 3 queues, it will use ZERO as the GPA to calculate the HVA, and then
>>> send the results
>>> to the user-mode backend ( we use*vhost-user*  ) by vhost_virtqueue_set_addr().
>>>
>>> When the EVS get these address, it will update a*idx*  which will be
>>> treated as  vq's
>>> last_avail_idx when virtio-net stop ( pls see vhost_virtqueue_stop() ).
>>>
>>> So we get the following result after virtio-net stop:
>>>    the desc/avail/used of the last 3 queues's vqs are all ZERO, but these vqs's
>>>    last_avail_idx is NOT ZERO.
>>>
>>> At last, virtio_load() reports an error:
>>> '''
>>>    if (!vdev->vq[i].vring.desc && vdev->vq[i].last_avail_idx) { // <--
>>> will be TRUE
>>>        error_report("VQ %d address 0x0 "
>>>                           "inconsistent with Host index 0x%x",
>>>                           i, vdev->vq[i].last_avail_idx);
>>>              return -1;
>>>     }
>>> '''
>>>
>>> BTW, the problem won't appear if use Linux guest, because the Linux virtio-net
>>> driver will set all 7 queues's desc/avail/used tables. And the problem
>>> won't appear
>>> if the VM use vhost-net, because vhost-net won't update*idx*  in SET_ADDR ioctl.
> 
> Just to make sure I understand here, I thought Windows guest + vhost_net hit
> this issue?
> 


Windows guest + vhost-user hit.
Windows guest + vhost-net is fine.

'''
In vhost_virtqueue_start(), it will calculate the HVA of
desc/avail/used tables, so for last
3 queues, it will use ZERO as the GPA to calculate the HVA, and then
send the results
to the user-mode backend ( we use *vhost-user* ) by vhost_virtqueue_set_addr().
'''
I think this is the root cause, it is strange, right ?

> Thanks
> 
>>>
>>> Sorry for my pool English, Maybe I could describe the problem in Chinese for you
>>> in private if necessary.
>>>
>>>
>>>> Thanks
>> -- Regards, Longpeng(Mike)
> 
> 
> .
> 


-- 
Regards,
Longpeng(Mike)