On Thu, Aug 28, 2025 at 10:51 AM Lorenzo Bianconi <
lorenzo.bianc...@redhat.com> wrote:
> Introduce dps_index_map dynamic_bitmap in order to track unused datapath
> indexes.
> This is required to add I-P for logical switch and logical router
> inserts.
>
> Acked-by: Mairtin O'Loingsigh <moloi...@redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
> ---
>
Hi Lorenzo,
thank you for the v6, I think there a few things missed from the API diff
see down below.
> northd/lflow-mgr.c | 8 +++++---
> northd/northd.c | 49 ++++++++++++++++++++++++++--------------------
> northd/northd.h | 5 +++--
> 3 files changed, 36 insertions(+), 26 deletions(-)
>
> diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
> index 31476286a..6a66a9718 100644
> --- a/northd/lflow-mgr.c
> +++ b/northd/lflow-mgr.c
> @@ -1007,12 +1007,12 @@ sync_lflow_to_sb(struct ovn_lflow *lflow,
>
> if (ovn_stage_to_datapath_type(lflow->stage) == DP_SWITCH) {
> n_datapaths = ods_size(ls_datapaths);
> - datapaths_array = ls_datapaths->array;
> + datapaths_array = vector_get_array(&ls_datapaths->dps);
> dp_groups = &lflow_table->ls_dp_groups;
> is_switch = true;
> } else {
> n_datapaths = ods_size(lr_datapaths);
> - datapaths_array = lr_datapaths->array;
> + datapaths_array = vector_get_array(&lr_datapaths->dps);
> dp_groups = &lflow_table->lr_dp_groups;
> is_switch = false;
> }
> @@ -1226,7 +1226,9 @@ ovn_sb_insert_or_update_logical_dp_group(
>
> sb = xmalloc(bitmap_count1(dpg_bitmap, ods_size(datapaths)) * sizeof
> *sb);
> BITMAP_FOR_EACH_1 (index, ods_size(datapaths), dpg_bitmap) {
> - sb[n++] = datapaths->array[index]->sdp->sb_dp;
> + struct ovn_datapath *od = vector_get(&datapaths->dps, index,
> + struct ovn_datapath *);
> + sb[n++] = od->sdp->sb_dp;
> }
> if (!dp_group) {
> struct uuid dpg_uuid = uuid_random();
> diff --git a/northd/northd.c b/northd/northd.c
> index bd30f5b9d..04be1c48b 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -839,18 +839,20 @@ parse_dynamic_routing_redistribute(
> static void
> ods_build_array_index(struct ovn_datapaths *datapaths)
> {
> + datapaths->dps = VECTOR_CAPACITY_INITIALIZER(struct ovn_datapath *,
> + ods_size(datapaths));
> + dynamic_bitmap_alloc(&datapaths->dps_index_map, ods_size(datapaths));
> +
> /* Assign unique sequential indexes to all datapaths. These are not
> * visible outside of the northd loop, so, unlike the tunnel keys, it
> * doesn't matter if they are different on every iteration. */
> - size_t index = 0;
> -
> - datapaths->array = xrealloc(datapaths->array,
> - ods_size(datapaths) * sizeof
> *datapaths->array);
> -
> struct ovn_datapath *od;
> HMAP_FOR_EACH (od, key_node, &datapaths->datapaths) {
> + size_t index = bitmap_scan(datapaths->dps_index_map.map, 0, 0,
> + datapaths->dps_index_map.capacity);
>
Can be replaced with "dynamic_bitmap_scan()".
+ dynamic_bitmap_set1(&datapaths->dps_index_map, index);
> od->index = index;
> - datapaths->array[index++] = od;
> + vector_push(&datapaths->dps, &od);
> od->datapaths = datapaths;
> }
> }
> @@ -3415,7 +3417,8 @@ build_lswitch_lbs_from_lrouter(struct ovn_datapaths
> *lr_datapaths,
>
> HMAP_FOR_EACH (lb_dps, hmap_node, lb_dps_map) {
> DYNAMIC_BITMAP_FOR_EACH_1 (index, &lb_dps->nb_lr_map) {
> - struct ovn_datapath *od = lr_datapaths->array[index];
> + struct ovn_datapath *od = vector_get(&lr_datapaths->dps,
> index,
> + struct ovn_datapath *);
> ovn_lb_datapaths_add_ls(lb_dps, vector_len(&od->ls_peers),
> vector_get_array(&od->ls_peers),
> ods_size(ls_datapaths));
> @@ -5015,7 +5018,7 @@ northd_handle_lb_data_changes(struct tracked_lb_data
> *trk_lb_data,
>
> size_t index;
> DYNAMIC_BITMAP_FOR_EACH_1 (index, &lb_dps->nb_ls_map) {
> - od = ls_datapaths->array[index];
> + od = vector_get(&ls_datapaths->dps, index, struct
> ovn_datapath *);
>
> /* Add the ls datapath to the northd tracked data. */
> hmapx_add(&nd_changes->ls_with_changed_lbs, od);
> @@ -5151,7 +5154,7 @@ northd_handle_lb_data_changes(struct tracked_lb_data
> *trk_lb_data,
> size_t index;
> BITMAP_FOR_EACH_1 (index, ods_size(ls_datapaths),
> lb_dps->nb_ls_map.map) {
> - od = ls_datapaths->array[index];
> + od = vector_get(&ls_datapaths->dps, index, struct
> ovn_datapath *);
>
> /* Add the ls datapath to the northd tracked data. */
> hmapx_add(&nd_changes->ls_with_changed_lbs, od);
> @@ -8094,7 +8097,8 @@ build_lb_rules(struct lflow_table *lflows, struct
> ovn_lb_datapaths *lb_dps,
>
> dp_non_meter = dynamic_bitmap_clone_map(&lb_dps->nb_ls_map);
> DYNAMIC_BITMAP_FOR_EACH_1 (index, &lb_dps->nb_ls_map) {
> - struct ovn_datapath *od = ls_datapaths->array[index];
> + struct ovn_datapath *od = vector_get(&ls_datapaths->dps,
> index,
> + struct ovn_datapath
> *);
>
> meter = copp_meter_get(COPP_REJECT, od->nbs->copp,
> meter_groups);
> @@ -12099,7 +12103,8 @@ build_gw_lrouter_nat_flows_for_lb(struct
> lrouter_nat_lb_flows_ctx *ctx,
> if (ctx->reject) {
> dp_non_meter = bitmap_clone(dp_bitmap, bitmap_len);
> BITMAP_FOR_EACH_1 (index, bitmap_len, dp_bitmap) {
> - struct ovn_datapath *od = lr_datapaths->array[index];
> + struct ovn_datapath *od = vector_get(&lr_datapaths->dps,
> index,
> + struct ovn_datapath *);
> const char *meter;
>
> meter = copp_meter_get(COPP_REJECT, od->nbr->copp,
> @@ -12236,7 +12241,8 @@ build_lrouter_nat_flows_for_lb(
> bool use_stateless_nat = smap_get_bool(&lb->nlb->options,
> "use_stateless_nat", false);
> DYNAMIC_BITMAP_FOR_EACH_1 (index, &lb_dps->nb_lr_map) {
> - struct ovn_datapath *od = lr_datapaths->array[index];
> + struct ovn_datapath *od = vector_get(&lr_datapaths->dps, index,
> + struct ovn_datapath *);
> enum lrouter_nat_lb_flow_type type;
>
> const struct lr_stateful_record *lr_stateful_rec =
> @@ -12335,7 +12341,8 @@ build_lswitch_flows_for_lb(struct ovn_lb_datapaths
> *lb_dps,
>
> size_t index;
> DYNAMIC_BITMAP_FOR_EACH_1 (index, &lb_dps->nb_ls_map) {
> - struct ovn_datapath *od = ls_datapaths->array[index];
> + struct ovn_datapath *od = vector_get(&ls_datapaths->dps,
> index,
> + struct ovn_datapath *);
>
> ovn_lflow_add_with_hint__(lflows, od,
> S_SWITCH_IN_PRE_LB, 130,
> ds_cstr(match),
> @@ -12413,7 +12420,8 @@ build_lrouter_allow_vip_traffic_template(struct
> lflow_table *lflows,
>
> size_t index;
> DYNAMIC_BITMAP_FOR_EACH_1 (index, &lb_dps->nb_lr_map) {
> - struct ovn_datapath *od = lr_dps->array[index];
> + struct ovn_datapath *od = vector_get(&lr_dps->dps, index,
> + struct ovn_datapath *);
> /* Do not drop ip traffic with destination the template VIP. */
> ds_clear(&match);
> ds_put_format(&match, "ip%d.dst == %s",
> @@ -12459,8 +12467,8 @@ build_lrouter_flows_for_lb(struct ovn_lb_datapaths
> *lb_dps,
> }
>
> DYNAMIC_BITMAP_FOR_EACH_1 (index, &lb_dps->nb_lr_map) {
> - struct ovn_datapath *od = lr_datapaths->array[index];
> -
> + struct ovn_datapath *od = vector_get(&lr_datapaths->dps,
> index,
> + struct ovn_datapath *);
> ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_DNAT,
> 130, ds_cstr(match),
> ds_cstr(action),
> NULL,
> @@ -12473,8 +12481,8 @@ build_lrouter_flows_for_lb(struct ovn_lb_datapaths
> *lb_dps,
>
> if (lb->skip_snat) {
> DYNAMIC_BITMAP_FOR_EACH_1 (index, &lb_dps->nb_lr_map) {
> - struct ovn_datapath *od = lr_datapaths->array[index];
> -
> + struct ovn_datapath *od = vector_get(&lr_datapaths->dps,
> index,
> + struct ovn_datapath *);
> ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 120,
> "flags.skip_snat_for_lb == 1 && ip", "next;",
> lb_dps->lflow_ref);
> @@ -18923,7 +18931,6 @@ static void
> ovn_datapaths_init(struct ovn_datapaths *datapaths)
> {
> hmap_init(&datapaths->datapaths);
> - datapaths->array = NULL;
> }
>
> static void
> @@ -18935,8 +18942,8 @@ ovn_datapaths_destroy(struct ovn_datapaths
> *datapaths)
> }
> hmap_destroy(&datapaths->datapaths);
>
> - free(datapaths->array);
> - datapaths->array = NULL;
> + dynamic_bitmap_free(&datapaths->dps_index_map);
> + vector_destroy(&datapaths->dps);
> }
>
> static void
> diff --git a/northd/northd.h b/northd/northd.h
> index 8f865e8b3..bb12ec781 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -93,7 +93,8 @@ struct ovn_datapaths {
> struct hmap datapaths;
>
> /* The array index of each element in 'datapaths'. */
> - struct ovn_datapath **array;
> + struct dynamic_bitmap dps_index_map;
> + struct vector dps;
> };
>
> static inline size_t
> @@ -457,7 +458,7 @@ ovn_datapaths_find_by_index(const struct ovn_datapaths
> *ovn_datapaths,
> size_t od_index)
> {
> ovs_assert(od_index <= hmap_count(&ovn_datapaths->datapaths));
> - return ovn_datapaths->array[od_index];
> + return vector_get(&ovn_datapaths->dps, od_index, struct ovn_datapath
> *);
> }
>
> struct ovn_datapath *ovn_datapath_from_sbrec(
> --
> 2.50.1
>
>
Thanks,
Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
