Merge "[OVN] Disable the mcast_flood_reports option for LSPs" into stable/yoga
This commit is contained in:
commit
c0de99de90
|
@ -160,6 +160,10 @@ class Backend(ovs_idl.Backend):
|
|||
cls.schema)
|
||||
return cls._schema_helper
|
||||
|
||||
@classmethod
|
||||
def get_schema_version(cls):
|
||||
return cls.schema_helper.schema_json['version']
|
||||
|
||||
@classmethod
|
||||
def schema_has_table(cls, table_name):
|
||||
return table_name in cls.schema_helper.schema_json['tables']
|
||||
|
|
|
@ -725,7 +725,7 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
|
|||
txn.add(cmd)
|
||||
raise periodics.NeverAgain()
|
||||
|
||||
# TODO(lucasagomes): Remove this in the Z cycle
|
||||
# TODO(lucasagomes): Remove this in the B+3 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)
|
||||
|
@ -736,21 +736,36 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
|
|||
cmds = []
|
||||
for port in self._nb_idl.lsp_list().execute(check_error=True):
|
||||
port_type = port.type.strip()
|
||||
if port_type in ("vtep", ovn_const.LSP_TYPE_LOCALPORT, "router"):
|
||||
continue
|
||||
|
||||
options = port.options
|
||||
if port_type == ovn_const.LSP_TYPE_LOCALNET:
|
||||
mcast_flood_value = options.get(
|
||||
mcast_flood_reports_value = options.get(
|
||||
ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS)
|
||||
if mcast_flood_value == 'false':
|
||||
continue
|
||||
options.update({ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false'})
|
||||
elif ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS in options:
|
||||
continue
|
||||
|
||||
options.update({ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true'})
|
||||
cmds.append(self._nb_idl.lsp_set_options(port.name, **options))
|
||||
if self._ovn_client.is_mcast_flood_broken:
|
||||
if port_type in ("vtep", ovn_const.LSP_TYPE_LOCALPORT,
|
||||
"router"):
|
||||
continue
|
||||
|
||||
if port_type == ovn_const.LSP_TYPE_LOCALNET:
|
||||
mcast_flood_value = options.pop(
|
||||
ovn_const.LSP_OPTIONS_MCAST_FLOOD, None)
|
||||
if mcast_flood_value:
|
||||
cmds.append(self._nb_idl.db_remove(
|
||||
'Logical_Switch_Port', port.name, 'options',
|
||||
ovn_const.LSP_OPTIONS_MCAST_FLOOD,
|
||||
if_exists=True))
|
||||
|
||||
if mcast_flood_reports_value == 'true':
|
||||
continue
|
||||
|
||||
options.update(
|
||||
{ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true'})
|
||||
cmds.append(self._nb_idl.lsp_set_options(port.name, **options))
|
||||
|
||||
elif (mcast_flood_reports_value and port_type !=
|
||||
ovn_const.LSP_TYPE_LOCALNET):
|
||||
cmds.append(self._nb_idl.db_remove(
|
||||
'Logical_Switch_Port', port.name, 'options',
|
||||
ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS, if_exists=True))
|
||||
|
||||
if cmds:
|
||||
with self._nb_idl.transaction(check_error=True) as txn:
|
||||
|
|
|
@ -39,6 +39,7 @@ from oslo_config import cfg
|
|||
from oslo_log import log
|
||||
from oslo_utils import excutils
|
||||
from oslo_utils import timeutils
|
||||
from oslo_utils import versionutils
|
||||
from ovsdbapp.backend.ovs_idl import idlutils
|
||||
import tenacity
|
||||
|
||||
|
@ -94,6 +95,7 @@ class OVNClient(object):
|
|||
|
||||
self._plugin_property = None
|
||||
self._l3_plugin_property = None
|
||||
self._is_mcast_flood_broken = None
|
||||
|
||||
# TODO(ralonsoh): handle the OVN client extensions with an ext. manager
|
||||
self._qos_driver = qos_extension.OVNClientQosExtension(driver=self)
|
||||
|
@ -338,6 +340,20 @@ class OVNClient(object):
|
|||
|
||||
self._transaction(cmd)
|
||||
|
||||
# TODO(lucasagomes): Remove this method and the logic around the broken
|
||||
# mcast_flood_reports configuration option on any other port that is not
|
||||
# type "localnet" when the fixed version of OVN becomes the norm.
|
||||
# The commit in core OVN fixing this issue is the
|
||||
# https://github.com/ovn-org/ovn/commit/6aeeccdf272bc60630581e46aa42d97f4f56d4fa
|
||||
@property
|
||||
def is_mcast_flood_broken(self):
|
||||
if self._is_mcast_flood_broken is None:
|
||||
schema_version = self._nb_idl.get_schema_version()
|
||||
self._is_mcast_flood_broken = (
|
||||
versionutils.convert_version_to_tuple(schema_version) <
|
||||
(6, 3, 0))
|
||||
return self._is_mcast_flood_broken
|
||||
|
||||
def _get_port_options(self, port):
|
||||
context = n_context.get_admin_context()
|
||||
binding_prof = utils.validate_and_get_data_from_binding_profile(port)
|
||||
|
@ -500,12 +516,8 @@ class OVNClient(object):
|
|||
if port_type != ovn_const.LSP_TYPE_VIRTUAL:
|
||||
options[ovn_const.LSP_OPTIONS_REQUESTED_CHASSIS_KEY] = chassis
|
||||
|
||||
# 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', ovn_const.LSP_TYPE_LOCALPORT, 'router'):
|
||||
if self.is_mcast_flood_broken and port_type not in (
|
||||
'vtep', ovn_const.LSP_TYPE_LOCALPORT, 'router'):
|
||||
options.update({ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true'})
|
||||
|
||||
device_owner = port.get('device_owner', '')
|
||||
|
|
|
@ -160,6 +160,7 @@ class FakeOvsdbNbOvnIdl(object):
|
|||
self.ha_chassis_group_add_chassis = mock.Mock()
|
||||
self.ha_chassis_group_del_chassis = mock.Mock()
|
||||
self.lrp_get = mock.Mock()
|
||||
self.get_schema_version = mock.Mock(return_value='3.6.0')
|
||||
|
||||
|
||||
class FakeOvsdbSbOvnIdl(object):
|
||||
|
@ -182,6 +183,7 @@ class FakeOvsdbSbOvnIdl(object):
|
|||
self.is_table_present = mock.Mock()
|
||||
self.is_table_present.return_value = False
|
||||
self.get_chassis_by_card_serial_from_cms_options = mock.Mock()
|
||||
self.get_schema_version = mock.Mock(return_value='3.6.0')
|
||||
|
||||
|
||||
class FakeOvsdbTransaction(object):
|
||||
|
|
|
@ -522,7 +522,8 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight,
|
|||
"lsp1", external_ids=external_ids
|
||||
)
|
||||
|
||||
def test_check_for_mcast_flood_reports(self):
|
||||
def test_check_for_mcast_flood_reports_broken(self):
|
||||
self.fake_ovn_client.is_mcast_flood_broken = True
|
||||
nb_idl = self.fake_ovn_client._nb_idl
|
||||
lsp0 = fakes.FakeOvsdbRow.create_one_ovsdb_row(
|
||||
attrs={'name': 'lsp0',
|
||||
|
@ -563,14 +564,86 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight,
|
|||
self.assertRaises(periodics.NeverAgain,
|
||||
self.periodic.check_for_mcast_flood_reports)
|
||||
|
||||
# Assert only lsp1, lsp5 and lsp6 were called because they are the
|
||||
# only ones meeting the criteria
|
||||
# Assert only lsp1 and lsp5 were called because they are the
|
||||
# only ones meeting to set mcast_flood_reports to 'true'
|
||||
expected_calls = [
|
||||
mock.call('lsp1', mcast_flood_reports='true'),
|
||||
mock.call('lsp5', mcast_flood_reports='true', mcast_flood='false'),
|
||||
mock.call('lsp6', mcast_flood_reports='true', mcast_flood='false')]
|
||||
mock.call('lsp5', mcast_flood_reports='true')]
|
||||
|
||||
nb_idl.lsp_set_options.assert_has_calls(expected_calls)
|
||||
self.assertEqual(2, nb_idl.lsp_set_options.call_count)
|
||||
|
||||
# Assert only lsp6 and lsp7 were called because they are the
|
||||
# only ones meeting to remove mcast_flood
|
||||
expected_calls = [
|
||||
mock.call('Logical_Switch_Port', 'lsp6', 'options',
|
||||
constants.LSP_OPTIONS_MCAST_FLOOD,
|
||||
if_exists=True),
|
||||
mock.call('Logical_Switch_Port', 'lsp7', 'options',
|
||||
constants.LSP_OPTIONS_MCAST_FLOOD,
|
||||
if_exists=True)]
|
||||
|
||||
nb_idl.db_remove.assert_has_calls(expected_calls)
|
||||
self.assertEqual(2, nb_idl.db_remove.call_count)
|
||||
|
||||
def test_check_for_mcast_flood_reports(self):
|
||||
self.fake_ovn_client.is_mcast_flood_broken = False
|
||||
nb_idl = self.fake_ovn_client._nb_idl
|
||||
|
||||
lsp0 = fakes.FakeOvsdbRow.create_one_ovsdb_row(
|
||||
attrs={'name': 'lsp0',
|
||||
'options': {
|
||||
constants.LSP_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': {
|
||||
constants.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true'},
|
||||
'type': "vtep"})
|
||||
lsp3 = fakes.FakeOvsdbRow.create_one_ovsdb_row(
|
||||
attrs={'name': 'lsp3', 'options': {},
|
||||
'type': constants.LSP_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': constants.LSP_TYPE_LOCALNET})
|
||||
lsp6 = fakes.FakeOvsdbRow.create_one_ovsdb_row(
|
||||
attrs={'name': 'lsp6',
|
||||
'options': {
|
||||
constants.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true',
|
||||
constants.LSP_OPTIONS_MCAST_FLOOD: 'true'},
|
||||
'type': constants.LSP_TYPE_LOCALNET})
|
||||
lsp7 = fakes.FakeOvsdbRow.create_one_ovsdb_row(
|
||||
attrs={'name': 'lsp7',
|
||||
'options': {
|
||||
constants.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true',
|
||||
constants.LSP_OPTIONS_MCAST_FLOOD: 'false'},
|
||||
'type': constants.LSP_TYPE_LOCALNET})
|
||||
|
||||
nb_idl.lsp_list.return_value.execute.return_value = [
|
||||
lsp0, lsp1, lsp2, lsp3, lsp4, lsp5, lsp6, lsp7]
|
||||
|
||||
# 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 lsp0 and lsp2 were called because they are the
|
||||
# only ones meeting the criteria
|
||||
expected_calls = [
|
||||
mock.call('Logical_Switch_Port', 'lsp0', 'options',
|
||||
constants.LSP_OPTIONS_MCAST_FLOOD_REPORTS,
|
||||
if_exists=True),
|
||||
mock.call('Logical_Switch_Port', 'lsp2', 'options',
|
||||
constants.LSP_OPTIONS_MCAST_FLOOD_REPORTS,
|
||||
if_exists=True)]
|
||||
|
||||
nb_idl.db_remove.assert_has_calls(expected_calls)
|
||||
self.assertEqual(2, nb_idl.db_remove.call_count)
|
||||
|
||||
def test_check_router_mac_binding_options(self):
|
||||
nb_idl = self.fake_ovn_client._nb_idl
|
||||
|
|
|
@ -0,0 +1,7 @@
|
|||
---
|
||||
fixes:
|
||||
- |
|
||||
For OVN versions v22.09.0 and above, the ``mcast_flood_reports`` option
|
||||
is now set to ``false`` on all ports except "localnet" types. In the past,
|
||||
this option was set to ``true`` as a workaround for a bug in core OVN
|
||||
multicast implementation.
|
Loading…
Reference in New Issue