diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index 5046878cbf8..57aee7d47ec 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -274,6 +274,8 @@ LSP_TYPE_VIRTUAL = 'virtual' LSP_TYPE_EXTERNAL = 'external' LSP_OPTIONS_VIRTUAL_PARENTS_KEY = 'virtual-parents' LSP_OPTIONS_VIRTUAL_IP_KEY = 'virtual-ip' +LSP_OPTIONS_MCAST_FLOOD_REPORTS = 'mcast_flood_reports' +LSP_OPTIONS_MCAST_FLOOD = 'mcast_flood' HA_CHASSIS_GROUP_DEFAULT_NAME = 'default_ha_chassis_group' HA_CHASSIS_GROUP_HIGHEST_PRIORITY = 32767 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 a6d31c34ea7..b6458b2bf2d 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -661,6 +661,34 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): txn.add(cmd) raise periodics.NeverAgain() + # TODO(lucasagomes): Remove this in the Y cycle + # 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_for_mcast_flood_reports(self): + cmds = [] + for port in self._nb_idl.lsp_list().execute(check_error=True): + port_type = port.type.strip() + if port_type in ("vtep", "localport", "router"): + continue + + options = port.options + if ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS in options: + continue + + options.update({ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true'}) + if port_type == ovn_const.LSP_TYPE_LOCALNET: + options.update({ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'true'}) + + cmds.append(self._nb_idl.lsp_set_options(port.name, **options)) + + if cmds: + with self._nb_idl.transaction(check_error=True) as txn: + for cmd in cmds: + txn.add(cmd) + + raise periodics.NeverAgain() + class HashRingHealthCheckPeriodics(object): 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 e08742937a8..4998eba6c00 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 @@ -298,6 +298,14 @@ class OVNClient(object): options.update({'requested-chassis': port.get(portbindings.HOST_ID, '')}) + # TODO(lucasagomes): Enable the mcast_flood_reports by default, + # according to core OVN developers it shouldn't cause any harm + # and will be ignored when mcast_snoop is False. We can revise + # this once https://bugzilla.redhat.com/show_bug.cgi?id=1933990 + # (see comment #3) is fixed in Core OVN. + if port_type not in ('vtep', 'localport', 'router'): + options.update({ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true'}) + device_owner = port.get('device_owner', '') sg_ids = ' '.join(utils.get_lsp_security_groups(port)) return OvnPortInfo(port_type, options, addresses, port_security, @@ -1557,6 +1565,9 @@ 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) + options = {'network_name': physnet, + ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true', + ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'true'} cmd = self._nb_idl.create_lswitch_port( lport_name=utils.ovn_provnet_port_name(segment['id']), lswitch_name=utils.ovn_name(network_id), @@ -1564,7 +1575,7 @@ class OVNClient(object): external_ids={}, type=ovn_const.LSP_TYPE_LOCALNET, tag=tag, - options={'network_name': physnet}) + options=options) self._transaction([cmd], txn=txn) def delete_provnet_port(self, network_id, segment): diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 24014c8e1ec..0422efc6c63 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -716,13 +716,11 @@ class TestProvnetPorts(base.TestOVNFunctionalBase): ovn_localnetport = self._find_port_row_by_name( utils.ovn_provnet_port_name(seg_db[0]['id'])) self.assertEqual(ovn_localnetport.tag, [100]) - self.assertEqual(ovn_localnetport.options, - {'network_name': 'physnet1'}) + self.assertEqual(ovn_localnetport.options['network_name'], 'physnet1') seg_2 = self.create_segment(n1['id'], 'physnet2', '222') ovn_localnetport = self._find_port_row_by_name( utils.ovn_provnet_port_name(seg_2['id'])) - self.assertEqual(ovn_localnetport.options, - {'network_name': 'physnet2'}) + self.assertEqual(ovn_localnetport.options['network_name'], 'physnet2') self.assertEqual(ovn_localnetport.tag, [222]) # Delete segments and ensure that localnet 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 afd36b9a086..03121f51c60 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 @@ -395,3 +395,40 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, priority=constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY - 1) ] nb_idl.ha_chassis_group_add_chassis.assert_has_calls(expected_calls) + + def test_check_for_mcast_flood_reports(self): + nb_idl = self.fake_ovn_client._nb_idl + lsp0 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'lsp0', + 'options': {'mcast_flood_reports': 'true'}, + 'type': ""}) + lsp1 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'lsp1', 'options': {}, 'type': ""}) + lsp2 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'lsp2', 'options': {}, + 'type': "vtep"}) + lsp3 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'lsp3', 'options': {}, + 'type': "localport"}) + lsp4 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'lsp4', 'options': {}, + 'type': "router"}) + lsp5 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'lsp5', 'options': {}, 'type': 'localnet'}) + + nb_idl.lsp_list.return_value.execute.return_value = [ + lsp0, lsp1, lsp2, lsp3, lsp4, lsp5] + + # Invoke the periodic method, it meant to run only once at startup + # so NeverAgain will be raised at the end + self.assertRaises(periodics.NeverAgain, + self.periodic.check_for_mcast_flood_reports) + + # Assert only lsp1 and lsp5 were called because they are the only + # ones meeting the criteria ("mcast_flood_reports" not yet set, + # and type "" or localnet) + expected_calls = [ + mock.call('lsp1', mcast_flood_reports='true'), + mock.call('lsp5', mcast_flood_reports='true', mcast_flood='true')] + + nb_idl.lsp_set_options.assert_has_calls(expected_calls) 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 27db955b9b1..5c2815453f9 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 @@ -715,7 +715,9 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase): external_ids={}, lport_name=ovn_utils.ovn_provnet_port_name(segments[0]['id']), lswitch_name=ovn_utils.ovn_name(net['id']), - options={'network_name': 'physnet1'}, + options={'network_name': 'physnet1', + ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true', + ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'true'}, tag=2, type='localnet') @@ -2132,7 +2134,9 @@ class TestOVNMechanismDriverSegment(test_segment.HostSegmentMappingTestCase): external_ids={}, lport_name=ovn_utils.ovn_provnet_port_name(new_segment['id']), lswitch_name=ovn_utils.ovn_name(net['id']), - options={'network_name': 'phys_net1'}, + options={'network_name': 'phys_net1', + ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true', + ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'true'}, tag=200, type='localnet') ovn_nb_api.create_lswitch_port.reset_mock() @@ -2144,7 +2148,9 @@ class TestOVNMechanismDriverSegment(test_segment.HostSegmentMappingTestCase): external_ids={}, lport_name=ovn_utils.ovn_provnet_port_name(new_segment['id']), lswitch_name=ovn_utils.ovn_name(net['id']), - options={'network_name': 'phys_net2'}, + options={'network_name': 'phys_net2', + ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true', + ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'true'}, tag=300, type='localnet') segments = segments_db.get_network_segments( diff --git a/releasenotes/notes/ovn-mcast-flood-reports-80fb529120f2af1c.yaml b/releasenotes/notes/ovn-mcast-flood-reports-80fb529120f2af1c.yaml new file mode 100644 index 00000000000..621d2b14bb0 --- /dev/null +++ b/releasenotes/notes/ovn-mcast-flood-reports-80fb529120f2af1c.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes a configuration problem in the OVN driver that prevented + external IGMP queries from reaching the Virtual Machines. See + `bug 1918108 `_ + for details.