Merge "[OVN] Add support for broadcast_arps_to_all_routers"

neutron/common/ovn/constants.py

@@ -422,6 +422,8 @@ BRIDGE_REDIRECT_TYPE = "bridged"
 # FDB AGE Settings
 LS_OPTIONS_FDB_AGE_THRESHOLD = 'fdb_age_threshold'
 
+LS_OPTIONS_BROADCAST_ARPS_ROUTERS = 'broadcast-arps-to-all-routers'
+
 # Port Binding types
 PB_TYPE_VIRTUAL = 'virtual'
 

neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py

@@ -221,6 +221,14 @@ ovn_opts = [
                default=0,
                help=_('The number of seconds to keep MAC_Binding entries in '
                       'the OVN DB. 0 to disable aging.')),
+    cfg.BoolOpt('broadcast_arps_to_all_routers',
+                default=True,
+                help=_('If enabled (default) OVN will flood ARP requests to '
+                       'all attached ports on a network. If set to False, '
+                       'ARP requests are only sent to routers on that network '
+                       'if the target MAC address matches. ARP requests that '
+                       'do not match a router will only be forwarded to '
+                       'non-router ports. Supported by OVN >= 23.06.')),
 ]
 
 nb_global_opts = [
@@ -376,3 +384,7 @@ def get_fdb_removal_limit():
 def get_ovn_mac_binding_age_threshold():
     # This value is always stored as a string in the OVN DB
     return str(cfg.CONF.ovn.mac_binding_age_threshold)
+
+
+def is_broadcast_arps_to_all_routers_enabled():
+    return cfg.CONF.ovn.broadcast_arps_to_all_routers

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py

@@ -19,6 +19,7 @@ import inspect
 import threading
 
 from futurist import periodics
+from neutron_lib.api.definitions import external_net
 from neutron_lib.api.definitions import portbindings
 from neutron_lib.api.definitions import provider_net as pnet
 from neutron_lib import constants as n_const
@@ -1227,6 +1228,39 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
 
         raise periodics.NeverAgain()
 
+    @has_lock_periodic(spacing=600, run_immediately=True)
+    def check_network_broadcast_arps_to_all_routers(self):
+        """Check the broadcast-arps-to-all-routers config
+
+        Ensure that the broadcast-arps-to-all-routers is set accordingly
+        to the ML2/OVN configuration option.
+        """
+        context = n_context.get_admin_context()
+        networks = self._ovn_client._plugin.get_networks(
+            context, filters={external_net.EXTERNAL: [True]})
+        cmds = []
+        for net in networks:
+            ls_name = utils.ovn_name(net['id'])
+            ls = self._nb_idl.get_lswitch(ls_name)
+            broadcast_value = ls.other_config.get(
+                ovn_const.LS_OPTIONS_BROADCAST_ARPS_ROUTERS)
+            expected_broadcast_value = ('true'
+                if ovn_conf.is_broadcast_arps_to_all_routers_enabled() else
+                'false')
+            # Assert the config value is the right one
+            if broadcast_value == expected_broadcast_value:
+                continue
+            # If not, set the right value
+            other_config = {ovn_const.LS_OPTIONS_BROADCAST_ARPS_ROUTERS:
+                            expected_broadcast_value}
+            cmds.append(self._nb_idl.db_set('Logical_Switch', ls_name,
+                                            ('other_config', other_config)))
+        if cmds:
+            with self._nb_idl.transaction(check_error=True) as txn:
+                for cmd in cmds:
+                    txn.add(cmd)
+        raise periodics.NeverAgain()
+
 
 class HashRingHealthCheckPeriodics(object):
 

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -2060,6 +2060,11 @@ class OVNClient(object):
         if utils.is_provider_network(network):
             params['other_config'][ovn_const.LS_OPTIONS_FDB_AGE_THRESHOLD] = (
                 ovn_conf.get_fdb_age_threshold())
+        if utils.is_external_network(network):
+            params['other_config'][
+                ovn_const.LS_OPTIONS_BROADCAST_ARPS_ROUTERS] = ('true'
+                if ovn_conf.is_broadcast_arps_to_all_routers_enabled() else
+                'false')
         return params
 
     def create_network(self, context, network):

neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py

@@ -1235,6 +1235,30 @@ class TestMaintenance(_TestMaintenanceHelper):
         self.assertEqual(net1[provnet_apidef.NETWORK_TYPE],
                          ls.external_ids.get(ovn_const.OVN_NETTYPE_EXT_ID_KEY))
 
+    def test_check_network_broadcast_arps_to_all_routers(self):
+        net = self._create_network('net', external=True)
+        ls = self.nb_api.get_lswitch(utils.ovn_name(net['id']))
+
+        self.assertEqual(
+            'true',
+            ls.other_config.get(ovn_const.LS_OPTIONS_BROADCAST_ARPS_ROUTERS))
+
+        # Change the value of the configuration
+        cfg.CONF.set_override(
+            'broadcast_arps_to_all_routers', False, group='ovn')
+
+        # Call the maintenance task and check that the value has been
+        # updated in the Logical Switch
+        self.assertRaises(
+            periodics.NeverAgain,
+            self.maint.check_network_broadcast_arps_to_all_routers)
+
+        ls = self.nb_api.get_lswitch(utils.ovn_name(net['id']))
+
+        self.assertEqual(
+            'false',
+            ls.other_config.get(ovn_const.LS_OPTIONS_BROADCAST_ARPS_ROUTERS))
+
 
 class TestLogMaintenance(_TestMaintenanceHelper,
                          test_log_driver.LogApiTestCaseBase):

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py

@@ -16,6 +16,7 @@
 from unittest import mock
 
 from futurist import periodics
+from neutron_lib.api.definitions import external_net
 from neutron_lib.api.definitions import portbindings
 from neutron_lib import constants as n_const
 from neutron_lib import context
@@ -1129,3 +1130,35 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight,
             utils.ovn_name('lr-id-b'),
             lrb_nat['uuid'],
             gateway_port=lrp.uuid)
+
+    def test_check_network_broadcast_arps_to_all_routers(self):
+        cfg.CONF.set_override('broadcast_arps_to_all_routers', 'true',
+                              group='ovn')
+        networks = [{'id': 'foo', external_net.EXTERNAL: True}]
+        self.fake_ovn_client._plugin.get_networks.return_value = networks
+        fake_ls = mock.Mock(other_config={})
+        self.fake_ovn_client._nb_idl.get_lswitch.return_value = fake_ls
+
+        self.assertRaises(
+            periodics.NeverAgain,
+            self.periodic.check_network_broadcast_arps_to_all_routers)
+
+        self.fake_ovn_client._nb_idl.db_set.assert_called_once_with(
+            'Logical_Switch', 'neutron-foo', ('other_config',
+            {constants.LS_OPTIONS_BROADCAST_ARPS_ROUTERS: 'true'}))
+
+    def test_check_network_broadcast_arps_to_all_routers_already_set(self):
+        cfg.CONF.set_override('broadcast_arps_to_all_routers', 'false',
+                              group='ovn')
+        networks = [{'id': 'foo', external_net.EXTERNAL: True}]
+        self.fake_ovn_client._plugin.get_networks.return_value = networks
+        fake_ls = mock.Mock(other_config={
+            constants.LS_OPTIONS_BROADCAST_ARPS_ROUTERS: 'false'})
+        self.fake_ovn_client._nb_idl.get_lswitch.return_value = fake_ls
+
+        self.assertRaises(
+            periodics.NeverAgain,
+            self.periodic.check_network_broadcast_arps_to_all_routers)
+
+        # Assert there was no transactions because the value was already set
+        self.fake_ovn_client._nb_idl.db_set.assert_not_called()

releasenotes/notes/broadcast-arp-to-all-routers-a9b1b997549d8b2f.yaml

@@ -0,0 +1,12 @@
+---
+features:
+  - |
+    Added a new configuration option called
+    ``broadcast_arps_to_all_routers`` to the ``[ovn]`` config section.
+    This option is responsible for configuring the external networks with
+    the ``broadcast-arps-to-all-routers`` config option available in
+    OVN 23.06 and newer. By enabling this option (default) OVN will flood
+    ARP requests to all attached ports on a network. If disabled, ARP
+    requests are only sent to routers on that network if the target MAC
+    address matches. ARP requests that do not match a router will only
+    be forwarded to non-router ports.