[ovs-dev] [PATCH v5 2/6] instruction: support goto-table action
Ben Pfaff
blp at nicira.com
Wed Aug 29 04:05:33 UTC 2012
On Wed, Aug 29, 2012 at 02:19:01AM +0900, Isaku Yamahata wrote:
> Signed-off-by: Isaku Yamahata <yamahata at valinux.co.jp>
I'm not sure why you're verifying that padding in instructions is
all-zero. (Is that something that I suggested?) I don't see anything
in OF1.1 that says that the padding fields must be zero, so it seems
inappropriate to insist that they be zero.
I find the ofpacts_put_openflow11_instructions() function hard to
follow. Without further study, I'm not sure that it is correct. I
think that it would be better to use a slightly different structure,
where a loop find the boundaries of the current instruction, whether
that instruction consists of a sequence of actions or a single
non-action instruction, and then process that after the boundaries have
been found. Did you consider such a structure?
The textual form of instructions isn't what I was envisioning. Here's
what I'd pictured a set of instructions would look like (adding extra
spaces):
actions=mod_vlan_vid:123, output:10, write_metadata(...),
goto_table(...)
I think that in the syntax that you are proposing, it would look like
this:
instructions=apply_actions(mod_vlan_vid:123, output:10),
write_metadata(...), goto_table(...)
The difference is, then, that "actions" becomes "instructions" and the
"apply_actions" keyword becomes required. I'm not sure of the benefit,
and the result is much longer. What do you think?
I'd move the str_to_inst_ofpacts() calls to ovs_fatal() into the switch
statement in parse_named_instruction().
I get a test failure:
# -*- compilation -*-
66. ofp-actions.at:245: testing OpenFlow 1.1 instruction translation ...
../../tests/ofp-actions.at:323: ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-ofp11-instructions < input.txt
--- expout 2012-08-28 20:36:00.000000000 -0700
+++ /home/blp/nicira/ovs/_build/tests/testsuite.dir/at-groups/66/stdout 2012-08-28 20:36:00.000000000 -0700
@@ -30,7 +30,7 @@
bad OF1.1 instructions: OFPBIC_UNSUP_INST
-instructions=clear_actions
+bad OF1.1 instructions: OFPBIC_UNSUP_INST
bad OF1.1 instructions: OFPBIC_BAD_EXPERIMENTER
Thanks,
Ben.
More information about the dev
mailing list