From cbc737aa18ecbcee18a56e77cc9de2d327f57130 Mon Sep 17 00:00:00 2001
From: Oleg Bondarev <obondarev@mirantis.com>
Date: Thu, 17 Dec 2015 16:06:29 +0300
Subject: [PATCH] Optimize get_ports_on_host_by_subnet() dvr rpc handler

Make one db query to get all needed ports.
No need to fetch all ports on the subnet and iterate through
them.

Change-Id: I76a8a75905a7ec4c202583f01b62f2aefb433026
---
 neutron/db/dvr_mac_db.py                 | 35 ++++++++++++------------
 neutron/tests/unit/db/test_dvr_mac_db.py | 32 ++++++++++++++++++++++
 2 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/neutron/db/dvr_mac_db.py b/neutron/db/dvr_mac_db.py
index 952125d38a7..d43ddc14d42 100644
--- a/neutron/db/dvr_mac_db.py
+++ b/neutron/db/dvr_mac_db.py
@@ -18,12 +18,15 @@ from oslo_db import exception as db_exc
 from oslo_log import helpers as log_helpers
 from oslo_log import log as logging
 import sqlalchemy as sa
+from sqlalchemy import or_
 from sqlalchemy.orm import exc
 
 from neutron._i18n import _, _LE
+from neutron.common import constants
 from neutron.common import exceptions as n_exc
 from neutron.common import utils
 from neutron.db import model_base
+from neutron.db import models_v2
 from neutron.extensions import dvr as ext_dvr
 from neutron.extensions import portbindings
 from neutron import manager
@@ -134,26 +137,24 @@ class DVRDbMixin(ext_dvr.DVRMacAddressPluginBase):
         :param subnet: subnet id to match and extract ports of interest
         :returns list -- Ports on the given subnet in the input host
         """
-        # FIXME(vivek, salv-orlando): improve this query by adding the
-        # capability of filtering by binding:host_id
-        ports_by_host = []
-        filter = {'fixed_ips': {'subnet_id': [subnet]}}
-        ports = self.plugin.get_ports(context, filters=filter)
-        LOG.debug("List of Ports on subnet %(subnet)s at host %(host)s "
-                  "received as %(ports)s",
-                  {'subnet': subnet, 'host': host, 'ports': ports})
-        for port in ports:
-            device_owner = port['device_owner']
-            if (utils.is_dvr_serviced(device_owner)):
-                if port[portbindings.HOST_ID] == host:
-                    port_dict = self.plugin._make_port_dict(port,
-                        process_extensions=False)
-                    ports_by_host.append(port_dict)
+        filters = {'fixed_ips': {'subnet_id': [subnet]},
+                   portbindings.HOST_ID: [host]}
+        ports_query = self.plugin._get_ports_query(context, filters=filters)
+        owner_filter = or_(
+            models_v2.Port.device_owner.startswith(
+                constants.DEVICE_OWNER_COMPUTE_PREFIX),
+            models_v2.Port.device_owner.in_(
+                utils.get_other_dvr_serviced_device_owners()))
+        ports_query = ports_query.filter(owner_filter)
+        ports = [
+            self.plugin._make_port_dict(port, process_extensions=False)
+            for port in ports_query.all()
+        ]
         LOG.debug("Returning list of dvr serviced ports on host %(host)s"
                   " for subnet %(subnet)s ports %(ports)s",
                   {'host': host, 'subnet': subnet,
-                   'ports': ports_by_host})
-        return ports_by_host
+                   'ports': ports})
+        return ports
 
     @log_helpers.log_method_call
     def get_subnet_for_dvr(self, context, subnet, fixed_ips=None):
diff --git a/neutron/tests/unit/db/test_dvr_mac_db.py b/neutron/tests/unit/db/test_dvr_mac_db.py
index c827aeea920..e7c3e54c770 100644
--- a/neutron/tests/unit/db/test_dvr_mac_db.py
+++ b/neutron/tests/unit/db/test_dvr_mac_db.py
@@ -16,9 +16,11 @@
 import mock
 from oslo_config import cfg
 
+from neutron.common import constants
 from neutron import context
 from neutron.db import dvr_mac_db
 from neutron.extensions import dvr
+from neutron.extensions import portbindings
 from neutron.tests.unit.plugins.ml2 import test_plugin
 
 
@@ -127,3 +129,33 @@ class DvrDbMixinTestCase(test_plugin.Ml2PluginV2TestCase):
                     self.ctx, subnet['subnet']['id'], fixed_ips)
                 self.assertEqual(gw_port['port']['mac_address'],
                                  dvr_subnet['gateway_mac'])
+
+    def test_get_ports_on_host_by_subnet(self):
+        HOST = 'host1'
+        host_arg = {portbindings.HOST_ID: HOST}
+        arg_list = (portbindings.HOST_ID,)
+        with self.subnet() as subnet,\
+                self.port(subnet=subnet,
+                          device_owner=constants.DEVICE_OWNER_COMPUTE_PREFIX,
+                          arg_list=arg_list, **host_arg) as compute_port,\
+                self.port(subnet=subnet,
+                          device_owner=constants.DEVICE_OWNER_DHCP,
+                          arg_list=arg_list, **host_arg) as dhcp_port,\
+                self.port(subnet=subnet,
+                          device_owner=constants.DEVICE_OWNER_LOADBALANCER,
+                          arg_list=arg_list, **host_arg) as lb_port,\
+                self.port(device_owner=constants.DEVICE_OWNER_COMPUTE_PREFIX,
+                          arg_list=arg_list, **host_arg),\
+                self.port(subnet=subnet,
+                          device_owner=constants.DEVICE_OWNER_COMPUTE_PREFIX,
+                          arg_list=arg_list,
+                          **{portbindings.HOST_ID: 'other'}),\
+                self.port(subnet=subnet,
+                          device_owner=constants.DEVICE_OWNER_NETWORK_PREFIX,
+                          arg_list=arg_list, **host_arg):
+            expected_ids = [port['port']['id'] for port in
+                            [compute_port, dhcp_port, lb_port]]
+            dvr_ports = self.mixin.get_ports_on_host_by_subnet(
+                self.ctx, HOST, subnet['subnet']['id'])
+            self.assertEqual(len(expected_ids), len(dvr_ports))
+            self.assertEqual(expected_ids, [port['id'] for port in dvr_ports])