Re: [PATCH] vchost: Notify application of ownership change

[Tetsuya Mukawa <mukawa@igel.co.jp>]

On 2015/10/26 15:30, Yuanhan Liu wrote:
> On Mon, Oct 26, 2015 at 02:54:07PM +0900, Tetsuya Mukawa wrote:
>> On 2015/10/25 2:16, Thomas Monjalon wrote:
>>> 2015-08-12 03:34, Xie, Huawei:
>>>> On 8/8/2015 1:21 AM, Jan Kiszka wrote:
>>>>> On VHOST_*_RESET_OWNER, we reinitialize the device but without telling
>>>>> the application. That will cause crashes when it continues to invoke
>>>>> vhost services on the device. Fix it by calling the destruction hook if
>>>>> the device is still in use.
>>> [...]
>>>>> --- a/lib/librte_vhost/virtio-net.c
>>>>> +++ b/lib/librte_vhost/virtio-net.c
>>>>> @@ -402,6 +402,9 @@ reset_owner(struct vhost_device_ctx ctx)
>>>>>
>>>>>  	ll_dev = get_config_ll_entry(ctx);
>>>>>
>>>>> +	if ((ll_dev->dev.flags & VIRTIO_DEV_RUNNING))
>>>>> +		notify_ops->destroy_device(&ll_dev->dev);
>>>> To me this patch makes sense here.
>>>> Whether RESET_OWNER is really needed is another question. Whenever the
>>>> vhost itself needs to process the vhost device, we need to notify the
>>>> switch application to remove it from data plane.
>>> Huawei,
>>> some patches have been accepted for RESET_OWNER management.
>>> Is this patch obsolete?
> I think it's still appliable, at least so far.
>
>> Hi Yuanhan and Huawei,
>>
>> I also have the same question. Do we have a patch for this issue?
>>
>> Today, I've download Yuanhan's multiple queues patches and applied it on
>> latest dpdk tree.
>> Then, tried to apply my vhost PMD patch on it.
>>
>> When I check the patch, it seems I've faced this issue.
>> Here are steps to reproduce.
> Above patch should fix your issue, right? If so, we need it.

Yes, the patch will fix the issue.

>> 1. Start vhost-user backend application.
>>      (In my case, testpmd using vhost PMD is the application)
>> 2. Start a VM with vhost-user.
>>      You can see below message from the backend application.
>>       VHOST_CONFIG: read message VHOST_USER_SET_VRING_ENABLE
>>       VHOST_CONFIG: set queue enable: 1 to qp idx: 0
>>       (snip)
>>       VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK
>> 3. After booting Linux on guest, bind the virtio-net device to igb_uio.
>>     Then below messages are shown.
>>     VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
>>     VHOST_CONFIG: read message VHOST_USER_RESET_OWNER
>>     VHOST_CONFIG: read message VHOST_USER_GET_VRING_BASE
>>
>> The point is we will have VHOST_USER_RESET_OWNER before
>> VHOST_USER_GET_VRING_BASE.
> Note that there is an ongoing work at QEMU community (from me) to
> handle RESET_OWNER correctly: it will be moved to somewhere else
> instead of before VHOST_USER_GET_VRING_BASE.
>
> 	--yliu

Sounds great! Thanks for handling it.

Tetsuya