Avoid race condition for reserved DHCP ports

This patch introduces mechanism similar to compare-and-swap
for updating reserved DHCP port.

This addresses a case when two DHCP agents that start nearly at
the same time are assigned to one network and there is a reserved
DHCP port in the network. Then each of agents will try to use it
because agents don't check if reserved port is still available.
Reserved DHCP port can be acquired by different agent between calls to
get_active_networks and update_port, so this patch adds a check for
this case.

Change-Id: I0277ab537ff9d3a664c03ea291b9ec2b0e784dbb
Closes-Bug: #1425402

neutron/agent/linux/dhcp.py

@@ -23,6 +23,7 @@ import time
 import netaddr
 from oslo_config import cfg
 from oslo_log import log as logging
+import oslo_messaging
 from oslo_utils import uuidutils
 import six
 
@@ -1092,9 +1093,16 @@ class DeviceManager(object):
         for port in network.ports:
             port_device_id = getattr(port, 'device_id', None)
             if port_device_id == constants.DEVICE_ID_RESERVED_DHCP_PORT:
-                port = self.plugin.update_dhcp_port(
-                    port.id, {'port': {'network_id': network.id,
-                                       'device_id': device_id}})
+                try:
+                    port = self.plugin.update_dhcp_port(
+                        port.id, {'port': {'network_id': network.id,
+                                           'device_id': device_id}})
+                except oslo_messaging.RemoteError as e:
+                    if e.exc_type == exceptions.DhcpPortInUse:
+                        LOG.info(_LI("Skipping DHCP port %s as it is "
+                                     "already in use"), port.id)
+                        continue
+                    raise
                 if port:
                     return port
 

neutron/api/rpc/handlers/dhcp_rpc.py

@@ -214,10 +214,15 @@ class DhcpRpcCallback(object):
         host = kwargs.get('host')
         port = kwargs.get('port')
         port['id'] = kwargs.get('port_id')
+        port['port'][portbindings.HOST_ID] = host
+        plugin = manager.NeutronManager.get_plugin()
+        old_port = plugin.get_port(context, port['id'])
+        if (old_port['device_id'] != constants.DEVICE_ID_RESERVED_DHCP_PORT
+            and old_port['device_id'] !=
+            utils.get_dhcp_agent_device_id(port['port']['network_id'], host)):
+            raise n_exc.DhcpPortInUse(port_id=port['id'])
         LOG.debug('Update dhcp port %(port)s '
                   'from %(host)s.',
                   {'port': port,
                    'host': host})
-        port['port'][portbindings.HOST_ID] = host
-        plugin = manager.NeutronManager.get_plugin()
         return self._port_action(plugin, context, port, 'update_port')

neutron/common/exceptions.py

@@ -172,6 +172,10 @@ class ServicePortInUse(InUse):
                 "port API: %(reason)s")
 
 
+class DhcpPortInUse(InUse):
+    message = _("Port %(port_id)s is already acquired by another DHCP agent")
+
+
 class PortBound(InUse):
     message = _("Unable to complete operation on port %(port_id)s, "
                 "port is already bound, port type: %(vif_type)s, "

neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py

@@ -19,6 +19,7 @@ from oslo_db import exception as db_exc
 from neutron.api.rpc.handlers import dhcp_rpc
 from neutron.common import constants
 from neutron.common import exceptions as n_exc
+from neutron.common import utils
 from neutron.tests import base
 
 
@@ -177,12 +178,46 @@ class TestDhcpRpcCallback(base.BaseTestCase):
         def _fake_port_action(plugin, context, port, action):
             self.assertEqual(expected_port, port)
 
+        self.plugin.get_port.return_value = {
+            'device_id': constants.DEVICE_ID_RESERVED_DHCP_PORT}
         self.callbacks._port_action = _fake_port_action
         self.callbacks.update_dhcp_port(mock.Mock(),
                                         host='foo_host',
                                         port_id='foo_port_id',
                                         port=port)
 
+    def test_update_reserved_dhcp_port(self):
+        port = {'port': {'network_id': 'foo_network_id',
+                         'device_owner': constants.DEVICE_OWNER_DHCP,
+                         'fixed_ips': [{'subnet_id': 'foo_subnet_id'}]}
+                }
+        expected_port = {'port': {'network_id': 'foo_network_id',
+                                  'device_owner': constants.DEVICE_OWNER_DHCP,
+                                  'binding:host_id': 'foo_host',
+                                  'fixed_ips': [{'subnet_id': 'foo_subnet_id'}]
+                                  },
+                         'id': 'foo_port_id'
+                         }
+
+        def _fake_port_action(plugin, context, port, action):
+            self.assertEqual(expected_port, port)
+
+        self.plugin.get_port.return_value = {
+            'device_id': utils.get_dhcp_agent_device_id('foo_network_id',
+                                                        'foo_host')}
+        self.callbacks._port_action = _fake_port_action
+        self.callbacks.update_dhcp_port(
+            mock.Mock(), host='foo_host', port_id='foo_port_id', port=port)
+
+        self.plugin.get_port.return_value = {
+            'device_id': 'other_id'}
+        self.assertRaises(n_exc.DhcpPortInUse,
+                          self.callbacks.update_dhcp_port,
+                          mock.Mock(),
+                          host='foo_host',
+                          port_id='foo_port_id',
+                          port=port)
+
     def test_update_dhcp_port(self):
         port = {'port': {'network_id': 'foo_network_id',
                          'device_owner': constants.DEVICE_OWNER_DHCP,
@@ -195,6 +230,8 @@ class TestDhcpRpcCallback(base.BaseTestCase):
                                   },
                          'id': 'foo_port_id'
                          }
+        self.plugin.get_port.return_value = {
+            'device_id': constants.DEVICE_ID_RESERVED_DHCP_PORT}
         self.callbacks.update_dhcp_port(mock.Mock(),
                                         host='foo_host',
                                         port_id='foo_port_id',