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
This commit is contained in:
Jakub Libosvar
Jakub Libosvar
parent
37aba76275
commit
b3b2df142e
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -441,7 +441,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