DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] vhost: fix vhost-user init failed
@ 2017-07-10  8:06 Zhiyong Yang
  2017-07-10  9:38 ` Maxime Coquelin
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Zhiyong Yang @ 2017-07-10  8:06 UTC (permalink / raw)
  To: dev; +Cc: yliu, maxime.coquelin, lei.a.yao, Zhiyong Yang

Exception handling is executed in the normal path and it will cause
vhost-user init failure.
Fixes: d6983a70e259("vhost: check return of pthread calls")

Reported-by: Lei Yao <lei.a.yao@intel.com>
Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
---
 lib/librte_vhost/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 57b86c0..9d2049c 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -668,7 +668,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
 	}
 
 	vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;
-
+	goto out;
 out_mutex:
 	if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
 		RTE_LOG(ERR, VHOST_CONFIG,
-- 
2.9.3

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: fix vhost-user init failed
  2017-07-10  8:06 [dpdk-dev] [PATCH] vhost: fix vhost-user init failed Zhiyong Yang
@ 2017-07-10  9:38 ` Maxime Coquelin
  2017-07-10  9:48 ` Jens Freimann
  2017-07-13  8:18 ` Yao, Lei A
  2 siblings, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2017-07-10  9:38 UTC (permalink / raw)
  To: Zhiyong Yang, dev; +Cc: yliu, lei.a.yao



On 07/10/2017 10:06 AM, Zhiyong Yang wrote:
> Exception handling is executed in the normal path and it will cause
> vhost-user init failure.
> Fixes: d6983a70e259("vhost: check return of pthread calls")
> 
> Reported-by: Lei Yao <lei.a.yao@intel.com>
> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> ---
>   lib/librte_vhost/socket.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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 vhost-user init failed
  2017-07-10  8:06 [dpdk-dev] [PATCH] vhost: fix vhost-user init failed Zhiyong Yang
  2017-07-10  9:38 ` Maxime Coquelin
@ 2017-07-10  9:48 ` Jens Freimann
  2017-07-12  2:41   ` Liu, Yu Y
  2017-07-14  2:43   ` Yuanhan Liu
  2017-07-13  8:18 ` Yao, Lei A
  2 siblings, 2 replies; 6+ messages in thread
From: Jens Freimann @ 2017-07-10  9:48 UTC (permalink / raw)
  To: Zhiyong Yang; +Cc: dev, yliu, maxime.coquelin, lei.a.yao

On Mon, Jul 10, 2017 at 04:06:48PM +0800, Zhiyong Yang wrote:
>Exception handling is executed in the normal path and it will cause
>vhost-user init failure.
>Fixes: d6983a70e259("vhost: check return of pthread calls")
>
>Reported-by: Lei Yao <lei.a.yao@intel.com>
>Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
>---
> lib/librte_vhost/socket.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
>index 57b86c0..9d2049c 100644
>--- a/lib/librte_vhost/socket.c
>+++ b/lib/librte_vhost/socket.c
>@@ -668,7 +668,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
> 	}
>
> 	vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;
>-
>+	goto out;
> out_mutex:
> 	if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
> 		RTE_LOG(ERR, VHOST_CONFIG,

Thanks for fixing this!

Sorry for introducing this bug, I was about to send this before I saw
your fix: 

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 57b86c0..b2158a7 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -668,6 +668,9 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
        }

        vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;
+out:
+       pthread_mutex_unlock(&vhost_user.mutex);
+       return ret;

 out_mutex:
        if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
@@ -677,9 +680,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
 out_free:
        free(vsocket->path);
        free(vsocket);
-out:
        pthread_mutex_unlock(&vhost_user.mutex);
-
        return ret;
 }

Both works fine, so I leave it up to the maintainers how to fix. 

Reviewed-by: Jens Freimann <jfreimann@redhat.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: fix vhost-user init failed
  2017-07-10  9:48 ` Jens Freimann
@ 2017-07-12  2:41   ` Liu, Yu Y
  2017-07-14  2:43   ` Yuanhan Liu
  1 sibling, 0 replies; 6+ messages in thread
From: Liu, Yu Y @ 2017-07-12  2:41 UTC (permalink / raw)
  To: Jens Freimann, Yang, Zhiyong; +Cc: dev, yliu, maxime.coquelin, Yao, Lei A

This is needed for RC2 as lots of test cases are affected.

Thanks & Regards,
Yu Liu

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jens Freimann
Sent: Monday, July 10, 2017 5:48 PM
To: Yang, Zhiyong <zhiyong.yang@intel.com>
Cc: dev@dpdk.org; yliu@fridaylinux.org; maxime.coquelin@redhat.com; Yao, Lei A <lei.a.yao@intel.com>
Subject: Re: [dpdk-dev] [PATCH] vhost: fix vhost-user init failed

On Mon, Jul 10, 2017 at 04:06:48PM +0800, Zhiyong Yang wrote:
>Exception handling is executed in the normal path and it will cause 
>vhost-user init failure.
>Fixes: d6983a70e259("vhost: check return of pthread calls")
>
>Reported-by: Lei Yao <lei.a.yao@intel.com>
>Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
>---
> lib/librte_vhost/socket.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c 
>index 57b86c0..9d2049c 100644
>--- a/lib/librte_vhost/socket.c
>+++ b/lib/librte_vhost/socket.c
>@@ -668,7 +668,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
> 	}
>
> 	vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;
>-
>+	goto out;
> out_mutex:
> 	if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
> 		RTE_LOG(ERR, VHOST_CONFIG,

Thanks for fixing this!

Sorry for introducing this bug, I was about to send this before I saw your fix: 

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c index 57b86c0..b2158a7 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -668,6 +668,9 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
        }

        vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;
+out:
+       pthread_mutex_unlock(&vhost_user.mutex);
+       return ret;

 out_mutex:
        if (pthread_mutex_destroy(&vsocket->conn_mutex)) { @@ -677,9 +680,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
 out_free:
        free(vsocket->path);
        free(vsocket);
-out:
        pthread_mutex_unlock(&vhost_user.mutex);
-
        return ret;
 }

Both works fine, so I leave it up to the maintainers how to fix. 

Reviewed-by: Jens Freimann <jfreimann@redhat.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: fix vhost-user init failed
  2017-07-10  8:06 [dpdk-dev] [PATCH] vhost: fix vhost-user init failed Zhiyong Yang
  2017-07-10  9:38 ` Maxime Coquelin
  2017-07-10  9:48 ` Jens Freimann
@ 2017-07-13  8:18 ` Yao, Lei A
  2 siblings, 0 replies; 6+ messages in thread
From: Yao, Lei A @ 2017-07-13  8:18 UTC (permalink / raw)
  To: Yang, Zhiyong, dev; +Cc: yliu, maxime.coquelin



> -----Original Message-----
> From: Yang, Zhiyong
> Sent: Monday, July 10, 2017 4:07 PM
> To: dev@dpdk.org
> Cc: yliu@fridaylinux.org; maxime.coquelin@redhat.com; Yao, Lei A
> <lei.a.yao@intel.com>; Yang, Zhiyong <zhiyong.yang@intel.com>
> Subject: [PATCH] vhost: fix vhost-user init failed
> 
> Exception handling is executed in the normal path and it will cause
> vhost-user init failure.
> Fixes: d6983a70e259("vhost: check return of pthread calls")
> 
> Reported-by: Lei Yao <lei.a.yao@intel.com>
> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
Tested-by: Lei Yao <lei.a.yao@intel.com>
This patch can fix the vhost-init issue on my server. 

> ---
>  lib/librte_vhost/socket.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index 57b86c0..9d2049c 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -668,7 +668,7 @@ rte_vhost_driver_register(const char *path, uint64_t
> flags)
>  	}
> 
>  	vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;
> -
> +	goto out;
>  out_mutex:
>  	if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
>  		RTE_LOG(ERR, VHOST_CONFIG,
> --
> 2.9.3

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: fix vhost-user init failed
  2017-07-10  9:48 ` Jens Freimann
  2017-07-12  2:41   ` Liu, Yu Y
@ 2017-07-14  2:43   ` Yuanhan Liu
  1 sibling, 0 replies; 6+ messages in thread
From: Yuanhan Liu @ 2017-07-14  2:43 UTC (permalink / raw)
  To: Jens Freimann; +Cc: Zhiyong Yang, dev, maxime.coquelin, lei.a.yao

On Mon, Jul 10, 2017 at 11:48:27AM +0200, Jens Freimann wrote:
> >@@ -668,7 +668,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
> >	}
> >
> >	vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;
> >-
> >+	goto out;
> >out_mutex:
> >	if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
> >		RTE_LOG(ERR, VHOST_CONFIG,
> 
> Thanks for fixing this!
> 
> Sorry for introducing this bug, I was about to send this before I saw
> your fix:
> 
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index 57b86c0..b2158a7 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -668,6 +668,9 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
>        }
> 
>        vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;
> +out:
> +       pthread_mutex_unlock(&vhost_user.mutex);
> +       return ret;
> 
> out_mutex:
>        if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
> @@ -677,9 +680,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
> out_free:
>        free(vsocket->path);
>        free(vsocket);
> -out:
>        pthread_mutex_unlock(&vhost_user.mutex);
> -
>        return ret;
> }
> 
> Both works fine, so I leave it up to the maintainers how to fix.
> 
> Reviewed-by: Jens Freimann <jfreimann@redhat.com>

I actually prefers below: I don't want a simple "out" on top of "out_mutex"
and "out_free", and an unconditional "goto" looks weird to me.

---
@@ -669,6 +669,9 @@ rte_vhost_driver_register(const char *path, uint64_t flags)

        vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;

+       pthread_mutex_unlock(&vhost_user.mutex);
+       return ret;
+
 out_mutex:
        if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
                RTE_LOG(ERR, VHOST_CONFIG,


Applied to dpdk-next-virtio, with above changes.

Thanks!

	--yliu

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-07-14  2:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-10  8:06 [dpdk-dev] [PATCH] vhost: fix vhost-user init failed Zhiyong Yang
2017-07-10  9:38 ` Maxime Coquelin
2017-07-10  9:48 ` Jens Freimann
2017-07-12  2:41   ` Liu, Yu Y
2017-07-14  2:43   ` Yuanhan Liu
2017-07-13  8:18 ` Yao, Lei A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).