[dpdk-dev] [PATCH] vhost user: unlink sockaddr when poll sched fails

[dpdk-dev] [PATCH] vhost user: unlink sockaddr when poll sched fails

[Gellert Babel]

From: Jan Wickbom <jan.wickbom@ericsson.com>

Issue:

Vhost user socket addresses left in /var/run/openvswitch.
This will lead to failure to add vhost user ports with names that
already exist in this directory.

When there is a failure to add a vhost user socket file descriptor to
the file descriptor set using fdset_add() in
rte_vhost_driver_register() the address bound to the socket is not
released.

Solution:
Add unlink of the file path corresponding to the socket address.

Signed-off-by: Jan Wickbom <jan.wickbom@ericsson.com>
Signed-off-by: Gellert Babel <gellert.babel@ericsson.com>
---
 lib/librte_vhost/socket.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 41aa3f9..eb9dae2 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -352,6 +352,7 @@ struct vhost_user {
 
 err:
 	close(fd);
+	unlink(path);
 	return -1;
 }
 
-- 
1.9.1

Re: [dpdk-dev] [PATCH] vhost user: unlink sockaddr when poll sched fails

[Tan, Jianfeng]

Hi,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Gellert Babel
> Sent: Monday, November 20, 2017 2:59 PM
> To: yliu@fridaylinux.org; maxime.coquelin@redhat.com
> Cc: dev@dpdk.org; Jan Wickbom; Gellert Babel
> Subject: [dpdk-dev] [PATCH] vhost user: unlink sockaddr when poll sched
> fails
> 
> From: Jan Wickbom <jan.wickbom@ericsson.com>
> 
> Issue:
> 
> Vhost user socket addresses left in /var/run/openvswitch.
> This will lead to failure to add vhost user ports with names that
> already exist in this directory.
> 
> When there is a failure to add a vhost user socket file descriptor to
> the file descriptor set using fdset_add() in
> rte_vhost_driver_register() the address bound to the socket is not
> released.
> 
> Solution:
> Add unlink of the file path corresponding to the socket address.

I'm afraid this overkills. Suppose we have an existing port binding on path1; and we (wrongly) add another port path1. This patch would remove the previous socket on path1.

Thanks,
Jianfeng

> 
> Signed-off-by: Jan Wickbom <jan.wickbom@ericsson.com>
> Signed-off-by: Gellert Babel <gellert.babel@ericsson.com>
> ---
>  lib/librte_vhost/socket.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index 41aa3f9..eb9dae2 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -352,6 +352,7 @@ struct vhost_user {
> 
>  err:
>  	close(fd);
> +	unlink(path);
>  	return -1;
>  }
> 
> --
> 1.9.1

Re: [dpdk-dev] [PATCH] vhost user: unlink sockaddr when poll sched fails

[Ilya Maximets]

Hi everyone,

This issue is already discussed many times here and in OVS mail-lists.
The basic restart issue was resolved by removing the sockets at exit
in OVS. The crash scenario is not handled but you're able to remove
all of the left files by some external script before restarting the OVS.

That is one of the discussions about:
http://dpdk.org/ml/archives/dev/2015-December/030326.html

The second point is that vhostuser ports are dpricated now in OVS and
will be removed in future release. So, it's better for you to migrate to
vhostuserclient and everything will be fine because QEMU properly handles
sockets itself.

So, IMHO, we should not add such overkill and, potentially, very dangerous
solutions to DPDK just to fix minor issue in deprecated OVS code.

Best regards, Ilya Maximets.

> From: Jan Wickbom <jan.wickbom at ericsson.com>
> 
> Issue:
> 
> Vhost user socket addresses left in /var/run/openvswitch.
> This will lead to failure to add vhost user ports with names that
> already exist in this directory.
> 
> When there is a failure to add a vhost user socket file descriptor to
> the file descriptor set using fdset_add() in
> rte_vhost_driver_register() the address bound to the socket is not
> released.
> 
> Solution:
> Add unlink of the file path corresponding to the socket address.
> 
> Signed-off-by: Jan Wickbom <jan.wickbom at ericsson.com>
> Signed-off-by: Gellert Babel <gellert.babel at ericsson.com>
> ---
>  lib/librte_vhost/socket.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index 41aa3f9..eb9dae2 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -352,6 +352,7 @@ struct vhost_user {
>  
>  err:
>  	close(fd);
> +	unlink(path);
>  	return -1;
>  }
>  
> -- 
> 1.9.1