On Fri, Jun 2, 2023 at 12:12 AM Han Zhou <hz...@ovn.org> wrote: > > This patch ensures logical flows remain persistent between engine runs, > given there are no changes. In case of any change, it will deconstruct > and reconstruct the hmap during recompute. This functionality is needed > for future incremental processing, particularly when logical flows need > to be removed. > > Signed-off-by: Han Zhou <hz...@ovn.org> > Reviewed-by: Ales Musil <amu...@redhat.com>
Acked-by: Numan Siddique <num...@ovn.org> Numan > --- > northd/en-lflow.c | 16 ++++++++++---- > northd/northd.c | 55 +++++++++++++++++++++++++++++++---------------- > northd/northd.h | 13 +++++++++-- > ovs | 2 +- > 4 files changed, 61 insertions(+), 25 deletions(-) > > diff --git a/northd/en-lflow.c b/northd/en-lflow.c > index bed7bb001e20..081ec7c353ed 100644 > --- a/northd/en-lflow.c > +++ b/northd/en-lflow.c > @@ -30,7 +30,7 @@ > > VLOG_DEFINE_THIS_MODULE(en_lflow); > > -void en_lflow_run(struct engine_node *node, void *data OVS_UNUSED) > +void en_lflow_run(struct engine_node *node, void *data) > { > const struct engine_context *eng_ctx = engine_get_context(); > > @@ -68,13 +68,17 @@ void en_lflow_run(struct engine_node *node, void *data > OVS_UNUSED) > lflow_input.ovn_internal_version_changed = > northd_data->ovn_internal_version_changed; > > + struct lflow_data *lflow_data = data; > + lflow_data_destroy(lflow_data); > + lflow_data_init(lflow_data); > + > stopwatch_start(BUILD_LFLOWS_STOPWATCH_NAME, time_msec()); > build_bfd_table(eng_ctx->ovnsb_idl_txn, > lflow_input.nbrec_bfd_table, > lflow_input.sbrec_bfd_table, > &bfd_connections, > &northd_data->lr_ports); > - build_lflows(&lflow_input, eng_ctx->ovnsb_idl_txn); > + build_lflows(eng_ctx->ovnsb_idl_txn, &lflow_input, &lflow_data->lflows); > bfd_cleanup_connections(lflow_input.nbrec_bfd_table, > &bfd_connections); > hmap_destroy(&bfd_connections); > @@ -82,12 +86,16 @@ void en_lflow_run(struct engine_node *node, void *data > OVS_UNUSED) > > engine_set_node_state(node, EN_UPDATED); > } > + > void *en_lflow_init(struct engine_node *node OVS_UNUSED, > struct engine_arg *arg OVS_UNUSED) > { > - return NULL; > + struct lflow_data *data = xmalloc(sizeof *data); > + lflow_data_init(data); > + return data; > } > > -void en_lflow_cleanup(void *data OVS_UNUSED) > +void en_lflow_cleanup(void *data) > { > + lflow_data_destroy(data); > } > diff --git a/northd/northd.c b/northd/northd.c > index 277f4780bd20..93f126aa32b4 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -15250,6 +15250,22 @@ build_lswitch_and_lrouter_flows(const struct > ovn_datapaths *ls_datapaths, > > static ssize_t max_seen_lflow_size = 128; > > +void > +lflow_data_init(struct lflow_data *data) > +{ > + fast_hmap_size_for(&data->lflows, max_seen_lflow_size); > +} > + > +void > +lflow_data_destroy(struct lflow_data *data) > +{ > + struct ovn_lflow *lflow; > + HMAP_FOR_EACH_SAFE (lflow, hmap_node, &data->lflows) { > + ovn_lflow_destroy(&data->lflows, lflow); > + } > + hmap_destroy(&data->lflows); > +} > + > void run_update_worker_pool(int n_threads) > { > /* If number of threads has been updated (or initially set), > @@ -15279,10 +15295,10 @@ build_mcast_groups(const struct > sbrec_igmp_group_table *sbrec_igmp_group_table, > > /* Updates the Logical_Flow and Multicast_Group tables in the OVN_SB > database, > * constructing their contents based on the OVN_NB database. */ > -void build_lflows(struct lflow_input *input_data, > - struct ovsdb_idl_txn *ovnsb_txn) > +void build_lflows(struct ovsdb_idl_txn *ovnsb_txn, > + struct lflow_input *input_data, > + struct hmap *lflows) > { > - struct hmap lflows; > struct hmap mcast_groups; > struct hmap igmp_groups; > > @@ -15292,13 +15308,11 @@ void build_lflows(struct lflow_input *input_data, > input_data->ls_ports, input_data->lr_ports, > &mcast_groups, &igmp_groups); > > - fast_hmap_size_for(&lflows, max_seen_lflow_size); > - > build_lswitch_and_lrouter_flows(input_data->ls_datapaths, > input_data->lr_datapaths, > input_data->ls_ports, > input_data->lr_ports, > - input_data->port_groups, &lflows, > + input_data->port_groups, lflows, > &mcast_groups, &igmp_groups, > input_data->meter_groups, > input_data->lbs, > input_data->bfd_connections, > @@ -15311,10 +15325,10 @@ void build_lflows(struct lflow_input *input_data, > /* Parallel build may result in a suboptimal hash. Resize the > * hash to a correct size before doing lookups */ > > - hmap_expand(&lflows); > + hmap_expand(lflows); > > - if (hmap_count(&lflows) > max_seen_lflow_size) { > - max_seen_lflow_size = hmap_count(&lflows); > + if (hmap_count(lflows) > max_seen_lflow_size) { > + max_seen_lflow_size = hmap_count(lflows); > } > > stopwatch_start(LFLOWS_DP_GROUPS_STOPWATCH_NAME, time_msec()); > @@ -15332,7 +15346,7 @@ void build_lflows(struct lflow_input *input_data, > fast_hmap_size_for(&single_dp_lflows, max_seen_lflow_size); > > struct ovn_lflow *lflow; > - HMAP_FOR_EACH_SAFE (lflow, hmap_node, &lflows) { > + HMAP_FOR_EACH_SAFE (lflow, hmap_node, lflows) { > struct ovn_datapath **datapaths_array; > size_t n_datapaths; > > @@ -15360,7 +15374,7 @@ void build_lflows(struct lflow_input *input_data, > /* Logical flow should be re-hashed to allow lookups. */ > uint32_t hash = hmap_node_hash(&lflow->hmap_node); > /* Remove from lflows. */ > - hmap_remove(&lflows, &lflow->hmap_node); > + hmap_remove(lflows, &lflow->hmap_node); > hash = > ovn_logical_flow_hash_datapath(&lflow->od->sb->header_.uuid, > hash); > /* Add to single_dp_lflows. */ > @@ -15370,13 +15384,14 @@ void build_lflows(struct lflow_input *input_data, > > /* Merge multiple and single dp hashes. */ > > - fast_hmap_merge(&lflows, &single_dp_lflows); > + fast_hmap_merge(lflows, &single_dp_lflows); > > hmap_destroy(&single_dp_lflows); > > stopwatch_stop(LFLOWS_DP_GROUPS_STOPWATCH_NAME, time_msec()); > stopwatch_start(LFLOWS_TO_SB_STOPWATCH_NAME, time_msec()); > > + struct hmap lflows_temp = HMAP_INITIALIZER(&lflows_temp); > /* Push changes to the Logical_Flow table to database. */ > const struct sbrec_logical_flow *sbflow; > SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_SAFE (sbflow, > @@ -15419,7 +15434,7 @@ void build_lflows(struct lflow_input *input_data, > = !strcmp(sbflow->pipeline, "ingress") ? P_IN : P_OUT; > > lflow = ovn_lflow_find( > - &lflows, dp_group ? NULL : logical_datapath_od, > + lflows, dp_group ? NULL : logical_datapath_od, > ovn_stage_build(ovn_datapath_get_type(logical_datapath_od), > pipeline, sbflow->table_id), > sbflow->priority, sbflow->match, sbflow->actions, > @@ -15480,13 +15495,15 @@ void build_lflows(struct lflow_input *input_data, > } > > /* This lflow updated. Not needed anymore. */ > - ovn_lflow_destroy(&lflows, lflow); > + hmap_remove(lflows, &lflow->hmap_node); > + hmap_insert(&lflows_temp, &lflow->hmap_node, > + hmap_node_hash(&lflow->hmap_node)); > } else { > sbrec_logical_flow_delete(sbflow); > } > } > > - HMAP_FOR_EACH_SAFE (lflow, hmap_node, &lflows) { > + HMAP_FOR_EACH_SAFE (lflow, hmap_node, lflows) { > const char *pipeline = ovn_stage_get_pipeline_name(lflow->stage); > uint8_t table = ovn_stage_get_table(lflow->stage); > struct hmap *dp_groups; > @@ -15550,10 +15567,12 @@ void build_lflows(struct lflow_input *input_data, > } > sbrec_logical_flow_set_external_ids(sbflow, &ids); > smap_destroy(&ids); > - > - ovn_lflow_destroy(&lflows, lflow); > + hmap_remove(lflows, &lflow->hmap_node); > + hmap_insert(&lflows_temp, &lflow->hmap_node, > + hmap_node_hash(&lflow->hmap_node)); > } > - hmap_destroy(&lflows); > + hmap_swap(lflows, &lflows_temp); > + hmap_destroy(&lflows_temp); > > stopwatch_stop(LFLOWS_TO_SB_STOPWATCH_NAME, time_msec()); > struct ovn_dp_group *dpg; > diff --git a/northd/northd.h b/northd/northd.h > index f073ceb6d9c2..edccb0ca89c0 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -99,6 +99,13 @@ struct northd_data { > struct chassis_features features; > }; > > +struct lflow_data { > + struct hmap lflows; > +}; > + > +void lflow_data_init(struct lflow_data *); > +void lflow_data_destroy(struct lflow_data *); > + > struct lflow_input { > /* Northbound table references */ > const struct nbrec_bfd_table *nbrec_bfd_table; > @@ -295,8 +302,10 @@ void northd_destroy(struct northd_data *data); > void northd_init(struct northd_data *data); > void northd_indices_create(struct northd_data *data, > struct ovsdb_idl *ovnsb_idl); > -void build_lflows(struct lflow_input *input_data, > - struct ovsdb_idl_txn *ovnsb_txn); > +void build_lflows(struct ovsdb_idl_txn *ovnsb_txn, > + struct lflow_input *input_data, > + struct hmap *lflows); > + > void build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn, > const struct nbrec_bfd_table *, > const struct sbrec_bfd_table *, > diff --git a/ovs b/ovs > index 0187eadfce45..b72a7f92573a 160000 > --- a/ovs > +++ b/ovs > @@ -1 +1 @@ > -Subproject commit 0187eadfce4505d502e57c0e688b830f0a1ec728 > +Subproject commit b72a7f92573aa4e6205e57cb978532b4c04702e1 > -- > 2.30.2 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev