[ovs-dev] [PATCH 1/3] ofp-parse: Separate fields properly.

Jesse Gross jesse at nicira.com
Tue Sep 1 02:57:04 UTC 2015


Currently, each token in an OpenFlow match field is treated separately -
whether this is a name, a value, or a single identifier. However, this
means that attempting to get a value may result in grabbing the next
token if no value exists. This avoids that problem by breaking the match
string down into its components and then individually separating it into
name/value pairs if appropriate.

Signed-off-by: Jesse Gross <jesse at nicira.com>
---
 lib/ofp-parse.c | 165 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 81 insertions(+), 84 deletions(-)

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index eaaa8ba..a6190ed 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -256,7 +256,7 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string,
     } fields;
     char *save_ptr = NULL;
     char *act_str = NULL;
-    char *name;
+    char *field;
 
     *usable_protocols = OFPUTIL_P_ANY;
 
@@ -339,116 +339,113 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string,
             return xstrdup("must specify an action");
         }
     }
-    for (name = strtok_r(string, "=, \t\r\n", &save_ptr); name;
-         name = strtok_r(NULL, "=, \t\r\n", &save_ptr)) {
+    for (field = strtok_r(string, ", \t\r\n", &save_ptr); field;
+         field = strtok_r(NULL, ", \t\r\n", &save_ptr)) {
         const struct protocol *p;
         char *error = NULL;
 
-        if (parse_protocol(name, &p)) {
+        if (parse_protocol(field, &p)) {
             match_set_dl_type(&fm->match, htons(p->dl_type));
             if (p->nw_proto) {
                 match_set_nw_proto(&fm->match, p->nw_proto);
             }
-        } else if (fields & F_FLAGS && !strcmp(name, "send_flow_rem")) {
+        } else if (fields & F_FLAGS && !strcmp(field, "send_flow_rem")) {
             fm->flags |= OFPUTIL_FF_SEND_FLOW_REM;
-        } else if (fields & F_FLAGS && !strcmp(name, "check_overlap")) {
+        } else if (fields & F_FLAGS && !strcmp(field, "check_overlap")) {
             fm->flags |= OFPUTIL_FF_CHECK_OVERLAP;
-        } else if (fields & F_FLAGS && !strcmp(name, "reset_counts")) {
+        } else if (fields & F_FLAGS && !strcmp(field, "reset_counts")) {
             fm->flags |= OFPUTIL_FF_RESET_COUNTS;
             *usable_protocols &= OFPUTIL_P_OF12_UP;
-        } else if (fields & F_FLAGS && !strcmp(name, "no_packet_counts")) {
+        } else if (fields & F_FLAGS && !strcmp(field, "no_packet_counts")) {
             fm->flags |= OFPUTIL_FF_NO_PKT_COUNTS;
             *usable_protocols &= OFPUTIL_P_OF13_UP;
-        } else if (fields & F_FLAGS && !strcmp(name, "no_byte_counts")) {
+        } else if (fields & F_FLAGS && !strcmp(field, "no_byte_counts")) {
             fm->flags |= OFPUTIL_FF_NO_BYT_COUNTS;
             *usable_protocols &= OFPUTIL_P_OF13_UP;
-        } else if (!strcmp(name, "no_readonly_table")
-                   || !strcmp(name, "allow_hidden_fields")) {
+        } else if (!strcmp(field, "no_readonly_table")
+                   || !strcmp(field, "allow_hidden_fields")) {
              /* ignore these fields. */
-        } else if (mf_from_name(name)) {
-            char *value;
-
-            value = strtok_r(NULL, ", \t\r\n", &save_ptr);
-            if (!value) {
-                /* If there's no value, we're just trying to match on the
-                 * existence of the field, so use a no-op value. */
-                value = "0/0";
-            }
-
-            error = parse_field(mf_from_name(name), value, &fm->match,
-                                usable_protocols);
-            if (error) {
-                return error;
-            }
         } else {
-            char *value;
+            char *name, *value;
 
-            value = strtok_r(NULL, ", \t\r\n", &save_ptr);
-            if (!value) {
-                return xasprintf("field %s missing value", name);
-            }
+            name = strsep(&field, "=");
+            value = field;
 
-            if (!strcmp(name, "table")) {
-                error = str_to_u8(value, "table", &fm->table_id);
-                if (fm->table_id != 0xff) {
-                    *usable_protocols &= OFPUTIL_P_TID;
+            if (mf_from_name(name)) {
+                if (!value) {
+                    /* If there's no value, we're just trying to match on the
+                     * existence of the field, so use a no-op value. */
+                     value = "0/0";
                 }
-            } else if (fields & F_OUT_PORT && !strcmp(name, "out_port")) {
-                if (!ofputil_port_from_string(value, &fm->out_port)) {
-                    error = xasprintf("%s is not a valid OpenFlow port",
-                                      value);
+                error = parse_field(mf_from_name(name), value, &fm->match,
+                                    usable_protocols);
+            } else {
+                if (!value) {
+                    return xasprintf("field %s missing value", name);
                 }
-            } else if (fields & F_PRIORITY && !strcmp(name, "priority")) {
-                uint16_t priority = 0;
-
-                error = str_to_u16(value, name, &priority);
-                fm->priority = priority;
-            } else if (fields & F_TIMEOUT && !strcmp(name, "idle_timeout")) {
-                error = str_to_u16(value, name, &fm->idle_timeout);
-            } else if (fields & F_TIMEOUT && !strcmp(name, "hard_timeout")) {
-                error = str_to_u16(value, name, &fm->hard_timeout);
-            } else if (fields & F_IMPORTANCE && !strcmp(name, "importance")) {
-                error = str_to_u16(value, name, &fm->importance);
-            } else if (!strcmp(name, "cookie")) {
-                char *mask = strchr(value, '/');
 
-                if (mask) {
-                    /* A mask means we're searching for a cookie. */
-                    if (command == OFPFC_ADD) {
-                        return xstrdup("flow additions cannot use "
-                                       "a cookie mask");
+                if (!strcmp(name, "table")) {
+                    error = str_to_u8(value, "table", &fm->table_id);
+                    if (fm->table_id != 0xff) {
+                        *usable_protocols &= OFPUTIL_P_TID;
                     }
-                    *mask = '\0';
-                    error = str_to_be64(value, &fm->cookie);
-                    if (error) {
-                        return error;
+                } else if (fields & F_OUT_PORT && !strcmp(name, "out_port")) {
+                    if (!ofputil_port_from_string(value, &fm->out_port)) {
+                        error = xasprintf("%s is not a valid OpenFlow port",
+                                          value);
                     }
-                    error = str_to_be64(mask + 1, &fm->cookie_mask);
-
-                    /* Matching of the cookie is only supported through NXM or
-                     * OF1.1+. */
-                    if (fm->cookie_mask != htonll(0)) {
-                        *usable_protocols &= OFPUTIL_P_NXM_OF11_UP;
+                } else if (fields & F_PRIORITY && !strcmp(name, "priority")) {
+                    uint16_t priority = 0;
+
+                    error = str_to_u16(value, name, &priority);
+                    fm->priority = priority;
+                } else if (fields & F_TIMEOUT && !strcmp(name, "idle_timeout")) {
+                    error = str_to_u16(value, name, &fm->idle_timeout);
+                } else if (fields & F_TIMEOUT && !strcmp(name, "hard_timeout")) {
+                    error = str_to_u16(value, name, &fm->hard_timeout);
+                } else if (fields & F_IMPORTANCE && !strcmp(name, "importance")) {
+                    error = str_to_u16(value, name, &fm->importance);
+                } else if (!strcmp(name, "cookie")) {
+                    char *mask = strchr(value, '/');
+
+                    if (mask) {
+                        /* A mask means we're searching for a cookie. */
+                        if (command == OFPFC_ADD) {
+                            return xstrdup("flow additions cannot use "
+                                           "a cookie mask");
+                        }
+                        *mask = '\0';
+                        error = str_to_be64(value, &fm->cookie);
+                        if (error) {
+                            return error;
+                        }
+                        error = str_to_be64(mask + 1, &fm->cookie_mask);
+
+                        /* Matching of the cookie is only supported through
+                         * NXM or OF1.1+. */
+                        if (fm->cookie_mask != htonll(0)) {
+                            *usable_protocols &= OFPUTIL_P_NXM_OF11_UP;
+                        }
+                    } else {
+                        /* No mask means that the cookie is being set. */
+                        if (command != OFPFC_ADD && command != OFPFC_MODIFY
+                            && command != OFPFC_MODIFY_STRICT) {
+                            return xstrdup("cannot set cookie");
+                        }
+                        error = str_to_be64(value, &fm->new_cookie);
+                        fm->modify_cookie = true;
                     }
+                } else if (!strcmp(name, "duration")
+                           || !strcmp(name, "n_packets")
+                           || !strcmp(name, "n_bytes")
+                           || !strcmp(name, "idle_age")
+                           || !strcmp(name, "hard_age")) {
+                    /* Ignore these, so that users can feed the output of
+                     * "ovs-ofctl dump-flows" back into commands that parse
+                     * flows. */
                 } else {
-                    /* No mask means that the cookie is being set. */
-                    if (command != OFPFC_ADD && command != OFPFC_MODIFY
-                        && command != OFPFC_MODIFY_STRICT) {
-                        return xstrdup("cannot set cookie");
-                    }
-                    error = str_to_be64(value, &fm->new_cookie);
-                    fm->modify_cookie = true;
+                    error = xasprintf("unknown keyword %s", name);
                 }
-            } else if (!strcmp(name, "duration")
-                       || !strcmp(name, "n_packets")
-                       || !strcmp(name, "n_bytes")
-                       || !strcmp(name, "idle_age")
-                       || !strcmp(name, "hard_age")) {
-                /* Ignore these, so that users can feed the output of
-                 * "ovs-ofctl dump-flows" back into commands that parse
-                 * flows. */
-            } else {
-                error = xasprintf("unknown keyword %s", name);
             }
 
             if (error) {
-- 
2.1.4




More information about the dev mailing list