From patchwork Tue Jan 21 14:40:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1226613 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.133; helo=hemlock.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 hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 482B6Z3tblz9sSn for ; Wed, 22 Jan 2020 01:40:54 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 0BF43877FF; Tue, 21 Jan 2020 14:40:53 +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 HINk2omm39W8; Tue, 21 Jan 2020 14:40:48 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 2A6E985B48; Tue, 21 Jan 2020 14:40:48 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 13D86C0176; Tue, 21 Jan 2020 14:40:48 +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 55EBCC0176 for ; Tue, 21 Jan 2020 14:40:46 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 41BEA877B3 for ; Tue, 21 Jan 2020 14:40:46 +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 VjPPzTWuf20b for ; Tue, 21 Jan 2020 14:40:41 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay12.mail.gandi.net (relay12.mail.gandi.net [217.70.178.232]) by hemlock.osuosl.org (Postfix) with ESMTPS id A05FD87880 for ; Tue, 21 Jan 2020 14:40:40 +0000 (UTC) Received: from nummac.local (unknown [116.75.117.25]) (Authenticated sender: numans@ovn.org) by relay12.mail.gandi.net (Postfix) with ESMTPSA id 1E334200006; Tue, 21 Jan 2020 14:40:35 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Tue, 21 Jan 2020 20:10:23 +0530 Message-Id: <20200121144023.245743-1-numans@ovn.org> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200121143955.245616-1-numans@ovn.org> References: <20200121143955.245616-1-numans@ovn.org> MIME-Version: 1.0 Cc: Han Zhou Subject: [ovs-dev] [PATCH ovn v2 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. Acked-by: Mark Michelson Acked-by: Han Zhou Signed-off-by: Numan Siddique --- 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); From patchwork Tue Jan 21 14:40:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1226614 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.133; helo=hemlock.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 hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 482B7059S8z9sRf for ; Wed, 22 Jan 2020 01:41:16 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 0C7FC8780D; Tue, 21 Jan 2020 14:41:15 +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 zz4n2MDQqGdx; Tue, 21 Jan 2020 14:41:09 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id CBE7887553; Tue, 21 Jan 2020 14:41:09 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C3EC2C0176; Tue, 21 Jan 2020 14:41:09 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1CBC4C0174 for ; Tue, 21 Jan 2020 14:41:09 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 182B6203B1 for ; Tue, 21 Jan 2020 14:41:09 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 13fZvIJj8HsG for ; Tue, 21 Jan 2020 14:41:03 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay12.mail.gandi.net (relay12.mail.gandi.net [217.70.178.232]) by silver.osuosl.org (Postfix) with ESMTPS id BBF7A204F8 for ; Tue, 21 Jan 2020 14:41:00 +0000 (UTC) Received: from nummac.local (unknown [116.75.117.25]) (Authenticated sender: numans@ovn.org) by relay12.mail.gandi.net (Postfix) with ESMTPSA id CDFBD200002; Tue, 21 Jan 2020 14:40:57 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Tue, 21 Jan 2020 20:10:50 +0530 Message-Id: <20200121144050.245868-1-numans@ovn.org> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200121143955.245616-1-numans@ovn.org> References: <20200121143955.245616-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v2 2/2] ovn-controller: Cache logical flow expr tree for each lflow. 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 This patch caches the logical flow expr tree for each logical flow. This cache is stored as an hashmap in the output_flow engine data. When a logical flow is deleted, the expr tree for that lflow is deleted in the flow_output_sb_logical_flow_handler(). Below are the details of the OVN resource in my setup No of logical switches - 49 No of logical ports - 1191 No of logical routers - 7 No of logical ports - 51 No of ACLs - 1221 No of Logical flows - 664736 On a chassis hosting 7 distributed router ports and around 1090 port bindings, the no of OVS rules was 921162. Without this patch, the function add_logical_flows() took ~15 seconds. And with this patch it took ~10 seconds. There is a good 34% improvement in logical flow processing. Acked-by: Mark Michelson Signed-off-by: Numan Siddique Acked-by: Han Zhou --- controller/lflow.c | 192 ++++++++++++++++++++++++++---------- controller/lflow.h | 9 +- controller/ovn-controller.c | 16 ++- 3 files changed, 160 insertions(+), 57 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 93ec53a1c..997c59662 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -75,11 +75,13 @@ static bool consider_logical_flow( const struct shash *port_groups, const struct sset *active_tunnels, const struct sset *local_lport_ids, + bool delete_expr_from_cache, struct ovn_desired_flow_table *, struct ovn_extend_table *group_table, struct ovn_extend_table *meter_table, struct lflow_resource_ref *lfrr, - uint32_t *conj_id_ofs); + uint32_t *conj_id_ofs, + struct hmap *lflow_expr_cache); static bool lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp) @@ -255,6 +257,66 @@ lflow_resource_destroy_lflow(struct lflow_resource_ref *lfrr, free(lfrn); } +/* Represents expr tree for the lflow. We don't store the + * match in this structure because - + * - Whenever a flow match or action needs to be modified, + * ovn-northd deletes the existing flow in the logical_flow + * table and adds a new one. + * We may want to revisit this and store match incase the behavior + * of ovn-northd changes. + */ +struct lflow_expr { + struct hmap_node node; + struct uuid lflow_uuid; /* key */ + struct expr *expr; +}; + +static void +lflow_expr_add(struct hmap *lflow_expr_cache, + const struct sbrec_logical_flow *lflow, + struct expr *lflow_expr) +{ + struct lflow_expr *le = xmalloc(sizeof *le); + le->lflow_uuid = lflow->header_.uuid; + le->expr = lflow_expr; + hmap_insert(lflow_expr_cache, &le->node, uuid_hash(&le->lflow_uuid)); +} + +static struct lflow_expr * +lflow_expr_get(struct hmap *lflow_expr_cache, + const struct sbrec_logical_flow *lflow) +{ + struct lflow_expr *le; + size_t hash = uuid_hash(&lflow->header_.uuid); + HMAP_FOR_EACH_WITH_HASH (le, node, hash, lflow_expr_cache) { + if (uuid_equals(&le->lflow_uuid, &lflow->header_.uuid)) { + return le; + } + } + + return NULL; +} + +static void +lflow_expr_delete(struct hmap *lflow_expr_cache, struct lflow_expr *le) +{ + hmap_remove(lflow_expr_cache, &le->node); + expr_destroy(le->expr); + free(le); +} + +void +lflow_expr_destroy(struct hmap *lflow_expr_cache) +{ + struct lflow_expr *le; + HMAP_FOR_EACH_POP (le, node, lflow_expr_cache) { + expr_destroy(le->expr); + free(le); + } + + hmap_destroy(lflow_expr_cache); +} + /* Adds the logical flows from the Logical_Flow table to flow tables. */ static void add_logical_flows( @@ -273,7 +335,8 @@ add_logical_flows( struct ovn_extend_table *group_table, struct ovn_extend_table *meter_table, struct lflow_resource_ref *lfrr, - uint32_t *conj_id_ofs) + uint32_t *conj_id_ofs, + struct hmap *lflow_expr_cache) { const struct sbrec_logical_flow *lflow; @@ -306,9 +369,9 @@ add_logical_flows( chassis, &dhcp_opts, &dhcpv6_opts, &nd_ra_opts, &controller_event_opts, addr_sets, port_groups, - active_tunnels, local_lport_ids, + active_tunnels, local_lport_ids, false, flow_table, group_table, meter_table, - lfrr, conj_id_ofs)) { + lfrr, conj_id_ofs, lflow_expr_cache)) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); VLOG_ERR_RL(&rl, "Conjunction id overflow when processing lflow " UUID_FMT, UUID_ARGS(&lflow->header_.uuid)); @@ -338,7 +401,8 @@ lflow_handle_changed_flows( struct ovn_extend_table *group_table, struct ovn_extend_table *meter_table, struct lflow_resource_ref *lfrr, - uint32_t *conj_id_ofs) + uint32_t *conj_id_ofs, + struct hmap *lflow_expr_cache) { bool ret = true; const struct sbrec_logical_flow *lflow; @@ -373,6 +437,12 @@ lflow_handle_changed_flows( ofctrl_remove_flows(flow_table, &lflow->header_.uuid); /* Delete entries from lflow resource reference. */ lflow_resource_destroy_lflow(lfrr, &lflow->header_.uuid); + + /* Delete the expr cache for this lflow. */ + struct lflow_expr *le = lflow_expr_get(lflow_expr_cache, lflow); + if (le) { + lflow_expr_delete(lflow_expr_cache, le); + } } } @@ -399,9 +469,9 @@ lflow_handle_changed_flows( chassis, &dhcp_opts, &dhcpv6_opts, &nd_ra_opts, &controller_event_opts, addr_sets, port_groups, - active_tunnels, local_lport_ids, + active_tunnels, local_lport_ids, true, flow_table, group_table, meter_table, - lfrr, conj_id_ofs)) { + lfrr, conj_id_ofs, lflow_expr_cache)) { ret = false; break; } @@ -434,6 +504,7 @@ lflow_handle_changed_ref( struct ovn_extend_table *meter_table, struct lflow_resource_ref *lfrr, uint32_t *conj_id_ofs, + struct hmap *lflow_expr_cache, bool *changed) { struct ref_lflow_node *rlfn = ref_lflow_lookup(&lfrr->ref_lflow_table, @@ -503,9 +574,9 @@ lflow_handle_changed_ref( chassis, &dhcp_opts, &dhcpv6_opts, &nd_ra_opts, &controller_event_opts, addr_sets, port_groups, - active_tunnels, local_lport_ids, + active_tunnels, local_lport_ids, true, flow_table, group_table, meter_table, - lfrr, conj_id_ofs)) { + lfrr, conj_id_ofs, lflow_expr_cache)) { ret = false; break; } @@ -551,11 +622,13 @@ consider_logical_flow( const struct shash *port_groups, const struct sset *active_tunnels, const struct sset *local_lport_ids, + bool delete_expr_from_cache, struct ovn_desired_flow_table *flow_table, struct ovn_extend_table *group_table, struct ovn_extend_table *meter_table, struct lflow_resource_ref *lfrr, - uint32_t *conj_id_ofs) + uint32_t *conj_id_ofs, + struct hmap *lflow_expr_cache) { /* Determine translation of logical table IDs to physical table IDs. */ bool ingress = !strcmp(lflow->pipeline, "ingress"); @@ -613,59 +686,77 @@ consider_logical_flow( /* Translate OVN match into table of OpenFlow matches. */ struct hmap matches; - struct expr *expr; + struct expr *expr = NULL; struct sset addr_sets_ref = SSET_INITIALIZER(&addr_sets_ref); struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref); - expr = expr_parse_string(lflow->match, &symtab, addr_sets, port_groups, - &addr_sets_ref, &port_groups_ref, &error); - const char *addr_set_name; - SSET_FOR_EACH (addr_set_name, &addr_sets_ref) { - lflow_resource_add(lfrr, REF_TYPE_ADDRSET, addr_set_name, - &lflow->header_.uuid); - } - const char *port_group_name; - SSET_FOR_EACH (port_group_name, &port_groups_ref) { - lflow_resource_add(lfrr, REF_TYPE_PORTGROUP, port_group_name, - &lflow->header_.uuid); - } - sset_destroy(&addr_sets_ref); - sset_destroy(&port_groups_ref); - - if (!error) { - if (prereqs) { - expr = expr_combine(EXPR_T_AND, expr, prereqs); - prereqs = NULL; + struct lflow_expr *le = lflow_expr_get(lflow_expr_cache, lflow); + if (le) { + if (delete_expr_from_cache) { + lflow_expr_delete(lflow_expr_cache, le); + } else { + expr = le->expr; } - expr = expr_annotate(expr, &symtab, &error); } - if (error) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "error parsing match \"%s\": %s", - lflow->match, error); - expr_destroy(prereqs); - free(error); - ovnacts_free(ovnacts.data, ovnacts.size); - ofpbuf_uninit(&ovnacts); - return true; + + if (!expr) { + expr = expr_parse_string(lflow->match, &symtab, addr_sets, port_groups, + &addr_sets_ref, &port_groups_ref, &error); + const char *addr_set_name; + SSET_FOR_EACH (addr_set_name, &addr_sets_ref) { + lflow_resource_add(lfrr, REF_TYPE_ADDRSET, addr_set_name, + &lflow->header_.uuid); + } + const char *port_group_name; + SSET_FOR_EACH (port_group_name, &port_groups_ref) { + lflow_resource_add(lfrr, REF_TYPE_PORTGROUP, port_group_name, + &lflow->header_.uuid); + } + sset_destroy(&addr_sets_ref); + sset_destroy(&port_groups_ref); + + if (!error) { + if (prereqs) { + expr = expr_combine(EXPR_T_AND, expr, prereqs); + prereqs = NULL; + } + expr = expr_annotate(expr, &symtab, &error); + } + if (error) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "error parsing match \"%s\": %s", + lflow->match, error); + expr_destroy(prereqs); + free(error); + ovnacts_free(ovnacts.data, ovnacts.size); + ofpbuf_uninit(&ovnacts); + return true; + } + + expr = expr_simplify(expr); + expr = expr_normalize(expr); + + lflow_expr_add(lflow_expr_cache, lflow, expr); } - struct lookup_port_aux aux = { - .sbrec_multicast_group_by_name_datapath - = sbrec_multicast_group_by_name_datapath, - .sbrec_port_binding_by_name = sbrec_port_binding_by_name, - .dp = lflow->logical_datapath - }; struct condition_aux cond_aux = { .sbrec_port_binding_by_name = sbrec_port_binding_by_name, .chassis = chassis, .active_tunnels = active_tunnels, }; - expr = expr_simplify(expr); - expr = expr_normalize(expr); + + expr = expr_clone(expr); expr = expr_evaluate_condition(expr, is_chassis_resident_cb, &cond_aux); + + struct lookup_port_aux aux = { + .sbrec_multicast_group_by_name_datapath + = sbrec_multicast_group_by_name_datapath, + .sbrec_port_binding_by_name = sbrec_port_binding_by_name, + .dp = lflow->logical_datapath + }; uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, &matches); + expr_destroy(expr); if (hmap_is_empty(&matches)) { @@ -907,7 +998,8 @@ lflow_run(struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath, struct ovn_extend_table *group_table, struct ovn_extend_table *meter_table, struct lflow_resource_ref *lfrr, - uint32_t *conj_id_ofs) + uint32_t *conj_id_ofs, + struct hmap *lflow_expr_cache) { COVERAGE_INC(lflow_run); @@ -916,7 +1008,7 @@ lflow_run(struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath, dhcpv6_options_table, logical_flow_table, local_datapaths, chassis, addr_sets, port_groups, active_tunnels, local_lport_ids, flow_table, group_table, - meter_table, lfrr, conj_id_ofs); + meter_table, lfrr, conj_id_ofs, lflow_expr_cache); add_neighbor_flows(sbrec_port_binding_by_name, mac_binding_table, flow_table); } diff --git a/controller/lflow.h b/controller/lflow.h index abdc55e96..a2fa087f8 100644 --- a/controller/lflow.h +++ b/controller/lflow.h @@ -132,7 +132,8 @@ void lflow_run(struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath, struct ovn_extend_table *group_table, struct ovn_extend_table *meter_table, struct lflow_resource_ref *, - uint32_t *conj_id_ofs); + uint32_t *conj_id_ofs, + struct hmap *lflow_expr_cache); bool lflow_handle_changed_flows( struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath, @@ -150,7 +151,8 @@ bool lflow_handle_changed_flows( struct ovn_extend_table *group_table, struct ovn_extend_table *meter_table, struct lflow_resource_ref *, - uint32_t *conj_id_ofs); + uint32_t *conj_id_ofs, + struct hmap *lflow_expr_cache); bool lflow_handle_changed_ref( enum ref_type, @@ -171,6 +173,7 @@ bool lflow_handle_changed_ref( struct ovn_extend_table *meter_table, struct lflow_resource_ref *, uint32_t *conj_id_ofs, + struct hmap *lflow_expr_cache, bool *changed); void lflow_handle_changed_neighbors( @@ -180,4 +183,6 @@ void lflow_handle_changed_neighbors( void lflow_destroy(void); +void lflow_expr_destroy(struct hmap *lflow_expr_cache); + #endif /* controller/lflow.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 17744d416..31ce1107c 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1211,6 +1211,9 @@ struct ed_type_flow_output { uint32_t conj_id_ofs; /* lflow resource cross reference */ struct lflow_resource_ref lflow_resource_ref; + + /* Cache of lflow expr tree. */ + struct hmap lflow_expr_cache; }; static void * @@ -1224,6 +1227,7 @@ en_flow_output_init(struct engine_node *node OVS_UNUSED, ovn_extend_table_init(&data->meter_table); data->conj_id_ofs = 1; lflow_resource_init(&data->lflow_resource_ref); + hmap_init(&data->lflow_expr_cache); return data; } @@ -1235,6 +1239,7 @@ en_flow_output_cleanup(void *data) ovn_extend_table_destroy(&flow_output_data->group_table); ovn_extend_table_destroy(&flow_output_data->meter_table); lflow_resource_destroy(&flow_output_data->lflow_resource_ref); + lflow_expr_destroy(&flow_output_data->lflow_expr_cache); } static void @@ -1335,7 +1340,8 @@ en_flow_output_run(struct engine_node *node, void *data) chassis, local_datapaths, addr_sets, port_groups, active_tunnels, local_lport_ids, flow_table, group_table, meter_table, lfrr, - conj_id_ofs); + conj_id_ofs, + &fo->lflow_expr_cache); struct sbrec_multicast_group_table *multicast_group_table = (struct sbrec_multicast_group_table *)EN_OVSDB_GET( @@ -1436,7 +1442,7 @@ flow_output_sb_logical_flow_handler(struct engine_node *node, void *data) local_datapaths, chassis, addr_sets, port_groups, active_tunnels, local_lport_ids, flow_table, group_table, meter_table, lfrr, - conj_id_ofs); + conj_id_ofs, &fo->lflow_expr_cache); engine_set_node_state(node, EN_UPDATED); return handled; @@ -1721,7 +1727,7 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data, local_datapaths, chassis, addr_sets, port_groups, active_tunnels, local_lport_ids, flow_table, group_table, meter_table, lfrr, - conj_id_ofs, &changed)) { + conj_id_ofs, &fo->lflow_expr_cache, &changed)) { return false; } if (changed) { @@ -1736,7 +1742,7 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data, local_datapaths, chassis, addr_sets, port_groups, active_tunnels, local_lport_ids, flow_table, group_table, meter_table, lfrr, - conj_id_ofs, &changed)) { + conj_id_ofs, &fo->lflow_expr_cache, &changed)) { return false; } if (changed) { @@ -1751,7 +1757,7 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data, local_datapaths, chassis, addr_sets, port_groups, active_tunnels, local_lport_ids, flow_table, group_table, meter_table, lfrr, - conj_id_ofs, &changed)) { + conj_id_ofs, &fo->lflow_expr_cache, &changed)) { return false; } if (changed) {