[ovs-dev] [ofp-print 18/18] ofp-print, ofp-parse: Add support for NXAST_REG_MOVE and NXAST_REG_LOAD.

Justin Pettit jpettit at nicira.com
Thu Dec 9 08:27:56 UTC 2010


On Dec 8, 2010, at 4:27 PM, Ben Pfaff wrote:

> +static const char *
> +parse_nxm_field_bits(const char *s, uint32_t *headerp, int *ofsp, int *n_bitsp)
> +{
> ...
> +    if (sscanf(s, "[%d..%d]", &start, &end) != 2) {
> +        if (sscanf(s, "[%d]", &start) != 1) {
> +            ovs_fatal(0, "%s: syntax error expecting [<bit>] or "
> +                      "[<start>..<end>]", full_s);
> +        } else {
> +            end = start;
> +        }

It surprises me that "src[start]" would be accepted to mean copy a single bit, but just specifying "src" to mean the entire field isn't supported.  I'm not sure how useful the former is, and the latter seems quite handy.

> +static void
> +format_nxm_field_bits(struct ds *s, uint32_t header, int ofs, int n_bits)
> +{
> +    format_nxm_field_name(s, header);
> +    if (n_bits != 1) {
> +        ds_put_format(s, "[%d..%d]", ofs, ofs + n_bits - 1);
> +    } else {
> +        ds_put_format(s, "[%d]", ofs);
> +    }

Do you think it's worth sanity-checking that "n_bits" is not 0?  It would be possible with a malformed "move" action.

> +.IP "\fBmove:\fIsrc\fB[\fIstart\fB:\fIend\fB]->\fIdst\fB[\fIstart\fB:\fIend\fB]\fR"
> +Copies the named bits from field \fIsrc\fR to field \fIdst\fR.
> +\fIsrc\fI and \fIdst\fR must be NXM field names as defined in
> +\fBnicira\-ext.h\fR, e.g. \fBNXM_OF_UDP_SRC\fR or \fBNXM_NX_REG0\fR.
> +Each \fIstart\fR and \fIend\fR pair, which are inclusive, must specify
> +the same number of bits and must fit within its respective field.
> +Example: \fBmove:NXM_NX_REG0[0..5]\->NXM_NX_REG1[26..31]\fR copies the
> +six bits numbered 0 through 5, inclusive, in register 0 into bits 26
> +through 31, inclusive.
> +.IP "\fBload:\fIvalue\fB\->\fIdst\fB[\fIstart\fB:\fIend\fB]"
> +Writes \fIvalue\fR to bits \fIstart\fR through \fIend\fR, inclusive,
> +in field \fBdst\fR.  Example: \fBload:55\->NXM_NX_REG2[0..5]\fR loads
> +value 55 (bit pattern \fB110111\fR) into bits 0 through 5, inclusive,
> +in register 2.

Was that MIME-encoded?  Yeesh.  :-)  Do you mind putting a "." line between the move and load definitions?

In the third line, that should be an "\fR" after "src".  In both definitions, the describe "[start:end]", but it should be "[start..end]".

--Justin






More information about the dev mailing list