From 1d937e0d9ff4f7fc719c4dc3ca6f5a0b1c3dbce6 Mon Sep 17 00:00:00 2001
From: fumihiko kakuma <kakuma@valinux.co.jp>
Date: Mon, 14 Apr 2014 12:54:39 +0900
Subject: [PATCH] OFAgent: Avoid processing ports which are not yet ready

Port the following patch to OFAgent.
commit: 2702baed390d094b0eac07d0ae167ed236868d00
https://review.openstack.org/#/c/65838/

Closes-Bug: 1307745

Change-Id: Ie1fe0944fb541ba45ffb5b03747d5da289cede51
---
 .../ofagent/agent/ofa_neutron_agent.py        | 20 ++++++++++++++++---
 .../unit/ofagent/test_ofa_neutron_agent.py    | 15 ++++++++++++--
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py
index 8eeb7577b6..58e3df3c0b 100644
--- a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py
+++ b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py
@@ -648,7 +648,7 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
         if cur_tag != str(lvm.vlan):
             self.int_br.set_db_attribute("Port", port.port_name, "tag",
                                          str(lvm.vlan))
-            if int(port.ofport) != -1:
+            if port.ofport != -1:
                 match = self.int_br.ofparser.OFPMatch(in_port=port.ofport)
                 msg = self.int_br.ofparser.OFPFlowMod(
                     self.int_br.datapath,
@@ -990,6 +990,13 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
     def treat_vif_port(self, vif_port, port_id, network_id, network_type,
                        physical_network, segmentation_id, admin_state_up):
         if vif_port:
+            # When this function is called for a port, the port should have
+            # an OVS ofport configured, as only these ports were considered
+            # for being treated. If that does not happen, it is a potential
+            # error condition of which operators should be aware
+            if not vif_port.ofport:
+                LOG.warn(_("VIF port: %s has no ofport configured, and might "
+                           "not be able to transmit"), vif_port.vif_id)
             if admin_state_up:
                 self.port_bound(vif_port, network_id, network_type,
                                 physical_network, segmentation_id)
@@ -1060,6 +1067,14 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
         resync = False
         for device in devices:
             LOG.debug(_("Processing port %s"), device)
+            port = self.int_br.get_vif_port_by_id(device)
+            if not port:
+                # The port has disappeared and should not be processed
+                # There is no need to put the port DOWN in the plugin as
+                # it never went up in the first place
+                LOG.info(_("Port %s was not found on the integration bridge "
+                           "and will therefore not be processed"), device)
+                continue
             try:
                 details = self.plugin_rpc.get_device_details(self.context,
                                                              device,
@@ -1070,7 +1085,6 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
                           {'device': device, 'e': e})
                 resync = True
                 continue
-            port = self.int_br.get_vif_port_by_id(details['device'])
             if 'port_id' in details:
                 LOG.info(_("Port %(device)s updated. Details: %(details)s"),
                          {'device': device, 'details': details})
@@ -1093,7 +1107,7 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
                 LOG.info(_("Configuration for device %s completed."), device)
             else:
                 LOG.warn(_("Device %s not defined on plugin"), device)
-                if (port and int(port.ofport) != -1):
+                if (port and port.ofport != -1):
                     self.port_dead(port)
         return resync
 
diff --git a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py
index 4b2f5556ac..2fb4ea0318 100644
--- a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py
+++ b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py
@@ -366,8 +366,11 @@ class TestOFANeutronAgent(OFAAgentTestCase):
         self.assertEqual(expected, actual)
 
     def test_treat_devices_added_returns_true_for_missing_device(self):
-        with mock.patch.object(self.agent.plugin_rpc, 'get_device_details',
-                               side_effect=Exception()):
+        with contextlib.nested(
+            mock.patch.object(self.agent.plugin_rpc, 'get_device_details',
+                              side_effect=Exception()),
+            mock.patch.object(self.agent.int_br, 'get_vif_port_by_id',
+                              return_value=mock.Mock())):
             self.assertTrue(self.agent.treat_devices_added_or_updated([{}]))
 
     def _mock_treat_devices_added_updated(self, details, port, func_name):
@@ -402,6 +405,14 @@ class TestOFANeutronAgent(OFAAgentTestCase):
         self.assertTrue(self._mock_treat_devices_added_updated(
             mock.MagicMock(), port, 'port_dead'))
 
+    def test_treat_devices_added_does_not_process_missing_port(self):
+        with contextlib.nested(
+            mock.patch.object(self.agent.plugin_rpc, 'get_device_details'),
+            mock.patch.object(self.agent.int_br, 'get_vif_port_by_id',
+                              return_value=None)
+        ) as (get_dev_fn, get_vif_func):
+            self.assertFalse(get_dev_fn.called)
+
     def test_treat_devices_added_updated_updates_known_port(self):
         details = mock.MagicMock()
         details.__contains__.side_effect = lambda x: True