From dc677682caddd2e9deff35d024f1c56064c6d068 Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Thu, 23 Sep 2021 02:01:17 +0000 Subject: [PATCH] ovn: use stateless NAT rules for FIPs Using stateless NAT in OVN should always be a better choice for FIPs because it allows to avoid hitting conntrack, potentially improving NAT performance, esp. where hardware offload for the openflow rules is involved. The only limitation for using stateless NAT in OVN is that it requires 1:1 IP mapping; which is always the case for FIPs. This is why this patch unconditionally switches to stateless for all FIPs. Before setting stateless key to NAT's options, check that 'options' are supported. (Support was added in OVN 20.03 as part of stateless NAT implementation.) If an older OVN version is used, nothing changes. The patch also adds a runtime migration rule for neutron-server to transform all existing stateful fips to stateless. Change-Id: I312a950131d62d93fb4bc121bc5e60febb8d35ee --- .../ovn/mech_driver/ovsdb/impl_idl_ovn.py | 8 ++++ .../ovn/mech_driver/ovsdb/maintenance.py | 23 +++++++++++ .../ovn/mech_driver/ovsdb/ovn_client.py | 6 +++ .../ovn/mech_driver/ovsdb/ovn_db_sync.py | 10 +++++ .../ovn/mech_driver/ovsdb/test_maintenance.py | 39 ++++++++++++++++++ .../ovn/mech_driver/ovsdb/test_ovn_db_sync.py | 2 + .../tests/unit/services/ovn_l3/test_plugin.py | 41 +++++++++++++------ ...ateless-nat-for-fips-e764c4ece4024be1.yaml | 6 +++ 8 files changed, 122 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/ovn-use-stateless-nat-for-fips-e764c4ece4024be1.yaml 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 24ef98583c5..2a19e26bb6c 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 @@ -293,6 +293,14 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): raise RuntimeError(_("Currently only supports " "delete by lport-name")) + def get_all_stateful_fip_nats(self): + cmd = self.db_find('NAT', + ('external_ids', '!=', {ovn_const.OVN_FIP_EXT_ID_KEY: ''}), + ('options', '!=', {'stateless': ''}), + ('type', '=', 'dnat_and_snat') + ) + return cmd.execute(check_error=True) + def get_all_logical_switches_with_ports(self): result = [] for lswitch in self._tables['Logical_Switch'].rows.values(): 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 a8cdafd740e..4faa54b7a07 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -274,6 +274,29 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): else: self._ovn_client.update_subnet(context, sn_db_obj, n_db_obj) + # 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. + @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. """ + if not self._ovn_client.is_stateless_nat_supported(): + raise periodics.NeverAgain() + + # Only the worker holding a valid lock within OVSDB will perform the + # migration. + if not self.has_lock: + return + + admin_context = n_context.get_admin_context() + 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) + raise periodics.NeverAgain() + # 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. 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 4d46afb30a5..e7ab1e8ef01 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 @@ -113,6 +113,10 @@ class OVNClient(object): return self._nb_idl.is_col_supports_value('ACL', 'action', 'allow-stateless') + # TODO(ihrachys) remove when min OVN version >= 20.03 + def is_stateless_nat_supported(self): + return self._nb_idl.is_col_present('NAT', 'options') + def _get_allowed_addresses_from_port(self, port): if not port.get(psec.PORTSECURITY): return [], [] @@ -734,6 +738,8 @@ class OVNClient(object): 'external_ip': floatingip['floating_ip_address'], 'logical_port': floatingip['port_id'], 'external_ids': ext_ids} + if self.is_stateless_nat_supported(): + columns['options'] = {'stateless': 'true'} 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 090696ba31a..9e86474e268 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 @@ -103,6 +103,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) def _create_port_in_ovn(self, ctx, port): # Remove any old ACLs for the port to avoid creating duplicate ACLs. @@ -1188,6 +1189,15 @@ 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. + 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'}))) + def migrate_to_port_groups(self, ctx): # This routine is responsible for migrating the current Security # Groups and SG Rules to the new Port Groups implementation. 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 9af35a0461b..1f9ca246cb7 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 @@ -139,6 +139,45 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, migration_expected=False, never_again=False) + def _test_migrate_to_stateless_fips_helper( + self, stateless_supported, migration_expected, never_again): + self.fake_ovn_client.is_stateless_nat_supported.return_value = ( + stateless_supported) + with mock.patch.object(ovn_db_sync.OvnNbSynchronizer, + 'migrate_to_stateless_fips') as mtsf: + if never_again: + self.assertRaises(periodics.NeverAgain, + self.periodic.migrate_to_stateless_fips) + else: + self.periodic.migrate_to_stateless_fips() + + if migration_expected: + mtsf.assert_called_once_with(mock.ANY) + else: + mtsf.assert_not_called() + + def test_migrate_to_stateless_fips_not_needed(self): + self._test_migrate_to_stateless_fips_helper( + stateless_supported=False, migration_expected=False, + never_again=True) + + def test_migrate_to_stateless_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(stateless_supported=True, + migration_expected=True, + never_again=True) + + def test_migrate_to_stateless_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( + stateless_supported=True, migration_expected=False, + never_again=False) + def _test_fix_create_update_network(self, ovn_rev, neutron_rev): with db_api.CONTEXT_WRITER.using(self.ctx): self.net['revision_number'] = 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 3532b7cf167..a939aab3a94 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,6 +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_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 f3f0aedb7b0..e666bf324e2 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -167,7 +167,9 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): ovn_const.OVN_FIP_PORT_EXT_ID_KEY: self.fake_floating_ip['port_id'], ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: utils.ovn_name( - self.fake_floating_ip['router_id'])}})) + self.fake_floating_ip['router_id'])}, + 'options': {'stateless': 'true'} + })) self.l3_inst = directory.get_plugin(plugin_constants.L3) self.lb_id = uuidutils.generate_uuid() self.member_subnet = {'id': 'subnet-id', @@ -941,7 +943,8 @@ 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) + external_ids=expected_ext_ids, + options={'stateless': 'true'}) def test_create_floatingip_distributed(self): self.l3_inst._ovn.is_col_present.return_value = True @@ -965,7 +968,8 @@ 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) + external_ids=expected_ext_ids, + options={'stateless': 'true'}) def test_create_floatingip_distributed_logical_port_down(self): # Check that when the port is down, the external_mac field is not @@ -993,7 +997,8 @@ 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) + external_ids=expected_ext_ids, + options={'stateless': 'true'}) def test_create_floatingip_external_ip_present_in_nat_rule(self): self.l3_inst._ovn.is_col_present.return_value = True @@ -1018,7 +1023,8 @@ 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) + external_ids=expected_ext_ids, + options={'stateless': 'true'}) def test_create_floatingip_external_ip_present_type_snat(self): self.l3_inst._ovn.is_col_present.return_value = True @@ -1044,7 +1050,8 @@ 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) + external_ids=expected_ext_ids, + options={'stateless': 'true'}) def test_create_floatingip_lsp_external_id(self): foo_lport = fake_resources.FakeOvsdbRow.create_one_ovsdb_row() @@ -1064,6 +1071,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): # Stop this mock. self.mock_is_lb_member_fip.stop() self.get_port.return_value = self.member_port + self.l3_inst._ovn.is_col_present.return_value = True self.l3_inst._ovn.lookup.return_value = self.lb_network self.l3_inst._ovn.get_lswitch_port.return_value = self.member_lsp fip = self.l3_inst.create_floatingip(self.context, 'floatingip') @@ -1081,12 +1089,14 @@ 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) + external_ids=expected_ext_ids, + options={'stateless': 'true'}) def test_create_floatingip_lb_vip_fip(self): config.cfg.CONF.set_override( 'enable_distributed_floating_ip', True, group='ovn') self.get_subnet.return_value = self.member_subnet + self.l3_inst._ovn.is_col_present.return_value = True self.l3_inst._ovn.get_lswitch_port.return_value = self.lb_vip_lsp self.l3_inst._ovn.db_find_rows.return_value.execute.side_effect = [ [self.ovn_lb], @@ -1110,7 +1120,8 @@ 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) + external_ids=expected_ext_ids, + options={'stateless': 'true'}) self.l3_inst._ovn.db_find_rows.assert_called_with( 'NAT', ('external_ids', '=', {ovn_const.OVN_FIP_PORT_EXT_ID_KEY: self.member_lsp.name})) @@ -1217,7 +1228,8 @@ 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) + external_ids=expected_ext_ids, + options={'stateless': 'true'}) @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.' 'update_floatingip') @@ -1243,7 +1255,8 @@ 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) + external_ids=expected_ext_ids, + options={'stateless': 'true'}) @mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.get_network') @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.' @@ -1277,7 +1290,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) + external_ids=expected_ext_ids, options={'stateless': 'true'}) @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.' 'update_floatingip') @@ -1310,7 +1323,8 @@ 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) + external_ids=expected_ext_ids, + options={'stateless': 'true'}) @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.' 'update_floatingip') @@ -1345,7 +1359,8 @@ 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) + external_ids=expected_ext_ids, + options={'stateless': 'true'}) @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.get_floatingips') def test_disassociate_floatingips(self, gfs): diff --git a/releasenotes/notes/ovn-use-stateless-nat-for-fips-e764c4ece4024be1.yaml b/releasenotes/notes/ovn-use-stateless-nat-for-fips-e764c4ece4024be1.yaml new file mode 100644 index 00000000000..fa37a20f7ae --- /dev/null +++ b/releasenotes/notes/ovn-use-stateless-nat-for-fips-e764c4ece4024be1.yaml @@ -0,0 +1,6 @@ +--- +other: + - | + OVN driver now uses stateless NAT for floating IP implementation. This allows + to avoid hitting conntrack, potentially improving performance and also allowing + to offload NAT rules to hardware.