On Thu, 04 Feb 2016 10:02:08 +0300 Ilya Maximets <i.maxim...@samsung.com> wrote:
> On 04.02.2016 00:55, Ben Pfaff wrote: > > On Wed, Feb 03, 2016 at 05:50:19PM +0300, Ilya Maximets wrote: > >> > >> On 03.02.2016 04:56, Daniele Di Proietto 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. > >>> > >>> 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", > >>> > >> > >> I still think that the removal of that we did not create is a bad > >> idea. > > > > It's standard practice for sockets. > > > >> The most proper solution that I come up with is to create unique > >> temporary directory for each socket via mkdtemp(). > >> See my RFC here: > >> http://openvswitch.org/pipermail/dev/2016-February/065583.html . > > > > Then nothing will ever remove the sockets and we'll end up wasting > > inodes essentially forever. > > If sending of untrappable signals is a favourite way for users to > terminate the application when they may use tmpfs for vhost_sock_dir. > tmpfs is actually designed for that. You can't count on that. The system might have years of uptime. -- fbl _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev