From 7e9cd643e4a3f864299e206a00d7f0f6dba936e3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?=C3=89douard=20Thuleau?= Date: Mon, 3 Mar 2014 18:08:33 +0100 Subject: [PATCH] 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 (cherry picked from commit 501213686886baccd3280e10b8856a25d3517519) --- neutron/agent/linux/ovs_lib.py | 11 +++-- .../tests/unit/openvswitch/test_ovs_lib.py | 44 +++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/neutron/agent/linux/ovs_lib.py b/neutron/agent/linux/ovs_lib.py index e1172234f..64cf1baf4 100644 --- a/neutron/agent/linux/ovs_lib.py +++ b/neutron/agent/linux/ovs_lib.py @@ -206,7 +206,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) @@ -214,8 +221,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, diff --git a/neutron/tests/unit/openvswitch/test_ovs_lib.py b/neutron/tests/unit/openvswitch/test_ovs_lib.py index a9c1b981a..7b7d9d167 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_lib.py +++ b/neutron/tests/unit/openvswitch/test_ovs_lib.py @@ -401,6 +401,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" -- 2.45.2