diff mbox series

[ovs-dev,4/4] ovn-controller: Cache logical flow expr matches.

Message ID 20200902184628.937854-1-numans@ovn.org
State Superseded
Headers show
Series Another attempt on lflow expr caching. | expand

Commit Message

Numan Siddique Sept. 2, 2020, 6:46 p.m. UTC
From: Numan Siddique <numans@ovn.org>

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 '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 <numans@ovn.org>
---
 controller/lflow.c    | 455 ++++++++++++++++++++++++++++--------------
 include/ovn/expr.h    |   2 +-
 lib/expr.c            |  17 +-
 tests/test-ovn.c      |   5 +-
 utilities/ovn-trace.c |   2 +-
 5 files changed, 323 insertions(+), 158 deletions(-)
diff mbox series

Patch

diff --git a/controller/lflow.c b/controller/lflow.c
index 63df5611ec..7bda5915a2 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,157 @@  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);
+}
+
+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);
+    e = expr_normalize(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 +813,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 +824,148 @@  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_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);
 
-    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);
+        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);
+    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);