Discussion:
[Ryu-devel] [PATCH] ofproto: Avoid emitting illegal instruction sets
IWAMOTO Toshihiro
2017-07-05 05:37:55 UTC
Permalink
The OpenFlow spec forbids multiple occurences of a same instruction
type within a mod_flow message, so make sure
ofp_instruction_from_jsondict doesn't emit such an instruction set.

Signed-off-by: IWAMOTO Toshihiro <***@valinux.co.jp>
---
ryu/ofproto/ofproto_parser.py | 28 +++++++++++++---------------
ryu/tests/unit/lib/test_ofctl_string.py | 16 ++++++++++++++++
2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/ryu/ofproto/ofproto_parser.py b/ryu/ofproto/ofproto_parser.py
index 7b56968..e230055 100644
--- a/ryu/ofproto/ofproto_parser.py
+++ b/ryu/ofproto/ofproto_parser.py
@@ -147,12 +147,17 @@ def ofp_instruction_from_jsondict(dp, jsonlist, encap=True):
"""
proto = dp.ofproto
parser = dp.ofproto_parser
+ actions = []
result = []
for jsondict in jsonlist:
assert len(jsondict) == 1
k, v = list(jsondict.items())[0]
cls = getattr(parser, k)
- if not issubclass(cls, parser.OFPAction):
+ if issubclass(cls, parser.OFPAction):
+ if encap:
+ actions.append(cls.from_jsondict(v))
+ continue
+ else:
ofpinst = getattr(parser, 'OFPInstruction', None)
if not ofpinst or not issubclass(cls, ofpinst):
raise ValueError("Supplied jsondict is of wrong type: %s",
@@ -161,20 +166,13 @@ def ofp_instruction_from_jsondict(dp, jsonlist, encap=True):

if not encap:
return result
- insts = []
- actions = []
- result.append(None) # sentinel
- for act_or_inst in result:
- if isinstance(act_or_inst, parser.OFPAction):
- actions.append(act_or_inst)
- else:
- if actions:
- insts.append(parser.OFPInstructionActions(
- proto.OFPIT_APPLY_ACTIONS, actions))
- actions = []
- if act_or_inst is not None:
- insts.append(act_or_inst)
- return insts
+
+ if actions:
+ # Although the OpenFlow spec says Apply Actions is executed first,
+ # let's place it in the head as a precaution.
+ result = [parser.OFPInstructionActions(
+ proto.OFPIT_APPLY_ACTIONS, actions)] + result
+ return result


class StringifyMixin(stringify.StringifyMixin):
diff --git a/ryu/tests/unit/lib/test_ofctl_string.py b/ryu/tests/unit/lib/test_ofctl_string.py
index fb491e3..cc0ce9f 100644
--- a/ryu/tests/unit/lib/test_ofctl_string.py
+++ b/ryu/tests/unit/lib/test_ofctl_string.py
@@ -149,3 +149,19 @@ class Test_OfctlString(unittest.TestCase):
{'len': 8,
'table_id': 33,
'type': 1}})
+
+ def test_multi_unordered(self):
+ self._test_str(self.fake_dp_of15,
+ 'pop_vlan,goto_table:33,output:1',
+ {'OFPInstructionActions':
+ {'actions': [{'OFPActionPopVlan': {'len': 8,
+ 'type': 18}},
+ {'OFPActionOutput': {'len': 16,
+ 'max_len': 65509,
+ 'port': 1,
+ 'type': 0}}],
+ 'type': 4}},
+ {'OFPInstructionGotoTable':
+ {'len': 8,
+ 'table_id': 33,
+ 'type': 1}})
--
2.1.4
FUJITA Tomonori
2017-07-21 13:13:18 UTC
Permalink
On Wed, 5 Jul 2017 14:37:55 +0900
Post by IWAMOTO Toshihiro
The OpenFlow spec forbids multiple occurences of a same instruction
type within a mod_flow message, so make sure
ofp_instruction_from_jsondict doesn't emit such an instruction set.
---
ryu/ofproto/ofproto_parser.py | 28 +++++++++++++---------------
ryu/tests/unit/lib/test_ofctl_string.py | 16 ++++++++++++++++
2 files changed, 29 insertions(+), 15 deletions(-)
Applied, thanks.

Loading...