diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py index c2eb06ec74c..311e0430615 100644 --- a/neutron/agent/linux/interface.py +++ b/neutron/agent/linux/interface.py @@ -27,6 +27,7 @@ from neutron._i18n import _, _LE, _LI, _LW from neutron.agent.common import ovs_lib from neutron.agent.linux import ip_lib from neutron.agent.linux import utils +from neutron.common import constants as n_const from neutron.common import exceptions @@ -53,8 +54,7 @@ def _get_veth(name1, name2, namespace2): @six.add_metaclass(abc.ABCMeta) class LinuxInterfaceDriver(object): - # from linux IF_NAMESIZE - DEV_NAME_LEN = 14 + DEV_NAME_LEN = n_const.LINUX_DEV_LEN DEV_NAME_PREFIX = constants.TAP_DEVICE_PREFIX def __init__(self, conf): diff --git a/neutron/agent/linux/ip_conntrack.py b/neutron/agent/linux/ip_conntrack.py index e6b661b95ca..4427ec741a6 100644 --- a/neutron/agent/linux/ip_conntrack.py +++ b/neutron/agent/linux/ip_conntrack.py @@ -11,22 +11,47 @@ # See the License for the specific language governing permissions and # limitations under the License. +import re + import netaddr +from oslo_concurrency import lockutils from oslo_log import log as logging from neutron._i18n import _LE from neutron.agent.linux import utils as linux_utils +from neutron.common import constants as n_const +from neutron.common import exceptions as n_exc LOG = logging.getLogger(__name__) +CONTRACK_MGRS = {} +MAX_CONNTRACK_ZONES = 65535 + + +@lockutils.synchronized('conntrack') +def get_conntrack(get_rules_for_table_func, filtered_ports, unfiltered_ports, + execute=None, namespace=None): + + try: + return CONTRACK_MGRS[namespace] + except KeyError: + ipconntrack = IpConntrackManager(get_rules_for_table_func, + filtered_ports, unfiltered_ports, + execute, namespace) + CONTRACK_MGRS[namespace] = ipconntrack + return CONTRACK_MGRS[namespace] class IpConntrackManager(object): """Smart wrapper for ip conntrack.""" - def __init__(self, zone_lookup_func, execute=None, namespace=None): - self.get_device_zone = zone_lookup_func + def __init__(self, get_rules_for_table_func, filtered_ports, + unfiltered_ports, execute=None, namespace=None): + self.get_rules_for_table_func = get_rules_for_table_func self.execute = execute or linux_utils.execute self.namespace = namespace + self.filtered_ports = filtered_ports + self.unfiltered_ports = unfiltered_ports + self._populate_initial_zone_map() @staticmethod def _generate_conntrack_cmd_by_rule(rule, namespace): @@ -92,3 +117,72 @@ class IpConntrackManager(object): device_info_list, rule, remote_ip) else: self._delete_conntrack_state(device_info_list, rule) + + def _populate_initial_zone_map(self): + """Setup the map between devices and zones based on current rules.""" + self._device_zone_map = {} + rules = self.get_rules_for_table_func('raw') + for rule in rules: + match = re.match(r'.* --physdev-in (?P[a-zA-Z0-9\-]+)' + r'.* -j CT --zone (?P\d+).*', rule) + if match: + # strip off any prefix that the interface is using + short_port_id = (match.group('dev') + [n_const.LINUX_DEV_PREFIX_LEN:]) + self._device_zone_map[short_port_id] = int(match.group('zone')) + LOG.debug("Populated conntrack zone map: %s", self._device_zone_map) + + def get_device_zone(self, port_id): + # we have to key the device_zone_map based on the fragment of the port + # UUID that shows up in the interface name. This is because the initial + # map is populated strictly based on interface names that we don't know + # the full UUID of. + short_port_id = port_id[:(n_const.LINUX_DEV_LEN - + n_const.LINUX_DEV_PREFIX_LEN)] + try: + return self._device_zone_map[short_port_id] + except KeyError: + return self._generate_device_zone(short_port_id) + + def _free_zones_from_removed_ports(self): + """Clears any entries from the zone map of removed ports.""" + existing_ports = [ + port['device'][:(n_const.LINUX_DEV_LEN - + n_const.LINUX_DEV_PREFIX_LEN)] + for port in (list(self.filtered_ports.values()) + + list(self.unfiltered_ports.values())) + ] + removed = set(self._device_zone_map) - set(existing_ports) + for dev in removed: + self._device_zone_map.pop(dev, None) + + def _generate_device_zone(self, short_port_id): + """Generates a unique conntrack zone for the passed in ID.""" + try: + zone = self._find_open_zone() + except n_exc.CTZoneExhaustedError: + # Free some zones and try again, repeat failure will not be caught + self._free_zones_from_removed_ports() + zone = self._find_open_zone() + + self._device_zone_map[short_port_id] = zone + LOG.debug("Assigned CT zone %(z)s to port %(dev)s.", + {'z': zone, 'dev': short_port_id}) + return self._device_zone_map[short_port_id] + + def _find_open_zone(self): + # call set to dedup because old ports may be mapped to the same zone. + zones_in_use = sorted(set(self._device_zone_map.values())) + if not zones_in_use: + return 1 + # attempt to increment onto the highest used zone first. if we hit the + # end, go back and look for any gaps left by removed devices. + last = zones_in_use[-1] + if last < MAX_CONNTRACK_ZONES: + return last + 1 + for index, used in enumerate(zones_in_use): + if used - index != 1: + # gap found, let's use it! + return index + 1 + # conntrack zones exhausted :( :( + raise n_exc.CTZoneExhaustedError() diff --git a/neutron/agent/linux/iptables_firewall.py b/neutron/agent/linux/iptables_firewall.py index 968ca691d2c..4e46078eb98 100644 --- a/neutron/agent/linux/iptables_firewall.py +++ b/neutron/agent/linux/iptables_firewall.py @@ -14,7 +14,6 @@ # under the License. import collections -import re import netaddr from neutron_lib import constants @@ -30,7 +29,7 @@ from neutron.agent.linux import ipset_manager from neutron.agent.linux import iptables_comments as ic from neutron.agent.linux import iptables_manager from neutron.agent.linux import utils -from neutron.common import exceptions as n_exc +from neutron.common import constants as n_const from neutron.common import ipv6_utils from neutron.common import utils as c_utils @@ -43,15 +42,11 @@ CHAIN_NAME_PREFIX = {firewall.INGRESS_DIRECTION: 'i', SPOOF_FILTER: 's'} IPSET_DIRECTION = {firewall.INGRESS_DIRECTION: 'src', firewall.EGRESS_DIRECTION: 'dst'} -# length of all device prefixes (e.g. qvo, tap, qvb) -LINUX_DEV_PREFIX_LEN = 3 -LINUX_DEV_LEN = 14 -MAX_CONNTRACK_ZONES = 65535 comment_rule = iptables_manager.comment_rule def get_hybrid_port_name(port_name): - return (constants.TAP_DEVICE_PREFIX + port_name)[:LINUX_DEV_LEN] + return (constants.TAP_DEVICE_PREFIX + port_name)[:n_const.LINUX_DEV_LEN] class mac_iptables(netaddr.mac_eui48): @@ -72,12 +67,12 @@ class IptablesFirewallDriver(firewall.FirewallDriver): # TODO(majopela, shihanzhang): refactor out ipset to a separate # driver composed over this one self.ipset = ipset_manager.IpsetManager(namespace=namespace) - self.ipconntrack = ip_conntrack.IpConntrackManager( - self.get_device_zone, namespace=namespace) - self._populate_initial_zone_map() # list of port which has security group self.filtered_ports = {} self.unfiltered_ports = {} + self.ipconntrack = ip_conntrack.get_conntrack( + self.iptables.get_rules_for_table, self.filtered_ports, + self.unfiltered_ports, namespace=namespace) self._add_fallback_chain_v4v6() self._defer_apply = False self._pre_defer_filtered_ports = None @@ -863,72 +858,6 @@ class IptablesFirewallDriver(firewall.FirewallDriver): self._pre_defer_filtered_ports = None self._pre_defer_unfiltered_ports = None - def _populate_initial_zone_map(self): - """Setup the map between devices and zones based on current rules.""" - self._device_zone_map = {} - rules = self.iptables.get_rules_for_table('raw') - for rule in rules: - match = re.match(r'.* --physdev-in (?P[a-zA-Z0-9\-]+)' - r'.* -j CT --zone (?P\d+).*', rule) - if match: - # strip off any prefix that the interface is using - short_port_id = match.group('dev')[LINUX_DEV_PREFIX_LEN:] - self._device_zone_map[short_port_id] = int(match.group('zone')) - LOG.debug("Populated conntrack zone map: %s", self._device_zone_map) - - def get_device_zone(self, port_id): - # we have to key the device_zone_map based on the fragment of the port - # UUID that shows up in the interface name. This is because the initial - # map is populated strictly based on interface names that we don't know - # the full UUID of. - short_port_id = port_id[:(LINUX_DEV_LEN - LINUX_DEV_PREFIX_LEN)] - try: - return self._device_zone_map[short_port_id] - except KeyError: - return self._generate_device_zone(short_port_id) - - def _free_zones_from_removed_ports(self): - """Clears any entries from the zone map of removed ports.""" - existing_ports = [ - port['device'][:(LINUX_DEV_LEN - LINUX_DEV_PREFIX_LEN)] - for port in (list(self.filtered_ports.values()) + - list(self.unfiltered_ports.values())) - ] - removed = set(self._device_zone_map) - set(existing_ports) - for dev in removed: - self._device_zone_map.pop(dev, None) - - def _generate_device_zone(self, short_port_id): - """Generates a unique conntrack zone for the passed in ID.""" - try: - zone = self._find_open_zone() - except n_exc.CTZoneExhaustedError: - # Free some zones and try again, repeat failure will not be caught - self._free_zones_from_removed_ports() - zone = self._find_open_zone() - - self._device_zone_map[short_port_id] = zone - LOG.debug("Assigned CT zone %(z)s to port %(dev)s.", - {'z': zone, 'dev': short_port_id}) - return self._device_zone_map[short_port_id] - - def _find_open_zone(self): - # call set to dedup because old ports may be mapped to the same zone. - zones_in_use = sorted(set(self._device_zone_map.values())) - if not zones_in_use: - return 1 - # attempt to increment onto the highest used zone first. if we hit the - # end, go back and look for any gaps left by removed devices. - last = zones_in_use[-1] - if last < MAX_CONNTRACK_ZONES: - return last + 1 - for index, used in enumerate(zones_in_use): - if used - index != 1: - # gap found, let's use it! - return index + 1 - # conntrack zones exhausted :( :( - raise n_exc.CTZoneExhaustedError() - class OVSHybridIptablesFirewallDriver(IptablesFirewallDriver): OVS_HYBRID_TAP_PREFIX = constants.TAP_DEVICE_PREFIX @@ -939,7 +868,7 @@ class OVSHybridIptablesFirewallDriver(IptablesFirewallDriver): '%s%s' % (CHAIN_NAME_PREFIX[direction], port['device'])) def _get_br_device_name(self, port): - return ('qvb' + port['device'])[:LINUX_DEV_LEN] + return ('qvb' + port['device'])[:n_const.LINUX_DEV_LEN] def _get_device_name(self, port): return get_hybrid_port_name(port['device']) @@ -950,7 +879,8 @@ class OVSHybridIptablesFirewallDriver(IptablesFirewallDriver): else: device = self._get_device_name(port) jump_rule = '-m physdev --physdev-in %s -j CT --zone %s' % ( - device, self.get_device_zone(port['device'])) + device, self.ipconntrack.get_device_zone( + port['device'])) return jump_rule def _add_raw_chain_rules(self, port, direction): diff --git a/neutron/common/constants.py b/neutron/common/constants.py index 2408154360c..759950f3ecb 100644 --- a/neutron/common/constants.py +++ b/neutron/common/constants.py @@ -67,6 +67,11 @@ PROVISIONAL_IPV6_PD_PREFIX = '::/64' # Timeout in seconds for getting an IPv6 LLA LLA_TASK_TIMEOUT = 40 +# length of all device prefixes (e.g. qvo, tap, qvb) +LINUX_DEV_PREFIX_LEN = 3 +# must be shorter than linux IFNAMSIZ (which is 16) +LINUX_DEV_LEN = 14 + # Possible prefixes to partial port IDs in interface names used by the OVS, # Linux Bridge, and IVS VIF drivers in Nova and the neutron agents. See the # 'get_ovs_interfaceid' method in Nova (nova/virt/libvirt/vif.py) for details. diff --git a/neutron/tests/unit/agent/linux/test_ip_conntrack.py b/neutron/tests/unit/agent/linux/test_ip_conntrack.py index 80aa30ba61b..3771d9ae7cc 100644 --- a/neutron/tests/unit/agent/linux/test_ip_conntrack.py +++ b/neutron/tests/unit/agent/linux/test_ip_conntrack.py @@ -23,11 +23,14 @@ 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) + self.filtered_port = {} + self.unfiltered_port = {} + self.mgr = ip_conntrack.IpConntrackManager( + self._get_rule_for_table, self.filtered_port, + self.unfiltered_port, self.execute) - def _zone_lookup(self, dev): - return 100 + def _get_rule_for_table(self, table): + return ['test --physdev-in tapdevice -j CT --zone 100'] def test_delete_conntrack_state_dedupes(self): rule = {'ethertype': 'IPv4', 'direction': 'ingress'} diff --git a/neutron/tests/unit/agent/linux/test_iptables_firewall.py b/neutron/tests/unit/agent/linux/test_iptables_firewall.py index 45849bd0907..897eee6eb24 100644 --- a/neutron/tests/unit/agent/linux/test_iptables_firewall.py +++ b/neutron/tests/unit/agent/linux/test_iptables_firewall.py @@ -23,6 +23,7 @@ import testtools from neutron.agent.common import config as a_cfg from neutron.agent import firewall +from neutron.agent.linux import ip_conntrack from neutron.agent.linux import ipset_manager from neutron.agent.linux import iptables_comments as ic from neutron.agent.linux import iptables_firewall @@ -96,9 +97,16 @@ class BaseIptablesFirewallTestCase(base.BaseTestCase): self.firewall.iptables = self.iptables_inst # don't mess with sysctl knobs in unit tests self.firewall._enabled_netfilter_for_bridges = True - - -class IptablesFirewallTestCase(BaseIptablesFirewallTestCase): + # initial data has 1, 2, and 9 in use, see RAW_TABLE_OUTPUT above. + self._dev_zone_map = {'61634509-31': 2, '8f46cf18-12': 9, + '95c24827-02': 2, 'e804433b-61': 1} + get_rules_for_table_func = lambda x: RAW_TABLE_OUTPUT.split('\n') + filtered_ports = {port_id: self._fake_port() + for port_id in self._dev_zone_map} + self.firewall.ipconntrack = ip_conntrack.IpConntrackManager( + get_rules_for_table_func, filtered_ports=filtered_ports, + unfiltered_ports=dict()) + self.firewall.ipconntrack._device_zone_map = self._dev_zone_map def _fake_port(self): return {'device': 'tapfake_dev', @@ -107,6 +115,9 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase): 'fixed_ips': [FAKE_IP['IPv4'], FAKE_IP['IPv6']]} + +class IptablesFirewallTestCase(BaseIptablesFirewallTestCase): + def test_prepare_port_filter_with_no_sg(self): port = self._fake_port() self.firewall.prepare_port_filter(port) @@ -1959,46 +1970,56 @@ class OVSHybridIptablesFirewallTestCase(BaseIptablesFirewallTestCase): def setUp(self): super(OVSHybridIptablesFirewallTestCase, self).setUp() - self.firewall = iptables_firewall.OVSHybridIptablesFirewallDriver() - # initial data has 1, 2, and 9 in use, see RAW_TABLE_OUTPUT above. - self._dev_zone_map = {'61634509-31': 2, '8f46cf18-12': 9, - '95c24827-02': 2, 'e804433b-61': 1} def test__populate_initial_zone_map(self): - self.assertEqual(self._dev_zone_map, self.firewall._device_zone_map) + self.assertEqual(self._dev_zone_map, + self.firewall.ipconntrack._device_zone_map) def test__generate_device_zone(self): # initial data has 1, 2, and 9 in use. # we fill from top up first. - self.assertEqual(10, self.firewall._generate_device_zone('test')) + self.assertEqual(10, + self.firewall.ipconntrack._generate_device_zone('test')) # once it's maxed out, it scans for gaps - self.firewall._device_zone_map['someport'] = ( - iptables_firewall.MAX_CONNTRACK_ZONES) + self.firewall.ipconntrack._device_zone_map['someport'] = ( + ip_conntrack.MAX_CONNTRACK_ZONES) for i in range(3, 9): - self.assertEqual(i, self.firewall._generate_device_zone(i)) + self.assertEqual(i, + self.firewall.ipconntrack._generate_device_zone(i)) # 9 and 10 are taken so next should be 11 - self.assertEqual(11, self.firewall._generate_device_zone('p11')) + self.assertEqual(11, + self.firewall.ipconntrack._generate_device_zone('p11')) # take out zone 1 and make sure it's selected - self.firewall._device_zone_map.pop('e804433b-61') - self.assertEqual(1, self.firewall._generate_device_zone('p1')) + self.firewall.ipconntrack._device_zone_map.pop('e804433b-61') + self.assertEqual(1, + self.firewall.ipconntrack._generate_device_zone('p1')) # fill it up and then make sure an extra throws an error for i in range(1, 65536): - self.firewall._device_zone_map['dev-%s' % i] = i + self.firewall.ipconntrack._device_zone_map['dev-%s' % i] = i with testtools.ExpectedException(n_exc.CTZoneExhaustedError): - self.firewall._find_open_zone() + self.firewall.ipconntrack._find_open_zone() # with it full, try again, this should trigger a cleanup and return 1 - self.assertEqual(1, self.firewall._generate_device_zone('p12')) - self.assertEqual({'p12': 1}, self.firewall._device_zone_map) + self.assertEqual(1, + self.firewall.ipconntrack._generate_device_zone('p12')) + self.assertEqual({'p12': 1}, + self.firewall.ipconntrack._device_zone_map) def test_get_device_zone(self): # initial data has 1, 2, and 9 in use. self.assertEqual(10, - self.firewall.get_device_zone('12345678901234567')) + self.firewall.ipconntrack.get_device_zone('12345678901234567')) # should have been truncated to 11 chars self._dev_zone_map.update({'12345678901': 10}) - self.assertEqual(self._dev_zone_map, self.firewall._device_zone_map) + self.assertEqual(self._dev_zone_map, + self.firewall.ipconntrack._device_zone_map) + + def test_multiple_firewall_with_common_conntrack(self): + self.firewall1 = iptables_firewall.OVSHybridIptablesFirewallDriver() + self.firewall2 = iptables_firewall.OVSHybridIptablesFirewallDriver() + self.assertEqual(id(self.firewall1.ipconntrack), + id(self.firewall2.ipconntrack)) diff --git a/neutron/tests/unit/agent/test_securitygroups_rpc.py b/neutron/tests/unit/agent/test_securitygroups_rpc.py index fbd9e53402e..ed82650e1f4 100644 --- a/neutron/tests/unit/agent/test_securitygroups_rpc.py +++ b/neutron/tests/unit/agent/test_securitygroups_rpc.py @@ -2601,6 +2601,7 @@ class TestSecurityGroupAgentWithIptables(base.BaseTestCase): oslo_messaging.UnsupportedVersion('1.2')) self.iptables = self.agent.firewall.iptables + self.ipconntrack = self.agent.firewall.ipconntrack # TODO(jlibosva) Get rid of mocking iptables execute and mock out # firewall instead self.iptables.use_ipv6 = True @@ -2756,6 +2757,7 @@ class TestSecurityGroupAgentWithIptables(base.BaseTestCase): return_value='') def test_prepare_remove_port(self): + self.ipconntrack._device_zone_map = {} self.rpc.security_group_rules_for_devices.return_value = self.devices1 self._replay_iptables(IPTABLES_FILTER_1, IPTABLES_FILTER_V6_1, IPTABLES_RAW_DEFAULT) @@ -2870,6 +2872,7 @@ class TestSecurityGroupAgentEnhancedRpcWithIptables( 'devices': devices_info2} def test_prepare_remove_port(self): + self.ipconntrack._device_zone_map = {} self.sg_info.return_value = self.devices_info1 self._replay_iptables(IPTABLES_FILTER_1, IPTABLES_FILTER_V6_1, IPTABLES_RAW_DEFAULT) @@ -2936,6 +2939,7 @@ class TestSecurityGroupAgentEnhancedIpsetWithIptables( "execute").start() def test_prepare_remove_port(self): + self.ipconntrack._device_zone_map = {} self.sg_info.return_value = self.devices_info1 self._replay_iptables(IPSET_FILTER_1, IPTABLES_FILTER_V6_1, IPTABLES_RAW_DEFAULT) @@ -3062,6 +3066,7 @@ class TestSecurityGroupAgentWithOVSIptables( self.agent.firewall._enabled_netfilter_for_bridges = True def test_prepare_remove_port(self): + self.ipconntrack._device_zone_map = {} self.rpc.security_group_rules_for_devices.return_value = self.devices1 self._replay_iptables(IPTABLES_FILTER_1, IPTABLES_FILTER_V6_1, IPTABLES_RAW_DEVICE_1) @@ -3074,6 +3079,7 @@ class TestSecurityGroupAgentWithOVSIptables( self._verify_mock_calls() def test_security_group_member_updated(self): + self.ipconntrack._device_zone_map = {} self.rpc.security_group_rules_for_devices.return_value = self.devices1 self._replay_iptables(IPTABLES_FILTER_1, IPTABLES_FILTER_V6_1, IPTABLES_RAW_DEVICE_1) @@ -3100,6 +3106,7 @@ class TestSecurityGroupAgentWithOVSIptables( self._verify_mock_calls() def test_security_group_rule_updated(self): + self.ipconntrack._device_zone_map = {} self.rpc.security_group_rules_for_devices.return_value = self.devices2 self._replay_iptables(IPTABLES_FILTER_2, IPTABLES_FILTER_V6_2, IPTABLES_RAW_DEVICE_2) diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index eedac3e6480..2652e2a1c52 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -1013,8 +1013,8 @@ class TestOvsNeutronAgent(object): 'neutron.agent.linux.iptables_firewall.' 'OVSHybridIptablesFirewallDriver', group='SECURITYGROUP') - with mock.patch('neutron.agent.linux.iptables_firewall.' - 'IptablesFirewallDriver._populate_initial_zone_map'): + with mock.patch('neutron.agent.linux.ip_conntrack.' + 'IpConntrackManager._populate_initial_zone_map'): agt = self._make_agent() self.assertTrue(agt.agent_state['configurations']['ovs_hybrid_plug'])