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.
>
> > + 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]>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
