[ovs-dev,v3,10/16] northd: Move ovn_lb_datapaths from lib to northd module.

From: Numan Siddique <numans@ovn.org>

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 lib/lb.c        |  96 -----------------------------------------
 lib/lb.h        |  57 -------------------------
 northd/northd.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++
 northd/northd.h |  28 ++++++++++++
 4 files changed, 139 insertions(+), 153 deletions(-)

On 11/28/23 03:36, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>  lib/lb.c        |  96 -----------------------------------------
>  lib/lb.h        |  57 -------------------------
>  northd/northd.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++
>  northd/northd.h |  28 ++++++++++++
>  4 files changed, 139 insertions(+), 153 deletions(-)
> 
> diff --git a/lib/lb.c b/lib/lb.c
> index d0d562b6fb..991f20299d 100644
> --- a/lib/lb.c
> +++ b/lib/lb.c
> @@ -1071,99 +1071,3 @@ remove_ips_from_lb_ip_set(struct ovn_lb_ip_set *lb_ips,
>          }
>      }
>  }
> -
> -/* lb datapaths functions */
> -struct  ovn_lb_datapaths *
> -ovn_lb_datapaths_create(const struct ovn_northd_lb *lb, size_t n_ls_datapaths,
> -                        size_t n_lr_datapaths)
> -{
> -    struct ovn_lb_datapaths *lb_dps = xzalloc(sizeof *lb_dps);
> -    lb_dps->lb = lb;
> -    lb_dps->nb_ls_map = bitmap_allocate(n_ls_datapaths);
> -    lb_dps->nb_lr_map = bitmap_allocate(n_lr_datapaths);
> -
> -    return lb_dps;
> -}
> -
> -struct ovn_lb_datapaths *
> -ovn_lb_datapaths_find(const struct hmap *lb_dps_map,
> -                      const struct uuid *lb_uuid)
> -{
> -    struct ovn_lb_datapaths *lb_dps;
> -    size_t hash = uuid_hash(lb_uuid);
> -    HMAP_FOR_EACH_WITH_HASH (lb_dps, hmap_node, hash, lb_dps_map) {
> -        if (uuid_equals(&lb_dps->lb->nlb->header_.uuid, lb_uuid)) {
> -            return lb_dps;
> -        }
> -    }
> -    return NULL;
> -}
> -
> -void
> -ovn_lb_datapaths_destroy(struct ovn_lb_datapaths *lb_dps)
> -{
> -    bitmap_free(lb_dps->nb_lr_map);
> -    bitmap_free(lb_dps->nb_ls_map);
> -    free(lb_dps);
> -}
> -
> -void
> -ovn_lb_datapaths_add_lr(struct ovn_lb_datapaths *lb_dps, size_t n,
> -                        struct ovn_datapath **ods)
> -{
> -    for (size_t i = 0; i < n; i++) {
> -        if (!bitmap_is_set(lb_dps->nb_lr_map, ods[i]->index)) {
> -            bitmap_set1(lb_dps->nb_lr_map, ods[i]->index);
> -            lb_dps->n_nb_lr++;
> -        }
> -    }
> -}
> -
> -void
> -ovn_lb_datapaths_add_ls(struct ovn_lb_datapaths *lb_dps, size_t n,
> -                        struct ovn_datapath **ods)
> -{
> -    for (size_t i = 0; i < n; i++) {
> -        if (!bitmap_is_set(lb_dps->nb_ls_map, ods[i]->index)) {
> -            bitmap_set1(lb_dps->nb_ls_map, ods[i]->index);
> -            lb_dps->n_nb_ls++;
> -        }
> -    }
> -}
> -
> -struct ovn_lb_group_datapaths *
> -ovn_lb_group_datapaths_create(const struct ovn_lb_group *lb_group,
> -                              size_t max_ls_datapaths,
> -                              size_t max_lr_datapaths)
> -{
> -    struct ovn_lb_group_datapaths *lb_group_dps =
> -        xzalloc(sizeof *lb_group_dps);
> -    lb_group_dps->lb_group = lb_group;
> -    lb_group_dps->ls = xmalloc(max_ls_datapaths * sizeof *lb_group_dps->ls);
> -    lb_group_dps->lr = xmalloc(max_lr_datapaths * sizeof *lb_group_dps->lr);
> -
> -    return lb_group_dps;
> -}
> -
> -void
> -ovn_lb_group_datapaths_destroy(struct ovn_lb_group_datapaths *lb_group_dps)
> -{
> -    free(lb_group_dps->ls);
> -    free(lb_group_dps->lr);
> -    free(lb_group_dps);
> -}
> -
> -struct ovn_lb_group_datapaths *
> -ovn_lb_group_datapaths_find(const struct hmap *lb_group_dps_map,
> -                            const struct uuid *lb_group_uuid)
> -{
> -    struct ovn_lb_group_datapaths *lb_group_dps;
> -    size_t hash = uuid_hash(lb_group_uuid);
> -
> -    HMAP_FOR_EACH_WITH_HASH (lb_group_dps, hmap_node, hash, lb_group_dps_map) {
> -        if (uuid_equals(&lb_group_dps->lb_group->uuid, lb_group_uuid)) {
> -            return lb_group_dps;
> -        }
> -    }
> -    return NULL;
> -}
> diff --git a/lib/lb.h b/lib/lb.h
> index b8e3c1e8fb..90ac39ee5a 100644
> --- a/lib/lb.h
> +++ b/lib/lb.h
> @@ -167,63 +167,6 @@ void ovn_lb_group_reinit(
>      const struct nbrec_load_balancer_group *,
>      const struct hmap *lbs);
>  
> -struct ovn_lb_datapaths {
> -    struct hmap_node hmap_node;
> -
> -    const struct ovn_northd_lb *lb;
> -    size_t n_nb_ls;
> -    unsigned long *nb_ls_map;
> -
> -    size_t n_nb_lr;
> -    unsigned long *nb_lr_map;
> -};
> -
> -struct ovn_lb_datapaths *ovn_lb_datapaths_create(const struct ovn_northd_lb *,
> -                                                 size_t n_ls_datapaths,
> -                                                 size_t n_lr_datapaths);
> -struct ovn_lb_datapaths *ovn_lb_datapaths_find(const struct hmap *,
> -                                               const struct uuid *);
> -void ovn_lb_datapaths_destroy(struct ovn_lb_datapaths *);
> -void ovn_lb_datapaths_add_lr(struct ovn_lb_datapaths *, size_t n,
> -                             struct ovn_datapath **);
> -void ovn_lb_datapaths_add_ls(struct ovn_lb_datapaths *, size_t n,
> -                             struct ovn_datapath **);
> -
> -struct ovn_lb_group_datapaths {
> -    struct hmap_node hmap_node;
> -
> -    const struct ovn_lb_group *lb_group;
> -
> -    /* Datapaths to which 'lb_group' is applied. */
> -    size_t n_ls;
> -    struct ovn_datapath **ls;
> -    size_t n_lr;
> -    struct ovn_datapath **lr;
> -};
> -
> -struct ovn_lb_group_datapaths *ovn_lb_group_datapaths_create(
> -    const struct ovn_lb_group *, size_t max_ls_datapaths,
> -    size_t max_lr_datapaths);
> -
> -void ovn_lb_group_datapaths_destroy(struct ovn_lb_group_datapaths *);
> -struct ovn_lb_group_datapaths *ovn_lb_group_datapaths_find(
> -    const struct hmap *lb_group_dps, const struct uuid *);
> -
> -static inline void
> -ovn_lb_group_datapaths_add_ls(struct ovn_lb_group_datapaths *lbg_dps, size_t n,
> -                               struct ovn_datapath **ods)
> -{
> -    memcpy(&lbg_dps->ls[lbg_dps->n_ls], ods, n * sizeof *ods);
> -    lbg_dps->n_ls += n;
> -}
> -
> -static inline void
> -ovn_lb_group_datapaths_add_lr(struct ovn_lb_group_datapaths *lbg_dps,
> -                               struct ovn_datapath *lr)
> -{
> -    lbg_dps->lr[lbg_dps->n_lr++] = lr;
> -}
> -
>  struct ovn_controller_lb {
>      struct hmap_node hmap_node;
>  
> diff --git a/northd/northd.c b/northd/northd.c
> index 7c5fe60e9a..104068e293 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -3514,6 +3514,117 @@ build_lb_vip_actions(const struct ovn_northd_lb *lb,
>      return reject;
>  }
>  
> +/* lb datapaths functions */
> +static struct  ovn_lb_datapaths *
> +ovn_lb_datapaths_create(const struct ovn_northd_lb *lb, size_t n_ls_datapaths,
> +                        size_t n_lr_datapaths)
> +{
> +    struct ovn_lb_datapaths *lb_dps = xzalloc(sizeof *lb_dps);
> +    lb_dps->lb = lb;
> +    lb_dps->nb_ls_map = bitmap_allocate(n_ls_datapaths);
> +    lb_dps->nb_lr_map = bitmap_allocate(n_lr_datapaths);
> +

Checking how the tree looks after the last patch is applied I see that
you needed to be able to store a 'lflow_ref' pointer in 'struct
ovn_lb_datapaths'.  If we'd move the northd-specific LB code from
lib/lb.c to a new northd/lb.{ch} (or similar) that should still be
possible and it would avoid adding some of the LB specific code back to
northd.c.

What do you think?

Thanks,
Dumitru

> +    return lb_dps;
> +}
> +
> +static void
> +ovn_lb_datapaths_destroy(struct ovn_lb_datapaths *lb_dps)
> +{
> +    bitmap_free(lb_dps->nb_lr_map);
> +    bitmap_free(lb_dps->nb_ls_map);
> +    free(lb_dps);
> +}
> +
> +static void
> +ovn_lb_datapaths_add_lr(struct ovn_lb_datapaths *lb_dps, size_t n,
> +                        struct ovn_datapath **ods)
> +{
> +    for (size_t i = 0; i < n; i++) {
> +        if (!bitmap_is_set(lb_dps->nb_lr_map, ods[i]->index)) {
> +            bitmap_set1(lb_dps->nb_lr_map, ods[i]->index);
> +            lb_dps->n_nb_lr++;
> +        }
> +    }
> +}
> +
> +static void
> +ovn_lb_datapaths_add_ls(struct ovn_lb_datapaths *lb_dps, size_t n,
> +                        struct ovn_datapath **ods)
> +{
> +    for (size_t i = 0; i < n; i++) {
> +        if (!bitmap_is_set(lb_dps->nb_ls_map, ods[i]->index)) {
> +            bitmap_set1(lb_dps->nb_ls_map, ods[i]->index);
> +            lb_dps->n_nb_ls++;
> +        }
> +    }
> +}
> +
> +static struct ovn_lb_group_datapaths *
> +ovn_lb_group_datapaths_create(const struct ovn_lb_group *lb_group,
> +                              size_t max_ls_datapaths,
> +                              size_t max_lr_datapaths)
> +{
> +    struct ovn_lb_group_datapaths *lb_group_dps =
> +        xzalloc(sizeof *lb_group_dps);
> +    lb_group_dps->lb_group = lb_group;
> +    lb_group_dps->ls = xmalloc(max_ls_datapaths * sizeof *lb_group_dps->ls);
> +    lb_group_dps->lr = xmalloc(max_lr_datapaths * sizeof *lb_group_dps->lr);
> +
> +    return lb_group_dps;
> +}
> +
> +static void
> +ovn_lb_group_datapaths_destroy(struct ovn_lb_group_datapaths *lb_group_dps)
> +{
> +    free(lb_group_dps->ls);
> +    free(lb_group_dps->lr);
> +    free(lb_group_dps);
> +}
> +
> +static void
> +ovn_lb_group_datapaths_add_ls(struct ovn_lb_group_datapaths *lbg_dps, size_t n,
> +                               struct ovn_datapath **ods)
> +{
> +    memcpy(&lbg_dps->ls[lbg_dps->n_ls], ods, n * sizeof *ods);
> +    lbg_dps->n_ls += n;
> +}
> +
> +static void
> +ovn_lb_group_datapaths_add_lr(struct ovn_lb_group_datapaths *lbg_dps,
> +                               struct ovn_datapath *lr)
> +{
> +    lbg_dps->lr[lbg_dps->n_lr++] = lr;
> +}
> +
> +struct ovn_lb_datapaths *
> +ovn_lb_datapaths_find(const struct hmap *lb_dps_map,
> +                      const struct uuid *lb_uuid)
> +{
> +    struct ovn_lb_datapaths *lb_dps;
> +    size_t hash = uuid_hash(lb_uuid);
> +    HMAP_FOR_EACH_WITH_HASH (lb_dps, hmap_node, hash, lb_dps_map) {
> +        if (uuid_equals(&lb_dps->lb->nlb->header_.uuid, lb_uuid)) {
> +            return lb_dps;
> +        }
> +    }
> +    return NULL;
> +}
> +
> +struct ovn_lb_group_datapaths *
> +ovn_lb_group_datapaths_find(const struct hmap *lb_group_dps_map,
> +                            const struct uuid *lb_group_uuid)
> +{
> +    struct ovn_lb_group_datapaths *lb_group_dps;
> +    size_t hash = uuid_hash(lb_group_uuid);
> +
> +    HMAP_FOR_EACH_WITH_HASH (lb_group_dps, hmap_node, hash, lb_group_dps_map) {
> +        if (uuid_equals(&lb_group_dps->lb_group->uuid, lb_group_uuid)) {
> +            return lb_group_dps;
> +        }
> +    }
> +    return NULL;
> +}
> +
>  static void
>  build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups,
>                     struct ovn_datapaths *ls_datapaths,
> diff --git a/northd/northd.h b/northd/northd.h
> index cce465b699..e2df53e6f8 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -92,6 +92,29 @@ ods_size(const struct ovn_datapaths *datapaths)
>  
>  bool od_has_lb_vip(const struct ovn_datapath *od);
>  
> +struct ovn_lb_datapaths {
> +    struct hmap_node hmap_node;
> +
> +    const struct ovn_northd_lb *lb;
> +    size_t n_nb_ls;
> +    unsigned long *nb_ls_map;
> +
> +    size_t n_nb_lr;
> +    unsigned long *nb_lr_map;
> +};
> +
> +struct ovn_lb_group_datapaths {
> +    struct hmap_node hmap_node;
> +
> +    const struct ovn_lb_group *lb_group;
> +
> +    /* Datapaths to which 'lb_group' is applied. */
> +    size_t n_ls;
> +    struct ovn_datapath **ls;
> +    size_t n_lr;
> +    struct ovn_datapath **lr;
> +};
> +
>  struct tracked_ovn_ports {
>      /* tracked created ports.
>       * hmapx node data is 'struct ovn_port *' */
> @@ -106,6 +129,11 @@ struct tracked_ovn_ports {
>      struct hmapx deleted;
>  };
>  
> +struct ovn_lb_datapaths *ovn_lb_datapaths_find(const struct hmap *,
> +                                               const struct uuid *);
> +struct ovn_lb_group_datapaths *ovn_lb_group_datapaths_find(
> +    const struct hmap *lb_group_dps, const struct uuid *);
> +
>  struct tracked_lbs {
>      /* Tracked created or updated load balancers.
>       * hmapx node data is 'struct ovn_lb_datapaths' */