openstack
/
neutron
Code Issues Proposed changes

Merge "Fix IGMP inconsistency across drivers"

This commit is contained in:
Zuul 2023-12-18 13:19:07 +00:00 committed by Gerrit Code Review
parent 0643e1c3af 114ca0f1be
commit ef3089547b
15 changed files with 200 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
doc/source/admin/ovn/igmp.rst
View File

@ -36,8 +36,7 @@ OVN Database information
The ``igmp_snooping_enable`` configuration from Neutron is translated
into the ``mcast_snoop`` option set in the ``other_config`` column
from the ``Logical_Switch`` table in the OVN Northbound Database
(``mcast_flood_unregistered`` is always "false"):
from the ``Logical_Switch`` table in the OVN Northbound Database:
.. code-block:: bash
@ -71,7 +70,6 @@ command below (populated only when igmp_snooping_enable is True):
groups and broadcast all the multicast traffic. This behavior can
impact when updating/upgrading the OVN services.
Extra information
~~~~~~~~~~~~~~~~~
@ -92,5 +90,11 @@ The permutations from different configurations are:
* With IGMP snooping enabled and multicast group address **is in**
the 224.0.0.X range: IP Multicast traffic **is** flooded.
* Apart from the ``igmp_snooping_enable`` configuration option mentioned
before, there are 3 other configuration options supported by the OVN
driver: ``igmp_flood``, ``igmp_flood_reports`` and
``igmp_flood_unregistered``. Check the :ref:`ML2 configuration
reference page <ML2_CONF>` for more information.
.. _`RFC 4541 session 2.1.2`: https://tools.ietf.org/html/rfc4541

2
doc/source/configuration/ml2-conf.rst
View File

@ -1,3 +1,5 @@
.. _ML2_CONF:
============
ml2_conf.ini
============

15
neutron/agent/common/ovs_lib.py
View File

@ -334,8 +334,14 @@ class OVSBridge(BaseOVS):
def set_igmp_snooping_state(self, state):
state = bool(state)
# NOTE(lucasagomes): The mcast-snooping-disable-flood-unregistered
# has the opposite value of the config in Neutron. That's because
# IGMP Neutron configs are more value consistent using True to
# enable a feature and False to disable it.
flood_value = ('false' if
cfg.CONF.OVS.igmp_flood_unregistered else 'true')
other_config = {
'mcast-snooping-disable-flood-unregistered': 'false'}
'mcast-snooping-disable-flood-unregistered': flood_value}
with self.ovsdb.transaction() as txn:
txn.add(
self.ovsdb.db_set('Bridge', self.br_name,
@ -344,11 +350,10 @@ class OVSBridge(BaseOVS):
self.ovsdb.db_set('Bridge', self.br_name,
('other_config', other_config)))
def set_igmp_snooping_flood(self, port_name, state):
state = str(state)
def set_igmp_snooping_flood(self, port_name):
other_config = {
'mcast-snooping-flood-reports': state,
'mcast-snooping-flood': state}
'mcast-snooping-flood-reports': ovs_conf.get_igmp_flood_reports(),
'mcast-snooping-flood': ovs_conf.get_igmp_flood()}
self.ovsdb.db_set(
'Port', port_name,
('other_config', other_config)).execute(

30
neutron/cmd/upgrade_checks/checks.py
View File

@ -229,6 +229,8 @@ class CoreChecks(base.BaseChecks):
self.extra_dhcp_options_check),
(_('OVN support for BM provisioning over IPv6 check'),
self.ovn_for_bm_provisioning_over_ipv6_check),
(_('ML2/OVS IGMP Flood check'),
self.ml2_ovs_igmp_flood_check),
]
@staticmethod
@ -639,3 +641,31 @@ class CoreChecks(base.BaseChecks):
'c5fd51bd154147a567097eaf61fbebc0b5b39e28 which added '
'support for iPXE over IPv6. It is available in '
'OVN >= 23.06.0.'))
@staticmethod
def ml2_ovs_igmp_flood_check(checker):
"""Check for IGMP related traffic behavior changes for ML2/OVS
Since LP#2044272, the default behavior of IGMP related traffic has
changed for the ML2/OVS driver. This check raises a warning and
instruct the user how to configure IGMP to keep the same behavior
as prior to the upgrade.
"""
# NOTE(lucasagomes): igmp_flood_reports is not checked as part
# of this function because its default is already True.
if ('ovn' not in cfg.CONF.ml2.mechanism_drivers and
cfg.CONF.OVS.igmp_snooping_enable and
not cfg.CONF.OVS.igmp_flood_unregistered and
not cfg.CONF.OVS.igmp_flood):
return upgradecheck.Result(
upgradecheck.Code.WARNING,
_('For non-ML2/OVN deployments where ``igmp_snooping_enable`` '
'is enabled, the default behavior of IGMP related traffic '
'has changed after LP#2044272. To keep the same behavior '
'as before please ensure that the configuration options: '
'``igmp_flood_unregistered`` and ``igmp_flood`` are also '
'enabled in the [OVS] section of the configuration file.'))
return upgradecheck.Result(
upgradecheck.Code.SUCCESS,
_('IGMP related traffic configuration is not affected.'))

37
neutron/conf/agent/ovs_conf.py
View File

@ -37,15 +37,40 @@ OPTS = [
help=_('Enable IGMP snooping for integration bridge. If this '
'option is set to True, support for Internet Group '
'Management Protocol (IGMP) is enabled in integration '
'bridge. '
'Setting this option to True will also enable the Open '
'vSwitch mcast-snooping-disable-flood-unregistered '
'flag. This option will disable flooding of '
'bridge.')),
cfg.BoolOpt('igmp_flood', default=False,
help=_('Multicast packets (except reports) are '
'unconditionally forwarded to the ports bridging a '
'logical network to a physical network.')),
cfg.BoolOpt('igmp_flood_reports', default=True,
help=_('Multicast reports are unconditionally forwarded '
'to the ports bridging a logical network to a '
'physical network.')),
cfg.BoolOpt('igmp_flood_unregistered', default=False,
help=_('This option enables or disables flooding of '
'unregistered multicast packets to all ports. '
'The switch will send unregistered multicast packets '
'only to ports connected to multicast routers.')),
'If False, The switch will send unregistered '
'multicast packets only to ports connected to '
'multicast routers.')),
]
def register_ovs_agent_opts(cfg=cfg.CONF):
cfg.register_opts(OPTS, 'OVS')
def get_igmp_snooping_enabled():
return str(cfg.CONF.OVS.igmp_snooping_enable).lower()
def get_igmp_flood():
return str(cfg.CONF.OVS.igmp_flood).lower()
def get_igmp_flood_reports():
return str(cfg.CONF.OVS.igmp_flood_reports).lower()
def get_igmp_flood_unregistered():
return str(cfg.CONF.OVS.igmp_flood_unregistered).lower()

4
neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py
View File

@ -370,10 +370,6 @@ 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
def is_ovn_dhcp_disabled_for_baremetal():
return cfg.CONF.ovn.disable_ovn_dhcp_for_baremetal_ports

7
neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
View File

@ -1558,9 +1558,7 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
"version of OVS does not support tunnels or patch "
"ports. Agent terminated!")
sys.exit(1)
self.int_br.set_igmp_snooping_flood(
self.conf.OVS.int_peer_patch_port,
self.conf.OVS.igmp_snooping_enable)
self.int_br.set_igmp_snooping_flood(self.conf.OVS.int_peer_patch_port)
if self.conf.AGENT.drop_flows_on_start:
self.tun_br.uninstall_flows(cookie=ovs_lib.COOKIE_ANY)
@ -1691,8 +1689,7 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
else:
int_ofport = self.int_br.add_patch_port(
int_if_name, ovs_const.NONEXISTENT_PEER)
self.int_br.set_igmp_snooping_flood(
int_if_name, self.conf.OVS.igmp_snooping_enable)
self.int_br.set_igmp_snooping_flood(int_if_name)
if br.port_exists(phys_if_name):
phys_ofport = br.get_port_ofport(phys_if_name)
else:

44
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py
View File

@ -34,6 +34,7 @@ from ovsdbapp.backend.ovs_idl import event as row_event
from neutron.common.ovn import constants as ovn_const
from neutron.common.ovn import utils
from neutron.conf.agent import ovs_conf
from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
from neutron.db import l3_attrs_db
from neutron.db import ovn_hash_ring_db as hash_ring_db
@ -471,18 +472,28 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
# once per lock due to the use of periodics.NeverAgain().
@has_lock_periodic(spacing=600, run_immediately=True)
def check_for_igmp_snoop_support(self):
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 or not ls.name):
continue
txn.add(self._nb_idl.db_set(
snooping_conf = ovs_conf.get_igmp_snooping_enabled()
flood_conf = ovs_conf.get_igmp_flood_unregistered()
cmds = []
for ls in self._nb_idl.ls_list().execute(check_error=True):
snooping = ls.other_config.get(ovn_const.MCAST_SNOOP)
flood = ls.other_config.get(ovn_const.MCAST_FLOOD_UNREGISTERED)
if (not ls.name or (snooping == snooping_conf and
flood == flood_conf)):
continue
cmds.append(self._nb_idl.db_set(
'Logical_Switch', ls.name,
('other_config', {
ovn_const.MCAST_SNOOP: value,
ovn_const.MCAST_FLOOD_UNREGISTERED: 'false'})))
ovn_const.MCAST_SNOOP: snooping_conf,
ovn_const.MCAST_FLOOD_UNREGISTERED: flood_conf})))
if cmds:
with self._nb_idl.transaction(check_error=True) as txn:
for cmd in cmds:
txn.add(cmd)
raise periodics.NeverAgain()
@ -557,12 +568,16 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
# once per lock due to the use of periodics.NeverAgain().
@has_lock_periodic(spacing=600, run_immediately=True)
def check_for_mcast_flood_reports(self):
mcast_flood_conf = ovs_conf.get_igmp_flood()
mcast_flood_reports_conf = ovs_conf.get_igmp_flood_reports()
cmds = []
for port in self._nb_idl.lsp_list().execute(check_error=True):
port_type = port.type.strip()
options = port.options
mcast_flood_reports_value = options.get(
ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS)
mcast_flood_value = options.get(
ovn_const.LSP_OPTIONS_MCAST_FLOOD)
if self._ovn_client.is_mcast_flood_broken:
if port_type in ("vtep", ovn_const.LSP_TYPE_LOCALPORT,
@ -591,6 +606,15 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
'Logical_Switch_Port', port.name, 'options',
ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS, if_exists=True))
elif (port_type == ovn_const.LSP_TYPE_LOCALNET and (
mcast_flood_conf != mcast_flood_value or
mcast_flood_reports_conf != mcast_flood_reports_value)):
options.update({
ovn_const.LSP_OPTIONS_MCAST_FLOOD: mcast_flood_conf,
ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS:
mcast_flood_reports_conf})
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:

22
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
View File

@ -47,6 +47,7 @@ from neutron.common.ovn import acl as ovn_acl
from neutron.common.ovn import constants as ovn_const
from neutron.common.ovn import utils
from neutron.common import utils as common_utils
from neutron.conf.agent import ovs_conf
from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
from neutron.db import ovn_revision_numbers_db as db_rev
from neutron.db import segments_db
@ -1889,10 +1890,13 @@ class OVNClient(object):
physnet = segment.get(segment_def.PHYSICAL_NETWORK)
fdb_enabled = ('true' if ovn_conf.is_learn_fdb_enabled()
else 'false')
options = {'network_name': physnet,
ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true',
ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false',
ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: fdb_enabled}
options = {
'network_name': physnet,
ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS:
ovs_conf.get_igmp_flood_reports(),
ovn_const.LSP_OPTIONS_MCAST_FLOOD:
ovs_conf.get_igmp_flood(),
ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: fdb_enabled}
cmd = self._nb_idl.create_lswitch_port(
lport_name=utils.ovn_provnet_port_name(segment['id']),
lswitch_name=utils.ovn_name(network_id),
@ -1920,12 +1924,14 @@ class OVNClient(object):
','.join(common_utils.get_az_hints(network))}}
# Enable IGMP snooping if igmp_snooping_enable is enabled in Neutron
value = 'true' if ovn_conf.is_igmp_snooping_enabled() else 'false'
vlan_transparent = (
'true' if network.get('vlan_transparent') else 'false')
params['other_config'] = {ovn_const.MCAST_SNOOP: value,
ovn_const.MCAST_FLOOD_UNREGISTERED: 'false',
ovn_const.VLAN_PASSTHRU: vlan_transparent}
params['other_config'] = {
ovn_const.MCAST_SNOOP:
ovs_conf.get_igmp_snooping_enabled(),
ovn_const.MCAST_FLOOD_UNREGISTERED:
ovs_conf.get_igmp_flood_unregistered(),
ovn_const.VLAN_PASSTHRU: vlan_transparent}
if utils.is_provider_network(network):
params['other_config'][ovn_const.LS_OPTIONS_FDB_AGE_THRESHOLD] = (
ovn_conf.get_fdb_age_threshold())

12
neutron/tests/functional/agent/common/test_ovs_lib.py
View File

@ -19,6 +19,7 @@ from unittest import mock
from neutron_lib import constants as p_const
from neutron_lib.plugins.ml2 import ovs_constants
from neutron_lib.services.qos import constants as qos_constants
from oslo_config import cfg
from oslo_utils import uuidutils
from ovsdbapp.backend.ovs_idl import event
@ -638,7 +639,11 @@ class BaseOVSTestCase(base.BaseSudoTestCase):
port_name = 'test_output_port_2'
self._create_bridge()
self._create_port(port_name)
self.ovs.set_igmp_snooping_flood(port_name, True)
# Enable flood
cfg.CONF.set_override('igmp_flood', True, group='OVS')
cfg.CONF.set_override('igmp_flood_reports', True, group='OVS')
self.ovs.set_igmp_snooping_flood(port_name)
ports_other_config = self.ovs.db_get_val('Port', port_name,
'other_config')
self.assertEqual(
@ -648,7 +653,10 @@ class BaseOVSTestCase(base.BaseSudoTestCase):
'true',
ports_other_config.get('mcast-snooping-flood-reports', '').lower())
self.ovs.set_igmp_snooping_flood(port_name, False)
# Disable flood
cfg.CONF.set_override('igmp_flood', False, group='OVS')
cfg.CONF.set_override('igmp_flood_reports', False, group='OVS')
self.ovs.set_igmp_snooping_flood(port_name)
ports_other_config = self.ovs.db_get_val('Port', port_name,
'other_config')
self.assertEqual(

4
neutron/tests/functional/agent/test_ovs_lib.py
View File

@ -205,8 +205,10 @@ class OVSBridgeTestCase(OVSBridgeTestBase):
br_other_config = self.ovs.ovsdb.db_find(
'Bridge', ('name', '=', self.br.br_name), columns=['other_config']
).execute()[0]['other_config']
expected_flood_value = ('false' if
cfg.CONF.OVS.igmp_flood_unregistered else 'true')
self.assertEqual(
'false',
expected_flood_value,
br_other_config.get(
'mcast-snooping-disable-flood-unregistered', '').lower())

6
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
View File

@ -1684,8 +1684,7 @@ class TestOvsNeutronAgent(object):
'int-br-eth', ovs_constants.NONEXISTENT_PEER),
]
expected_calls += [
mock.call.int_br.set_igmp_snooping_flood(
'int-br-eth', igmp_snooping_enabled),
mock.call.int_br.set_igmp_snooping_flood('int-br-eth'),
mock.call.phys_br.port_exists('phy-br-eth'),
]
if port_exists:
@ -1778,8 +1777,7 @@ class TestOvsNeutronAgent(object):
'int-br-eth', ovs_constants.NONEXISTENT_PEER),
]
expected_calls += [
mock.call.int_br.set_igmp_snooping_flood(
'int-br-eth', False),
mock.call.int_br.set_igmp_snooping_flood('int-br-eth'),
mock.call.phys_br.port_exists('phy-br-eth'),
]
if port_exists:

5
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py
View File

@ -228,8 +228,7 @@ class TunnelTest(object):
mock.call.port_exists('int-%s' % self.MAP_TUN_BRIDGE),
mock.call.add_patch_port('int-%s' % self.MAP_TUN_BRIDGE,
ovs_constants.NONEXISTENT_PEER),
mock.call.set_igmp_snooping_flood('int-%s' % self.MAP_TUN_BRIDGE,
igmp_snooping),
mock.call.set_igmp_snooping_flood('int-%s' % self.MAP_TUN_BRIDGE),
]
self.mock_int_bridge_expected += [
@ -259,7 +258,7 @@ class TunnelTest(object):
self.mock_int_bridge_expected += [
mock.call.port_exists('patch-tun'),
mock.call.add_patch_port('patch-tun', 'patch-int'),
mock.call.set_igmp_snooping_flood('patch-tun', igmp_snooping),
mock.call.set_igmp_snooping_flood('patch-tun'),
]
self.mock_int_bridge_expected += [
mock.call.get_vif_ports((ovs_lib.INVALID_OFPORT,

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

@ -52,6 +52,7 @@ from neutron.common.ovn import constants as ovn_const
from neutron.common.ovn import exceptions as ovn_exceptions
from neutron.common.ovn import hash_ring_manager
from neutron.common.ovn import utils as ovn_utils
from neutron.conf.agent import ovs_conf
from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
from neutron.db import db_base_plugin_v2
from neutron.db import ovn_revision_numbers_db
@ -881,10 +882,13 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
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',
ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true',
ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false',
ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: 'false'},
options={
'network_name': 'physnet1',
ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS:
ovs_conf.get_igmp_flood_reports(),
ovn_const.LSP_OPTIONS_MCAST_FLOOD:
ovs_conf.get_igmp_flood(),
ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: 'false'},
tag=2,
type='localnet')
@ -3190,10 +3194,13 @@ class TestOVNMechanismDriverSegment(MechDriverSetupBase,
external_ids={},
lport_name=ovn_utils.ovn_provnet_port_name(new_segment['id']),
lswitch_name=ovn_utils.ovn_name(net['id']),
options={'network_name': 'physnet1',
ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true',
ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false',
ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: 'false'},
options={
'network_name': 'physnet1',
ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS:
ovs_conf.get_igmp_flood_reports(),
ovn_const.LSP_OPTIONS_MCAST_FLOOD:
ovs_conf.get_igmp_flood(),
ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: 'false'},
tag=200,
type='localnet')
ovn_nb_api.create_lswitch_port.reset_mock()
@ -3205,10 +3212,13 @@ class TestOVNMechanismDriverSegment(MechDriverSetupBase,
external_ids={},
lport_name=ovn_utils.ovn_provnet_port_name(new_segment['id']),
lswitch_name=ovn_utils.ovn_name(net['id']),
options={'network_name': 'physnet2',
ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true',
ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false',
ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: 'false'},
options={
'network_name': 'physnet2',
ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS:
ovs_conf.get_igmp_flood_reports(),
ovn_const.LSP_OPTIONS_MCAST_FLOOD:
ovs_conf.get_igmp_flood(),
ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: 'false'},
tag=300,
type='localnet')
segments = segments_db.get_network_segments(

31
releasenotes/notes/igmp-flood-f1b8aa5c799679ea.yaml Normal file
View File

@ -0,0 +1,31 @@
---
prelude: >
The ML2/OVS and ML2/OVN drivers had inconsistencies on how IGMP was
configured. Prior to this patch Neutron only exposed a single IGMP
configuration option (``igmp_snooping_enabled``) and other features
such as flooding IGMP packets, flooding reports and flooding to
unregistered ports were hard coded differently in each driver.
This patch introduces Neutron configuration options for those listed
IGMP features.
As part of this work, default values in the IGMP snooping
configuration had to be changed for the ML2/OVS backend. Please
check in the following sections for more details.
features:
- |
New configuration options for IGMP were added in the [OVS] section:
``igmp_flood``, ``igmp_flood_reports`` and ``igmp_flood_unregistered``.
This gives operators full control on how IGMP should be configured for
their deployments.
upgrade:
- |
In order to make IGMP configuration consistent across drivers
some defaults had to be changed for ML2/OVS. This change did not
affect the defaults in the ML2/OVN driver.
If ML2/OVS is used and ``igmp_snooping_enable`` is enabled, in order
to make the IGMP related traffic run as before please ensure that
the following configuration options are enabled after the upgrade.
``[OVS]/igmp_flood_unregistered`` = True
``[OVS]/igmp_flood`` = True
``[OVS]/igmp_flood_reports`` = True