From patchwork Wed Sep 9 10:09:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1360534 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 4Bmd7K1bxvz9sTN for ; Wed, 9 Sep 2020 20:10:21 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 732708756F; Wed, 9 Sep 2020 10:10:19 +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 weVOk7gjrqTZ; Wed, 9 Sep 2020 10:10:17 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 25053875A3; Wed, 9 Sep 2020 10:10:17 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0D79FC0859; Wed, 9 Sep 2020 10:10:17 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 7A39DC0891 for ; Wed, 9 Sep 2020 10:10:16 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 65B768705B for ; Wed, 9 Sep 2020 10:10:16 +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 YuwgAaenOT8S for ; Wed, 9 Sep 2020 10:10:11 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [217.70.183.198]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 26CD187096 for ; Wed, 9 Sep 2020 10:10:06 +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 relay6-d.mail.gandi.net (Postfix) with ESMTPSA id EA702C000E; Wed, 9 Sep 2020 10:10:02 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Wed, 9 Sep 2020 15:39:58 +0530 Message-Id: <20200909100958.649186-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 4/4] ovn-controller: Cache logical flow expr matches. 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 is a second attempt in caching the expr tree of logical flows. Earlier attemp [1] was reverted. In this approach, we do the following - If a logical flow match has any port group/address set references, we don't cache expr match or expr tree. - If a logical flow match doesn't have a port group/address set reference and expr condition match - is_chassis_resident, we cache the expr matches (which is hmap of struct expr_match *). - If a logical flow match has expr condition match - is_chassis_resident we cache the simplified 'expr' tree. Caching is configurable and can be disabled if desired. The second patch of this series added this support. In my testing, a complete lflow_run() took around 5 seconds for around 90k logical flows, where as it took around 2 seconds with caching. Signed-off-by: Numan Siddique --- controller/lflow.c | 459 ++++++++++++++++++++++++++++-------------- include/ovn/expr.h | 2 +- lib/expr.c | 17 +- tests/test-ovn.c | 5 +- utilities/ovn-trace.c | 2 +- 5 files changed, 327 insertions(+), 158 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index c6e1586283..59b32ab803 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -269,14 +269,34 @@ lflow_resource_destroy_lflow(struct lflow_resource_ref *lfrr, free(lfrn); } +enum lflow_cache_type { + LCACHE_T_NO_CACHE, + LCACHE_T_MATCHES, + LCACHE_T_EXPR, +}; + /* Represents an lflow cache which * - stores the conjunction id offset if the lflow matches * results in conjunctive OpenvSwitch flows. + * + * - Caches + * (1) Nothing if the logical flow has port group/address set references. + * (2) expr tree if the logical flow has is_chassis_resident() match. + * (3) expr matches if (1) and (2) are false. */ struct lflow_cache { struct hmap_node node; struct uuid lflow_uuid; /* key */ uint32_t conj_id_ofs; + + enum lflow_cache_type type; + union { + struct { + struct hmap *expr_matches; + size_t n_conjs; + }; + struct expr *expr; + }; }; static struct lflow_cache * @@ -286,6 +306,7 @@ lflow_cache_add(struct hmap *lflow_cache_map, struct lflow_cache *lc = xmalloc(sizeof *lc); lc->lflow_uuid = lflow->header_.uuid; lc->conj_id_ofs = 0; + lc->type = LCACHE_T_NO_CACHE; hmap_insert(lflow_cache_map, &lc->node, uuid_hash(&lc->lflow_uuid)); return lc; } @@ -312,6 +333,12 @@ lflow_cache_delete(struct hmap *lflow_cache_map, struct lflow_cache *lc = lflow_cache_get(lflow_cache_map, lflow); if (lc) { hmap_remove(lflow_cache_map, &lc->node); + if (lc->type == LCACHE_T_MATCHES) { + expr_matches_destroy(lc->expr_matches); + free(lc->expr_matches); + } else if (lc->type == LCACHE_T_EXPR) { + expr_destroy(lc->expr); + } free(lc); } } @@ -327,6 +354,12 @@ lflow_cache_destroy(struct hmap *lflow_cache_map) { struct lflow_cache *lc; HMAP_FOR_EACH_POP (lc, node, lflow_cache_map) { + if (lc->type == LCACHE_T_MATCHES) { + expr_matches_destroy(lc->expr_matches); + free(lc->expr_matches); + } else if (lc->type == LCACHE_T_EXPR) { + expr_destroy(lc->expr); + } free(lc); } @@ -567,6 +600,159 @@ update_conj_id_ofs(uint32_t *conj_id_ofs, uint32_t n_conjs) return true; } +static void +add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, + struct hmap *matches, size_t conj_id_ofs, + uint8_t ptable, uint8_t output_ptable, + struct ofpbuf *ovnacts, + bool ingress, struct lflow_ctx_in *l_ctx_in, + struct lflow_ctx_out *l_ctx_out) +{ + struct lookup_port_aux aux = { + .sbrec_multicast_group_by_name_datapath + = l_ctx_in->sbrec_multicast_group_by_name_datapath, + .sbrec_port_binding_by_name = l_ctx_in->sbrec_port_binding_by_name, + .dp = lflow->logical_datapath + }; + + /* Encode OVN logical actions into OpenFlow. */ + uint64_t ofpacts_stub[1024 / 8]; + struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub); + struct ovnact_encode_params ep = { + .lookup_port = lookup_port_cb, + .tunnel_ofport = tunnel_ofport_cb, + .aux = &aux, + .is_switch = datapath_is_switch(lflow->logical_datapath), + .group_table = l_ctx_out->group_table, + .meter_table = l_ctx_out->meter_table, + .lflow_uuid = lflow->header_.uuid, + + .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, + .ingress_ptable = OFTABLE_LOG_INGRESS_PIPELINE, + .egress_ptable = OFTABLE_LOG_EGRESS_PIPELINE, + .output_ptable = output_ptable, + .mac_bind_ptable = OFTABLE_MAC_BINDING, + .mac_lookup_ptable = OFTABLE_MAC_LOOKUP, + }; + ovnacts_encode(ovnacts->data, ovnacts->size, &ep, &ofpacts); + + struct expr_match *m; + HMAP_FOR_EACH (m, hmap_node, matches) { + match_set_metadata(&m->match, + htonll(lflow->logical_datapath->tunnel_key)); + if (m->match.wc.masks.conj_id) { + m->match.flow.conj_id += conj_id_ofs; + } + if (datapath_is_switch(lflow->logical_datapath)) { + unsigned int reg_index + = (ingress ? MFF_LOG_INPORT : MFF_LOG_OUTPORT) - MFF_REG0; + int64_t port_id = m->match.flow.regs[reg_index]; + if (port_id) { + int64_t dp_id = lflow->logical_datapath->tunnel_key; + char buf[16]; + get_unique_lport_key(dp_id, port_id, buf, sizeof(buf)); + lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_PORTBINDING, buf, + &lflow->header_.uuid); + if (!sset_contains(l_ctx_in->local_lport_ids, buf)) { + VLOG_DBG("lflow "UUID_FMT + " port %s in match is not local, skip", + UUID_ARGS(&lflow->header_.uuid), + buf); + continue; + } + } + } + if (!m->n) { + ofctrl_add_flow(l_ctx_out->flow_table, ptable, lflow->priority, + lflow->header_.uuid.parts[0], &m->match, &ofpacts, + &lflow->header_.uuid); + } else { + uint64_t conj_stubs[64 / 8]; + struct ofpbuf conj; + + ofpbuf_use_stub(&conj, conj_stubs, sizeof conj_stubs); + for (int i = 0; i < m->n; i++) { + const struct cls_conjunction *src = &m->conjunctions[i]; + struct ofpact_conjunction *dst; + + dst = ofpact_put_CONJUNCTION(&conj); + dst->id = src->id + conj_id_ofs; + dst->clause = src->clause; + dst->n_clauses = src->n_clauses; + } + + ofctrl_add_or_append_flow(l_ctx_out->flow_table, ptable, + lflow->priority, 0, + &m->match, &conj, &lflow->header_.uuid); + ofpbuf_uninit(&conj); + } + } + + ofpbuf_uninit(&ofpacts); +} + +/* Converts the actions and returns the simplified expre tree. + * The caller should evaluate the conditions and normalize the + * expr tree. */ +static struct expr * +convert_acts_to_expr(const struct sbrec_logical_flow *lflow, + struct expr *prereqs, + struct lflow_ctx_in *l_ctx_in, + struct lflow_ctx_out *l_ctx_out, + bool *pg_addr_set_ref, char **errorp) +{ + struct sset addr_sets_ref = SSET_INITIALIZER(&addr_sets_ref); + struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref); + char *error; + + struct expr *e = expr_parse_string(lflow->match, &symtab, + l_ctx_in->addr_sets, + l_ctx_in->port_groups, &addr_sets_ref, + &port_groups_ref, + lflow->logical_datapath->tunnel_key, + &error); + const char *addr_set_name; + SSET_FOR_EACH (addr_set_name, &addr_sets_ref) { + lflow_resource_add(l_ctx_out->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(l_ctx_out->lfrr, REF_TYPE_PORTGROUP, + port_group_name, &lflow->header_.uuid); + } + + if (!error) { + if (prereqs) { + e = expr_combine(EXPR_T_AND, e, prereqs); + prereqs = NULL; + } + e = expr_annotate(e, &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); + sset_destroy(&addr_sets_ref); + sset_destroy(&port_groups_ref); + *errorp = error; + return NULL; + } + + e = expr_simplify(e); + + if (pg_addr_set_ref) { + *pg_addr_set_ref = (!sset_is_empty(&port_groups_ref) || + !sset_is_empty(&addr_sets_ref)); + } + + sset_destroy(&addr_sets_ref); + sset_destroy(&port_groups_ref); + + *errorp = NULL; + return e; +} + static bool consider_logical_flow(const struct sbrec_logical_flow *lflow, struct hmap *dhcp_opts, struct hmap *dhcpv6_opts, @@ -629,48 +815,6 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow, return true; } - /* Translate OVN match into table of OpenFlow matches. */ - struct hmap matches; - struct expr *expr; - - 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, l_ctx_in->addr_sets, - l_ctx_in->port_groups, - &addr_sets_ref, &port_groups_ref, - lflow->logical_datapath->tunnel_key, - &error); - const char *addr_set_name; - SSET_FOR_EACH (addr_set_name, &addr_sets_ref) { - lflow_resource_add(l_ctx_out->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(l_ctx_out->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; - } - struct lookup_port_aux aux = { .sbrec_multicast_group_by_name_datapath = l_ctx_in->sbrec_multicast_group_by_name_datapath, @@ -682,131 +826,150 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow, .chassis = l_ctx_in->chassis, .active_tunnels = l_ctx_in->active_tunnels, .lflow = lflow, - .lfrr = l_ctx_out->lfrr + .lfrr = l_ctx_out->lfrr, }; - 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); - expr_destroy(expr); - if (hmap_is_empty(&matches)) { - VLOG_DBG("lflow "UUID_FMT" matches are empty, skip", - UUID_ARGS(&lflow->header_.uuid)); + struct expr *expr = NULL; + if (!l_ctx_out->lflow_cache_map) { + /* Caching is disabled. */ + expr = convert_acts_to_expr(lflow, prereqs, l_ctx_in, + l_ctx_out, NULL, &error); + if (error) { + expr_destroy(prereqs); + ovnacts_free(ovnacts.data, ovnacts.size); + ofpbuf_uninit(&ovnacts); + free(error); + return true; + } + + expr = expr_evaluate_condition(expr, is_chassis_resident_cb, &cond_aux, + NULL); + expr = expr_normalize(expr); + struct hmap matches = HMAP_INITIALIZER(&matches); + uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, + &matches); + expr_destroy(expr); + if (hmap_is_empty(&matches)) { + VLOG_DBG("lflow "UUID_FMT" matches are empty, skip", + UUID_ARGS(&lflow->header_.uuid)); + ovnacts_free(ovnacts.data, ovnacts.size); + ofpbuf_uninit(&ovnacts); + expr_matches_destroy(&matches); + return true; + } + + add_matches_to_flow_table(lflow, &matches, *l_ctx_out->conj_id_ofs, + ptable, output_ptable, &ovnacts, ingress, + l_ctx_in, l_ctx_out); + ovnacts_free(ovnacts.data, ovnacts.size); ofpbuf_uninit(&ovnacts); expr_matches_destroy(&matches); - return true; + return update_conj_id_ofs(l_ctx_out->conj_id_ofs, n_conjs); } - /* Encode OVN logical actions into OpenFlow. */ - uint64_t ofpacts_stub[1024 / 8]; - struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub); - struct ovnact_encode_params ep = { - .lookup_port = lookup_port_cb, - .tunnel_ofport = tunnel_ofport_cb, - .aux = &aux, - .is_switch = datapath_is_switch(ldp), - .group_table = l_ctx_out->group_table, - .meter_table = l_ctx_out->meter_table, - .lflow_uuid = lflow->header_.uuid, + /* Caching is enabled. */ + struct lflow_cache *lc = + lflow_cache_get(l_ctx_out->lflow_cache_map, lflow); - .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, - .ingress_ptable = OFTABLE_LOG_INGRESS_PIPELINE, - .egress_ptable = OFTABLE_LOG_EGRESS_PIPELINE, - .output_ptable = output_ptable, - .mac_bind_ptable = OFTABLE_MAC_BINDING, - .mac_lookup_ptable = OFTABLE_MAC_LOOKUP, - }; - ovnacts_encode(ovnacts.data, ovnacts.size, &ep, &ofpacts); - ovnacts_free(ovnacts.data, ovnacts.size); - ofpbuf_uninit(&ovnacts); - - uint32_t conj_id_ofs = *l_ctx_out->conj_id_ofs; - if (n_conjs) { - if (l_ctx_out->lflow_cache_map) { - struct lflow_cache *lc = - lflow_cache_get(l_ctx_out->lflow_cache_map, lflow); - if (!lc) { - lc = lflow_cache_add(l_ctx_out->lflow_cache_map, lflow); - } + if (lc && lc->type == LCACHE_T_MATCHES) { + /* 'matches' is cached. No need to do expr parsing. + * Add matches to flow table and return. */ + add_matches_to_flow_table(lflow, lc->expr_matches, lc->conj_id_ofs, + ptable, output_ptable, &ovnacts, ingress, + l_ctx_in, l_ctx_out); + ovnacts_free(ovnacts.data, ovnacts.size); + ofpbuf_uninit(&ovnacts); + expr_destroy(prereqs); + return true; + } - if (!lc->conj_id_ofs) { - lc->conj_id_ofs = *l_ctx_out->conj_id_ofs; - if (!update_conj_id_ofs(l_ctx_out->conj_id_ofs, n_conjs)) { - lc->conj_id_ofs = 0; - expr_matches_destroy(&matches); - return false; - } - } + if (!lc) { + /* Create the lflow_cache for the lflow. */ + lc = lflow_cache_add(l_ctx_out->lflow_cache_map, lflow); + } - conj_id_ofs = lc->conj_id_ofs; - } else { - /* lflow caching is disabled. */ - if (!update_conj_id_ofs(l_ctx_out->conj_id_ofs, n_conjs)) { - expr_matches_destroy(&matches); - return false; - } - } + if (lc && lc->type == LCACHE_T_EXPR) { + expr = lc->expr; } - /* Prepare the OpenFlow matches for adding to the flow table. */ - struct expr_match *m; - HMAP_FOR_EACH (m, hmap_node, &matches) { - match_set_metadata(&m->match, - htonll(lflow->logical_datapath->tunnel_key)); - if (m->match.wc.masks.conj_id) { - m->match.flow.conj_id += conj_id_ofs; - } - if (datapath_is_switch(ldp)) { - unsigned int reg_index - = (ingress ? MFF_LOG_INPORT : MFF_LOG_OUTPORT) - MFF_REG0; - int64_t port_id = m->match.flow.regs[reg_index]; - if (port_id) { - int64_t dp_id = lflow->logical_datapath->tunnel_key; - char buf[16]; - get_unique_lport_key(dp_id, port_id, buf, sizeof(buf)); - lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_PORTBINDING, buf, - &lflow->header_.uuid); - if (!sset_contains(l_ctx_in->local_lport_ids, buf)) { - VLOG_DBG("lflow "UUID_FMT - " port %s in match is not local, skip", - UUID_ARGS(&lflow->header_.uuid), - buf); - continue; - } - } + bool pg_addr_set_ref = false; + if (!expr) { + expr = convert_acts_to_expr(lflow, prereqs, l_ctx_in, l_ctx_out, + &pg_addr_set_ref, &error); + if (error) { + expr_destroy(prereqs); + ovnacts_free(ovnacts.data, ovnacts.size); + ofpbuf_uninit(&ovnacts); + free(error); + return true; } - if (!m->n) { - ofctrl_add_flow(l_ctx_out->flow_table, ptable, lflow->priority, - lflow->header_.uuid.parts[0], &m->match, &ofpacts, - &lflow->header_.uuid); - } else { - uint64_t conj_stubs[64 / 8]; - struct ofpbuf conj; + } else { + expr_destroy(prereqs); + } - ofpbuf_use_stub(&conj, conj_stubs, sizeof conj_stubs); - for (int i = 0; i < m->n; i++) { - const struct cls_conjunction *src = &m->conjunctions[i]; - struct ofpact_conjunction *dst; + ovs_assert(lc && expr); - dst = ofpact_put_CONJUNCTION(&conj); - dst->id = src->id + conj_id_ofs; - dst->clause = src->clause; - dst->n_clauses = src->n_clauses; - } + /* Cache the 'expr' only if the lflow doesn't reference a port group and + * address set. */ + if (!pg_addr_set_ref) { + /* Note: If the expr doesn't have 'is_chassis_resident, then the + * type will be set to LCACHE_T_MATCHES and 'matches' will be + * cached instead. See below. */ + lc->type = LCACHE_T_EXPR; + lc->expr = expr; + } - ofctrl_add_or_append_flow(l_ctx_out->flow_table, ptable, - lflow->priority, 0, - &m->match, &conj, &lflow->header_.uuid); - ofpbuf_uninit(&conj); + if (lc->type == LCACHE_T_EXPR) { + expr = expr_clone(lc->expr); + } + + bool is_cr_cond_present = false; + expr = expr_evaluate_condition(expr, is_chassis_resident_cb, &cond_aux, + &is_cr_cond_present); + expr = expr_normalize(expr); + struct hmap *matches = xmalloc(sizeof *matches); + uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, + matches); + expr_destroy(expr); + if (hmap_is_empty(matches)) { + VLOG_DBG("lflow "UUID_FMT" matches are empty, skip", + UUID_ARGS(&lflow->header_.uuid)); + ovnacts_free(ovnacts.data, ovnacts.size); + ofpbuf_uninit(&ovnacts); + expr_matches_destroy(matches); + free(matches); + return true; + } + + if (n_conjs && !lc->conj_id_ofs) { + lc->conj_id_ofs = *l_ctx_out->conj_id_ofs; + if (!update_conj_id_ofs(l_ctx_out->conj_id_ofs, n_conjs)) { + lc->conj_id_ofs = 0; + expr_matches_destroy(matches); + free(matches); + return false; } } - /* Clean up. */ - expr_matches_destroy(&matches); - ofpbuf_uninit(&ofpacts); + /* Encode OVN logical actions into OpenFlow. */ + add_matches_to_flow_table(lflow, matches, lc->conj_id_ofs, + ptable, output_ptable, &ovnacts, ingress, + l_ctx_in, l_ctx_out); + + if (!is_cr_cond_present && lc->type == LCACHE_T_EXPR) { + /* If 'is_chassis_resident' match is not present, then cache + * 'matches'. */ + expr_destroy(lc->expr); + lc->type = LCACHE_T_MATCHES; + lc->expr_matches = matches; + } + + if (lc->type != LCACHE_T_MATCHES) { + expr_matches_destroy(matches); + free(matches); + } + return true; } diff --git a/include/ovn/expr.h b/include/ovn/expr.h index ed7b054144..ec8e95b2d5 100644 --- a/include/ovn/expr.h +++ b/include/ovn/expr.h @@ -437,7 +437,7 @@ struct expr *expr_evaluate_condition( struct expr *, bool (*is_chassis_resident)(const void *c_aux, const char *port_name), - const void *c_aux); + const void *c_aux, bool *condition_present); struct expr *expr_normalize(struct expr *); bool expr_honors_invariants(const struct expr *); diff --git a/lib/expr.c b/lib/expr.c index bff48dfd20..3877cf9d45 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -2065,14 +2065,17 @@ expr_simplify_relational(struct expr *expr) static struct expr * expr_evaluate_condition__(struct expr *expr, bool (*is_chassis_resident)(const void *c_aux, - const char *port_name), - const void *c_aux) + const char *port_name), + const void *c_aux, bool *condition_present) { bool result; switch (expr->cond.type) { case EXPR_COND_CHASSIS_RESIDENT: result = is_chassis_resident(c_aux, expr->cond.string); + if (condition_present != NULL) { + *condition_present = true; + } break; default: @@ -2088,7 +2091,7 @@ struct expr * 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 *condition_present) { struct expr *sub, *next; @@ -2098,14 +2101,15 @@ expr_evaluate_condition(struct expr *expr, 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); + c_aux, condition_present); 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); + return expr_evaluate_condition__(expr, is_chassis_resident, c_aux, + condition_present); case EXPR_T_CMP: case EXPR_T_BOOLEAN: @@ -3425,7 +3429,8 @@ expr_parse_microflow__(struct lexer *lexer, expr_format(e, &annotated); e = expr_simplify(e); - e = expr_evaluate_condition(e, microflow_is_chassis_resident_cb, NULL); + e = expr_evaluate_condition(e, microflow_is_chassis_resident_cb, + NULL, NULL); e = expr_normalize(e); struct match m = MATCH_CATCHALL_INITIALIZER; diff --git a/tests/test-ovn.c b/tests/test-ovn.c index 544fee4c87..d94ab025d9 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -314,7 +314,7 @@ test_parse_expr__(int steps) if (steps > 1) { expr = expr_simplify(expr); expr = expr_evaluate_condition(expr, is_chassis_resident_cb, - &ports); + &ports, NULL); } if (steps > 2) { expr = expr_normalize(expr); @@ -917,7 +917,8 @@ test_tree_shape_exhaustively(struct expr *expr, struct shash *symtab, } else if (operation >= OP_SIMPLIFY) { modified = expr_simplify(expr_clone(expr)); modified = expr_evaluate_condition( - expr_clone(modified), tree_shape_is_chassis_resident_cb, NULL); + expr_clone(modified), tree_shape_is_chassis_resident_cb, + NULL, NULL); ovs_assert(expr_honors_invariants(modified)); if (operation >= OP_NORMALIZE) { diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c index 39839cb709..33afc4f43c 100644 --- a/utilities/ovn-trace.c +++ b/utilities/ovn-trace.c @@ -934,7 +934,7 @@ read_flows(void) match = expr_simplify(match); match = expr_evaluate_condition(match, ovntrace_is_chassis_resident, - NULL); + NULL, NULL); } struct ovntrace_flow *flow = xzalloc(sizeof *flow);