[ovs-dev] [OVN Patch v3] northd: Optimize dp/lflow postprocessing

Mark Gray mark.d.gray at redhat.com
Mon Oct 11 13:11:08 UTC 2021


On 11/10/2021 13:57, anton.ivanov at cambridgegreys.com wrote:
> From: Anton Ivanov <anton.ivanov at cambridgegreys.com>
> 
> 1. Compute dp group hash only if there will be dp group processing.
> 2. Remove hmapx interim storage and related hmapx computation for
> single dp flows and replace it with a pre-sized hmap.
> 
> Signed-off-by: Anton Ivanov <anton.ivanov at cambridgegreys.com>
> ---
>  northd/northd.c | 50 ++++++++++++++++++++++++++++---------------------
>  1 file changed, 29 insertions(+), 21 deletions(-)
> 
> diff --git a/northd/northd.c b/northd/northd.c
> index e42795ca0..49948d568 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -13331,10 +13331,20 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths,
>      stopwatch_start(LFLOWS_DP_GROUPS_STOPWATCH_NAME, time_msec());
>      /* Collecting all unique datapath groups. */
>      struct hmap dp_groups = HMAP_INITIALIZER(&dp_groups);
> -    struct hmapx single_dp_lflows = HMAPX_INITIALIZER(&single_dp_lflows);
> -    struct ovn_lflow *lflow;
> -    HMAP_FOR_EACH (lflow, hmap_node, &lflows) {
> -        uint32_t hash = hash_int(hmapx_count(&lflow->od_group), 0);
> +    struct hmap single_dp_lflows;
> +
> +    /* Single dp_flows will never grow bigger than lflows,
> +     * thus the two hmaps will remain the same size regardless
> +     * of how many elements we remove from lflows and add to
> +     * single_dp_lflows.
> +     * Note - lflows is always sized for at least 128 flows.
> +     */
> +    fast_hmap_size_for(&single_dp_lflows, max_seen_lflow_size);
> +
> +    struct ovn_lflow *lflow, *next_lflow;
> +    struct hmapx_node *node;
> +    HMAP_FOR_EACH_SAFE (lflow, next_lflow, hmap_node, &lflows) {
> +        uint32_t hash;
>          struct ovn_dp_group *dpg;
>  
>          ovs_assert(hmapx_count(&lflow->od_group));
> @@ -13342,17 +13352,24 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths,
>          if (hmapx_count(&lflow->od_group) == 1) {
>              /* There is only one datapath, so it should be moved out of the
>               * group to a single 'od'. */
> -            const struct hmapx_node *node;
>              HMAPX_FOR_EACH (node, &lflow->od_group) {
>                  lflow->od = node->data;
>                  break;
>              }
>              hmapx_clear(&lflow->od_group);
> -            /* Logical flow should be re-hashed later to allow lookups. */
> -            hmapx_add(&single_dp_lflows, lflow);
> +
> +            /* Logical flow should be re-hashed to allow lookups. */
> +            hash = hmap_node_hash(&lflow->hmap_node);
> +            /* Remove from lflows. */
> +            hmap_remove(&lflows, &lflow->hmap_node);
> +            hash = ovn_logical_flow_hash_datapath(&lflow->od->sb->header_.uuid,
> +                                                  hash);
> +            /* Add to single_dp_lflows. */
> +            hmap_insert_fast(&single_dp_lflows, &lflow->hmap_node, hash);
>              continue;
>          }
>  
> +        hash = hash_int(hmapx_count(&lflow->od_group), 0);
>          dpg = ovn_dp_group_find(&dp_groups, &lflow->od_group, hash);
>          if (!dpg) {
>              dpg = xzalloc(sizeof *dpg);
> @@ -13362,19 +13379,11 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths,
>          lflow->dpg = dpg;
>      }
>  
> -    /* Adding datapath to the flow hash for logical flows that have only one,
> -     * so they could be found by the southbound db record. */
> -    const struct hmapx_node *node;
> -    uint32_t hash;
> -    HMAPX_FOR_EACH (node, &single_dp_lflows) {
> -        lflow = node->data;
> -        hash = hmap_node_hash(&lflow->hmap_node);
> -        hmap_remove(&lflows, &lflow->hmap_node);
> -        hash = ovn_logical_flow_hash_datapath(&lflow->od->sb->header_.uuid,
> -                                              hash);
> -        hmap_insert(&lflows, &lflow->hmap_node, hash);
> -    }
> -    hmapx_destroy(&single_dp_lflows);
> +    /* Merge multiple and single dp hashes. */
> +
> +    fast_hmap_merge(&lflows, &single_dp_lflows);
> +
> +    hmap_destroy(&single_dp_lflows);
>  
>      /* Push changes to the Logical_Flow table to database. */
>      const struct sbrec_logical_flow *sbflow, *next_sbflow;
> @@ -13507,7 +13516,6 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths,
>      }
>  
>      stopwatch_stop(LFLOWS_DP_GROUPS_STOPWATCH_NAME, time_msec());
> -    struct ovn_lflow *next_lflow;
>      HMAP_FOR_EACH_SAFE (lflow, next_lflow, hmap_node, &lflows) {
>          const char *pipeline = ovn_stage_get_pipeline_name(lflow->stage);
>          uint8_t table = ovn_stage_get_table(lflow->stage);
> 
Acked-by: Mark D. Gray <mark.d.gray at redhat.com>



More information about the dev mailing list