[ovs-dev] [nxm-fixes 3/3] nx-match: Add tests for error return values for unknown NXM fields.

Justin Pettit jpettit at nicira.com
Tue Dec 7 05:24:24 UTC 2010


Looks fine.  I think it would be nice to add to the comment that parsing the field name as a 32-bit hex integer is done for tests; otherwise it looks really odd.

--Justin


On Dec 2, 2010, at 2:22 PM, Ben Pfaff wrote:

> ---
> lib/nx-match.c     |   40 ++++++++++++++++++++++++++--------------
> tests/ovs-ofctl.at |   24 ++++++++++++++++++++++++
> 2 files changed, 50 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index ea31c79..89c8093 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -720,18 +720,27 @@ nx_match_to_string(const uint8_t *p, unsigned int match_len)
>     return ds_steal_cstr(&s);
> }
> 
> -static const struct nxm_field *
> -lookup_nxm_field(const char *name, int name_len)
> +static uint32_t
> +parse_nxm_field_name(const char *name, int name_len)
> {
>     const struct nxm_field *f;
> 
> +    /* Check whether it's a field name. */
>     for (f = nxm_fields; f < &nxm_fields[ARRAY_SIZE(nxm_fields)]; f++) {
>         if (!strncmp(f->name, name, name_len) && f->name[name_len] == '\0') {
> -            return f;
> +            return f->header;
>         }
>     }
> 
> -    return NULL;
> +    /* Check whether it's a 32-bit field header value as hex. */
> +    if (name_len == 8) {
> +        uint32_t header = hexits_value(name, name_len, NULL);
> +        if (header != UINT_MAX) {
> +            return header;
> +        }
> +    }
> +
> +    return 0;
> }
> 
> static const char *
> @@ -769,35 +778,38 @@ nx_match_from_string(const char *s, struct ofpbuf *b)
>     }
> 
>     for (s += strspn(s, ", "); *s; s += strspn(s, ", ")) {
> -        const struct nxm_field *f;
> +        const char *name;
> +        uint32_t header;
>         int name_len;
> 
> +        name = s;
>         name_len = strcspn(s, "(");
>         if (s[name_len] != '(') {
>             ovs_fatal(0, "%s: missing ( at end of nx_match", full_s);
>         }
> 
> -        f = lookup_nxm_field(s, name_len);
> -        if (!f) {
> +        header = parse_nxm_field_name(name, name_len);
> +        if (!header) {
>             ovs_fatal(0, "%s: unknown field `%.*s'", full_s, name_len, s);
>         }
> 
>         s += name_len + 1;
> 
> -        nxm_put_header(b, f->header);
> -        s = parse_hex_bytes(b, s, nxm_field_bytes(f->header));
> -        if (NXM_HASMASK(f->header)) {
> +        nxm_put_header(b, header);
> +        s = parse_hex_bytes(b, s, nxm_field_bytes(header));
> +        if (NXM_HASMASK(header)) {
>             s += strspn(s, " ");
>             if (*s != '/') {
> -                ovs_fatal(0, "%s: missing / in masked field %s",
> -                          full_s, f->name);
> +                ovs_fatal(0, "%s: missing / in masked field %.*s",
> +                          full_s, name_len, name);
>             }
> -            s = parse_hex_bytes(b, s + 1, nxm_field_bytes(f->header));
> +            s = parse_hex_bytes(b, s + 1, nxm_field_bytes(header));
>         }
> 
>         s += strspn(s, " ");
>         if (*s != ')') {
> -            ovs_fatal(0, "%s: missing ) following field %s", full_s, f->name);
> +            ovs_fatal(0, "%s: missing ) following field %.*s",
> +                      full_s, name_len, name);
>         }
>         s++;
>     }
> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> index 793e1d5..95f01f6 100644
> --- a/tests/ovs-ofctl.at
> +++ b/tests/ovs-ofctl.at
> @@ -133,6 +133,18 @@ NXM_NX_TUN_ID(00000000abcdef01)
> # Register 0.
> NXM_NX_REG0(acebdf56)
> NXM_NX_REG0_W(a0e0d050/f0f0f0f0)
> +
> +# Invalid field number.
> +01020304(1111/2222)
> +
> +# Unimplemented registers.
> +#
> +# This test assumes that at least two registers, but fewer than 16,
> +# registers are implemented.
> +00010004(12345678)
> +00010108(12345678/12345678)
> +00011e04(12345678)
> +00011f08(12345678/12345678)
> ])
> AT_CHECK([ovs-ofctl parse-nx-match < nx-match.txt], [0], [dnl
> <any>
> @@ -238,5 +250,17 @@ NXM_NX_TUN_ID(00000000abcdef01)
> # Register 0.
> NXM_NX_REG0(acebdf56)
> NXM_NX_REG0_W(a0e0d050/f0f0f0f0)
> +
> +# Invalid field number.
> +nx_pull_match() returned error 44010101
> +
> +# Unimplemented registers.
> +#
> +# This test assumes that at least two registers, but fewer than 16,
> +# registers are implemented.
> +NXM_NX_REG0(12345678)
> +NXM_NX_REG0_W(12345678/12345678)
> +nx_pull_match() returned error 44010101
> +nx_pull_match() returned error 44010101
> ])
> AT_CLEANUP
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org





More information about the dev mailing list