From 48fd90d55f13195cc6331e50da85bf6440f1f8f9 Mon Sep 17 00:00:00 2001 From: shihanzhang Date: Thu, 25 Sep 2014 16:11:29 +0800 Subject: [PATCH] Fix L2 agent does not remove unused ipset set The patch fixes L2 agent does not remove unused ipset set when security group contains rules for only IPv4 or IPv6. Change-Id: I375b1683cd763c0a33dc935558c637874d36ffa1 Closes-bug: #1373287 --- neutron/agent/linux/iptables_firewall.py | 64 ++++++++++++-------- neutron/tests/unit/test_iptables_firewall.py | 32 +++++++++- 2 files changed, 70 insertions(+), 26 deletions(-) diff --git a/neutron/agent/linux/iptables_firewall.py b/neutron/agent/linux/iptables_firewall.py index 102e90634..837a1ce2f 100644 --- a/neutron/agent/linux/iptables_firewall.py +++ b/neutron/agent/linux/iptables_firewall.py @@ -320,12 +320,14 @@ class IptablesFirewallDriver(firewall.FirewallDriver): def _get_remote_sg_ids(self, port, direction): sg_ids = port.get('security_groups', []) - remote_sg_ids = [] + remote_sg_ids = {constants.IPv4: [], constants.IPv6: []} for sg_id in sg_ids: - remote_sg_ids.extend([rule['remote_group_id'] - for rule in self.sg_rules.get(sg_id, []) if - rule['direction'] == direction - and rule.get('remote_group_id')]) + for rule in self.sg_rules.get(sg_id, []): + if 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) return remote_sg_ids def _add_rule_by_security_group(self, port, direction): @@ -394,16 +396,15 @@ class IptablesFirewallDriver(firewall.FirewallDriver): self.ipset_chains[chain_name].remove(del_ip) def _update_ipset_chain_member(self, security_group_ids): - for sg_id in security_group_ids or []: - for ethertype in ['IPv4', 'IPv6']: + for ethertype, sg_ids in security_group_ids.items(): + for sg_id in sg_ids: add_ips = self._get_new_sg_member_ips(sg_id, ethertype) del_ips = self._get_deleted_sg_member_ips(sg_id, ethertype) cur_member_ips = self._get_cur_sg_member_ips(sg_id, ethertype) chain_name = ethertype + sg_id[:IPSET_CHAIN_LEN] if chain_name not in self.ipset_chains and cur_member_ips: self.ipset_chains[chain_name] = [] - self.ipset.create_ipset_chain( - chain_name, ethertype) + self.ipset.create_ipset_chain(chain_name, ethertype) self._bulk_set_ips_to_chain(chain_name, cur_member_ips, ethertype) elif (len(add_ips) + len(del_ips) @@ -537,32 +538,47 @@ class IptablesFirewallDriver(firewall.FirewallDriver): self._defer_apply = True def _remove_unused_security_group_info(self): - need_removed_ipset_chains = set() + need_removed_ipset_chains = {constants.IPv4: set(), + constants.IPv6: set()} need_removed_security_groups = set() - remote_group_ids = set() + remote_group_ids = {constants.IPv4: set(), + constants.IPv6: set()} cur_group_ids = set() for port in self.filtered_ports.values(): - source_groups = port.get('security_group_source_groups', []) - remote_group_ids.update(source_groups) + for direction in INGRESS_DIRECTION, EGRESS_DIRECTION: + for ethertype, sg_ids in self._get_remote_sg_ids( + port, direction).items(): + remote_group_ids[ethertype].update(sg_ids) groups = port.get('security_groups', []) cur_group_ids.update(groups) - need_removed_ipset_chains.update( - [x for x in self.pre_sg_members if x not in remote_group_ids]) - need_removed_security_groups.update( - [x for x in self.pre_sg_rules if x not in cur_group_ids]) - # Remove unused remote security group member ips - for remove_chain_id in need_removed_ipset_chains: - if remove_chain_id in self.sg_members: - self.sg_members.pop(remove_chain_id, None) - if self.enable_ipset: - for ethertype in ['IPv4', 'IPv6']: + for ethertype in [constants.IPv4, constants.IPv6]: + need_removed_ipset_chains[ethertype].update( + [x for x in self.pre_sg_members if x not in remote_group_ids[ + ethertype]]) + need_removed_security_groups.update( + [x for x in self.pre_sg_rules if x not in cur_group_ids]) + + # Remove unused remote ipset set + for ethertype, remove_chain_ids in need_removed_ipset_chains.items(): + for remove_chain_id in remove_chain_ids: + if self.sg_members.get(remove_chain_id, {}).get(ethertype, []): + self.sg_members[remove_chain_id][ethertype] = [] + if self.enable_ipset: removed_chain = ( ethertype + remove_chain_id[:IPSET_CHAIN_LEN]) if removed_chain in self.ipset_chains: - self.ipset.destroy_ipset_chain_by_name(removed_chain) + self.ipset.destroy_ipset_chain_by_name( + removed_chain) self.ipset_chains.pop(removed_chain, None) + # Remove unused remote security group member ips + sg_ids = self.sg_members.keys() + for sg_id in sg_ids: + if not (self.sg_members[sg_id].get(constants.IPv4, []) + or self.sg_members[sg_id].get(constants.IPv6, [])): + self.sg_members.pop(sg_id, None) + # Remove unused security group rules for remove_group_id in need_removed_security_groups: if remove_group_id in self.sg_rules: diff --git a/neutron/tests/unit/test_iptables_firewall.py b/neutron/tests/unit/test_iptables_firewall.py index c4c0cafb2..f2bc3cac1 100644 --- a/neutron/tests/unit/test_iptables_firewall.py +++ b/neutron/tests/unit/test_iptables_firewall.py @@ -1419,7 +1419,10 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): def _fake_sg_rule(self): return {'fake_sgid': [ - {'direction': 'ingress', 'remote_group_id': 'fake_sgid'}]} + {'direction': 'ingress', 'remote_group_id': 'fake_sgid', + 'ethertype': 'IPv4'}, + {'direction': 'ingress', 'remote_group_id': 'fake_sgid', + 'ethertype': 'IPv6'}]} def test_prepare_port_filter_with_default_sg(self): self.firewall.sg_rules = self._fake_sg_rule() @@ -1523,7 +1526,8 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): def test_prepare_port_filter_with_sg_no_member(self): self.firewall.sg_rules = self._fake_sg_rule() self.firewall.sg_rules['fake_sgid'].append( - {'direction': 'ingress', 'remote_group_id': 'fake_sgid2'}) + {'direction': 'ingress', 'remote_group_id': 'fake_sgid2', + 'ethertype': 'IPv4'}) self.firewall.sg_rules.update() self.firewall.sg_members = {'fake_sgid': { 'IPv4': ['10.0.0.1', '10.0.0.2'], 'IPv6': ['fe80::1']}} @@ -1539,3 +1543,27 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): 'IPv6fake_sgid', ['fe80::1'], 'IPv6')] self.firewall.ipset.assert_has_calls(calls) + + def test_filter_defer_apply_off_with_sg_only_ipv6_rule(self): + self.firewall.sg_rules = self._fake_sg_rule() + self.firewall.pre_sg_rules = self._fake_sg_rule() + self.firewall.ipset_chains = {'IPv4fake_sgid': ['10.0.0.2'], + 'IPv6fake_sgid': ['fe80::1']} + self.firewall.sg_members = {'fake_sgid': { + 'IPv4': ['10.0.0.2'], + 'IPv6': ['fe80::1']}} + self.firewall.pre_sg_members = {'fake_sgid': { + 'IPv4': ['10.0.0.2'], + 'IPv6': ['fe80::1']}} + self.firewall.sg_rules['fake_sgid'].remove( + {'direction': 'ingress', 'remote_group_id': 'fake_sgid', + 'ethertype': 'IPv4'}) + self.firewall.sg_rules.update() + self.firewall._defer_apply = True + port = self._fake_port() + self.firewall.filtered_ports['tapfake_dev'] = port + self.firewall._pre_defer_filtered_ports = {} + self.firewall.filter_defer_apply_off() + calls = [mock.call.destroy_ipset_chain_by_name('IPv4fake_sgid')] + + self.firewall.ipset.assert_has_calls(calls, True) -- 2.45.2