Open JamesGuthrie opened 9 years ago
Hi James, the compilable action check is indeed too strict. However, it needs to reject the case where two actions modify different sets of fields, as in:
Action 1: set VLAN = 1, send out port 1 Action 2: set SrcMAC = 1, send out port 2
In this (slightly artificial) case, the intended behavior would be to emit two packets, one with VLAN set to 1 and the other with the SrcMAC set to 1 (but with the original VLAN value). As far as I know, no list of actions can accomplish this in OF 1.0: if the VLAN is set first, there's no way to revert that change to recover the original VLAN value of the packet, and the same is true for modifying the SrcMAC (or any other field).
The example you included avoids this scenario, because---as you noted---the packet with just a modification to the output port can be emitted first, after which the VLAN can be set and the second packet emitted to a different output port. However, if you changed the first action to also set, say, the DstMAC, it would no longer work.
That being said, the implementation of this check should be fixed to be more accurate.
Thanks for the explanation - it looks like I had also misinterpreted the OF1.0 spec.
It seems as though the correct check would be to ignore the output port field modification and the check to see if the field modifications of one action are a subset of the field modifications of another. This would allow an ordering to be applied to the actions which would result in their correct execution.
Do you see any problems with that idea?
Right. To rephrase what you said, the check should be: does there exist an ordering on actions such that the fields modified by each preceding action are a subset of (or equal to) the fields modified by the following action. If two actions are incomparable---neither is a subset of the other---then the check should fail.
I'm currently coming up against an error in Pyretic in the "check_OF_rule_has_compilable_action_list" method here: https://github.com/frenetic-lang/pyretic/blob/master/pyretic/core/runtime.py#L772
I commented out the check and the resulting actions could be compiled and installed to an Open vSwitch, so it seems as though this check is too strict. In my case I have two actions with different sets of modifications, one just sets the output port, the other modifies the vlan id and sets a different output port. This appears to be supported by the OpenFlow specification, so I'm not sure what the purpose of this check was in the first place.