"dev" <[email protected]> wrote on 06/14/2016 07:13:54 PM:

> From: Nirapada Ghosh/San Jose/IBM@IBMUS
> To: [email protected]
> Date: 06/14/2016 07:14 PM
> Subject: [ovs-dev] [PATCH V8] ovn-controller: reload configured SB probe
timer
> Sent by: "dev" <[email protected]>
>
> 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 <[email protected]>
>

Nit: this doesn't apply completely cleanly:
git am
~/ovs_patches_to_review/ovs-dev-V8-ovn-controller-reload-configured-SB-probe-timer.patch
Applying: ovn-controller: reload configured SB probe timer
/home/stack/git/ovs/.git/rebase-apply/patch:102: new blank line at EOF.
+
warning: 1 line adds whitespace errors.


> ---
>  ovn/controller/ovn-controller.c | 61 ++++++++++++++++++++
> +--------------------
>  1 file changed, 31 insertions(+), 30 deletions(-)
>
> diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
> index 356a94b..dc20ce5 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);

This needs to assign to cfg, doesn't it??? i.e.
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,23 @@ 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));
> +}
> +

I believe that this last line is the source of the whitespace error nit.

Ryan Moats
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to