From patchwork Wed Sep 9 10:09:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1360533 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=fraxinus.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Bmd794Nrzz9sTN for ; Wed, 9 Sep 2020 20:10:13 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id D124E852D5; Wed, 9 Sep 2020 10:10:11 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8yZNG33pPAtp; Wed, 9 Sep 2020 10:10:08 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 7378E87051; Wed, 9 Sep 2020 10:10:06 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 566FDC0859; Wed, 9 Sep 2020 10:10:06 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 45F83C0051 for ; Wed, 9 Sep 2020 10:10:05 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 42E1587146 for ; Wed, 9 Sep 2020 10:10:05 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hyN7N3bj9M-O for ; Wed, 9 Sep 2020 10:10:02 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by whitealder.osuosl.org (Postfix) with ESMTPS id B327887136 for ; Wed, 9 Sep 2020 10:10:01 +0000 (UTC) X-Originating-IP: 27.7.129.187 Received: from nusiddiq.home.org.com (unknown [27.7.129.187]) (Authenticated sender: numans@ovn.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 8FF7B240006; Wed, 9 Sep 2020 10:09:57 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Wed, 9 Sep 2020 15:39:53 +0530 Message-Id: <20200909100953.649117-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200909100903.648778-1-numans@ovn.org> References: <20200909100903.648778-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v2 3/4] expr: Evaluate the condition expression in a separate step. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Numan Siddique 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 Acked-by: Mark Michelson --- 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);