metaflow: Convert hex parsing to use new utility functions.

We now have functions that can do parsing and printing of long hex
strings, so we should use them for meta flow fields to ensure
consistent behavior.

Since these functions can handle infinitely long strings, we can
also increase the maximum field size for MFS_HEXADECIMAL types to
the limit allowed by NXM/OXM. This is useful for future large fields,
such as Geneve options.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>

2 files changed, 35 insertions, 47 deletions

diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index 315552d12..ca2ca044e 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -21,16 +21,16 @@ TYPES = {"u8": 1,
          "be64": 8,
          "IPv6": 16}
 
-FORMATTING = {"decimal":            ("MFS_DECIMAL",      1,  8),
-              "hexadecimal":        ("MFS_HEXADECIMAL",  1,  8),
-              "Ethernet":           ("MFS_ETHERNET",     6,  6),
-              "IPv4":               ("MFS_IPV4",         4,  4),
-              "IPv6":               ("MFS_IPV6",        16, 16),
-              "OpenFlow 1.0 port":  ("MFS_OFP_PORT",     2,  2),
-              "OpenFlow 1.1+ port": ("MFS_OFP_PORT_OXM", 4,  4),
-              "frag":               ("MFS_FRAG",         1,  1),
-              "tunnel flags":       ("MFS_TNL_FLAGS",    2,  2),
-              "TCP flags":          ("MFS_TCP_FLAGS",    2,  2)}
+FORMATTING = {"decimal":            ("MFS_DECIMAL",      1,   8),
+              "hexadecimal":        ("MFS_HEXADECIMAL",  1, 127),
+              "Ethernet":           ("MFS_ETHERNET",     6,   6),
+              "IPv4":               ("MFS_IPV4",         4,   4),
+              "IPv6":               ("MFS_IPV6",        16,  16),
+              "OpenFlow 1.0 port":  ("MFS_OFP_PORT",     2,   2),
+              "OpenFlow 1.1+ port": ("MFS_OFP_PORT_OXM", 4,   4),
+              "frag":               ("MFS_FRAG",         1,   1),
+              "tunnel flags":       ("MFS_TNL_FLAGS",    2,   2),
+              "TCP flags":          ("MFS_TCP_FLAGS",    2,   2)}
 
 PREREQS = {"none": "MFP_NONE",
            "ARP": "MFP_ARP",
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 757843dfb..3bdca62be 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -1684,39 +1684,35 @@ static char *
 mf_from_integer_string(const struct mf_field *mf, const char *s,
                        uint8_t *valuep, uint8_t *maskp)
 {
-    unsigned long long int integer, mask;
     char *tail;
-    int i;
+    const char *err_str = "";
+    int err;
 
-    errno = 0;
-    integer = strtoull(s, &tail, 0);
-    if (errno || (*tail != '\0' && *tail != '/')) {
+    err = parse_int_string(s, valuep, mf->n_bytes, &tail);
+    if (err || (*tail != '\0' && *tail != '/')) {
+        err_str = "value";
         goto syntax_error;
     }
 
     if (*tail == '/') {
-        mask = strtoull(tail + 1, &tail, 0);
-        if (errno || *tail != '\0') {
+        err = parse_int_string(tail + 1, maskp, mf->n_bytes, &tail);
+        if (err || *tail != '\0') {
+            err_str = "mask";
             goto syntax_error;
         }
     } else {
-        mask = ULLONG_MAX;
+        memset(maskp, 0xff, mf->n_bytes);
     }
 
-    for (i = mf->n_bytes - 1; i >= 0; i--) {
-        valuep[i] = integer;
-        maskp[i] = mask;
-        integer >>= 8;
-        mask >>= 8;
-    }
-    if (integer) {
-        return xasprintf("%s: value too large for %u-byte field %s",
-                         s, mf->n_bytes, mf->name);
-    }
     return NULL;
 
 syntax_error:
-    return xasprintf("%s: bad syntax for %s", s, mf->name);
+    if (err == ERANGE) {
+        return xasprintf("%s: %s too large for %u-byte field %s",
+                         s, err_str, mf->n_bytes, mf->name);
+    } else {
+        return xasprintf("%s: bad syntax for %s %s", s, mf->name, err_str);
+    }
 }
 
 static char *
@@ -2111,33 +2107,25 @@ static void
 mf_format_integer_string(const struct mf_field *mf, const uint8_t *valuep,
                          const uint8_t *maskp, struct ds *s)
 {
-    unsigned long long int integer;
-    int i;
-
-    ovs_assert(mf->n_bytes <= 8);
-
-    integer = 0;
-    for (i = 0; i < mf->n_bytes; i++) {
-        integer = (integer << 8) | valuep[i];
-    }
     if (mf->string == MFS_HEXADECIMAL) {
-        ds_put_format(s, "%#llx", integer);
+        ds_put_hex(s, valuep, mf->n_bytes);
     } else {
-        ds_put_format(s, "%lld", integer);
-    }
-
-    if (maskp) {
-        unsigned long long int mask;
+        unsigned long long int integer = 0;
+        int i;
 
-        mask = 0;
+        ovs_assert(mf->n_bytes <= 8);
         for (i = 0; i < mf->n_bytes; i++) {
-            mask = (mask << 8) | maskp[i];
+            integer = (integer << 8) | valuep[i];
         }
+        ds_put_format(s, "%lld", integer);
+    }
 
+    if (maskp) {
         /* I guess we could write the mask in decimal for MFS_DECIMAL but I'm
          * not sure that that a bit-mask written in decimal is ever easier to
          * understand than the same bit-mask written in hexadecimal. */
-        ds_put_format(s, "/%#llx", mask);
+        ds_put_char(s, '/');
+        ds_put_hex(s, maskp, mf->n_bytes);
     }
 }