Avoid re-wiring ports unnecessarily

In the majority of cases a port_update notification pertains
a change in the properties affecting port filter, and does
not affect port wiring, ie: the local vlan tag.

This patch simply avoids doing port wiring/unwiring if the
local vlan tag did not change.
The extra overhead for the ovs-db get operation is offset
by the fact that get commands are generally faster than
set commands, and by avoiding executing the ovs-ofctl operation.

Partial-Bug: #1253993
Partially implements blueprint: neutron-tempest-parallel

Change-Id: Ia0bd2dc4e5a2634a4c863ff32ccc5cabe8e21337
This commit is contained in:
Salvatore Orlando 2013-12-20 16:59:05 -08:00 committed by Mark McClain
parent ec2ad649ab
commit 11990f75ed
3 changed files with 71 additions and 27 deletions

View File

@ -594,11 +594,13 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
physical_network, segmentation_id)
lvm = self.local_vlan_map[net_uuid]
lvm.vif_ports[port.vif_id] = port
self.int_br.set_db_attribute("Port", port.port_name, "tag",
str(lvm.vlan))
if int(port.ofport) != -1:
self.int_br.delete_flows(in_port=port.ofport)
# Do not bind a port if it's already bound
cur_tag = self.int_br.db_get_val("Port", port.port_name, "tag")
if cur_tag != str(lvm.vlan):
self.int_br.set_db_attribute("Port", port.port_name, "tag",
str(lvm.vlan))
if int(port.ofport) != -1:
self.int_br.delete_flows(in_port=port.ofport)
def port_unbound(self, vif_id, net_uuid=None):
'''Unbind port.
@ -628,9 +630,13 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
:param port: a ovs_lib.VifPort object.
'''
self.int_br.set_db_attribute("Port", port.port_name, "tag",
DEAD_VLAN_TAG)
self.int_br.add_flow(priority=2, in_port=port.ofport, actions="drop")
# Don't kill a port if it's already dead
cur_tag = self.int_br.db_get_val("Port", port.port_name, "tag")
if cur_tag != DEAD_VLAN_TAG:
self.int_br.set_db_attribute("Port", port.port_name, "tag",
DEAD_VLAN_TAG)
self.int_br.add_flow(priority=2, in_port=port.ofport,
actions="drop")
def setup_integration_br(self):
'''Setup the integration bridge.
@ -1024,8 +1030,6 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
# VIF wiring needs to be performed always for 'new' devices.
# For updated ports, re-wiring is not needed in most cases, but needs
# to be performed anyway when the admin state of a device is changed.
# TODO(salv-orlando): Optimize for avoiding unnecessary VIF
# processing for updated ports whose admin state is left unchanged
# A device might be both in the 'added' and 'updated'
# list at the same time; avoid processing it twice.
devices_added_updated = (port_info.get('added', set()) |

View File

@ -126,32 +126,70 @@ class TestOvsNeutronAgent(base.BaseTestCase):
self.agent.tun_br = mock.Mock()
self.agent.sg_agent = mock.Mock()
def _mock_port_bound(self, ofport=None):
def _mock_port_bound(self, ofport=None, new_local_vlan=None,
old_local_vlan=None):
port = mock.Mock()
port.ofport = ofport
net_uuid = 'my-net-uuid'
with mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.'
'set_db_attribute',
return_value=True):
with mock.patch.object(self.agent.int_br,
'delete_flows') as delete_flows_func:
self.agent.port_bound(port, net_uuid, 'local', None, None)
self.assertEqual(delete_flows_func.called, ofport != -1)
if old_local_vlan is not None:
self.agent.local_vlan_map[net_uuid] = (
ovs_neutron_agent.LocalVLANMapping(
old_local_vlan, None, None, None))
with contextlib.nested(
mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.'
'set_db_attribute', return_value=True),
mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.'
'db_get_val', return_value=str(old_local_vlan)),
mock.patch.object(self.agent.int_br, 'delete_flows')
) as (set_ovs_db_func, get_ovs_db_func, delete_flows_func):
self.agent.port_bound(port, net_uuid, 'local', None, None)
get_ovs_db_func.assert_called_once_with("Port", mock.ANY, "tag")
if new_local_vlan != old_local_vlan:
set_ovs_db_func.assert_called_once_with(
"Port", mock.ANY, "tag", str(new_local_vlan))
if ofport != -1:
delete_flows_func.assert_called_once_with(in_port=port.ofport)
else:
self.assertFalse(delete_flows_func.called)
else:
self.assertFalse(set_ovs_db_func.called)
self.assertFalse(delete_flows_func.called)
def test_port_bound_deletes_flows_for_valid_ofport(self):
self._mock_port_bound(ofport=1)
self._mock_port_bound(ofport=1, new_local_vlan=1)
def test_port_bound_ignores_flows_for_invalid_ofport(self):
self._mock_port_bound(ofport=-1)
self._mock_port_bound(ofport=-1, new_local_vlan=1)
def test_port_bound_does_not_rewire_if_already_bound(self):
self._mock_port_bound(ofport=-1, new_local_vlan=1, old_local_vlan=1)
def _test_port_dead(self, cur_tag=None):
port = mock.Mock()
port.ofport = 1
with contextlib.nested(
mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.'
'set_db_attribute', return_value=True),
mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.'
'db_get_val', return_value=cur_tag),
mock.patch.object(self.agent.int_br, 'add_flow')
) as (set_ovs_db_func, get_ovs_db_func, add_flow_func):
self.agent.port_dead(port)
get_ovs_db_func.assert_called_once_with("Port", mock.ANY, "tag")
if cur_tag == ovs_neutron_agent.DEAD_VLAN_TAG:
self.assertFalse(set_ovs_db_func.called)
self.assertFalse(add_flow_func.called)
else:
set_ovs_db_func.assert_called_once_with(
"Port", mock.ANY, "tag", str(ovs_neutron_agent.DEAD_VLAN_TAG))
add_flow_func.assert_called_once_with(
priority=2, in_port=port.ofport, actions="drop")
def test_port_dead(self):
with mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.'
'set_db_attribute',
return_value=True):
with mock.patch.object(self.agent.int_br,
'add_flow') as add_flow_func:
self.agent.port_dead(mock.Mock())
self.assertTrue(add_flow_func.called)
self._test_port_dead()
def test_port_dead_with_port_already_dead(self):
self._test_port_dead(ovs_neutron_agent.DEAD_VLAN_TAG)
def mock_scan_ports(self, vif_port_set=None, registered_ports=None,
updated_ports=None):

View File

@ -417,6 +417,7 @@ class TunnelTest(base.BaseTestCase):
def test_port_bound(self):
self.mock_int_bridge_expected += [
mock.call.db_get_val('Port', VIF_PORT.port_name, 'tag'),
mock.call.set_db_attribute('Port', VIF_PORT.port_name,
'tag', str(LVM.vlan)),
mock.call.delete_flows(in_port=VIF_PORT.ofport)
@ -447,6 +448,7 @@ class TunnelTest(base.BaseTestCase):
def test_port_dead(self):
self.mock_int_bridge_expected += [
mock.call.db_get_val('Port', VIF_PORT.port_name, 'tag'),
mock.call.set_db_attribute(
'Port', VIF_PORT.port_name,
'tag', ovs_neutron_agent.DEAD_VLAN_TAG),