ovsfw: Use bundle when adding security group rules
Sometimes ports can have a lot of security group rules. Adding OpenFlow
rules to the integration bridge can be a bottle neck when bundle option
is not used.
As OVS firewall currently uses CLI interface, the bundle support is
implemented only for that. The native interface has on-going work but
before we switch completely to the native, we can benefit from bundle
option now.
e.g. when adding 100 000 flows the bundle option takes the time down
from ~10 seconds to 1 seconds. So it's about 10 times faster.
Change-Id: I1feaeb659c8badf23230e36145235d49d50b6bfb
(cherry picked from commit b3b2df142e
)
This commit is contained in:
parent
43df2709ac
commit
98d73ee058
|
@ -378,7 +378,7 @@ class OVSBridge(BaseOVS):
|
|||
self.br_name)
|
||||
raise RuntimeError('No datapath_id on bridge %s' % self.br_name)
|
||||
|
||||
def do_action_flows(self, action, kwargs_list):
|
||||
def do_action_flows(self, action, kwargs_list, use_bundle=False):
|
||||
# we can't mix strict and non-strict, so we'll use the first kw
|
||||
# and check against other kw being different
|
||||
strict = kwargs_list[0].get('strict', False)
|
||||
|
@ -412,7 +412,7 @@ class OVSBridge(BaseOVS):
|
|||
msg = "cannot use 'strict' with 'add' action"
|
||||
raise exceptions.InvalidInput(error_message=msg)
|
||||
|
||||
strict_param = ["--strict"] if strict else []
|
||||
extra_param = ["--strict"] if strict else []
|
||||
|
||||
if action == 'del' and {} in kwargs_list:
|
||||
# the 'del' case simplifies itself if kwargs_list has at least
|
||||
|
@ -421,7 +421,9 @@ class OVSBridge(BaseOVS):
|
|||
else:
|
||||
flow_strs = [_build_flow_expr_str(kw, action, strict)
|
||||
for kw in kwargs_list]
|
||||
self.run_ofctl('%s-flows' % action, strict_param + ['-'],
|
||||
if use_bundle:
|
||||
extra_param.append('--bundle')
|
||||
self.run_ofctl('%s-flows' % action, extra_param + ['-'],
|
||||
'\n'.join(flow_strs))
|
||||
|
||||
def add_flow(self, **kwargs):
|
||||
|
@ -922,12 +924,14 @@ class DeferredOVSBridge(object):
|
|||
ALLOWED_PASSTHROUGHS = 'add_port', 'add_tunnel_port', 'delete_port'
|
||||
|
||||
def __init__(self, br, full_ordered=False,
|
||||
order=('add', 'mod', 'del')):
|
||||
order=('add', 'mod', 'del'), use_bundle=False):
|
||||
'''Constructor.
|
||||
|
||||
:param br: wrapped bridge
|
||||
:param full_ordered: Optional, disable flow reordering (slower)
|
||||
:param order: Optional, define in which order flow are applied
|
||||
:param use_bundle: Optional, a bool whether --bundle should be passed
|
||||
to all ofctl commands. Default is set to False.
|
||||
'''
|
||||
|
||||
self.br = br
|
||||
|
@ -936,6 +940,7 @@ class DeferredOVSBridge(object):
|
|||
if not self.full_ordered:
|
||||
self.weights = dict((y, x) for x, y in enumerate(self.order))
|
||||
self.action_flow_tuples = []
|
||||
self.use_bundle = use_bundle
|
||||
|
||||
def __getattr__(self, name):
|
||||
if name in self.ALLOWED_PASSTHROUGHS:
|
||||
|
@ -965,7 +970,7 @@ class DeferredOVSBridge(object):
|
|||
itemgetter_1 = operator.itemgetter(1)
|
||||
for action, action_flow_list in grouped:
|
||||
flows = list(map(itemgetter_1, action_flow_list))
|
||||
self.br.do_action_flows(action, flows)
|
||||
self.br.do_action_flows(action, flows, self.use_bundle)
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
|
|
@ -440,7 +440,7 @@ class OVSFirewallDriver(firewall.FirewallDriver):
|
|||
@staticmethod
|
||||
def initialize_bridge(int_br):
|
||||
int_br.add_protocols(*OVSFirewallDriver.REQUIRED_PROTOCOLS)
|
||||
return int_br.deferred(full_ordered=True)
|
||||
return int_br.deferred(full_ordered=True, use_bundle=True)
|
||||
|
||||
def _drop_all_unmatched_flows(self):
|
||||
for table in ovs_consts.OVS_FIREWALL_TABLES:
|
||||
|
|
|
@ -1054,9 +1054,9 @@ class TestDeferredOVSBridge(base.BaseTestCase):
|
|||
|
||||
def test_apply_on_exit(self):
|
||||
expected_calls = [
|
||||
mock.call('add', [self.add_flow_dict1]),
|
||||
mock.call('mod', [self.mod_flow_dict1]),
|
||||
mock.call('del', [self.del_flow_dict1]),
|
||||
mock.call('add', [self.add_flow_dict1], False),
|
||||
mock.call('mod', [self.mod_flow_dict1], False),
|
||||
mock.call('del', [self.del_flow_dict1], False),
|
||||
]
|
||||
|
||||
with ovs_lib.DeferredOVSBridge(self.br) as deferred_br:
|
||||
|
@ -1080,9 +1080,9 @@ class TestDeferredOVSBridge(base.BaseTestCase):
|
|||
|
||||
def test_apply(self):
|
||||
expected_calls = [
|
||||
mock.call('add', [self.add_flow_dict1]),
|
||||
mock.call('mod', [self.mod_flow_dict1]),
|
||||
mock.call('del', [self.del_flow_dict1]),
|
||||
mock.call('add', [self.add_flow_dict1], False),
|
||||
mock.call('mod', [self.mod_flow_dict1], False),
|
||||
mock.call('del', [self.del_flow_dict1], False),
|
||||
]
|
||||
|
||||
with ovs_lib.DeferredOVSBridge(self.br) as deferred_br:
|
||||
|
@ -1096,9 +1096,12 @@ class TestDeferredOVSBridge(base.BaseTestCase):
|
|||
|
||||
def test_apply_order(self):
|
||||
expected_calls = [
|
||||
mock.call('del', [self.del_flow_dict1, self.del_flow_dict2]),
|
||||
mock.call('mod', [self.mod_flow_dict1, self.mod_flow_dict2]),
|
||||
mock.call('add', [self.add_flow_dict1, self.add_flow_dict2]),
|
||||
mock.call(
|
||||
'del', [self.del_flow_dict1, self.del_flow_dict2], False),
|
||||
mock.call(
|
||||
'mod', [self.mod_flow_dict1, self.mod_flow_dict2], False),
|
||||
mock.call(
|
||||
'add', [self.add_flow_dict1, self.add_flow_dict2], False),
|
||||
]
|
||||
|
||||
order = 'del', 'mod', 'add'
|
||||
|
@ -1113,11 +1116,12 @@ class TestDeferredOVSBridge(base.BaseTestCase):
|
|||
|
||||
def test_apply_full_ordered(self):
|
||||
expected_calls = [
|
||||
mock.call('add', [self.add_flow_dict1]),
|
||||
mock.call('mod', [self.mod_flow_dict1]),
|
||||
mock.call('del', [self.del_flow_dict1, self.del_flow_dict2]),
|
||||
mock.call('add', [self.add_flow_dict2]),
|
||||
mock.call('mod', [self.mod_flow_dict2]),
|
||||
mock.call('add', [self.add_flow_dict1], False),
|
||||
mock.call('mod', [self.mod_flow_dict1], False),
|
||||
mock.call(
|
||||
'del', [self.del_flow_dict1, self.del_flow_dict2], False),
|
||||
mock.call('add', [self.add_flow_dict2], False),
|
||||
mock.call('mod', [self.mod_flow_dict2], False),
|
||||
]
|
||||
|
||||
with ovs_lib.DeferredOVSBridge(self.br,
|
||||
|
@ -1160,3 +1164,12 @@ class TestDeferredOVSBridge(base.BaseTestCase):
|
|||
deferred_br.add_flow(actions='drop')
|
||||
deferred_br.mod_flow(actions='drop')
|
||||
f.assert_has_calls(expected_calls)
|
||||
|
||||
@vsctl_only
|
||||
def test_add_flow_with_bundle(self):
|
||||
br = ovs_lib.OVSBridge("foo")
|
||||
deferred = br.deferred(use_bundle=True)
|
||||
with mock.patch.object(utils, "execute", spec=utils.execute) as mexec:
|
||||
deferred.add_flow(in_port=1, actions='drop')
|
||||
deferred.apply_flows()
|
||||
self.assertIn('--bundle', mexec.call_args[0][0])
|
||||
|
|
|
@ -66,7 +66,7 @@ class OVSTunnelBridgeTest(ovs_bridge_test_base.OVSBridgeTestBase,
|
|||
'output:%s' % patch_int_ofport},
|
||||
{'priority': 0, 'table': 20, 'actions': 'resubmit(,22)'}
|
||||
]
|
||||
expected = [call.do_action_flows('add', flow_args),
|
||||
expected = [call.do_action_flows('add', flow_args, False),
|
||||
call.add_flow(priority=0, table=22, actions='drop')]
|
||||
self.assertEqual(expected, self.mock.mock_calls)
|
||||
|
||||
|
@ -103,7 +103,7 @@ class OVSTunnelBridgeTest(ovs_bridge_test_base.OVSBridgeTestBase,
|
|||
{'priority': 0, 'table': 20, 'actions': 'resubmit(,22)'},
|
||||
{'priority': 0, 'table': 21, 'actions': 'resubmit(,22)'}
|
||||
]
|
||||
expected = [call.do_action_flows('add', flow_args),
|
||||
expected = [call.do_action_flows('add', flow_args, False),
|
||||
call.add_flow(priority=0, table=22, actions='drop')]
|
||||
self.assertEqual(expected, self.mock.mock_calls)
|
||||
|
||||
|
|
Loading…
Reference in New Issue