* [dpdk-dev] [PATCH] vhost: fix silent queue enabling with legacy guests
2019-04-12 13:09 ` [dpdk-dev] [PATCH] vhost: fix silent queue enabling with legacy guests Ilya Maximets
@ 2019-04-12 13:09 ` Ilya Maximets
2019-04-17 7:32 ` Maxime Coquelin
2019-04-17 7:53 ` Maxime Coquelin
2 siblings, 0 replies; 6+ messages in thread
From: Ilya Maximets @ 2019-04-12 13:09 UTC (permalink / raw)
To: dev, Maxime Coquelin
Cc: Tiwei Bie, Jens Freimann, David Marchand, Ilya Maximets, stable
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);
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: fix silent queue enabling with legacy guests
2019-04-12 13:09 ` [dpdk-dev] [PATCH] vhost: fix silent queue enabling with legacy guests Ilya Maximets
2019-04-12 13:09 ` Ilya Maximets
@ 2019-04-17 7:32 ` Maxime Coquelin
2019-04-17 7:32 ` Maxime Coquelin
2019-04-17 7:53 ` Maxime Coquelin
2 siblings, 1 reply; 6+ messages in thread
From: Maxime Coquelin @ 2019-04-17 7:32 UTC (permalink / raw)
To: Ilya Maximets, dev; +Cc: Tiwei Bie, Jens Freimann, David Marchand, stable
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: fix silent queue enabling with legacy guests
2019-04-17 7:32 ` Maxime Coquelin
@ 2019-04-17 7:32 ` Maxime Coquelin
0 siblings, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2019-04-17 7:32 UTC (permalink / raw)
To: Ilya Maximets, dev; +Cc: Tiwei Bie, Jens Freimann, David Marchand, stable
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: fix silent queue enabling with legacy guests
2019-04-12 13:09 ` [dpdk-dev] [PATCH] vhost: fix silent queue enabling with legacy guests Ilya Maximets
2019-04-12 13:09 ` Ilya Maximets
2019-04-17 7:32 ` Maxime Coquelin
@ 2019-04-17 7:53 ` Maxime Coquelin
2019-04-17 7:53 ` Maxime Coquelin
2 siblings, 1 reply; 6+ messages in thread
From: Maxime Coquelin @ 2019-04-17 7:53 UTC (permalink / raw)
To: Ilya Maximets, dev; +Cc: Tiwei Bie, Jens Freimann, David Marchand, stable
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: fix silent queue enabling with legacy guests
2019-04-17 7:53 ` Maxime Coquelin
@ 2019-04-17 7:53 ` Maxime Coquelin
0 siblings, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2019-04-17 7:53 UTC (permalink / raw)
To: Ilya Maximets, dev; +Cc: Tiwei Bie, Jens Freimann, David Marchand, stable
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
^ permalink raw reply [flat|nested] 6+ messages in thread