[ovs-dev] [PATCH V4] ovn-controller: reload configured SB probe timer
There are four sessions established from ovn-controller to the following:
OVN Southbound — JSONRPC based
Local ovsdb — JSONRPC based
Local vswitchd — openflow based from ofctrl
Local vswitchd — openflow based from pinctrl
All of these sessions have their own probe_interval, and currently one
[SB] of them can be configured using ovn-vsctl command, but that is not
effective on the fly —in other words, ovn-controller has to be restarted to
use that probe_timer value, this patch takes care of that.
For the local ovsdb connection, probe timer is already disabled. For the last
two connections, they do not need probe_timer as they are over unix domain
socket. This patch takes care of that as well.
This change has been tested putting logs in several places like in
ovn-controller.c, lib/rconn.c to make sure the right probe_timer
values are in effect. Also, by making sure from ovn-controller's
log file that there is no more reconnect happening due to probe
under heavy load.
Author: Nirapada Ghosh
Date: Wed Mar 30 19:03:10 2016 -0700
Signed-off-by: Nirapada Ghosh
diff --git a/lib/rconn.c b/lib/rconn.c
index 6de4c63..54f3745 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -339,6 +339,9 @@ rconn_connect(struct rconn *rc, const char *target, const
char *name)
rconn_disconnect__(rc);
rconn_set_target__(rc, target, name);
rc->reliable = true;
+if (!stream_or_pstream_needs_probes()) {
+rc->probe_interval =0;
+}
reconnect(rc);
ovs_mutex_unlock(&rc->mutex);
}
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 7c68c9d..92e9543 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -56,6 +56,13 @@ static unixctl_cb_func ovn_controller_exit;
static unixctl_cb_func ct_zone_list;
#define DEFAULT_BRIDGE_NAME "br-int"
+#define DEFAULT_PROBE_INTERVAL 5
+
+static void set_probe_timer_if_changed(const struct ovsrec_open_vswitch *cfg,
+ const struct ovsdb_idl *sb_idl);
+static bool extract_probe_timer(const struct ovsrec_open_vswitch *cfg,
+char *key_name,
+int *ret_value);
static void parse_options(int argc, char *argv[]);
OVS_NO_RETURN static void usage(void);
@@ -217,32 +224,6 @@ 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) {
-return false;
-}
-
-const char *probe_interval =
-smap_get(&cfg->external_ids, "ovn-remote-probe-interval");
-if (probe_interval) {
-if (str_to_int(probe_interval, 10, value)) {
-return true;
-}
-
-VLOG_WARN("Invalid value for OVN remote probe interval: %s",
- probe_interval);
-}
-
-return false;
-}
-
int
main(int argc, char *argv[])
{
@@ -306,10 +287,12 @@ 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)) {
-ovsdb_idl_set_probe_interval(ovnsb_idl_loop.idl, probe_interval);
-}
+const struct ovsrec_open_vswitch *cfg =
+ovsrec_open_vswitch_first(ovs_idl_loop.idl);
+if (!cfg) {
+return false;
+ }
+set_probe_timer_if_changed(cfg,ovnsb_idl_loop.idl);
/* Initialize connection tracking zones. */
struct simap ct_zones = SIMAP_INITIALIZER(&ct_zones);
@@ -327,6 +310,7 @@ main(int argc, char *argv[])
.ovnsb_idl = ovnsb_idl_loop.idl,
.ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop),
};
+set_probe_timer_if_changed(cfg,ovnsb_idl_loop.idl);
/* Contains "struct local_datpath" nodes whose hash values are the
* tunnel_key of datapaths with at least one local port binding. */
@@ -561,3 +545,55 @@ ct_zone_list(struct unixctl_conn *conn, int argc
OVS_UNUSED,
unixctl_command_reply(conn, ds_cstr(&ds));
ds_destroy(&ds);
}
+
+/* If SB probe timer is changed using ovs-vsctl command, this function
+ * will set that probe timer value for the session.
+ * cfg: Holding the external-id values read from southbound DB.
+ * sb_idl: pointer to the ovs_idl connection to OVN southbound.
+ */
+static void
+set_probe_timer_if_changed(const struct ovsrec_open_vswitch *cfg,
+ const struct ovsdb_idl *sb_idl)
+{
+static int probe_int_sb = DEFAULT_PROBE_INTERVAL * 1000;
+int probe_int_sb_new = probe_int_sb;
+
+extract_probe_timer(cfg, "ovn-remote-prob
Re: [ovs-dev] [PATCH V4] ovn-controller: reload configured SB probe timer
On 4/14/16, 3:24 PM, "dev on behalf of ngh...@us.ibm.com"
wrote:
>There are four sessions established from ovn-controller to the following:
>OVN Southbound — JSONRPC based
>Local ovsdb — JSONRPC based
>Local vswitchd — openflow based from ofctrl
>Local vswitchd — openflow based from pinctrl
>
>All of these sessions have their own probe_interval, and currently one
>[SB] of them can be configured using ovn-vsctl command, but that is not
>effective on the fly —in other words, ovn-controller has to be restarted to
>use that probe_timer value, this patch takes care of that.
>For the local ovsdb connection, probe timer is already disabled. For the last
>two connections, they do not need probe_timer as they are over unix domain
>socket. This patch takes care of that as well.
>
>This change has been tested putting logs in several places like in
>ovn-controller.c, lib/rconn.c to make sure the right probe_timer
>values are in effect. Also, by making sure from ovn-controller's
>log file that there is no more reconnect happening due to probe
>under heavy load.
>
>Author: Nirapada Ghosh
>Date: Wed Mar 30 19:03:10 2016 -0700
>Signed-off-by: Nirapada Ghosh
>
>diff --git a/lib/rconn.c b/lib/rconn.c
>index 6de4c63..54f3745 100644
>--- a/lib/rconn.c
>+++ b/lib/rconn.c
>@@ -339,6 +339,9 @@ rconn_connect(struct rconn *rc, const char *target, const
>char *name)
> rconn_disconnect__(rc);
> rconn_set_target__(rc, target, name);
> rc->reliable = true;
>+if (!stream_or_pstream_needs_probes()) {
stream_or_pstream_needs_probes() is called without argument ‘target’, compile
gives a warning:
warning: implicit declaration of function ` stream_or_stream_needs_probs’ …
Header file stream.h should be included.
>+rc->probe_interval =0;
Need a whitespace at right side of ‘=‘.
>+}
> reconnect(rc);
> ovs_mutex_unlock(&rc->mutex);
> }
>diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
>index 7c68c9d..92e9543 100644
>--- a/ovn/controller/ovn-controller.c
>+++ b/ovn/controller/ovn-controller.c
>@@ -56,6 +56,13 @@ static unixctl_cb_func ovn_controller_exit;
> static unixctl_cb_func ct_zone_list;
>
> #define DEFAULT_BRIDGE_NAME "br-int"
>+#define DEFAULT_PROBE_INTERVAL 5
>+
>+static void set_probe_timer_if_changed(const struct ovsrec_open_vswitch *cfg,
>+ const struct ovsdb_idl *sb_idl);
>+static bool extract_probe_timer(const struct ovsrec_open_vswitch *cfg,
>+char *key_name,
>+int *ret_value);
>
> static void parse_options(int argc, char *argv[]);
> OVS_NO_RETURN static void usage(void);
>@@ -217,32 +224,6 @@ 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) {
>-return false;
>-}
>-
>-const char *probe_interval =
>-smap_get(&cfg->external_ids, "ovn-remote-probe-interval");
>-if (probe_interval) {
>-if (str_to_int(probe_interval, 10, value)) {
>-return true;
>-}
>-
>-VLOG_WARN("Invalid value for OVN remote probe interval: %s",
>- probe_interval);
>-}
>-
>-return false;
>-}
>-
> int
> main(int argc, char *argv[])
> {
>@@ -306,10 +287,12 @@ 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)) {
>-ovsdb_idl_set_probe_interval(ovnsb_idl_loop.idl, probe_interval);
>-}
>+const struct ovsrec_open_vswitch *cfg =
>+ovsrec_open_vswitch_first(ovs_idl_loop.idl);
>+if (!cfg) {
>+return false;
>+ }
>+set_probe_timer_if_changed(cfg,ovnsb_idl_loop.idl);
>
> /* Initialize connection tracking zones. */
> struct simap ct_zones = SIMAP_INITIALIZER(&ct_zones);
>@@ -327,6 +310,7 @@ main(int argc, char *argv[])
> .ovnsb_idl = ovnsb_idl_loop.idl,
> .ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop),
> };
>+set_probe_timer_if_changed(cfg,ovnsb_idl_loop.idl);
Need a whitespace after comma.
>
> /* Contains "struct local_datpath" nodes whose hash values are the
> * tunnel_key of datapaths with at least one local port binding. */
>@@ -561,3 +545,55 @@ ct_zone_list(struct unixctl_conn *conn, int argc
>OVS_UNUSED,
> unixctl_command_reply(conn, ds_cstr(&ds));
> ds_destroy(&ds);
> }
>+
>+/* If SB probe timer is changed using ovs-vsctl command, this function
Re: [ovs-dev] [PATCH V4] ovn-controller: reload configured SB probe timer
On 4/14/16, 3:24 PM, "dev on behalf of ngh...@us.ibm.com" <
dev-boun...@openvswitch.org on behalf of ngh...@us.ibm.com> wrote:
There are four sessions established from ovn-controller to the following:
OVN Southbound — JSONRPC based
Local ovsdb — JSONRPC based
Local vswitchd — openflow based from ofctrl
Local vswitchd — openflow based from pinctrl
All of these sessions have their own probe_interval, and currently one
[SB] of them can be configured using ovn-vsctl command, but that is not
effective on the fly —in other words, ovn-controller has to be restarted to
use that probe_timer value, this patch takes care of that.
For the local ovsdb connection, probe timer is already disabled. For the
last
two connections, they do not need probe_timer as they are over unix domain
socket. This patch takes care of that as well.
This change has been tested putting logs in several places like in
ovn-controller.c, lib/rconn.c to make sure the right probe_timer
values are in effect. Also, by making sure from ovn-controller's
log file that there is no more reconnect happening due to probe
under heavy load.
Author: Nirapada Ghosh
Date: Wed Mar 30 19:03:10 2016 -0700
Signed-off-by: Nirapada Ghosh
diff --git a/lib/rconn.c b/lib/rconn.c
index 6de4c63..54f3745 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -339,6 +339,9 @@ rconn_connect(struct rconn *rc, const char *target,
const char *name)
rconn_disconnect__(rc);
rconn_set_target__(rc, target, name);
rc->reliable = true;
+if (!stream_or_pstream_needs_probes()) {
stream_or_pstream_needs_probes() is called without argument ‘target’,
compile gives a warning:
warning: implicit declaration of function ` stream_or_stream_needs_probs’ …
Header file stream.h should be included.
+rc->probe_interval =0;
Need a whitespace at right side of ‘=‘.
+}
reconnect(rc);
ovs_mutex_unlock(&rc->mutex);
}
diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
index 7c68c9d..92e9543 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -56,6 +56,13 @@ static unixctl_cb_func ovn_controller_exit;
static unixctl_cb_func ct_zone_list;
#define DEFAULT_BRIDGE_NAME "br-int"
+#define DEFAULT_PROBE_INTERVAL 5
+
+static void set_probe_timer_if_changed(const struct ovsrec_open_vswitch
*cfg,
+ const struct ovsdb_idl *sb_idl);
+static bool extract_probe_timer(const struct ovsrec_open_vswitch *cfg,
+char *key_name,
+int *ret_value);
static void parse_options(int argc, char *argv[]);
OVS_NO_RETURN static void usage(void);
@@ -217,32 +224,6 @@ 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) {
-return false;
-}
-
-const char *probe_interval =
-smap_get(&cfg->external_ids, "ovn-remote-probe-interval");
-if (probe_interval) {
-if (str_to_int(probe_interval, 10, value)) {
-return true;
-}
-
-VLOG_WARN("Invalid value for OVN remote probe interval: %s",
- probe_interval);
-}
-
-return false;
-}
-
int
main(int argc, char *argv[])
{
@@ -306,10 +287,12 @@ 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)) {
-ovsdb_idl_set_probe_interval(ovnsb_idl_loop.idl, probe_interval);
-}
+const struct ovsrec_open_vswitch *cfg =
+ovsrec_open_vswitch_first(ovs_idl_loop.idl);
+if (!cfg) {
+return false;
+ }
+set_probe_timer_if_changed(cfg,ovnsb_idl_loop.idl);
/* Initialize connection tracking zones. */
struct simap ct_zones = SIMAP_INITIALIZER(&ct_zones);
@@ -327,6 +310,7 @@ main(int argc, char *argv[])
.ovnsb_idl = ovnsb_idl_loop.idl,
.ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop),
};
+set_probe_timer_if_changed(cfg,ovnsb_idl_loop.idl);
Need a whitespace after comma.
/* Contains "struct local_datpath" nodes whose hash values are the
* tunnel_key of datapaths with at least one local port binding. */
@@ -561,3 +545,55 @@ ct_zone_list(struct unixctl_conn *conn, int argc
OVS_UNUSED,
unixctl_command_reply(conn, ds_cstr(&ds));
ds_destroy(&ds);
}
+
+/* If SB probe timer is changed using ovs-vsctl command, this function
+ * will set that probe timer value for the session.
+ * cfg: Holding the external-id val
Re: [ovs-dev] [PATCH V4] ovn-controller: reload configured SB probe timer
On Thu, Apr 14, 2016 at 12:24:03AM -0700, ngh...@us.ibm.com wrote:
> There are four sessions established from ovn-controller to the following:
> OVN Southbound — JSONRPC based
> Local ovsdb — JSONRPC based
> Local vswitchd — openflow based from ofctrl
> Local vswitchd — openflow based from pinctrl
>
> All of these sessions have their own probe_interval, and currently one
> [SB] of them can be configured using ovn-vsctl command, but that is not
> effective on the fly —in other words, ovn-controller has to be restarted to
> use that probe_timer value, this patch takes care of that.
> For the local ovsdb connection, probe timer is already disabled. For the last
> two connections, they do not need probe_timer as they are over unix domain
> socket. This patch takes care of that as well.
>
> This change has been tested putting logs in several places like in
> ovn-controller.c, lib/rconn.c to make sure the right probe_timer
> values are in effect. Also, by making sure from ovn-controller's
> log file that there is no more reconnect happening due to probe
> under heavy load.
I agree with Lei Huang's comments.
Fails to build:
../lib/rconn.c:342:10: error: implicit declaration of function
'stream_or_pstream_needs_probes' is invalid in C99
[-Werror,-Wimplicit-function-declaration]
In addition, please be careful about style issues. For example, there
should be a space on either side of "=", and a space after each ",".
Author and date should not be part of the commit message:
> Author: Nirapada Ghosh
> Date: Wed Mar 30 19:03:10 2016 -0700
> Signed-off-by: Nirapada Ghosh
> +/* If SB probe timer is changed using ovs-vsctl command, this function
> + * will set that probe timer value for the session.
> + * cfg: Holding the external-id values read from southbound DB.
> + * sb_idl: pointer to the ovs_idl connection to OVN southbound.
> + */
> +static void
> +set_probe_timer_if_changed(const struct ovsrec_open_vswitch *cfg,
> + const struct ovsdb_idl *sb_idl)
> +{
> +static int probe_int_sb = DEFAULT_PROBE_INTERVAL * 1000;
> +int probe_int_sb_new = probe_int_sb;
> +
> +extract_probe_timer(cfg, "ovn-remote-probe-interval", &probe_int_sb_new);
> +if (probe_int_sb_new != probe_int_sb) {
> +ovsdb_idl_set_probe_interval(sb_idl, probe_int_sb_new);
We generally don't log this kind of thing:
> +VLOG_INFO("OVN SB probe interval changed %d->%d ",
> + probe_int_sb,
> + probe_int_sb_new);
> +probe_int_sb = probe_int_sb_new;
> +}
> +}
> +
> +/* Given key_name, the following function retrieves probe_timer value from
> the
> + * configuration passed, this configuration comes from the "external-ids"
> + * which were configured via ovs-vsctl command.
> + *
> + * cfg: Holding the external-id values read from NB database.
> + * keyname: Name to extract the value for.
> + * ret_value_ptr: Pointer to integer location where the value read
> + * should be copied.
> + * The function returns true if success, keeps the original
> + * value of ret_value_ptr intact in case of a failure.
> + */
> +static bool
> +extract_probe_timer(const struct ovsrec_open_vswitch *cfg,
> +char *key_name,
> +int *ret_value_ptr)
> +{
> +const char *probe_interval= smap_get(&cfg->external_ids, key_name);
This comment seems unnecessary:
> +int ret_value_temp=0; /* Temporary location to hold the value, in case of
> + * failure, str_to_int() sets the ret_value to 0,
> + * which is a valid value of probe_timer. */
> +if ((!probe_interval) ||
> +(!str_to_int(probe_interval, 10, &ret_value_temp))) {
> +VLOG_WARN("OVN OVSDB invalid remote probe interval:%s for %s",
> + probe_interval, key_name);
> +return false;
> +}
> +*ret_value_ptr = ret_value_temp;
> +return true;
> +}
Thanks,
Ben.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
