diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py index c24aa2acef6..0a6ba8f774d 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py @@ -290,10 +290,10 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): raise RuntimeError(_("Currently only supports " "delete by lport-name")) - def get_all_stateful_fip_nats(self): + def get_all_stateless_fip_nats(self): cmd = self.db_find('NAT', ('external_ids', '!=', {ovn_const.OVN_FIP_EXT_ID_KEY: ''}), - ('options', '!=', {'stateless': ''}), + ('options', '=', {'stateless': 'true'}), ('type', '=', 'dnat_and_snat') ) return cmd.execute(check_error=True) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index 981ad5b7573..de19ebf57f5 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -278,11 +278,11 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): # The migration will run just once per neutron-server instance. If the lock # is held by some other neutron-server instance in the cloud, we'll attempt # to perform the migration every 10 seconds until completed. - # TODO(ihrachys): Remove the migration to stateless fips at some point. + # TODO(ihrachys): Remove the migration to stateful fips in Z+1. @periodics.periodic(spacing=10, run_immediately=True) @rerun_on_schema_updates - def migrate_to_stateless_fips(self): - """Perform the migration from stateful to stateless Floating IPs. """ + def migrate_to_stateful_fips(self): + """Perform the migration from stateless to stateful Floating IPs. """ # Only the worker holding a valid lock within OVSDB will perform the # migration. if not self.has_lock: @@ -292,7 +292,7 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): nb_sync = ovn_db_sync.OvnNbSynchronizer( self._ovn_client._plugin, self._nb_idl, self._ovn_client._sb_idl, None, None) - nb_sync.migrate_to_stateless_fips(admin_context) + nb_sync.migrate_to_stateful_fips(admin_context) raise periodics.NeverAgain() # The migration will run just once per neutron-server instance. If the lock 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 470079b4701..0cc085a1feb 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 @@ -821,8 +821,7 @@ class OVNClient(object): 'logical_ip': floatingip['fixed_ip_address'], 'external_ip': floatingip['floating_ip_address'], 'logical_port': floatingip['port_id'], - 'external_ids': ext_ids, - 'options': {'stateless': 'true'}} + 'external_ids': ext_ids} if ovn_conf.is_ovn_distributed_floating_ip(): if self._nb_idl.lsp_get_up(floatingip['port_id']).execute(): diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py index f100864a8d0..e0461cfce43 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py @@ -106,7 +106,7 @@ class OvnNbSynchronizer(OvnDbSynchronizer): self.sync_port_dns_records(ctx) self.sync_acls(ctx) self.sync_routers_and_rports(ctx) - self.migrate_to_stateless_fips(ctx) + self.migrate_to_stateful_fips(ctx) self.sync_port_qos_policies(ctx) self.sync_fip_qos_policies(ctx) @@ -1190,14 +1190,13 @@ class OvnNbSynchronizer(OvnDbSynchronizer): txn.add(self.ovn_api.pg_add_ports( utils.ovn_port_group_name(sg), port['id'])) - def migrate_to_stateless_fips(self, ctx): - # This routine will set options:stateless=true for all dnat_and_snats - # that belong to neutron fips. + def migrate_to_stateful_fips(self, ctx): + # This routine will clear options:stateless=true for all dnat_and_snats + # that belong to neutron fips. Since we don't set any other options, + # just clear the whole column. with self.ovn_api.transaction(check_error=True) as txn: - for nat in self.ovn_api.get_all_stateful_fip_nats(): - txn.add(self.ovn_api.db_set( - 'NAT', nat['_uuid'], - ('options', {'stateless': 'true'}))) + for nat in self.ovn_api.get_all_stateless_fip_nats(): + txn.add(self.ovn_api.db_clear('NAT', nat['_uuid'], 'options')) def migrate_to_port_groups(self, ctx): # This routine is responsible for migrating the current Security diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py index f23065242d8..5aa08d306a8 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py @@ -1749,6 +1749,62 @@ class TestOvnNbSync(base.TestOVNFunctionalBase): nb_synchronizer.sync_fip_qos_policies(ctx) self._validate_qos_records() + def test_fip_nat_revert_to_stateful(self): + res = self._create_network(self.fmt, 'n1_ext', True, + arg_list=('router:external', ), + **{'router:external': True}) + net_ext = self.deserialize(self.fmt, res)['network'] + res = self._create_subnet(self.fmt, net_ext['id'], '10.0.0.0/24') + subnet_ext = self.deserialize(self.fmt, res)['subnet'] + + res = self._create_network(self.fmt, 'n1_int', True) + net_int = self.deserialize(self.fmt, res)['network'] + self._create_subnet(self.fmt, net_int['id'], '10.10.0.0/24') + + port = self._make_port(self.fmt, net_int['id'], + name='test-port')['port'] + + data = {'name': 'r1', 'admin_state_up': True, + 'tenant_id': self._tenant_id, + 'external_gateway_info': { + 'enable_snat': True, + 'network_id': net_ext['id'], + 'external_fixed_ips': [{'ip_address': '10.0.0.5', + 'subnet_id': subnet_ext['id']}]} + } + router = self.l3_plugin.create_router(self.context, {'router': data}) + self.l3_plugin.add_router_interface( + self.context, router['id'], {'port_id': port['id']}) + + body = {'tenant_id': self._tenant_id, + 'floating_network_id': net_ext['id'], + 'port_id': port['id']} + self.l3_plugin.create_floatingip(self.context, {'floatingip': body}) + + self.assertEqual(0, len(self.nb_api.get_all_stateless_fip_nats())) + + def get_all_stateful_fip_nats(): + cmd = self.nb_api.db_find('NAT', + ('external_ids', '!=', {ovn_const.OVN_FIP_EXT_ID_KEY: ''}), + ('options', '=', {}), + ('type', '=', 'dnat_and_snat')) + return cmd.execute(check_error=True) + + with self.nb_api.transaction(check_error=True) as txn: + for nat in get_all_stateful_fip_nats(): + txn.add(self.nb_api.db_set( + 'NAT', nat['_uuid'], + ('options', {'stateless': 'true'}))) + + self.assertEqual(1, len(self.nb_api.get_all_stateless_fip_nats())) + + nb_synchronizer = ovn_db_sync.OvnNbSynchronizer( + self.plugin, self.mech_driver.nb_ovn, self.mech_driver.sb_ovn, + 'repair', self.mech_driver) + nb_synchronizer.migrate_to_stateful_fips(self.context) + + self.assertEqual(0, len(self.nb_api.get_all_stateless_fip_nats())) + class TestOvnSbSync(base.TestOVNFunctionalBase): diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 54858259686..508b5c1880f 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -141,34 +141,34 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, migration_expected=False, never_again=False) - def _test_migrate_to_stateless_fips_helper( + def _test_migrate_to_stateful_fips_helper( self, migration_expected, never_again): with mock.patch.object(ovn_db_sync.OvnNbSynchronizer, - 'migrate_to_stateless_fips') as mtsf: + 'migrate_to_stateful_fips') as mtsf: if never_again: self.assertRaises(periodics.NeverAgain, - self.periodic.migrate_to_stateless_fips) + self.periodic.migrate_to_stateful_fips) else: - self.periodic.migrate_to_stateless_fips() + self.periodic.migrate_to_stateful_fips() if migration_expected: mtsf.assert_called_once_with(mock.ANY) else: mtsf.assert_not_called() - def test_migrate_to_stateless_fips(self): + def test_migrate_to_stateful_fips(self): # Check normal migration path: if the migration has to be done, it will # take place and won't be attempted in the future. - self._test_migrate_to_stateless_fips_helper(migration_expected=True, - never_again=True) + self._test_migrate_to_stateful_fips_helper(migration_expected=True, + never_again=True) - def test_migrate_to_stateless_fips_no_lock(self): + def test_migrate_to_stateful_fips_no_lock(self): with mock.patch.object(maintenance.DBInconsistenciesPeriodics, 'has_lock', mock.PropertyMock( return_value=False)): # Check that if this worker doesn't have the lock, it won't # perform the migration and it will try again later. - self._test_migrate_to_stateless_fips_helper( + self._test_migrate_to_stateful_fips_helper( migration_expected=False, never_again=False) def _test_fix_create_update_network(self, ovn_rev, neutron_rev): diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py index a939aab3a94..a2a586c99cd 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py @@ -453,8 +453,8 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase): ovn_api.get_all_logical_switches_with_ports.return_value = ( self.lswitches_with_ports) - ovn_api.get_all_stateful_fip_nats = mock.Mock() - ovn_api.get_all_stateful_fip_nats.return_value = [] + ovn_api.get_all_stateless_fip_nats = mock.Mock() + ovn_api.get_all_stateless_fip_nats.return_value = [] ovn_api.get_all_logical_routers_with_rports = mock.Mock() ovn_api.get_all_logical_routers_with_rports.return_value = ( self.lrouters_with_rports) diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index e6ad2b35dc9..11c233627e7 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -168,7 +168,6 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): self.fake_floating_ip['port_id'], ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: utils.ovn_name( self.fake_floating_ip['router_id'])}, - 'options': {'stateless': 'true'} })) self.l3_inst = directory.get_plugin(plugin_constants.L3) self.lb_id = uuidutils.generate_uuid() @@ -958,8 +957,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): logical_ip='10.0.0.10', external_ip='192.168.0.10', logical_port='port_id', - external_ids=expected_ext_ids, - options={'stateless': 'true'}) + external_ids=expected_ext_ids) def test_create_floatingip_distributed(self): self.l3_inst._nb_ovn.is_col_present.return_value = True @@ -983,8 +981,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): 'neutron-router-id', type='dnat_and_snat', logical_ip='10.0.0.10', external_ip='192.168.0.10', external_mac='00:01:02:03:04:05', logical_port='port_id', - external_ids=expected_ext_ids, - options={'stateless': 'true'}) + external_ids=expected_ext_ids) def test_create_floatingip_distributed_logical_port_down(self): # Check that when the port is down, the external_mac field is not @@ -1012,8 +1009,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): 'neutron-router-id', type='dnat_and_snat', logical_ip='10.0.0.10', external_ip='192.168.0.10', logical_port='port_id', - external_ids=expected_ext_ids, - options={'stateless': 'true'}) + external_ids=expected_ext_ids) def test_create_floatingip_external_ip_present_in_nat_rule(self): self.l3_inst._nb_ovn.is_col_present.return_value = True @@ -1038,8 +1034,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): logical_ip='10.0.0.10', external_ip='192.168.0.10', logical_port='port_id', - external_ids=expected_ext_ids, - options={'stateless': 'true'}) + external_ids=expected_ext_ids) def test_create_floatingip_external_ip_present_type_snat(self): self.l3_inst._nb_ovn.is_col_present.return_value = True @@ -1065,8 +1060,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): logical_ip='10.0.0.10', external_ip='192.168.0.10', logical_port='port_id', - external_ids=expected_ext_ids, - options={'stateless': 'true'}) + external_ids=expected_ext_ids) def test_create_floatingip_lsp_external_id(self): foo_lport = fake_resources.FakeOvsdbRow.create_one_ovsdb_row() @@ -1104,8 +1098,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): external_ip='192.168.0.10', logical_ip='10.0.0.10', type='dnat_and_snat', - external_ids=expected_ext_ids, - options={'stateless': 'true'}) + external_ids=expected_ext_ids) def test_create_floatingip_lb_vip_fip(self): config.cfg.CONF.set_override( @@ -1135,8 +1128,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): logical_ip='10.0.0.10', logical_port='port_id', type='dnat_and_snat', - external_ids=expected_ext_ids, - options={'stateless': 'true'}) + external_ids=expected_ext_ids) self.l3_inst._nb_ovn.db_find_rows.assert_called_with( 'NAT', ('external_ids', '=', {ovn_const.OVN_FIP_PORT_EXT_ID_KEY: self.member_lsp.name})) @@ -1246,8 +1238,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): logical_ip='10.10.10.10', external_ip='192.168.0.10', logical_port='new-port_id', - external_ids=expected_ext_ids, - options={'stateless': 'true'}) + external_ids=expected_ext_ids) @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.' 'update_floatingip') @@ -1273,8 +1264,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): logical_ip='10.10.10.10', external_ip='192.168.0.10', logical_port='new-port_id', - external_ids=expected_ext_ids, - options={'stateless': 'true'}) + external_ids=expected_ext_ids) @mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.get_network') @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.' @@ -1308,7 +1298,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): 'neutron-new-router-id', type='dnat_and_snat', logical_ip='10.10.10.10', external_ip='192.168.0.10', external_mac='00:01:02:03:04:05', logical_port='new-port_id', - external_ids=expected_ext_ids, options={'stateless': 'true'}) + external_ids=expected_ext_ids) @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.' 'update_floatingip') @@ -1342,8 +1332,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): logical_ip='10.10.10.10', external_ip='192.168.0.10', logical_port='foo', - external_ids=expected_ext_ids, - options={'stateless': 'true'}) + external_ids=expected_ext_ids) @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.' 'update_floatingip') @@ -1379,8 +1368,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): logical_ip='10.10.10.10', external_ip='192.168.0.10', logical_port='port_id', - external_ids=expected_ext_ids, - options={'stateless': 'true'}) + external_ids=expected_ext_ids) @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.get_floatingips') def test_disassociate_floatingips(self, gfs): diff --git a/releasenotes/notes/revert-ovn-stateless-nat-88076892fd6f7354.yaml b/releasenotes/notes/revert-ovn-stateless-nat-88076892fd6f7354.yaml new file mode 100644 index 00000000000..4f59d70b193 --- /dev/null +++ b/releasenotes/notes/revert-ovn-stateless-nat-88076892fd6f7354.yaml @@ -0,0 +1,8 @@ +--- +other: + - | + OVN driver reverted to using stateful NAT for floating IP implementation. + The previous switch to stateless didn't materialize the expected + performance benefits and instead introduced problems with potential + hardware offloading. +