From c95292c075841fa46415bd700ee403a94d939842 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elvira=20Garc=C3=ADa?= Date: Thu, 18 Nov 2021 18:08:13 +0100 Subject: [PATCH] [OVN] Fix gateway_mtu option should not always be set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OVN Driver currently fixes gateway_mtu MTU to the provider MTU value without considering if the private networks in the associated router have greater MTU values than the provider. This is unnecesary and adds extra actions for each packet. This patch fixes that, as now gateway_mtu is only set in case the provider MTU is smaller than the private MTU. The changes in create_router_port and delete_router_port were necessary as there could be a use case when the user first sets the gateway router and later adds subnets from networks with greater MTU, so this parameter needs to be checked after adding a subnet. Closes-Bug: #1951559 Signed-off-by: Elvira García Change-Id: If56f1a3dcdc8c57303d5641df79ea919ba7c170d (cherry picked from commit 0725533a6fa6a42d361e518b027f69da8e1e1ec5) --- .../ovn/mech_driver/ovsdb/ovn_client.py | 26 ++++++++++++++++--- .../ovn/mech_driver/test_mech_driver.py | 8 +++++- .../tests/unit/services/ovn_l3/test_plugin.py | 18 +++++++++---- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 100c15cf076..209d56e2fe5 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -1368,8 +1368,9 @@ class OVNClient(object): def _gen_router_port_options(self, port, network=None): options = {} + admin_context = n_context.get_admin_context() if network is None: - network = self._plugin.get_network(n_context.get_admin_context(), + network = self._plugin.get_network(admin_context, port['network_id']) # For VLAN type networks we need to set the # "reside-on-redirect-chassis" option so the routing for this @@ -1383,9 +1384,15 @@ class OVNClient(object): is_gw_port = const.DEVICE_OWNER_ROUTER_GW == port.get( 'device_owner') if is_gw_port and ovn_conf.is_ovn_emit_need_to_frag_enabled(): - options[ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION] = str( - network['mtu']) - + network_ids = set([port['network_id'] for port in + self._get_router_ports(admin_context, + port['device_id'])]) + for net in self._plugin.get_networks(admin_context, + filters={'id': network_ids}): + if net['mtu'] > network['mtu']: + options[ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION] = str( + network['mtu']) + break return options def _create_lrouter_port(self, context, router, port, txn=None): @@ -1457,6 +1464,11 @@ class OVNClient(object): if subnet['ip_version'] == 4: cidr = subnet['cidr'] + if ovn_conf.is_ovn_emit_need_to_frag_enabled(): + provider_net = self._plugin.get_network(context, + router[l3.EXTERNAL_GW_INFO]['network_id']) + self.set_gateway_mtu(context, provider_net) + if utils.is_snat_enabled(router) and cidr: self.update_nat_rules(router, networks=[cidr], enable_snat=True, txn=txn) @@ -1557,6 +1569,12 @@ class OVNClient(object): elif port: subnet_ids = utils.get_port_subnet_ids(port) + if (ovn_conf.is_ovn_emit_need_to_frag_enabled() and + router.get('gw_port_id')): + provider_net = self._plugin.get_network(context, + router[l3.EXTERNAL_GW_INFO]['network_id']) + self.set_gateway_mtu(context, provider_net, txn=txn) + cidr = None for sid in subnet_ids: try: 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 db5bdfd437f..8d4939a8440 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 @@ -1842,7 +1842,9 @@ 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): + @mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.' + 'ovn_client.OVNClient._get_router_ports') + def _test_update_network_fragmentation(self, new_mtu, expected_opts, grps): network_attrs = {external_net.EXTERNAL: True} network = self._make_network( self.fmt, 'net1', True, arg_list=(external_net.EXTERNAL,), @@ -1851,6 +1853,10 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase): with self.subnet(network=network) as subnet: with self.port(subnet=subnet, device_owner=const.DEVICE_OWNER_ROUTER_GW) as port: + + grps.return_value = [{'port_id': port['port']['id'], + 'network_id':network['network']['id']}] + # Let's update the MTU to something different network['network']['mtu'] = new_mtu fake_ctx = mock.MagicMock(current=network['network']) diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index a759f5d7f2f..4e621bde011 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -1483,20 +1483,28 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): self.nb_idl().get_unhosted_gateways.assert_called_once_with( {'foo-1': 'physnet1'}, mock.ANY, mock.ANY) - @mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.get_network') + @mock.patch('neutron.plugins.ml2.plugin.Ml2Plugin.get_network') + @mock.patch('neutron.plugins.ml2.plugin.Ml2Plugin.get_networks') @mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.' 'ovn_client.OVNClient._get_router_ports') @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.add_router_interface') - def test_add_router_interface_need_to_frag_enabled(self, ari, grps, gn): + def test_add_router_interface_need_to_frag_enabled( + self, ari, grps, gns, 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'} + interface_info = {'port_id': 'router-port-id', + 'network_id': 'priv-net'} ari.return_value = self.fake_router_interface_info + grps.return_value = [interface_info] self.get_router.return_value = self.fake_router_with_ext_gw - gn.return_value = self.fake_network + network_attrs = {'id': 'prov-net', 'mtu': 1200} + prov_net = fake_resources.FakeNetwork.create_one_network( + attrs=network_attrs).info() self.fake_router_port['device_owner'] = ( constants.DEVICE_OWNER_ROUTER_GW) + gn.return_value = prov_net + gns.return_value = [self.fake_network] self.l3_inst.add_router_interface(self.context, router_id, interface_info) @@ -1506,7 +1514,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): 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'])} + str(prov_net['mtu'])} self.l3_inst._ovn.add_lrouter_port.assert_called_once_with( **fake_router_port_assert)