On Thu, Jan 8, 2026 at 9:47 PM Mark Michelson via dev < [email protected]> wrote:
> This removes the "index" field from struct ovn_datapath, since we can > now use the embedded struct ovn_synced_datapath's index instead. When > adding the ovn_datapath to its own sparse array, we use the > sparse_array_add_at() function to ensure that the indexes match up. > > Signed-off-by: Mark Michelson <[email protected]> > --- > Hi Mark, thank you for the v2, I have two small comments down below. > northd/en-lr-nat.c | 2 +- > northd/en-lr-stateful.c | 2 +- > northd/en-ls-arp.c | 2 +- > northd/en-ls-stateful.c | 2 +- > northd/lb.c | 4 ++-- > northd/lflow-mgr.c | 30 +++++++++++++++--------------- > northd/northd.c | 8 ++++---- > northd/northd.h | 3 --- > 8 files changed, 25 insertions(+), 28 deletions(-) > > diff --git a/northd/en-lr-nat.c b/northd/en-lr-nat.c > index 5a1f47697..45bdbab39 100644 > --- a/northd/en-lr-nat.c > +++ b/northd/en-lr-nat.c > @@ -376,7 +376,7 @@ lr_nat_record_init(struct lr_nat_record *lrnat_rec, > const struct ovn_datapath *od, > const struct hmap *lr_ports) > { > - lrnat_rec->lr_index = od->index; > + lrnat_rec->lr_index = od->sdp->index; > lrnat_rec->nbr_uuid = od->nbr->header_.uuid; > > shash_init(&lrnat_rec->snat_ips); > diff --git a/northd/en-lr-stateful.c b/northd/en-lr-stateful.c > index 5eec1e11a..212c0641c 100644 > --- a/northd/en-lr-stateful.c > +++ b/northd/en-lr-stateful.c > @@ -464,7 +464,7 @@ lr_stateful_record_create(struct lr_stateful_table > *table, > xzalloc(sizeof *lr_stateful_rec); > lr_stateful_rec->nbr_uuid = od->nbr->header_.uuid; > lr_stateful_rec->lrnat_rec = lrnat_rec; > - lr_stateful_rec->lr_index = od->index; > + lr_stateful_rec->lr_index = od->sdp->index; > lr_stateful_rec->lflow_ref = lflow_ref_create(); > > > diff --git a/northd/en-ls-arp.c b/northd/en-ls-arp.c > index 72622fce5..d571dc941 100644 > --- a/northd/en-ls-arp.c > +++ b/northd/en-ls-arp.c > @@ -130,7 +130,7 @@ ls_arp_record_create(struct ls_arp_table *table, > { > struct ls_arp_record *ls_arp_record = xzalloc(sizeof *ls_arp_record); > > - ls_arp_record->ls_index = od->index; > + ls_arp_record->ls_index = od->sdp->index; > ls_arp_record->nbs_uuid = od->nbs->header_.uuid; > > hmapx_init(&ls_arp_record->nat_records); > diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c > index 0dea24aee..bfef7bd91 100644 > --- a/northd/en-ls-stateful.c > +++ b/northd/en-ls-stateful.c > @@ -333,7 +333,7 @@ ls_stateful_record_create(struct ls_stateful_table > *table, > { > struct ls_stateful_record *ls_stateful_rec = > xzalloc(sizeof *ls_stateful_rec); > - ls_stateful_rec->ls_index = od->index; > + ls_stateful_rec->ls_index = od->sdp->index; > ls_stateful_rec->nbs_uuid = od->nbs->header_.uuid; > uuidset_init(&ls_stateful_rec->related_acls); > ls_stateful_record_init(ls_stateful_rec, od, ls_pgs); > diff --git a/northd/lb.c b/northd/lb.c > index 919557ec4..0b9a3ee87 100644 > --- a/northd/lb.c > +++ b/northd/lb.c > @@ -655,7 +655,7 @@ ovn_lb_datapaths_add_lr(struct ovn_lb_datapaths > *lb_dps, size_t n, > { > dynamic_bitmap_realloc(&lb_dps->nb_lr_map, n_lr_datapaths); > for (size_t i = 0; i < n; i++) { > - dynamic_bitmap_set1(&lb_dps->nb_lr_map, ods[i]->index); > + dynamic_bitmap_set1(&lb_dps->nb_lr_map, ods[i]->sdp->index); > } > } > > @@ -666,7 +666,7 @@ ovn_lb_datapaths_add_ls(struct ovn_lb_datapaths > *lb_dps, size_t n, > { > dynamic_bitmap_realloc(&lb_dps->nb_ls_map, n_ls_datapaths); > for (size_t i = 0; i < n; i++) { > - dynamic_bitmap_set1(&lb_dps->nb_ls_map, ods[i]->index); > + dynamic_bitmap_set1(&lb_dps->nb_ls_map, ods[i]->sdp->index); > } > } > > diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c > index 21a0b361e..87ac0b25e 100644 > --- a/northd/lflow-mgr.c > +++ b/northd/lflow-mgr.c > @@ -497,10 +497,10 @@ lflow_table_sync_to_sb(struct lflow_table > *lflow_table, > * L3 is referenced by E1 and E2 > * L4 is referenced by just E2 > * > - * L1->dpg_bitmap = [E1->od->index, E2->od->index] > - * L2->dpg_bitmap = [E1->od->index] > - * L3->dpg_bitmap = [E1->od->index, E2->od->index] > - * L4->dpg_bitmap = [E2->od->index] > + * L1->dpg_bitmap = [E1->od->sdp->index, E2->od->index] > + * L2->dpg_bitmap = [E1->od->sdp->index] > + * L3->dpg_bitmap = [E1->od->sdp->index, E2->od->index] > + * L4->dpg_bitmap = [E2->od->sdp->index] > * > * > * When 'E' gets updated, > @@ -516,10 +516,10 @@ lflow_table_sync_to_sb(struct lflow_table > *lflow_table, > * > * bitmap status of all lflows in the lflows table > * ----------------------------------------------- > - * L1->dpg_bitmap = [E2->od->index] > + * L1->dpg_bitmap = [E2->od->sdp->index] > * L2->dpg_bitmap = [] > - * L3->dpg_bitmap = [E2->od->index] > - * L4->dpg_bitmap = [E2->od->index] > + * L3->dpg_bitmap = [E2->od->sdp->index] > + * L4->dpg_bitmap = [E2->od->sdp->index] > * > * 2. In step (2), client should generate the logical flows again for > 'E1'. > * Lets say it calls: > @@ -536,11 +536,11 @@ lflow_table_sync_to_sb(struct lflow_table > *lflow_table, > * > * bitmap status of all lflows in the lflow table after end of step > (2) > * > -------------------------------------------------------------------- > - * L1->dpg_bitmap = [E2->od->index] > + * L1->dpg_bitmap = [E2->od->sdp->index] > * L2->dpg_bitmap = [] > - * L3->dpg_bitmap = [E1->od->index, E2->od->index] > - * L4->dpg_bitmap = [E2->od->index] > - * L5->dpg_bitmap = [E1->od->index] > + * L3->dpg_bitmap = [E1->od->sdp->index, E2->od->index] > + * L4->dpg_bitmap = [E2->od->sdp->index] > + * L5->dpg_bitmap = [E1->od->sdp->index] > * > * 3. In step (3), client should sync the E1's lflows by calling > * lflow_ref_sync_lflows(E1->lflow_ref,....); > @@ -717,7 +717,7 @@ lflow_ref_sync_lflows(struct lflow_ref *lflow_ref, > * > * If a logical flow L(M, A) for the 'match' and 'actions' already exist > then > * - It will be no-op if L(M,A) was already added for the same datapath. > - * - if its a different datapath, then the datapath index (od->index) > + * - if its a different datapath, then the datapath index > (od->sdp->index) > * is set in the lflow dp group bitmap. > * > * If 'lflow_ref' is not NULL then > @@ -773,7 +773,7 @@ lflow_table_add_lflow(struct lflow_table *lflow_table, > lrn->dpgrp_bitmap = bitmap_clone(dp_bitmap, > dp_bitmap_len); > lrn->dpgrp_bitmap_len = dp_bitmap_len; > } else { > - lrn->dp_index = od->index; > + lrn->dp_index = od->sdp->index; > } > ovs_list_insert(&lflow->referenced_by, &lrn->ref_list_node); > hmap_insert(&lflow_ref->lflow_ref_nodes, &lrn->ref_node, > hash); > @@ -853,7 +853,7 @@ ovn_dp_group_create(struct ovsdb_idl_txn *ovnsb_txn, > if (!datapath_od || ovn_datapath_is_stale(datapath_od)) { > break; > } > - dynamic_bitmap_set1(&dpg_bitmap, datapath_od->index); > + dynamic_bitmap_set1(&dpg_bitmap, datapath_od->sdp->index); > } > if (!sb_group || i != sb_group->n_datapaths) { > /* No group or stale group. Not going to be used. */ > @@ -1331,7 +1331,7 @@ ovn_dp_group_add_with_reference(struct ovn_lflow > *lflow_ref, > OVS_REQUIRES(fake_hash_mutex) > { > if (od) { > - dynamic_bitmap_set1(&lflow_ref->dpg_bitmap, od->index); > + dynamic_bitmap_set1(&lflow_ref->dpg_bitmap, od->sdp->index); > } > if (dp_bitmap) { > dynamic_bitmap_or(&lflow_ref->dpg_bitmap, dp_bitmap, bitmap_len); > diff --git a/northd/northd.c b/northd/northd.c > index a6fbcc10a..a9f1686dd 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -886,7 +886,7 @@ ods_build_array_index(struct ovn_datapaths *datapaths) > * doesn't matter if they are different on every iteration. */ > struct ovn_datapath *od; > HMAP_FOR_EACH (od, key_node, &datapaths->datapaths) { > - od->index = sparse_array_add(&datapaths->dps, od); > + sparse_array_add_at(&datapaths->dps, od, od->sdp->index); > We should assert that the return value from sparse_array_add_at is NULL, it's a bug if we replace existing od at index. > od->datapaths = datapaths; > } > } > @@ -895,7 +895,7 @@ static void > ods_assign_array_index(struct ovn_datapaths *datapaths, > struct ovn_datapath *od) > { > - od->index = sparse_array_add(&datapaths->dps, od); > + sparse_array_add_at(&datapaths->dps, od, od->sdp->index); > Same here. > od->datapaths = datapaths; > } > > @@ -4915,7 +4915,7 @@ northd_handle_ls_changes(struct ovsdb_idl_txn > *ovnsb_idl_txn, > } > > hmap_remove(&nd->ls_datapaths.datapaths, &od->key_node); > - sparse_array_remove(&nd->ls_datapaths.dps, od->index); > + sparse_array_remove(&nd->ls_datapaths.dps, od->sdp->index); > > const struct sbrec_ip_multicast *ip_mcast = > ip_mcast_lookup(ni->sbrec_ip_mcast_by_dp, od->sdp->sb_dp); > @@ -5225,7 +5225,7 @@ northd_handle_lr_changes(const struct northd_input > *ni, > } > > hmap_remove(&nd->lr_datapaths.datapaths, &od->key_node); > - sparse_array_remove(&nd->lr_datapaths.dps, od->index); > + sparse_array_remove(&nd->lr_datapaths.dps, od->sdp->index); > > hmapx_add(&nd->trk_data.trk_routers.deleted, od); > } > diff --git a/northd/northd.h b/northd/northd.h > index b2575adec..8d760ab4d 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -401,9 +401,6 @@ struct ovn_datapath { > struct hmap_node key_node; /* Index on 'key'. */ > struct uuid key; /* (nbs/nbr)->header_.uuid. */ > > - size_t index; /* A unique index across all datapaths. > - * Datapath indexes start from zero. */ > - > struct ovn_datapaths *datapaths; /* The collection of datapaths that > contains this datapath. */ > > -- > 2.51.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > With that addressed: Acked-by: Ales Musil <[email protected]> Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
