Make OVN driver validate Geneve max_header_size

Also updates the docs to be clearer on OVN-Geneve relation topics.

Co-Authored-By: Elvira García Ruiz <egarciar@redhat.com>
Change-Id: Ia253cc4d85261ce1535f4d27b3da91275d879903
Closes-bug: #1868137

doc/source/install/ovn/manual_install.rst

@@ -192,7 +192,14 @@ primary node. See the :doc:`/ovn/faq/index` for more information.
         network segments. However, OVN ignores the actual values. Thus, the ID
         range only determines the quantity of Geneve networks in the
         environment. For example, a range of ``5001:6000`` defines a maximum
-        of 1000 Geneve networks.
+        of 1000 Geneve networks. On the other hand, these values are still
+        relevant in Neutron context so ``1:1000`` and ``5001:6000`` are *not*
+        simply interchangeable.
+
+      .. warning::
+
+        The default for ``max_header_size``, ``30``, is too low for OVN.
+        OVN requires at least ``38``.
 
    * Optionally, enable support for VLAN provider and self-service
      networks on one or more physical networks. If you specify only

neutron/conf/plugins/ml2/drivers/driver_type.py

@@ -41,15 +41,18 @@ geneve_opts = [
                 default=[],
                 help=_("Comma-separated list of <vni_min>:<vni_max> tuples "
                        "enumerating ranges of Geneve VNI IDs that are "
-                       "available for tenant network allocation")),
+                       "available for tenant network allocation. "
+                       "Note OVN does not use the actual values.")),
     cfg.IntOpt('max_header_size',
                default=p_const.GENEVE_ENCAP_MIN_OVERHEAD,
-               help=_("Geneve encapsulation header size is dynamic, this "
+               help=_("The maximum allowed Geneve encapsulation header size "
-                      "value is used to calculate the maximum MTU "
+                      "(in bytes). "
-                      "for the driver. "
+                      "Geneve header is extensible, this value is used to "
-                      "The default size for this field is 30, which is the "
+                      "calculate the maximum MTU for Geneve-based networks. "
-                      "size of the Geneve header without any additional "
+                      "The default is 30, which is the size of the Geneve "
-                      "option headers.")),
+                      "header without any additional option headers. "
+                      "Note the default is not enough for OVN which "
+                      "requires at least 38.")),
 ]
 
 vxlan_opts = [

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

@@ -65,6 +65,7 @@ import neutron.wsgi
 
 
 LOG = log.getLogger(__name__)
+OVN_MIN_GENEVE_MAX_HEADER_SIZE = 38
 
 
 class OVNPortUpdateError(n_exc.BadRequest):
@@ -117,6 +118,13 @@ class OVNMechanismDriver(api.MechanismDriver):
         self._post_fork_event = threading.Event()
         if cfg.CONF.SECURITYGROUP.firewall_driver:
             LOG.warning('Firewall driver configuration is ignored')
+        if (cfg.CONF.ml2_type_geneve.max_header_size <
+                OVN_MIN_GENEVE_MAX_HEADER_SIZE):
+            LOG.critical('Geneve max_header_size set too low for OVN '
+                         '(%d vs %d)',
+                      cfg.CONF.ml2_type_geneve.max_header_size,
+                      OVN_MIN_GENEVE_MAX_HEADER_SIZE)
+            raise SystemExit(1)
         self._setup_vif_port_bindings()
         if impl_idl_ovn.OvsdbSbOvnIdl.schema_has_table('Chassis_Private'):
             self.agent_chassis_table = 'Chassis_Private'

neutron/tests/functional/base.py

@@ -177,6 +177,9 @@ class TestOVNFunctionalBase(test_plugin.Ml2PluginV2TestCase,
         ml2_config.cfg.CONF.set_override('vni_ranges',
                                      ['1:65536'],
                                      group='ml2_type_geneve')
+        # ensure viable minimum is set for OVN's Geneve
+        ml2_config.cfg.CONF.set_override('max_header_size', 38,
+                                         group='ml2_type_geneve')
         ovn_conf.cfg.CONF.set_override('dns_servers',
                                        ['10.10.10.10'],
                                        group='ovn')

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

@@ -760,13 +760,14 @@ class TestNBDbResources(base.TestOVNFunctionalBase):
         # is not configured as domain_name.
         # Parameter taken from configuration
         # should be set instead.
+        mtu = str(1480 - cfg.CONF.ml2_type_geneve.max_header_size)
         expected_dhcp_options_rows = {
             'cidr': '10.0.0.0/24',
             'external_ids': {'subnet_id': subnet['id']},
             'options': {'dns_server': '{10.10.10.10}',
                         'domain_name': '"%s"' % cfg.CONF.dns_domain,
                         'lease_time': '43200',
-                        'mtu': '1450',
+                        'mtu': mtu,
                         'router': '10.0.0.1',
                         'server_id': '10.0.0.1',
                         'server_mac': dhcp_mac}}
@@ -781,13 +782,14 @@ class TestNBDbResources(base.TestOVNFunctionalBase):
         p = self._make_port(self.fmt, n1['network']['id'],
                             fixed_ips=[{'subnet_id': subnet['id']}])
         dhcp_mac = self._get_subnet_dhcp_mac(subnet)
+        mtu = str(1480 - cfg.CONF.ml2_type_geneve.max_header_size)
         # Make sure that domain_name is not included.
         expected_dhcp_options_rows = {
             'cidr': '10.0.0.0/24',
             'external_ids': {'subnet_id': subnet['id']},
             'options': {'dns_server': '{10.10.10.10}',
                         'lease_time': '43200',
-                        'mtu': '1450',
+                        'mtu': mtu,
                         'router': '10.0.0.1',
                         'server_id': '10.0.0.1',
                         'server_mac': dhcp_mac}}

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

@@ -448,7 +448,7 @@ class TestRouter(base.TestOVNFunctionalBase):
                 'address_mode': ovn_utils.get_ovn_ipv6_address_mode(
                     address_mode),
                 'send_periodic': 'true',
-                'mtu': '1450'}
+                'mtu': '1442'}
         else:
             expected_ra_configs = {}
         self._validate_router_ipv6_ra_configs(lrp_name, expected_ra_configs)

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

@@ -92,6 +92,9 @@ class TestOVNMechanismDriverBase(MechDriverSetupBase,
         cfg.CONF.set_override('vni_ranges',
                               ['1:65536'],
                               group='ml2_type_geneve')
+        # ensure viable minimum is set for OVN's Geneve
+        cfg.CONF.set_override('max_header_size', 38,
+                              group='ml2_type_geneve')
         ovn_conf.cfg.CONF.set_override('ovn_metadata_enabled', False,
                                        group='ovn')
         ovn_conf.cfg.CONF.set_override('dns_servers', ['8.8.8.8'],
@@ -2010,6 +2013,9 @@ class OVNMechanismDriverTestCase(MechDriverSetupBase,
         cfg.CONF.set_override('vni_ranges',
                               ['1:65536'],
                               group='ml2_type_geneve')
+        # ensure viable minimum is set for OVN's Geneve
+        cfg.CONF.set_override('max_header_size', 38,
+                              group='ml2_type_geneve')
         ovn_conf.cfg.CONF.set_override('dns_servers', ['8.8.8.8'], group='ovn')
         mock.patch.object(impl_idl_ovn.Backend, 'schema_helper').start()
         super(OVNMechanismDriverTestCase, self).setUp()
@@ -2155,6 +2161,8 @@ class TestOVNMechanismDriverSegment(MechDriverSetupBase,
     _mechanism_drivers = ['logger', 'ovn']
 
     def setUp(self):
+        cfg.CONF.set_override('max_header_size', 38,
+                              group='ml2_type_geneve')
         mock.patch.object(impl_idl_ovn.Backend, 'schema_helper').start()
         super(TestOVNMechanismDriverSegment, self).setUp()
         p = mock.patch.object(ovn_utils, 'get_revision_number', return_value=1)
@@ -2175,11 +2183,12 @@ class TestOVNMechanismDriverSegment(MechDriverSetupBase,
             network_id=network['id'], physical_network='phys_net1',
             segmentation_id=200, network_type='vlan')['segment']
 
-        # As geneve networks mtu shouldn't be more than 1450, update it
+        # As geneve networks mtu shouldn't be more than 1442 considering the
-        data = {'network': {'mtu': 1450}}
+        # Geneve max_header_size for OVN must be at least 38), update it
+        data = {'network': {'mtu': 1442}}
         req = self.new_update_request('networks', data, network['id'])
         res = self.deserialize(self.fmt, req.get_response(self.api))
-        self.assertEqual(1450, res['network']['mtu'])
+        self.assertEqual(1442, res['network']['mtu'])
 
         self._test_create_segment(
             network_id=network['id'],
@@ -2949,6 +2958,9 @@ class TestOVNMechanismDriverSecurityGroup(MechDriverSetupBase,
         cfg.CONF.set_override('mechanism_drivers',
                               ['logger', 'ovn'],
                               'ml2')
+        # ensure viable minimum is set for OVN's Geneve
+        cfg.CONF.set_override('max_header_size', 38,
+                              group='ml2_type_geneve')
         cfg.CONF.set_override('dns_servers', ['8.8.8.8'], group='ovn')
         mock.patch.object(impl_idl_ovn.Backend, 'schema_helper').start()
         super(TestOVNMechanismDriverSecurityGroup, self).setUp()
@@ -3258,10 +3270,13 @@ class TestOVNMechanismDriverMetadataPort(MechDriverSetupBase,
 
     def setUp(self):
         mock.patch.object(impl_idl_ovn.Backend, 'schema_helper').start()
+        cfg.CONF.set_override('max_header_size', 38,
+                              group='ml2_type_geneve')
         super(TestOVNMechanismDriverMetadataPort, self).setUp()
         self.nb_ovn = self.mech_driver.nb_ovn
         self.sb_ovn = self.mech_driver.sb_ovn
         self.ctx = context.get_admin_context()
+        # ensure viable minimum is set for OVN's Geneve
         ovn_conf.cfg.CONF.set_override('ovn_metadata_enabled', True,
                                        group='ovn')
         p = mock.patch.object(ovn_utils, 'get_revision_number', return_value=1)

neutron/tests/unit/plugins/ml2/test_ext_portsecurity.py

@@ -35,10 +35,13 @@ class PSExtDriverTestCase(test_plugin.Ml2PluginV2TestCase,
     def test_create_net_port_security_default(self):
         _core_plugin = directory.get_plugin()
         admin_ctx = context.get_admin_context()
+        # Maximum MTU depends on the GENEVE max header size, which is variable.
+        mtu = 1530 - cfg.CONF.ml2_type_geneve.max_header_size
         args = {'network':
                 {'name': 'test',
                  'tenant_id': '',
                  'shared': False,
+                 'mtu': mtu,
                  'admin_state_up': True,
                  'status': 'ACTIVE'}}
         network = None

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

@@ -58,9 +58,12 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
     def setUp(self):
         mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.'
                    'impl_idl_ovn.Backend.schema_helper').start()
+        cfg.CONF.set_override('max_header_size', 38, group='ml2_type_geneve')
         super(TestOVNL3RouterPlugin, self).setUp()
         revision_plugin.RevisionPlugin()
-        network_attrs = {external_net.EXTERNAL: True, 'mtu': 1500}
+        # 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}
         self.fake_network = \
             fake_resources.FakeNetwork.create_one_network(
                 attrs=network_attrs).info()

releasenotes/notes/bug-1868137-5bf3354d016c988b.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Ensures that OVN's mechanism driver does not start when
+    ``[ml2_type_geneve]/max_header_size`` is set below the required 38.
+    `LP#1868137 <https://launchpad.net/bugs/1868137>`__