| Message ID | 1466008158-61778-1-git-send-email-nghosh@us.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
"dev" <dev-bounces@openvswitch.org> wrote on 06/15/2016 11:29:18 AM: > From: Nirapada Ghosh/San Jose/IBM@IBMUS > To: dev@openvswitch.org > Date: 06/15/2016 11:29 AM > Subject: [ovs-dev] [PATCH V9] ovn-controller: reload configured SB probe timer > Sent by: "dev" <dev-bounces@openvswitch.org> > > From: Nirapada Ghosh <nghosh@us.ibm.com> > > The probe timer between ovn-controller and OVN Southbound > 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. > > 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. > Signed-off-by: Nirapada Ghosh <nghosh@us.ibm.com> > > --- This needs more work, both in terms of the dependency on pkg-config (which IIRC is OS specific) and the potential infinite recursion loop when the feature is turned on... Ryan
"dev" <dev-bounces@openvswitch.org> wrote on 06/15/2016 02:21:48 PM: > From: Ryan Moats/Omaha/IBM@IBMUS > To: Nirapada Ghosh/San Jose/IBM@IBMUS > Cc: dev@openvswitch.org > Date: 06/15/2016 02:22 PM > Subject: Re: [ovs-dev] [PATCH V9] ovn-controller: reload configured > SB probetimer > Sent by: "dev" <dev-bounces@openvswitch.org> > > "dev" <dev-bounces@openvswitch.org> wrote on 06/15/2016 11:29:18 AM: > > > From: Nirapada Ghosh/San Jose/IBM@IBMUS > > To: dev@openvswitch.org > > Date: 06/15/2016 11:29 AM > > Subject: [ovs-dev] [PATCH V9] ovn-controller: reload configured SB probe > timer > > Sent by: "dev" <dev-bounces@openvswitch.org> > > > > From: Nirapada Ghosh <nghosh@us.ibm.com> > > > > The probe timer between ovn-controller and OVN Southbound > > 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. > > > > 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. > > Signed-off-by: Nirapada Ghosh <nghosh@us.ibm.com> > > > > --- > > This needs more work, both in terms of the dependency on pkg-config > (which IIRC is OS specific) and the potential infinite recursion > loop when the feature is turned on... Please disregard the above - I replied to the wrong patch with it (my bad). However, my review makes me question the following code snippet: @@ -418,6 +392,13 @@ main(int argc, char *argv[]) .ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop), }; + if (!cfg) { + ovsrec_open_vswitch_first(ovs_idl_loop.idl); Shouldn't the above be cfg = ovsrec_open_vswitch_first(ovs_idl_loop.idl) ? + } + if (cfg) { + update_probe_interval(cfg, ovnsb_idl_loop.idl); + } + Ryan Moats
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 356a94b..66d6bf7 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -60,7 +60,10 @@ static unixctl_cb_func ovn_controller_exit; static unixctl_cb_func ct_zone_list; #define DEFAULT_BRIDGE_NAME "br-int" +#define DEFAULT_PROBE_INTERVAL_MSEC 5000 +static void update_probe_interval(const struct ovsrec_open_vswitch *cfg, + const struct ovsdb_idl *sb_idl); static void parse_options(int argc, char *argv[]); OVS_NO_RETURN static void usage(void); @@ -228,32 +231,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; -} - static void update_ct_zones(struct sset *lports, struct hmap *patched_datapaths, struct simap *ct_zones, unsigned long *ct_zone_bitmap) @@ -385,10 +362,7 @@ 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 = NULL; /* Initialize connection tracking zones. */ struct simap ct_zones = SIMAP_INITIALIZER(&ct_zones); @@ -418,6 +392,13 @@ main(int argc, char *argv[]) .ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop), }; + if (!cfg) { + ovsrec_open_vswitch_first(ovs_idl_loop.idl); + } + if (cfg) { + update_probe_interval(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. */ struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths); @@ -657,3 +638,22 @@ ct_zone_list(struct unixctl_conn *conn, int argc OVS_UNUSED, unixctl_command_reply(conn, ds_cstr(&ds)); ds_destroy(&ds); } + +/* This function will set the SB probe timer value for the session. + * Arguments: + * 'cfg': Holding the external-id values read from southbound DB. + * 'sb_idl': pointer to the ovs_idl connection to OVN southbound. + */ +static void +update_probe_interval(const struct ovsrec_open_vswitch *cfg, + const struct ovsdb_idl *sb_idl) +{ + if (cfg == NULL) { + return; + } + int interval = smap_get_int(&cfg->external_ids, + "ovn-remote-probe-interval", 0); + ovsdb_idl_set_probe_interval(sb_idl, (interval >= 0 + ? interval + : DEFAULT_PROBE_INTERVAL_MSEC)); +}