On 2 February 2016 at 17:56, Daniele Di Proietto <diproiet...@vmware.com>
wrote:
> If ovs-vswitchd crashes, it will not be able to recreate the same
> vhost user ports, since the socket will still be in the file system.
>
> This commit introduces an unlink() before creation to remove an eventual
> preexisting vhost user socket.
>
> This mimics the behavior of the OpenFlow management socket for a bridge.
>
> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
> ---
> There have been security concerns about the introduction of a similar
> mechanism, because the port name is basically untrusted input.
>
> I decided to post this anyway since:
> * The previous commit introduces a check to discard port names with '/'
> * This is how the OpenFlow management socket is creation is handled.
> As pointed out by Ben, though, an OpenFlow socket always ends in '.mgmt'
> while a vhost user socket doesn't have any suffix.
>
How about introducing something like:
unlink_only_if_is_orphaned_socket(...)?
ls -la is able to indicate that inode is socket (the s prefix):
aatteka@aatteka-PowerEdge-T110:~/Git/openvswitch$ ls -la
/var/run/openvswitch/
srwxr-x--- 1 root root 0 Feb 3 14:43 db.sock
And ss -p is able to confirm existence of the other end of the socket:
aatteka@aatteka-PowerEdge-T110:~/Git/openvswitch$ sudo ss -p | grep
"db.sock"
u_str ESTAB 0 0 /var/run/openvswitch/db.sock 38934130
* 38933374 users:(("ovsdb-server",pid=3658,fd=18))
I did not look into "ss -p" internals so I can't comment about portability
or complexity if you want to pursue this path. Also, I believe it still
does not solve a minor leak problem if ovs-vswitchd was abruptly killed and
the bridge did not show up in OVSDB on the next ovs-vswitchd startup hence
relics of old socket remained there. I don't have a clean solution for this
unless there is convention that OVS can wipe out all contents of a
dedicated directory where Unix Domain Sockets are created.
> What do you guys think?
> ---
> lib/netdev-dpdk.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 6b33c02..e839eaa 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -694,6 +694,11 @@ netdev_dpdk_vhost_user_construct(struct netdev
> *netdev_)
> snprintf(netdev->vhost_id, sizeof(netdev->vhost_id), "%s/%s",
> vhost_sock_dir, name);
>
> + if (unlink(netdev->vhost_id) && errno != ENOENT) {
> + VLOG_WARN("unlinking \"%s\": %s\n",
> + netdev->vhost_id, ovs_strerror(errno));
> + }
> +
> err = rte_vhost_driver_register(netdev->vhost_id);
> if (err) {
> VLOG_ERR("vhost-user socket device setup failure for socket %s\n",
> --
> 2.1.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
