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

Russell Bryant russell at ovn.org
Mon Feb 29 16:26:38 UTC 2016


On Mon, Feb 29, 2016 at 7:26 AM, Huang Lei <148012453 at qq.com> wrote:

> From: Huang Lei <lhuang8 at ebay.com>
>
> For setting the inactivity probe interval of the json session from
> ovn-controller to the OVN southbound database.
>
> Signed-off-by: Huang Lei <lhuang8 at ebay.com>
> ---
>  lib/ovsdb-idl.c                     | 10 ++++++++++
>  lib/ovsdb-idl.h                     |  4 ++++
>  ovn/controller/ovn-controller.8.xml | 14 ++++++++++++++
>  ovn/controller/ovn-controller.c     | 32 ++++++++++++++++++++++++++++++++
>  4 files changed, 60 insertions(+)
>
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index 4cb1c81..022f0cf 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -565,6 +565,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_idl_set_probe_interval(const struct ovsdb_idl *idl, int
> probe_interval)
> +{
> +       jsonrpc_session_set_probe_interval(idl->session, 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..96d9436 100644
> --- a/lib/ovsdb-idl.h
> +++ b/lib/ovsdb-idl.h
> @@ -68,7 +68,11 @@ 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_idl_set_probe_interval(const struct ovsdb_idl *, int
> probe_interval);
> +
>
> +
>

It looks like you added an extra new blank line here.


>  /* Choosing columns and tables to replicate. */
>
>  /* Modes with which the IDL can monitor a column.
> diff --git a/ovn/controller/ovn-controller.8.xml
> b/ovn/controller/ovn-controller.8.xml
> index b261af9..6e57fc6 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 the value 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 f5769b5..358b69d 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -34,7 +34,9 @@
>  #include "poll-loop.h"
>  #include "fatal-signal.h"
>  #include "lib/hmap.h"
> +#include "lib/ovsdb-idl.h"
>  #include "lib/vswitch-idl.h"
> +#include "lib/util.h"
>  #include "smap.h"
>  #include "stream.h"
>  #include "stream-ssl.h"
> @@ -198,6 +200,31 @@ 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().
> + *
> + */
> +static bool
> +get_ovnsb_remote_probe_interval(struct ovsdb_idl *ovs_idl, int *value)
> +{
> +    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) {
> +            if (str_to_int(probe_interval, 10, value))
> +            {
> +                VLOG_INFO("OVN OVSDB remote probe interval is %d ms",
> *value);
> +                return true;
> +            }
> +        }
> +    }
> +
> +    return false;
> +}
>

There's one coding style issue here.  The { should be on the same line as
if () in the middle of the function.

Feel free to ignore the rest of this if you'd like ...

I like to avoid nested blocks of code where it's easy enough to do so.  I
think it helps make things more readable.  In this case, I'd do something
like:

{
    const struct ovsrec_open_vswitch *cfg = ...;
    if (!cfg) {
        return false;
    }

    const char *probe_interval = ...;
    if (!probe_interval) {
        return false;
    }

    if (!str_to_int(...)) {
        ... maybe add a new warning log message here for the error
condition? ...
        return false;
    }

    VLOG_INFO("...");

    return true;
}


> +
>  int
>  main(int argc, char *argv[])
>  {
> @@ -261,6 +288,11 @@ 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 = 0;
> +    if (get_ovnsb_remote_probe_interval(ovs_idl_loop.idl,
> &probe_interval)) {
>

I might add a comment here noting that this function will ignore any value
of probe_interval that is less than 1000, and that's why we're not trying
to do any validation in this code.


> +        ovsdb_idl_set_probe_interval(ovnsb_idl_loop.idl, probe_interval);
> +    }
> +
>      /* 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
>



-- 
Russell Bryant



More information about the dev mailing list