On 08/08/2017 08:24 PM, Guru Shetty wrote:
> 
> 
> On 8 August 2017 at 11:05, Russell Bryant <russ...@ovn.org
> <mailto:russ...@ovn.org>> wrote:
> 
>     On Tue, Aug 8, 2017 at 1:32 PM, Guru Shetty <g...@ovn.org
>     <mailto:g...@ovn.org>> wrote:
>     > On 8 August 2017 at 07:42, Timothy Redaelli <tredae...@redhat.com 
> <mailto:tredae...@redhat.com>> wrote:
>     >
>     >> The reload procedure will trigger a script that saves the flows and tlv
>     >> maps then it restarts ovsdb-server, it stops ovs-vswitchd, it sets
>     >> other_config:flow-restore-wait=true (to wait till flow restore is
>     >> finished), it starts ovs-vswitchd, it restore the backupped flows/tlv
>     >> maps and it removes other_config:flow-restore-wait=true (logic mostly
>     >> ripped
>     >> from ovs-ctl).
>     >>
>     >> It uses systemctl with --job-mode=ignore-dependencies to restart
>     >> ovsdb-server
>     >> and stop and start ovs-vswitchd in order to avoid systemd to restart 
> the
>     >> other components due to dependencies (as explained in
>     >> rhel/README.RHEL.rst).
>     >>
>     >> It also uses --bundle, when available, in order to minimize the 
> downtime.
>     >>
>     >
>     > The core script can probably be added to ovs-ctl, reuse some functions 
> and
>     > then you can call it? This way, other platforms can benefit too.
> 
>     Sounds like a good suggestion.

The script is actually "specialized" to how Fedora/RHEL starts
openvswitch since, I think, only Fedora/RHEL have ovsdb-server and
ovs-vswitchd as splitted systemd unit files.

Ubuntu uses only one .service file (openvswitch-nonetwork.service) that
starts both ovsdb-server and ovs-vswitch at the same time, so this
approach can't work for them.

(It seems) Debian only has /etc/init.d/openvswitch-switch and so this
doesn't apply either.

>     What would you think about updating the stop/start behavior to do
>     this, as well?  It looks like this makes "reload" actually just an
>     "intelligent restart".  systemctl has a "restart" command too, which
>     looks like it just does stop/start, and it'd be nice to make that work
>     better too.

You can't change the behavior of "systemctl restart", systemd will
always call ExecStop and ExecStart, that's the reason I add the new
script as "reload" target, but that makes sense since it's not a
"standard" restart (it keeps the flows). Moreover I'd like to keep user
to have the choice if reloading or restarting the daemon.

> We had this discussion when we added intelligent restart
> to rhel/etc_init.d_openvswitch and debian/openvswitch-switch.init
> 
> Currently both rhel and debian will take a "--save-flows=yes" option to
> restart and it will do the same things as the script proposed here does
> (except for --bundle) via ovs-ctl's restart function . We wanted
> "--save-flows=yes" to be the default behavior instead of explicitly
> calling it. But a long term user of OVS said that they use "restart" for
> a fresh start. That is, they bake in the assumption that a "restart"
> means the added flows disappear.

My implementation doesn't change this assumption, the only assumption is
that if you use "reload" you want to keep all the flows (that's the same
assumption you have when you use "systemctl reload" on nginx or apache)

> What this patch does in addition to the "restart" function in ovs-ctl is
> that it uses --bundle. "restart" function in ovs-ctl calls functions in
> "ovs-save". So it may make sense for this patch to add the "--bundle"
> feature to ovs-save and just re-use it?

Sounds like a good suggestion.

I'll do another patchset with a commit that adds the --bundle option +
get_highest_ofp_version in ovs-save and another commit that changes
ovs-reload script to use it.

get_highest_ofp_version is used to find if we can use --bundle and to
make ovs-ofctl work when you have a bridge with only OpenFlow15+
protocols enabled.

Any other suggestions?
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to