The '/* Push changes to the Logical_Flow table to database. */' loop reads all the datapaths from the datapath group and checks them even if it's not required.
To check that flow has at least one valid datapath, we need to find one valid datapath. To find Sb logical flow we need a datapath type and for this we only need one datapath again. Technically, the only place where we need to check all datapaths is when we're checking certain Sb datapath group for the first time. In all other cases we can validate/discard the flow by other characteristics that we already know. The change saves a fair amount of time in this loop. Up to a few seconds in case of a big database. More comments added to make the code easier to read. Signed-off-by: Ilya Maximets <[email protected]> --- northd/ovn-northd.c | 73 ++++++++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 30 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index ae5874518..b7388d317 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -13231,21 +13231,10 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, const struct sbrec_logical_flow *sbflow, *next_sbflow; SBREC_LOGICAL_FLOW_FOR_EACH_SAFE (sbflow, next_sbflow, ctx->ovnsb_idl) { struct sbrec_logical_dp_group *dp_group = sbflow->logical_dp_group; - struct ovn_datapath **od, *logical_datapath_od = NULL; - int n_datapaths = 0; + struct ovn_datapath *logical_datapath_od = NULL; size_t i; - od = xmalloc((dp_group ? dp_group->n_datapaths + 1 : 1) * sizeof *od); - /* Check all logical datapaths from the group. */ - for (i = 0; dp_group && i < dp_group->n_datapaths; i++) { - od[n_datapaths] = ovn_datapath_from_sbrec(datapaths, - dp_group->datapaths[i]); - if (!od[n_datapaths] || ovn_datapath_is_stale(od[n_datapaths])) { - continue; - } - n_datapaths++; - } - + /* Find one valid datapath to get the datapath type. */ struct sbrec_datapath_binding *dp = sbflow->logical_datapath; if (dp) { logical_datapath_od = ovn_datapath_from_sbrec(datapaths, dp); @@ -13254,26 +13243,29 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, logical_datapath_od = NULL; } } + for (i = 0; dp_group && i < dp_group->n_datapaths; i++) { + logical_datapath_od = ovn_datapath_from_sbrec( + datapaths, dp_group->datapaths[i]); + if (logical_datapath_od + && !ovn_datapath_is_stale(logical_datapath_od)) { + break; + } + logical_datapath_od = NULL; + } - if (!n_datapaths && !logical_datapath_od) { + if (!logical_datapath_od) { /* This lflow has no valid logical datapaths. */ sbrec_logical_flow_delete(sbflow); - free(od); continue; } enum ovn_pipeline pipeline = !strcmp(sbflow->pipeline, "ingress") ? P_IN : P_OUT; - enum ovn_datapath_type dp_type; - if (n_datapaths) { - dp_type = od[0]->nbs ? DP_SWITCH : DP_ROUTER; - } else { - dp_type = logical_datapath_od->nbs ? DP_SWITCH : DP_ROUTER; - } lflow = ovn_lflow_find( - &lflows, logical_datapath_od, - ovn_stage_build(dp_type, pipeline, sbflow->table_id), + &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, sbflow->controller_meter, sbflow->hash); if (lflow) { @@ -13307,24 +13299,46 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, * updates. */ bool update_dp_group = false; - if (n_datapaths != hmapx_count(&lflow->od_group)) { - /* Groups are different. */ + if ((!lflow->dpg && dp_group) || (lflow->dpg && !dp_group)) { + /* Need to add or delete datapath group. */ update_dp_group = true; - } else if (lflow->dpg && lflow->dpg->dp_group) { + } else if (!lflow->dpg && !dp_group) { + /* No datapath group and not needed. */ + } else if (lflow->dpg->dp_group) { /* We know the datapath group in Sb that should be used. */ if (lflow->dpg->dp_group != dp_group) { /* Flow has different datapath group in the database. */ update_dp_group = true; } /* Datapath group is already up to date. */ - } else if (n_datapaths) { + } else { + /* There is a datapath group and we need to perfrom + * a full comparison. */ + struct ovn_datapath **od; + int n_datapaths = 0; + + od = xmalloc(dp_group->n_datapaths * sizeof *od); + /* Check all logical datapaths from the group. */ + for (i = 0; i < dp_group->n_datapaths; i++) { + od[n_datapaths] = ovn_datapath_from_sbrec( + datapaths, dp_group->datapaths[i]); + if (!od[n_datapaths] + || ovn_datapath_is_stale(od[n_datapaths])) { + continue; + } + n_datapaths++; + } + + if (n_datapaths != hmapx_count(&lflow->od_group)) { + update_dp_group = true; + } /* Have to compare datapath groups in full. */ - for (i = 0; i < n_datapaths; i++) { + for (i = 0; !update_dp_group && i < n_datapaths; i++) { if (od[i] && !hmapx_contains(&lflow->od_group, od[i])) { update_dp_group = true; - break; } } + free(od); } if (update_dp_group) { @@ -13341,7 +13355,6 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, } else { sbrec_logical_flow_delete(sbflow); } - free(od); } struct ovn_lflow *next_lflow; -- 2.31.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
