From aa4fa7b8195103fd1569672c9a71e33093be2e6a Mon Sep 17 00:00:00 2001 From: Carl Baldwin Date: Tue, 14 Jul 2015 19:00:45 +0000 Subject: [PATCH] Use only the lower 16 bits of iptables mark for marking Since a packet can only have one mark, and we will need to mark a packet for multiple purposes, we need to use a coordinated bitmask for the two cases of simple marking that we currently do in Neutron leaving the other bits for address scopes. DocImpact Change-Id: Id0517758d06e036a36dc8b8772e41af55d986b4e Partially-Implements: blueprint address-scopes --- neutron/agent/l3/config.py | 6 ++++-- neutron/agent/l3/router_info.py | 8 ++++---- neutron/agent/metadata/driver.py | 7 ++++--- neutron/common/constants.py | 2 ++ neutron/tests/unit/agent/l3/test_agent.py | 16 ++++++++++------ .../unit/agent/linux/test_iptables_manager.py | 13 +++++++------ neutron/tests/unit/agent/metadata/test_driver.py | 6 ++++-- 7 files changed, 35 insertions(+), 23 deletions(-) diff --git a/neutron/agent/l3/config.py b/neutron/agent/l3/config.py index e98147765..edb5c5c90 100644 --- a/neutron/agent/l3/config.py +++ b/neutron/agent/l3/config.py @@ -84,9 +84,11 @@ OPTS = [ cfg.StrOpt('metadata_access_mark', default='0x1', help=_('Iptables mangle mark used to mark metadata valid ' - 'requests')), + 'requests. This mark will be masked with 0xffff so ' + 'that only the lower 16 bits will be used.')), cfg.StrOpt('external_ingress_mark', default='0x2', help=_('Iptables mangle mark used to mark ingress from ' - 'external network')), + 'external network. This mark will be masked with ' + '0xffff so that only the lower 16 bits will be used.')), ] diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index 85ca4a9a3..f3077c32d 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -30,7 +30,6 @@ LOG = logging.getLogger(__name__) INTERNAL_DEV_PREFIX = namespaces.INTERNAL_DEV_PREFIX EXTERNAL_DEV_PREFIX = namespaces.EXTERNAL_DEV_PREFIX -EXTERNAL_INGRESS_MARK_MASK = '0xffffffff' FLOATINGIP_STATUS_NOCHANGE = object() @@ -551,9 +550,10 @@ class RouterInfo(object): # Makes replies come back through the router to reverse DNAT ext_in_mark = self.agent_conf.external_ingress_mark snat_internal_traffic_to_floating_ip = ( - 'snat', '-m mark ! --mark %s ' + 'snat', '-m mark ! --mark %s/%s ' '-m conntrack --ctstate DNAT ' - '-j SNAT --to-source %s' % (ext_in_mark, ex_gw_ip)) + '-j SNAT --to-source %s' + % (ext_in_mark, l3_constants.ROUTER_MARK_MASK, ex_gw_ip)) return [dont_snat_traffic_to_internal_ports_if_not_to_floating_ip, snat_normal_external_traffic, @@ -563,7 +563,7 @@ class RouterInfo(object): mark = self.agent_conf.external_ingress_mark mark_packets_entering_external_gateway_port = ( 'mark', '-i %s -j MARK --set-xmark %s/%s' % - (interface_name, mark, EXTERNAL_INGRESS_MARK_MASK)) + (interface_name, mark, l3_constants.ROUTER_MARK_MASK)) return [mark_packets_entering_external_gateway_port] def _empty_snat_chains(self, iptables_manager): diff --git a/neutron/agent/metadata/driver.py b/neutron/agent/metadata/driver.py index 94e2a3092..338a78c94 100644 --- a/neutron/agent/metadata/driver.py +++ b/neutron/agent/metadata/driver.py @@ -24,12 +24,12 @@ from neutron.agent.linux import utils from neutron.callbacks import events from neutron.callbacks import registry from neutron.callbacks import resources +from neutron.common import constants from neutron.common import exceptions LOG = logging.getLogger(__name__) # Access with redirection to metadata proxy iptables mark mask -METADATA_ACCESS_MARK_MASK = '0xffffffff' METADATA_SERVICE_NAME = 'metadata-proxy' @@ -45,7 +45,8 @@ class MetadataDriver(object): @classmethod def metadata_filter_rules(cls, port, mark): - return [('INPUT', '-m mark --mark %s -j ACCEPT' % mark), + return [('INPUT', '-m mark --mark %s/%s -j ACCEPT' % + (mark, constants.ROUTER_MARK_MASK)), ('INPUT', '-p tcp -m tcp --dport %s ' '-j DROP' % port)] @@ -55,7 +56,7 @@ class MetadataDriver(object): '-p tcp -m tcp --dport 80 ' '-j MARK --set-xmark %(value)s/%(mask)s' % {'value': mark, - 'mask': METADATA_ACCESS_MARK_MASK})] + 'mask': constants.ROUTER_MARK_MASK})] @classmethod def metadata_nat_rules(cls, port): diff --git a/neutron/common/constants.py b/neutron/common/constants.py index fc9c4b246..ced7d9316 100644 --- a/neutron/common/constants.py +++ b/neutron/common/constants.py @@ -178,3 +178,5 @@ RPC_NAMESPACE_STATE = None # Default network MTU value when not configured DEFAULT_NETWORK_MTU = 0 + +ROUTER_MARK_MASK = "0xffff" diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 09416ba0a..0de6becd9 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -687,15 +687,17 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): '! -i %s ! -o %s -m conntrack ! --ctstate DNAT -j ACCEPT' % (interface_name, interface_name), '-o %s -j SNAT --to-source %s' % (interface_name, source_nat_ip), - '-m mark ! --mark 0x2 -m conntrack --ctstate DNAT ' - '-j SNAT --to-source %s' % source_nat_ip] + '-m mark ! --mark 0x2/%s -m conntrack --ctstate DNAT ' + '-j SNAT --to-source %s' % + (l3_constants.ROUTER_MARK_MASK, source_nat_ip)] for r in nat_rules: if negate: self.assertNotIn(r.rule, expected_rules) else: self.assertIn(r.rule, expected_rules) expected_rules = [ - '-i %s -j MARK --set-xmark 0x2/0xffffffff' % interface_name] + '-i %s -j MARK --set-xmark 0x2/%s' % + (interface_name, l3_constants.ROUTER_MARK_MASK)] for r in mangle_rules: if negate: self.assertNotIn(r.rule, expected_rules) @@ -1527,10 +1529,11 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): wrap_name) snat_rule1 = ("-A %s-snat -o iface -j SNAT --to-source %s") % ( wrap_name, ex_gw_port['fixed_ips'][0]['ip_address']) - snat_rule2 = ("-A %s-snat -m mark ! --mark 0x2 " + snat_rule2 = ("-A %s-snat -m mark ! --mark 0x2/%s " "-m conntrack --ctstate DNAT " "-j SNAT --to-source %s") % ( - wrap_name, ex_gw_port['fixed_ips'][0]['ip_address']) + wrap_name, l3_constants.ROUTER_MARK_MASK, + ex_gw_port['fixed_ips'][0]['ip_address']) self.assertIn(jump_float_rule, nat_rules) @@ -1541,7 +1544,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): mangle_rules = map(str, ri.iptables_manager.ipv4['mangle'].rules) mangle_rule = ("-A %s-mark -i iface " - "-j MARK --set-xmark 0x2/0xffffffff") % wrap_name + "-j MARK --set-xmark 0x2/%s" % + (wrap_name, l3_constants.ROUTER_MARK_MASK)) self.assertIn(mangle_rule, mangle_rules) def test_process_router_delete_stale_internal_devices(self): diff --git a/neutron/tests/unit/agent/linux/test_iptables_manager.py b/neutron/tests/unit/agent/linux/test_iptables_manager.py index 674b1a872..d6a1f9116 100644 --- a/neutron/tests/unit/agent/linux/test_iptables_manager.py +++ b/neutron/tests/unit/agent/linux/test_iptables_manager.py @@ -22,6 +22,7 @@ import testtools from neutron.agent.linux import iptables_comments as ic from neutron.agent.linux import iptables_manager +from neutron.common import constants from neutron.common import exceptions as n_exc from neutron.tests import base from neutron.tests import tools @@ -29,7 +30,8 @@ from neutron.tests import tools IPTABLES_ARG = {'bn': iptables_manager.binary_name, 'snat_out_comment': ic.SNAT_OUT, - 'filter_rules': ''} + 'filter_rules': '', + 'mark': constants.ROUTER_MARK_MASK} NAT_TEMPLATE = ('# Generated by iptables_manager\n' '*nat\n' @@ -603,10 +605,9 @@ class IptablesManagerStateFulTestCase(base.BaseTestCase): '[0:0] -A OUTPUT -j %(bn)s-OUTPUT\n' '[0:0] -A POSTROUTING -j %(bn)s-POSTROUTING\n' '[0:0] -A %(bn)s-PREROUTING -j %(bn)s-mark\n' - '[0:0] -A %(bn)s-PREROUTING -j MARK --set-xmark 0x1/0xffffffff\n' + '[0:0] -A %(bn)s-PREROUTING -j MARK --set-xmark 0x1/%(mark)s\n' 'COMMIT\n' - '# Completed by iptables_manager\n' - % IPTABLES_ARG) + '# Completed by iptables_manager\n' % IPTABLES_ARG) expected_calls_and_values = [ (mock.call(['iptables-save', '-c'], @@ -635,13 +636,13 @@ class IptablesManagerStateFulTestCase(base.BaseTestCase): self.iptables.ipv4['mangle'].add_chain('mangle') self.iptables.ipv4['mangle'].add_rule( 'PREROUTING', - '-j MARK --set-xmark 0x1/0xffffffff') + '-j MARK --set-xmark 0x1/%s' % constants.ROUTER_MARK_MASK) self.iptables.apply() self.iptables.ipv4['mangle'].remove_rule( 'PREROUTING', - '-j MARK --set-xmark 0x1/0xffffffff') + '-j MARK --set-xmark 0x1/%s' % constants.ROUTER_MARK_MASK) self.iptables.ipv4['mangle'].remove_chain('mangle') self.iptables.apply() diff --git a/neutron/tests/unit/agent/metadata/test_driver.py b/neutron/tests/unit/agent/metadata/test_driver.py index 5cbfd182f..d86c4fbce 100644 --- a/neutron/tests/unit/agent/metadata/test_driver.py +++ b/neutron/tests/unit/agent/metadata/test_driver.py @@ -23,6 +23,7 @@ from neutron.agent.l3 import config as l3_config from neutron.agent.l3 import ha as l3_ha_agent from neutron.agent.metadata import config from neutron.agent.metadata import driver as metadata_driver +from neutron.common import constants from neutron.tests import base @@ -39,7 +40,8 @@ class TestMetadataDriverRules(base.BaseTestCase): metadata_driver.MetadataDriver.metadata_nat_rules(8775)) def test_metadata_filter_rules(self): - rules = [('INPUT', '-m mark --mark 0x1 -j ACCEPT'), + rules = [('INPUT', '-m mark --mark 0x1/%s -j ACCEPT' % + constants.ROUTER_MARK_MASK), ('INPUT', '-p tcp -m tcp --dport 8775 -j DROP')] self.assertEqual( rules, @@ -49,7 +51,7 @@ class TestMetadataDriverRules(base.BaseTestCase): rule = ('PREROUTING', '-d 169.254.169.254/32 ' '-p tcp -m tcp --dport 80 ' '-j MARK --set-xmark 0x1/%s' % - metadata_driver.METADATA_ACCESS_MARK_MASK) + constants.ROUTER_MARK_MASK) self.assertEqual( [rule], metadata_driver.MetadataDriver.metadata_mangle_rules('0x1')) -- 2.45.2