From cf13776a733ebabc6bbc588e6e8ecbd9133cb2d0 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Mon, 16 Nov 2020 10:28:09 +0000 Subject: [PATCH] [OVN] Fix inconsistent IGMP configuration Prior to this patch the IGMP configuration for ML2/OVN was inconsistent with the configuration option description and also the ML2/OVS driver because it was flooding traffic to unregistered VMs [0]. The "igmp_snooping_enable" configuration option says: "Setting this option to True will also enable Open vSwitch mcast-snooping-disable-flood-unregistered flag. This option will disable flooding of unregistered multicast packets to all ports." But, in ML2/OVN that behavior was inconsistent prior to this patch because it allowed traffic to flood to unregistered VMs. This patch fixes it. Conflicts: neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py [0] https://opendev.org/openstack/neutron/src/branch/master/neutron/conf/agent/ovs_conf.py#L36-L47 Change-Id: I5cbe09e26120905b29351d61bbadb30b5dd14938 Closes-Bug: #1904399 Signed-off-by: Lucas Alvares Gomes (cherry picked from commit 9dc8bca7401106313cb227cb85d07eba7d987cb5) --- doc/source/admin/ovn/igmp.rst | 8 ++++---- .../ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py | 2 +- .../ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py | 2 +- .../ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py | 2 +- .../ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py | 6 +++--- .../ml2/drivers/ovn/mech_driver/test_mech_driver.py | 2 +- .../ovn-igmp-flood-unregistered-82aeb640f5dded84.yaml | 8 ++++++++ 7 files changed, 19 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/ovn-igmp-flood-unregistered-82aeb640f5dded84.yaml diff --git a/doc/source/admin/ovn/igmp.rst b/doc/source/admin/ovn/igmp.rst index e5b804ecd69..7f25dbf4e65 100644 --- a/doc/source/admin/ovn/igmp.rst +++ b/doc/source/admin/ovn/igmp.rst @@ -35,15 +35,15 @@ OVN Database information ~~~~~~~~~~~~~~~~~~~~~~~~ The ``igmp_snooping_enable`` configuration from Neutron is translated -into the ``mcast_snoop`` and ``mcast_flood_unregistered`` options set -in the ``other_config`` column from the ``Logical_Switch`` table in the -OVN Northbound Database: +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"): .. code-block:: bash $ ovn-nbctl list Logical_Switch _uuid : d6a2fbcd-aaa4-4b9e-8274-184238d66a15 - other_config : {mcast_flood_unregistered="true", mcast_snoop="true"} + other_config : {mcast_flood_unregistered="false", mcast_snoop="true"} ... .. end 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 0a510c6a9f8..a6d31c34ea7 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -566,7 +566,7 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): 'Logical_Switch', ls.name, ('other_config', { ovn_const.MCAST_SNOOP: value, - ovn_const.MCAST_FLOOD_UNREGISTERED: value}))) + ovn_const.MCAST_FLOOD_UNREGISTERED: 'false'}))) raise periodics.NeverAgain() 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 372ad975e53..be1aeac497c 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 @@ -1600,7 +1600,7 @@ class OVNClient(object): # 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} + ovn_const.MCAST_FLOOD_UNREGISTERED: 'false'} return params def create_network(self, context, network): 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 f6c618126ec..5c37a6f263d 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 @@ -846,7 +846,7 @@ class TestMaintenance(_TestMaintenanceHelper): self.assertEqual('true', ls['other_config'][ovn_const.MCAST_SNOOP]) self.assertEqual( - 'true', ls['other_config'][ovn_const.MCAST_FLOOD_UNREGISTERED]) + 'false', ls['other_config'][ovn_const.MCAST_FLOOD_UNREGISTERED]) def test_floating_ip(self): ext_net = self._create_network('ext_networktest', external=True) 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 e8a643babd5..afd36b9a086 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 @@ -319,7 +319,7 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, attrs={'name': 'ls2', 'other_config': { constants.MCAST_SNOOP: 'true', - constants.MCAST_FLOOD_UNREGISTERED: 'true'}}) + constants.MCAST_FLOOD_UNREGISTERED: 'false'}}) nb_idl.ls_list.return_value.execute.return_value = [ls0, ls1, ls2] @@ -332,11 +332,11 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, mock.call('Logical_Switch', 'ls0', ('other_config', { constants.MCAST_SNOOP: 'true', - constants.MCAST_FLOOD_UNREGISTERED: 'true'})), + constants.MCAST_FLOOD_UNREGISTERED: 'false'})), mock.call('Logical_Switch', 'ls1', ('other_config', { constants.MCAST_SNOOP: 'true', - constants.MCAST_FLOOD_UNREGISTERED: 'true'})), + constants.MCAST_FLOOD_UNREGISTERED: 'false'})), ] 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 793b65ad59a..cef9e14b89d 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 @@ -683,7 +683,7 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase): 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}) + ovn_const.MCAST_FLOOD_UNREGISTERED: 'false'}) def test_create_network_igmp_snoop_enabled(self): self._create_network_igmp_snoop(enabled=True) diff --git a/releasenotes/notes/ovn-igmp-flood-unregistered-82aeb640f5dded84.yaml b/releasenotes/notes/ovn-igmp-flood-unregistered-82aeb640f5dded84.yaml new file mode 100644 index 00000000000..a06aae61c71 --- /dev/null +++ b/releasenotes/notes/ovn-igmp-flood-unregistered-82aeb640f5dded84.yaml @@ -0,0 +1,8 @@ +--- +issues: + - | + Even with the "igmp_snooping_enable" configuration option stating that + traffic would not be flooded to unregistered VMs when this option was + enabled, the ML2/OVN driver didn't follow that behavior. This + has now been fixed and ML2/OVN will no longer flood traffic to + unregistered VMs when this configuration option is set to True.