Re: [dpdk-dev] [PATCH] vhost: fix passing destroyed device to destroy callback

[Ilya Maximets <i.maximets@samsung.com>]

On 12.04.2019 7:39, Tiwei Bie wrote:
> On Tue, Apr 09, 2019 at 04:36:22PM +0300, Ilya Maximets wrote:
>> Application should be able to obtain information like 'ifname' from
>> the 'vid' passed to 'destroy_connection' callback. Currently, all the
>> API calls with passed 'vid' fails with 'device not found'.
>>
>> Fixes: efba12a78ddf ("vhost: add user callbacks for socket open/close")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>>  lib/librte_vhost/socket.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
>> index 3da9de62c..43f091d10 100644
>> --- a/lib/librte_vhost/socket.c
>> +++ b/lib/librte_vhost/socket.c
>> @@ -297,11 +297,12 @@ vhost_user_read_cb(int connfd, void *dat, int *remove)
>>  	if (ret < 0) {
>>  		close(connfd);
>>  		*remove = 1;
>> -		vhost_destroy_device(conn->vid);
>>  
>>  		if (vsocket->notify_ops->destroy_connection)
>>  			vsocket->notify_ops->destroy_connection(conn->vid);
>>  
>> +		vhost_destroy_device(conn->vid);
> 
> By design, notify_ops->destroy_device() should be called before
> notify_ops->destroy_connection(). I.e. something like:
> 
> 	vhost_destroy_device_notify(...); // device stop
> 
> 	if (vsocket->notify_ops->destroy_connection) // socket close
> 		vsocket->notify_ops->destroy_connection(conn->vid);
> 
> 	vhost_destroy_device(conn->vid); // free
> 
> As destroy_device() callback means device stop (which also
> happens in e.g. GET_VRING_BASE).

Agree. Thanks. Will send v2.

> 
> 
>> +
>>  		pthread_mutex_lock(&vsocket->conn_mutex);
>>  		TAILQ_REMOVE(&vsocket->conn_list, conn, next);
>>  		pthread_mutex_unlock(&vsocket->conn_mutex);
>> -- 
>> 2.17.1
>>
> 
>