| Message ID | 20211027172331.628389-7-mark.d.gray@redhat.com |
|---|---|
| State | Superseded |
| Delegated to: | Han Zhou |
| Headers | show |
| Series | northd: Introduce incremental processing framework | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Wed, Oct 27, 2021 at 10:24 AM Mark Gray <mark.d.gray@redhat.com> wrote: > > In order to remove the IDL loop variable from the engine context, > we do not calculate the database sequence numbers incrementally. > > Signed-off-by: Mark Gray <mark.d.gray@redhat.com> > --- > lib/inc-proc-eng.h | 2 - > northd/en-northd.c | 3 +- > northd/inc-proc-northd.c | 2 - > northd/inc-proc-northd.h | 1 - > northd/northd.c | 84 ++++------------------------------------ > northd/northd.h | 3 +- > northd/ovn-northd.c | 84 ++++++++++++++++++++++++++++++++++++++-- > 7 files changed, 90 insertions(+), 89 deletions(-) > > diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h > index 1823750c814c..6f3918ae7789 100644 > --- a/lib/inc-proc-eng.h > +++ b/lib/inc-proc-eng.h > @@ -73,8 +73,6 @@ struct engine_context { > struct ovsdb_idl_txn *ovnsb_idl_txn; > struct ovsdb_idl_txn *ovnnb_idl_txn; > > - struct ovsdb_idl_loop *ovnsb_idl_loop; > - > void *client_ctx; > }; > > diff --git a/northd/en-northd.c b/northd/en-northd.c > index 36ea890535fe..295a74721678 100644 > --- a/northd/en-northd.c > +++ b/northd/en-northd.c > @@ -119,8 +119,7 @@ void en_northd_run(struct engine_node *node, void *data) > > northd_run(data, > eng_ctx->ovnnb_idl_txn, > - eng_ctx->ovnsb_idl_txn, > - eng_ctx->ovnsb_idl_loop); > + eng_ctx->ovnsb_idl_txn); > engine_set_node_state(node, EN_UPDATED); > > } > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > index 56c05a0fd6f3..44b53432c071 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -248,7 +248,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > > void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, > struct ovsdb_idl_txn *ovnsb_txn, > - struct ovsdb_idl_loop *ovnsb_idl_loop, > bool recompute) { > engine_set_force_recompute(recompute); > engine_init_run(); > @@ -256,7 +255,6 @@ void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, > struct engine_context eng_ctx = { > .ovnnb_idl_txn = ovnnb_txn, > .ovnsb_idl_txn = ovnsb_txn, > - .ovnsb_idl_loop = ovnsb_idl_loop, > }; > > engine_set_context(&eng_ctx); > diff --git a/northd/inc-proc-northd.h b/northd/inc-proc-northd.h > index 4aeb387b7b0f..1300253791b6 100644 > --- a/northd/inc-proc-northd.h > +++ b/northd/inc-proc-northd.h > @@ -10,7 +10,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > struct ovsdb_idl_loop *sb); > void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, > struct ovsdb_idl_txn *ovnsb_txn, > - struct ovsdb_idl_loop *ovnsb_idl_loop, > bool recompute); > void inc_proc_northd_cleanup(void); > > diff --git a/northd/northd.c b/northd/northd.c > index 7bccd054cdb4..1e4273ec5e88 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -14502,9 +14502,7 @@ ovnnb_db_run(struct northd_data *data, > struct ovsdb_idl_txn *ovnnb_txn, > struct ovsdb_idl_txn *ovnsb_txn, > struct ovsdb_idl_index *sbrec_chassis_by_name, > - struct ovsdb_idl_index *sbrec_chassis_by_hostname, > - struct ovsdb_idl_loop *sb_loop, > - int64_t loop_start_time) > + struct ovsdb_idl_index *sbrec_chassis_by_hostname) > { > if (!ovnsb_txn || !ovnnb_txn) { > return; > @@ -14527,12 +14525,7 @@ ovnnb_db_run(struct northd_data *data, > if (nb->ipsec != sb->ipsec) { > sbrec_sb_global_set_ipsec(sb, nb->ipsec); > } > - if (nb->nb_cfg != sb->nb_cfg) { > - sbrec_sb_global_set_nb_cfg(sb, nb->nb_cfg); > - nbrec_nb_global_set_nb_cfg_timestamp(nb, loop_start_time); > - } > sbrec_sb_global_set_options(sb, &nb->options); > - sb_loop->next_cfg = nb->nb_cfg; > > const char *mac_addr_prefix = set_mac_prefix(smap_get(&nb->options, > "mac_prefix")); > @@ -14834,68 +14827,12 @@ handle_port_binding_changes(struct northd_data *data, > } > } > > -/* Updates the sb_cfg and hv_cfg columns in the northbound NB_Global table. */ > -static void > -update_northbound_cfg(struct northd_data *data, > - struct ovsdb_idl_loop *sb_loop, > - int64_t loop_start_time) > -{ > - /* Update northbound sb_cfg if appropriate. */ > - const struct nbrec_nb_global *nbg = nbrec_nb_global_table_first( > - data->input.nbrec_nb_global_table); > - int64_t sb_cfg = sb_loop->cur_cfg; > - if (nbg && sb_cfg && nbg->sb_cfg != sb_cfg) { > - nbrec_nb_global_set_sb_cfg(nbg, sb_cfg); > - nbrec_nb_global_set_sb_cfg_timestamp(nbg, loop_start_time); > - } > - > - /* Update northbound hv_cfg if appropriate. */ > - if (nbg) { > - /* Find minimum nb_cfg among all chassis. */ > - const struct sbrec_chassis_private *chassis_priv; > - int64_t hv_cfg = nbg->nb_cfg; > - int64_t hv_cfg_ts = 0; > - SBREC_CHASSIS_PRIVATE_TABLE_FOR_EACH (chassis_priv, > - data->input.sbrec_chassis_private_table) { > - const struct sbrec_chassis *chassis = chassis_priv->chassis; > - if (chassis) { > - if (smap_get_bool(&chassis->other_config, > - "is-remote", false)) { > - /* Skip remote chassises. */ > - continue; > - } > - } else { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > - VLOG_WARN_RL(&rl, "Chassis does not exist for " > - "Chassis_Private record, name: %s", > - chassis_priv->name); > - } > - > - if (chassis_priv->nb_cfg < hv_cfg) { > - hv_cfg = chassis_priv->nb_cfg; > - hv_cfg_ts = chassis_priv->nb_cfg_timestamp; > - } else if (chassis_priv->nb_cfg == hv_cfg && > - chassis_priv->nb_cfg_timestamp > hv_cfg_ts) { > - hv_cfg_ts = chassis_priv->nb_cfg_timestamp; > - } > - } > - > - /* Update hv_cfg. */ > - if (nbg->hv_cfg != hv_cfg) { > - nbrec_nb_global_set_hv_cfg(nbg, hv_cfg); > - nbrec_nb_global_set_hv_cfg_timestamp(nbg, hv_cfg_ts); > - } > - } > -} > - > /* Handle a fairly small set of changes in the southbound database. */ > static void > ovnsb_db_run(struct northd_data *data, > struct ovsdb_idl_txn *ovnnb_txn, > struct ovsdb_idl_txn *ovnsb_txn, > - struct ovsdb_idl_loop *sb_loop, > - struct hmap *ports, > - int64_t loop_start_time) > + struct hmap *ports) > { > if (!ovnnb_txn || > !ovsdb_idl_has_ever_connected(ovsdb_idl_txn_get_idl(ovnsb_txn))) { > @@ -14904,30 +14841,23 @@ ovnsb_db_run(struct northd_data *data, > > struct shash ha_ref_chassis_map = SHASH_INITIALIZER(&ha_ref_chassis_map); > handle_port_binding_changes(data, ovnsb_txn, ports, &ha_ref_chassis_map); > - update_northbound_cfg(data, sb_loop, loop_start_time); > if (ovnsb_txn) { > update_sb_ha_group_ref_chassis(data, &ha_ref_chassis_map); > } > shash_destroy(&ha_ref_chassis_map); > } > > -void > -northd_run(struct northd_data *data, > - struct ovsdb_idl_txn *ovnnb_txn, > - struct ovsdb_idl_txn *ovnsb_txn, > - struct ovsdb_idl_loop *sb_loop) > +void northd_run(struct northd_data *data, > + struct ovsdb_idl_txn *ovnnb_txn, > + struct ovsdb_idl_txn *ovnsb_txn) > { > - int64_t start_time = time_wall_msec(); > - > stopwatch_start(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec()); > ovnnb_db_run(data, ovnnb_txn, ovnsb_txn, > data->input.sbrec_chassis_by_name, > - data->input.sbrec_chassis_by_hostname, > - sb_loop, start_time); > + data->input.sbrec_chassis_by_hostname); > stopwatch_stop(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec()); > stopwatch_start(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec()); > - ovnsb_db_run(data, ovnnb_txn, ovnsb_txn, > - sb_loop, &data->ports, start_time); > + ovnsb_db_run(data, ovnnb_txn, ovnsb_txn, &data->ports); > stopwatch_stop(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec()); > } > > diff --git a/northd/northd.h b/northd/northd.h > index 5ceb2c6c6216..113688694cbb 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -79,8 +79,7 @@ struct northd_data { > > void northd_run(struct northd_data *data, > struct ovsdb_idl_txn *ovnnb_txn, > - struct ovsdb_idl_txn *ovnsb_txn, > - struct ovsdb_idl_loop *sb_loop); > + struct ovsdb_idl_txn *ovnsb_txn); > void northd_destroy(struct northd_data *data); > void northd_init(struct northd_data *data); > void northd_indices_create(struct northd_data *data, > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index cce757fb57d9..7b8fa115f14c 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -439,6 +439,79 @@ check_and_add_supported_dhcpv6_opts_to_sb_db(struct ovsdb_idl_txn *ovnsb_txn, > > hmap_destroy(&dhcpv6_opts_to_add); > } > + > +/* Updates the nb_cfg, sb_cfg and hv_cfg columns in NB/SB databases. */ > +static void > +update_sequence_numbers(struct ovsdb_idl *ovnnb_idl, > + struct ovsdb_idl *ovnsb_idl, > + struct ovsdb_idl_txn *ovnnb_idl_txn, > + struct ovsdb_idl_txn *ovnsb_idl_txn, > + struct ovsdb_idl_loop *sb_loop) > +{ > + int64_t loop_start_time = time_msec(); This patch all looks good except that the loop_start_time is supposed to be recorded when the loop starts, i.e. before the northd_run which could take a long time. So it needs to be recorded before the I-P engine run and passed in here as an argument. Thanks, Han > + > + /* Create rows in global tables if neccessary */ > + const struct nbrec_nb_global *nb = nbrec_nb_global_first(ovnnb_idl); > + if (!nb) { > + nb = nbrec_nb_global_insert(ovnnb_idl_txn); > + } > + const struct sbrec_sb_global *sb = sbrec_sb_global_first(ovnsb_idl); > + if (!sb) { > + sb = sbrec_sb_global_insert(ovnsb_idl_txn); > + } > + > + /* Copy nb_cfg from northbound to southbound database. > + * Also set up to update sb_cfg once our southbound transaction commits. */ > + if (nb->nb_cfg != sb->nb_cfg) { > + sbrec_sb_global_set_nb_cfg(sb, nb->nb_cfg); > + nbrec_nb_global_set_nb_cfg_timestamp(nb, loop_start_time); > + } > + sb_loop->next_cfg = nb->nb_cfg; > + > + /* Update northbound sb_cfg if appropriate. */ > + int64_t sb_cfg = sb_loop->cur_cfg; > + if (nb && sb_cfg && nb->sb_cfg != sb_cfg) { > + nbrec_nb_global_set_sb_cfg(nb, sb_cfg); > + nbrec_nb_global_set_sb_cfg_timestamp(nb, loop_start_time); > + } > + > + /* Update northbound hv_cfg if appropriate. */ > + if (nb) { > + /* Find minimum nb_cfg among all chassis. */ > + const struct sbrec_chassis_private *chassis_priv; > + int64_t hv_cfg = nb->nb_cfg; > + int64_t hv_cfg_ts = 0; > + SBREC_CHASSIS_PRIVATE_FOR_EACH (chassis_priv, ovnsb_idl) { > + const struct sbrec_chassis *chassis = chassis_priv->chassis; > + if (chassis) { > + if (smap_get_bool(&chassis->other_config, > + "is-remote", false)) { > + /* Skip remote chassises. */ > + continue; > + } > + } else { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > + VLOG_WARN_RL(&rl, "Chassis does not exist for " > + "Chassis_Private record, name: %s", > + chassis_priv->name); > + } > + > + if (chassis_priv->nb_cfg < hv_cfg) { > + hv_cfg = chassis_priv->nb_cfg; > + hv_cfg_ts = chassis_priv->nb_cfg_timestamp; > + } else if (chassis_priv->nb_cfg == hv_cfg && > + chassis_priv->nb_cfg_timestamp > hv_cfg_ts) { > + hv_cfg_ts = chassis_priv->nb_cfg_timestamp; > + } > + } > + > + /* Update hv_cfg. */ > + if (nb->hv_cfg != hv_cfg) { > + nbrec_nb_global_set_hv_cfg(nb, hv_cfg); > + nbrec_nb_global_set_hv_cfg_timestamp(nb, hv_cfg_ts); > + } > + } > +} > > static void > add_column_noalert(struct ovsdb_idl *idl, > @@ -995,9 +1068,7 @@ main(int argc, char *argv[]) > } > > if (ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) { > - inc_proc_northd_run(ovnnb_txn, ovnsb_txn, > - &ovnsb_idl_loop, > - recompute); > + inc_proc_northd_run(ovnnb_txn, ovnsb_txn, recompute); > recompute = false; > if (ovnsb_txn) { > check_and_add_supported_dhcp_opts_to_sb_db( > @@ -1007,6 +1078,13 @@ main(int argc, char *argv[]) > check_and_update_rbac( > ovnsb_txn, ovnsb_idl_loop.idl); > } > + > + if (ovnnb_txn && ovnsb_txn) { > + update_sequence_numbers(ovnnb_idl_loop.idl, > + ovnsb_idl_loop.idl, > + ovnnb_txn, ovnsb_txn, > + &ovnsb_idl_loop); > + } > } > } else { > /* ovn-northd is paused > -- > 2.27.0 >
On Thu, Nov 4, 2021 at 12:02 AM Han Zhou <hzhou@ovn.org> wrote: > On Wed, Oct 27, 2021 at 10:24 AM Mark Gray <mark.d.gray@redhat.com> wrote: > > > > In order to remove the IDL loop variable from the engine context, > > we do not calculate the database sequence numbers incrementally. > > > > Signed-off-by: Mark Gray <mark.d.gray@redhat.com> > > --- > > lib/inc-proc-eng.h | 2 - > > northd/en-northd.c | 3 +- > > northd/inc-proc-northd.c | 2 - > > northd/inc-proc-northd.h | 1 - > > northd/northd.c | 84 ++++------------------------------------ > > northd/northd.h | 3 +- > > northd/ovn-northd.c | 84 ++++++++++++++++++++++++++++++++++++++-- > > 7 files changed, 90 insertions(+), 89 deletions(-) > > > > diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h > > index 1823750c814c..6f3918ae7789 100644 > > --- a/lib/inc-proc-eng.h > > +++ b/lib/inc-proc-eng.h > > @@ -73,8 +73,6 @@ struct engine_context { > > struct ovsdb_idl_txn *ovnsb_idl_txn; > > struct ovsdb_idl_txn *ovnnb_idl_txn; > > > > - struct ovsdb_idl_loop *ovnsb_idl_loop; > > - > > void *client_ctx; > > }; > > > > diff --git a/northd/en-northd.c b/northd/en-northd.c > > index 36ea890535fe..295a74721678 100644 > > --- a/northd/en-northd.c > > +++ b/northd/en-northd.c > > @@ -119,8 +119,7 @@ void en_northd_run(struct engine_node *node, void > *data) > > > > northd_run(data, > > eng_ctx->ovnnb_idl_txn, > > - eng_ctx->ovnsb_idl_txn, > > - eng_ctx->ovnsb_idl_loop); > > + eng_ctx->ovnsb_idl_txn); > > engine_set_node_state(node, EN_UPDATED); > > > > } > > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > > index 56c05a0fd6f3..44b53432c071 100644 > > --- a/northd/inc-proc-northd.c > > +++ b/northd/inc-proc-northd.c > > @@ -248,7 +248,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > > > > void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, > > struct ovsdb_idl_txn *ovnsb_txn, > > - struct ovsdb_idl_loop *ovnsb_idl_loop, > > bool recompute) { > > engine_set_force_recompute(recompute); > > engine_init_run(); > > @@ -256,7 +255,6 @@ void inc_proc_northd_run(struct ovsdb_idl_txn > *ovnnb_txn, > > struct engine_context eng_ctx = { > > .ovnnb_idl_txn = ovnnb_txn, > > .ovnsb_idl_txn = ovnsb_txn, > > - .ovnsb_idl_loop = ovnsb_idl_loop, > > }; > > > > engine_set_context(&eng_ctx); > > diff --git a/northd/inc-proc-northd.h b/northd/inc-proc-northd.h > > index 4aeb387b7b0f..1300253791b6 100644 > > --- a/northd/inc-proc-northd.h > > +++ b/northd/inc-proc-northd.h > > @@ -10,7 +10,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > > struct ovsdb_idl_loop *sb); > > void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, > > struct ovsdb_idl_txn *ovnsb_txn, > > - struct ovsdb_idl_loop *ovnsb_idl_loop, > > bool recompute); > > void inc_proc_northd_cleanup(void); > > > > diff --git a/northd/northd.c b/northd/northd.c > > index 7bccd054cdb4..1e4273ec5e88 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -14502,9 +14502,7 @@ ovnnb_db_run(struct northd_data *data, > > struct ovsdb_idl_txn *ovnnb_txn, > > struct ovsdb_idl_txn *ovnsb_txn, > > struct ovsdb_idl_index *sbrec_chassis_by_name, > > - struct ovsdb_idl_index *sbrec_chassis_by_hostname, > > - struct ovsdb_idl_loop *sb_loop, > > - int64_t loop_start_time) > > + struct ovsdb_idl_index *sbrec_chassis_by_hostname) > > { > > if (!ovnsb_txn || !ovnnb_txn) { > > return; > > @@ -14527,12 +14525,7 @@ ovnnb_db_run(struct northd_data *data, > > if (nb->ipsec != sb->ipsec) { > > sbrec_sb_global_set_ipsec(sb, nb->ipsec); > > } > > - if (nb->nb_cfg != sb->nb_cfg) { > > - sbrec_sb_global_set_nb_cfg(sb, nb->nb_cfg); > > - nbrec_nb_global_set_nb_cfg_timestamp(nb, loop_start_time); > > - } > > sbrec_sb_global_set_options(sb, &nb->options); > > - sb_loop->next_cfg = nb->nb_cfg; > > > > const char *mac_addr_prefix = set_mac_prefix(smap_get(&nb->options, > > > "mac_prefix")); > > @@ -14834,68 +14827,12 @@ handle_port_binding_changes(struct northd_data > *data, > > } > > } > > > > -/* Updates the sb_cfg and hv_cfg columns in the northbound NB_Global > table. */ > > -static void > > -update_northbound_cfg(struct northd_data *data, > > - struct ovsdb_idl_loop *sb_loop, > > - int64_t loop_start_time) > > -{ > > - /* Update northbound sb_cfg if appropriate. */ > > - const struct nbrec_nb_global *nbg = nbrec_nb_global_table_first( > > - data->input.nbrec_nb_global_table); > > - int64_t sb_cfg = sb_loop->cur_cfg; > > - if (nbg && sb_cfg && nbg->sb_cfg != sb_cfg) { > > - nbrec_nb_global_set_sb_cfg(nbg, sb_cfg); > > - nbrec_nb_global_set_sb_cfg_timestamp(nbg, loop_start_time); > > - } > > - > > - /* Update northbound hv_cfg if appropriate. */ > > - if (nbg) { > > - /* Find minimum nb_cfg among all chassis. */ > > - const struct sbrec_chassis_private *chassis_priv; > > - int64_t hv_cfg = nbg->nb_cfg; > > - int64_t hv_cfg_ts = 0; > > - SBREC_CHASSIS_PRIVATE_TABLE_FOR_EACH (chassis_priv, > > - > data->input.sbrec_chassis_private_table) { > > - const struct sbrec_chassis *chassis = chassis_priv->chassis; > > - if (chassis) { > > - if (smap_get_bool(&chassis->other_config, > > - "is-remote", false)) { > > - /* Skip remote chassises. */ > > - continue; > > - } > > - } else { > > - static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(1, 1); > > - VLOG_WARN_RL(&rl, "Chassis does not exist for " > > - "Chassis_Private record, name: %s", > > - chassis_priv->name); > > - } > > - > > - if (chassis_priv->nb_cfg < hv_cfg) { > > - hv_cfg = chassis_priv->nb_cfg; > > - hv_cfg_ts = chassis_priv->nb_cfg_timestamp; > > - } else if (chassis_priv->nb_cfg == hv_cfg && > > - chassis_priv->nb_cfg_timestamp > hv_cfg_ts) { > > - hv_cfg_ts = chassis_priv->nb_cfg_timestamp; > > - } > > - } > > - > > - /* Update hv_cfg. */ > > - if (nbg->hv_cfg != hv_cfg) { > > - nbrec_nb_global_set_hv_cfg(nbg, hv_cfg); > > - nbrec_nb_global_set_hv_cfg_timestamp(nbg, hv_cfg_ts); > > - } > > - } > > -} > > - > > /* Handle a fairly small set of changes in the southbound database. */ > > static void > > ovnsb_db_run(struct northd_data *data, > > struct ovsdb_idl_txn *ovnnb_txn, > > struct ovsdb_idl_txn *ovnsb_txn, > > - struct ovsdb_idl_loop *sb_loop, > > - struct hmap *ports, > > - int64_t loop_start_time) > > + struct hmap *ports) > > { > > if (!ovnnb_txn || > > !ovsdb_idl_has_ever_connected(ovsdb_idl_txn_get_idl(ovnsb_txn))) > { > > @@ -14904,30 +14841,23 @@ ovnsb_db_run(struct northd_data *data, > > > > struct shash ha_ref_chassis_map = > SHASH_INITIALIZER(&ha_ref_chassis_map); > > handle_port_binding_changes(data, ovnsb_txn, ports, > &ha_ref_chassis_map); > > - update_northbound_cfg(data, sb_loop, loop_start_time); > > if (ovnsb_txn) { > > update_sb_ha_group_ref_chassis(data, &ha_ref_chassis_map); > > } > > shash_destroy(&ha_ref_chassis_map); > > } > > > > -void > > -northd_run(struct northd_data *data, > > - struct ovsdb_idl_txn *ovnnb_txn, > > - struct ovsdb_idl_txn *ovnsb_txn, > > - struct ovsdb_idl_loop *sb_loop) > > +void northd_run(struct northd_data *data, > > + struct ovsdb_idl_txn *ovnnb_txn, > > + struct ovsdb_idl_txn *ovnsb_txn) > > { > > - int64_t start_time = time_wall_msec(); > > - > > stopwatch_start(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec()); > > ovnnb_db_run(data, ovnnb_txn, ovnsb_txn, > > data->input.sbrec_chassis_by_name, > > - data->input.sbrec_chassis_by_hostname, > > - sb_loop, start_time); > > + data->input.sbrec_chassis_by_hostname); > > stopwatch_stop(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec()); > > stopwatch_start(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec()); > > - ovnsb_db_run(data, ovnnb_txn, ovnsb_txn, > > - sb_loop, &data->ports, start_time); > > + ovnsb_db_run(data, ovnnb_txn, ovnsb_txn, &data->ports); > > stopwatch_stop(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec()); > > } > > > > diff --git a/northd/northd.h b/northd/northd.h > > index 5ceb2c6c6216..113688694cbb 100644 > > --- a/northd/northd.h > > +++ b/northd/northd.h > > @@ -79,8 +79,7 @@ struct northd_data { > > > > void northd_run(struct northd_data *data, > > struct ovsdb_idl_txn *ovnnb_txn, > > - struct ovsdb_idl_txn *ovnsb_txn, > > - struct ovsdb_idl_loop *sb_loop); > > + struct ovsdb_idl_txn *ovnsb_txn); > > void northd_destroy(struct northd_data *data); > > void northd_init(struct northd_data *data); > > void northd_indices_create(struct northd_data *data, > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index cce757fb57d9..7b8fa115f14c 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -439,6 +439,79 @@ check_and_add_supported_dhcpv6_opts_to_sb_db(struct > ovsdb_idl_txn *ovnsb_txn, > > > > hmap_destroy(&dhcpv6_opts_to_add); > > } > > + > > +/* Updates the nb_cfg, sb_cfg and hv_cfg columns in NB/SB databases. */ > > +static void > > +update_sequence_numbers(struct ovsdb_idl *ovnnb_idl, > > + struct ovsdb_idl *ovnsb_idl, > > + struct ovsdb_idl_txn *ovnnb_idl_txn, > > + struct ovsdb_idl_txn *ovnsb_idl_txn, > > + struct ovsdb_idl_loop *sb_loop) > > +{ > > + int64_t loop_start_time = time_msec(); > > This patch all looks good except that the loop_start_time is supposed to be > recorded when the loop starts, i.e. before the northd_run which could take > a long time. So it needs to be recorded before the I-P engine run and > passed in here as an argument. > > Good catch :). I will capture it just before inc_proc_northd_run() > Thanks, > Han > > > + > > + /* Create rows in global tables if neccessary */ > > + const struct nbrec_nb_global *nb = nbrec_nb_global_first(ovnnb_idl); > > + if (!nb) { > > + nb = nbrec_nb_global_insert(ovnnb_idl_txn); > > + } > > + const struct sbrec_sb_global *sb = sbrec_sb_global_first(ovnsb_idl); > > + if (!sb) { > > + sb = sbrec_sb_global_insert(ovnsb_idl_txn); > > + } > > + > > + /* Copy nb_cfg from northbound to southbound database. > > + * Also set up to update sb_cfg once our southbound transaction > commits. */ > > + if (nb->nb_cfg != sb->nb_cfg) { > > + sbrec_sb_global_set_nb_cfg(sb, nb->nb_cfg); > > + nbrec_nb_global_set_nb_cfg_timestamp(nb, loop_start_time); > > + } > > + sb_loop->next_cfg = nb->nb_cfg; > > + > > + /* Update northbound sb_cfg if appropriate. */ > > + int64_t sb_cfg = sb_loop->cur_cfg; > > + if (nb && sb_cfg && nb->sb_cfg != sb_cfg) { > > + nbrec_nb_global_set_sb_cfg(nb, sb_cfg); > > + nbrec_nb_global_set_sb_cfg_timestamp(nb, loop_start_time); > > + } > > + > > + /* Update northbound hv_cfg if appropriate. */ > > + if (nb) { > > + /* Find minimum nb_cfg among all chassis. */ > > + const struct sbrec_chassis_private *chassis_priv; > > + int64_t hv_cfg = nb->nb_cfg; > > + int64_t hv_cfg_ts = 0; > > + SBREC_CHASSIS_PRIVATE_FOR_EACH (chassis_priv, ovnsb_idl) { > > + const struct sbrec_chassis *chassis = chassis_priv->chassis; > > + if (chassis) { > > + if (smap_get_bool(&chassis->other_config, > > + "is-remote", false)) { > > + /* Skip remote chassises. */ > > + continue; > > + } > > + } else { > > + static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(1, 1); > > + VLOG_WARN_RL(&rl, "Chassis does not exist for " > > + "Chassis_Private record, name: %s", > > + chassis_priv->name); > > + } > > + > > + if (chassis_priv->nb_cfg < hv_cfg) { > > + hv_cfg = chassis_priv->nb_cfg; > > + hv_cfg_ts = chassis_priv->nb_cfg_timestamp; > > + } else if (chassis_priv->nb_cfg == hv_cfg && > > + chassis_priv->nb_cfg_timestamp > hv_cfg_ts) { > > + hv_cfg_ts = chassis_priv->nb_cfg_timestamp; > > + } > > + } > > + > > + /* Update hv_cfg. */ > > + if (nb->hv_cfg != hv_cfg) { > > + nbrec_nb_global_set_hv_cfg(nb, hv_cfg); > > + nbrec_nb_global_set_hv_cfg_timestamp(nb, hv_cfg_ts); > > + } > > + } > > +} > > > > static void > > add_column_noalert(struct ovsdb_idl *idl, > > @@ -995,9 +1068,7 @@ main(int argc, char *argv[]) > > } > > > > if (ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) { > > - inc_proc_northd_run(ovnnb_txn, ovnsb_txn, > > - &ovnsb_idl_loop, > > - recompute); > > + inc_proc_northd_run(ovnnb_txn, ovnsb_txn, recompute); > > recompute = false; > > if (ovnsb_txn) { > > check_and_add_supported_dhcp_opts_to_sb_db( > > @@ -1007,6 +1078,13 @@ main(int argc, char *argv[]) > > check_and_update_rbac( > > ovnsb_txn, ovnsb_idl_loop.idl); > > } > > + > > + if (ovnnb_txn && ovnsb_txn) { > > + update_sequence_numbers(ovnnb_idl_loop.idl, > > + ovnsb_idl_loop.idl, > > + ovnnb_txn, ovnsb_txn, > > + &ovnsb_idl_loop); > > + } > > } > > } else { > > /* ovn-northd is paused > > -- > > 2.27.0 > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h index 1823750c814c..6f3918ae7789 100644 --- a/lib/inc-proc-eng.h +++ b/lib/inc-proc-eng.h @@ -73,8 +73,6 @@ struct engine_context { struct ovsdb_idl_txn *ovnsb_idl_txn; struct ovsdb_idl_txn *ovnnb_idl_txn; - struct ovsdb_idl_loop *ovnsb_idl_loop; - void *client_ctx; }; diff --git a/northd/en-northd.c b/northd/en-northd.c index 36ea890535fe..295a74721678 100644 --- a/northd/en-northd.c +++ b/northd/en-northd.c @@ -119,8 +119,7 @@ void en_northd_run(struct engine_node *node, void *data) northd_run(data, eng_ctx->ovnnb_idl_txn, - eng_ctx->ovnsb_idl_txn, - eng_ctx->ovnsb_idl_loop); + eng_ctx->ovnsb_idl_txn); engine_set_node_state(node, EN_UPDATED); } diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 56c05a0fd6f3..44b53432c071 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -248,7 +248,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, struct ovsdb_idl_txn *ovnsb_txn, - struct ovsdb_idl_loop *ovnsb_idl_loop, bool recompute) { engine_set_force_recompute(recompute); engine_init_run(); @@ -256,7 +255,6 @@ void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, struct engine_context eng_ctx = { .ovnnb_idl_txn = ovnnb_txn, .ovnsb_idl_txn = ovnsb_txn, - .ovnsb_idl_loop = ovnsb_idl_loop, }; engine_set_context(&eng_ctx); diff --git a/northd/inc-proc-northd.h b/northd/inc-proc-northd.h index 4aeb387b7b0f..1300253791b6 100644 --- a/northd/inc-proc-northd.h +++ b/northd/inc-proc-northd.h @@ -10,7 +10,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, struct ovsdb_idl_loop *sb); void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, struct ovsdb_idl_txn *ovnsb_txn, - struct ovsdb_idl_loop *ovnsb_idl_loop, bool recompute); void inc_proc_northd_cleanup(void); diff --git a/northd/northd.c b/northd/northd.c index 7bccd054cdb4..1e4273ec5e88 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -14502,9 +14502,7 @@ ovnnb_db_run(struct northd_data *data, struct ovsdb_idl_txn *ovnnb_txn, struct ovsdb_idl_txn *ovnsb_txn, struct ovsdb_idl_index *sbrec_chassis_by_name, - struct ovsdb_idl_index *sbrec_chassis_by_hostname, - struct ovsdb_idl_loop *sb_loop, - int64_t loop_start_time) + struct ovsdb_idl_index *sbrec_chassis_by_hostname) { if (!ovnsb_txn || !ovnnb_txn) { return; @@ -14527,12 +14525,7 @@ ovnnb_db_run(struct northd_data *data, if (nb->ipsec != sb->ipsec) { sbrec_sb_global_set_ipsec(sb, nb->ipsec); } - if (nb->nb_cfg != sb->nb_cfg) { - sbrec_sb_global_set_nb_cfg(sb, nb->nb_cfg); - nbrec_nb_global_set_nb_cfg_timestamp(nb, loop_start_time); - } sbrec_sb_global_set_options(sb, &nb->options); - sb_loop->next_cfg = nb->nb_cfg; const char *mac_addr_prefix = set_mac_prefix(smap_get(&nb->options, "mac_prefix")); @@ -14834,68 +14827,12 @@ handle_port_binding_changes(struct northd_data *data, } } -/* Updates the sb_cfg and hv_cfg columns in the northbound NB_Global table. */ -static void -update_northbound_cfg(struct northd_data *data, - struct ovsdb_idl_loop *sb_loop, - int64_t loop_start_time) -{ - /* Update northbound sb_cfg if appropriate. */ - const struct nbrec_nb_global *nbg = nbrec_nb_global_table_first( - data->input.nbrec_nb_global_table); - int64_t sb_cfg = sb_loop->cur_cfg; - if (nbg && sb_cfg && nbg->sb_cfg != sb_cfg) { - nbrec_nb_global_set_sb_cfg(nbg, sb_cfg); - nbrec_nb_global_set_sb_cfg_timestamp(nbg, loop_start_time); - } - - /* Update northbound hv_cfg if appropriate. */ - if (nbg) { - /* Find minimum nb_cfg among all chassis. */ - const struct sbrec_chassis_private *chassis_priv; - int64_t hv_cfg = nbg->nb_cfg; - int64_t hv_cfg_ts = 0; - SBREC_CHASSIS_PRIVATE_TABLE_FOR_EACH (chassis_priv, - data->input.sbrec_chassis_private_table) { - const struct sbrec_chassis *chassis = chassis_priv->chassis; - if (chassis) { - if (smap_get_bool(&chassis->other_config, - "is-remote", false)) { - /* Skip remote chassises. */ - continue; - } - } else { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "Chassis does not exist for " - "Chassis_Private record, name: %s", - chassis_priv->name); - } - - if (chassis_priv->nb_cfg < hv_cfg) { - hv_cfg = chassis_priv->nb_cfg; - hv_cfg_ts = chassis_priv->nb_cfg_timestamp; - } else if (chassis_priv->nb_cfg == hv_cfg && - chassis_priv->nb_cfg_timestamp > hv_cfg_ts) { - hv_cfg_ts = chassis_priv->nb_cfg_timestamp; - } - } - - /* Update hv_cfg. */ - if (nbg->hv_cfg != hv_cfg) { - nbrec_nb_global_set_hv_cfg(nbg, hv_cfg); - nbrec_nb_global_set_hv_cfg_timestamp(nbg, hv_cfg_ts); - } - } -} - /* Handle a fairly small set of changes in the southbound database. */ static void ovnsb_db_run(struct northd_data *data, struct ovsdb_idl_txn *ovnnb_txn, struct ovsdb_idl_txn *ovnsb_txn, - struct ovsdb_idl_loop *sb_loop, - struct hmap *ports, - int64_t loop_start_time) + struct hmap *ports) { if (!ovnnb_txn || !ovsdb_idl_has_ever_connected(ovsdb_idl_txn_get_idl(ovnsb_txn))) { @@ -14904,30 +14841,23 @@ ovnsb_db_run(struct northd_data *data, struct shash ha_ref_chassis_map = SHASH_INITIALIZER(&ha_ref_chassis_map); handle_port_binding_changes(data, ovnsb_txn, ports, &ha_ref_chassis_map); - update_northbound_cfg(data, sb_loop, loop_start_time); if (ovnsb_txn) { update_sb_ha_group_ref_chassis(data, &ha_ref_chassis_map); } shash_destroy(&ha_ref_chassis_map); } -void -northd_run(struct northd_data *data, - struct ovsdb_idl_txn *ovnnb_txn, - struct ovsdb_idl_txn *ovnsb_txn, - struct ovsdb_idl_loop *sb_loop) +void northd_run(struct northd_data *data, + struct ovsdb_idl_txn *ovnnb_txn, + struct ovsdb_idl_txn *ovnsb_txn) { - int64_t start_time = time_wall_msec(); - stopwatch_start(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec()); ovnnb_db_run(data, ovnnb_txn, ovnsb_txn, data->input.sbrec_chassis_by_name, - data->input.sbrec_chassis_by_hostname, - sb_loop, start_time); + data->input.sbrec_chassis_by_hostname); stopwatch_stop(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec()); stopwatch_start(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec()); - ovnsb_db_run(data, ovnnb_txn, ovnsb_txn, - sb_loop, &data->ports, start_time); + ovnsb_db_run(data, ovnnb_txn, ovnsb_txn, &data->ports); stopwatch_stop(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec()); } diff --git a/northd/northd.h b/northd/northd.h index 5ceb2c6c6216..113688694cbb 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -79,8 +79,7 @@ struct northd_data { void northd_run(struct northd_data *data, struct ovsdb_idl_txn *ovnnb_txn, - struct ovsdb_idl_txn *ovnsb_txn, - struct ovsdb_idl_loop *sb_loop); + struct ovsdb_idl_txn *ovnsb_txn); void northd_destroy(struct northd_data *data); void northd_init(struct northd_data *data); void northd_indices_create(struct northd_data *data, diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index cce757fb57d9..7b8fa115f14c 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -439,6 +439,79 @@ check_and_add_supported_dhcpv6_opts_to_sb_db(struct ovsdb_idl_txn *ovnsb_txn, hmap_destroy(&dhcpv6_opts_to_add); } + +/* Updates the nb_cfg, sb_cfg and hv_cfg columns in NB/SB databases. */ +static void +update_sequence_numbers(struct ovsdb_idl *ovnnb_idl, + struct ovsdb_idl *ovnsb_idl, + struct ovsdb_idl_txn *ovnnb_idl_txn, + struct ovsdb_idl_txn *ovnsb_idl_txn, + struct ovsdb_idl_loop *sb_loop) +{ + int64_t loop_start_time = time_msec(); + + /* Create rows in global tables if neccessary */ + const struct nbrec_nb_global *nb = nbrec_nb_global_first(ovnnb_idl); + if (!nb) { + nb = nbrec_nb_global_insert(ovnnb_idl_txn); + } + const struct sbrec_sb_global *sb = sbrec_sb_global_first(ovnsb_idl); + if (!sb) { + sb = sbrec_sb_global_insert(ovnsb_idl_txn); + } + + /* Copy nb_cfg from northbound to southbound database. + * Also set up to update sb_cfg once our southbound transaction commits. */ + if (nb->nb_cfg != sb->nb_cfg) { + sbrec_sb_global_set_nb_cfg(sb, nb->nb_cfg); + nbrec_nb_global_set_nb_cfg_timestamp(nb, loop_start_time); + } + sb_loop->next_cfg = nb->nb_cfg; + + /* Update northbound sb_cfg if appropriate. */ + int64_t sb_cfg = sb_loop->cur_cfg; + if (nb && sb_cfg && nb->sb_cfg != sb_cfg) { + nbrec_nb_global_set_sb_cfg(nb, sb_cfg); + nbrec_nb_global_set_sb_cfg_timestamp(nb, loop_start_time); + } + + /* Update northbound hv_cfg if appropriate. */ + if (nb) { + /* Find minimum nb_cfg among all chassis. */ + const struct sbrec_chassis_private *chassis_priv; + int64_t hv_cfg = nb->nb_cfg; + int64_t hv_cfg_ts = 0; + SBREC_CHASSIS_PRIVATE_FOR_EACH (chassis_priv, ovnsb_idl) { + const struct sbrec_chassis *chassis = chassis_priv->chassis; + if (chassis) { + if (smap_get_bool(&chassis->other_config, + "is-remote", false)) { + /* Skip remote chassises. */ + continue; + } + } else { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "Chassis does not exist for " + "Chassis_Private record, name: %s", + chassis_priv->name); + } + + if (chassis_priv->nb_cfg < hv_cfg) { + hv_cfg = chassis_priv->nb_cfg; + hv_cfg_ts = chassis_priv->nb_cfg_timestamp; + } else if (chassis_priv->nb_cfg == hv_cfg && + chassis_priv->nb_cfg_timestamp > hv_cfg_ts) { + hv_cfg_ts = chassis_priv->nb_cfg_timestamp; + } + } + + /* Update hv_cfg. */ + if (nb->hv_cfg != hv_cfg) { + nbrec_nb_global_set_hv_cfg(nb, hv_cfg); + nbrec_nb_global_set_hv_cfg_timestamp(nb, hv_cfg_ts); + } + } +} static void add_column_noalert(struct ovsdb_idl *idl, @@ -995,9 +1068,7 @@ main(int argc, char *argv[]) } if (ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) { - inc_proc_northd_run(ovnnb_txn, ovnsb_txn, - &ovnsb_idl_loop, - recompute); + inc_proc_northd_run(ovnnb_txn, ovnsb_txn, recompute); recompute = false; if (ovnsb_txn) { check_and_add_supported_dhcp_opts_to_sb_db( @@ -1007,6 +1078,13 @@ main(int argc, char *argv[]) check_and_update_rbac( ovnsb_txn, ovnsb_idl_loop.idl); } + + if (ovnnb_txn && ovnsb_txn) { + update_sequence_numbers(ovnnb_idl_loop.idl, + ovnsb_idl_loop.idl, + ovnnb_txn, ovnsb_txn, + &ovnsb_idl_loop); + } } } else { /* ovn-northd is paused
In order to remove the IDL loop variable from the engine context, we do not calculate the database sequence numbers incrementally. Signed-off-by: Mark Gray <mark.d.gray@redhat.com> --- lib/inc-proc-eng.h | 2 - northd/en-northd.c | 3 +- northd/inc-proc-northd.c | 2 - northd/inc-proc-northd.h | 1 - northd/northd.c | 84 ++++------------------------------------ northd/northd.h | 3 +- northd/ovn-northd.c | 84 ++++++++++++++++++++++++++++++++++++++-- 7 files changed, 90 insertions(+), 89 deletions(-)