[ovs-dev] [ovs-dev, RFC, 3/3] netdev-dpdk: Unlink vhost user socket before creation.

Ben Pfaff blp at ovn.org
Wed Feb 3 21:55:57 UTC 2016


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 <diproiettod at 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.



More information about the dev mailing list