On Wed, Aug 10, 2011 at 05:21:43PM -0700, Ethan Jackson wrote: > The NXAST_OUTPUT_REG outputs to the OpenFlow port contained in a > supplied NXM field.
I think that the NEWS item should say that we've added an OpenFlow extension to output to the port given in a field, instead of just saying that "output" now supports that. It would also be good to say in the ovs-ofctl manpage that outputting through a field is an extension, or to move that description to the section that describes Nicira action extensions. s/It's/Its/ in the comment on struct nx_action_output_reg. Let's add a requirement that pad[] in struct nx_action_output_reg be all-bytes-zero, in case we want to extend it later. In the call to nxm_src_check(), is there any real need to ensure that the field is at least 16 bits wide? In actions that *write* port numbers, we may need that many bits of space for port numbers, so we make that requirement, but I don't know a reason to require it here. (This requirement is not mentioned in the comment on struct nx_action_output_reg.) The nxm_read_field() that you defined in a previous commit didn't, as I recall, shift the selected bits down to the bottom of the field, so I think that specifying any part of a register that doesn't include the least-significant bit will have surprising results in xlate_output_reg_action(). I don't see any test for that case, although the manpage suggests using output:NXM_NX_REG0[16..31]. If the field is wider than 16 bits, I think that xlate_output_reg_action() will just discard the upper bits, so that outputting to port 0x10001 will actually output to port 1. It's probably better to just not output at all if the value read is greater than UINT16_MAX. Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
