[ovs-dev] [PATCH v2] Avoid update probe interval to non-zero for unix socket.

[Han Zhou]

On Mon, May 1, 2017 at 11:19 AM, Ben Pfaff <blp at ovn.org> wrote:
>
> On Tue, Apr 18, 2017 at 11:12:14AM -0700, Han Zhou wrote:
> > In commit c1bfdd9d it disables probe when not needed, but commit
> > 715038b6 updates ovn-controller probe interval for OVNSB DB
> > periodically according to ovn-remote-probe-interval config, and sets
> > it to DEFAULT_PROBE_INTERVAL_MSEC if not configured, even if the
> > connection type is unix socket which doesn't need probe.
> >
> > This fix avoids probe interval update if not needed (always set to 0).
> >
> > Signed-off-by: Han Zhou <zhouhan at gmail.com>
> > ---
> >
> > Notes:
> >     v1->v2: fix commit id mentioned in commit message.
>
> Thanks for reporting and fixing this bug.  I agree there's a bug here,
> but I'd rather honor the user's explicit request to set a nonzero probe
> interval, if there is such a request in ovsdb.
>
> How about this?  I have not tested it, beyond verifying that it
> compiles.
>
> Thanks,
>
> Ben.
>

Yes, this idea is better. But the code has a problem.

> @@ -933,6 +934,8 @@ update_probe_interval(struct controller_ctx *ctx)
>                      ? smap_get_int(&cfg->external_ids,
>                                     "ovn-remote-probe-interval",
>                                     DEFAULT_PROBE_INTERVAL_MSEC)
> -                    : DEFAULT_PROBE_INTERVAL_MSEC);
> +                    : stream_or_pstream_needs_probes(ovnsb_remote)
> +                    ? DEFAULT_PROBE_INTERVAL_MSEC
> +                    : 0);

Here is the fix (tested):

--8<--------------------------cut here-------------------------->8--
diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
index e00f57a..d7423ab 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -64,7 +64,8 @@ static unixctl_cb_func inject_pkt;
 #define DEFAULT_BRIDGE_NAME "br-int"
 #define DEFAULT_PROBE_INTERVAL_MSEC 5000

-static void update_probe_interval(struct controller_ctx *);
+static void update_probe_interval(struct controller_ctx *,
+                                  const char *ovnsb_remote);
 static void parse_options(int argc, char *argv[]);
 OVS_NO_RETURN static void usage(void);

@@ -594,7 +595,7 @@ main(int argc, char *argv[])
             .ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop),
         };

-        update_probe_interval(&ctx);
+        update_probe_interval(&ctx, ovnsb_remote);

         update_ssl_config(ctx.ovs_idl);

@@ -925,14 +926,21 @@ inject_pkt(struct unixctl_conn *conn, int argc
OVS_UNUSED,
 /* Get the desired SB probe timer from the OVS database and configure it
into
  * the SB database. */
 static void
-update_probe_interval(struct controller_ctx *ctx)
+update_probe_interval(struct controller_ctx *ctx, const char *ovnsb_remote)
 {
     const struct ovsrec_open_vswitch *cfg
         = ovsrec_open_vswitch_first(ctx->ovs_idl);
-    int interval = (cfg
-                    ? smap_get_int(&cfg->external_ids,
-                                   "ovn-remote-probe-interval",
-                                   DEFAULT_PROBE_INTERVAL_MSEC)
-                    : DEFAULT_PROBE_INTERVAL_MSEC);
+    int interval = -1;
+    if (cfg) {
+        interval = smap_get_int(&cfg->external_ids,
+                                "ovn-remote-probe-interval",
+                                -1);
+    }
+    if (interval == -1) {
+        interval = stream_or_pstream_needs_probes(ovnsb_remote)
+                   ? DEFAULT_PROBE_INTERVAL_MSEC
+                   : 0;
+    }
+
     ovsdb_idl_set_probe_interval(ctx->ovnsb_idl, interval);
 }