[ovs-dev] [PATCH 6/6] nicra-ext: New action NXAST_OUTPUT_REG.

[Ben Pfaff]

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.