[OVN] Set MTU of the VETH interfaces between OVS and metadata

The VETH pair between the metadata namespace and the local OVS now
has the same MTU as the network associated to this metadata service.
The "LSP.external_ids" field has a new key defined: "neutron:mtu".
This is the value of the network MTU.

This patch does not update the previous metadata datapaths nor the
existing LSP. This change will affect only to new created ports
and the corresponding metadata datapaths.

Conflicts:
    neutron/agent/ovn/metadata/agent.py

Closes-Bug: #2053274
Change-Id: I7ff300e9634e5e3fc68d70540392109fd8b9babc
(cherry picked from commit 47b4d14)
(cherry picked from commit 101898f)

Author: ralonsoh

neutron/agent/ovn/metadata/agent.py

@@ -100,7 +100,7 @@ def run(self, event, row, old):
         with _SYNC_STATE_LOCK.read_lock():
             self.log_row(row)
             try:
-                self.agent.provision_datapath(row.datapath)
+                self.agent.provision_datapath(row)
             except ConfigException:
                 # We're now in the reader lock mode, we need to exit the
                 # context and then use writer lock
@@ -400,12 +400,12 @@ def _get_ovn_bridge(self):
                         "br-int instead.")
             return 'br-int'
 
-    def get_networks_datapaths(self):
-        """Return a set of datapath objects of the VIF ports on the current
+    def get_networks_port_bindings(self):
+        """Return a set of Port_Binding objects of the VIF ports on the current
         chassis.
         """
         ports = self.sb_idl.get_ports_on_chassis(self.chassis)
-        return set(p.datapath for p in self._vif_ports(ports))
+        return list(self._vif_ports(ports))
 
     @_sync_lock
     def sync(self, provision=True):
@@ -420,12 +420,12 @@ def sync(self, provision=True):
         system_namespaces = tuple(
             ns.decode('utf-8') if isinstance(ns, bytes) else ns
             for ns in ip_lib.list_network_namespaces())
-        net_datapaths = self.get_networks_datapaths()
-        metadata_namespaces = [
+        net_port_bindings = self.get_networks_port_bindings()
+        metadata_namespaces = set(
             self._get_namespace_name(
                 ovn_utils.get_network_name_from_datapath(datapath))
-            for datapath in net_datapaths
-        ]
+            for datapath in (pb.datapath for pb in net_port_bindings)
+        )
         unused_namespaces = [ns for ns in system_namespaces if
                              ns.startswith(NS_PREFIX) and
                              ns not in metadata_namespaces]
@@ -439,8 +439,8 @@ def sync(self, provision=True):
         # even those that are already running. This is to make sure
         # everything within each namespace is up to date.
         if provision:
-            for datapath in net_datapaths:
-                self.provision_datapath(datapath)
+            for port_binding in net_port_bindings:
+                self.provision_datapath(port_binding)
 
     @staticmethod
     def _get_veth_name(datapath):
@@ -611,19 +611,21 @@ def _get_provision_params(self, datapath):
 
         return net_name, datapath_ports_ips, metadata_port_info
 
-    def provision_datapath(self, datapath):
+    def provision_datapath(self, port_binding):
         """Provision the datapath so that it can serve metadata.
 
         This function will create the namespace and VETH pair if needed
         and assign the IP addresses to the interface corresponding to the
         metadata port of the network. It will also remove existing IP from
         the namespace if they are no longer needed.
 
-        :param datapath: datapath object.
-        :return: The metadata namespace name for the datapath or None
-                 if namespace was not provisioned
+        :param port_binding: Port_Binding object.
+        :return: The metadata namespace name for the Port_Binding.datapath or
+                 None if namespace was not provisioned
         """
-
+        datapath = port_binding.datapath
+        mtu = int(port_binding.external_ids.get(
+            ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY) or '0')
         provision_params = self._get_provision_params(datapath)
         if not provision_params:
             return
@@ -652,6 +654,11 @@ def provision_datapath(self, datapath):
         # Configure the MAC address.
         ip2.link.set_address(metadata_port_info.mac)
 
+        # Set VETH ports MTU.
+        if mtu:
+            ip1.link.set_mtu(mtu)
+            ip2.link.set_mtu(mtu)
+
         # Make sure both ends of the VETH are up
         ip1.link.set_up()
         ip2.link.set_up()

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -79,6 +79,7 @@
         "address6_scope_id",
         "vnic_type",
         "capabilities",
+        "mtu",
     ],
 )
 
@@ -324,6 +325,7 @@ def _get_port_options(self, port):
         address6_scope_id = ""
         dhcpv4_options = self._get_port_dhcp_options(port, const.IP_VERSION_4)
         dhcpv6_options = self._get_port_dhcp_options(port, const.IP_VERSION_6)
+        mtu = ''
         if vtep_physical_switch:
             vtep_logical_switch = bp_info.bp_param.get('vtep-logical-switch')
             port_type = 'vtep'
@@ -421,10 +423,10 @@ def _get_port_options(self, port):
                     ovn_const.VIF_DETAILS_PF_MAC_ADDRESS in bp_info.bp_param):
                 port_net = self._plugin.get_network(
                     context, port['network_id'])
+                mtu = str(port_net['mtu'])
                 options.update({
                     ovn_const.LSP_OPTIONS_VIF_PLUG_TYPE_KEY: 'representor',
-                    ovn_const.LSP_OPTIONS_VIF_PLUG_MTU_REQUEST_KEY: str(
-                        port_net['mtu']),
+                    ovn_const.LSP_OPTIONS_VIF_PLUG_MTU_REQUEST_KEY: mtu,
                     ovn_const.LSP_OPTIONS_VIF_PLUG_REPRESENTOR_PF_MAC_KEY: (
                         bp_info.bp_param.get(
                             ovn_const.VIF_DETAILS_PF_MAC_ADDRESS)),
@@ -465,7 +467,7 @@ def _get_port_options(self, port):
                            parent_name, tag, dhcpv4_options, dhcpv6_options,
                            cidrs.strip(), device_owner, sg_ids,
                            address4_scope_id, address6_scope_id,
-                           bp_info.vnic_type, bp_info.capabilities
+                           bp_info.vnic_type, bp_info.capabilities, mtu
                            )
 
     def update_port_dhcp_options(self, port_info, txn):
@@ -506,6 +508,7 @@ def get_external_ids_from_port(self, port):
             ovn_const.OVN_PORT_VNIC_TYPE_KEY: port_info.vnic_type,
             ovn_const.OVN_PORT_BP_CAPABILITIES_KEY:
                 ';'.join(port_info.capabilities),
+            ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY: port_info.mtu,
         }
         return port_info, external_ids
 

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

@@ -28,6 +28,7 @@
 from neutron.agent.linux.ip_lib import IPWrapper as ip_wrap
 from neutron.agent.ovn.metadata import agent
 from neutron.agent.ovn.metadata import driver
+from neutron.common.ovn import constants as ovn_const
 from neutron.conf.agent.metadata import config as meta_conf
 from neutron.conf.agent.ovn.metadata import config as ovn_meta_conf
 from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
@@ -100,14 +101,8 @@ def test_sync(self):
 
             self.agent.sync()
 
-            pdp.assert_has_calls(
-                [
-                    mock.call(p.datapath)
-                    for p in self.ports
-                ],
-                any_order=True
-            )
-
+            pdp.assert_has_calls([mock.call(p) for p in self.ports],
+                                 any_order=True)
             lnn.assert_called_once_with()
             tdp.assert_not_called()
 
@@ -124,13 +119,8 @@ def test_sync_teardown_namespace(self):
 
             self.agent.sync()
 
-            pdp.assert_has_calls(
-                [
-                    mock.call(p.datapath)
-                    for p in self.ports
-                ],
-                any_order=True
-            )
+            pdp.assert_has_calls([mock.call(p) for p in self.ports],
+                                 any_order=True)
             lnn.assert_called_once_with()
             tdp.assert_called_once_with('3')
 
@@ -149,27 +139,23 @@ def test_sync_teardown_namespace_does_not_crash_on_error(self):
                     side_effect=Exception()) as tdp:
             self.agent.sync()
 
-            pdp.assert_has_calls(
-                [
-                    mock.call(p.datapath)
-                    for p in self.ports
-                ],
-                any_order=True
-            )
+            pdp.assert_has_calls([mock.call(p) for p in self.ports],
+                                 any_order=True)
             lnn.assert_called_once_with()
             tdp.assert_called_once_with('3')
 
-    def test_get_networks_datapaths(self):
-        """Test get_networks_datapaths returns only datapath objects for the
-        networks containing vif ports of type ''(blank) and 'external'.
+    def test_get_networks_port_bindings(self):
+        """Test get_networks_port_bindings returns only the port binding
+        objects for ports with VIF type empty ('') or 'external'.
         This test simulates that this chassis has the following ports:
-            * datapath '1': 1 port type '' , 1 port 'external' and
-                            1 port 'unknown'
-            * datapath '2': 1 port type ''
-            * datapath '3': 1 port with type 'external'
-            * datapath '4': 1 port with type 'unknown'
-
-        It is expected that only datapaths '1', '2' and '3' are returned
+            * port0: datapath 1, type ''
+            * port1: datapath 1, type 'external'
+            * port2: datapath 1, type 'unknown'
+            * port3: datapath 2, type ''
+            * port4: datapath 3, type 'external'
+            * port5: datapath 4, type 'unknown'
+
+        Only port bindings from ports 0, 1, 3, and 4 are expected.
         """
 
         datapath_1 = DatapathInfo(uuid='uuid1',
@@ -192,11 +178,8 @@ def test_get_networks_datapaths(self):
 
         with mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis',
                               return_value=ports):
-            expected_datapaths = set([datapath_1, datapath_2, datapath_3])
-            self.assertSetEqual(
-                expected_datapaths,
-                self.agent.get_networks_datapaths()
-            )
+            self.assertEqual([ports[0], ports[1], ports[3], ports[4]],
+                             self.agent.get_networks_port_bindings())
 
     def test_teardown_datapath(self):
         """Test teardown datapath.
@@ -424,7 +407,8 @@ def test_provision_datapath(self):
                 mock.patch.object(agent.MetadataAgent, '_get_namespace_name',
                                   return_value=nemaspace_name),\
                 mock.patch.object(ip_link, 'set_up') as link_set_up,\
-                mock.patch.object(ip_link, 'set_address') as link_set_addr,\
+                mock.patch.object(ip_link, 'set_address') as link_set_addr, \
+                mock.patch.object(ip_link, 'set_mtu') as link_set_mtu, \
                 mock.patch.object(ip_addr, 'list', return_value=[]),\
                 mock.patch.object(
                     ip_addr, 'add_multiple') as ip_addr_add_multiple,\
@@ -442,7 +426,11 @@ def test_provision_datapath(self):
             # We need to assert that it was deleted first.
             self.agent.ovs_idl.list_br.return_value.execute.return_value = (
                 ['br-int', 'br-fake'])
-            self.agent.provision_datapath('fake_datapath')
+            mtu = 1500
+            port_binding = mock.Mock(
+                datapath='fake_datapath',
+                external_ids={ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY: str(mtu)})
+            self.agent.provision_datapath(port_binding)
 
             # Check that the port was deleted from br-fake
             self.agent.ovs_idl.del_port.assert_called_once_with(
@@ -452,6 +440,7 @@ def test_provision_datapath(self):
                 nemaspace_name)
             # Make sure that the two ends of the VETH pair have been set as up.
             self.assertEqual(2, link_set_up.call_count)
+            link_set_mtu.assert_has_calls([mock.call(mtu), mock.call(mtu)])
             link_set_addr.assert_called_once_with('aa:bb:cc:dd:ee:ff')
             # Make sure that the port has been added to OVS.
             self.agent.ovs_idl.add_port.assert_called_once_with(