Thanks for your review. BR Huang Lei
From: Andy Zhou <az...@ovn.org<mailto:az...@ovn.org>> Date: Monday, February 22, 2016 at 7:38 AM To: Huang Lei <148012...@qq.com<mailto:148012...@qq.com>> Cc: "dev@openvswitch.org<mailto:dev@openvswitch.org>" <dev@openvswitch.org<mailto:dev@openvswitch.org>>, "Huang, Lei" <lhua...@ebay.com<mailto:lhua...@ebay.com>> Subject: Re: [ovs-dev] [PATCH] ovn-controller: Add external-ids:ovn-remote-probe-interval On Sun, Feb 21, 2016 at 1:26 PM, Huang Lei <148012...@qq.com<mailto:148012...@qq.com>> wrote: From: Huang Lei <lhua...@ebay.com<mailto:lhua...@ebay.com>> For setting the inactivity probe interval of the json session to the OVN southbound database. Signed-off-by: Huang Lei <lhua...@ebay.com<mailto:lhua...@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@openvswitch.org<mailto:dev@openvswitch.org> http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev