Re: [dpdk-dev] [PATCH v2 2/3] virtio_user: don't set/get_status until FEATURES_OK

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 10/20/20 5:20 PM, Adrian Moreno wrote:
> According to the virtio spec, ACK and DRIVER status bits should be set
> before feature negotiation.
> 
> However, until the protocol features are negotiated, the driver does not
> know if the device actually supports the those vhost-user messages.

s/the those/those/

> Therefore, until FEATURES_OK is set, the GET/SET_STATUS messages should
> not be sent.
> 
> Fixes: 57912824615f ("net/virtio-user: support vhost status setting")
> Cc: maxime.coquelin@redhat.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  drivers/net/virtio/virtio_user/vhost_user.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
> index ef290c357..450d77e92 100644
> --- a/drivers/net/virtio/virtio_user/vhost_user.c
> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
> @@ -278,8 +278,9 @@ vhost_user_sock(struct virtio_user_dev *dev,
>  
>  	switch (req) {
>  	case VHOST_USER_GET_STATUS:
> -		if (!(dev->protocol_features &
> -				(1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
> +		if (!(dev->status & VIRTIO_CONFIG_STATUS_FEATURES_OK) ||
> +		    (!(dev->protocol_features &
> +				(1ULL << VHOST_USER_PROTOCOL_F_STATUS))))
>  			return 0;
>  		/* Fallthrough */
>  	case VHOST_USER_GET_FEATURES:
> @@ -288,8 +289,9 @@ vhost_user_sock(struct virtio_user_dev *dev,
>  		break;
>  
>  	case VHOST_USER_SET_STATUS:
> -		if (!(dev->protocol_features &
> -				(1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
> +		if (!(dev->status & VIRTIO_CONFIG_STATUS_FEATURES_OK) ||
> +		    (!(dev->protocol_features &
> +				(1ULL << VHOST_USER_PROTOCOL_F_STATUS))))
>  			return 0;
>  
>  		if (has_reply_ack)
> 


Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime