Defer freeing of conntrack zone ids until allocation fails

Instead of freeing the conntrack zone ids from removed ports
right before allocating a new one, wait until there is an
allocation error.  This removes the overhead from the initial
zone id creation.  Since there are 64K zone ids we might not
ever need to clean stale ones anyway.

Change-Id: Ie2d33f8d4650b7798d4ffdeb3ea79cc1092d6c2c

diff --git a/neutron/agent/linux/iptables_firewall.py b/neutron/agent/linux/iptables_firewall.py
index 1be09976a0c2c1fca4adbbd05a1f84967577592d..a42722d70e8e1104eb53c9b765b08a1557c41d26 100644 (file)
--- a/neutron/agent/linux/iptables_firewall.py
+++ b/neutron/agent/linux/iptables_firewall.py
@@ -28,6 +28,7 @@ from neutron.agent.linux import iptables_comments as ic
 from neutron.agent.linux import iptables_manager
 from neutron.agent.linux import utils
 from neutron.common import constants
+from neutron.common import exceptions as n_exc
 from neutron.common import ipv6_utils
 from neutron.extensions import portsecurity as psec
 from neutron.i18n import _LI
@@ -817,7 +818,6 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
         try:
             return self._device_zone_map[short_port_id]
         except KeyError:
-            self._free_zones_from_removed_ports()
             return self._generate_device_zone(short_port_id)
 
     def _free_zones_from_removed_ports(self):
@@ -833,7 +833,13 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
 
     def _generate_device_zone(self, short_port_id):
         """Generates a unique conntrack zone for the passed in ID."""
-        zone = self._find_open_zone()
+        try:
+            zone = self._find_open_zone()
+        except n_exc.CTZoneExhaustedError:
+            # Free some zones and try again, repeat failure will not be caught
+            self._free_zones_from_removed_ports()
+            zone = self._find_open_zone()
+
         self._device_zone_map[short_port_id] = zone
         LOG.debug("Assigned CT zone %(z)s to port %(dev)s.",
                   {'z': zone, 'dev': short_port_id})
@@ -854,8 +860,7 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
                 # gap found, let's use it!
                 return index + 1
         # conntrack zones exhausted :( :(
-        raise RuntimeError("iptables conntrack zones exhausted. "
-                           "iptables rules cannot be applied.")
+        raise n_exc.CTZoneExhaustedError()
 
 
 class OVSHybridIptablesFirewallDriver(IptablesFirewallDriver):

diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py
index 3c05b05e9097c2a25433d02a560a78531c4d6ce2..91edc56a4ed7f1e4db4d615bf6bb6622c8551366 100644 (file)
--- a/neutron/common/exceptions.py
+++ b/neutron/common/exceptions.py
@@ -521,3 +521,8 @@ class NetworkSubnetPoolAffinityError(BadRequest):
 
 class ObjectActionError(NeutronException):
     message = _('Object action %(action)s failed because: %(reason)s')
+
+
+class CTZoneExhaustedError(NeutronException):
+    message = _("IPtables conntrack zones exhausted, iptables rules cannot "
+                "be applied.")

diff --git a/neutron/tests/unit/agent/linux/test_iptables_firewall.py b/neutron/tests/unit/agent/linux/test_iptables_firewall.py
index 61494d851d72083de772470c4c3089e672d8f9f7..adfa2a5c85a63017270a98fab0a348531d8573c0 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_iptables_firewall.py
+++ b/neutron/tests/unit/agent/linux/test_iptables_firewall.py
@@ -26,6 +26,7 @@ from neutron.agent.linux import iptables_comments as ic
 from neutron.agent.linux import iptables_firewall
 from neutron.agent import securitygroups_rpc as sg_cfg
 from neutron.common import constants
+from neutron.common import exceptions as n_exc
 from neutron.tests import base
 from neutron.tests.unit.api.v2 import test_base
 
@@ -1079,7 +1080,8 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase):
                 cmd.extend(['-d', 'fe80::1'])
             else:
                 cmd.extend(['-s', 'fe80::1'])
-        cmd.extend(['-w', 1])
+        # initial data has 1, 2, and 9 in use, CT zone will start at 10.
+        cmd.extend(['-w', 10])
         calls = [
             mock.call(cmd, run_as_root=True, check_exit_code=True,
                       extra_ok_codes=[1])]
@@ -1108,12 +1110,13 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase):
         self.firewall.filtered_ports[port['device']] = new_port
         self.firewall.filter_defer_apply_off()
         calls = [
+            # initial data has 1, 2, and 9 in use, CT zone will start at 10.
             mock.call(['conntrack', '-D', '-f', 'ipv4', '-d', '10.0.0.1',
-                       '-w', 1],
+                       '-w', 10],
                       run_as_root=True, check_exit_code=True,
                       extra_ok_codes=[1]),
             mock.call(['conntrack', '-D', '-f', 'ipv6', '-d', 'fe80::1',
-                       '-w', 1],
+                       '-w', 10],
                       run_as_root=True, check_exit_code=True,
                       extra_ok_codes=[1])]
         self.utils_exec.assert_has_calls(calls)
@@ -1836,11 +1839,12 @@ class OVSHybridIptablesFirewallTestCase(BaseIptablesFirewallTestCase):
     def setUp(self):
         super(OVSHybridIptablesFirewallTestCase, self).setUp()
         self.firewall = iptables_firewall.OVSHybridIptablesFirewallDriver()
+        # inital data has 1, 2, and 9 in use, see RAW_TABLE_OUTPUT above.
+        self._dev_zone_map = {'61634509-31': 2, '8f46cf18-12': 9,
+                              '95c24827-02': 2, 'e804433b-61': 1}
 
     def test__populate_initial_zone_map(self):
-        expected = {'61634509-31': 2, '8f46cf18-12': 9,
-                    '95c24827-02': 2, 'e804433b-61': 1}
-        self.assertEqual(expected, self.firewall._device_zone_map)
+        self.assertEqual(self._dev_zone_map, self.firewall._device_zone_map)
 
     def test__generate_device_zone(self):
         # inital data has 1, 2, and 9 in use.
@@ -1863,12 +1867,17 @@ class OVSHybridIptablesFirewallTestCase(BaseIptablesFirewallTestCase):
         # fill it up and then make sure an extra throws an error
         for i in range(1, 65536):
             self.firewall._device_zone_map['dev-%s' % i] = i
-        with testtools.ExpectedException(RuntimeError):
+        with testtools.ExpectedException(n_exc.CTZoneExhaustedError):
             self.firewall._find_open_zone()
 
+        # with it full, try again, this should trigger a cleanup and return 1
+        self.assertEqual(1, self.firewall._generate_device_zone('p12'))
+        self.assertEqual({'p12': 1}, self.firewall._device_zone_map)
+
     def test_get_device_zone(self):
-        # calling get_device_zone should clear out all of the other entries
-        # since they aren't in the filtered ports list
-        self.assertEqual(1, self.firewall.get_device_zone('12345678901234567'))
+        # initial data has 1, 2, and 9 in use.
+        self.assertEqual(10,
+                         self.firewall.get_device_zone('12345678901234567'))
         # should have been truncated to 11 chars
-        self.assertEqual({'12345678901': 1}, self.firewall._device_zone_map)
+        self._dev_zone_map.update({'12345678901': 10})
+        self.assertEqual(self._dev_zone_map, self.firewall._device_zone_map)