[dpdk-stable] patch 'vhost: fix device cleanup at stop' has been queued to LTS release 16.11.7

[Maxime Coquelin]

Hi Luca, Tomasz,

While testing 16.11 branch, I noticed vhost lib is broken.
The symptoms is no packets are sent or received.

I ran a bisect which points to this commit. Reverting it solves the
issue.

Debugging a bit more, I can see that the callfd is valid when no packets
are being transmitted. I think the problem is that the callfd is
received after the kickfd:

VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK
VHOST_CONFIG: vring kick idx:0 file:16
VHOST_CONFIG: virtio is not ready for processing.
VHOST_CONFIG: read message VHOST_USER_SET_VRING_CALL
VHOST_CONFIG: vring call idx:0 file:17

And in 16.11, the new_device callback is called from the
VHOST_USER_SET_VRING_KICK handling, only if the device is ready.
This is different in later versions, where the new_device callback
can be called for any request.

The right way to fix this would be to move the new_device callback call
for any request, but I think there is a non-negligible risk of
regression so we'd need to be careful doing that.

Other option is to simply revert Tomasz patch in 16.11 LTS. This is not
ideal because Tomasz patch is fixing a real issue (new_device get called
with an outdated/invalid callfd). However, I don't know if it has real
consequences, as the callfd is updated right after.

Except if someone face real issue due to callfd being updated after
new_device is called, then my suggestion would be to revert Tomasz patch
as we are at 6 months of 16.11 EOL.

Luca, Tomasz, what's your take on this?

Regards,
Maxime

On 05/01/2018 12:44 PM, luca.boccassi at gmail.com wrote:
> Hi,
> 
> FYI, your patch has been queued to LTS release 16.11.7
> 
> Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
> It will be pushed if I get no objections before 05/03/18. So please
> shout if anyone has objections.
> 
> Thanks.
> 
> Luca Boccassi
> 
> ---
>  From 357f27736c79d0258409666277b6e113d11c757b Mon Sep 17 00:00:00 2001
> From: Tomasz Kulasek <tomaszx.kulasek at intel.com>
> Date: Fri, 9 Feb 2018 18:10:00 +0100
> Subject: [PATCH] vhost: fix device cleanup at stop
> 
> [ upstream commit ace7b6b7859e1dc410589610a8e436c1a3b430f3 ]
> 
> This prevents from destroying & recreating user device in "incomplete"
> vring state. virtio_is_ready() was returning true for devices with
> vrings which did not have valid callfd (their VHOST_USER_SET_VRING_CALL
> hasn't arrived yet)
> 
> Fixes: 8f972312b8f4 ("vhost: support vhost-user")
> 
> Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk at intel.com>
> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek at intel.com>
> Reviewed-by: Jianfeng Tan <jianfeng.tan at intel.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin at redhat.com>
> ---
>   lib/librte_vhost/vhost_user.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index abcb6ac14..0d30f11ef 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -800,6 +800,11 @@ vhost_user_get_vring_base(struct virtio_net *dev,
>   
>   	vq->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;
>   
> +	if (vq->callfd >= 0)
> +		close(vq->callfd);
> +
> +	vq->callfd = VIRTIO_UNINITIALIZED_EVENTFD;
> +
>   	if (dev->dequeue_zero_copy)
>   		free_zmbufs(vq);
>   	rte_free(vq->shadow_used_ring);
>