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

@@ -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.')),
 ]

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):

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):

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"

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 '
+            '-m mark ! --mark 0x2/%s -m conntrack --ctstate DNAT '
-            '-j SNAT --to-source %s' % source_nat_ip]
+            '-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):

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'
+            '# Completed by iptables_manager\n' % IPTABLES_ARG)
-            % 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()

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'))