To make it easier to understand what the function does we now explicitly
pass the input/output arguments instead of the complete
lflow_ctx_in/lflow_ctx_out context.
Also:
- change the error handling to avoid forcing the caller to supply
(and free) an error string pointer.
- update function comments in expr.c to mention that port_groups sets
are also used for parsing.
CC: Han Zhou <[email protected]>
CC: Numan Siddique <[email protected]>
Signed-off-by: Dumitru Ceara <[email protected]>
---
v2: Fix typos and comments as suggested by Mark Gray.
---
controller/lflow.c | 68 +++++++++++++++++++++++++-----------------------------
lib/expr.c | 18 +++++++--------
2 files changed, 40 insertions(+), 46 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index 4d71dfd..f631679 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -755,41 +755,48 @@ add_matches_to_flow_table(const struct sbrec_logical_flow
*lflow,
ofpbuf_uninit(&ofpacts);
}
-/* Converts the match and returns the simplified expre tree.
- * The caller should evaluate the conditions and normalize the
- * expr tree. */
+/* Converts the match and returns the simplified expr tree.
+ *
+ * The caller should evaluate the conditions and normalize the expr tree.
+ */
static struct expr *
convert_match_to_expr(const struct sbrec_logical_flow *lflow,
struct expr *prereqs,
- struct lflow_ctx_in *l_ctx_in,
- struct lflow_ctx_out *l_ctx_out,
- bool *pg_addr_set_ref, char **errorp)
+ const struct shash *addr_sets,
+ const struct shash *port_groups,
+ struct lflow_resource_ref *lfrr,
+ bool *pg_addr_set_ref)
{
struct sset addr_sets_ref = SSET_INITIALIZER(&addr_sets_ref);
struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref);
- char *error;
+ char *error = NULL;
- struct expr *e = expr_parse_string(lflow->match, &symtab,
- l_ctx_in->addr_sets,
- l_ctx_in->port_groups, &addr_sets_ref,
+ struct expr *e = expr_parse_string(lflow->match, &symtab, addr_sets,
+ port_groups, &addr_sets_ref,
&port_groups_ref,
lflow->logical_datapath->tunnel_key,
&error);
const char *addr_set_name;
SSET_FOR_EACH (addr_set_name, &addr_sets_ref) {
- lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_ADDRSET, addr_set_name,
+ lflow_resource_add(lfrr, REF_TYPE_ADDRSET, addr_set_name,
&lflow->header_.uuid);
}
const char *port_group_name;
SSET_FOR_EACH (port_group_name, &port_groups_ref) {
- lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_PORTGROUP,
- port_group_name, &lflow->header_.uuid);
+ lflow_resource_add(lfrr, REF_TYPE_PORTGROUP, port_group_name,
+ &lflow->header_.uuid);
}
+ if (pg_addr_set_ref) {
+ *pg_addr_set_ref = (!sset_is_empty(&port_groups_ref) ||
+ !sset_is_empty(&addr_sets_ref));
+ }
+ sset_destroy(&addr_sets_ref);
+ sset_destroy(&port_groups_ref);
+
if (!error) {
if (prereqs) {
e = expr_combine(EXPR_T_AND, e, prereqs);
- prereqs = NULL;
}
e = expr_annotate(e, &symtab, &error);
}
@@ -797,24 +804,11 @@ convert_match_to_expr(const struct sbrec_logical_flow
*lflow,
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
VLOG_WARN_RL(&rl, "error parsing match \"%s\": %s",
lflow->match, error);
- sset_destroy(&addr_sets_ref);
- sset_destroy(&port_groups_ref);
- *errorp = error;
+ free(error);
return NULL;
}
- e = expr_simplify(e);
-
- if (pg_addr_set_ref) {
- *pg_addr_set_ref = (!sset_is_empty(&port_groups_ref) ||
- !sset_is_empty(&addr_sets_ref));
- }
-
- sset_destroy(&addr_sets_ref);
- sset_destroy(&port_groups_ref);
-
- *errorp = NULL;
- return e;
+ return expr_simplify(e);
}
static bool
@@ -896,13 +890,13 @@ consider_logical_flow(const struct sbrec_logical_flow
*lflow,
struct expr *expr = NULL;
if (!l_ctx_out->lflow_cache_map) {
/* Caching is disabled. */
- expr = convert_match_to_expr(lflow, prereqs, l_ctx_in,
- l_ctx_out, NULL, &error);
- if (error) {
+ expr = convert_match_to_expr(lflow, prereqs, l_ctx_in->addr_sets,
+ l_ctx_in->port_groups, l_ctx_out->lfrr,
+ NULL);
+ if (!expr) {
expr_destroy(prereqs);
ovnacts_free(ovnacts.data, ovnacts.size);
ofpbuf_uninit(&ovnacts);
- free(error);
return true;
}
@@ -959,13 +953,13 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow,
bool pg_addr_set_ref = false;
if (!expr) {
- expr = convert_match_to_expr(lflow, prereqs, l_ctx_in, l_ctx_out,
- &pg_addr_set_ref, &error);
- if (error) {
+ expr = convert_match_to_expr(lflow, prereqs, l_ctx_in->addr_sets,
+ l_ctx_in->port_groups, l_ctx_out->lfrr,
+ &pg_addr_set_ref);
+ if (!expr) {
expr_destroy(prereqs);
ovnacts_free(ovnacts.data, ovnacts.size);
ofpbuf_uninit(&ovnacts);
- free(error);
return true;
}
} else {
diff --git a/lib/expr.c b/lib/expr.c
index f6b22cf..acb1f3a 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -1317,9 +1317,9 @@ expr_parse__(struct expr_context *ctx)
}
/* Parses an expression from 'lexer' using the symbols in 'symtab' and
- * address set table in 'addr_sets'. If successful, returns the new
- * expression; on failure, returns NULL. Returns nonnull if and only if
- * lexer->error is NULL. */
+ * address set table in 'addr_sets' and 'port_groups'. If successful, returns
+ * the new expression; on failure, returns NULL. Returns nonnull if and only
+ * if lexer->error is NULL. */
struct expr *
expr_parse(struct lexer *lexer, const struct shash *symtab,
const struct shash *addr_sets,
@@ -1339,9 +1339,9 @@ expr_parse(struct lexer *lexer, const struct shash
*symtab,
}
/* Parses the expression in 's' using the symbols in 'symtab' and
- * address set table in 'addr_sets'. If successful, returns the new
- * expression and sets '*errorp' to NULL. On failure, returns NULL and
- * sets '*errorp' to an explanatory error message. The caller must
+ * address set table in 'addr_sets' and 'port_groups'. If successful, returns
+ * the new expression and sets '*errorp' to NULL. On failure, returns NULL
+ * and sets '*errorp' to an explanatory error message. The caller must
* eventually free the returned expression (with expr_destroy()) or
* error (with free()). */
struct expr *
@@ -3481,9 +3481,9 @@ expr_parse_microflow__(struct lexer *lexer,
}
/* Parses 's' as a microflow, using symbols from 'symtab', address set
- * table from 'addr_sets', and looking up port numbers using 'lookup_port'
- * and 'aux'. On success, stores the result in 'uflow' and returns
- * NULL, otherwise zeros 'uflow' and returns an error message that the
+ * table from 'addr_sets' and 'port_groups', and looking up port numbers using
+ * 'lookup_port' and 'aux'. On success, stores the result in 'uflow' and
+ * returns NULL, otherwise zeros 'uflow' and returns an error message that the
* caller must free().
*
* A "microflow" is a description of a single stream of packets, such as half
a
