Merge "Optimize get_local_ports_to_ofport_mapping"

dragonflow/controller/df_local_controller.py

@@ -216,8 +216,6 @@ class DfLocalController(object):
 
     def _logical_port_process(self, lport, original_lport=None):
         chassis = lport.get_chassis()
-        chassis_to_ofport, lport_to_ofport = (
-            self.vswitch_api.get_local_ports_to_ofport_mapping())
         local_network_id = self.get_network_id(
             lport.get_lswitch_id(),
         )
@@ -234,8 +232,8 @@ class DfLocalController(object):
 
         if chassis == self.chassis_name:
             lport.set_external_value('is_local', True)
-            ofport = lport_to_ofport.get(lport.get_id(), 0)
-            if ofport != 0:
+            ofport = self.vswitch_api.get_port_ofport_by_id(lport.get_id())
+            if ofport:
                 lport.set_external_value('ofport', ofport)
                 self.db_store.set_port(lport.get_id(), lport, True)
                 if original_lport is None:
@@ -254,8 +252,8 @@ class DfLocalController(object):
                          str(lport))
         else:
             lport.set_external_value('is_local', False)
-            ofport = chassis_to_ofport.get(chassis, 0)
-            if ofport != 0:
+            ofport = self.vswitch_api.get_chassis_ofport(chassis)
+            if ofport:
                 lport.set_external_value('ofport', ofport)
                 self.db_store.set_port(lport.get_id(), lport, False)
                 if original_lport is None:

dragonflow/db/drivers/ovsdb_vswitch_impl.py

@@ -20,6 +20,7 @@ from neutron.agent.ovsdb.native import connection
 from neutron.agent.ovsdb.native import helpers
 from neutron.agent.ovsdb.native import idlutils
 from oslo_config import cfg
+from oslo_log import log
 from ovs.db import idl
 from ovs import poller
 from ovs import vlog
@@ -27,9 +28,15 @@ import retrying
 import six
 import threading
 
+from dragonflow._i18n import _LW
 from dragonflow.common import constants
 from dragonflow.db import api_vswitch
 
+LOG = log.getLogger(__name__)
+
+OFPORT_RANGE_MIN = 1
+OFPORT_RANGE_MAX = 65533
+
 
 ovsdb_monitor_table_filter_default = {
     'Interface': [
@@ -185,6 +192,9 @@ class OvsdbSwitchApi(api_vswitch.SwitchApi):
         return self.ovsdb.db_get(table, record, column).execute(
             check_error=check_error, log_errors=log_errors)
 
+    def _get_bridge_for_iface(self, iface_name):
+        return self.ovsdb.iface_to_br(iface_name).execute()
+
     def set_controller(self, bridge, targets):
         self.ovsdb.set_controller(bridge, targets).execute()
 
@@ -217,28 +227,40 @@ class OvsdbSwitchApi(api_vswitch.SwitchApi):
         self.ovsdb.del_port(switch_port.get_name(),
                             self.integration_bridge).execute()
 
-    def get_local_ports_to_ofport_mapping(self):
-        lport_to_ofport = {}
-        chassis_to_ofport = {}
-        ports = self.ovsdb.get_bridge_ports(self.integration_bridge).execute()
-        for port in ports:
-            chassis_id = port.external_ids.get('df-chassis-id')
-            for interface in port.interfaces:
-                if interface.ofport is None:
-                    # TODO(gsagie) log error
-                    continue
-                ofport = interface.ofport[0]
-                if ofport < 1 or ofport > 65533:
-                    # TODO(gsagie) log error
-                    continue
-                if chassis_id is not None:
-                    chassis_to_ofport[chassis_id] = ofport
-                else:
-                    ifaceid = interface.external_ids.get('iface-id')
-                    if ifaceid is not None:
-                        lport_to_ofport[ifaceid] = ofport
+    @staticmethod
+    def _check_ofport(port_name, ofport):
+        if ofport is None:
+            LOG.warning(_LW("Can't find ofport for port %s."), port_name)
+            return False
+        if ofport < OFPORT_RANGE_MIN or ofport > OFPORT_RANGE_MAX:
+            LOG.warning(_LW("ofport %(ofport)s for port %(port)s is invalid."),
+                        {'ofport': ofport, 'port': port_name})
+            return False
 
-        return chassis_to_ofport, lport_to_ofport
+        return True
+
+    def get_chassis_ofport(self, chassis_id):
+        # TODO(xiaohhui): Can we just call get_port_ofport('df-'+chassis_id)?
+        ports = self.ovsdb.db_find(
+            'Port', ('external_ids', '=', {'df-chassis-id': chassis_id}),
+            columns=['external_ids', 'name']).execute()
+        for port in ports:
+            ofport = self.get_port_ofport(port['name'])
+            if self._check_ofport(port['name'], ofport):
+                return ofport
+
+    def get_port_ofport_by_id(self, port_id):
+        ifaces = self.ovsdb.db_find(
+            'Interface', ('external_ids', '=', {'iface-id': port_id}),
+            columns=['external_ids', 'name', 'ofport']).execute()
+        for iface in ifaces:
+            if (self.integration_bridge !=
+                    self._get_bridge_for_iface(iface['name'])):
+                # iface-id is the port id in neutron, the same neutron port
+                # might create multiple interfaces in different bridges
+                continue
+            if self._check_ofport(iface['name'], iface['ofport']):
+                return iface['ofport']
 
     def create_patch_port(self, bridge, port, remote_name):
         self.ovsdb.add_br(bridge, datapath_type='system').execute()

dragonflow/tests/unit/test_app_base.py

@@ -44,8 +44,6 @@ class DFAppTestBase(tests_base.BaseTestCase):
         self.open_flow_app.load(self.controller.open_flow_app, **kwargs)
         self.controller.topology = topology.Topology(self.controller, False)
 
-        self.vswitch_api.get_local_ports_to_ofport_mapping.return_value = (
-            {}, {fake_local_port1.get_id(): 2})
         # Add basic network topology
         self.controller.logical_switch_updated(fake_logic_switch1)
         self.controller.logical_switch_updated(fake_external_switch1)