[OVN] Set always the GW LRP "gateway_mtu" option

If the configuration option "ovn.ovn_emit_need_to_frag" is set, write
always the "LSP.options.gateway_mtu" value, using the minimum MTU
value of all networks connected to the router. That will limit the
N/S traffic MTU regardless if the gateway network MTU is greater or
less than the internal network MTU.

Closes-Bug: #2032817
Change-Id: Ia1f0c0955d2fd90c7ff6307b1bc9819721ee2f9b

doc/source/admin/ovn/routing.rst

@@ -181,3 +181,21 @@ encapsulation) it's been included for completeness.
 Traffic goes directly from instance to instance through br-int in the case
 of both instances living in the same host (VM1 and VM2), or via
 encapsulation when living on different hosts (VM3 and VM4).
+
+
+Packet fragmentation
+~~~~~~~~~~~~~~~~~~~~
+
+The Neutron configuration variable ``[ovn]ovn_emit_need_to_frag`` configures
+OVN to emit the "need to frag" packets in case of MTU mismatches. This
+configuration option allows Neutron to set, in the router gateway
+``Logical_Router_Port``, the option "gateway_mtu". If a packet from any
+network reaches the gateway ``Logical_Router_Port``, OVN will send the "need
+for frag" message.
+
+In order to allow any E/W or N/S traffic to cross the router, the value of
+"gateway_mtu" will have the lowest MTU value off all networks connected to the
+router. This could impact the performance of the traffic using the networks
+connected to the router if the MTU defined is low. But the user can unset the
+Neutron configuration flag in order to avoid the fragmentation, at the cost
+of limiting the communication between networks with different MTUs.

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

@@ -1686,14 +1686,12 @@ class OVNClient:
                         for net in networks) else 'false'
         return reside_redir_ch
 
-    def _gen_router_port_options(self, port, network_mtu=None):
+    def _gen_router_port_options(self, port):
         options = {}
         admin_context = n_context.get_admin_context()
         ls_name = utils.ovn_name(port['network_id'])
         ls = self._nb_idl.ls_get(ls_name).execute(check_error=True)
         network_type = ls.external_ids[ovn_const.OVN_NETTYPE_EXT_ID_KEY]
-        network_mtu = network_mtu or int(
-            ls.external_ids[ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY])
         # For provider networks (VLAN, FLAT types) we need to set the
         # "reside-on-redirect-chassis" option so the routing for this
         # logical router port is centralized in the chassis hosting the
@@ -1717,16 +1715,16 @@ class OVNClient:
                 LOG.debug("Router %s not found", port['device_id'])
             else:
                 network_ids = {port['network_id'] for port in router_ports}
+                # If this method is called during a port creation, the port
+                # won't be present yet in the router ports list.
+                network_ids.add(port['network_id'])
                 networks = None
                 if ovn_conf.is_ovn_emit_need_to_frag_enabled():
                     networks = self._plugin.get_networks(
                         admin_context, filters={'id': network_ids})
-                    for net in networks:
+                    # Set the lower MTU of all networks connected to the router
-                        if net['mtu'] > network_mtu:
+                    min_mtu = str(min(net['mtu'] for net in networks))
-                            options[
+                    options[ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION] = min_mtu
-                                ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION] = str(
-                                    network_mtu)
-                            break
                 if ovn_conf.is_ovn_distributed_floating_ip():
                     # NOTE(ltomasbo): For VLAN type networks connected through
                     # the gateway port there is a need to set the redirect-type
@@ -2136,8 +2134,7 @@ class OVNClient:
         commands = []
         for port in ports:
             lrp_name = utils.ovn_lrouter_port_name(port['id'])
-            options = self._gen_router_port_options(
+            options = self._gen_router_port_options(port)
-                port, network_mtu=prov_net['mtu'])
             # Do not fail for cases where logical router port get deleted
             commands.append(self._nb_idl.lrp_set_options(lrp_name,
                                                          if_exists=True,

neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py

@@ -223,8 +223,7 @@ class TestOVNClient(base.TestOVNFunctionalBase,
         ovn_client.delete_address_group(self.context, ag['id'])
         self.assertEqual((None, None), _find_address_set_for_ag())
 
-    def test_update_network_lrp_mtu_updated(self):
+    def _check_gw_lrp_mtu(self, router_id, mtu):
-        def check_gw_lrp_mtu(router_id, mtu):
         # Find gateway LRP and check the MTU value.
         lr = self.nb_api.lookup('Logical_Router',
                                 ovn_utils.ovn_name(router_id))
@@ -238,6 +237,7 @@ class TestOVNClient(base.TestOVNFunctionalBase,
         self.fail('Gateway Logical_Router_Port not found for '
                   'router %s' % router_id)
 
+    def test_update_network_lrp_mtu_updated(self):
         self.add_fake_chassis('host1', enable_chassis_as_gw=True, azs=[])
         net_ext_args = {provider_net.NETWORK_TYPE: 'geneve',
                         external_net.EXTERNAL: True,
@@ -260,11 +260,52 @@ class TestOVNClient(base.TestOVNFunctionalBase,
                         'add', router_id, snet_int['subnet']['id'],
                         None)
 
-                    check_gw_lrp_mtu(router_id, 1300)
+                    self._check_gw_lrp_mtu(router_id, 1300)
 
                     # Update external network MTU.
                     net_ext_args = {'network': {mtu_def.MTU: 1350}}
                     req = self.new_update_request('networks', net_ext_args,
                                                   net_ext['network']['id'])
                     req.get_response(self.api)
-                    check_gw_lrp_mtu(router_id, 1350)
+                    self._check_gw_lrp_mtu(router_id, 1350)
+
+    def test_add_new_router_interfaces_lrp_mtu_updated(self):
+        mtu_ext = 1350
+        internal_interfaces = [{'cidr': '10.2.0.0/24', 'mtu': mtu_ext - 10},
+                               {'cidr': '10.3.0.0/24', 'mtu': mtu_ext + 10},
+                               {'cidr': '10.4.0.0/24', 'mtu': mtu_ext - 20}]
+        self.add_fake_chassis('host1', enable_chassis_as_gw=True, azs=[])
+        net_ext_args = {provider_net.NETWORK_TYPE: 'geneve',
+                        external_net.EXTERNAL: True,
+                        mtu_def.MTU: mtu_ext}
+        # Store the network's MTUs connected to the router.
+        router_attached_net_mtus = {mtu_ext}
+        with self.network(
+                'test-ext-net', as_admin=True,
+                arg_list=tuple(net_ext_args.keys()), **net_ext_args) as \
+                net_ext:
+            ext_gw = {'network_id': net_ext['network']['id']}
+            with self.subnet(net_ext, cidr='10.1.0.0/24'), \
+                    self.router(external_gateway_info=ext_gw) as router:
+                router_id = router['router']['id']
+                for _int in internal_interfaces:
+                    # Attach a new internal interface to the router. If the new
+                    # network MTU is lower than the minimum MTU connected, the
+                    # GW LRP.options.gateway_mtu will be updated.
+                    net_int_args = {provider_net.NETWORK_TYPE: 'geneve',
+                                    mtu_def.MTU: _int['mtu']}
+                    router_attached_net_mtus.add(_int['mtu'])
+                    net_int = self._make_network(
+                        self.fmt, name='nettest', admin_state_up=True,
+                        **net_int_args)
+                    snet_int = self._make_subnet(
+                        self.fmt, net_int, constants.ATTR_NOT_SPECIFIED,
+                        _int['cidr'])
+                    self._router_interface_action(
+                        'add', router_id, snet_int['subnet']['id'],
+                        None)
+
+                    # The GW LRP "gateway_mtu" value should be the minimum
+                    # MTU connected to the router.
+                    self._check_gw_lrp_mtu(router_id,
+                                           min(router_attached_net_mtus))

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py

@@ -61,6 +61,7 @@ from neutron.db import ovn_revision_numbers_db
 from neutron.db import provisioning_blocks
 from neutron.db import securitygroups_db
 from neutron.db import segments_db
+from neutron.objects import network as network_obj
 from neutron.plugins.ml2.drivers.ovn.agent import neutron_agent
 from neutron.plugins.ml2.drivers.ovn.mech_driver import mech_driver
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import impl_idl_ovn
@@ -2621,6 +2622,12 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
         with self.subnet(network=network) as subnet:
             with self.port(subnet=subnet,
                            device_owner=const.DEVICE_OWNER_ROUTER_GW) as port:
+                # Manually set the network DB register MTU without calling
+                # the network update API.
+                net_obj = network_obj.Network.get_object(
+                    self.context, id=network['network']['id'])
+                net_obj.mtu = new_mtu
+                net_obj.update()
 
                 grps.return_value = [{'port_id': port['port']['id'],
                                       'network_id':network['network']['id']}]

neutron/tests/unit/services/ovn_l3/test_plugin.py

@@ -74,7 +74,8 @@ class BaseTestOVNL3RouterPluginMixin():
         revision_plugin.RevisionPlugin()
         # MTU needs to be 1442 instead of 1500 because GENEVE headers size
         # must be at least 38 when using OVN
-        network_attrs = {external_net.EXTERNAL: True, 'mtu': 1442}
+        default_mtu = 1442
+        network_attrs = {external_net.EXTERNAL: True, 'mtu': default_mtu}
         self.fake_network = \
             fake_resources.FakeNetwork.create_one_network(
                 attrs=network_attrs).info()
@@ -173,7 +174,10 @@ class BaseTestOVNL3RouterPluginMixin():
                 ovn_const.OVN_ROUTER_IS_EXT_GW: 'True',
                 ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'router-id',
             },
-            'options': {}}
+            'options': {
+                ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION: str(default_mtu)
+            }
+        }
         self.fake_floating_ip_attrs = {'floating_ip_address': '192.168.0.10',
                                        'fixed_ip_address': '10.0.0.10'}
         self.fake_floating_ip = fake_resources.FakeFloatingIp.create_one_fip(
@@ -2063,7 +2067,7 @@ class TestOVNL3RouterPlugin(BaseTestOVNL3RouterPluginMixin,
         self.fake_router_port['device_owner'] = (
             constants.DEVICE_OWNER_ROUTER_GW)
         gn.return_value = prov_net
-        gns.return_value = [self.fake_network]
+        gns.return_value = [self.fake_network, prov_net]
         ext_ids = {
             ovn_const.OVN_NETTYPE_EXT_ID_KEY: constants.TYPE_GENEVE,
             ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY: mtu,
@@ -2109,6 +2113,7 @@ class TestOVNL3RouterPluginEmitNeedToFrag(
         super().setUp()
         config.cfg.CONF.set_override(
             'ovn_emit_need_to_frag', False, group='ovn')
+        self.fake_ext_gw_port_assert['options'] = {}
 
 
 class OVNL3ExtrarouteTests(test_l3_gw.ExtGwModeIntTestCase,

releasenotes/notes/ovn_router_gateway_mtu-ac69e07b30093193.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    Now if the configuration option ``[ovn]ovn_emit_need_to_frag`` is set,
+    OVN will always set the "gateway_mtu" option in the gateway
+    ``Logical_Router_Port``. The value defined will be the lowest MTU of all
+    networks connected to this router.