From 7dfbdf65a71b7da2865d475cd91988728f734652 Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Fri, 17 Mar 2023 15:59:05 +0100 Subject: [PATCH] Add support for localnet_learn_fdb OVN option In OVN 22.09, the option "localnet_learn_fdb" was added so that localnet ports can learn MAC addresses and store them in the FDB table. This avoids flooding issues for VMs on provider networks when port security is disabled Closes-Bug: #2012069 Change-Id: I93574b4fe9a79b649bfe755cf7e0697ccc7eb83a --- neutron/cmd/sanity/checks.py | 14 +++ neutron/cmd/sanity_check.py | 12 +++ neutron/common/ovn/constants.py | 1 + .../conf/plugins/ml2/drivers/ovn/ovn_conf.py | 11 +++ .../ovn/mech_driver/ovsdb/maintenance.py | 30 ++++++ .../ovn/mech_driver/ovsdb/ovn_client.py | 5 +- .../ovn/mech_driver/ovsdb/test_maintenance.py | 96 +++++++++++++++++++ .../ovn/mech_driver/test_mech_driver.py | 9 +- .../localnet-learn-fdb-22469280b49701fc.yaml | 23 +++++ 9 files changed, 197 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/localnet-learn-fdb-22469280b49701fc.yaml diff --git a/neutron/cmd/sanity/checks.py b/neutron/cmd/sanity/checks.py index b111c1c2f4e..2a49363eb65 100644 --- a/neutron/cmd/sanity/checks.py +++ b/neutron/cmd/sanity/checks.py @@ -53,6 +53,7 @@ OVN_NB_DB_SCHEMA_GATEWAY_CHASSIS = '5.7' OVN_NB_DB_SCHEMA_PORT_GROUP = '5.11' OVN_NB_DB_SCHEMA_STATELESS_NAT = '5.17' OVN_SB_DB_SCHEMA_VIRTUAL_PORT = '2.5' +OVN_LOCALNET_LEARN_FDB = '22.09' class OVNCheckType(enum.Enum): @@ -657,3 +658,16 @@ def ovn_nb_db_schema_gateway_chassis_supported(): 'Exception: %s', e) return False return True + + +def ovn_localnet_learn_fdb_support(): + try: + ver = _get_ovn_version(OVNCheckType.nb_version) + minver = versionutils.convert_version_to_tuple(OVN_LOCALNET_LEARN_FDB) + if ver < minver: + return False + except (OSError, RuntimeError, ValueError) as e: + LOG.debug('Exception while checking OVN version. ' + 'Exception: %s', e) + return False + return True diff --git a/neutron/cmd/sanity_check.py b/neutron/cmd/sanity_check.py index 903b49db0d3..8bd4318305b 100644 --- a/neutron/cmd/sanity_check.py +++ b/neutron/cmd/sanity_check.py @@ -347,6 +347,14 @@ def check_ovn_nb_db_schema_gateway_chassis(): return result +def check_ovn_localnet_learn_fdb_support(): + result = checks.ovn_localnet_learn_fdb_support() + if not result: + LOG.warning('OVN does not support localnet_learn_fdb option. ' + 'This support was added in OVN 22.09.') + return result + + # Define CLI opts to test specific features, with a callback for the test OPTS = [ BoolOptCallback('ovs_vxlan', check_ovs_vxlan, default=False, @@ -431,6 +439,10 @@ OPTS = [ check_ovn_nb_db_schema_gateway_chassis, help=_('Check OVN NB DB schema support Gateway_Chassis'), default=False), + BoolOptCallback('ovn_localnet_learn_fdb_support', + check_ovn_localnet_learn_fdb_support, + help=_('Check OVN supports localnet_learn_fdb option'), + default=False), ] diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index 2a4afbf4a92..fa1a59dc32a 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -391,6 +391,7 @@ LSP_OPTIONS_REQUESTED_CHASSIS_KEY = 'requested-chassis' LSP_OPTIONS_MCAST_FLOOD_REPORTS = 'mcast_flood_reports' LSP_OPTIONS_MCAST_FLOOD = 'mcast_flood' LSP_OPTIONS_QOS_MIN_RATE = 'qos_min_rate' +LSP_OPTIONS_LOCALNET_LEARN_FDB = 'localnet_learn_fdb' LRP_OPTIONS_RESIDE_REDIR_CH = 'reside-on-redirect-chassis' LRP_OPTIONS_REDIRECT_TYPE = 'redirect-type' diff --git a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py index be75c405a8d..0706477c4ff 100644 --- a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py +++ b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py @@ -217,6 +217,13 @@ ovn_opts = [ 'order to disable the ``stateful-security-group`` API ' 'extension as ``allow-stateless`` keyword is only ' 'supported by OVN >= 21.06.')), + cfg.BoolOpt('localnet_learn_fdb', + default=False, + help=_('If enabled it will allow localnet ports to learn MAC ' + 'addresses and store them in FDB SB table. This avoids ' + 'flooding for traffic towards unknown IPs when port ' + 'security is disabled. It requires OVN 22.09 or ' + 'newer.')), ] @@ -330,3 +337,7 @@ def is_igmp_snooping_enabled(): def is_ovn_dhcp_disabled_for_baremetal(): return cfg.CONF.ovn.disable_ovn_dhcp_for_baremetal_ports + + +def is_learn_fdb_enabled(): + return cfg.CONF.ovn.localnet_learn_fdb 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 29a3848b285..8378103253a 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -650,6 +650,36 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): raise periodics.NeverAgain() + # A static spacing value is used here, but this method will only run + # once per lock due to the use of periodics.NeverAgain(). + @periodics.periodic(spacing=600, run_immediately=True) + def check_localnet_port_has_learn_fdb(self): + if not self.has_lock: + return + + ports = self._nb_idl.db_find_rows( + "Logical_Switch_Port", ("type", "=", ovn_const.LSP_TYPE_LOCALNET) + ).execute(check_error=True) + + with self._nb_idl.transaction(check_error=True) as txn: + for port in ports: + if ovn_conf.is_learn_fdb_enabled(): + fdb_opt = port.options.get( + ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB) + if not fdb_opt or fdb_opt == 'false': + txn.add(self._nb_idl.db_set( + 'Logical_Switch_Port', port.name, + ('options', + {ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: 'true'} + ))) + elif port.options.get( + ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB) == 'true': + txn.add(self._nb_idl.db_set( + 'Logical_Switch_Port', port.name, + ('options', + {ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: 'false'}))) + raise periodics.NeverAgain() + # TODO(lucasagomes): Remove this in the Z cycle # A static spacing value is used here, but this method will only run # once per lock due to the use of periodics.NeverAgain(). 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 f5607e28493..b070a91ca4e 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 @@ -1914,9 +1914,12 @@ class OVNClient(object): def create_provnet_port(self, network_id, segment, txn=None): tag = segment.get(segment_def.SEGMENTATION_ID, []) physnet = segment.get(segment_def.PHYSICAL_NETWORK) + fdb_enabled = ('true' if ovn_conf.is_learn_fdb_enabled() + else 'false') options = {'network_name': physnet, ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true', - ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false'} + ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false', + ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: fdb_enabled} cmd = self._nb_idl.create_lswitch_port( lport_name=utils.ovn_provnet_port_name(segment['id']), lswitch_name=utils.ovn_name(network_id), 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 b22e0a54701..20f41b44d21 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 @@ -586,6 +586,102 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, nb_idl.lsp_set_options.assert_has_calls(expected_calls) + def test_check_localnet_port_has_learn_fdb(self): + cfg.CONF.set_override('localnet_learn_fdb', 'True', + group='ovn') + nb_idl = self.fake_ovn_client._nb_idl + + # Already has the learn fdb option enabled + lsp0 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={ + "name": "lsp0", + "options": { + constants.LSP_OPTIONS_LOCALNET_LEARN_FDB: "true", + }, + } + ) + + # learn fdb option missing, needs update + lsp1 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={ + "name": "lsp1", + "options": {}, + } + ) + + # learn fdb option set to false, needs update + lsp2 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={ + "name": "lsp2", + "options": { + constants.LSP_OPTIONS_LOCALNET_LEARN_FDB: "false", + }, + } + ) + + nb_idl.db_find_rows.return_value.execute.return_value = [ + lsp0, + lsp1, + lsp2, + ] + + self.assertRaises( + periodics.NeverAgain, + self.periodic.check_localnet_port_has_learn_fdb) + + options = {constants.LSP_OPTIONS_LOCALNET_LEARN_FDB: 'true'} + expected_calls = [mock.call('Logical_Switch_Port', 'lsp1', + ('options', options)), + mock.call('Logical_Switch_Port', 'lsp2', + ('options', options))] + nb_idl.db_set.assert_has_calls(expected_calls) + + def test_check_localnet_port_has_learn_fdb_disabled(self): + nb_idl = self.fake_ovn_client._nb_idl + + # learn fdb option enabled, needs update + lsp0 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={ + "name": "lsp0", + "options": { + constants.LSP_OPTIONS_LOCALNET_LEARN_FDB: "true", + }, + } + ) + + # learn fdb option missing, no update needed + lsp1 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={ + "name": "lsp1", + "options": {}, + } + ) + + # learn fdb option set to false, no update needed + lsp2 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={ + "name": "lsp2", + "options": { + constants.LSP_OPTIONS_LOCALNET_LEARN_FDB: "false", + }, + } + ) + + nb_idl.db_find_rows.return_value.execute.return_value = [ + lsp0, + lsp1, + lsp2, + ] + + self.assertRaises( + periodics.NeverAgain, + self.periodic.check_localnet_port_has_learn_fdb) + + options = {constants.LSP_OPTIONS_LOCALNET_LEARN_FDB: 'false'} + expected_calls = [mock.call('Logical_Switch_Port', 'lsp0', + ('options', options))] + nb_idl.db_set.assert_has_calls(expected_calls) + def test_check_router_mac_binding_options(self): nb_idl = self.fake_ovn_client._nb_idl lr0 = fakes.FakeOvsdbRow.create_one_ovsdb_row( 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 f768bf7635a..fb94b262757 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 @@ -877,7 +877,8 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): lswitch_name=ovn_utils.ovn_name(net['id']), options={'network_name': 'physnet1', ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true', - ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false'}, + ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false', + ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: 'false'}, tag=2, type='localnet') @@ -2949,7 +2950,8 @@ class TestOVNMechanismDriverSegment(MechDriverSetupBase, lswitch_name=ovn_utils.ovn_name(net['id']), options={'network_name': 'physnet1', ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true', - ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false'}, + ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false', + ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: 'false'}, tag=200, type='localnet') ovn_nb_api.create_lswitch_port.reset_mock() @@ -2963,7 +2965,8 @@ class TestOVNMechanismDriverSegment(MechDriverSetupBase, lswitch_name=ovn_utils.ovn_name(net['id']), options={'network_name': 'physnet2', ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true', - ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false'}, + ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false', + ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: 'false'}, tag=300, type='localnet') segments = segments_db.get_network_segments( diff --git a/releasenotes/notes/localnet-learn-fdb-22469280b49701fc.yaml b/releasenotes/notes/localnet-learn-fdb-22469280b49701fc.yaml new file mode 100644 index 00000000000..9af9ca48be7 --- /dev/null +++ b/releasenotes/notes/localnet-learn-fdb-22469280b49701fc.yaml @@ -0,0 +1,23 @@ +--- +issues: + - | + In OVN 22.09 the option "localnet_learn_fdb" was added, enabling localnet + ports to learn MAC addresses and store them at the FDB table. + There is no aging mechanism for those MACs (that is the reason for not + having this option enabled by default) and therefore it needs to be used + with care, specially when provider networks are big. It is recommended to + perform periodic manual cleanups of FDB table, to avoid scalability + issues -- until OVN implements an aging mechanism for this, tracked at + https://bugzilla.redhat.com/show_bug.cgi?id=2179942. +fixes: + - | + By default localnet ports don't learn MAC addresses and therefore they are + not stored in the FDB table at OVN SB DB. This leads to flooding issues + when the destination traffic is an unknown IP by OpenStack. In OVN 22.09 + the option "localnet_learn_fdb" was added, enabling those ports to learn + MAC addresses and store them at the FDB table. Note there is no aging + mechanism for those MACs, thus this is not enabled by default and needs + to be used carefully, specially when provider networks are big, and/or + performing manual cleanup of FDB table over time to avoid scalability + issues, until OVN implements it at + https://bugzilla.redhat.com/show_bug.cgi?id=2179942.