openstack
/
neutron
Code Issues Proposed changes

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
This commit is contained in:
Carl Baldwin 2014-01-16 21:12:23 +00:00
parent fe76c155fd
commit 2541ff7cad
4 changed files with 29 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
neutron/agent/l3_agent.py
View File

@ -602,15 +602,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
@ -639,13 +636,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 = []

17
neutron/agent/linux/interface.py
View File

@ -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."""

8
neutron/tests/unit/test_l3_agent.py
View File

@ -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(

2
neutron/tests/unit/test_linux_interface.py
View File

@ -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',