On Thu, 2011-06-16 at 14:12 -0700, Ben Pfaff wrote:
> On Wed, Jun 15, 2011 at 06:55:38PM -0700, Andrew Evans wrote:
> > When an error is encountered while parsing flows from a file, ovs-ofctl
> > doesn't
> > print the flow it can't parse, so it's not always obvious which flow is
> > causing
> > the error. Print the flow before the error message to make it clear.
>
> Could you write a helper function to do the job of this:
>
> if (verbose) {
> fprintf(stderr, "%s:\n", str_);
> }
> ovs_fatal(0, "must specify an action");
Ok, this looks a bit tidier:
---
lib/ofp-parse.c | 45 +++++++++++++++++++++++++++++++++------------
lib/ofp-parse.h | 5 +++--
utilities/ovs-ofctl.c | 6 +++---
3 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index a4679a3..f0b943d 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(bool verbose, const char *flow, 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(verbose, str_, "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(verbose, str_, "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(verbose, str_, "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_);
+ }
+ ovs_fatal(verbose, str_, "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);
+ ovs_fatal(verbose, str_, "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;
diff --git a/lib/ofp-parse.h b/lib/ofp-parse.h
index 8209298..d55159a 100644
--- a/lib/ofp-parse.h
+++ b/lib/ofp-parse.h
@@ -29,11 +29,12 @@ struct flow_stats_request;
struct list;
struct ofpbuf;
-void parse_ofp_str(struct flow_mod *, struct ofpbuf *actions, char *string);
+void parse_ofp_str(struct flow_mod *, struct ofpbuf *actions, const char *str_,
+ bool verbose);
void parse_ofp_flow_mod_str(struct list *packets,
enum nx_flow_format *cur, bool *flow_mod_table_id,
- char *string, uint16_t command);
+ char *string, uint16_t command, bool verbose);
bool parse_ofp_flow_mod_file(struct list *packets,
enum nx_flow_format *cur, bool *flow_mod_table_id,
FILE *, uint16_t command);
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 6c7e3ac..7d3b7fb 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -658,7 +658,7 @@ do_flow_mod__(int argc, char *argv[], uint16_t command)
flow_mod_table_id = false;
parse_ofp_flow_mod_str(&requests, &flow_format, &flow_mod_table_id,
- argc > 2 ? argv[2] : "", command);
+ argc > 2 ? argv[2] : "", command, false);
check_final_format_for_flow_mod(flow_format);
open_vconn(argv[1], &vconn);
@@ -1035,7 +1035,7 @@ read_flows_from_file(const char *filename, struct
classifier *cls, int index)
struct flow_mod fm;
ofpbuf_init(&actions, 64);
- parse_ofp_str(&fm, &actions, ds_cstr(&s));
+ parse_ofp_str(&fm, &actions, ds_cstr(&s), true);
version = xmalloc(sizeof *version);
version->cookie = fm.cookie;
@@ -1308,7 +1308,7 @@ do_parse_flow(int argc OVS_UNUSED, char *argv[])
list_init(&packets);
parse_ofp_flow_mod_str(&packets, &flow_format, &flow_mod_table_id,
- argv[1], OFPFC_ADD);
+ argv[1], OFPFC_ADD, false);
print_packet_list(&packets);
}
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
