Merge "[OVN] Set mcast_flood_reports on LSPs" into stable/ussuri

neutron/common/ovn/constants.py

@@ -275,6 +275,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

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py

@@ -644,6 +644,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):
 

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -294,6 +294,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,
@@ -1549,6 +1557,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),
@@ -1556,7 +1567,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):

neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py

@@ -660,13 +660,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

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)

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py

@@ -671,7 +671,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')
 
@@ -2075,7 +2077,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()
@@ -2087,7 +2091,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(

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 <https://bugs.launchpad.net/neutron/+bug/1918108>`_
+    for details.