From 114ca0f1be8c10915d7e755e68ac2117f7db78e7 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Wed, 22 Nov 2023 13:05:24 +0000 Subject: [PATCH] Fix IGMP inconsistency across drivers Prior to this patch, ML2/OVS and ML2/OVN had inconsistent IGMP configurations. Neutron only exposed one configuration option for IGMP: igmp_snooping_enabled. Other features such as IGMP flood, IGMP flood reports and IGMP flood unregistered were hardcoded differently on each driver (see LP#2044272 for a more details). These hardcoded values has led to many changes over the years tweaking them to work on different scenarios but they were never final because the fix for one case would break the other. This patch introduces 3 new configuration options for these other IGMP features that can be enabled or disabled on both backends. Operators can now fine tune their deployments in the way that will work for them. As a consequence of the hardcoded values for each driver we had to break some defaults and, in the case of ML2/OVS, if operators want to keep things as they were before this patch they will need to enable the new mcast_flood and mcast_flood_unregistered configuration options. That said, the for ML2/OVS there was also an inconsistency with the help string of igmp_snooping_enabled configuration option as it mentioned that enabling snooping would disable flooding to unregistered ports but that was not true anymore after the fix [0]. [0] https://bugs.launchpad.net/neutron/+bug/1884723 Closes-Bug: #2044272 Change-Id: Ic4dde46aa0ea2b03362329c87341c83b24d32176 Signed-off-by: Lucas Alvares Gomes --- doc/source/admin/ovn/igmp.rst | 10 +++-- doc/source/configuration/ml2-conf.rst | 2 + neutron/agent/common/ovs_lib.py | 15 ++++--- neutron/cmd/upgrade_checks/checks.py | 30 +++++++++++++ neutron/conf/agent/ovs_conf.py | 37 +++++++++++++--- .../conf/plugins/ml2/drivers/ovn/ovn_conf.py | 4 -- .../openvswitch/agent/ovs_neutron_agent.py | 7 +-- .../ovn/mech_driver/ovsdb/maintenance.py | 44 ++++++++++++++----- .../ovn/mech_driver/ovsdb/ovn_client.py | 22 ++++++---- .../functional/agent/common/test_ovs_lib.py | 12 ++++- .../tests/functional/agent/test_ovs_lib.py | 4 +- .../agent/test_ovs_neutron_agent.py | 6 +-- .../openvswitch/agent/test_ovs_tunnel.py | 5 +-- .../ovn/mech_driver/test_mech_driver.py | 34 +++++++++----- .../notes/igmp-flood-f1b8aa5c799679ea.yaml | 31 +++++++++++++ 15 files changed, 200 insertions(+), 63 deletions(-) create mode 100644 releasenotes/notes/igmp-flood-f1b8aa5c799679ea.yaml diff --git a/doc/source/admin/ovn/igmp.rst b/doc/source/admin/ovn/igmp.rst index 7f25dbf4e65..35f285eceb7 100644 --- a/doc/source/admin/ovn/igmp.rst +++ b/doc/source/admin/ovn/igmp.rst @@ -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 ` for more information. + .. _`RFC 4541 session 2.1.2`: https://tools.ietf.org/html/rfc4541 diff --git a/doc/source/configuration/ml2-conf.rst b/doc/source/configuration/ml2-conf.rst index b29abd015d1..f9a3a567b56 100644 --- a/doc/source/configuration/ml2-conf.rst +++ b/doc/source/configuration/ml2-conf.rst @@ -1,3 +1,5 @@ +.. _ML2_CONF: + ============ ml2_conf.ini ============ diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index fd8c75a1585..6eb910ad487 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -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( diff --git a/neutron/cmd/upgrade_checks/checks.py b/neutron/cmd/upgrade_checks/checks.py index 74d5c57155b..140eea86539 100644 --- a/neutron/cmd/upgrade_checks/checks.py +++ b/neutron/cmd/upgrade_checks/checks.py @@ -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.')) diff --git a/neutron/conf/agent/ovs_conf.py b/neutron/conf/agent/ovs_conf.py index 5f9c9b76792..9b26aec39fc 100644 --- a/neutron/conf/agent/ovs_conf.py +++ b/neutron/conf/agent/ovs_conf.py @@ -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() diff --git a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py index ded3e7a706a..8630ea0ba85 100644 --- a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py +++ b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py @@ -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 diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index 45a79e7363d..41ba0e8bf54 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -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: 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 14ba3fccda5..e5697024545 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -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: 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 d4a0c5c3ced..af920d0afee 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 @@ -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()) diff --git a/neutron/tests/functional/agent/common/test_ovs_lib.py b/neutron/tests/functional/agent/common/test_ovs_lib.py index 374b8e692ad..f35d10b4264 100644 --- a/neutron/tests/functional/agent/common/test_ovs_lib.py +++ b/neutron/tests/functional/agent/common/test_ovs_lib.py @@ -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( diff --git a/neutron/tests/functional/agent/test_ovs_lib.py b/neutron/tests/functional/agent/test_ovs_lib.py index 4bcad26f472..71b471b5ec5 100644 --- a/neutron/tests/functional/agent/test_ovs_lib.py +++ b/neutron/tests/functional/agent/test_ovs_lib.py @@ -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()) diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 0ee994cf79e..d8dd778e906 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -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: diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py index aedd89d852c..8102e82a3e6 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py @@ -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, 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 706a9622ba6..9ec5d336fdc 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 @@ -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( diff --git a/releasenotes/notes/igmp-flood-f1b8aa5c799679ea.yaml b/releasenotes/notes/igmp-flood-f1b8aa5c799679ea.yaml new file mode 100644 index 00000000000..9ee175c017d --- /dev/null +++ b/releasenotes/notes/igmp-flood-f1b8aa5c799679ea.yaml @@ -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