[dpdk-dev] [PATCH] vhost: stop device before updating public vring data

[Maxime Coquelin]

Hi Tomasz,

On 03/05/2018 05:11 PM, Tomasz Kulasek wrote:
> For now DPDK assumes that callfd, kickfd and last_idx are being set just
> once during vring initialization and device cannot be running while DPDK
> receives SET_VRING_KICK, SET_VRING_CALL and SET_VRING_BASE messages.
> However, that assumption is wrong. For Vhost SCSI messages might arrive
> at any point of time, possibly multiple times, one after another.
> 
> QEMU issues SET_VRING_CALL once during device initialization, then again
> during device start. The second message will close previous callfd,
> which is still being used by the user-implementation of vhost device.
> This results in writing to invalid (closed) callfd.
> 
> Other messages like SET_FEATURES, SET_VRING_ADDR etc also will change
> internal state of VQ or device. To prevent race condition device should
> also be stopped before updateing vring data.
> 
> Signed-off-by: Dariusz Stojaczyk<dariuszx.stojaczyk at intel.com>
> Signed-off-by: Pawel Wodkowski<pawelx.wodkowski at intel.com>
> Signed-off-by: Tomasz Kulasek<tomaszx.kulasek at intel.com>
> ---
>   lib/librte_vhost/vhost_user.c | 40 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 40 insertions(+)

In last release, we have introduced a per-virtqueue lock to protect
vring handling against asynchronous device changes.

I think that would solve the issue you are facing, but you would need
to export the VQs locking functions to the vhost-user lib API to be
able to use it.

I don't think your current patch is the right solution anyway, because
it destroys the device in case we don't want it to remain alive, like
set_log_base, or set_features when only the logging feature gets
enabled.

Cheers,
Maxime