[ovs-dev] [PATCH] ovs-ofctl: Better ovs-ofctl add-flows error reporting

Andy Zhou azhou at nicira.com
Fri Mar 1 03:36:47 UTC 2013


When error is encountered with add-flows command, this patch
adds file name and line number in addition to the parser
error message to the output.

The file name and line number will not be added to
the output of "ovs-ofctl add-flow", only parser error
message will appear. Same as before.

Signed-off-by: Andy Zhou <azhou at nicira.com>
---
 lib/dynamic-string.c  |   43 +++++++----
 lib/dynamic-string.h  |    1 +
 lib/ofp-parse.c       |  205 +++++++++++++++++++++++++++++++++----------------
 lib/ofp-parse.h       |   10 ++-
 utilities/ovs-ofctl.c |   36 +++++++--
 5 files changed, 204 insertions(+), 91 deletions(-)

diff --git a/lib/dynamic-string.c b/lib/dynamic-string.c
index bd1cf45..020097a 100644
--- a/lib/dynamic-string.c
+++ b/lib/dynamic-string.c
@@ -223,6 +223,30 @@ ds_get_line(struct ds *ds, FILE *file)
     }
 }
 
+/* Preprocess a line in 'ds', clearing anything initially in 'ds'.
+ * Deletes comments introduced by "#".
+ *
+ * Returns 0 if non-blank line was found, -1 otherwise.
+ */
+int
+ds_preprocess_line(struct ds *ds)
+{
+    char *line = ds_cstr(ds);
+    char *comment;
+
+    /* Delete comments. */
+    comment = strchr(line, '#');
+    if (comment) {
+        *comment = '\0';
+    }
+
+    /* Return -1 if line is empty. */
+    if (line[strspn(line, " \t\n")] == '\0') {
+        return -1;
+    }
+    return 0;
+}
+
 /* Reads a line from 'file' into 'ds', clearing anything initially in 'ds'.
  * Deletes comments introduced by "#" and skips lines that contains only white
  * space (after deleting comments).
@@ -231,22 +255,13 @@ ds_get_line(struct ds *ds, FILE *file)
 int
 ds_get_preprocessed_line(struct ds *ds, FILE *file)
 {
-    while (!ds_get_line(ds, file)) {
-        char *line = ds_cstr(ds);
-        char *comment;
-
-        /* Delete comments. */
-        comment = strchr(line, '#');
-        if (comment) {
-            *comment = '\0';
+    do {
+        if (ds_get_line(ds, file)) {
+            return EOF;
         }
+    } while (ds_preprocess_line(ds));
 
-        /* Return successfully unless the line is all spaces. */
-        if (line[strspn(line, " \t\n")] != '\0') {
-            return 0;
-        }
-    }
-    return EOF;
+    return 0;
 }
 
 /* Reads a line from 'file' into 'ds' and does some preprocessing on it:
diff --git a/lib/dynamic-string.h b/lib/dynamic-string.h
index 098caaf..55620c9 100644
--- a/lib/dynamic-string.h
+++ b/lib/dynamic-string.h
@@ -61,6 +61,7 @@ void ds_put_strftime(struct ds *, const char *, bool utc)
 void ds_put_hex_dump(struct ds *ds, const void *buf_, size_t size,
                      uintptr_t ofs, bool ascii);
 int ds_get_line(struct ds *, FILE *);
+int ds_preprocess_line(struct ds *);
 int ds_get_preprocessed_line(struct ds *, FILE *);
 int ds_get_test_line(struct ds *, FILE *);
 
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 5fda08a..f635247 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -41,8 +41,8 @@
 
 VLOG_DEFINE_THIS_MODULE(ofp_parse);
 
-static void ofp_fatal(const char *flow, bool verbose, const char *format, ...)
-    NO_RETURN;
+static char *ofp_err_str(const char *flow, bool verbose,
+                         const char *format, ...);
 
 static uint8_t
 str_to_table_id(const char *str)
@@ -377,7 +377,7 @@ parse_metadata(struct ofpbuf *b, char *arg)
     om->metadata = htonll(str_to_u64(arg));
 }
 
-static void
+static char*
 parse_named_action(enum ofputil_action_code code, const struct flow *flow,
                    char *arg, struct ofpbuf *ofpacts)
 {
@@ -401,7 +401,7 @@ parse_named_action(enum ofputil_action_code code, const struct flow *flow,
     case OFPUTIL_OFPAT11_SET_VLAN_VID:
         vid = str_to_u32(arg);
         if (vid & ~VLAN_VID_MASK) {
-            ovs_fatal(0, "%s: not a valid VLAN VID", arg);
+            return ofp_err_str(NULL, false, "%s: not a valid VLAN VID", arg);
         }
         ofpact_put_SET_VLAN_VID(ofpacts)->vlan_vid = vid;
         break;
@@ -410,7 +410,7 @@ parse_named_action(enum ofputil_action_code code, const struct flow *flow,
     case OFPUTIL_OFPAT11_SET_VLAN_PCP:
         pcp = str_to_u32(arg);
         if (pcp & ~7) {
-            ovs_fatal(0, "%s: not a valid VLAN PCP", arg);
+            return ofp_err_str(NULL, false, "%s: not a valid VLAN PCP", arg);
         }
         ofpact_put_SET_VLAN_PCP(ofpacts)->vlan_pcp = pcp;
         break;
@@ -428,7 +428,8 @@ parse_named_action(enum ofputil_action_code code, const struct flow *flow,
         ethertype = str_to_u16(arg, "ethertype");
         if (ethertype != ETH_TYPE_VLAN_8021Q) {
             /* XXX ETH_TYPE_VLAN_8021AD case isn't supported */
-            ovs_fatal(0, "%s: not a valid VLAN ethertype", arg);
+            return ofp_err_str(NULL, false,
+                               "%s: not a valid VLAN ethertype", arg);
         }
         ofpact_put_PUSH_VLAN(ofpacts);
         break;
@@ -464,7 +465,7 @@ parse_named_action(enum ofputil_action_code code, const struct flow *flow,
     case OFPUTIL_OFPAT11_SET_NW_TOS:
         tos = str_to_u32(arg);
         if (tos & ~IP_DSCP_MASK) {
-            ovs_fatal(0, "%s: not a valid TOS", arg);
+            return ofp_err_str(NULL, false, "%s: not a valid TOS", arg);
         }
         ofpact_put_SET_IPV4_DSCP(ofpacts)->dscp = tos;
         break;
@@ -570,61 +571,69 @@ parse_named_action(enum ofputil_action_code code, const struct flow *flow,
             htons(str_to_u16(arg, "pop_mpls"));
         break;
     }
+
+    return NULL;
 }
 
-static bool
+static char*
 str_to_ofpact__(const struct flow *flow, char *pos, char *act, char *arg,
                 struct ofpbuf *ofpacts, int n_actions)
 {
     int code = ofputil_action_code_from_name(act);
+    char *err_str = NULL;
     if (code >= 0) {
-        parse_named_action(code, flow, arg, ofpacts);
+        err_str = parse_named_action(code, flow, arg, ofpacts);
     } else if (!strcasecmp(act, "drop")) {
         if (n_actions) {
-            ovs_fatal(0, "Drop actions must not be preceded by other "
-                      "actions");
+            err_str = ofp_err_str(NULL, false, "Drop actions must not"
+                                  " be preceded by other actions");
         } else if (ofputil_parse_key_value(&pos, &act, &arg)) {
-            ovs_fatal(0, "Drop actions must not be followed by other "
-                      "actions");
+            err_str = ofp_err_str(NULL, false, "Drop actions must not"
+                                  " be followed by other actions");
         }
-        return false;
     } else {
         uint16_t port;
         if (ofputil_port_from_string(act, &port)) {
             ofpact_put_OUTPUT(ofpacts)->port = port;
         } else {
-            ovs_fatal(0, "Unknown action: %s", act);
+            err_str = ofp_err_str(NULL, false, "Unknown action: %s", act);
         }
     }
 
-    return true;
+    return err_str;
 }
 
-static void
+static char*
 str_to_ofpacts(const struct flow *flow, char *str, struct ofpbuf *ofpacts)
 {
     char *pos, *act, *arg;
     enum ofperr error;
     int n_actions;
+    char *err_str = NULL;
 
     pos = str;
     n_actions = 0;
     while (ofputil_parse_key_value(&pos, &act, &arg)) {
-        if (!str_to_ofpact__(flow, pos, act, arg, ofpacts, n_actions)) {
-            break;
+        err_str = str_to_ofpact__(flow, pos, act, arg, ofpacts, n_actions);
+        if (err_str) {
+            return err_str;
         }
         n_actions++;
     }
 
-    error = ofpacts_verify(ofpacts->data, ofpacts->size);
-    if (error) {
-        ovs_fatal(0, "Incorrect action ordering");
+    if (!err_str) {
+        error = ofpacts_verify(ofpacts->data, ofpacts->size);
+        if (!error) {
+            ofpact_pad(ofpacts);
+        }else {
+            err_str = ofp_err_str(NULL, false, "Incorrect action ordering");
+        }
     }
 
-    ofpact_pad(ofpacts);
+    return err_str;
 }
 
-static void
+static char*
 parse_named_instruction(enum ovs_instruction_type type,
                         char *arg, struct ofpbuf *ofpacts)
 {
@@ -637,7 +646,8 @@ parse_named_instruction(enum ovs_instruction_type type,
 
     case OVSINST_OFPIT11_WRITE_ACTIONS:
         /* XXX */
-        ovs_fatal(0, "instruction write-actions is not supported yet");
+        return ofp_err_str(NULL, false, "instruction write-actions"
+                           " is not supported yet");
         break;
 
     case OVSINST_OFPIT11_CLEAR_ACTIONS:
@@ -652,7 +662,8 @@ parse_named_instruction(enum ovs_instruction_type type,
         struct ofpact_goto_table *ogt = ofpact_put_GOTO_TABLE(ofpacts);
         char *table_s = strsep(&arg, ",");
         if (!table_s || !table_s[0]) {
-            ovs_fatal(0, "instruction goto-table needs table id");
+            return ofp_err_str(NULL, false,  "instruction goto-table"
+                               " needs table id");
         }
         ogt->table_id = str_to_table_id(table_s);
         break;
@@ -663,11 +674,13 @@ parse_named_instruction(enum ovs_instruction_type type,
        could be invalid. */
     error = ofpacts_verify(ofpacts->data, ofpacts->size);
     if (error) {
-        ovs_fatal(0, "Incorrect instruction ordering");
+        return ofp_err_str(NULL, false, "Incorrect instruction ordering");
     }
+
+    return NULL;
 }
 
-static void
+static char*
 str_to_inst_ofpacts(const struct flow *flow, char *str, struct ofpbuf *ofpacts)
 {
     char *pos, *inst, *arg;
@@ -675,14 +688,16 @@ str_to_inst_ofpacts(const struct flow *flow, char *str, struct ofpbuf *ofpacts)
     const char *prev_inst = NULL;
     int prev_type = -1;
     int n_actions = 0;
+    char *err_str = NULL;
 
     pos = str;
     while (ofputil_parse_key_value(&pos, &inst, &arg)) {
         type = ofpact_instruction_type_from_name(inst);
         if (type < 0) {
-            if (!str_to_ofpact__(flow, pos, inst, arg, ofpacts, n_actions)) {
-                break;
-            }
+            err_str = str_to_ofpact__(flow, pos, inst, arg, ofpacts,
+                                      n_actions);
+
+            if (err_str) return err_str;
 
             type = OVSINST_OFPIT11_APPLY_ACTIONS;
             if (prev_type == type) {
@@ -690,19 +705,21 @@ str_to_inst_ofpacts(const struct flow *flow, char *str, struct ofpbuf *ofpacts)
                 continue;
             }
         } else if (type == OVSINST_OFPIT11_APPLY_ACTIONS) {
-            ovs_fatal(0, "%s isn't supported. Just write actions then "
-                      "it is interpreted as apply_actions", inst);
+            return ofp_err_str(NULL, false, "%s isn't supported. Just write"
+                               " actions then it is interpreted as"
+                               " apply_actions", inst);
         } else {
-            parse_named_instruction(type, arg, ofpacts);
+            err_str = parse_named_instruction(type, arg, ofpacts);
+            if (err_str) return err_str;
         }
 
         if (type == prev_type) {
-            ovs_fatal(0, "instruction can be specified at most once: %s",
-                      inst);
+            return ofp_err_str(NULL, false, "Instruction can be specified"
+                               " at most once: %s", inst);
         }
         if (type <= prev_type) {
-            ovs_fatal(0, "Instruction %s must be specified before %s",
-                      inst, prev_inst);
+            return ofp_err_str(NULL, false, "Instruction %s must be specified"
+                               " before %s", inst, prev_inst);
         }
 
         prev_inst = inst;
@@ -710,6 +727,7 @@ str_to_inst_ofpacts(const struct flow *flow, char *str, struct ofpbuf *ofpacts)
         n_actions++;
     }
     ofpact_pad(ofpacts);
+    return NULL;
 }
 
 struct protocol {
@@ -748,20 +766,25 @@ parse_protocol(const char *name, const struct protocol **p_out)
     return false;
 }
 
-static void
-ofp_fatal(const char *flow, bool verbose, const char *format, ...)
+static char*
+ofp_err_str(const char *flow, bool verbose, const char *format, ...)
 {
     va_list args;
+    struct ds err_str;
+
+    ds_init(&err_str);
 
     if (verbose) {
-        fprintf(stderr, "%s:\n", flow);
+        ds_put_format(&err_str, "%s\n", flow);
     }
 
     va_start(args, format);
-    ovs_fatal_valist(0, format, args);
+    ds_put_format_valist(&err_str, format, args);
+
+    return err_str.string;
 }
 
-static void
+static char*
 parse_field(const struct mf_field *mf, const char *s, struct match *match)
 {
     union mf_value value, mask;
@@ -769,10 +792,11 @@ parse_field(const struct mf_field *mf, const char *s, struct match *match)
 
     error = mf_parse(mf, s, &value, &mask);
     if (error) {
-        ovs_fatal(0, "%s", error);
+        return ofp_err_str(NULL, 0, "%s", error);
     }
 
     mf_set(mf, &value, &mask, match);
+    return NULL;
 }
 
 /* Convert 'str_' (as described in the Flow Syntax section of the ovs-ofctl man
@@ -782,8 +806,11 @@ parse_field(const struct mf_field *mf, const char *s, struct match *match)
  *
  * To parse syntax for an OFPT_FLOW_MOD (or NXT_FLOW_MOD), use an OFPFC_*
  * constant for 'command'.  To parse syntax for an OFPST_FLOW or
- * OFPST_AGGREGATE (or NXST_FLOW or NXST_AGGREGATE), use -1 for 'command'. */
-void
+ * OFPST_AGGREGATE (or NXST_FLOW or NXST_AGGREGATE), use -1 for 'command'.
+ *
+ * Return 0 if successful, otherwise a string indicating the parser error.
+ */
+char *
 parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
               bool verbose)
 {
@@ -848,13 +875,13 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
     if (fields & F_ACTIONS) {
         act_str = strstr(string, "action");
         if (!act_str) {
-            ofp_fatal(str_, verbose, "must specify an action");
+            return ofp_err_str(str_, verbose, "must specify an action");
         }
         *act_str = '\0';
 
         act_str = strchr(act_str + 1, '=');
         if (!act_str) {
-            ofp_fatal(str_, verbose, "must specify an action");
+            return ofp_err_str(str_, verbose, "must specify an action");
         }
 
         act_str++;
@@ -883,15 +910,17 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
 
             value = strtok_r(NULL, ", \t\r\n", &save_ptr);
             if (!value) {
-                ofp_fatal(str_, verbose, "field %s missing value", name);
+                return ofp_err_str(str_, verbose,
+                                   "field %s missing value", name);
             }
 
             if (!strcmp(name, "table")) {
                 fm->table_id = str_to_table_id(value);
             } else if (!strcmp(name, "out_port")) {
                 if (!ofputil_port_from_string(name, &fm->out_port)) {
-                    ofp_fatal(str_, verbose, "%s is not a valid OpenFlow port",
-                              name);
+                    return ofp_err_str(str_, verbose,
+                                       "%s is not a valid OpenFlow port",
+                                        name);
                 }
             } else if (fields & F_PRIORITY && !strcmp(name, "priority")) {
                 fm->priority = str_to_u16(value, name);
@@ -905,8 +934,9 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
                 if (mask) {
                     /* A mask means we're searching for a cookie. */
                     if (command == OFPFC_ADD) {
-                        ofp_fatal(str_, verbose, "flow additions cannot use "
-                                  "a cookie mask");
+                        return ofp_err_str(str_, verbose,
+                                           "flow additions cannot use"
+                                           " a cookie mask");
                     }
                     *mask = '\0';
                     fm->cookie = htonll(str_to_u64(value));
@@ -915,12 +945,15 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
                     /* No mask means that the cookie is being set. */
                     if (command != OFPFC_ADD && command != OFPFC_MODIFY
                             && command != OFPFC_MODIFY_STRICT) {
-                        ofp_fatal(str_, verbose, "cannot set cookie");
+                        return ofp_err_str(str_, verbose,
+                                           "cannot set cookie");
                     }
                     fm->new_cookie = htonll(str_to_u64(value));
                 }
             } else if (mf_from_name(name)) {
-                parse_field(mf_from_name(name), value, &fm->match);
+                char *err_str = parse_field(mf_from_name(name),
+                                            value, &fm->match);
+                if (err_str) return err_str;
             } else if (!strcmp(name, "duration")
                        || !strcmp(name, "n_packets")
                        || !strcmp(name, "n_bytes")
@@ -930,7 +963,7 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
                  * "ovs-ofctl dump-flows" back into commands that parse
                  * flows. */
             } else {
-                ofp_fatal(str_, verbose, "unknown keyword %s", name);
+                return ofp_err_str(str_, verbose, "unknown keyword %s", name);
             }
         }
     }
@@ -943,9 +976,12 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
     }
     if (fields & F_ACTIONS) {
         struct ofpbuf ofpacts;
+        char *err_str;
 
         ofpbuf_init(&ofpacts, 32);
-        str_to_inst_ofpacts(&fm->match.flow, act_str, &ofpacts);
+        err_str = str_to_inst_ofpacts(&fm->match.flow, act_str, &ofpacts);
+        if (err_str) return err_str;
+
         fm->ofpacts_len = ofpacts.size;
         fm->ofpacts = ofpbuf_steal_data(&ofpacts);
     } else {
@@ -954,6 +990,7 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
     }
 
     free(string);
+    return NULL;
 }
 
 /* Convert 'str_' (as described in the documentation for the "monitor" command
@@ -1023,29 +1060,36 @@ parse_flow_monitor_request(struct ofputil_flow_monitor_request *fmr,
  *
  * Prints an error on stderr and aborts the program if 's' syntax is
  * invalid. */
-void
+char *
 parse_ofpacts(const char *s_, struct ofpbuf *ofpacts)
 {
     char *s = xstrdup(s_);
-    str_to_ofpacts(NULL, s, ofpacts);
+    char *err_str;
+    err_str = str_to_ofpacts(NULL, s, ofpacts);
     free(s);
+    return err_str;
 }
 
 /* Parses 'string' as an OFPT_FLOW_MOD or NXT_FLOW_MOD with command 'command'
  * (one of OFPFC_*) into 'fm'. */
-void
+char *
 parse_ofp_flow_mod_str(struct ofputil_flow_mod *fm, const char *string,
                        uint16_t command, bool verbose)
 {
     struct match match_copy;
+    char *err_str;
 
-    parse_ofp_str(fm, command, string, verbose);
+    err_str = parse_ofp_str(fm, command, string, verbose);
+
+    if (!err_str) {
+        /* Normalize a copy of the match.  This ensures that non-normalized
+           flows get logged but doesn't affect what gets sent to the switch,
+           so that the switch can do whatever it likes with the flow. */
+        match_copy = fm->match;
+        ofputil_normalize_match(&match_copy);
+    }
 
-    /* Normalize a copy of the match.  This ensures that non-normalized flows
-     * get logged but doesn't affect what gets sent to the switch, so that the
-     * switch can do whatever it likes with the flow. */
-    match_copy = fm->match;
-    ofputil_normalize_match(&match_copy);
+    return err_str;
 }
 
 void
@@ -1055,6 +1099,7 @@ parse_ofp_flow_mod_file(const char *file_name, uint16_t command,
     size_t allocated_fms;
     FILE *stream;
     struct ds s;
+    int lineno = 0;
 
     stream = !strcmp(file_name, "-") ? stdin : fopen(file_name, "r");
     if (stream == NULL) {
@@ -1063,11 +1108,24 @@ parse_ofp_flow_mod_file(const char *file_name, uint16_t command,
 
     allocated_fms = *n_fms;
     ds_init(&s);
-    while (!ds_get_preprocessed_line(&s, stream)) {
+    while (!ds_get_line(&s, stream)) {
+        char *err_str;
+        lineno++;
+
+        /* If the line is empty, skip it. */
+        if (ds_preprocess_line(&s)) {
+            continue;
+        }
+
         if (*n_fms >= allocated_fms) {
             *fms = x2nrealloc(*fms, &allocated_fms, sizeof **fms);
         }
-        parse_ofp_flow_mod_str(&(*fms)[*n_fms], ds_cstr(&s), command, false);
+        err_str = parse_ofp_flow_mod_str(&(*fms)[*n_fms], ds_cstr(&s),
+                                         command, false);
+        if (err_str) {
+            fprintf(stderr, "OFCTL-ERR [%s:%d] ", file_name, lineno);
+            parser_fatal_exit_with_reason(err_str);
+        }
         *n_fms += 1;
     }
     ds_destroy(&s);
@@ -1172,3 +1230,14 @@ exit:
     }
     return error;
 }
+
+/* Display the err_str to stderr, then exit
+ *
+ * This function will free the memory allocated for err_str
+ */
+void parser_fatal_exit_with_reason(char *err_str)
+{
+    fprintf(stderr, "%s\n", err_str);
+    free(err_str);
+    exit(-1);
+}
diff --git a/lib/ofp-parse.h b/lib/ofp-parse.h
index d2d3c3c..cbc816c 100644
--- a/lib/ofp-parse.h
+++ b/lib/ofp-parse.h
@@ -29,10 +29,10 @@ struct ofputil_flow_mod;
 struct ofputil_flow_monitor_request;
 struct ofputil_flow_stats_request;
 
-void parse_ofp_str(struct ofputil_flow_mod *, int command, const char *str_,
-                   bool verbose);
+char* parse_ofp_str(struct ofputil_flow_mod *, int command,
+                    const char *str_, bool verbose);
 
-void parse_ofp_flow_mod_str(struct ofputil_flow_mod *, const char *string,
+char* parse_ofp_flow_mod_str(struct ofputil_flow_mod *, const char *string,
                             uint16_t command, bool verbose);
 void parse_ofp_flow_mod_file(const char *file_name, uint16_t command,
                              struct ofputil_flow_mod **fms, size_t *n_fms);
@@ -41,11 +41,13 @@ void parse_ofp_flow_stats_request_str(struct ofputil_flow_stats_request *,
                                       bool aggregate, const char *string);
 
 
-void parse_ofpacts(const char *, struct ofpbuf *ofpacts);
+char* parse_ofpacts(const char *, struct ofpbuf *ofpacts);
 
 char *parse_ofp_exact_flow(struct flow *, const char *);
 
 void parse_flow_monitor_request(struct ofputil_flow_monitor_request *,
                                 const char *);
 
+void parser_fatal_exit_with_reason(char* err_str);
+
 #endif /* ofp-parse.h */
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index ec775c7..4420991 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1090,7 +1090,12 @@ ofctl_flow_mod(int argc, char *argv[], uint16_t command)
         ofctl_flow_mod_file(argc, argv, command);
     } else {
         struct ofputil_flow_mod fm;
-        parse_ofp_flow_mod_str(&fm, argc > 2 ? argv[2] : "", command, false);
+        char *err_str = parse_ofp_flow_mod_str(&fm, argc > 2 ? argv[2] : "",
+                                               command, false);
+        if (err_str) {
+            parser_fatal_exit_with_reason(err_str);
+        }
+
         ofctl_flow_mod__(argv[1], &fm, 1);
     }
 }
@@ -1511,9 +1516,13 @@ ofctl_packet_out(int argc, char *argv[])
     struct ofpbuf ofpacts;
     struct vconn *vconn;
     int i;
+    char* err_str;
 
     ofpbuf_init(&ofpacts, 64);
-    parse_ofpacts(argv[3], &ofpacts);
+    err_str = parse_ofpacts(argv[3], &ofpacts);
+    if (err_str) {
+        parser_fatal_exit_with_reason(err_str);
+    }
 
     po.buffer_id = UINT32_MAX;
     po.in_port = str_to_port_no(argv[1], argv[2]);
@@ -1879,6 +1888,7 @@ read_flows_from_file(const char *filename, struct classifier *cls, int index)
     enum ofputil_protocol usable_protocols;
     struct ds s;
     FILE *file;
+    int lineno = 0;
 
     file = !strcmp(filename, "-") ? stdin : fopen(filename, "r");
     if (file == NULL) {
@@ -1887,11 +1897,23 @@ read_flows_from_file(const char *filename, struct classifier *cls, int index)
 
     ds_init(&s);
     usable_protocols = OFPUTIL_P_ANY;
-    while (!ds_get_preprocessed_line(&s, file)) {
+    while (!ds_get_line(&s, file)) {
         struct fte_version *version;
         struct ofputil_flow_mod fm;
+        char *err_str;
 
-        parse_ofp_str(&fm, OFPFC_ADD, ds_cstr(&s), true);
+        lineno++;
+
+        /* Skip parsing an empty lines. */
+        if (ds_preprocess_line(&s)) {
+            continue;
+        }
+
+        err_str = parse_ofp_str(&fm, OFPFC_ADD, ds_cstr(&s), true);
+        if (err_str) {
+            fprintf(stderr, "OFCTL-ERR[%s:%d] ", filename, lineno);
+            parser_fatal_exit_with_reason(err_str);
+        }
 
         version = xmalloc(sizeof *version);
         version->cookie = fm.new_cookie;
@@ -2204,8 +2226,12 @@ static void
 ofctl_parse_flow(int argc OVS_UNUSED, char *argv[])
 {
     struct ofputil_flow_mod fm;
+    char* err_str;
 
-    parse_ofp_flow_mod_str(&fm, argv[1], OFPFC_ADD, false);
+    err_str = parse_ofp_flow_mod_str(&fm, argv[1], OFPFC_ADD, false);
+    if (err_str) {
+        parser_fatal_exit_with_reason(err_str);
+    }
     ofctl_parse_flows__(&fm, 1);
 }
 
-- 
1.7.9.5




More information about the dev mailing list