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
