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

diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py
index 2f3c87b6dd1be4dd20eaa0d10dbcb5c50b081f01..00eb744cfa89efa33a75c6c1639b5c3776c4c129 100644 (file)
--- a/neutron/agent/linux/dhcp.py
+++ b/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
 

diff --git a/neutron/api/rpc/handlers/dhcp_rpc.py b/neutron/api/rpc/handlers/dhcp_rpc.py
index d7388e9e0d8b4cd7574bba9a940ab908a564f7ed..959e006c0ca4e6666d4e74150f9f2dc30b00fb3f 100644 (file)
--- a/neutron/api/rpc/handlers/dhcp_rpc.py
+++ b/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')

diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py
index 94e52466fd036cde69296aeb92eda425e2a877b6..cb882975354c2b5c74a06c6fd0808d7893281d09 100644 (file)
--- a/neutron/common/exceptions.py
+++ b/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, "

diff --git a/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py b/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py
index 0f42bfc7ddd553c307a274254eff3ada62a362db..9e772bf06665d9c441e6c087a631102dc5d86b16 100644 (file)
--- a/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py
+++ b/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',