From 7e117c13fd3fb125c857dadfa2945799b39e1634 Mon Sep 17 00:00:00 2001 From: Rawlin Peters Date: Thu, 18 Jun 2015 11:22:13 -0600 Subject: [PATCH] Optimize ipset usage in IptablesFirewallDriver Currently, IptablesFirewallDriver._update_ipset_members() iterates through a list of security group IDs and makes a call to IpsetManager.set_members() for each security group ID in the list. The problem is that set_members() is repeatedly called with the same arguments over and over again because the list of security group IDs contains duplicates. These duplicated calls are unnecessary because they are idempotent. For instance, with a security group of 50 rules created in this manner: neutron security-group-rule-create $SECGRP --remote_group_id $SECGRP --protocol tcp --port_range_min $i --port_range_max $i Adding a server to that security group will cause 50 calls to IpsetManager.set_members() because the list of security group IDs is 50 of the same ID. Only one call to IpsetManager.set_members() is necessary per security group ID. This patch converts that list of security group IDs into a set, which eliminates the duplicate idempotent calls to IpsetManager.set_members() with the same arguments. This will affect performance by reducing the amount of file locking around ipset when adding servers to security groups. Change-Id: Id2c8c8c1093c8abcf1fd897b23b0358aeb55b526 Closes-Bug: 1466921 --- neutron/agent/linux/iptables_firewall.py | 8 ++++---- .../tests/unit/agent/linux/test_iptables_firewall.py | 11 +++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/neutron/agent/linux/iptables_firewall.py b/neutron/agent/linux/iptables_firewall.py index a686e0e82..ff12802e1 100644 --- a/neutron/agent/linux/iptables_firewall.py +++ b/neutron/agent/linux/iptables_firewall.py @@ -432,14 +432,14 @@ class IptablesFirewallDriver(firewall.FirewallDriver): def _get_remote_sg_ids(self, port, direction=None): sg_ids = port.get('security_groups', []) - remote_sg_ids = {constants.IPv4: [], constants.IPv6: []} + remote_sg_ids = {constants.IPv4: set(), constants.IPv6: set()} for sg_id in sg_ids: for rule in self.sg_rules.get(sg_id, []): if not direction or rule['direction'] == direction: remote_sg_id = rule.get('remote_group_id') ether_type = rule.get('ethertype') if remote_sg_id and ether_type: - remote_sg_ids[ether_type].append(remote_sg_id) + remote_sg_ids[ether_type].add(remote_sg_id) return remote_sg_ids def _add_rules_by_security_group(self, port, direction): @@ -651,8 +651,8 @@ class IptablesFirewallDriver(firewall.FirewallDriver): constants.IPv6: set()} for port in filtered_ports: remote_sg_ids = self._get_remote_sg_ids(port) - for ip_version, sg_ids in six.iteritems(remote_sg_ids): - remote_group_id_sets[ip_version].update(sg_ids) + for ip_version in (constants.IPv4, constants.IPv6): + remote_group_id_sets[ip_version] |= remote_sg_ids[ip_version] return remote_group_id_sets def _determine_sg_rules_to_remove(self, filtered_ports): diff --git a/neutron/tests/unit/agent/linux/test_iptables_firewall.py b/neutron/tests/unit/agent/linux/test_iptables_firewall.py index 7491d5a87..96cf75a98 100644 --- a/neutron/tests/unit/agent/linux/test_iptables_firewall.py +++ b/neutron/tests/unit/agent/linux/test_iptables_firewall.py @@ -1507,6 +1507,17 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): {_IPv4: set([FAKE_SGID]), _IPv6: set([OTHER_SGID])}, self.firewall._get_remote_sg_ids_sets_by_ipversion(ports)) + def test_get_remote_sg_ids(self): + self.firewall.sg_rules = self._fake_sg_rules( + remote_groups={_IPv4: [FAKE_SGID, FAKE_SGID, FAKE_SGID], + _IPv6: [OTHER_SGID, OTHER_SGID, OTHER_SGID]}) + + port = self._fake_port() + + self.assertEqual( + {_IPv4: set([FAKE_SGID]), _IPv6: set([OTHER_SGID])}, + self.firewall._get_remote_sg_ids(port)) + def test_determine_sg_rules_to_remove(self): self.firewall.pre_sg_rules = self._fake_sg_rules(sg_id=OTHER_SGID) ports = [self._fake_port()] -- 2.45.2