diff mbox series

[ovs-dev,v3] utilities: increase OVSDB inactivity probe interval for ovn-*ctl

Message ID 20230504154250.4023895-1-odivlad@gmail.com
State Superseded, archived
Headers show
Series [ovs-dev,v3] utilities: increase OVSDB inactivity probe interval for ovn-*ctl | expand

Checks

Context Check Description
ovsrobot/apply-robot fail apply and check: fail

Commit Message

Vladislav Odintsov May 4, 2023, 3:42 p.m. UTC
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>
---
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(+)

Comments

0-day Robot May 4, 2023, 3:57 p.m. UTC | #1
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.


git-am:
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 utilities: increase OVSDB inactivity probe interval for ovn-*ctl
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index fc1b68324..8ae32a81e 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,13 @@  Post v23.03.0
     addresses and CIDRs.
   - Add support to configure OVSDB inactivity probe interval for ovn-ic and
     ovn-controller-vtep.
+  - 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 5f0b436c2..a22ca1c71 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -16259,6 +16259,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 fd32070f2..ee0aa5372 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 45572fd30..b620f7e2e 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 **);
@@ -7935,6 +7949,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,