On 7/25/22 23:34, Han Zhou wrote: > In some cases we need to know if ovn-controller started long enough and > has enough iterations of input processing, primarily to ensure it has > downloaded and handled a complete initial view of the SB DB (and of > course the local OVS DB), so that it won't delete things too early by > mistake based on incomplete data. > > The mechanism will be used in follow up patches. > > Suggested-by: Dumitru Ceara <dce...@redhat.com> > Signed-off-by: Han Zhou <hz...@ovn.org> > ---
Hi Han, > controller/ovn-controller.c | 22 +++++++++++++--- > lib/inc-proc-eng.c | 11 ++++++++ > lib/inc-proc-eng.h | 4 +++ > lib/ovn-util.c | 50 +++++++++++++++++++++++++++++++++++++ > lib/ovn-util.h | 4 +++ > 5 files changed, 88 insertions(+), 3 deletions(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 2e9138036..8fc554201 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -93,6 +93,7 @@ static unixctl_cb_func debug_dump_lflow_conj_ids; > static unixctl_cb_func lflow_cache_flush_cmd; > static unixctl_cb_func lflow_cache_show_stats_cmd; > static unixctl_cb_func debug_delay_nb_cfg_report; > +static unixctl_cb_func debug_ignore_startup_delay; > > #define DEFAULT_BRIDGE_NAME "br-int" > #define DEFAULT_DATAPATH "system" > @@ -863,11 +864,12 @@ static void > store_nb_cfg(struct ovsdb_idl_txn *sb_txn, struct ovsdb_idl_txn *ovs_txn, > const struct sbrec_chassis_private *chassis, > const struct ovsrec_bridge *br_int, > - unsigned int delay_nb_cfg_report, int64_t startup_ts) > + unsigned int delay_nb_cfg_report) > { > struct ofctrl_acked_seqnos *acked_nb_cfg_seqnos = > ofctrl_acked_seqnos_get(ofctrl_seq_type_nb_cfg); > uint64_t cur_cfg = acked_nb_cfg_seqnos->last_acked; > + int64_t startup_ts = daemon_startup_ts(); > > if (ovs_txn && br_int > && startup_ts != smap_get_ullong(&br_int->external_ids, > @@ -3796,6 +3798,9 @@ main(int argc, char *argv[]) > debug_dump_lflow_conj_ids, > &lflow_output_data->conj_ids); > > + unixctl_command_register("debug/ignore-startup-delay", "", 0, 0, > + debug_ignore_startup_delay, NULL); > + > unsigned int ovs_cond_seqno = UINT_MAX; > unsigned int ovnsb_cond_seqno = UINT_MAX; > unsigned int ovnsb_expected_cond_seqno = UINT_MAX; > @@ -3817,7 +3822,6 @@ main(int argc, char *argv[]) > /* Main loop. */ > exiting = false; > restart = false; > - int64_t startup_ts = time_wall_msec(); > bool sb_monitor_all = false; > while (!exiting) { > memory_run(); > @@ -3997,6 +4001,10 @@ main(int argc, char *argv[]) > } > stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME, > time_msec()); > + if (engine_has_updated()) { > + daemon_started_recently_countdown(); > + } > + > ct_zones_data = engine_get_data(&en_ct_zones); > if (ovs_idl_txn) { > if (ct_zones_data) { > @@ -4159,7 +4167,7 @@ main(int argc, char *argv[]) > } > > store_nb_cfg(ovnsb_idl_txn, ovs_idl_txn, chassis_private, > - br_int, delay_nb_cfg_report, startup_ts); > + br_int, delay_nb_cfg_report); > > if (pending_pkt.conn) { > struct ed_type_addr_sets *as_data = > @@ -4633,3 +4641,11 @@ debug_dump_lflow_conj_ids(struct unixctl_conn *conn, > int argc OVS_UNUSED, > unixctl_command_reply(conn, ds_cstr(&conj_ids_dump)); > ds_destroy(&conj_ids_dump); > } > + > +static void > +debug_ignore_startup_delay(struct unixctl_conn *conn, int argc OVS_UNUSED, > + const char *argv[] OVS_UNUSED, void *arg > OVS_UNUSED) > +{ > + daemon_started_recently_ignore(); > + unixctl_command_reply(conn, NULL); > +} > diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c > index 575b774ae..2e2b31033 100644 > --- a/lib/inc-proc-eng.c > +++ b/lib/inc-proc-eng.c > @@ -312,6 +312,17 @@ engine_has_run(void) > return false; > } > > +bool > +engine_has_updated(void) > +{ > + for (size_t i = 0; i < engine_n_nodes; i++) { > + if (engine_nodes[i]->state == EN_UPDATED) { > + return true; > + } > + } > + return false; > +} > + > bool > engine_aborted(void) > { > diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h > index 9bfab1f7c..dc365dc18 100644 > --- a/lib/inc-proc-eng.h > +++ b/lib/inc-proc-eng.h > @@ -238,6 +238,10 @@ bool engine_node_changed(struct engine_node *node); > /* Return true if the engine has run in the last iteration. */ > bool engine_has_run(void); > > +/* Return true if the engine has any update in any node, i.e. any input > + * has changed; false if nothing has changed. */ > +bool engine_has_updated(void); > + > /* Returns true if during the last engine run we had to abort processing. */ > bool engine_aborted(void); > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > index 616999eab..693735841 100644 > --- a/lib/ovn-util.c > +++ b/lib/ovn-util.c > @@ -883,3 +883,53 @@ get_bridge(const struct ovsrec_bridge_table > *bridge_table, const char *br_name) > } > return NULL; > } > + > +#define DAEMON_STARTUP_DELAY_SEED 20 > +#define DAEMON_STARTUP_DELAY_MS 10000 > + > +static int64_t startup_ts; > +static int startup_delay = DAEMON_STARTUP_DELAY_SEED; > +static bool ignore_startup_delay = false; > + > +OVS_CONSTRUCTOR(startup_ts_initializer) { > + startup_ts = time_wall_msec(); A bit of a nit, but if we moved the "debug/ignore-startup-delay" unixctl registration here, we wouldn't need the daemon_started_recently_ignore() api either. I think I would also add a comment specifying that this is needed just for the sake of making the test reliable. > +} > + > +int64_t > +daemon_startup_ts(void) > +{ > + return startup_ts; > +} > + > +void > +daemon_started_recently_countdown(void) > +{ > + if (startup_delay > 0) { > + startup_delay--; > + } > +} > + > +void > +daemon_started_recently_ignore(void) > +{ > + ignore_startup_delay = true; > +} > + > +bool > +daemon_started_recently(void) > +{ > + if (ignore_startup_delay) { > + return false; > + } > + > + VLOG_DBG("startup_delay: %d, startup_ts: %"PRId64, startup_delay, > + startup_ts); > + > + /* Ensure that at least an amount of updates has been handled. */ > + if (startup_delay) { > + return true; > + } > + > + /* Ensure that at least an amount of time has passed. */ > + return time_wall_msec() - startup_ts <= DAEMON_STARTUP_DELAY_MS; > +} > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > index b3905ef7b..145f974ed 100644 > --- a/lib/ovn-util.h > +++ b/lib/ovn-util.h > @@ -309,5 +309,9 @@ struct ovsrec_bridge_table; > const struct ovsrec_bridge *get_bridge(const struct ovsrec_bridge_table *, > const char *br_name); > > +void daemon_started_recently_countdown(void); > +void daemon_started_recently_ignore(void); > +bool daemon_started_recently(void); > +int64_t daemon_startup_ts(void); > > #endif /* OVN_UTIL_H */ Thanks, Dumitru _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev