Avoid unnecessarily checking the existence of a device

Plugging a device usually involves checking for the existence of the
device twice, once before calling plug and once after.  It turns out
that these calls are expensive, often taking a half second or more
each.  For that reason, it is worth the effort to make sure we check
only once.

The device driver is now responsible for cleanly plugging/unplugging
the device without knowing whether it exists or not.  Pushing this
responsibility to the device driver allows implementing it more
efficiently in terms of calls made out to the operating system.

This is targetted at the neutron-tempest-parallel bp because it shaves
time off the time to set up a router, something that hinders parallel
performance.

Change-Id: I391fafe68b76e1c620d2b25e8613ba507fd25dfd
Partial-Bug: #1287824

diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py
index d611108f38ff20c92098dd23acc41816d8e35ad3..eb06b380bbc3a5b7e7e9880df42ce5dee7c4050b 100644 (file)
--- a/neutron/agent/l3_agent.py
+++ b/neutron/agent/l3_agent.py
@@ -609,15 +609,12 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
     def external_gateway_added(self, ri, ex_gw_port,
                                interface_name, internal_cidrs):
 
-        if not ip_lib.device_exists(interface_name,
-                                    root_helper=self.root_helper,
-                                    namespace=ri.ns_name()):
-            self.driver.plug(ex_gw_port['network_id'],
-                             ex_gw_port['id'], interface_name,
-                             ex_gw_port['mac_address'],
-                             bridge=self.conf.external_network_bridge,
-                             namespace=ri.ns_name(),
-                             prefix=EXTERNAL_DEV_PREFIX)
+        self.driver.plug(ex_gw_port['network_id'],
+                         ex_gw_port['id'], interface_name,
+                         ex_gw_port['mac_address'],
+                         bridge=self.conf.external_network_bridge,
+                         namespace=ri.ns_name(),
+                         prefix=EXTERNAL_DEV_PREFIX)
 
         # Compute a list of addresses this router is supposed to have.
         # This avoids unnecessarily removing those addresses and
@@ -646,13 +643,10 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
     def external_gateway_removed(self, ri, ex_gw_port,
                                  interface_name, internal_cidrs):
 
-        if ip_lib.device_exists(interface_name,
-                                root_helper=self.root_helper,
-                                namespace=ri.ns_name()):
-            self.driver.unplug(interface_name,
-                               bridge=self.conf.external_network_bridge,
-                               namespace=ri.ns_name(),
-                               prefix=EXTERNAL_DEV_PREFIX)
+        self.driver.unplug(interface_name,
+                           bridge=self.conf.external_network_bridge,
+                           namespace=ri.ns_name(),
+                           prefix=EXTERNAL_DEV_PREFIX)
 
     def metadata_filter_rules(self):
         rules = []

diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py
index c763b03a30d62771f73cb6be5ef979d64b3e12ac..c3a97ecb209613f5f5d26ddf724cc3edb5e938ee 100644 (file)
--- a/neutron/agent/linux/interface.py
+++ b/neutron/agent/linux/interface.py
@@ -162,12 +162,12 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
         if not bridge:
             bridge = self.conf.ovs_integration_bridge
 
-        self.check_bridge_exists(bridge)
-
         if not ip_lib.device_exists(device_name,
                                     self.root_helper,
                                     namespace=namespace):
 
+            self.check_bridge_exists(bridge)
+
             ip = ip_lib.IPWrapper(self.root_helper)
             tap_name = self._get_tap_name(device_name, prefix)
 
@@ -199,7 +199,7 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
             if self.conf.ovs_use_veth:
                 root_dev.link.set_up()
         else:
-            LOG.warn(_("Device %s already exists"), device_name)
+            LOG.info(_("Device %s already exists"), device_name)
 
     def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         """Unplug the interface."""
@@ -253,14 +253,17 @@ class MidonetInterfaceDriver(LinuxInterfaceDriver):
             utils.execute(cmd, self.root_helper)
 
         else:
-            LOG.warn(_("Device %s already exists"), device_name)
+            LOG.info(_("Device %s already exists"), device_name)
 
     def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         # the port will be deleted by the dhcp agent that will call the plugin
         device = ip_lib.IPDevice(device_name,
                                  self.root_helper,
                                  namespace)
-        device.link.delete()
+        try:
+            device.link.delete()
+        except RuntimeError:
+            LOG.error(_("Failed unplugging interface '%s'"), device_name)
         LOG.debug(_("Unplugged interface '%s'"), device_name)
 
         ip_lib.IPWrapper(
@@ -312,7 +315,7 @@ class IVSInterfaceDriver(LinuxInterfaceDriver):
             ns_dev.link.set_up()
             root_dev.link.set_up()
         else:
-            LOG.warn(_("Device %s already exists"), device_name)
+            LOG.info(_("Device %s already exists"), device_name)
 
     def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         """Unplug the interface."""
@@ -361,7 +364,7 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver):
             ns_veth.link.set_up()
 
         else:
-            LOG.warn(_("Device %s already exists"), device_name)
+            LOG.info(_("Device %s already exists"), device_name)
 
     def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         """Unplug the interface."""

diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py
index c684362472e4cec686b470d9f86e11589d977593..6df43edaef129222d63e3f2386f96bd9b877cc3d 100644 (file)
--- a/neutron/tests/unit/test_l3_agent.py
+++ b/neutron/tests/unit/test_l3_agent.py
@@ -388,6 +388,7 @@ class TestBasicRouterOperations(base.BaseTestCase):
         agent.process_router_floating_ip_nat_rules = mock.Mock()
         agent.process_router_floating_ip_addresses.return_value = {
             fake_fip_id: 'ACTIVE'}
+        agent.external_gateway_added = mock.Mock()
         router = self._prepare_router_data()
         fake_floatingips1 = {'floatingips': [
             {'id': fake_fip_id,
@@ -577,6 +578,7 @@ class TestBasicRouterOperations(base.BaseTestCase):
         router = self._prepare_router_data(enable_snat=True)
         ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper,
                                  self.conf.use_namespaces, router=router)
+        agent.external_gateway_added = mock.Mock()
         # Process with NAT
         agent.process_router(ri)
         orig_nat_rules = ri.iptables_manager.ipv4['nat'].rules[:]
@@ -598,6 +600,7 @@ class TestBasicRouterOperations(base.BaseTestCase):
         router = self._prepare_router_data(enable_snat=False)
         ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper,
                                  self.conf.use_namespaces, router=router)
+        agent.external_gateway_added = mock.Mock()
         # Process without NAT
         agent.process_router(ri)
         orig_nat_rules = ri.iptables_manager.ipv4['nat'].rules[:]
@@ -619,6 +622,7 @@ class TestBasicRouterOperations(base.BaseTestCase):
         router = self._prepare_router_data()
         ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper,
                                  self.conf.use_namespaces, router=router)
+        agent.external_gateway_added = mock.Mock()
         # Process with NAT
         agent.process_router(ri)
         orig_nat_rules = ri.iptables_manager.ipv4['nat'].rules[:]
@@ -648,6 +652,7 @@ class TestBasicRouterOperations(base.BaseTestCase):
         router = self._prepare_router_data(num_internal_ports=2)
         ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper,
                                  self.conf.use_namespaces, router=router)
+        agent.external_gateway_added = mock.Mock()
         # Process with NAT
         agent.process_router(ri)
         orig_nat_rules = ri.iptables_manager.ipv4['nat'].rules[:]
@@ -669,6 +674,7 @@ class TestBasicRouterOperations(base.BaseTestCase):
         router = self._prepare_router_data()
         ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper,
                                  self.conf.use_namespaces, router=router)
+        agent.external_gateway_added = mock.Mock()
         with mock.patch.object(
                 l3_agent.L3NATAgent,
                 'internal_network_added') as internal_network_added:
@@ -694,6 +700,7 @@ class TestBasicRouterOperations(base.BaseTestCase):
         router = self._prepare_router_data()
         ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper,
                                  self.conf.use_namespaces, router=router)
+        agent.external_gateway_added = mock.Mock()
         # add an internal port
         agent.process_router(ri)
 
@@ -734,6 +741,7 @@ class TestBasicRouterOperations(base.BaseTestCase):
 
             ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper,
                                      self.conf.use_namespaces, router=router)
+            agent.external_gateway_added = mock.Mock()
             agent.process_router(ri)
             # Assess the call for putting the floating IP up was performed
             mock_update_fip_status.assert_called_once_with(

diff --git a/neutron/tests/unit/test_linux_interface.py b/neutron/tests/unit/test_linux_interface.py
index 09bf8df27fdc9a6c00835a393b8e905355eea1cd..7c84623fc0071e449cc1aa1912302ccfaa10395a 100644 (file)
--- a/neutron/tests/unit/test_linux_interface.py
+++ b/neutron/tests/unit/test_linux_interface.py
@@ -336,7 +336,7 @@ class TestBridgeInterfaceDriver(TestBase):
 
     def test_plug_dev_exists(self):
         self.device_exists.return_value = True
-        with mock.patch('neutron.agent.linux.interface.LOG.warn') as log:
+        with mock.patch('neutron.agent.linux.interface.LOG.info') as log:
             br = interface.BridgeInterfaceDriver(self.conf)
             br.plug('01234567-1234-1234-99',
                     'port-1234',