From 80c521c6d86be2f83cf6e41916083bf86cd51386 Mon Sep 17 00:00:00 2001 From: Carl Baldwin Date: Thu, 16 Jan 2014 21:12:23 +0000 Subject: [PATCH] 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 --- neutron/agent/l3_agent.py | 26 +++++++++------------- neutron/agent/linux/interface.py | 17 ++++++++------ neutron/tests/unit/test_l3_agent.py | 8 +++++++ neutron/tests/unit/test_linux_interface.py | 2 +- 4 files changed, 29 insertions(+), 24 deletions(-) diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index d611108f3..eb06b380b 100644 --- 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 c763b03a3..c3a97ecb2 100644 --- 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 c68436247..6df43edae 100644 --- 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 09bf8df27..7c84623fc 100644 --- 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', -- 2.45.2