Add support for OVN MAC_Binding aging

OVN added support for aging out MAC_Binding entries [1][2].
Without this feature, the MAC_Bindings table can grow indefinitely.

[1] 1a947dd307
[2] cecac71c0e

Closes-Bug: 2033932
Change-Id: I91070ad6addb30ffdedba5d561984d2f6626e2b7

neutron/common/ovn/constants.py

@@ -405,6 +405,8 @@ LSP_OPTIONS_MCAST_FLOOD = 'mcast_flood'
 LSP_OPTIONS_QOS_MIN_RATE = 'qos_min_rate'
 LSP_OPTIONS_LOCALNET_LEARN_FDB = 'localnet_learn_fdb'
 
+LR_OPTIONS_MAC_AGE_LIMIT = 'mac_binding_age_threshold'
+
 LRP_OPTIONS_RESIDE_REDIR_CH = 'reside-on-redirect-chassis'
 LRP_OPTIONS_REDIRECT_TYPE = 'redirect-type'
 BRIDGE_REDIRECT_TYPE = "bridged"

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

@@ -232,6 +232,11 @@ ovn_opts = [
                help=_('The number of seconds to keep FDB entries in the OVN '
                       'DB. The value defaults to 0, which means disabled. '
                       'This is supported by OVN >= 23.09.')),
+    cfg.IntOpt('mac_binding_age_threshold',
+               min=0,
+               default=0,
+               help=_('The number of seconds to keep MAC_Binding entries in '
+                      'the OVN DB. 0 to disable aging.')),
 ]
 
 nb_global_opts = [
@@ -255,6 +260,14 @@ nb_global_opts = [
                       'is 0, which is unlimited. When the limit is reached, '
                       'the next batch removal is delayed by 5 seconds. '
                       'This is supported by OVN >= 23.09.')),
+    cfg.IntOpt('mac_binding_removal_limit',
+               min=0,
+               default=0,
+               help=_('MAC binding aging bulk removal limit. This limits how '
+                      'many entries can expire in a single transaction. '
+                      'The default is 0 which is unlimited. When the limit '
+                      'is reached, the next batch removal is delayed by '
+                      '5 seconds.')),
 ]
 
 
@@ -382,3 +395,8 @@ def get_fdb_age_threshold():
 
 def get_fdb_removal_limit():
     return str(cfg.CONF.ovn_nb_global.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)

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

@@ -182,6 +182,15 @@ class API(api.API, metaclass=abc.ABCMeta):
         :returns:            :class:`Command` with no result
         """
 
+    @abc.abstractmethod
+    def set_router_mac_age_limit(self, router=None):
+        """Set the OVN MAC_Binding age threshold
+
+        :param router: The name or UUID of a router, or None for all routers
+        :type router:  uuid.UUID or string or None
+        :returns:      :class:`Command` with no result
+        """
+
     @abc.abstractmethod
     def add_acl(self, lswitch, lport, **columns):
         """Create an ACL for a logical port.

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

@@ -472,6 +472,40 @@ class SetLRouterPortInLSwitchPortCommand(command.BaseCommand):
         setattr(port, 'addresses', self.lsp_address)
 
 
+class SetLRouterMacAgeLimitCommand(command.BaseCommand):
+    def __init__(self, api, router, threshold):
+        super().__init__(api)
+        self.router = router
+        self.threshold = str(threshold)  # Just in case an integer sneaks in
+
+    def run_idl(self, txn):
+        # Creating a Command object that iterates over the list of Routers
+        # from inside a transaction avoids the issue of doing two
+        # transactions: one for list_rows() and the other for setting the
+        # values on routers, which would allow routers to be added and removed
+        # between the two transactions.
+        if self.router is None:
+            routers = self.api.tables["Logical_Router"].rows.values()
+        else:
+            routers = [self.api.lookup("Logical_Router", self.router)]
+
+        for router in routers:
+            # It's not technically necessary to check the value before setting
+            # it as python-ovs is smart enough to avoid sending operations to
+            # the server that would result in no change. The overhead of
+            # setkey() though is > than the overhead of checking the value here
+            try:
+                if (router.options.get(ovn_const.LR_OPTIONS_MAC_AGE_LIMIT) ==
+                        self.threshold):
+                    continue
+            except AttributeError:
+                # The Logical_Router is newly created in this txn and has no
+                # "options" set yet, which the following setkey will rectify
+                pass
+            router.setkey("options", ovn_const.LR_OPTIONS_MAC_AGE_LIMIT,
+                          self.threshold)
+
+
 class AddACLCommand(command.BaseCommand):
     def __init__(self, api, lswitch, lport, **columns):
         super(AddACLCommand, self).__init__(api)

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

@@ -853,6 +853,11 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
         LOG.debug("Setting NB_Global options: %s", options)
         return self.db_set("NB_Global", ".", options=options)
 
+    def set_router_mac_age_limit(self, router=None):
+        # Set the MAC_Binding age limit on OVN Logical Routers
+        return cmd.SetLRouterMacAgeLimitCommand(
+            self, router, cfg.get_ovn_mac_binding_age_threshold())
+
 
 class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend):
     def __init__(self, connection):

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

@@ -832,6 +832,15 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
                     txn.add(cmd)
         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().
+    @has_lock_periodic(spacing=600, run_immediately=True)
+    def update_mac_aging_settings(self):
+        """Ensure that MAC_Binding aging options are set"""
+        with self._nb_idl.transaction(check_error=True) as txn:
+            txn.add(self._nb_idl.set_router_mac_age_limit())
+        raise periodics.NeverAgain()
+
     # TODO(fnordahl): Remove this in the B+3 cycle. This method removes the
     # now redundant  "external_ids:OVN_GW_NETWORK_EXT_ID_KEY" and
     # "external_ids:OVN_GW_PORT_EXT_ID_KEY" from to each router.

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

@@ -1346,7 +1346,9 @@ class OVNClient(object):
         lrouter_name = utils.ovn_name(router['id'])
         added_gw_port = None
         options = {'always_learn_from_arp_request': 'false',
-                   'dynamic_neigh_routers': 'true'}
+                   'dynamic_neigh_routers': 'true',
+                   ovn_const.LR_OPTIONS_MAC_AGE_LIMIT:
+                   ovn_conf.get_ovn_mac_binding_age_threshold()}
         with self._nb_idl.transaction(check_error=True) as txn:
             txn.add(self._nb_idl.create_lrouter(lrouter_name,
                                                 external_ids=external_ids,

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

@@ -383,6 +383,27 @@ class TestNbApi(BaseOvnIdlTest):
         lsp = self.nbapi.lookup('Logical_Switch_Port', lsp_name)
         self.assertEqual([], lsp.ha_chassis_group)
 
+    def test_set_router_mac_aging(self):
+        name = "aging_router1"
+        with self.nbapi.transaction(check_error=True) as txn:
+            r = txn.add(self.nbapi.lr_add(name))
+            txn.add(self.nbapi.set_router_mac_age_limit(router=name))
+        self.assertEqual(r.result.options[ovn_const.LR_OPTIONS_MAC_AGE_LIMIT],
+                         ovn_conf.get_ovn_mac_binding_age_threshold())
+
+    def test_set_router_mac_aging_all(self):
+        ovn_conf.cfg.CONF.set_override("mac_binding_age_threshold", 5,
+                                       group="ovn")
+        names = ["aging_router2", "aging_router3"]
+        with self.nbapi.transaction(check_error=True) as txn:
+            for name in names:
+                txn.add(self.nbapi.lr_add(name))
+            txn.add(self.nbapi.set_router_mac_age_limit())
+        for name in names:
+            r = self.nbapi.lookup("Logical_Router", name)
+            self.assertEqual(r.options[ovn_const.LR_OPTIONS_MAC_AGE_LIMIT],
+                             ovn_conf.get_ovn_mac_binding_age_threshold())
+
 
 class TestIgnoreConnectionTimeout(BaseOvnIdlTest):
     @classmethod

neutron/tests/functional/services/ovn_l3/test_plugin.py

@@ -26,6 +26,7 @@ from ovsdbapp.backend.ovs_idl import idlutils
 from neutron.common.ovn import constants as ovn_const
 from neutron.common.ovn import utils as ovn_utils
 from neutron.common import utils as n_utils
+from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
 from neutron.scheduler import l3_ovn_scheduler as l3_sched
 from neutron.tests.functional import base
 from neutron.tests.functional.resources.ovsdb import events
@@ -595,3 +596,12 @@ class TestRouter(base.TestOVNFunctionalBase):
                 'Logical_Router_Port'].rows.values():
             self.assertEqual(ovn_const.MAX_GW_CHASSIS,
                              len(row.gateway_chassis))
+
+    def test_set_router_mac_age_limit(self):
+        name = "macage_router1"
+        router = self._create_router(name)
+        lr_name = ovn_utils.ovn_name(router['id'])
+        options = self.nb_api.db_get(
+            "Logical_Router", lr_name, "options").execute(check_error=True)
+        self.assertEqual(ovn_conf.get_ovn_mac_binding_age_threshold(),
+                         options[ovn_const.LR_OPTIONS_MAC_AGE_LIMIT])

neutron/tests/unit/services/ovn_l3/test_plugin.py

@@ -555,8 +555,11 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
                         ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: ''}
         self.l3_inst._nb_ovn.create_lrouter.assert_called_once_with(
             'neutron-router-id', external_ids=external_ids,
-            enabled=True, options={'always_learn_from_arp_request': 'false',
-                                   'dynamic_neigh_routers': 'true'})
+            enabled=True,
+            options={'always_learn_from_arp_request': 'false',
+                     'dynamic_neigh_routers': 'true',
+                     ovn_const.LR_OPTIONS_MAC_AGE_LIMIT:
+                     config.get_ovn_mac_binding_age_threshold()})
         self.l3_inst._nb_ovn.add_lrouter_port.assert_called_once_with(
             **self.fake_ext_gw_port_assert)
         expected_calls = [

releasenotes/notes/support-ovn-mac-aging-8a555c608ccf8b04.yaml

@@ -0,0 +1,9 @@
+---
+features:
+  - |
+    MAC address aging in the OVN ML2 mech driver is now supported and can
+    be configured globally with the new ``[ovn] mac_binding_aging_threshold``
+    and ``[ovn_nb_global] mac_binding_removal_limit`` configuration
+    options. Setting the value per-router is not currently supported. This
+    feature is available in OVN versions >= 22.09.0+. Previous versions will
+    ignore the new options.