Merge "Ensure traffic is not centralized if DVR is enabled" into stable/2023.1

Author: Zuul

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

@@ -1052,7 +1052,7 @@ def get_workers(self):
         # See doc/source/design/ovn_worker.rst for more details.
         return [worker.MaintenanceWorker()]
 
-    def _update_dnat_entry_if_needed(self, port_id, up=True):
+    def _update_dnat_entry_if_needed(self, port_id):
         """Update DNAT entry if using distributed floating ips."""
         if not self.nb_ovn:
             self.nb_ovn = self._ovn_client._nb_idl
@@ -1073,9 +1073,9 @@ def _update_dnat_entry_if_needed(self, port_id, up=True):
                                 {ovn_const.OVN_FIP_EXT_MAC_KEY:
                                  nat['external_mac']})).execute()
 
-        if up and ovn_conf.is_ovn_distributed_floating_ip():
-            mac = nat['external_ids'][ovn_const.OVN_FIP_EXT_MAC_KEY]
-            if nat['external_mac'] != mac:
+        if ovn_conf.is_ovn_distributed_floating_ip():
+            mac = nat['external_ids'].get(ovn_const.OVN_FIP_EXT_MAC_KEY)
+            if mac and nat['external_mac'] != mac:
                 LOG.debug("Setting external_mac of port %s to %s",
                           port_id, mac)
                 self.nb_ovn.db_set(
@@ -1141,7 +1141,7 @@ def set_port_status_down(self, port_id):
         # to prevent another entity from bypassing the block with its own
         # port status update.
         LOG.info("OVN reports status down for port: %s", port_id)
-        self._update_dnat_entry_if_needed(port_id, False)
+        self._update_dnat_entry_if_needed(port_id)
         admin_context = n_context.get_admin_context()
         try:
             db_port = ml2_db.get_port(admin_context, port_id)

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

@@ -1124,7 +1124,7 @@ def _test_set_port_status_down(self, is_compute_port=False):
                 resources.PORT,
                 provisioning_blocks.L2_AGENT_ENTITY
             )
-            ude.assert_called_once_with(port1['port']['id'], False)
+            ude.assert_called_once_with(port1['port']['id'])
 
             # If the port does NOT bellong to compute, do not notify Nova
             # about it's status changes
@@ -1174,7 +1174,7 @@ def test_set_port_status_concurrent_delete(self):
                 resources.PORT,
                 provisioning_blocks.L2_AGENT_ENTITY
             )
-            ude.assert_called_once_with(port1['port']['id'], False)
+            ude.assert_called_once_with(port1['port']['id'])
 
     def test_bind_port_unsupported_vnic_type(self):
         fake_port = fakes.FakePort.create_one_port(
@@ -2374,9 +2374,10 @@ def test_agent_with_nb_cfg_timestamp_not_timeout(self):
         self.assertTrue(agent.alive, "Agent of type %s alive=%s" % (
             agent.agent_type, agent.alive))
 
-    def _test__update_dnat_entry_if_needed(self, up=True):
-        ovn_conf.cfg.CONF.set_override(
-            'enable_distributed_floating_ip', True, group='ovn')
+    def _test__update_dnat_entry_if_needed(self, dvr=True):
+        if dvr:
+            ovn_conf.cfg.CONF.set_override(
+                'enable_distributed_floating_ip', True, group='ovn')
         port_id = 'fake-port-id'
         fake_ext_mac_key = 'fake-ext-mac-key'
         fake_nat_uuid = uuidutils.generate_uuid()
@@ -2389,22 +2390,24 @@ def _test__update_dnat_entry_if_needed(self, up=True):
         fake_db_find.execute.return_value = [nat_row]
         self.nb_ovn.db_find.return_value = fake_db_find
 
-        self.mech_driver._update_dnat_entry_if_needed(port_id, up=up)
+        self.mech_driver._update_dnat_entry_if_needed(port_id)
 
-        if up:
+        if dvr:
             # Assert that we are setting the external_mac in the NAT table
             self.nb_ovn.db_set.assert_called_once_with(
                 'NAT', fake_nat_uuid, ('external_mac', fake_ext_mac_key))
+            self.nb_ovn.db_clear.assert_not_called()
         else:
+            self.nb_ovn.db_set.assert_not_called()
             # Assert that we are cleaning the external_mac from the NAT table
             self.nb_ovn.db_clear.assert_called_once_with(
                 'NAT', fake_nat_uuid, 'external_mac')
 
-    def test__update_dnat_entry_if_needed_up(self):
+    def test__update_dnat_entry_if_needed_dvr(self):
         self._test__update_dnat_entry_if_needed()
 
-    def test__update_dnat_entry_if_needed_down(self):
-        self._test__update_dnat_entry_if_needed(up=False)
+    def test__update_dnat_entry_if_needed_no_dvr(self):
+        self._test__update_dnat_entry_if_needed(dvr=False)
 
     @mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.'
                 'ovn_client.OVNClient._get_router_ports')

releasenotes/notes/dvr-external-mac-934409413e515eb2.yaml

@@ -0,0 +1,10 @@
+---
+other:
+  - |
+    The external_mac entry in the NAT table is used to distribute/centralize
+    the traffic to the FIPs. When there is an external_mac set the traffic
+    is distributed (DVR). When it is empty it is centralized through the
+    gateway port (no DVR). Upon port status transition to down, the
+    external_mac was removed regardless of DVR being enabled or not, leading
+    to centralize the FIP traffic for DVR -- though it was for down ports that
+    won't accept traffic anyway.