[ovs-dev] [PATCH ovn 2/4] ovn-controller: Persist the conjunction ids allocated for conjuctive matches.

numans at ovn.org numans at ovn.org
Wed Sep 2 18:46:14 UTC 2020


From: Numan Siddique <numans at ovn.org>

For a logical flow which results in conjuctive OF matches, we are not persisting
the allocated conjunction ids for it. There are few side effects because of this.
  - When a port group or address set gets modified, the logical flows which references
    these port groups/address sets gets reprocessed again and the resulting OpenvSwitch flows
    with conjunctive matches gets modified in the vswitchd if the conjunction id changes.

  - And because of this there is small probability of a packet getting dropped when the
    OF flows gets updated with different conjunction ids.

This patch fixes this issue by persisting the conjunction ids. Earlier, logical flow caching
support was added [1] to ovn-controller and then reverted [2] later due to some issues.

This patch takes the lflow caching approach to persist the conjunction ids. But it only
creates the cache for logical flows which result in conjunctive matches. And it doesn't
cache the expr tree.

The lflow caching is made configurable and enabled by default. Any user can disable caching
by setting 'ovn-enable-lflow-cache' to 'false' in the OVS db.
 - ovs-vsctl set open . external_ids:ovn-enable-lflow-cache=false

Note: Changing the option 'ovn-enable-lflow-cache' doesn't result in full recompute of
I-P engine. But ovn-controller updates the chassis.other_config column. And when it
receives the update2/update3 message, this results in full recompute (as any chassis
changes results in full recompute). This is the case with all the config options in OVS db.

An upcoming patch series, will attempt to add back the expr caching (addressing all the issues.)

[1] - 8795bec737b("ovn-controller: Cache logical flow expr tree for each lflow.)
[2] - 065bcf46218("ovn-controller: Revert lflow expr caching")

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1858878
Signed-off-by: Numan Siddique <numans at ovn.org>
---
 controller/chassis.c        |  22 +++++-
 controller/lflow.c          | 118 ++++++++++++++++++++++++++++++-
 controller/lflow.h          |   8 ++-
 controller/ovn-controller.c |  97 +++++++++++++++++++++++---
 tests/ovn.at                | 135 ++++++++++++++++++++++++++++++++++++
 5 files changed, 363 insertions(+), 17 deletions(-)

diff --git a/controller/chassis.c b/controller/chassis.c
index d392d4f208..7ead8d8bb2 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -86,6 +86,7 @@ struct ovs_chassis_cfg {
     const char *cms_options;
     const char *monitor_all;
     const char *chassis_macs;
+    const char *enable_lflow_cache;
 
     /* Set of encap types parsed from the 'ovn-encap-type' external-id. */
     struct sset encap_type_set;
@@ -165,6 +166,12 @@ get_monitor_all(const struct smap *ext_ids)
     return smap_get_def(ext_ids, "ovn-monitor-all", "false");
 }
 
+static const char *
+get_enable_lflow_cache(const struct smap *ext_ids)
+{
+    return smap_get_def(ext_ids, "ovn-enable-lflow-cache", "true");
+}
+
 static const char *
 get_encap_csum(const struct smap *ext_ids)
 {
@@ -286,6 +293,7 @@ chassis_parse_ovs_config(const struct ovsrec_open_vswitch_table *ovs_table,
     ovs_cfg->cms_options = get_cms_options(&cfg->external_ids);
     ovs_cfg->monitor_all = get_monitor_all(&cfg->external_ids);
     ovs_cfg->chassis_macs = get_chassis_mac_mappings(&cfg->external_ids);
+    ovs_cfg->enable_lflow_cache = get_enable_lflow_cache(&cfg->external_ids);
 
     if (!chassis_parse_ovs_encap_type(encap_type, &ovs_cfg->encap_type_set)) {
         return false;
@@ -312,12 +320,14 @@ static void
 chassis_build_other_config(struct smap *config, const char *bridge_mappings,
                            const char *datapath_type, const char *cms_options,
                            const char *monitor_all, const char *chassis_macs,
-                           const char *iface_types, bool is_interconn)
+                           const char *iface_types,
+                           const char *enable_lflow_cache, bool is_interconn)
 {
     smap_replace(config, "ovn-bridge-mappings", bridge_mappings);
     smap_replace(config, "datapath-type", datapath_type);
     smap_replace(config, "ovn-cms-options", cms_options);
     smap_replace(config, "ovn-monitor-all", monitor_all);
+    smap_replace(config, "ovn-enable-lflow-cache", enable_lflow_cache);
     smap_replace(config, "iface-types", iface_types);
     smap_replace(config, "ovn-chassis-mac-mappings", chassis_macs);
     smap_replace(config, "is-interconn", is_interconn ? "true" : "false");
@@ -332,6 +342,7 @@ chassis_other_config_changed(const char *bridge_mappings,
                              const char *cms_options,
                              const char *monitor_all,
                              const char *chassis_macs,
+                             const char *enable_lflow_cache,
                              const struct ds *iface_types,
                              bool is_interconn,
                              const struct sbrec_chassis *chassis_rec)
@@ -364,6 +375,13 @@ chassis_other_config_changed(const char *bridge_mappings,
         return true;
     }
 
+    const char *chassis_enable_lflow_cache =
+        get_enable_lflow_cache(&chassis_rec->other_config);
+
+    if (strcmp(enable_lflow_cache, chassis_enable_lflow_cache)) {
+        return true;
+    }
+
     const char *chassis_mac_mappings =
         get_chassis_mac_mappings(&chassis_rec->other_config);
     if (strcmp(chassis_macs, chassis_mac_mappings)) {
@@ -573,6 +591,7 @@ chassis_update(const struct sbrec_chassis *chassis_rec,
                                      ovs_cfg->cms_options,
                                      ovs_cfg->monitor_all,
                                      ovs_cfg->chassis_macs,
+                                     ovs_cfg->enable_lflow_cache,
                                      &ovs_cfg->iface_types,
                                      ovs_cfg->is_interconn,
                                      chassis_rec)) {
@@ -585,6 +604,7 @@ chassis_update(const struct sbrec_chassis *chassis_rec,
                                    ovs_cfg->monitor_all,
                                    ovs_cfg->chassis_macs,
                                    ds_cstr_ro(&ovs_cfg->iface_types),
+                                   ovs_cfg->enable_lflow_cache,
                                    ovs_cfg->is_interconn);
         sbrec_chassis_verify_other_config(chassis_rec);
         sbrec_chassis_set_other_config(chassis_rec, &other_config);
diff --git a/controller/lflow.c b/controller/lflow.c
index 151561210a..c2f939f90f 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -269,6 +269,70 @@ lflow_resource_destroy_lflow(struct lflow_resource_ref *lfrr,
     free(lfrn);
 }
 
+/* Represents an lflow cache which
+ *  - stores the conjunction id offset if the lflow matches
+ *    results in conjunctive OpenvSwitch flows.
+ */
+struct lflow_cache {
+    struct hmap_node node;
+    struct uuid lflow_uuid; /* key */
+    uint32_t conj_id_ofs;
+};
+
+static struct lflow_cache *
+lflow_cache_add(struct hmap *lflow_cache_map,
+               const struct sbrec_logical_flow *lflow)
+{
+    struct lflow_cache *lc = xmalloc(sizeof *lc);
+    lc->lflow_uuid = lflow->header_.uuid;
+    lc->conj_id_ofs = 0;
+    hmap_insert(lflow_cache_map, &lc->node, uuid_hash(&lc->lflow_uuid));
+    return lc;
+}
+
+static struct lflow_cache *
+lflow_cache_get(struct hmap *lflow_cache_map,
+                const struct sbrec_logical_flow *lflow)
+{
+    struct lflow_cache *lc;
+    size_t hash = uuid_hash(&lflow->header_.uuid);
+    HMAP_FOR_EACH_WITH_HASH (lc, node, hash, lflow_cache_map) {
+        if (uuid_equals(&lc->lflow_uuid, &lflow->header_.uuid)) {
+            return lc;
+        }
+    }
+
+    return NULL;
+}
+
+static void
+lflow_cache_delete(struct hmap *lflow_cache_map,
+                   const struct sbrec_logical_flow *lflow)
+{
+    struct lflow_cache *lc = lflow_cache_get(lflow_cache_map, lflow);
+    if (lc) {
+        hmap_remove(lflow_cache_map, &lc->node);
+        free(lc);
+    }
+}
+
+void
+lflow_cache_init(struct hmap *lflow_cache_map)
+{
+    hmap_init(lflow_cache_map);
+}
+
+void
+lflow_cache_destroy(struct hmap *lflow_cache_map)
+{
+    struct lflow_cache *lc;
+    HMAP_FOR_EACH_POP (lc, node, lflow_cache_map) {
+        free(lc);
+    }
+
+    hmap_destroy(lflow_cache_map);
+}
+
 /* Adds the logical flows from the Logical_Flow table to flow tables. */
 static void
 add_logical_flows(struct lflow_ctx_in *l_ctx_in,
@@ -306,6 +370,7 @@ add_logical_flows(struct lflow_ctx_in *l_ctx_in,
             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));
+            l_ctx_out->conj_id_overflow = true;
         }
     }
 
@@ -355,6 +420,9 @@ lflow_handle_changed_flows(struct lflow_ctx_in *l_ctx_in,
             /* Delete entries from lflow resource reference. */
             lflow_resource_destroy_lflow(l_ctx_out->lfrr,
                                          &lflow->header_.uuid);
+            if (l_ctx_out->lflow_cache_map) {
+                lflow_cache_delete(l_ctx_out->lflow_cache_map, lflow);
+            }
         }
     }
 
@@ -382,6 +450,7 @@ lflow_handle_changed_flows(struct lflow_ctx_in *l_ctx_in,
                                        &nd_ra_opts, &controller_event_opts,
                                        l_ctx_in, l_ctx_out)) {
                 ret = false;
+                l_ctx_out->conj_id_overflow = true;
                 break;
             }
         }
@@ -467,6 +536,7 @@ lflow_handle_changed_ref(enum ref_type ref_type, const char *ref_name,
                                    &nd_ra_opts, &controller_event_opts,
                                    l_ctx_in, l_ctx_out)) {
             ret = false;
+            l_ctx_out->conj_id_overflow = true;
             break;
         }
         *changed = true;
@@ -652,13 +722,41 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow,
     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->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;
+                }
+            }
+
+            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;
+            }
+        }
+    }
+
     /* 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 += *l_ctx_out->conj_id_ofs;
+            m->match.flow.conj_id += conj_id_ofs;
         }
         if (datapath_is_switch(ldp)) {
             unsigned int reg_index
@@ -693,7 +791,7 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow,
                 struct ofpact_conjunction *dst;
 
                 dst = ofpact_put_CONJUNCTION(&conj);
-                dst->id = src->id + *l_ctx_out->conj_id_ofs;
+                dst->id = src->id + conj_id_ofs;
                 dst->clause = src->clause;
                 dst->n_clauses = src->n_clauses;
             }
@@ -708,7 +806,7 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow,
     /* Clean up. */
     expr_matches_destroy(&matches);
     ofpbuf_uninit(&ofpacts);
-    return update_conj_id_ofs(l_ctx_out->conj_id_ofs, n_conjs);
+    return true;
 }
 
 static void
@@ -858,6 +956,19 @@ lflow_run(struct lflow_ctx_in *l_ctx_in, struct lflow_ctx_out *l_ctx_out)
 {
     COVERAGE_INC(lflow_run);
 
+    /* when lflow_run is called, it's possible that some of the logical flows
+     * are deleted. We need to delete the lflow cache for these
+     * lflows (if present), otherwise, they will not be deleted at all. */
+    if (l_ctx_out->lflow_cache_map) {
+        const struct sbrec_logical_flow *lflow;
+        SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_TRACKED (lflow,
+                                                l_ctx_in->logical_flow_table) {
+            if (sbrec_logical_flow_is_deleted(lflow)) {
+                lflow_cache_delete(l_ctx_out->lflow_cache_map, lflow);
+            }
+        }
+    }
+
     add_logical_flows(l_ctx_in, l_ctx_out);
     add_neighbor_flows(l_ctx_in->sbrec_port_binding_by_name,
                        l_ctx_in->mac_binding_table, l_ctx_in->local_datapaths,
@@ -914,6 +1025,7 @@ lflow_add_flows_for_datapath(const struct sbrec_datapath_binding *dp,
                                    &nd_ra_opts, &controller_event_opts,
                                    l_ctx_in, l_ctx_out)) {
             handled = false;
+            l_ctx_out->conj_id_overflow = true;
             break;
         }
     }
diff --git a/controller/lflow.h b/controller/lflow.h
index ae02eaf5e8..c66b318e9c 100644
--- a/controller/lflow.h
+++ b/controller/lflow.h
@@ -141,12 +141,13 @@ struct lflow_ctx_out {
     struct ovn_extend_table *group_table;
     struct ovn_extend_table *meter_table;
     struct lflow_resource_ref *lfrr;
-    struct hmap *lflow_expr_cache;
+    struct hmap *lflow_cache_map;
     uint32_t *conj_id_ofs;
+    bool conj_id_overflow;
 };
 
 void lflow_init(void);
-void lflow_run(struct lflow_ctx_in *, struct lflow_ctx_out *);
+void  lflow_run(struct lflow_ctx_in *, struct lflow_ctx_out *);
 bool lflow_handle_changed_flows(struct lflow_ctx_in *, struct lflow_ctx_out *);
 bool lflow_handle_changed_ref(enum ref_type, const char *ref_name,
                               struct lflow_ctx_in *, struct lflow_ctx_out *,
@@ -159,7 +160,8 @@ void lflow_handle_changed_neighbors(
 
 void lflow_destroy(void);
 
-void lflow_expr_destroy(struct hmap *lflow_expr_cache);
+void lflow_cache_init(struct hmap *);
+void lflow_cache_destroy(struct hmap *);
 
 bool lflow_add_flows_for_datapath(const struct sbrec_datapath_binding *,
                                   struct lflow_ctx_in *,
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 874087cfd1..31fff9f1c3 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -76,6 +76,7 @@ static unixctl_cb_func cluster_state_reset_cmd;
 static unixctl_cb_func debug_pause_execution;
 static unixctl_cb_func debug_resume_execution;
 static unixctl_cb_func debug_status_execution;
+static unixctl_cb_func flush_lflow_cache;
 
 #define DEFAULT_BRIDGE_NAME "br-int"
 #define DEFAULT_PROBE_INTERVAL_MSEC 5000
@@ -485,7 +486,8 @@ get_ofctrl_probe_interval(struct ovsdb_idl *ovs_idl)
  * updates 'sbdb_idl' with that pointer. */
 static void
 update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl,
-             bool *monitor_all_p, bool *reset_ovnsb_idl_min_index)
+             bool *monitor_all_p, bool *reset_ovnsb_idl_min_index,
+             bool *enable_lflow_cache)
 {
     const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
     if (!cfg) {
@@ -521,6 +523,11 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl,
         ovsdb_idl_reset_min_index(ovnsb_idl);
         *reset_ovnsb_idl_min_index = false;
     }
+
+    if (enable_lflow_cache != NULL) {
+        *enable_lflow_cache =
+            smap_get_bool(&cfg->external_ids, "ovn-enable-lflow-cache", true);
+    }
 }
 
 static void
@@ -811,6 +818,10 @@ enum ovs_engine_node {
     OVS_NODES
 #undef OVS_NODE
 
+struct controller_engine_ctx {
+    bool enable_lflow_cache;
+};
+
 struct ed_type_ofctrl_is_connected {
     bool connected;
 };
@@ -1542,6 +1553,12 @@ physical_flow_changes_ovs_iface_handler(struct engine_node *node, void *data)
     return true;
 }
 
+struct flow_output_persistent_data {
+    uint32_t conj_id_ofs;
+    struct hmap lflow_cache_map;
+    bool lflow_cache_enabled;
+};
+
 struct ed_type_flow_output {
     /* desired flows */
     struct ovn_desired_flow_table flow_table;
@@ -1549,10 +1566,12 @@ struct ed_type_flow_output {
     struct ovn_extend_table group_table;
     /* meter ids for QoS */
     struct ovn_extend_table meter_table;
-    /* conjunction id offset */
-    uint32_t conj_id_ofs;
     /* lflow resource cross reference */
     struct lflow_resource_ref lflow_resource_ref;
+
+    /* Data which is persistent and not cleared during
+     * full recompute. */
+    struct flow_output_persistent_data pd;
 };
 
 static void init_physical_ctx(struct engine_node *node,
@@ -1703,7 +1722,13 @@ static void init_lflow_ctx(struct engine_node *node,
     l_ctx_out->group_table = &fo->group_table;
     l_ctx_out->meter_table = &fo->meter_table;
     l_ctx_out->lfrr = &fo->lflow_resource_ref;
-    l_ctx_out->conj_id_ofs = &fo->conj_id_ofs;
+    l_ctx_out->conj_id_ofs = &fo->pd.conj_id_ofs;
+    if (fo->pd.lflow_cache_enabled) {
+        l_ctx_out->lflow_cache_map = &fo->pd.lflow_cache_map;
+    } else {
+        l_ctx_out->lflow_cache_map = NULL;
+    }
+    l_ctx_out->conj_id_overflow = false;
 }
 
 static void *
@@ -1715,8 +1740,10 @@ en_flow_output_init(struct engine_node *node OVS_UNUSED,
     ovn_desired_flow_table_init(&data->flow_table);
     ovn_extend_table_init(&data->group_table);
     ovn_extend_table_init(&data->meter_table);
-    data->conj_id_ofs = 1;
+    data->pd.conj_id_ofs = 1;
     lflow_resource_init(&data->lflow_resource_ref);
+    lflow_cache_init(&data->pd.lflow_cache_map);
+    data->pd.lflow_cache_enabled = true;
     return data;
 }
 
@@ -1728,6 +1755,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_cache_destroy(&flow_output_data->pd.lflow_cache_map);
 }
 
 static void
@@ -1761,7 +1789,6 @@ en_flow_output_run(struct engine_node *node, void *data)
     struct ovn_desired_flow_table *flow_table = &fo->flow_table;
     struct ovn_extend_table *group_table = &fo->group_table;
     struct ovn_extend_table *meter_table = &fo->meter_table;
-    uint32_t *conj_id_ofs = &fo->conj_id_ofs;
     struct lflow_resource_ref *lfrr = &fo->lflow_resource_ref;
 
     static bool first_run = true;
@@ -1774,12 +1801,39 @@ en_flow_output_run(struct engine_node *node, void *data)
         lflow_resource_clear(lfrr);
     }
 
-    *conj_id_ofs = 1;
+    struct controller_engine_ctx *ctrl_ctx = engine_get_context()->client_ctx;
+    if (fo->pd.lflow_cache_enabled && !ctrl_ctx->enable_lflow_cache) {
+        lflow_cache_destroy(&fo->pd.lflow_cache_map);
+        lflow_cache_init(&fo->pd.lflow_cache_map);
+    }
+    fo->pd.lflow_cache_enabled = ctrl_ctx->enable_lflow_cache;
+
+    if (!fo->pd.lflow_cache_enabled) {
+        fo->pd.conj_id_ofs = 1;
+    }
+
     struct lflow_ctx_in l_ctx_in;
     struct lflow_ctx_out l_ctx_out;
     init_lflow_ctx(node, rt_data, fo, &l_ctx_in, &l_ctx_out);
     lflow_run(&l_ctx_in, &l_ctx_out);
 
+    if (l_ctx_out.conj_id_overflow) {
+        /* Conjunction ids overflow. There can be many holes in between.
+         * Destroy lflow cache and call lflow_run() again. */
+        ovn_desired_flow_table_clear(flow_table);
+        ovn_extend_table_clear(group_table, false /* desired */);
+        ovn_extend_table_clear(meter_table, false /* desired */);
+        lflow_resource_clear(lfrr);
+        fo->pd.conj_id_ofs = 1;
+        lflow_cache_destroy(&fo->pd.lflow_cache_map);
+        lflow_cache_init(&fo->pd.lflow_cache_map);
+        l_ctx_out.conj_id_overflow = false;
+        lflow_run(&l_ctx_in, &l_ctx_out);
+        if (l_ctx_out.conj_id_overflow) {
+            VLOG_WARN("Conjunction id overflow.");
+        }
+    }
+
     struct physical_ctx p_ctx;
     init_physical_ctx(node, rt_data, &p_ctx);
 
@@ -2344,6 +2398,8 @@ main(int argc, char *argv[])
 
     unixctl_command_register("recompute", "", 0, 0, engine_recompute_cmd,
                              NULL);
+    unixctl_command_register("flush-lflow-cache", "", 0, 0, flush_lflow_cache,
+                             &flow_output_data->pd);
 
     bool reset_ovnsb_idl_min_index = false;
     unixctl_command_register("sb-cluster-state-reset", "", 0, 0,
@@ -2361,6 +2417,10 @@ main(int argc, char *argv[])
     unsigned int ovs_cond_seqno = UINT_MAX;
     unsigned int ovnsb_cond_seqno = UINT_MAX;
 
+    struct controller_engine_ctx ctrl_engine_ctx = {
+        .enable_lflow_cache = true
+    };
+
     /* Main loop. */
     exiting = false;
     restart = false;
@@ -2389,7 +2449,8 @@ main(int argc, char *argv[])
         }
 
         update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl, &sb_monitor_all,
-                     &reset_ovnsb_idl_min_index);
+                     &reset_ovnsb_idl_min_index,
+                     &ctrl_engine_ctx.enable_lflow_cache);
         update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl));
         ofctrl_set_probe_interval(get_ofctrl_probe_interval(ovs_idl_loop.idl));
 
@@ -2407,7 +2468,8 @@ main(int argc, char *argv[])
 
         struct engine_context eng_ctx = {
             .ovs_idl_txn = ovs_idl_txn,
-            .ovnsb_idl_txn = ovnsb_idl_txn
+            .ovnsb_idl_txn = ovnsb_idl_txn,
+            .client_ctx = &ctrl_engine_ctx
         };
 
         engine_set_context(&eng_ctx);
@@ -2644,7 +2706,8 @@ loop_done:
     if (!restart) {
         bool done = !ovsdb_idl_has_ever_connected(ovnsb_idl_loop.idl);
         while (!done) {
-            update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl, NULL, NULL);
+            update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl,
+                         NULL, NULL, NULL);
             update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl));
 
             struct ovsdb_idl_txn *ovs_idl_txn
@@ -2874,6 +2937,20 @@ engine_recompute_cmd(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED,
     unixctl_command_reply(conn, NULL);
 }
 
+static void
+flush_lflow_cache(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED,
+                     const char *argv[] OVS_UNUSED, void *arg_)
+{
+    VLOG_INFO("User triggered lflow cache flush.");
+    struct flow_output_persistent_data *fo_pd = arg_;
+    lflow_cache_destroy(&fo_pd->lflow_cache_map);
+    lflow_cache_init(&fo_pd->lflow_cache_map);
+    fo_pd->conj_id_ofs = 1;
+    engine_set_force_recompute(true);
+    poll_immediate_wake();
+    unixctl_command_reply(conn, NULL);
+}
+
 static void
 cluster_state_reset_cmd(struct unixctl_conn *conn, int argc OVS_UNUSED,
                const char *argv[] OVS_UNUSED, void *idl_reset_)
diff --git a/tests/ovn.at b/tests/ovn.at
index 5ad51c0957..1f56fc1c84 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -21409,3 +21409,138 @@ OVS_WAIT_UNTIL([test x$(as hv1 ovn-appctl -t ovn-controller debug/status) = "xru
 
 OVN_CLEANUP([hv1])
 AT_CLEANUP
+
+AT_SETUP([ovn -- lflow cache for conjunctions])
+ovn_start
+net_add n1
+sim_add hv1
+
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+ovn-nbctl ls-add sw0
+ovn-nbctl lsp-add sw0 sw0-p1
+ovn-nbctl lsp-set-addresses sw0-p1 "10:14:00:00:00:03 10.0.0.3"
+ovn-nbctl lsp-set-port-security sw0-p1 "10:14:00:00:00:03 10.0.0.3"
+
+ovn-nbctl lsp-add sw0 sw0-p2
+ovn-nbctl lsp-set-addresses sw0-p2 "10:14:00:00:00:04 10.0.0.4"
+ovn-nbctl lsp-set-port-security sw0-p2 "10:14:00:00:00:04 10.0.0.4"
+
+ovn-nbctl lsp-add sw0 sw0-p3
+ovn-nbctl lsp-set-addresses sw0-p3 "10:14:00:00:00:05 10.0.0.5"
+ovn-nbctl lsp-set-port-security sw0-p3 "10:14:00:00:00:05 10.0.0.5"
+
+ovn-nbctl lsp-add sw0 sw0-p4
+ovn-nbctl lsp-set-addresses sw0-p4 "10:14:00:00:00:06 10.0.0.6"
+ovn-nbctl lsp-set-port-security sw0-p4 "10:14:00:00:00:06 10.0.0.6"
+
+as hv1
+ovs-vsctl -- add-port br-int hv1-vif1 -- \
+    set interface hv1-vif1 external-ids:iface-id=sw0-p1 \
+    options:tx_pcap=hv1/vif1-tx.pcap \
+    options:rxq_pcap=hv1/vif1-rx.pcap \
+    ofport-request=1
+ovs-vsctl -- add-port br-int hv1-vif2 -- \
+    set interface hv1-vif2 external-ids:iface-id=sw0-p2 \
+    options:tx_pcap=hv1/vif2-tx.pcap \
+    options:rxq_pcap=hv1/vif2-rx.pcap \
+    ofport-request=2
+ovs-vsctl -- add-port br-int hv1-vif3 -- \
+    set interface hv1-vif3 external-ids:iface-id=sw0-p3 \
+    options:tx_pcap=hv1/vif3-tx.pcap \
+    options:rxq_pcap=hv1/vif3-rx.pcap \
+    ofport-request=3
+ovs-vsctl -- add-port br-int hv1-vif4 -- \
+    set interface hv1-vif4 external-ids:iface-id=sw0-p4 \
+    options:tx_pcap=hv1/vif4-tx.pcap \
+    options:rxq_pcap=hv1/vif4-rx.pcap \
+    ofport-request=4
+
+OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p1) = xup])
+OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p2) = xup])
+OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p3) = xup])
+OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p4) = xup])
+
+ovn-nbctl pg-add pg0 sw0-p1 sw0-p2
+ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && tcp.dst >= 80 && tcp.dst <= 82" allow
+ovn-nbctl --wait=hv sync
+
+OVS_WAIT_UNTIL([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id=2")])
+
+# Add sw0-p3 to the port group pg0. The conj_id should be 2.
+ovn-nbctl pg-set-ports pg0 sw0-p1 sw0-p2 sw0-p3
+OVS_WAIT_UNTIL([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id=2")])
+
+# Add sw0p4 to the port group pg0. The conj_id should be 2.
+ovn-nbctl pg-set-ports pg0 sw0-p1 sw0-p2 sw0-p3 sw0-p4
+OVS_WAIT_UNTIL([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id=2")])
+
+# Add another ACL with conjunction.
+ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && udp.dst >= 80 && udp.dst <= 82" allow
+OVS_WAIT_UNTIL([test 2 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep tcp | grep -c "conj_id=2")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep udp | grep -c "conj_id=3")])
+
+# Delete tcp ACL.
+ovn-nbctl acl-del pg0 to-lport 1002 "outport == @pg0 && ip4 && tcp.dst >= 80 && tcp.dst <= 82"
+OVS_WAIT_UNTIL([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep udp | grep -c "conj_id=3")])
+
+# Add back the tcp ACL.
+ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && tcp.dst >= 80 && tcp.dst <= 82" allow
+OVS_WAIT_UNTIL([test 2 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep udp | grep -c "conj_id=3")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep tcp | grep -c "conj_id=4")])
+
+ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && inport == @pg0 && ip4 && tcp.dst >= 84 && tcp.dst <= 86" allow
+OVS_WAIT_UNTIL([test 3 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep udp | grep -c "conj_id=3")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep tcp | grep -c "conj_id=4")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep tcp | grep -c "conj_id=5")])
+
+ovn-nbctl clear port_group pg0 acls
+OVS_WAIT_UNTIL([test 0 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id")])
+
+ovn-nbctl --wait=hv acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && tcp.dst >= 80 && tcp.dst <= 82" allow
+ovn-nbctl --wait=hv acl-add pg0 to-lport 1002 "outport == @pg0 && ip4 && udp.dst >= 80 && udp.dst <= 82" allow
+OVS_WAIT_UNTIL([test 2 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep tcp | grep -c "conj_id=6")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep udp | grep -c "conj_id=7")])
+
+# Flush the lflow cache.
+as hv1 ovn-appctl -t ovn-controller flush-lflow-cache
+OVS_WAIT_UNTIL([test 2 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id=2")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id=3")])
+
+# Disable lflow caching.
+
+as hv1 ovs-vsctl set open . external_ids:ovn-enable-lflow-cache=false
+
+# Wait until ovn-enble-lflow-cache is processed by ovn-controller.
+OVS_WAIT_UNTIL([
+    test $(ovn-sbctl get chassis hv1 other_config:ovn-enable-lflow-cache) = '"false"'
+])
+
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id=2")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id=3")])
+
+# Remove port sw0-p4 from port group.
+ovn-nbctl pg-set-ports pg0 sw0-p1 sw0-p2 sw0-p3
+OVS_WAIT_UNTIL([test 2 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id=4")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id=5")])
+
+as hv1 ovn-appctl -t ovn-controller recompute
+
+OVS_WAIT_UNTIL([test 2 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id")])
+OVS_WAIT_UNTIL([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id=2")])
+AT_CHECK([test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=44 | grep -c "conj_id=3")])
+
+OVN_CLEANUP([hv1])
+
+AT_CLEANUP
-- 
2.26.2



More information about the dev mailing list