On Mon, Feb 29, 2016 at 7:26 AM, Huang Lei <148012...@qq.com> wrote:
> From: Huang Lei <lhua...@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 <lhua...@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@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
--
Russell Bryant
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
