[ovs-dev] [PATCH v2] ofp-actions: Implement writing to metadata field

[Joe Stringer]

Hi Ben,

I've got a working copy that fixes the above criticisms, following the
suggested approach of verifying correct ordering at the end of the
parsing functions. I have a few queries regarding this.

In the above patch, there is a test for "ofproto - packet-out with
metadata (NXM)", in tests/ofproto.at:-
AT_CHECK([ovs-ofctl packet-out br0 none
'write_metadata(0xfafafafa5a5a5a5a), controller'
'0001020304050010203040501234'])

As I understand, with this approach the above ordering of actions
should cause an error ("write_metadata" is before "controller"). I've
got this working. However, when I correct the order of these actions,
the packet appears to be sent back to the controller before the
metadata is applied:-
NXT_PACKET_IN (xid=0x0): total_len=14 in_port=NONE tun_id=0x0
metadata=0x0 reg0=0x0 reg1=0x0 reg2=0x0 reg3=0x0 reg4=0x0 reg5=0x0
reg6=0x0 reg7=0x0 (via action) data_len=14 (unbuffered)

If I remove the enforcement of correct ordering (and use the original
ovs-ofctl command above), then the metadata is returned correctly as
expected.

My understanding of the pipeline is that actions should be added to
the "action set" first, then instructions are executed, followed by
applying the action set. However, given that instructions aren't
supported yet, the intermediate step is yet unimplemented (or I've
missed it.. confirm?). Additionally, because we are implementing this
instruction as an action, it is (in the above patch) executed in the
"apply action" phase.

I see two possibilities:-
- This is expected behaviour; the order follows the command line.
Perhaps my testing should differ -- could use a flow_mod + barrier +
packet_out to verify that the instruction does what it should.
- This is not correct; Perhaps write_metadata should be forced to come
before "output" type actions? Or some modifications need to be made to
get this "action" to be applied (because it's really an instruction)
before the other actions are, in ofproto-dpif?