Hi Ben,
Thanks for the review. For the ovn-controller changes, do you mean I will
make the call to ovsdb_idl_set_probe_interval() without checking if the
value was indeed changed ? Or should I include that check along with your
suggestion ?
Thanks,
Nirapada
Ben Pfaff <b...@ovn.org> wrote on 05/06/2016 02:08:09 PM:
> From: Ben Pfaff <b...@ovn.org>
> To: Nirapada Ghosh/San Jose/IBM@IBMUS
> Cc: dev@openvswitch.org
> Date: 05/06/2016 02:08 PM
> Subject: Re: [ovs-dev] [PATCH V5] ovn-controller: reload configured
> SB probe timer
>
> On Fri, Apr 22, 2016 at 04:10:31PM -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.
> >
> > Signed-off-by: Nirapada Ghosh <ngh...@us.ibm.com>
>
> Hi Nirapada, thanks for the revision.
>
> There are two logical changes here, and each of them should be a
> separate commit. The first change is the one that changes
> rconn_connect() to set the probe interval to zero if probes are not
> needed.
>
> The second change is the one to ovn-controller. This change seems to
> add a lot more code than necessary. Why not something simple like this?
>
> static void
> update_probe_interval(const struct ovsrec_open_vswitch *cfg,
> const struct ovsdb_idl *sb_idl)
> {
> 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));
> }
>
> Thanks,
>
> Ben.
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
