From: Brian Haley Date: Fri, 21 Aug 2015 21:21:44 +0000 (-0400) Subject: Defer freeing of conntrack zone ids until allocation fails X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=a2f1c69d4cf952e8ed36ef3df139bc216127c9c5;p=openstack-build%2Fneutron-build.git 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 1be09976a..a42722d70 100644 --- 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 3c05b05e9..91edc56a4 100644 --- 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 61494d851..adfa2a5c8 100644 --- 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)