[OVN] Ensure metadata checksum

Ensure the TCP traffic leaving the OVN metadata namespace has
the checksum correctly populated. This is necessary when the
OVS datapath is "netdev".

Because the overhead added is minimal and only applies to the
metadata traffic inside the metadata namespace, this rule is
always set.

Change-Id: I7e39f40b325a6974a46ed34641cec5226c9e5a3f
Closes-Bug: #1904871

neutron/agent/linux/iptables_manager.py

@@ -308,7 +308,8 @@ class IptablesManager(object):
     _random_fully = None
 
     def __init__(self, _execute=None, state_less=False, use_ipv6=False,
-                 nat=True, namespace=None, binary_name=binary_name):
+                 nat=True, namespace=None, binary_name=binary_name,
+                 external_lock=True):
         if _execute:
             self.execute = _execute
         else:
@@ -318,6 +319,7 @@ class IptablesManager(object):
         self.namespace = namespace
         self.iptables_apply_deferred = False
         self.wrap_name = binary_name[:16]
+        self.external_lock = external_lock
 
         self.ipv4 = {'filter': IptablesTable(binary_name=self.wrap_name)}
         self.ipv6 = {'filter': IptablesTable(binary_name=self.wrap_name)}
@@ -463,7 +465,8 @@ class IptablesManager(object):
         # NOTE(ihrachys) we may get rid of the lock once all supported
         # platforms get iptables with 999eaa241212d3952ddff39a99d0d55a74e3639e
         # ("iptables-restore: support acquiring the lock.")
-        with lockutils.lock(lock_name, runtime.SYNCHRONIZED_PREFIX, True):
+        with lockutils.lock(lock_name, runtime.SYNCHRONIZED_PREFIX,
+                            external=self.external_lock):
             first = self._apply_synchronized()
             if not cfg.CONF.AGENT.debug_iptables_rules:
                 return first

neutron/agent/ovn/metadata/agent.py

@@ -18,6 +18,7 @@ import re
 
 from neutron.agent.linux import external_process
 from neutron.agent.linux import ip_lib
+from neutron.agent.linux import iptables_manager
 from neutron.agent.ovn.metadata import driver as metadata_driver
 from neutron.agent.ovn.metadata import ovsdb
 from neutron.agent.ovn.metadata import server as metadata_server
@@ -361,6 +362,23 @@ class MetadataAgent(object):
         else:
             self.teardown_datapath(datapath)
 
+    def _ensure_datapath_checksum(self, namespace):
+        """Ensure the correct checksum in the metadata packets in DPDK bridges
+
+        (LP#1904871) In DPDK deployments (integration bridge datapath_type ==
+        "netdev"), the checksum between the metadata namespace and OVS is not
+        correctly populated.
+        """
+        if (self.ovs_idl.db_get('Bridge', self.ovn_bridge, 'datapath_type') !=
+                ovn_const.CHASSIS_DATAPATH_NETDEV):
+            return
+
+        iptables_mgr = iptables_manager.IptablesManager(
+            use_ipv6=True, nat=False, namespace=namespace, external_lock=False)
+        rule = '-p tcp -m tcp -j CHECKSUM --checksum-fill'
+        iptables_mgr.ipv4['mangle'].add_rule('POSTROUTING', rule, wrap=False)
+        iptables_mgr.apply()
+
     def provision_datapath(self, datapath):
         """Provision the datapath so that it can serve metadata.
 
@@ -468,6 +486,9 @@ class MetadataAgent(object):
             'Interface', veth_name[0],
             ('external_ids', {'iface-id': port.logical_port})).execute()
 
+        # Ensure the correct checksum in the metadata traffic.
+        self._ensure_datapath_checksum(namespace)
+
         # Spawn metadata proxy if it's not already running.
         metadata_driver.MetadataDriver.spawn_monitored_metadata_proxy(
             self._process_monitor, namespace, n_const.METADATA_PORT,

neutron/tests/functional/agent/ovn/metadata/test_metadata_agent.py

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import re
 from unittest import mock
 
 from oslo_config import fixture as fixture_config
@@ -21,6 +22,7 @@ from ovsdbapp.backend.ovs_idl import event
 from ovsdbapp.backend.ovs_idl import idlutils
 from ovsdbapp.tests.functional.schema.ovn_southbound import event as test_event
 
+from neutron.agent.linux import iptables_manager
 from neutron.agent.ovn.metadata import agent
 from neutron.agent.ovn.metadata import ovsdb
 from neutron.agent.ovn.metadata import server as metadata_server
@@ -28,6 +30,7 @@ from neutron.common.ovn import constants as ovn_const
 from neutron.common import utils as n_utils
 from neutron.conf.agent.metadata import config as meta_config
 from neutron.conf.agent.ovn.metadata import config as meta_config_ovn
+from neutron.tests.common import net_helpers
 from neutron.tests.functional import base
 
 
@@ -56,7 +59,12 @@ class TestMetadataAgent(base.TestOVNFunctionalBase):
         super(TestMetadataAgent, self).setUp()
         self.handler = self.sb_api.idl.notify_handler
         # We only have OVN NB and OVN SB running for functional tests
-        mock.patch.object(ovsdb, 'MetadataAgentOvsIdl').start()
+        self.mock_ovsdb_idl = mock.Mock()
+        mock_metadata_instance = mock.Mock()
+        mock_metadata_instance.start.return_value = self.mock_ovsdb_idl
+        mock_metadata = mock.patch.object(
+            ovsdb, 'MetadataAgentOvsIdl').start()
+        mock_metadata.return_value = mock_metadata_instance
         self._mock_get_ovn_br = mock.patch.object(
             agent.MetadataAgent,
             '_get_ovn_bridge',
@@ -303,3 +311,19 @@ class TestMetadataAgent(base.TestOVNFunctionalBase):
             ('external_ids', {'test': 'value'})).execute(check_error=True)
         self.assertTrue(event2.wait())
         self.assertFalse(event.wait())
+
+    def test__ensure_datapath_checksum_if_dpdk(self):
+        self.mock_ovsdb_idl.db_get.return_value = (
+            ovn_const.CHASSIS_DATAPATH_NETDEV)
+        regex = re.compile(r'-A POSTROUTING -p tcp -m tcp '
+                           r'-j CHECKSUM --checksum-fill')
+        namespace = self.useFixture(net_helpers.NamespaceFixture()).name
+        self.agent._ensure_datapath_checksum(namespace)
+        iptables_mgr = iptables_manager.IptablesManager(
+            use_ipv6=True, nat=False, namespace=namespace, external_lock=False)
+        for rule in iptables_mgr.get_rules_for_table('mangle'):
+            if regex.match(rule):
+                return
+        else:
+            self.fail('Rule not found in "mangle" table, in namespace %s' %
+                      namespace)

neutron/tests/unit/agent/ovn/metadata/test_agent.py

@@ -235,7 +235,9 @@ class TestMetadataAgent(base.BaseTestCase):
                     'update_chassis_metadata_networks') as update_chassis,\
                 mock.patch.object(
                     driver.MetadataDriver,
-                    'spawn_monitored_metadata_proxy') as spawn_mdp:
+                    'spawn_monitored_metadata_proxy') as spawn_mdp, \
+                mock.patch.object(
+                    self.agent, '_ensure_datapath_checksum') as mock_checksum:
 
             # Simulate that the VETH pair was already present in 'br-fake'.
             # We need to assert that it was deleted first.
@@ -268,6 +270,7 @@ class TestMetadataAgent(base.BaseTestCase):
                 bind_address=n_const.METADATA_V4_IP, network_id='1')
             # Check that the chassis has been updated with the datapath.
             update_chassis.assert_called_once_with('1')
+            mock_checksum.assert_called_once_with('namespace')
 
     def _test_update_chassis_metadata_networks_helper(
             self, dp, remove, expected_dps, txn_called=True):