diff mbox series

vhost: fix silent queue enabling with legacy guests

Message ID 20190412130949.14299-1-i.maximets@samsung.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: fix silent queue enabling with legacy guests |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Ilya Maximets April 12, 2019, 1:09 p.m. UTC 
  vhost should notify the application in case of all vring state changes.

In general, application should not care about negotiation of
VHOST_USER_F_PROTOCOL_FEATURES. Protocol details like this should
be hidden by the vhost library.

With this patch applications like OVS will be able to assume that
all vrings disabled by default and only process 'vring_state_changed'
events.

Fixes: 321203a54ba7 ("vhost: enable rings at the right time")
Cc: stable@dpdk.org

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 lib/librte_vhost/vhost_user.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Maxime Coquelin April 17, 2019, 7:32 a.m. UTC | #1 
On 4/12/19 3:09 PM, Ilya Maximets wrote:
> vhost should notify the application in case of all vring state changes.
> 
> In general, application should not care about negotiation of
> VHOST_USER_F_PROTOCOL_FEATURES. Protocol details like this should
> be hidden by the vhost library.
> 
> With this patch applications like OVS will be able to assume that
> all vrings disabled by default and only process 'vring_state_changed'
> events.
> 
> Fixes: 321203a54ba7 ("vhost: enable rings at the right time")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>   lib/librte_vhost/vhost_user.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 23beed97d..c9e29ece8 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -1231,8 +1231,12 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *msg,
>   	 * the ring starts already enabled. Otherwise, it is enabled via
>   	 * the SET_VRING_ENABLE message.
>   	 */
> -	if (!(dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)))
> +	if (!(dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) {
>   		vq->enabled = 1;
> +		if (dev->notify_ops->vring_state_changed)
> +			dev->notify_ops->vring_state_changed(
> +				dev->vid, file.index, 1);
> +	}
>   
>   	if (vq->kickfd >= 0)
>   		close(vq->kickfd);
> 

Nice, this is indeed the right thing to do:

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

Thanks,
Maxime
Maxime Coquelin April 17, 2019, 7:53 a.m. UTC | #2 
On 4/12/19 3:09 PM, Ilya Maximets wrote:
> vhost should notify the application in case of all vring state changes.
> 
> In general, application should not care about negotiation of
> VHOST_USER_F_PROTOCOL_FEATURES. Protocol details like this should
> be hidden by the vhost library.
> 
> With this patch applications like OVS will be able to assume that
> all vrings disabled by default and only process 'vring_state_changed'
> events.
> 
> Fixes: 321203a54ba7 ("vhost: enable rings at the right time")
> Cc:stable@dpdk.org
> 
> Signed-off-by: Ilya Maximets<i.maximets@samsung.com>
> ---
>   lib/librte_vhost/vhost_user.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)


Applied to dpdk-next-virtio/master.

Thanks,
Maxime
diff mbox series

Patch

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 23beed97d..c9e29ece8 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -1231,8 +1231,12 @@  vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	 * the ring starts already enabled. Otherwise, it is enabled via
 	 * the SET_VRING_ENABLE message.
 	 */
-	if (!(dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)))
+	if (!(dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) {
 		vq->enabled = 1;
+		if (dev->notify_ops->vring_state_changed)
+			dev->notify_ops->vring_state_changed(
+				dev->vid, file.index, 1);
+	}
 
 	if (vq->kickfd >= 0)
 		close(vq->kickfd);