diff --git a/doc/source/install/ovn/manual_install.rst b/doc/source/install/ovn/manual_install.rst index 9a0ef2ff4fd..29518f8a287 100644 --- a/doc/source/install/ovn/manual_install.rst +++ b/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 diff --git a/neutron/conf/plugins/ml2/drivers/driver_type.py b/neutron/conf/plugins/ml2/drivers/driver_type.py index 3b63f43e30d..0765b2f46a2 100644 --- a/neutron/conf/plugins/ml2/drivers/driver_type.py +++ b/neutron/conf/plugins/ml2/drivers/driver_type.py @@ -41,15 +41,18 @@ geneve_opts = [ default=[], help=_("Comma-separated list of : 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 " - "value is used to calculate the maximum MTU " - "for the driver. " - "The default size for this field is 30, which is the " - "size of the Geneve header without any additional " - "option headers.")), + help=_("The maximum allowed Geneve encapsulation header size " + "(in bytes). " + "Geneve header is extensible, this value is used to " + "calculate the maximum MTU for Geneve-based networks. " + "The default is 30, which is the size of the Geneve " + "header without any additional option headers. " + "Note the default is not enough for OVN which " + "requires at least 38.")), ] vxlan_opts = [ diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index e1e1e7c3ca3..c61aeae5b77 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -66,6 +66,7 @@ import neutron.wsgi LOG = log.getLogger(__name__) +OVN_MIN_GENEVE_MAX_HEADER_SIZE = 38 class OVNPortUpdateError(n_exc.BadRequest): @@ -118,6 +119,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' diff --git a/neutron/tests/functional/base.py b/neutron/tests/functional/base.py index f068778da3f..348148e1a5a 100644 --- a/neutron/tests/functional/base.py +++ b/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') diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_resources.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_resources.py index 21e14d8b24a..2897d32fe0f 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_resources.py +++ b/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}} diff --git a/neutron/tests/functional/services/ovn_l3/test_plugin.py b/neutron/tests/functional/services/ovn_l3/test_plugin.py index de5b1338399..37fd0c329cc 100644 --- a/neutron/tests/functional/services/ovn_l3/test_plugin.py +++ b/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) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 6b959c21f87..e6f1d47c0ea 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -93,6 +93,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'], @@ -2113,6 +2116,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() @@ -2258,6 +2264,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) @@ -2278,11 +2286,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 - data = {'network': {'mtu': 1450}} + # As geneve networks mtu shouldn't be more than 1442 considering the + # 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'], @@ -3052,6 +3061,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() @@ -3363,10 +3375,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) diff --git a/neutron/tests/unit/plugins/ml2/test_ext_portsecurity.py b/neutron/tests/unit/plugins/ml2/test_ext_portsecurity.py index b813d8aebfc..2b38d391f59 100644 --- a/neutron/tests/unit/plugins/ml2/test_ext_portsecurity.py +++ b/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 diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index ee26c9f759d..f3f0aedb7b0 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/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() diff --git a/releasenotes/notes/bug-1868137-5bf3354d016c988b.yaml b/releasenotes/notes/bug-1868137-5bf3354d016c988b.yaml new file mode 100644 index 00000000000..3ffb2850ae1 --- /dev/null +++ b/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 `__