[ovs-dev] [PATCH ovn 1/2] expr: Evaluate the condition expression in a separate step.

Han Zhou hzhou at ovn.org
Tue Jan 21 07:58:41 UTC 2020


On Thu, Jan 9, 2020 at 9:37 AM <numans at ovn.org> wrote:
>
> From: Numan Siddique <numans at 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 at 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 at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Acked-by: Han Zhou <hzhou at ovn.org>


More information about the dev mailing list