[ovs-dev] [PATCH] ovn-controller: Add external-ids:ovn-remote-probe-interval

[Andy Zhou]

On Sun, Feb 21, 2016 at 1:26 PM, Huang Lei <148012453 at qq.com> wrote:

> From: Huang Lei <lhuang8 at ebay.com>
>
> For setting the inactivity probe interval of the json session to the OVN
> southbound database.
>
> Signed-off-by: Huang Lei <lhuang8 at ebay.com>
>

Thanks for contributing!. Please see comments in line.

> ---
>  lib/ovsdb-idl.c                     | 10 ++++++++++
>  lib/ovsdb-idl.h                     |  3 +++
>  ovn/controller/ovn-controller.8.xml | 14 ++++++++++++++
>  ovn/controller/ovn-controller.c     | 33 +++++++++++++++++++++++++++++++++
>  4 files changed, 60 insertions(+)
>
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index 7e84138..844d5b1 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -557,6 +557,16 @@ ovsdb_idl_get_last_error(const struct ovsdb_idl *idl)
>  {
>      return jsonrpc_session_get_last_error(idl->session);
>  }
> +
> +/* Sets the "probe interval" for 'idl->session' to 'probe_interval', in
> + * milliseconds.
> + */
> +void
> +ovsdb_set_probe_interval(const struct ovsdb_idl *idl, int probe_interval)
> +{
> +       jsonrpc_session_set_probe_interval(idl->session, probe_interval);
> +}
> +
>
Should this function be called ovsdb_idl_set_probe_interval()?

>
>  static unsigned char *
>  ovsdb_idl_get_mode(struct ovsdb_idl *idl,
> diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
> index 136c38c..224914f 100644
> --- a/lib/ovsdb-idl.h
> +++ b/lib/ovsdb-idl.h
> @@ -68,6 +68,9 @@ void ovsdb_idl_verify_write_only(struct ovsdb_idl *);
>
>  bool ovsdb_idl_is_alive(const struct ovsdb_idl *);
>  int ovsdb_idl_get_last_error(const struct ovsdb_idl *);
> +
> +void ovsdb_set_probe_interval(const struct ovsdb_idl *, int
> probe_interval);
> +
>
 /* Choosing columns and tables to replicate. */
>
> diff --git a/ovn/controller/ovn-controller.8.xml
> b/ovn/controller/ovn-controller.8.xml
> index 6dcc579..4202b59 100644
> --- a/ovn/controller/ovn-controller.8.xml
> +++ b/ovn/controller/ovn-controller.8.xml
> @@ -100,6 +100,20 @@
>          </p>
>        </dd>
>
> +      <dt><code>external_ids:ovn-remote-probe-interval</code></dt>
> +      <dd>
> +        <p>
> +          The inactivity probe interval of the connection to the OVN
> database,
> +          in milliseconds.
> +          If the value is zero, it disables the connection keepalive
> feature.
> +        </p>
> +
> +        <p>
> +          If 'probe_interval' is nonzero, then it will be forced to a
> value of
> +          at least 1000 ms
> +        </p>
> +      </dd>
> +
>        <dt><code>external_ids:ovn-encap-type</code></dt>
>        <dd>
>          <p>
> diff --git a/ovn/controller/ovn-controller.c
> b/ovn/controller/ovn-controller.c
> index 02ecb3e..427d121 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -33,7 +33,10 @@
>  #include "ovn/lib/ovn-sb-idl.h"
>  #include "poll-loop.h"
>  #include "fatal-signal.h"
> +#include "lib/ovsdb-idl.h"
>  #include "lib/vswitch-idl.h"
> +#include "lib/util.h"
> +#include "lib/reconnect.h"
>  #include "smap.h"
>  #include "stream.h"
>  #include "stream-ssl.h"
> @@ -197,6 +200,33 @@ get_ovnsb_remote(struct ovsdb_idl *ovs_idl)
>      }
>  }
>
> +/* Retrieves the OVN Southbound remote's json session probe interval from
> the
> + * "external-ids:ovn-remote-probe-interval" key in 'ovs_idl' and returns
> it.
> + *
> + * This function Must be called after get_ovnsb_remote().
> + *
> + */
>
s /Must/must


> +static int
> +get_ovnsb_remote_probe_interval(struct ovsdb_idl *ovs_idl)
> +{
> +    const struct ovsrec_open_vswitch *cfg =
> ovsrec_open_vswitch_first(ovs_idl);
> +    if (cfg) {
> +        const char * probe_interval =
> +                smap_get(&cfg->external_ids, "ovn-remote-probe-interval");
> +        if (probe_interval) {
> +            int ret = 0;
> +            if (str_to_int(probe_interval, 10, & ret))
> +            {
> +                VLOG_INFO("OVN OVSDB remote probe interval is %d ms",
> ret);
> +                return ret;
> +            }
> +        }
> +    }
> +
> +    return RECONNECT_DEFAULT_PROBE_INTERVAL;
> +}
> +
>
I think I will be slightly better if get_ovnsb_remote_probe_interval()
returns whether the field is set
in OVSDB. if it is not set, or can not be read correctly.  In those case,
we can simply
skip the call to ovsdb_set_probe_interval(). This way, we don't have to
provide the default from here.


> +
>  int
>  main(int argc, char *argv[])
>  {
> @@ -260,6 +290,9 @@ main(int argc, char *argv[])
>          ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class, true, true));
>      ovsdb_idl_get_initial_snapshot(ovnsb_idl_loop.idl);
>
> +    int probe_interval =
> get_ovnsb_remote_probe_interval(ovs_idl_loop.idl);
> +    ovsdb_set_probe_interval(ovnsb_idl_loop.idl, probe_interval);
> +


Currently, connection made over unix domain socket will not enable probe,
since probing is not necessary.
It seems this patch would enable probing for unix domain socket, did I read
the patch wrong?

     /* Initialize connection tracking zones. */
>      struct simap ct_zones = SIMAP_INITIALIZER(&ct_zones);
>      unsigned long ct_zone_bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)];
> --
> 2.5.4 (Apple Git-61)
>
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>