On Fri, Sep 17, 2021 at 1:41 PM Han Zhou <hz...@ovn.org> wrote:
>
>
>
> On Thu, Sep 16, 2021 at 8:06 PM Zhen Wang <zhew...@nvidia.com> wrote:
> >
> > From: zhen wang <zhew...@nvidia.com>
> >
> > When ovn-northd work in HA mode, ovn-northd will not update the probe
> > interval in standby mode. This patch address the problem by updating
> > the value in main loop.
> >
>
> Thanks Zhen for the fix! Maybe the impact and steps to reproduce the
problem can be mentioned. It may be:
> step1: power off the NB/SB leader
> step2: kill the active northd (and the standbys would never take over)
>
> Acked-by: Han Zhou <hz...@ovn.org>
>
> > Signed-off-by: zhen wang <zhew...@nvidia.com>
> > ---
> > northd/northd.c | 25 -------------------------
> > northd/ovn-northd.c | 29 +++++++++++++++++++++++++++++
> > 2 files changed, 29 insertions(+), 25 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index b7e64470f..89b0e4921 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -72,10 +72,6 @@ static struct eth_addr svc_monitor_mac_ea;
> > * Otherwise, it will avoid using it. The default is true. */
> > static bool use_ct_inv_match = true;
> >
> > -/* Default probe interval for NB and SB DB connections. */
> > -#define DEFAULT_PROBE_INTERVAL_MSEC 5000
> > -static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC;
> > -static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC;
> > #define MAX_OVN_TAGS 4096
> >
> > /* Pipeline stages. */
> > @@ -14082,20 +14078,6 @@ build_meter_groups(struct northd_context *ctx,
> > }
> > }
> >
> > -static int
> > -get_probe_interval(const char *db, const struct nbrec_nb_global *nb)
> > -{
> > - int default_interval = (db && !stream_or_pstream_needs_probes(db)
> > - ? 0 : DEFAULT_PROBE_INTERVAL_MSEC);
> > - int interval = smap_get_int(&nb->options,
> > - "northd_probe_interval",
default_interval);
> > -
> > - if (interval > 0 && interval < 1000) {
> > - interval = 1000;
> > - }
> > - return interval;
> > -}
> > -
> > static void
> > ovnnb_db_run(struct northd_context *ctx,
> > struct ovsdb_idl_index *sbrec_chassis_by_name,
> > @@ -14182,13 +14164,6 @@ ovnnb_db_run(struct northd_context *ctx,
> >
> > smap_destroy(&options);
> >
> > - /* Update the probe interval. */
> > - northd_probe_interval_nb = get_probe_interval(ctx->ovnnb_db, nb);
> > - northd_probe_interval_sb = get_probe_interval(ctx->ovnsb_db, nb);
> > -
> > - ovsdb_idl_set_probe_interval(ctx->ovnnb_idl,
northd_probe_interval_nb);
> > - ovsdb_idl_set_probe_interval(ctx->ovnsb_idl,
northd_probe_interval_sb);
> > -
> > use_parallel_build =
> > (smap_get_bool(&nb->options, "use_parallel_build", false) &&
> > can_parallelize_hashes(false));
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 6d4c5defc..0a9fd8190 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -65,6 +65,10 @@ static const char *ssl_private_key_file;
> > static const char *ssl_certificate_file;
> > static const char *ssl_ca_cert_file;
> >
> > +/* Default probe interval for NB and SB DB connections. */
> > +#define DEFAULT_PROBE_INTERVAL_MSEC 5000
> > +static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC;
> > +static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC;
> > static bool use_parallel_build = true;
> > static struct hashrow_locks lflow_locks;
> >
> > @@ -577,6 +581,20 @@ update_ssl_config(void)
> > }
> > }
> >
> > +static int
> > +get_probe_interval(const char *db, const struct nbrec_nb_global *nb)
> > +{
> > + int default_interval = (db && !stream_or_pstream_needs_probes(db)
> > + ? 0 : DEFAULT_PROBE_INTERVAL_MSEC);
> > + int interval = smap_get_int(&nb->options,
> > + "northd_probe_interval",
default_interval);
> > +
> > + if (interval > 0 && interval < 1000) {
> > + interval = 1000;
> > + }
> > + return interval;
> > +}
> > +
> > int
> > main(int argc, char *argv[])
> > {
> > @@ -911,6 +929,12 @@ main(int argc, char *argv[])
> >
> > while (!exiting) {
> > update_ssl_config();
> > + const struct nbrec_nb_global *nb =
nbrec_nb_global_first(ovnnb_idl_loop.idl);
> > + /* Update the probe interval. */
> > + if (nb) {
> > + northd_probe_interval_nb = get_probe_interval(ovnnb_db,
nb);
> > + northd_probe_interval_sb = get_probe_interval(ovnsb_db,
nb);
> > + }
Sorry that I forgot a minor comment here: it is better to move this down,
right before setting the probe intervals. Otherwise, since this is before
ovsdb_idl_loop_run(), it reads the config from the last iteration, and the
new setting (if updated) will be applied at the next iteration only.
> > memory_run();
> > if (memory_should_report()) {
> > struct simap usage = SIMAP_INITIALIZER(&usage);
> > @@ -1000,6 +1024,11 @@ main(int argc, char *argv[])
> > poll_immediate_wake();
> > }
> >
> > + ovsdb_idl_set_probe_interval(ovnnb_idl_loop.idl,
> > + northd_probe_interval_nb);
> > + ovsdb_idl_set_probe_interval(ovnsb_idl_loop.idl,
> > + northd_probe_interval_sb);
> > +
> > if (reset_ovnsb_idl_min_index) {
> > VLOG_INFO("Resetting southbound database cluster state");
> > ovsdb_idl_reset_min_index(ovnsb_idl_loop.idl);
> > --
> > 2.20.1
> >
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
