From d7aeb8dd4b1d122e17eef8687192cd122b79fd6e Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Mon, 14 Mar 2016 13:19:54 -0700 Subject: [PATCH] De-dup conntrack deletions before running them During a lot of port deletions, the OVS agent will build up a lot of remote security group member updates for a single device. Once the call to delete all of the removed remote IP conntrack state gets issued, there will be many duplicated entries for the same device in the devices_with_updated_sg_members dicionary of lists. This results in many duplicated calls to remove conntrack entries that are just a waste of time. The longer it takes to remove conntrack entries, the more of these duplicates build up for other pending changes, to the point where there can be hundreds of duplicate calls for a single device. This just adjusts the conntrack manager clearing logic to make sure it de-duplicates all of its delete commands before it issues them. In a local test on a single host I have 11 threads create 11 ports each, plug them into OVS, and then delete them. Here are the number of conntrack delete calls issued: Before this patch - ~232000 With this patch - ~5200 While the remaining number still seems high, the agent is now fast enough to keep up with all of the deletes. Closes-Bug: #1513765 Change-Id: Icba88ab47ee17bf5d6ccdfc0f78bec911987ca90 --- neutron/agent/linux/ip_conntrack.py | 6 +-- .../unit/agent/linux/test_ip_conntrack.py | 37 +++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 neutron/tests/unit/agent/linux/test_ip_conntrack.py diff --git a/neutron/agent/linux/ip_conntrack.py b/neutron/agent/linux/ip_conntrack.py index 4bbd835c1d1..094955e6afa 100644 --- a/neutron/agent/linux/ip_conntrack.py +++ b/neutron/agent/linux/ip_conntrack.py @@ -45,7 +45,7 @@ class IpConntrackManager(object): return cmd_ns def _get_conntrack_cmds(self, device_info_list, rule, remote_ip=None): - conntrack_cmds = [] + conntrack_cmds = set() cmd = self._generate_conntrack_cmd_by_rule(rule, self.namespace) ethertype = rule.get('ethertype') for device_info in device_info_list: @@ -59,7 +59,7 @@ class IpConntrackManager(object): if remote_ip and str( netaddr.IPNetwork(remote_ip).version) in ethertype: ip_cmd.extend(['-s', str(remote_ip)]) - conntrack_cmds.append(cmd + ip_cmd) + conntrack_cmds.add(tuple(cmd + ip_cmd)) return conntrack_cmds def _delete_conntrack_state(self, device_info_list, rule, remote_ip=None): @@ -67,7 +67,7 @@ class IpConntrackManager(object): rule, remote_ip) for cmd in conntrack_cmds: try: - self.execute(cmd, run_as_root=True, + self.execute(list(cmd), run_as_root=True, check_exit_code=True, extra_ok_codes=[1]) except RuntimeError: diff --git a/neutron/tests/unit/agent/linux/test_ip_conntrack.py b/neutron/tests/unit/agent/linux/test_ip_conntrack.py new file mode 100644 index 00000000000..80aa30ba61b --- /dev/null +++ b/neutron/tests/unit/agent/linux/test_ip_conntrack.py @@ -0,0 +1,37 @@ +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import mock + +from neutron.agent.linux import ip_conntrack +from neutron.tests import base + + +class IPConntrackTestCase(base.BaseTestCase): + + def setUp(self): + super(IPConntrackTestCase, self).setUp() + self.execute = mock.Mock() + self.mgr = ip_conntrack.IpConntrackManager(self._zone_lookup, + self.execute) + + def _zone_lookup(self, dev): + return 100 + + def test_delete_conntrack_state_dedupes(self): + rule = {'ethertype': 'IPv4', 'direction': 'ingress'} + dev_info = {'device': 'device', 'fixed_ips': ['1.2.3.4']} + dev_info_list = [dev_info for _ in range(10)] + self.mgr._delete_conntrack_state(dev_info_list, rule) + self.assertEqual(1, len(self.execute.mock_calls))