On Thu, May 4, 2023 at 6:55 PM Vladislav Odintsov <[email protected]> wrote:
> For large OVN_Southbound (or other) databases the default interval of 5000 > ms > could be not sufficient to run. This patch adds configuration of OVSDB > inactivity probes for ovn-*ctl utilities. > > Initially, on the utility start the hardcoded value of 120000 ms is set. > For daemon-mode it is possible to configure intervals in OVN Northbound and > OVN Southbound databases for ovn-nbctl and ovn-sbctl utilities > respectively. > > Use > OVN_Northbound.NB_Global.options.nbctl_probe_interval for ovn-nbctl and > OVN_Southbound.SB_Global.options.sbctl_probe_interval for ovn-sbctl > utilities. > > If no DB configuration option was provided, the initial (120000 ms) > interval > is left. > > Signed-off-by: Vladislav Odintsov <[email protected]> > --- > v3 -> v4: > - Rebased on a fresh main branch. > > v2 -> v3: > - Addressed Dumitru's and Mark's suggestion to split ovn-{n,s}bctl > configuration option dbctl_probe_interval for nbctl_... and sbctl_... . > - Added NEWS entry. > - Fixes typos. > - Added ovn-sb man entry for new option. > - Moved constant from ovn-util.h to ovn-dbctl.h. > --- > NEWS | 7 +++++++ > northd/northd.c | 9 +++++++++ > ovn-nb.xml | 18 ++++++++++++++++++ > ovn-sb.xml | 25 +++++++++++++++++++++++++ > utilities/ovn-dbctl.c | 14 ++++++++++++++ > utilities/ovn-dbctl.h | 3 +++ > utilities/ovn-ic-nbctl.c | 5 +++++ > utilities/ovn-ic-sbctl.c | 5 +++++ > utilities/ovn-nbctl.c | 15 +++++++++++++++ > utilities/ovn-sbctl.c | 15 +++++++++++++++ > 10 files changed, 116 insertions(+) > > diff --git a/NEWS b/NEWS > index 60467581a..54303f834 100644 > --- a/NEWS > +++ b/NEWS > @@ -14,6 +14,13 @@ Post v23.03.0 > existing behaviour of flooding these arp requests to all attached > Ports. > - Always allow IPv6 Router Discovery, Neighbor Discovery, and Multicast > Listener Discovery protocols, regardless of ACLs defined. > + - Increased ovn-{ic-,}{n,s}bctl default OVSDB inactivity probe interval > from > + 5000 ms to 120000 ms to give the ability to connect to large databases > + (mainly, OVN_Southbound). Also, for daemon mode it is possible to > + configure inactivity probe interval via OVN_Northbound and > OVN_Southbound > + databases for ovn-nbctl and ovn-sbctl respectively. See man ovn-nb > and > + man ovn-sb for 'nbctl_probe_interval' and 'sbctl_probe_interval' > + options for more details. > > OVN v23.03.0 - 03 Mar 2023 > -------------------------- > diff --git a/northd/northd.c b/northd/northd.c > index b58f11633..eca1e6068 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -16715,6 +16715,15 @@ ovnnb_db_run(struct northd_input *input_data, > } else { > smap_remove(&options, "lb_hairpin_use_ct_mark"); > } > + > + /* Hackaround SB_global.options overwrite by NB_Global.options for > + * 'sbctl_probe_interval' option. > + */ > + const char *sip = smap_get(&sb->options, "sbctl_probe_interval"); > + if (sip) { > + smap_replace(&options, "sbctl_probe_interval", sip); > + } > + > if (!smap_equal(&sb->options, &options)) { > sbrec_sb_global_set_options(sb, &options); > } > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 0552eff19..0c1954792 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -215,6 +215,24 @@ > </p> > </column> > > + <column name="options" key="nbctl_probe_interval"> > + <p> > + The inactivity probe interval of the connection to the OVN > Northbound > + database from <code>ovn-nbctl</code> utility, in milliseconds. > + If the value is zero, it disables the connection keepalive > feature. > + </p> > + > + <p> > + If the value is nonzero, then it will be forced to a value of > + at least 1000 ms. > + </p> > + > + <p> > + If the value is less than zero, then the default inactivity > probe > + interval for <code>ovn-nbctl</code> would be left intact > (120000 ms). > + </p> > + </column> > + > <column name="options" key="northd_trim_timeout"> > <p> > When used, this configuration value specifies the time, in > diff --git a/ovn-sb.xml b/ovn-sb.xml > index a77f8f4ef..aa78383d9 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -248,6 +248,31 @@ > </column> > </group> > > + <group title="Options for configuring ovn-sbctl"> > + <p> > + These options apply when <code>ovn-sbctl</code> connects to > + OVN Southbound database. > + </p> > + > + <column name="options" key="sbctl_probe_interval"> > + <p> > + The inactivity probe interval of the connection to the OVN > + Southbound database from <code>ovn-sbctl</code> utility, in > + milliseconds. If the value is zero, it disables the > connection > + keepalive feature. > + </p> > + > + <p> > + If the value is nonzero, then it will be forced to a value of > + at least 1000 ms. > + </p> > + > + <p> > + If the value is less than zero, then the default inactivity > probe > + interval for <code>ovn-sbctl</code> would be left intact > (120000 ms). > + </p> > + </column> > + </group> > </group> > > <group title="Connection Options"> > diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c > index 369a6a663..1d41157df 100644 > --- a/utilities/ovn-dbctl.c > +++ b/utilities/ovn-dbctl.c > @@ -205,6 +205,9 @@ ovn_dbctl_main(int argc, char *argv[], > ovsdb_idl_set_remote(idl, db, daemon_mode); > ovsdb_idl_set_leader_only(idl, leader_only); > > + /* Set reasonable high probe interval. */ > + set_idl_probe_interval(idl, db, DEFAULT_UTILS_PROBE_INTERVAL_MSEC); > + > if (daemon_mode) { > server_loop(dbctl_options, idl, argc, argv_); > } else { > @@ -1094,6 +1097,13 @@ out: > free(argv); > } > > +static void > +update_inactivity_probe(struct server_cmd_run_ctx *ctx) > +{ > + set_idl_probe_interval(ctx->idl, db, > + > ctx->dbctl_options->get_inactivity_probe(ctx->idl)); > +} > + > static void > server_loop(const struct ovn_dbctl_options *dbctl_options, > struct ovsdb_idl *idl, int argc, char *argv[]) > @@ -1125,6 +1135,10 @@ server_loop(const struct ovn_dbctl_options > *dbctl_options, > > for (;;) { > update_ssl_config(); > + > + /* Configure inactivity probe from connected DB. */ > + update_inactivity_probe(&server_cmd_run_ctx); > + > memory_run(); > if (memory_should_report()) { > struct simap usage = SIMAP_INITIALIZER(&usage); > diff --git a/utilities/ovn-dbctl.h b/utilities/ovn-dbctl.h > index a1fbede6b..5cfc355e7 100644 > --- a/utilities/ovn-dbctl.h > +++ b/utilities/ovn-dbctl.h > @@ -20,6 +20,8 @@ > #include <stdbool.h> > #include "ovsdb-idl.h" > > +#define DEFAULT_UTILS_PROBE_INTERVAL_MSEC 120000 > + > struct timer; > > enum nbctl_wait_type { > @@ -52,6 +54,7 @@ struct ovn_dbctl_options { > const struct timer *wait_timeout, > long long int start_time, bool print_wait_time); > > + int (*get_inactivity_probe)(struct ovsdb_idl *); > struct ctl_context *(*ctx_create)(void); > void (*ctx_destroy)(struct ctl_context *); > }; > diff --git a/utilities/ovn-ic-nbctl.c b/utilities/ovn-ic-nbctl.c > index f3d8039a8..721dc4586 100644 > --- a/utilities/ovn-ic-nbctl.c > +++ b/utilities/ovn-ic-nbctl.c > @@ -25,6 +25,7 @@ > #include "command-line.h" > #include "compiler.h" > #include "db-ctl-base.h" > +#include "ovn-dbctl.h" > #include "dirs.h" > #include "fatal-signal.h" > #include "openvswitch/dynamic-string.h" > @@ -116,6 +117,10 @@ main(int argc, char *argv[]) > ovsdb_idl_set_remote(idl, db, false); > ovsdb_idl_set_db_change_aware(idl, false); > ovsdb_idl_set_leader_only(idl, leader_only); > + > + /* Set reasonable high probe interval. */ > + set_idl_probe_interval(idl, db, DEFAULT_UTILS_PROBE_INTERVAL_MSEC); > + > run_prerequisites(commands, n_commands, idl); > > /* Execute the commands. > diff --git a/utilities/ovn-ic-sbctl.c b/utilities/ovn-ic-sbctl.c > index 3060b48b9..f9b1954d6 100644 > --- a/utilities/ovn-ic-sbctl.c > +++ b/utilities/ovn-ic-sbctl.c > @@ -25,6 +25,7 @@ > #include "command-line.h" > #include "compiler.h" > #include "db-ctl-base.h" > +#include "ovn-dbctl.h" > #include "dirs.h" > #include "fatal-signal.h" > #include "openvswitch/dynamic-string.h" > @@ -115,6 +116,10 @@ main(int argc, char *argv[]) > ovsdb_idl_set_remote(idl, db, false); > ovsdb_idl_set_db_change_aware(idl, false); > ovsdb_idl_set_leader_only(idl, leader_only); > + > + /* Set reasonable high probe interval. */ > + set_idl_probe_interval(idl, db, DEFAULT_UTILS_PROBE_INTERVAL_MSEC); > + > run_prerequisites(commands, n_commands, idl); > > /* Execute the commands. > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > index 9399f9462..c2bdbf4a3 100644 > --- a/utilities/ovn-nbctl.c > +++ b/utilities/ovn-nbctl.c > @@ -153,6 +153,20 @@ nbctl_post_execute(struct ovsdb_idl *idl, struct > ovsdb_idl_txn *txn, > } > } > > +static int > +get_inactivity_probe(struct ovsdb_idl *idl) > +{ > + const struct nbrec_nb_global *nb = nbrec_nb_global_first(idl); > + int interval = DEFAULT_UTILS_PROBE_INTERVAL_MSEC; > + > + if (nb) { > + interval = smap_get_int(&nb->options, "nbctl_probe_interval", > + interval); > + } > + > + return interval; > +} > + > static char * OVS_WARN_UNUSED_RESULT dhcp_options_get( > struct ctl_context *ctx, const char *id, bool must_exist, > const struct nbrec_dhcp_options **); > @@ -7933,6 +7947,7 @@ main(int argc, char *argv[]) > .add_base_prerequisites = nbctl_add_base_prerequisites, > .pre_execute = nbctl_pre_execute, > .post_execute = nbctl_post_execute, > + .get_inactivity_probe = get_inactivity_probe, > > .ctx_create = nbctl_ctx_create, > .ctx_destroy = nbctl_ctx_destroy, > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c > index 542ab9ffa..3948cae3f 100644 > --- a/utilities/ovn-sbctl.c > +++ b/utilities/ovn-sbctl.c > @@ -150,6 +150,20 @@ Other options:\n\ > * gracefully. */ > #define ctl_fatal dont_use_ctl_fatal_use_ctl_error_and_return > > +static int > +get_inactivity_probe(struct ovsdb_idl *idl) > +{ > + const struct sbrec_sb_global *sb = sbrec_sb_global_first(idl); > + int interval = DEFAULT_UTILS_PROBE_INTERVAL_MSEC; > + > + if (sb) { > + interval = smap_get_int(&sb->options, "sbctl_probe_interval", > + interval); > + } > + > + return interval; > +} > + > /* ovs-sbctl specific context. Inherits the 'struct ctl_context' as > base. */ > struct sbctl_context { > struct ctl_context base; > @@ -1590,6 +1604,7 @@ main(int argc, char *argv[]) > .add_base_prerequisites = sbctl_add_base_prerequisites, > .pre_execute = sbctl_pre_execute, > .post_execute = NULL, > + .get_inactivity_probe = get_inactivity_probe, > > .ctx_create = sbctl_ctx_create, > .ctx_destroy = sbctl_ctx_destroy, > -- > 2.36.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Except for the 0-day warning, which can be addressed during merge, it looks good to me, thanks. Acked-by: Ales Musil <[email protected]> -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] IM: amusil <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
