[dpdk-stable] [PATCH v2 1/4] vhost: fix uninitialized vhost queue

[dpdk-stable] [PATCH v2 1/4] vhost: fix uninitialized vhost queue

[Jiayu Hu]

This patch allocates vhost queue by rte_zmalloc() to avoid
undefined values.

Fixes: a277c7159876 ("vhost: refactor code structure")
Cc: stable@dpdk.org

Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
---
 lib/librte_vhost/vhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index c9d1371..8657bbe 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -609,7 +609,7 @@ alloc_vring_queue(struct virtio_net *dev, uint32_t vring_idx)
 		if (dev->virtqueue[i])
 			continue;
 
-		vq = rte_malloc(NULL, sizeof(struct vhost_virtqueue), 0);
+		vq = rte_zmalloc(NULL, sizeof(struct vhost_virtqueue), 0);
 		if (vq == NULL) {
 			VHOST_LOG_CONFIG(ERR,
 				"Failed to allocate memory for vring:%u.\n", i);
-- 
2.7.4

Re: [dpdk-stable] [PATCH v2 1/4] vhost: fix uninitialized vhost queue

[Maxime Coquelin]

On 4/2/21 3:03 PM, Jiayu Hu wrote:
> This patch allocates vhost queue by rte_zmalloc() to avoid
> undefined values.
> 
> Fixes: a277c7159876 ("vhost: refactor code structure")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> ---
>  lib/librte_vhost/vhost.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
> index c9d1371..8657bbe 100644
> --- a/lib/librte_vhost/vhost.c
> +++ b/lib/librte_vhost/vhost.c
> @@ -609,7 +609,7 @@ alloc_vring_queue(struct virtio_net *dev, uint32_t vring_idx)
>  		if (dev->virtqueue[i])
>  			continue;
>  
> -		vq = rte_malloc(NULL, sizeof(struct vhost_virtqueue), 0);
> +		vq = rte_zmalloc(NULL, sizeof(struct vhost_virtqueue), 0);
>  		if (vq == NULL) {
>  			VHOST_LOG_CONFIG(ERR,
>  				"Failed to allocate memory for vring:%u.\n", i);
> 

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

Thanks,
Maxime

[dpdk-stable] [PATCH v3 1/4] vhost: fix uninitialized vhost queue

[Jiayu Hu]

This patch allocates vhost queue by rte_zmalloc() to avoid
undefined values.

Fixes: a277c7159876 ("vhost: refactor code structure")
Cc: stable@dpdk.org

Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Tested-by: Yinan Wang <yinan.wang@intel.com>
---
 lib/librte_vhost/vhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index a70fe01..ea38cf2 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -608,7 +608,7 @@ alloc_vring_queue(struct virtio_net *dev, uint32_t vring_idx)
 		if (dev->virtqueue[i])
 			continue;
 
-		vq = rte_malloc(NULL, sizeof(struct vhost_virtqueue), 0);
+		vq = rte_zmalloc(NULL, sizeof(struct vhost_virtqueue), 0);
 		if (vq == NULL) {
 			VHOST_LOG_CONFIG(ERR,
 				"Failed to allocate memory for vring:%u.\n", i);
-- 
2.7.4

[dpdk-stable] [PATCH v3 3/4] vhost: fix unnecessary vring_state_changed call

[Jiayu Hu]

When VHOST_USER_F_PROTOCOL_FEATURES is not negotiated,
there is no need for vhost_user_set_vring_kick() to
notify the application of vring enabled, as
vhost_user_msg_handler() also notifies the application.

This patch is to remove unnecessary vring_state_changed() call.

Fixes: 966027b4b3a3 ("vhost: fix silent queue enabling with legacy guests")
Cc: stable@dpdk.org

Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
Tested-by: Yinan Wang <yinan.wang@intel.com>
---
 lib/librte_vhost/vhost_user.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index fa8929f..611ff20 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -1922,9 +1922,6 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	 */
 	if (!(dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) {
 		vq->enabled = true;
-		if (dev->notify_ops->vring_state_changed)
-			dev->notify_ops->vring_state_changed(
-				dev->vid, file.index, 1);
 	}
 
 	if (vq->ready) {
-- 
2.7.4

Re: [dpdk-stable] [PATCH v3 3/4] vhost: fix unnecessary vring_state_changed call

[Maxime Coquelin]

On 4/20/21 10:57 AM, Jiayu Hu wrote:
> When VHOST_USER_F_PROTOCOL_FEATURES is not negotiated,
> there is no need for vhost_user_set_vring_kick() to
> notify the application of vring enabled, as
> vhost_user_msg_handler() also notifies the application.
> 
> This patch is to remove unnecessary vring_state_changed() call.
> 
> Fixes: 966027b4b3a3 ("vhost: fix silent queue enabling with legacy guests")

Sorry, I thought the vring_state_changed cb was introduced when Matan
did the rework last year, but it is actually older.

If we backport the patch on v19.11, we will create a regression as it
will revert Ilya's patch.

I think that your fix is correct only once Matan's series is present, so
we should have below Fixes tag:

Fixes: d0fcc38f5fa4 ("vhost: improve device readiness notifications")

> Cc: stable@dpdk.org
> 
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> Tested-by: Yinan Wang <yinan.wang@intel.com>
> ---
>  lib/librte_vhost/vhost_user.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index fa8929f..611ff20 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -1922,9 +1922,6 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *msg,
>  	 */
>  	if (!(dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) {
>  		vq->enabled = true;
> -		if (dev->notify_ops->vring_state_changed)
> -			dev->notify_ops->vring_state_changed(
> -				dev->vid, file.index, 1);
>  	}
>  
>  	if (vq->ready) {
> 

With the fixes tag changed:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Chanbo, can you change the Fixes tag while applying or Jiayu needs to
send a new revision?

Thanks,
Maxime

Re: [dpdk-stable] [PATCH v3 3/4] vhost: fix unnecessary vring_state_changed call

[Xia, Chenbo]

Hi Maxime & Jiayu,

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, April 20, 2021 5:40 PM
> To: Hu, Jiayu <jiayu.hu@intel.com>; dev@dpdk.org; Xia, Chenbo
> <chenbo.xia@intel.com>
> Cc: Wang, Yinan <yinan.wang@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>;
> Jiang, Cheng1 <cheng1.jiang@intel.com>; stable@dpdk.org
> Subject: Re: [PATCH v3 3/4] vhost: fix unnecessary vring_state_changed call
> 
> 
> 
> On 4/20/21 10:57 AM, Jiayu Hu wrote:
> > When VHOST_USER_F_PROTOCOL_FEATURES is not negotiated,
> > there is no need for vhost_user_set_vring_kick() to
> > notify the application of vring enabled, as
> > vhost_user_msg_handler() also notifies the application.
> >
> > This patch is to remove unnecessary vring_state_changed() call.
> >
> > Fixes: 966027b4b3a3 ("vhost: fix silent queue enabling with legacy guests")
> 
> Sorry, I thought the vring_state_changed cb was introduced when Matan
> did the rework last year, but it is actually older.
> 
> If we backport the patch on v19.11, we will create a regression as it
> will revert Ilya's patch.
> 
> I think that your fix is correct only once Matan's series is present, so
> we should have below Fixes tag:
> 
> Fixes: d0fcc38f5fa4 ("vhost: improve device readiness notifications")
> 
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> > Tested-by: Yinan Wang <yinan.wang@intel.com>
> > ---
> >  lib/librte_vhost/vhost_user.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> > index fa8929f..611ff20 100644
> > --- a/lib/librte_vhost/vhost_user.c
> > +++ b/lib/librte_vhost/vhost_user.c
> > @@ -1922,9 +1922,6 @@ vhost_user_set_vring_kick(struct virtio_net **pdev,
> struct VhostUserMsg *msg,
> >  	 */
> >  	if (!(dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) {
> >  		vq->enabled = true;
> > -		if (dev->notify_ops->vring_state_changed)
> > -			dev->notify_ops->vring_state_changed(
> > -				dev->vid, file.index, 1);
> >  	}
> >
> >  	if (vq->ready) {
> >
> 
> With the fixes tag changed:
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Chanbo, can you change the Fixes tag while applying or Jiayu needs to
> send a new revision?

No worry. Will change fix tag while applying.

Thanks!
Chenbo

> 
> Thanks,
> Maxime