Message ID | 20200109173656.1482707-1-numans@ovn.org |
---|---|
State | Superseded |
Headers | show |
Series | Caching logical flow expr tree for each lflow | expand |
On Thu, Jan 9, 2020 at 9:37 AM <numans@ovn.org> wrote: > > From: Numan Siddique <numans@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 <numans@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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Acked-by: Han Zhou <hzhou@ovn.org>
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);