On Thu, Jun 29, 2023 at 10:06 PM Numan Siddique <[email protected]> wrote: > > On Fri, Jun 30, 2023 at 7:01 AM Han Zhou <[email protected]> wrote: > > > > On Thu, Jun 29, 2023 at 4:57 AM Dumitru Ceara <[email protected]> wrote: > > > > > > On 6/18/23 08:17, Han Zhou wrote: > > > > For incremental processing, we need to maintain SB lflow uuids in > > > > northd. For this reason, we generate the row uuid when creating the > > > > Logical_Flow record in SB DB, rather than waiting for SB DB to populate > > > > back. > > > > > > > > Signed-off-by: Han Zhou <[email protected]> > > > > --- > > > > northd/northd.c | 12 ++++++++++-- > > > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/northd/northd.c b/northd/northd.c > > > > index a45c8b53ad4e..98f528f93cfc 100644 > > > > --- a/northd/northd.c > > > > +++ b/northd/northd.c > > > > @@ -5592,6 +5592,8 @@ struct ovn_lflow { > > > > * 'dpg_bitmap'. */ > > > > struct ovn_dp_group *dpg; /* Link to unique Sb datapath group. > > */ > > > > const char *where; > > > > + > > > > + struct uuid sb_uuid; /* SB DB row uuid, specified by > > northd. */ > > > > > > Nit: can you please align this comment with the ones above? > > > > > > > }; > > > > > > > > static void ovn_lflow_destroy(struct hmap *lflows, struct ovn_lflow > > *lflow); > > > > @@ -5649,6 +5651,7 @@ ovn_lflow_init(struct ovn_lflow *lflow, struct > > ovn_datapath *od, > > > > lflow->ctrl_meter = ctrl_meter; > > > > lflow->dpg = NULL; > > > > lflow->where = where; > > > > + lflow->sb_uuid = UUID_ZERO; > > > > } > > > > > > > > /* The lflow_hash_lock is a mutex array that protects updates to the > > shared > > > > @@ -16020,6 +16023,7 @@ void build_lflows(struct ovsdb_idl_txn > > *ovnsb_txn, > > > > size_t n_datapaths; > > > > bool is_switch; > > > > > > > > + lflow->sb_uuid = sbflow->header_.uuid; > > > > is_switch = ovn_stage_to_datapath_type(lflow->stage) == > > DP_SWITCH; > > > > if (is_switch) { > > > > n_datapaths = ods_size(input_data->ls_datapaths); > > > > @@ -16095,7 +16099,9 @@ void build_lflows(struct ovsdb_idl_txn > > *ovnsb_txn, > > > > dp_groups = &lr_dp_groups; > > > > } > > > > > > > > - sbflow = sbrec_logical_flow_insert(ovnsb_txn); > > > > + lflow->sb_uuid = uuid_random(); > > > > > > If we ever decide to parallelize the lflow synchronization code this > > > will hurt performance. Until then we're fine I guess. > > > > Good point! I will add a comment to remind us before merging this. > > Interesting. Only after looking into the uuid_random() code can one > figure that it acquires a lock. > > > > > > > > > > + sbflow = sbrec_logical_flow_insert_persist_uuid(ovnsb_txn, > > > > + > > &lflow->sb_uuid); > > > > if (lflow->od) { > > > > sbrec_logical_flow_set_logical_datapath(sbflow, > > lflow->od->sb); > > > > } else { > > > > @@ -16305,7 +16311,9 @@ bool lflow_handle_northd_ls_changes(struct > > ovsdb_idl_txn *ovnsb_txn, > > > > > > > > /* Sync to SB. */ > > > > const struct sbrec_logical_flow *sbflow; > > > > - sbflow = sbrec_logical_flow_insert(ovnsb_txn); > > > > + lflow->sb_uuid = uuid_random(); > > > > + sbflow = sbrec_logical_flow_insert_persist_uuid(ovnsb_txn, > > > > + > > &lflow->sb_uuid); > > > > const char *pipeline = > > ovn_stage_get_pipeline_name(lflow->stage); > > > > uint8_t table = ovn_stage_get_table(lflow->stage); > > > > sbrec_logical_flow_set_logical_datapath(sbflow, > > lflow->od->sb); > > > > > > With the small nit about the comment indentation addressed: > > > > > > Acked-by: Dumitru Ceara <[email protected]> > > Acked-by: Numan Siddique <[email protected]> > > Numan
Thanks Dumitru and Numan. I applied this patch to main. > > > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
