ovs-ofctl: Print the offending flow on parse error when reading from a file.

When an error is encountered while parsing flows from a file, ovs-ofctl doesn't
print the erroneous flow, so it's not always obvious which flow is causing
the error. Print the flow before the error message to make it clear.

1 files changed, 33 insertions, 12 deletions

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index a4679a3a..25fd2b96 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -614,6 +614,19 @@ struct field {
     flow_wildcards_t wildcard;  /* FWW_* bit. */
 };
 
+static void
+ofp_fatal(const char *flow, bool verbose, const char *format, ...)
+{
+    va_list args;
+
+    if (verbose) {
+        fprintf(stderr, "%s:\n", flow);
+    }
+
+    va_start(args, format);
+    ovs_fatal_valist(0, format, args);
+}
+
 static bool
 parse_field_name(const char *name, const struct field **f_out)
 {
@@ -777,12 +790,14 @@ parse_reg_value(struct cls_rule *rule, int reg_idx, const char *value)
     }
 }
 
-/* Convert 'string' (as described in the Flow Syntax section of the ovs-ofctl
- * man page) into 'pf'.  If 'actions' is specified, an action must be in
+/* Convert 'str_' (as described in the Flow Syntax section of the ovs-ofctl
+ * man page) into 'fm'.  If 'actions' is specified, an action must be in
  * 'string' and may be expanded or reallocated. */
 void
-parse_ofp_str(struct flow_mod *fm, struct ofpbuf *actions, char *string)
+parse_ofp_str(struct flow_mod *fm, struct ofpbuf *actions, const char *str_,
+              bool verbose)
 {
+    char *string = xstrdup(str_);
     char *save_ptr = NULL;
     char *name;
 
@@ -798,13 +813,13 @@ parse_ofp_str(struct flow_mod *fm, struct ofpbuf *actions, char *string)
     if (actions) {
         char *act_str = strstr(string, "action");
         if (!act_str) {
-            ovs_fatal(0, "must specify an action");
+            ofp_fatal(str_, verbose, "must specify an action");
         }
         *act_str = '\0';
 
         act_str = strchr(act_str + 1, '=');
         if (!act_str) {
-            ovs_fatal(0, "must specify an action");
+            ofp_fatal(str_, verbose, "must specify an action");
         }
 
         act_str++;
@@ -831,7 +846,7 @@ parse_ofp_str(struct flow_mod *fm, struct ofpbuf *actions, char *string)
 
             value = strtok_r(NULL, ", \t\r\n", &save_ptr);
             if (!value) {
-                ovs_fatal(0, "field %s missing value", name);
+                ofp_fatal(str_, verbose, "field %s missing value", name);
             }
 
             if (!strcmp(name, "table")) {
@@ -875,7 +890,10 @@ parse_ofp_str(struct flow_mod *fm, struct ofpbuf *actions, char *string)
                        && isdigit((unsigned char) name[3])) {
                 unsigned int reg_idx = atoi(name + 3);
                 if (reg_idx >= FLOW_N_REGS) {
-                    ovs_fatal(0, "only %d registers supported", FLOW_N_REGS);
+                    if (verbose) {
+                        fprintf(stderr, "%s:\n", str_);
+                    }
+                    ofp_fatal(str_, verbose, "only %d registers supported", FLOW_N_REGS);
                 }
                 parse_reg_value(&fm->cr, reg_idx, value);
             } else if (!strcmp(name, "duration")
@@ -885,10 +903,12 @@ parse_ofp_str(struct flow_mod *fm, struct ofpbuf *actions, char *string)
                  * "ovs-ofctl dump-flows" back into commands that parse
                  * flows. */
             } else {
-                ovs_fatal(0, "unknown keyword %s", name);
+                ofp_fatal(str_, verbose, "unknown keyword %s", name);
             }
         }
     }
+
+    free(string);
 }
 
 /* Parses 'string' as an OFPT_FLOW_MOD or NXT_FLOW_MOD with command 'command'
@@ -899,7 +919,8 @@ parse_ofp_str(struct flow_mod *fm, struct ofpbuf *actions, char *string)
  * flow. */
 void
 parse_ofp_flow_mod_str(struct list *packets, enum nx_flow_format *cur_format,
-                       bool *flow_mod_table_id, char *string, uint16_t command)
+                       bool *flow_mod_table_id, char *string, uint16_t command,
+                       bool verbose)
 {
     bool is_del = command == OFPFC_DELETE || command == OFPFC_DELETE_STRICT;
     enum nx_flow_format min_format, next_format;
@@ -909,7 +930,7 @@ parse_ofp_flow_mod_str(struct list *packets, enum nx_flow_format *cur_format,
     struct flow_mod fm;
 
     ofpbuf_init(&actions, 64);
-    parse_ofp_str(&fm, is_del ? NULL : &actions, string);
+    parse_ofp_str(&fm, is_del ? NULL : &actions, string, verbose);
     fm.command = command;
 
     min_format = ofputil_min_flow_format(&fm.cr);
@@ -953,7 +974,7 @@ parse_ofp_flow_mod_file(struct list *packets,
     ok = ds_get_preprocessed_line(&s, stream) == 0;
     if (ok) {
         parse_ofp_flow_mod_str(packets, cur, flow_mod_table_id,
-                               ds_cstr(&s), command);
+                               ds_cstr(&s), command, true);
     }
     ds_destroy(&s);
 
@@ -966,7 +987,7 @@ parse_ofp_flow_stats_request_str(struct flow_stats_request *fsr,
 {
     struct flow_mod fm;
 
-    parse_ofp_str(&fm, NULL, string);
+    parse_ofp_str(&fm, NULL, string, false);
     fsr->aggregate = aggregate;
     fsr->match = fm.cr;
     fsr->out_port = fm.out_port;