OVN to emit ICMP "fragmentation needed" packets

A new configuration option called "ovn_emit_need_to_frag" has been added
and defaults to False. Reason being, it requires a specific kernel
version for it to work (upstream >= 5.2). It's also possible to find out
if OVN supports that operation by running the following command as root:

$ ovs-appctl -t ovs-vswitchd dpif/show-dp-features br-int
(check for "Check pkt length action: Yes").

WARNING: Enabling this option without kernel support will result in a
real performance degradation.

At earlier versions of this patch we thought about having networking-ovn
to discover if this option is supported or not (by issuing that
ovs-appctl command above) but, since networking-ovn doesn't run in the
network nodes we thought that an upper level tool (such as puppet-ovn or
TripleO) could do a better job at checking for it and configuring things
accordingly.

Closes-Bug: #1838405
Change-Id: I089f95b40803a6cd5e01990acacd599ced3bbd91
Signed-off-by: Lucas Alvares Gomes <lucasagomes@gmail.com>

networking_ovn/common/config.py

@@ -180,6 +180,15 @@ ovn_opts = [
                        "- ntp_server:,wpad:1.2.3.5 - Unset ntp_server and "
                        "set wpad\n"
                        "See the ovn-nb(5) man page for available options.")),
+    cfg.BoolOpt('ovn_emit_need_to_frag',
+                default=False,
+                help=_('Configure OVN to emit "need to frag" packets in '
+                       'case of MTU mismatch.\n'
+                       'Before enabling this configuration make sure that '
+                       'its supported by the host kernel (version >= 5.2) '
+                       'or by checking the output of the following command: \n'
+                       'ovs-appctl -t ovs-vswitchd dpif/show-dp-features '
+                       'br-int | grep "Check pkt length action".')),
 ]
 
 cfg.CONF.register_opts(ovn_opts, group='ovn')
@@ -288,3 +297,7 @@ def setup_logging():
              {'prog': sys.argv[0],
               'version': version.version_info.release_string()})
     LOG.debug("command line: %s", " ".join(sys.argv))
+
+
+def is_ovn_emit_need_to_frag_enabled():
+    return cfg.CONF.ovn.ovn_emit_need_to_frag

networking_ovn/common/constants.py

@@ -53,6 +53,7 @@ OVN_GATEWAY_CHASSIS_KEY = 'redirect-chassis'
 OVN_CHASSIS_REDIRECT = 'chassisredirect'
 OVN_GATEWAY_NAT_ADDRESSES_KEY = 'nat-addresses'
 OVN_DROP_PORT_GROUP_NAME = 'neutron_pg_drop'
+OVN_ROUTER_PORT_GW_MTU_OPTION = 'gateway_mtu'
 
 OVN_PROVNET_PORT_NAME_PREFIX = 'provnet-'
 

networking_ovn/common/maintenance.py

@@ -17,6 +17,7 @@ import inspect
 import threading
 
 from futurist import periodics
+from neutron_lib.api.definitions import external_net
 from neutron_lib import constants as n_const
 from neutron_lib import context as n_context
 from neutron_lib import exceptions as n_exc
@@ -435,6 +436,20 @@ class DBInconsistenciesPeriodics(object):
 
         raise periodics.NeverAgain()
 
+    # A static spacing value is used here, but this method will only run
+    # once per lock due to the use of periodics.NeverAgain().
+    @periodics.periodic(spacing=600, run_immediately=True)
+    def check_for_fragmentation_support(self):
+        if not self.has_lock:
+            return
+
+        context = n_context.get_admin_context()
+        for net in self._ovn_client._plugin.get_networks(
+                context, {external_net.EXTERNAL: [True]}):
+            self._ovn_client.set_gateway_mtu(context, net)
+
+        raise periodics.NeverAgain()
+
 
 class HashRingHealthCheckPeriodics(object):
 

networking_ovn/common/ovn_client.py

@@ -1157,6 +1157,27 @@ class OVNClient(object):
 
         return ext_ids
 
+    def _gen_router_port_options(self, port, network=None):
+        options = {}
+        if network is None:
+            network = self._plugin.get_network(n_context.get_admin_context(),
+                                               port['network_id'])
+        # For VLAN type networks 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
+        # distributed gateway port.
+        # https://github.com/openvswitch/ovs/commit/85706c34d53d4810f54bec1de662392a3c06a996
+        if network.get(pnet.NETWORK_TYPE) == const.TYPE_VLAN:
+            options['reside-on-redirect-chassis'] = 'true'
+
+        is_gw_port = const.DEVICE_OWNER_ROUTER_GW == port.get(
+            'device_owner')
+        if is_gw_port and config.is_ovn_emit_need_to_frag_enabled():
+            options[ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION] = str(
+                network['mtu'])
+
+        return options
+
     def _create_lrouter_port(self, router_id, port, txn=None):
         """Create a logical router port."""
         lrouter = utils.ovn_name(router_id)
@@ -1167,17 +1188,11 @@ class OVNClient(object):
         is_gw_port = const.DEVICE_OWNER_ROUTER_GW == port.get(
             'device_owner')
         columns = {}
-        port_net = self._plugin.get_network(n_context.get_admin_context(),
-                                            port['network_id'])
-        # For VLAN type networks 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
-        # distributed gateway port.
-        # https://github.com/openvswitch/ovs/commit/85706c34d53d4810f54bec1de662392a3c06a996
-        if port_net.get(pnet.NETWORK_TYPE) == const.TYPE_VLAN:
-            columns['options'] = {'reside-on-redirect-chassis': 'true'}
+        columns['options'] = self._gen_router_port_options(port)
 
         if is_gw_port:
+            port_net = self._plugin.get_network(n_context.get_admin_context(),
+                                                port['network_id'])
             physnet = self._get_physnet(port_net)
             candidates = self.get_candidates_for_scheduling(physnet)
             selected_chassis = self._ovn_scheduler.select(
@@ -1254,6 +1269,7 @@ class OVNClient(object):
             self._nb_idl.update_lrouter_port(
                 name=lrp_name,
                 external_ids=self._gen_router_port_ext_ids(port),
+                options=self._gen_router_port_options(port),
                 if_exists=if_exists,
                 **update),
             self._nb_idl.set_lrouter_port_in_lswitch_port(
@@ -1429,6 +1445,21 @@ class OVNClient(object):
         return new_qos_id != ovn_net.external_ids[
             ovn_const.OVN_QOS_POLICY_EXT_ID_KEY]
 
+    def set_gateway_mtu(self, context, prov_net, txn=None):
+        ports = self._plugin.get_ports(
+            context, filters=dict(network_id=[prov_net['id']],
+                                  device_owner=[const.DEVICE_OWNER_ROUTER_GW]))
+        commands = []
+        for port in ports:
+            lrp_name = utils.ovn_lrouter_port_name(port['id'])
+            # TODO(lucasagomes): Use lrp_set_options() once
+            # https://review.opendev.org/671765 is merged and a new version
+            # of ovsdbapp is released
+            options = self._gen_router_port_options(port, prov_net)
+            commands.append(self._nb_idl.update_lrouter_port(
+                            name=lrp_name, if_exists=True, options=options))
+        self._transaction(commands, txn=txn)
+
     def update_network(self, network):
         lswitch_name = utils.ovn_name(network['id'])
         # Check if QoS needs to be update, before updating OVNDB
@@ -1480,6 +1511,9 @@ class OVNClient(object):
                 for subnet in subnets:
                     self.update_subnet(subnet, network, txn)
 
+                if utils.is_provider_network(network):
+                    self.set_gateway_mtu(context, network, txn)
+
         if check_rev_cmd.result == ovn_const.TXN_COMMITTED:
             if qos_update_required:
                 self._qos_driver.update_network(network)

networking_ovn/common/utils.py

@@ -15,6 +15,7 @@ import os
 import re
 
 import netaddr
+from neutron_lib.api.definitions import external_net
 from neutron_lib.api.definitions import extra_dhcp_opt as edo_ext
 from neutron_lib.api.definitions import l3
 from neutron_lib.api.definitions import port_security as psec
@@ -414,3 +415,7 @@ def is_gateway_chassis_invalid(chassis_name, gw_chassis,
     elif gw_chassis and chassis_name not in gw_chassis:
         return True
     return False
+
+
+def is_provider_network(network):
+    return external_net.EXTERNAL in network

networking_ovn/tests/unit/l3/test_l3_ovn.py

@@ -20,6 +20,7 @@ from neutron.tests.unit.api import test_extensions
 from neutron.tests.unit.extensions import test_extraroute
 from neutron.tests.unit.extensions import test_l3
 from neutron.tests.unit.extensions import test_l3_ext_gw_mode as test_l3_gw
+from neutron_lib.api.definitions import external_net
 from neutron_lib.api.definitions import portbindings
 from neutron_lib.api.definitions import provider_net as pnet
 from neutron_lib.callbacks import events
@@ -51,7 +52,7 @@ class OVNL3RouterPlugin(test_mech_driver.OVNMechanismDriverTestCase):
     def setUp(self):
         super(OVNL3RouterPlugin, self).setUp()
         revision_plugin.RevisionPlugin()
-        network_attrs = {'router:external': True}
+        network_attrs = {external_net.EXTERNAL: True, 'mtu': 1500}
         self.fake_network = \
             fakes.FakeNetwork.create_one_network(attrs=network_attrs).info()
         self.fake_router_port = {'device_id': '',
@@ -67,6 +68,7 @@ class OVNL3RouterPlugin(test_mech_driver.OVNMechanismDriverTestCase):
             'name': 'lrp-router-port-id',
             'may_exist': True,
             'networks': ['10.0.0.100/24'],
+            'options': {},
             'external_ids': {
                 ovn_const.OVN_SUBNET_EXT_IDS_KEY: 'subnet-id',
                 ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1',
@@ -128,7 +130,8 @@ class OVNL3RouterPlugin(test_mech_driver.OVNMechanismDriverTestCase):
                 ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1',
                 ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY:
                 utils.ovn_name(self.fake_network['id'])},
-            'gateway_chassis': ['hv1']}
+            'gateway_chassis': ['hv1'],
+            'options': {}}
         self.fake_floating_ip_attrs = {'floating_ip_address': '192.168.0.10',
                                        'fixed_ip_address': '10.0.0.10'}
         self.fake_floating_ip = fakes.FakeFloatingIp.create_one_fip(
@@ -291,6 +294,7 @@ class OVNL3RouterPlugin(test_mech_driver.OVNMechanismDriverTestCase):
             if_exists=False, name='lrp-router-port-id',
             ipv6_ra_configs={},
             networks=['10.0.0.100/24'],
+            options={},
             external_ids={
                 ovn_const.OVN_SUBNET_EXT_IDS_KEY: 'subnet-id',
                 ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1',
@@ -1274,6 +1278,50 @@ class OVNL3RouterPlugin(test_mech_driver.OVNMechanismDriverTestCase):
             mock.call('lrp-foo-3',
                       gateway_chassis=['chassis3', 'chassis2', 'chassis1'])])
 
+    @mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.get_network')
+    @mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.get_port')
+    @mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.get_subnet')
+    @mock.patch('networking_ovn.common.ovn_client.OVNClient._get_router_ports')
+    @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.get_router')
+    @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.add_router_interface')
+    def test_add_router_interface_need_to_frag_enabled(self, ari, gr, grps,
+                                                       gs, gp, gn):
+
+        config.cfg.CONF.set_override(
+            'ovn_emit_need_to_frag', True, group='ovn')
+        router_id = 'router-id'
+        interface_info = {'port_id': 'router-port-id'}
+        ari.return_value = self.fake_router_interface_info
+        gr.return_value = self.fake_router_with_ext_gw
+        gs.return_value = self.fake_subnet
+        gn.return_value = self.fake_network
+        self.fake_router_port['device_owner'] = (
+            constants.DEVICE_OWNER_ROUTER_GW)
+        gp.return_value = self.fake_router_port
+
+        self.l3_inst.add_router_interface(self.context, router_id,
+                                          interface_info)
+
+        # Make sure that the "gateway_mtu" option was set to the router port
+        fake_router_port_assert = self.fake_router_port_assert
+        fake_router_port_assert['gateway_chassis'] = mock.ANY
+        fake_router_port_assert['options'] = {
+            ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION:
+            str(self.fake_network['mtu'])}
+
+        self.l3_inst._ovn.add_lrouter_port.assert_called_once_with(
+            **fake_router_port_assert)
+        self.l3_inst._ovn.set_lrouter_port_in_lswitch_port.\
+            assert_called_once_with(
+                'router-port-id', 'lrp-router-port-id', is_gw_port=True,
+                lsp_address=ovn_const.DEFAULT_ADDR_FOR_LSP_WITH_PEER)
+        self.l3_inst._ovn.add_nat_rule_in_lrouter.assert_called_once_with(
+            'neutron-router-id', logical_ip='10.0.0.0/24',
+            external_ip='192.168.1.1', type='snat')
+
+        self.bump_rev_p.assert_called_with(self.fake_router_port,
+                                           ovn_const.TYPE_ROUTER_PORTS)
+
 
 class OVNL3ExtrarouteTests(test_l3_gw.ExtGwModeIntTestCase,
                            test_l3.L3NatDBIntTestCase,

networking_ovn/tests/unit/ml2/test_mech_driver.py

@@ -25,6 +25,7 @@ from neutron.tests.unit.extensions import test_segment
 from neutron.tests.unit.plugins.ml2 import test_ext_portsecurity
 from neutron.tests.unit.plugins.ml2 import test_plugin
 from neutron.tests.unit.plugins.ml2 import test_security_group
+from neutron_lib.api.definitions import external_net
 from neutron_lib.api.definitions import portbindings
 from neutron_lib.api.definitions import provider_net as pnet
 from neutron_lib.callbacks import events
@@ -1624,6 +1625,43 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
     def test__update_dnat_entry_if_needed_down(self):
         self._test__update_dnat_entry_if_needed(up=False)
 
+    def _test_update_network_fragmentation(self, new_mtu, expected_opts):
+        network_attrs = {external_net.EXTERNAL: True}
+        network = self._make_network(
+            self.fmt, 'net1', True, arg_list=(external_net.EXTERNAL,),
+            **network_attrs)
+
+        with self.subnet(network=network) as subnet:
+            with self.port(subnet=subnet,
+                           device_owner=const.DEVICE_OWNER_ROUTER_GW) as port:
+                # Let's update the MTU to something different
+                network['network']['mtu'] = new_mtu
+                fake_ctx = mock.Mock(current=network['network'])
+                fake_ctx._plugin_context.session.is_active = False
+
+                self.mech_driver.update_network_postcommit(fake_ctx)
+
+                lrp_name = ovn_utils.ovn_lrouter_port_name(port['port']['id'])
+                self.nb_ovn.update_lrouter_port.assert_called_once_with(
+                    if_exists=True, name=lrp_name, options=expected_opts)
+
+    def test_update_network_need_to_frag_enabled(self):
+        ovn_config.cfg.CONF.set_override(
+            'ovn_emit_need_to_frag', True, group='ovn')
+        new_mtu = 1234
+        expected_opts = {ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION:
+                         str(new_mtu)}
+        self._test_update_network_fragmentation(new_mtu, expected_opts)
+
+    def test_update_network_need_to_frag_disabled(self):
+        ovn_config.cfg.CONF.set_override(
+            'ovn_emit_need_to_frag', False, group='ovn')
+        new_mtu = 1234
+        # Assert that the options column is empty (cleaning up an '
+        # existing value if set before)
+        expected_opts = {}
+        self._test_update_network_fragmentation(new_mtu, expected_opts)
+
 
 class OVNMechanismDriverTestCase(test_plugin.Ml2PluginV2TestCase):
     _mechanism_drivers = ['logger', 'ovn']

releasenotes/notes/fragmentation-support-2860870dc7b8bb6b.yaml

@@ -0,0 +1,11 @@
+---
+features:
+  - |
+    Adds a new configuration option called ``ovn_emit_need_to_frag``
+    which defaults to False. When enabled, networking-ovn will configure
+    OVN into emitting ICMP "need to frag" packets whenever needed. By
+    default this option is disabled because it requires a newer kernel
+    version for it to work (upstream >= 5.2). It's also possible to find
+    out if it's supported by the host by running the following command
+    as root: ``ovs-appctl -t ovs-vswitchd dpif/show-dp-features br-int``
+    (check for "Check pkt length action: Yes").