diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index 4d8136ad609..8f61eb3f094 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -203,3 +203,7 @@ METADATA_DEFAULT_IP = '169.254.169.254' METADATA_DEFAULT_CIDR = '%s/%d' % (METADATA_DEFAULT_IP, METADATA_DEFAULT_PREFIX) METADATA_PORT = 80 + +# OVN igmp options +MCAST_SNOOP = 'mcast_snoop' +MCAST_FLOOD_UNREGISTERED = 'mcast_flood_unregistered' diff --git a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py index fe3f7957157..ded41c5920c 100644 --- a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py +++ b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py @@ -16,6 +16,7 @@ from oslo_log import log as logging from ovsdbapp.backend.ovs_idl import vlog from neutron._i18n import _ +from neutron.conf.agent import ovs_conf LOG = logging.getLogger(__name__) @@ -193,11 +194,13 @@ ovn_opts = [ ] cfg.CONF.register_opts(ovn_opts, group='ovn') +ovs_conf.register_ovs_agent_opts() def list_opts(): return [ ('ovn', ovn_opts), + ('ovs', ovs_conf.OPTS) ] @@ -291,3 +294,7 @@ def get_global_dhcpv6_opts(): def is_ovn_emit_need_to_frag_enabled(): return cfg.CONF.ovn.ovn_emit_need_to_frag + + +def is_igmp_snooping_enabled(): + return cfg.CONF.OVS.igmp_snooping_enable diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py index b33143e5521..7d2793cf6e6 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py @@ -23,19 +23,6 @@ from neutron.common.ovn import constants as ovn_const @six.add_metaclass(abc.ABCMeta) class API(api.API): - @abc.abstractmethod - def set_lswitch_ext_ids(self, name, ext_ids, if_exists=True): - """Create a command to set OVN lswitch external ids - - :param name: The name of the lswitch - :type name: string - :param ext_ids The external ids to set for the lswitch - :type ext_ids: dictionary - :param if_exists: Do not fail if lswitch does not exist - :type if_exists: bool - :returns: :class:`Command` with no result - """ - @abc.abstractmethod def create_lswitch_port(self, lport_name, lswitch_name, may_exist=True, **columns): diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py index 525f1737632..22c4d34993f 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -101,31 +101,6 @@ class CheckLivenessCommand(command.BaseCommand): self.result = self.api.nb_global.nb_cfg -class LSwitchSetExternalIdsCommand(command.BaseCommand): - def __init__(self, api, name, ext_ids, if_exists): - super(LSwitchSetExternalIdsCommand, self).__init__(api) - self.name = name - self.ext_ids = ext_ids - self.if_exists = if_exists - - def run_idl(self, txn): - try: - lswitch = idlutils.row_by_value(self.api.idl, 'Logical_Switch', - 'name', self.name) - - except idlutils.RowNotFound: - if self.if_exists: - return - msg = _("Logical Switch %s does not exist") % self.name - raise RuntimeError(msg) - - lswitch.verify('external_ids') - external_ids = getattr(lswitch, 'external_ids', {}) - for key, value in self.ext_ids.items(): - external_ids[key] = value - lswitch.external_ids = external_ids - - class AddLSwitchPortCommand(command.BaseCommand): def __init__(self, api, lport, lswitch, may_exist, **columns): super(AddLSwitchPortCommand, self).__init__(api) 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 7fff3a2c355..cb79fd5ffee 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 @@ -185,10 +185,6 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): except ovn_exc.RevisionConflict as e: LOG.info('Transaction aborted. Reason: %s', e) - def set_lswitch_ext_ids(self, lswitch_id, ext_ids, if_exists=True): - return cmd.LSwitchSetExternalIdsCommand(self, lswitch_id, ext_ids, - if_exists) - def create_lswitch_port(self, lport_name, lswitch_name, may_exist=True, **columns): return cmd.AddLSwitchPortCommand(self, lport_name, lswitch_name, 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 655344f6f78..276888c2dee 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -459,6 +459,27 @@ class DBInconsistenciesPeriodics(object): 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_for_igmp_snoop_support(self): + if not self.has_lock: + return + + with self._nb_idl.transaction(check_error=True) as txn: + value = ('true' if ovn_conf.is_igmp_snooping_enabled() + else 'false') + for ls in self._nb_idl.ls_list().execute(check_error=True): + if ls.other_config.get(ovn_const.MCAST_SNOOP, None) == value: + continue + txn.add(self._nb_idl.db_set( + 'Logical_Switch', ls.name, + ('other_config', { + ovn_const.MCAST_SNOOP: value, + ovn_const.MCAST_FLOOD_UNREGISTERED: value}))) + + 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 a3e4d2ffa89..18afcf36578 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 @@ -1719,31 +1719,36 @@ class OVNClient(object): tag=tag if tag else [], options={'network_name': physnet})) - def _gen_network_external_ids(self, network): - ext_ids = { + def _gen_network_parameters(self, network): + params = {'external_ids': { ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: network['name'], ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY: str(network['mtu']), ovn_const.OVN_REV_NUM_EXT_ID_KEY: str( - utils.get_revision_number(network, ovn_const.TYPE_NETWORKS))} + utils.get_revision_number(network, ovn_const.TYPE_NETWORKS))}} # NOTE(lucasagomes): There's a difference between the # "qos_policy_id" key existing and it being None, the latter is a # valid value. Since we can't save None in OVSDB, we are converting # it to "null" as a placeholder. if 'qos_policy_id' in network: - ext_ids[ovn_const.OVN_QOS_POLICY_EXT_ID_KEY] = ( + params['external_ids'][ovn_const.OVN_QOS_POLICY_EXT_ID_KEY] = ( network['qos_policy_id'] or 'null') - return ext_ids + + # Enable IGMP snooping if igmp_snooping_enable is enabled in Neutron + value = 'true' if ovn_conf.is_igmp_snooping_enabled() else 'false' + params['other_config'] = {ovn_const.MCAST_SNOOP: value, + ovn_const.MCAST_FLOOD_UNREGISTERED: value} + return params def create_network(self, network): # Create a logical switch with a name equal to the Neutron network # UUID. This provides an easy way to refer to the logical switch # without having to track what UUID OVN assigned to it. admin_context = n_context.get_admin_context() - ext_ids = self._gen_network_external_ids(network) + lswitch_params = self._gen_network_parameters(network) lswitch_name = utils.ovn_name(network['id']) with self._nb_idl.transaction(check_error=True) as txn: - txn.add(self._nb_idl.ls_add(lswitch_name, external_ids=ext_ids, + txn.add(self._nb_idl.ls_add(lswitch_name, **lswitch_params, may_exist=True)) physnet = network.get(pnet.PHYSICAL_NETWORK) if physnet: @@ -1832,9 +1837,10 @@ class OVNClient(object): admin_context = n_context.get_admin_context() with self._nb_idl.transaction(check_error=True) as txn: txn.add(check_rev_cmd) - ext_ids = self._gen_network_external_ids(network) + lswitch_params = self._gen_network_parameters(network) lswitch = self._nb_idl.get_lswitch(lswitch_name) - txn.add(self._nb_idl.set_lswitch_ext_ids(lswitch_name, ext_ids)) + txn.add(self._nb_idl.db_set( + 'Logical_Switch', lswitch_name, *lswitch_params.items())) # Check if previous mtu is different than current one, # checking will help reduce number of operations if (not lswitch or diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index a15ea19c2c0..3bace092ca6 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -14,6 +14,7 @@ # under the License. import mock +from oslo_config import cfg from futurist import periodics from neutron_lib.api.definitions import external_net as extnet_apidef @@ -798,3 +799,30 @@ class TestMaintenance(_TestMaintenanceHelper): # for the port self.assertTrue(ovn_port['port_security']) self.assertNotIn('unknown', ovn_port['addresses']) + + def test_check_for_igmp_snooping_enabled(self): + cfg.CONF.set_override('igmp_snooping_enable', False, group='OVS') + net = self._create_network('net') + ls = self.nb_api.db_find('Logical_Switch', + ('name', '=', utils.ovn_name(net['id']))).execute( + check_error=True)[0] + + self.assertEqual('false', ls['other_config'][ovn_const.MCAST_SNOOP]) + self.assertEqual( + 'false', ls['other_config'][ovn_const.MCAST_FLOOD_UNREGISTERED]) + + # Change the value of the configuration + cfg.CONF.set_override('igmp_snooping_enable', True, group='OVS') + + # Call the maintenance task and check that the value has been + # updated in the Logical Switch + self.assertRaises(periodics.NeverAgain, + self.maint.check_for_igmp_snoop_support) + + ls = self.nb_api.db_find('Logical_Switch', + ('name', '=', utils.ovn_name(net['id']))).execute( + check_error=True)[0] + + self.assertEqual('true', ls['other_config'][ovn_const.MCAST_SNOOP]) + self.assertEqual( + 'true', ls['other_config'][ovn_const.MCAST_FLOOD_UNREGISTERED]) diff --git a/neutron/tests/unit/fake_resources.py b/neutron/tests/unit/fake_resources.py index 343a8dbb3f4..3181790bafa 100644 --- a/neutron/tests/unit/fake_resources.py +++ b/neutron/tests/unit/fake_resources.py @@ -52,7 +52,6 @@ class FakeOvsdbNbOvnIdl(object): self.transaction = mock.MagicMock() self.create_transaction = mock.MagicMock() self.ls_add = mock.Mock() - self.set_lswitch_ext_ids = mock.Mock() self.ls_del = mock.Mock() self.create_lswitch_port = mock.Mock() self.set_lswitch_port = mock.Mock() diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py index 91c397727ba..76b612a73f8 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py @@ -94,43 +94,6 @@ class TestCheckLivenessCommand(TestBaseCommand): self.assertNotEqual(cmd.result, old_ng_cfg) -class TestLSwitchSetExternalIdsCommand(TestBaseCommand): - - def _test_lswitch_extid_update_no_exist(self, if_exists=True): - with mock.patch.object(idlutils, 'row_by_value', - side_effect=idlutils.RowNotFound): - cmd = commands.LSwitchSetExternalIdsCommand( - self.ovn_api, 'fake-lswitch', - {ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'neutron-network'}, - if_exists=if_exists) - if if_exists: - cmd.run_idl(self.transaction) - else: - self.assertRaises(RuntimeError, cmd.run_idl, self.transaction) - - def test_lswitch_no_exist_ignore(self): - self._test_lswitch_extid_update_no_exist(if_exists=True) - - def test_lswitch_no_exist_fail(self): - self._test_lswitch_extid_update_no_exist(if_exists=False) - - def test_lswitch_extid_update(self): - network_name = 'private' - new_network_name = 'private-new' - ext_ids = {ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: network_name} - new_ext_ids = {ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: new_network_name} - fake_lswitch = fakes.FakeOvsdbRow.create_one_ovsdb_row( - attrs={'external_ids': ext_ids}) - with mock.patch.object(idlutils, 'row_by_value', - return_value=fake_lswitch): - cmd = commands.LSwitchSetExternalIdsCommand( - self.ovn_api, fake_lswitch.name, - {ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: new_network_name}, - if_exists=True) - cmd.run_idl(self.transaction) - self.assertEqual(new_ext_ids, fake_lswitch.external_ids) - - class TestAddLSwitchPortCommand(TestBaseCommand): def test_lswitch_not_found(self): 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 a99b6b3e6fe..70d338647a8 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 @@ -17,6 +17,7 @@ import mock from futurist import periodics from neutron_lib import context +from oslo_config import cfg from neutron.common.ovn import constants from neutron.common.ovn import utils @@ -24,6 +25,7 @@ from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.db import ovn_revision_numbers_db from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import maintenance from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_db_sync +from neutron.tests.unit import fake_resources as fakes from neutron.tests.unit.plugins.ml2 import test_security_group as test_sg from neutron.tests.unit import testlib_api @@ -39,7 +41,7 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, self.fmt, name='net1', admin_state_up=True)['network'] self.port = self._make_port( self.fmt, self.net['id'], name='port1')['port'] - self.fake_ovn_client = mock.Mock() + self.fake_ovn_client = mock.MagicMock() self.periodic = maintenance.DBInconsistenciesPeriodics( self.fake_ovn_client) self.ctx = context.get_admin_context() @@ -269,3 +271,39 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, incst = [mock.Mock(resource_type=constants.TYPE_NETWORKS)] * 2 self.periodic._log_maintenance_inconsistencies(incst, []) self.assertFalse(mock_log.called) + + def test_check_for_igmp_snoop_support(self): + cfg.CONF.set_override('igmp_snooping_enable', True, group='OVS') + nb_idl = self.fake_ovn_client._nb_idl + ls0 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'ls0', + 'other_config': { + constants.MCAST_SNOOP: 'false', + constants.MCAST_FLOOD_UNREGISTERED: 'false'}}) + ls1 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'ls1', + 'other_config': {}}) + ls2 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'ls2', + 'other_config': { + constants.MCAST_SNOOP: 'true', + constants.MCAST_FLOOD_UNREGISTERED: 'true'}}) + + nb_idl.ls_list.return_value.execute.return_value = [ls0, ls1, ls2] + + self.assertRaises(periodics.NeverAgain, + self.periodic.check_for_igmp_snoop_support) + + # "ls2" is not part of the transaction because it already + # have the right value set + expected_calls = [ + mock.call('Logical_Switch', 'ls0', + ('other_config', { + constants.MCAST_SNOOP: 'true', + constants.MCAST_FLOOD_UNREGISTERED: 'true'})), + mock.call('Logical_Switch', 'ls1', + ('other_config', { + constants.MCAST_SNOOP: 'true', + constants.MCAST_FLOOD_UNREGISTERED: 'true'})), + ] + nb_idl.db_set.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 fc0da610ef3..97b9b3c8c41 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 @@ -576,6 +576,24 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase): self.mech_driver.update_network_precommit, fake_network_context) + def _create_network_igmp_snoop(self, enabled): + cfg.CONF.set_override('igmp_snooping_enable', enabled, group='OVS') + nb_idl = self.mech_driver._ovn_client._nb_idl + net = self._make_network(self.fmt, name='net1', + admin_state_up=True)['network'] + value = 'true' if enabled else 'false' + nb_idl.ls_add.assert_called_once_with( + ovn_utils.ovn_name(net['id']), external_ids=mock.ANY, + may_exist=True, + other_config={ovn_const.MCAST_SNOOP: value, + ovn_const.MCAST_FLOOD_UNREGISTERED: value}) + + def test_create_network_igmp_snoop_enabled(self): + self._create_network_igmp_snoop(enabled=True) + + def test_create_network_igmp_snoop_disabled(self): + self._create_network_igmp_snoop(enabled=False) + def test_create_port_without_security_groups(self): kwargs = {'security_groups': []} with self.network(set_context=True, tenant_id='test') as net1: diff --git a/releasenotes/notes/ovn-igmp-snooping-support-1a6ec8e703311fce.yaml b/releasenotes/notes/ovn-igmp-snooping-support-1a6ec8e703311fce.yaml new file mode 100644 index 00000000000..9c4b8809314 --- /dev/null +++ b/releasenotes/notes/ovn-igmp-snooping-support-1a6ec8e703311fce.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Adds support for IGMP snooping (Multicast) in the OVN driver. Defaults + to False. IGMP snooping requires OVN version 2.12 or above.