openstack
/
neutron
Code Issues Proposed changes

Delete conntrack when remote ipset member removed 

Through [1] ipset members are updated in update_security_group_members
instead of updating during firewall apply. In the same way, we will
delete conntrack entries immediately after deleting remote ipset
members(in update_security_group_members) instead of deleting them after
firewall apply.

As explained in [2], this change partially fixes bug #1580377 i.e it
deletes conntrack entries on remote hosts for a removed port.

[1] https://review.openstack.org/#/c/347068/
[2] https://bugs.launchpad.net/neutron/+bug/1580377/comments/13

Co-Authored-By:shihanzhang <shihanzhang@huawei.com>
Partial-Bug: #1580377
Change-Id: Iea3344a24e2a068b794c44796b4c945432379c13
This commit is contained in:
venkata anil 2016-08-08 14:11:11 +00:00
parent 8e27076b32
commit 9168dbf93d
5 changed files with 57 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
neutron/agent/linux/ipset_manager.py
View File

@ -73,16 +73,15 @@ class IpsetManager(object):
"""Create or update a specific set by name and ethertype.
It will make sure that a set is created, updated to
add / remove new members, or swapped atomically if
that's faster.
that's faster, and return added / removed members.
"""
member_ips = self._sanitize_addresses(member_ips)
set_name = self.get_name(id, ethertype)
add_ips = self._get_new_set_ips(set_name, member_ips)
del_ips = self._get_deleted_set_ips(set_name, member_ips)
if not add_ips and not del_ips and self.set_name_exists(set_name):
# nothing to do because no membership changes and the ipset exists
return
self.set_members_mutate(set_name, ethertype, member_ips)
if add_ips or del_ips or not self.set_name_exists(set_name):
self.set_members_mutate(set_name, ethertype, member_ips)
return add_ips, del_ips
@utils.synchronized('ipset', external=True)
def set_members_mutate(self, set_name, ethertype, member_ips):

17
neutron/agent/linux/iptables_firewall.py
View File

@ -151,8 +151,18 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
LOG.debug("Update members of security group (%s)", sg_id)
self.sg_members[sg_id] = collections.defaultdict(list, sg_members)
if self.enable_ipset:
for ip_version, current_ips in sg_members.items():
self.ipset.set_members(sg_id, ip_version, current_ips)
self._update_ipset_members(sg_id, sg_members)
def _update_ipset_members(self, sg_id, sg_members):
devices = self.devices_with_updated_sg_members.pop(sg_id, None)
for ip_version, current_ips in sg_members.items():
add_ips, del_ips = self.ipset.set_members(
sg_id, ip_version, current_ips)
if devices and del_ips:
# remove prefix from del_ips
ips = [str(netaddr.IPNetwork(del_ip).ip) for del_ip in del_ips]
self.ipconntrack.delete_conntrack_state_by_remote_ips(
devices, ip_version, ips)
def _set_ports(self, port):
if not firewall.port_sec_enabled(port):
@ -826,7 +836,8 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
def _remove_conntrack_entries_from_sg_updates(self):
self._clean_deleted_sg_rule_conntrack_entries()
self._clean_updated_sg_member_conntrack_entries()
self._clean_deleted_remote_sg_members_conntrack_entries()
if not self.enable_ipset:
self._clean_deleted_remote_sg_members_conntrack_entries()
def _get_sg_members(self, sg_info, sg_id, ethertype):
return set(sg_info.get(sg_id, {}).get(ethertype, []))

2
neutron/agent/securitygroups_rpc.py
View File

@ -282,7 +282,7 @@ class SecurityGroupAgentRpc(object):
LOG.debug("Refreshing firewall for all filtered devices")
self.refresh_firewall()
else:
if self.use_enhanced_rpc:
if self.use_enhanced_rpc and updated_devices:
self.firewall.security_group_updated('sg_member', [],
updated_devices)
# If a device is both in new and updated devices

59
neutron/tests/unit/agent/linux/test_iptables_firewall.py
View File

@ -1187,38 +1187,46 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase):
protocol, direction):
port = self._fake_port()
port['security_groups'] = ['fake_sg_id']
self.firewall.sg_rules.setdefault('fake_sg_id', [])
self.firewall.sg_rules['fake_sg_id'].append(
{'direction': direction, 'remote_group_id': 'fake_sg_id2',
'ethertype': ethertype})
port['security_group_source_groups'] = ['fake_sg_id2']
port['security_group_rules'] = [{'security_group_id': 'fake_sg_id',
'direction': direction,
'remote_group_id': 'fake_sg_id2',
'ethertype': ethertype}]
self.firewall.filtered_ports = {port['device']: port}
self.firewall.filter_defer_apply_on()
self.firewall.devices_with_updated_sg_members['fake_sg_id2'] = [port]
if ethertype == "IPv4":
self.firewall.pre_sg_members = {'fake_sg_id2': {
'IPv4': ['10.0.0.2', '10.0.0.3']}}
self.firewall.sg_members = {'fake_sg_id2': {
'IPv4': ['10.0.0.3']}}
ethertype = "ipv4"
members_add = {'IPv4': ['10.0.0.2', '10.0.0.3']}
members_after_delete = {'IPv4': ['10.0.0.3']}
else:
self.firewall.pre_sg_members = {'fake_sg_id2': {
'IPv6': ['fe80::2', 'fe80::3']}}
self.firewall.sg_members = {'fake_sg_id2': {
'IPv6': ['fe80::3']}}
ethertype = "ipv6"
self.firewall.filter_defer_apply_off()
direction = '-d' if direction == 'ingress' else '-s'
remote_ip_direction = '-s' if direction == '-d' else '-d'
members_add = {'IPv6': ['fe80::2', 'fe80::3']}
members_after_delete = {'IPv6': ['fe80::3']}
# add ['10.0.0.2', '10.0.0.3'] or ['fe80::2', 'fe80::3']
self.firewall.security_group_updated('sg_member', ['fake_sg_id2'])
self.firewall.update_security_group_members(
'fake_sg_id2', members_add)
# delete '10.0.0.2' or 'fe80::2'
self.firewall.security_group_updated('sg_member', ['fake_sg_id2'])
self.firewall.update_security_group_members(
'fake_sg_id2', members_after_delete)
# check conntrack deletion from '10.0.0.1' to '10.0.0.2' or
# from 'fe80::1' to 'fe80::2'
ips = {"ipv4": ['10.0.0.1', '10.0.0.2'],
"ipv6": ['fe80::1', 'fe80::2']}
calls = [
# initial data has 1, 2, and 9 in use, CT zone will start
# at 10.
mock.call(['conntrack', '-D', '-f', ethertype, direction,
ips[ethertype][0], '-w', 10,
remote_ip_direction, ips[ethertype][1]],
run_as_root=True, check_exit_code=True,
extra_ok_codes=[1])]
calls = []
for direction in ['ingress', 'egress']:
direction = '-d' if direction == 'ingress' else '-s'
remote_ip_direction = '-s' if direction == '-d' else '-d'
calls.append(mock.call(['conntrack', '-D', '-f', ethertype,
direction, ips[ethertype][0], '-w', 10,
remote_ip_direction, ips[ethertype][1]],
run_as_root=True, check_exit_code=True,
extra_ok_codes=[1]))
self.utils_exec.assert_has_calls(calls)
def test_user_sg_rules_deduped_before_call_to_iptables_manager(self):
@ -1667,6 +1675,7 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase):
self.firewall.ipset.get_name.side_effect = (
ipset_manager.IpsetManager.get_name)
self.firewall.ipset.set_name_exists.return_value = True
self.firewall.ipset.set_members = mock.Mock(return_value=([], []))
def _fake_port(self, sg_id=FAKE_SGID):
return {'device': 'tapfake_dev',

4
neutron/tests/unit/agent/test_securitygroups_rpc.py
View File

@ -3025,6 +3025,8 @@ class TestSecurityGroupAgentEnhancedIpsetWithIptables(
def test_security_group_member_updated(self):
self.sg_info.return_value = self.devices_info1
self.ipset._get_new_set_ips = mock.Mock(return_value=['10.0.0.3'])
self.ipset._get_deleted_set_ips = mock.Mock(return_value=[])
self._replay_iptables(IPSET_FILTER_1, IPTABLES_FILTER_V6_1,
IPTABLES_RAW_DEFAULT)
self._replay_iptables(IPSET_FILTER_1, IPTABLES_FILTER_V6_1,
@ -3052,6 +3054,8 @@ class TestSecurityGroupAgentEnhancedIpsetWithIptables(
2, self.agent.firewall.security_group_updated.call_count)
def test_security_group_rule_updated(self):
self.ipset._get_new_set_ips = mock.Mock(return_value=['10.0.0.3'])
self.ipset._get_deleted_set_ips = mock.Mock(return_value=[])
self.sg_info.return_value = self.devices_info2
self._replay_iptables(IPSET_FILTER_2, IPTABLES_FILTER_V6_2,
IPTABLES_RAW_DEFAULT)