Merge "[OVN] Add IGMP snooping support"
This commit is contained in:
Zuul
Gerrit Code Review
commit
1f79ce8736
|
|
@ -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'
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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])
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
Loading…
Reference in New Issue