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

numans at ovn.org numans at ovn.org
Tue Jan 21 14:40:23 UTC 2020


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.

Acked-by: Mark Michelson <mmichels at redhat.com>
Acked-by: Han Zhou <hzhou at ovn.org>
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



More information about the dev mailing list