| Message ID | 20230504165510.4026066-1-odivlad@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v4] utilities: increase OVSDB inactivity probe interval for ovn-*ctl | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Bleep bloop. Greetings Vladislav Odintsov, I am a robot and I have tried out your patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Line is 81 characters long (recommended limit is 79)
#132 FILE: ovn-sb.xml:272:
interval for <code>ovn-sbctl</code> would be left intact (120000 ms).
Lines checked: 314, Warnings: 1, Errors: 0
Please check this out. If you feel there has been an error, please email aconole@redhat.com
Thanks,
0-day Robot
On Thu, May 4, 2023 at 6:55 PM Vladislav Odintsov <odivlad@gmail.com> 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 <odivlad@gmail.com> > --- > 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 > dev@openvswitch.org > 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 <amusil@redhat.com>
Thanks Vladislav and Ales. I pushed the change to main. On 5/5/23 04:54, Ales Musil wrote: > On Thu, May 4, 2023 at 6:55 PM Vladislav Odintsov <odivlad@gmail.com> 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 <odivlad@gmail.com> >> --- >> 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 >> dev@openvswitch.org >> 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 <amusil@redhat.com> >
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,
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 <odivlad@gmail.com> --- 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(+)