On Thu, Feb 04, 2016 at 10:02:08AM +0300, Ilya Maximets 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.
>
> Anyway, termination of ovs with open vhost-user ports is a highly unlikely
> scenario because this requires restarting of all VMs.
Much as we work to make OVS highly reliable, segfaults happen sometimes.
Usually they only happen once, and then if it's one of our customers
they send us a core dump and we track it down and fix it. But once in a
while there's a bug that causes repeated segfaults. We're careful to
make sure that that only yields a single core dump, to avoid consuming
disk space. I don't like the idea that in a case like this we have to
worry about slowly eating up all the inodes. It's a corner case of a
corner case, but... it happens.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
