[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