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.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to