From: Numan Siddique <num...@ovn.org> A similar patch was committed earlier [1] and then reverted [2]. This patch is similar to [1], but not exactly same.
A new function is added - expr_evaluate_condition() which evaluates the condition expression - is_chassis_resident. expr_simply() will no longer evaluates this condition. expr_normalize() is still expected to be called after expr_evaluate_condition(). Otherwise it will assert if 'is_chassis_resident()' is not evaluated. An upcoming commit needs this in order to cache the logical flow expressions for logical flows having 'is_chassis_resident()' condition in their match. The expr tree after expr_simplify() for such logical flows is cached. Since we can't cache the is_chassis_resident condition, it is separated out. (For logical flows which do not have this condition in their match, the expr matches will be cached.) [1] - 99e3a145927("expr: Evaluate the condition expression in a separate step.") [2] - 065bcf46218("ovn-controller: Revert lflow expr caching") Signed-off-by: Numan Siddique <num...@ovn.org> --- controller/lflow.c | 3 ++- include/ovn/expr.h | 10 +++++---- lib/expr.c | 50 +++++++++++++++++++++++++++++++++---------- tests/test-ovn.c | 11 +++++----- utilities/ovn-trace.c | 6 ++++-- 5 files changed, 57 insertions(+), 23 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index c2f939f90f..c6e1586283 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -684,7 +684,8 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow, .lflow = lflow, .lfrr = l_ctx_out->lfrr }; - expr = expr_simplify(expr, is_chassis_resident_cb, &cond_aux); + expr = expr_simplify(expr); + expr = expr_evaluate_condition(expr, is_chassis_resident_cb, &cond_aux); expr = expr_normalize(expr); uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, &matches); diff --git a/include/ovn/expr.h b/include/ovn/expr.h index b34fb0e813..ed7b054144 100644 --- a/include/ovn/expr.h +++ b/include/ovn/expr.h @@ -432,10 +432,12 @@ void expr_destroy(struct expr *); struct expr *expr_annotate(struct expr *, const struct shash *symtab, char **errorp); -struct expr *expr_simplify(struct expr *, - bool (*is_chassis_resident)(const void *c_aux, - const char *port_name), - const void *c_aux); +struct expr *expr_simplify(struct expr *); +struct expr *expr_evaluate_condition( + struct expr *, + bool (*is_chassis_resident)(const void *c_aux, + const char *port_name), + const void *c_aux); struct expr *expr_normalize(struct expr *); bool expr_honors_invariants(const struct expr *); diff --git a/lib/expr.c b/lib/expr.c index 6fb96757ad..bff48dfd20 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -2063,10 +2063,10 @@ expr_simplify_relational(struct expr *expr) /* Resolves condition and replaces the expression with a boolean. */ static struct expr * -expr_simplify_condition(struct expr *expr, - bool (*is_chassis_resident)(const void *c_aux, +expr_evaluate_condition__(struct expr *expr, + bool (*is_chassis_resident)(const void *c_aux, const char *port_name), - const void *c_aux) + const void *c_aux) { bool result; @@ -2084,13 +2084,41 @@ expr_simplify_condition(struct expr *expr, return expr_create_boolean(result); } +struct expr * +expr_evaluate_condition(struct expr *expr, + bool (*is_chassis_resident)(const void *c_aux, + const char *port_name), + const void *c_aux) +{ + struct expr *sub, *next; + + switch (expr->type) { + case EXPR_T_AND: + case EXPR_T_OR: + LIST_FOR_EACH_SAFE (sub, next, node, &expr->andor) { + ovs_list_remove(&sub->node); + struct expr *e = expr_evaluate_condition(sub, is_chassis_resident, + c_aux); + e = expr_fix(e); + expr_insert_andor(expr, next, e); + } + return expr_fix(expr); + + case EXPR_T_CONDITION: + return expr_evaluate_condition__(expr, is_chassis_resident, c_aux); + + case EXPR_T_CMP: + case EXPR_T_BOOLEAN: + return expr; + } + + OVS_NOT_REACHED(); +} + /* Takes ownership of 'expr' and returns an equivalent expression whose * EXPR_T_CMP nodes use only tests for equality (EXPR_R_EQ). */ struct expr * -expr_simplify(struct expr *expr, - bool (*is_chassis_resident)(const void *c_aux, - const char *port_name), - const void *c_aux) +expr_simplify(struct expr *expr) { struct expr *sub, *next; @@ -2105,8 +2133,7 @@ expr_simplify(struct expr *expr, case EXPR_T_OR: LIST_FOR_EACH_SAFE (sub, next, node, &expr->andor) { ovs_list_remove(&sub->node); - expr_insert_andor(expr, next, - expr_simplify(sub, is_chassis_resident, c_aux)); + expr_insert_andor(expr, next, expr_simplify(sub)); } return expr_fix(expr); @@ -2114,7 +2141,7 @@ expr_simplify(struct expr *expr, return expr; case EXPR_T_CONDITION: - return expr_simplify_condition(expr, is_chassis_resident, c_aux); + return expr; } OVS_NOT_REACHED(); } @@ -3397,7 +3424,8 @@ expr_parse_microflow__(struct lexer *lexer, struct ds annotated = DS_EMPTY_INITIALIZER; expr_format(e, &annotated); - e = expr_simplify(e, microflow_is_chassis_resident_cb, NULL); + e = expr_simplify(e); + e = expr_evaluate_condition(e, microflow_is_chassis_resident_cb, NULL); e = expr_normalize(e); struct match m = MATCH_CATCHALL_INITIALIZER; diff --git a/tests/test-ovn.c b/tests/test-ovn.c index ba628288bb..544fee4c87 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -312,8 +312,9 @@ test_parse_expr__(int steps) } if (!error) { if (steps > 1) { - expr = expr_simplify(expr, is_chassis_resident_cb, - &ports); + expr = expr_simplify(expr); + expr = expr_evaluate_condition(expr, is_chassis_resident_cb, + &ports); } if (steps > 2) { expr = expr_normalize(expr); @@ -914,9 +915,9 @@ test_tree_shape_exhaustively(struct expr *expr, struct shash *symtab, exit(EXIT_FAILURE); } } else if (operation >= OP_SIMPLIFY) { - modified = expr_simplify(expr_clone(expr), - tree_shape_is_chassis_resident_cb, - NULL); + modified = expr_simplify(expr_clone(expr)); + modified = expr_evaluate_condition( + expr_clone(modified), tree_shape_is_chassis_resident_cb, NULL); ovs_assert(expr_honors_invariants(modified)); if (operation >= OP_NORMALIZE) { diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c index 50a32b7149..39839cb709 100644 --- a/utilities/ovn-trace.c +++ b/utilities/ovn-trace.c @@ -931,8 +931,10 @@ read_flows(void) continue; } if (match) { - match = expr_simplify(match, ovntrace_is_chassis_resident, - NULL); + match = expr_simplify(match); + match = expr_evaluate_condition(match, + ovntrace_is_chassis_resident, + NULL); } struct ovntrace_flow *flow = xzalloc(sizeof *flow); -- 2.26.2 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev