diff -Nru neutron-2014.1.5/debian/changelog neutron-2014.1.5/debian/changelog --- neutron-2014.1.5/debian/changelog 2016-07-06 18:52:58.000000000 -0700 +++ neutron-2014.1.5/debian/changelog 2016-08-29 15:07:22.000000000 -0700 @@ -1,3 +1,12 @@ +neutron (1:2014.1.5-0ubuntu6) trusty; urgency=medium + + * iptables_manager can run very slowly when a large number of security group + rules are present (LP: #1453264) + - d/p/use-dictionary-for-iptables-find.patch: Use a dictionary for looking + up iptables rules rather than an iterator. + + -- Billy Olsen Mon, 29 Aug 2016 15:06:06 -0700 + neutron (1:2014.1.5-0ubuntu5) trusty; urgency=medium * Backport performance fix by refactoring logging statements. (LP: #1414218): diff -Nru neutron-2014.1.5/debian/patches/series neutron-2014.1.5/debian/patches/series --- neutron-2014.1.5/debian/patches/series 2016-07-06 18:52:58.000000000 -0700 +++ neutron-2014.1.5/debian/patches/series 2016-08-29 15:05:48.000000000 -0700 @@ -7,3 +7,4 @@ ovs-restart.patch fix-neutron-agent-fanout-queue-not-found-loop.patch refactor-log-in-loop.patch +use-dictionary-for-iptables-find.patch diff -Nru neutron-2014.1.5/debian/patches/use-dictionary-for-iptables-find.patch neutron-2014.1.5/debian/patches/use-dictionary-for-iptables-find.patch --- neutron-2014.1.5/debian/patches/use-dictionary-for-iptables-find.patch 1969-12-31 17:00:00.000000000 -0700 +++ neutron-2014.1.5/debian/patches/use-dictionary-for-iptables-find.patch 2016-08-29 15:05:48.000000000 -0700 @@ -0,0 +1,165 @@ +From cf67a337d79a4d3c7acf5e2170a18d77e0a4c5b9 Mon Sep 17 00:00:00 2001 +From: Kevin Benton +Date: Wed, 6 Jul 2016 16:47:04 -0700 +Subject: [PATCH 1/1] Switch to dictionary for iptables find + +The code to find the matching entry was scanning through a +list of all rules for every rule. This became extremely slow +as the number of rules became large, leading to long delays +waiting for firewall rules to be applied. + +This patch switches to the use of a dictionary so the cost +becomes a hash lookup instead of a list scan. + +Closes-Bug: #1453264 +Closes-Bug: #1455675 +(cherry picked from commit 7a3934d982ef29d8851450b5586319201baa0122) +(cherry picked from commit e6a0e7d27054d5d179c513b3c826a5eaef2077dd) + +Conflicts: + + neutron/agent/linux/iptables_manager.py + +Change-Id: I1e6fe5e50b9c13066c966c252cadc8ed1d08f686 +--- + neutron/agent/linux/iptables_manager.py | 50 +++++++++++++++++++++++------ + neutron/tests/unit/test_iptables_manager.py | 19 +++++++++-- + 2 files changed, 56 insertions(+), 13 deletions(-) + +diff --git a/neutron/agent/linux/iptables_manager.py b/neutron/agent/linux/iptables_manager.py +index 9857a95..fd60874 100644 +--- a/neutron/agent/linux/iptables_manager.py ++++ b/neutron/agent/linux/iptables_manager.py +@@ -21,6 +21,7 @@ + + """Implements iptables rules using linux utilities.""" + ++import collections + import inspect + import os + +@@ -429,12 +430,12 @@ class IptablesManager(object): + + return rules_index + +- def _find_last_entry(self, filter_list, match_str): +- # find a matching entry, starting from the bottom +- for s in reversed(filter_list): +- s = s.strip() +- if match_str in s: +- return s ++ def _find_last_entry(self, filter_map, match_str): ++ # find last matching entry ++ try: ++ return filter_map[match_str][-1] ++ except KeyError: ++ pass + + def _modify_rules(self, current_lines, table, table_name): + unwrapped_chains = table.unwrapped_chains +@@ -457,6 +458,9 @@ class IptablesManager(object): + (old_filter if self.wrap_name in line else + new_filter).append(line.strip()) + ++ old_filter_map = make_filter_map(old_filter) ++ new_filter_map = make_filter_map(new_filter) ++ + rules_index = self._find_rules_index(new_filter) + + all_chains = [':%s' % name for name in unwrapped_chains] +@@ -468,9 +472,9 @@ class IptablesManager(object): + for chain in all_chains: + chain_str = str(chain).strip() + +- old = self._find_last_entry(old_filter, chain_str) ++ old = self._find_last_entry(old_filter_map, chain_str) + if not old: +- dup = self._find_last_entry(new_filter, chain_str) ++ dup = self._find_last_entry(new_filter_map, chain_str) + new_filter = [s for s in new_filter if chain_str not in s.strip()] + + # if no old or duplicates, use original chain +@@ -491,9 +495,9 @@ class IptablesManager(object): + # Further down, we weed out duplicates from the bottom of the + # list, so here we remove the dupes ahead of time. + +- old = self._find_last_entry(old_filter, rule_str) ++ old = self._find_last_entry(old_filter_map, rule_str) + if not old: +- dup = self._find_last_entry(new_filter, rule_str) ++ dup = self._find_last_entry(new_filter_map, rule_str) + new_filter = [s for s in new_filter if rule_str not in s.strip()] + + # if no old or duplicates, use original rule +@@ -640,3 +644,29 @@ class IptablesManager(object): + acc['bytes'] += int(data[1]) + + return acc ++ ++ ++def make_filter_map(filter_list): ++ filter_map = collections.defaultdict(list) ++ for data in filter_list: ++ # strip any [packet:byte] counts at start or end of lines, ++ # for example, chains look like ":neutron-foo - [0:0]" ++ # and rules look like "[0:0] -A neutron-foo..." ++ if data.startswith('['): ++ key = data.rpartition('] ')[2] ++ elif data.endswith(']'): ++ key = data.rsplit(' [', 1)[0] ++ if key.endswith(' -'): ++ key = key[:-2] ++ else: ++ # things like COMMIT, *filter, and *nat land here ++ continue ++ filter_map[key].append(data) ++ # regular IP(v6) entries are translated into /32s or /128s so we ++ # include a lookup without the CIDR here to match as well ++ for cidr in ('/32', '/128'): ++ if cidr in key: ++ alt_key = key.replace(cidr, '') ++ filter_map[alt_key].append(data) ++ # return a regular dict so readers don't accidentally add entries ++ return dict(filter_map) +diff --git a/neutron/tests/unit/test_iptables_manager.py b/neutron/tests/unit/test_iptables_manager.py +index fdd28ed..ad8a660 100644 +--- a/neutron/tests/unit/test_iptables_manager.py ++++ b/neutron/tests/unit/test_iptables_manager.py +@@ -771,11 +771,11 @@ class IptablesManagerStateFulTestCase(base.BaseTestCase): + '[0:0] -A FORWARD -j neutron-filter-top', + '[0:0] -A OUTPUT -j neutron-filter-top' + % IPTABLES_ARG] +- +- return self.iptables._find_last_entry(filter_list, find_str) ++ filter_map = iptables_manager.make_filter_map(filter_list) ++ return self.iptables._find_last_entry(filter_map, find_str) + + def test_find_last_entry_old_dup(self): +- find_str = 'neutron-filter-top' ++ find_str = '-A OUTPUT -j neutron-filter-top' + match_str = '[0:0] -A OUTPUT -j neutron-filter-top' + ret_str = self._test_find_last_entry(find_str) + self.assertEqual(ret_str, match_str) +@@ -785,6 +785,19 @@ class IptablesManagerStateFulTestCase(base.BaseTestCase): + ret_str = self._test_find_last_entry(find_str) + self.assertIsNone(ret_str) + ++ def test_make_filter_map_cidr_stripping(self): ++ filter_rules = ('[0:0] -A OUTPUT -j DROP', ++ '[0:0] -A INPUT -d 192.168.0.2/32 -j DROP', ++ '[0:0] -A INPUT -d 1234:31::001F/128 -j DROP', ++ 'OUTPUT - [0:0]') ++ filter_map = iptables_manager.make_filter_map(filter_rules) ++ # make sure /128 works without CIDR ++ self.assertEqual(filter_rules[2], ++ filter_map['-A INPUT -d 1234:31::001F -j DROP'][0]) ++ # make sure /32 works without CIDR ++ self.assertEqual(filter_rules[1], ++ filter_map['-A INPUT -d 192.168.0.2 -j DROP'][0]) ++ + + class IptablesManagerStateLessTestCase(base.BaseTestCase): + +-- +2.7.4 +