From patchwork Thu Jan 9 17:36:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1220557 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.138; helo=whitealder.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 whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 47ttbh2KBYz9s29 for ; Fri, 10 Jan 2020 04:37:20 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 879AA86E53; Thu, 9 Jan 2020 17:37:18 +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 62uparBaRiMp; Thu, 9 Jan 2020 17:37:13 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 5748686DA8; Thu, 9 Jan 2020 17:37:12 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4B3D2C1D85; Thu, 9 Jan 2020 17:37:12 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 6C01AC0881 for ; Thu, 9 Jan 2020 17:37:10 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 51C6887924 for ; Thu, 9 Jan 2020 17:37:10 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id vZaAZ1eoEfLE for ; Thu, 9 Jan 2020 17:37:09 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by hemlock.osuosl.org (Postfix) with ESMTPS id A59C786DA5 for ; Thu, 9 Jan 2020 17:37:08 +0000 (UTC) X-Originating-IP: 115.99.61.78 Received: from nummac.local (unknown [115.99.61.78]) (Authenticated sender: numans@ovn.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 7A43BFF80C; Thu, 9 Jan 2020 17:37:05 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 9 Jan 2020 23:06:56 +0530 Message-Id: <20200109173656.1482707-1-numans@ovn.org> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200109173629.1482618-1-numans@ovn.org> References: <20200109173629.1482618-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 1/2] 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 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 Acked-by: Han Zhou --- 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);