On Thu, Jan 9, 2020 at 9:37 AM <num...@ovn.org> wrote: > > From: Numan Siddique <num...@ovn.org> > > A new function is added - expr_evaluate_condition() which evaluates > the condition expression - is_chassis_resident. expr_simply() will > no longer evaluates this condition. > > An upcoming commit needs this in order to cache the logical flow expressions > so that every lflow_*() function which calls consider_logical_flow() doesn't > need to convert the logical flow match to an expression. Instead the expr tree > for the logical flow is cached. Since we can't cache the is_chassis_resident > condition, it is separated out. > > Signed-off-by: Numan Siddique <num...@ovn.org> > --- > controller/lflow.c | 3 ++- > include/ovn/expr.h | 10 ++++---- > lib/expr.c | 55 +++++++++++++++++++++++++++++++++---------- > tests/test-ovn.c | 10 ++++---- > utilities/ovn-trace.c | 5 +++- > 5 files changed, 60 insertions(+), 23 deletions(-) > > diff --git a/controller/lflow.c b/controller/lflow.c > index a6893201e..93ec53a1c 100644 > --- a/controller/lflow.c > +++ b/controller/lflow.c > @@ -661,8 +661,9 @@ consider_logical_flow( > .chassis = chassis, > .active_tunnels = active_tunnels, > }; > - expr = expr_simplify(expr, is_chassis_resident_cb, &cond_aux); > + expr = expr_simplify(expr); > expr = expr_normalize(expr); > + expr = expr_evaluate_condition(expr, is_chassis_resident_cb, &cond_aux); > uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, > &matches); > expr_destroy(expr); > diff --git a/include/ovn/expr.h b/include/ovn/expr.h > index 21bf51c22..00a021236 100644 > --- a/include/ovn/expr.h > +++ b/include/ovn/expr.h > @@ -404,10 +404,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 e5e4d21b7..12557e3ca 100644 > --- a/lib/expr.c > +++ b/lib/expr.c > @@ -2033,10 +2033,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; > > @@ -2054,13 +2054,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; > > @@ -2075,8 +2103,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); > > @@ -2084,7 +2111,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(); > } > @@ -2649,7 +2676,7 @@ expr_normalize_and(struct expr *expr) > > struct expr *sub; > LIST_FOR_EACH (sub, node, &expr->andor) { > - if (sub->type == EXPR_T_CMP) { > + if (sub->type == EXPR_T_CMP || sub->type == EXPR_T_CONDITION) { > continue; > } > > @@ -2706,7 +2733,8 @@ expr_normalize_or(struct expr *expr) > expr_insert_andor(expr, next, new); > } > } else { > - ovs_assert(sub->type == EXPR_T_CMP); > + ovs_assert(sub->type == EXPR_T_CMP || > + sub->type == EXPR_T_CONDITION); > } > } > if (ovs_list_is_empty(&expr->andor)) { > @@ -3365,7 +3393,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 536690535..9b4f19dd2 100644 > --- a/tests/test-ovn.c > +++ b/tests/test-ovn.c > @@ -295,7 +295,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); > @@ -896,9 +898,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 264543876..ccf279a6e 100644 > --- a/utilities/ovn-trace.c > +++ b/utilities/ovn-trace.c > @@ -907,7 +907,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.24.1 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Acked-by: Han Zhou <hz...@ovn.org> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev