openstack
/
neutron
Code Issues Proposed changes

OVS lib defer apply doesn't handle concurrency 

The OVS lib deferred apply methods use a dict to save flows to add,
modify or delete when deffered apply is switched off.
If another thread adds, modifies or deletes flows on that dict during
another process called deffered_apply_off, its flows could be ignored.

This fix stash reference flows list and point the flows list to a new
cleared flows list. Then, it applies flows from the stashed flows list.

Closes-bug: #1263866
Change-Id: Ia3c6ce181e1599d1474da7eb944feff7d84f1d73
This commit is contained in:
Édouard Thuleau 2014-03-03 18:08:33 +01:00
parent 70214434ca
commit 5012136868
2 changed files with 52 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
neutron/agent/linux/ovs_lib.py
View File

@ -208,7 +208,14 @@ class OVSBridge(BaseOVS):
def defer_apply_off(self):
LOG.debug(_('defer_apply_off'))
for action, flows in self.deferred_flows.items():
# Note(ethuleau): stash flows and disable deferred mode. Then apply
# flows from the stashed reference to be sure to not purge flows that
# were added between two ofctl commands.
stashed_deferred_flows, self.deferred_flows = (
self.deferred_flows, {'add': '', 'mod': '', 'del': ''}
)
self.defer_apply_flows = False
for action, flows in stashed_deferred_flows.items():
if flows:
LOG.debug(_('Applying following deferred flows '
'to bridge %s'), self.br_name)
@ -216,8 +223,6 @@ class OVSBridge(BaseOVS):
LOG.debug(_('%(action)s: %(flow)s'),
{'action': action, 'flow': line})
self.run_ofctl('%s-flows' % action, ['-'], flows)
self.defer_apply_flows = False
self.deferred_flows = {'add': '', 'mod': '', 'del': ''}
def add_tunnel_port(self, port_name, remote_ip, local_ip,
tunnel_type=p_const.TYPE_GRE,

44
neutron/tests/unit/agent/linux/test_ovs_lib.py
View File

@ -407,6 +407,50 @@ class OVS_Lib_Test(base.BaseTestCase):
mock.call('del-flows', ['-'], 'deleted_flow_1\n')
])
def test_defer_apply_flows_concurrently(self):
flow_expr = mock.patch.object(ovs_lib, '_build_flow_expr_str').start()
flow_expr.side_effect = ['added_flow_1', 'deleted_flow_1',
'modified_flow_1', 'added_flow_2',
'deleted_flow_2', 'modified_flow_2']
run_ofctl = mock.patch.object(self.br, 'run_ofctl').start()
def run_ofctl_fake(cmd, args, process_input=None):
self.br.defer_apply_on()
if cmd == 'add-flows':
self.br.add_flow(flow='added_flow_2')
elif cmd == 'del-flows':
self.br.delete_flows(flow='deleted_flow_2')
elif cmd == 'mod-flows':
self.br.mod_flow(flow='modified_flow_2')
run_ofctl.side_effect = run_ofctl_fake
self.br.defer_apply_on()
self.br.add_flow(flow='added_flow_1')
self.br.delete_flows(flow='deleted_flow_1')
self.br.mod_flow(flow='modified_flow_1')
self.br.defer_apply_off()
run_ofctl.side_effect = None
self.br.defer_apply_off()
flow_expr.assert_has_calls([
mock.call({'flow': 'added_flow_1'}, 'add'),
mock.call({'flow': 'deleted_flow_1'}, 'del'),
mock.call({'flow': 'modified_flow_1'}, 'mod'),
mock.call({'flow': 'added_flow_2'}, 'add'),
mock.call({'flow': 'deleted_flow_2'}, 'del'),
mock.call({'flow': 'modified_flow_2'}, 'mod')
])
run_ofctl.assert_has_calls([
mock.call('add-flows', ['-'], 'added_flow_1\n'),
mock.call('del-flows', ['-'], 'deleted_flow_1\n'),
mock.call('mod-flows', ['-'], 'modified_flow_1\n'),
mock.call('add-flows', ['-'], 'added_flow_2\n'),
mock.call('del-flows', ['-'], 'deleted_flow_2\n'),
mock.call('mod-flows', ['-'], 'modified_flow_2\n')
])
def test_add_tunnel_port(self):
pname = "tap99"
local_ip = "1.1.1.1"