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

Reply via email to