On Wed, Aug 29, 2012 at 02:19:01AM +0900, Isaku Yamahata wrote: > Signed-off-by: Isaku Yamahata <yamah...@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. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev