OVSBridge: add --strict to allow priority in delete_flows

n8g-sfc currently has its own variant of OVSBridge to allow the use
of priority in a delete_flows call

This change is meant to make this available outside n8g-sfc and
simplify n8g-sfc code.

This change adds a 'strict' boolean parameter to mod_flow and delete_flows
that results in ovs-ofctl to be run with --strict for del-flows and
mod-flows actions.  When strict is set, the use of priority is allowed
and hence not rejected anymore.

Note that for batched actions in a deffered bridge, we disallow mixing
calls with strict and without strict, which can't be translated in one CLI
call.

Needed-By: I3bf939590dd43bff685f133bff86eb7e9068de91
Change-Id: I289d546780f10dc1002ab6bc2e1b38c9ef2d728f
This commit is contained in:
Thomas Morin 2017-03-29 11:11:01 -05:00
parent acc2a91c02
commit 5981bf416d
5 changed files with 61 additions and 12 deletions

View File

@ -338,6 +338,10 @@ class OVSBridge(BaseOVS):
self.br_name, 'datapath_id')
def do_action_flows(self, action, kwargs_list):
# 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)
for kw in kwargs_list:
if action is 'del':
if kw.get('cookie') == COOKIE_ANY:
@ -357,14 +361,27 @@ class OVSBridge(BaseOVS):
if 'cookie' not in kw:
kw['cookie'] = self._default_cookie
if action in ('mod', 'del'):
if kw.pop('strict', False) != strict:
msg = ("cannot mix 'strict' and not 'strict' in a batch "
"call")
raise exceptions.InvalidInput(error_message=msg)
else:
if kw.pop('strict', False):
msg = "cannot use 'strict' with 'add' action"
raise exceptions.InvalidInput(error_message=msg)
strict_param = ["--strict"] if strict else []
if action == 'del' and {} in kwargs_list:
# the 'del' case simplifies itself if kwargs_list has at least
# one item that matches everything
self.run_ofctl('%s-flows' % action, [])
else:
flow_strs = [_build_flow_expr_str(kw, action)
flow_strs = [_build_flow_expr_str(kw, action, strict)
for kw in kwargs_list]
self.run_ofctl('%s-flows' % action, ['-'], '\n'.join(flow_strs))
self.run_ofctl('%s-flows' % action, strict_param + ['-'],
'\n'.join(flow_strs))
def add_flow(self, **kwargs):
self.do_action_flows('add', [kwargs])
@ -733,7 +750,7 @@ class DeferredOVSBridge(object):
self.br.br_name)
def _build_flow_expr_str(flow_dict, cmd):
def _build_flow_expr_str(flow_dict, cmd, strict):
flow_expr_arr = []
actions = None
@ -745,8 +762,10 @@ def _build_flow_expr_str(flow_dict, cmd):
flow_expr_arr.append("priority=%s" %
flow_dict.pop('priority', '1'))
elif 'priority' in flow_dict:
msg = _("Cannot match priority on flow deletion or modification")
raise exceptions.InvalidInput(error_message=msg)
if not strict:
msg = _("Cannot match priority on flow deletion or modification "
"without 'strict'")
raise exceptions.InvalidInput(error_message=msg)
if cmd != 'del':
if "actions" not in flow_dict:

View File

@ -95,7 +95,8 @@ def ofctl_arg_supported(cmd, **kwargs):
br_name = common_utils.get_rand_device_name(prefix='br-test-')
with ovs_lib.OVSBridge(br_name) as test_br:
full_args = ["ovs-ofctl", cmd, test_br.br_name,
ovs_lib._build_flow_expr_str(kwargs, cmd.split('-')[0])]
ovs_lib._build_flow_expr_str(kwargs, cmd.split('-')[0],
False)]
try:
agent_utils.execute(full_args, run_as_root=True)
except RuntimeError as e:

View File

@ -474,6 +474,14 @@ class OVSBridgeTestCase(OVSBridgeTestBase):
self.br.delete_flows(cookie=ovs_lib.COOKIE_ANY)
self.assertEqual([], self.br.dump_all_flows())
def test_delete_flows_strict(self):
self.br.delete_flows(cookie=ovs_lib.COOKIE_ANY) # remove NORMAL action
self.br.add_flow(in_port=1, actions="output:2")
self.br.add_flow(in_port=1, priority=100, actions="output:3")
self.assertEqual(2, len(self.br.dump_all_flows()))
self.br.delete_flows(in_port=1, priority=100, strict=True)
self.assertEqual(1, len(self.br.dump_all_flows()))
class OVSLibTestCase(base.BaseOVSLinuxTestCase):

View File

@ -374,13 +374,32 @@ class OVS_Lib_Test(base.BaseTestCase):
]
self.execute.assert_has_calls(expected_calls)
def test_delete_flow_with_priority_set(self):
params = {'in_port': '1',
'priority': '1'}
def test_mod_delete_flows_strict(self):
self.br.delete_flows(in_port=5, priority=1, strict=True)
self.br.mod_flow(in_port=5, priority=1, strict=True, actions='drop')
cookie_spec = "cookie=%s" % self.br._default_cookie
expected_calls = [
self._ofctl_mock("del-flows", self.BR_NAME, '--strict', '-',
process_input=StringSetMatcher(
"%s/-1,in_port=5,priority=1" % cookie_spec)),
self._ofctl_mock("mod-flows", self.BR_NAME, '--strict', '-',
process_input=StringSetMatcher(
"%s,in_port=5,priority=1,actions=drop" %
cookie_spec)),
]
self.execute.assert_has_calls(expected_calls)
def test_mod_delete_flows_priority_without_strict(self):
self.assertRaises(exceptions.InvalidInput,
self.br.delete_flows,
**params)
in_port=5, priority=1)
def test_mod_delete_flows_mixed_strict(self):
deferred_br = self.br.deferred()
deferred_br.delete_flows(in_port=5)
deferred_br.delete_flows(in_port=5, priority=1, strict=True)
self.assertRaises(exceptions.InvalidInput,
deferred_br.apply_flows)
def test_dump_flows(self):
table = 23

View File

@ -83,10 +83,12 @@ class TestOVSCookieBridgeOFCtl(ovs_test_base.OVSOFCtlTestBase):
self.assertIn(self.tested_bridge._default_cookie,
self.bridge.reserved_cookies)
def assert_mock_build_flow_expr_str_call(self, action, kwargs_list):
def assert_mock_build_flow_expr_str_call(self, action, kwargs_list,
strict=False):
self.mock_build_flow_expr_str.assert_called_once_with(
kwargs_list[0],
action
action,
strict
)
def test_add_flow_without_cookie(self):