Re: [ovs-dev] [PATCH] rhel: Add support for "systemctl reload openvswitch"

[Ben Pfaff]

On Fri, Nov 03, 2017 at 07:26:05PM +0100, Timothy M. Redaelli wrote:
> On 11/03/2017 07:20 PM, Ben Pfaff wrote:
> [...]
> >>
> >> This looks good to me, my only comment is that the script is in rhel/
> >> and uses systemd so maybe we should reflect that in the script's name?
> >> Because OVS itself might provide an ovs-reload too and then we will
> >> have issues.
> >>
> >> What do you think?
> > 
> > Is this script reasonably generic, as opposed to RHEL-specific?  If so
> > then we could move it into utilities or make it a command under ovs-ctl.
> 
> Unlucky this is specific for RHEL (and derivates) since Debian/Ubuntu
> uses only one service file for both ovsdb-server and ovs-vswitchd and so
> this script can't work, but I think they can use "ovs-ctl restart" that
> have the same behavior when you don't have multiple systemd service files.

OK.  If Debian/Ubuntu changes someday, we can reconsider.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] rhel: Add support for "systemctl reload openvswitch"

[Timothy M. Redaelli]

On 11/03/2017 07:20 PM, Ben Pfaff wrote:
[...]
>>
>> This looks good to me, my only comment is that the script is in rhel/
>> and uses systemd so maybe we should reflect that in the script's name?
>> Because OVS itself might provide an ovs-reload too and then we will
>> have issues.
>>
>> What do you think?
> 
> Is this script reasonably generic, as opposed to RHEL-specific?  If so
> then we could move it into utilities or make it a command under ovs-ctl.

Unlucky this is specific for RHEL (and derivates) since Debian/Ubuntu
uses only one service file for both ovsdb-server and ovs-vswitchd and so
this script can't work, but I think they can use "ovs-ctl restart" that
have the same behavior when you don't have multiple systemd service files.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] rhel: Add support for "systemctl reload openvswitch"

[Ben Pfaff]

On Fri, Nov 03, 2017 at 01:55:00PM -0200, Flavio Leitner wrote:
> On Tue, 31 Oct 2017 12:11:32 +0100
> Timothy Redaelli  wrote:
> 
> > The reload procedure will trigger a script that saves the flows and tlv
> > maps (using ovs-save) 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).
> > 
> > Signed-off-by: Timothy Redaelli 
> > ---
> >  rhel/automake.mk |  1 +
> >  rhel/openvswitch-fedora.spec.in  |  5 
> >  rhel/usr_lib_systemd_system_openvswitch.service  |  2 +-
> >  rhel/usr_lib_systemd_system_ovsdb-server.service |  1 -
> >  rhel/usr_share_openvswitch_scripts_ovs-reload| 36 
> > 
> >  5 files changed, 43 insertions(+), 2 deletions(-)
> >  create mode 100755 rhel/usr_share_openvswitch_scripts_ovs-reload
> > 
> > diff --git a/rhel/automake.mk b/rhel/automake.mk
> > index 9336f0912..0955dceed 100644
> > --- a/rhel/automake.mk
> > +++ b/rhel/automake.mk
> > @@ -24,6 +24,7 @@ EXTRA_DIST += \
> > rhel/openvswitch.spec.in \
> > rhel/openvswitch-fedora.spec \
> > rhel/openvswitch-fedora.spec.in \
> > +   rhel/usr_share_openvswitch_scripts_ovs-reload \
> > rhel/usr_share_openvswitch_scripts_sysconfig.template \
> > rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
> > rhel/usr_lib_udev_rules.d_91-vfio.rules \
> > diff --git a/rhel/openvswitch-fedora.spec.in 
> > b/rhel/openvswitch-fedora.spec.in
> > index fb7d918c6..87bec39c9 100644
> > --- a/rhel/openvswitch-fedora.spec.in
> > +++ b/rhel/openvswitch-fedora.spec.in
> > @@ -314,6 +314,10 @@ install -d -m 0755 
> > $RPM_BUILD_ROOT%{_prefix}/lib/ocf/resource.d/ovn
> >  ln -s %{_datadir}/openvswitch/scripts/ovndb-servers.ocf \
> >$RPM_BUILD_ROOT%{_prefix}/lib/ocf/resource.d/ovn/ovndb-servers
> >  
> > +install -p -D -m 0755 \
> > +rhel/usr_share_openvswitch_scripts_ovs-reload \
> > +$RPM_BUILD_ROOT%{_datadir}/openvswitch/scripts/ovs-reload
> > +
> >  # remove unpackaged files
> >  rm -f $RPM_BUILD_ROOT%{_bindir}/ovs-parse-backtrace \
> >  $RPM_BUILD_ROOT%{_sbindir}/ovs-vlan-bug-workaround \
> > @@ -539,6 +543,7 @@ fi
> >  %{_datadir}/openvswitch/scripts/ovs-save
> >  %{_datadir}/openvswitch/scripts/ovs-vtep
> >  %{_datadir}/openvswitch/scripts/ovs-ctl
> > +%{_datadir}/openvswitch/scripts/ovs-reload
> >  %config %{_datadir}/openvswitch/vswitch.ovsschema
> >  %config %{_datadir}/openvswitch/vtep.ovsschema
> >  %{_bindir}/ovs-appctl
> > diff --git a/rhel/usr_lib_systemd_system_openvswitch.service 
> > b/rhel/usr_lib_systemd_system_openvswitch.service
> > index faca44b54..2cf29f0e9 100644
> > --- a/rhel/usr_lib_systemd_system_openvswitch.service
> > +++ b/rhel/usr_lib_systemd_system_openvswitch.service
> > @@ -9,7 +9,7 @@ Requires=ovs-vswitchd.service
> >  [Service]
> >  Type=oneshot
> >  ExecStart=/bin/true
> > -ExecReload=/bin/true
> > +ExecReload=/usr/share/openvswitch/scripts/ovs-reload
> >  ExecStop=/bin/true
> >  RemainAfterExit=yes
> >  
> > diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
> > b/rhel/usr_lib_systemd_system_ovsdb-server.service
> > index 5baac822d..234d39355 100644
> > --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
> > +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
> > @@ -3,7 +3,6 @@ Description=Open vSwitch Database Unit
> >  After=syslog.target network-pre.target
> >  Before=network.target network.service
> >  Wants=ovs-delete-transient-ports.service
> > -ReloadPropagatedFrom=openvswitch.service
> >  PartOf=openvswitch.service
> >  
> >  [Service]
> > diff --git a/rhel/usr_share_openvswitch_scripts_ovs-reload 
> > b/rhel/usr_share_openvswitch_scripts_ovs-reload
> > new file mode 100755
> > index 0..3ac1a46c6
> > --- /dev/null
> > +++ b/rhel/usr_share_openvswitch_scripts_ovs-reload
> > @@ -0,0 +1,36 @@
> > +#! /bin/sh
> > +
> > +# Copyright (c) 2017 Red Hat, Inc.
> > +#
> > +# Licensed under the Apache License, Version 2.0 (the "License");
> > +# you may not use this file except in compliance with the License.
> > +# You may obtain a copy of the License at:
> > +#
> > +# http://www.apache.org/licenses/LICENSE-2.0
> > +#
> > +# Unless required by applicable law or agreed to in writing, software
> > +# distributed under the License is distributed on an "AS IS" BASIS,
> > +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > +# See the License for the specific language governing 

Re: [ovs-dev] [PATCH] rhel: Add support for "systemctl reload openvswitch"

[Flavio Leitner]

On Tue, 31 Oct 2017 12:11:32 +0100
Timothy Redaelli  wrote:

> The reload procedure will trigger a script that saves the flows and tlv
> maps (using ovs-save) 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).
> 
> Signed-off-by: Timothy Redaelli 
> ---
>  rhel/automake.mk |  1 +
>  rhel/openvswitch-fedora.spec.in  |  5 
>  rhel/usr_lib_systemd_system_openvswitch.service  |  2 +-
>  rhel/usr_lib_systemd_system_ovsdb-server.service |  1 -
>  rhel/usr_share_openvswitch_scripts_ovs-reload| 36 
> 
>  5 files changed, 43 insertions(+), 2 deletions(-)
>  create mode 100755 rhel/usr_share_openvswitch_scripts_ovs-reload
> 
> diff --git a/rhel/automake.mk b/rhel/automake.mk
> index 9336f0912..0955dceed 100644
> --- a/rhel/automake.mk
> +++ b/rhel/automake.mk
> @@ -24,6 +24,7 @@ EXTRA_DIST += \
>   rhel/openvswitch.spec.in \
>   rhel/openvswitch-fedora.spec \
>   rhel/openvswitch-fedora.spec.in \
> + rhel/usr_share_openvswitch_scripts_ovs-reload \
>   rhel/usr_share_openvswitch_scripts_sysconfig.template \
>   rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
>   rhel/usr_lib_udev_rules.d_91-vfio.rules \
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index fb7d918c6..87bec39c9 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -314,6 +314,10 @@ install -d -m 0755 
> $RPM_BUILD_ROOT%{_prefix}/lib/ocf/resource.d/ovn
>  ln -s %{_datadir}/openvswitch/scripts/ovndb-servers.ocf \
>$RPM_BUILD_ROOT%{_prefix}/lib/ocf/resource.d/ovn/ovndb-servers
>  
> +install -p -D -m 0755 \
> +rhel/usr_share_openvswitch_scripts_ovs-reload \
> +$RPM_BUILD_ROOT%{_datadir}/openvswitch/scripts/ovs-reload
> +
>  # remove unpackaged files
>  rm -f $RPM_BUILD_ROOT%{_bindir}/ovs-parse-backtrace \
>  $RPM_BUILD_ROOT%{_sbindir}/ovs-vlan-bug-workaround \
> @@ -539,6 +543,7 @@ fi
>  %{_datadir}/openvswitch/scripts/ovs-save
>  %{_datadir}/openvswitch/scripts/ovs-vtep
>  %{_datadir}/openvswitch/scripts/ovs-ctl
> +%{_datadir}/openvswitch/scripts/ovs-reload
>  %config %{_datadir}/openvswitch/vswitch.ovsschema
>  %config %{_datadir}/openvswitch/vtep.ovsschema
>  %{_bindir}/ovs-appctl
> diff --git a/rhel/usr_lib_systemd_system_openvswitch.service 
> b/rhel/usr_lib_systemd_system_openvswitch.service
> index faca44b54..2cf29f0e9 100644
> --- a/rhel/usr_lib_systemd_system_openvswitch.service
> +++ b/rhel/usr_lib_systemd_system_openvswitch.service
> @@ -9,7 +9,7 @@ Requires=ovs-vswitchd.service
>  [Service]
>  Type=oneshot
>  ExecStart=/bin/true
> -ExecReload=/bin/true
> +ExecReload=/usr/share/openvswitch/scripts/ovs-reload
>  ExecStop=/bin/true
>  RemainAfterExit=yes
>  
> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
> b/rhel/usr_lib_systemd_system_ovsdb-server.service
> index 5baac822d..234d39355 100644
> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
> @@ -3,7 +3,6 @@ Description=Open vSwitch Database Unit
>  After=syslog.target network-pre.target
>  Before=network.target network.service
>  Wants=ovs-delete-transient-ports.service
> -ReloadPropagatedFrom=openvswitch.service
>  PartOf=openvswitch.service
>  
>  [Service]
> diff --git a/rhel/usr_share_openvswitch_scripts_ovs-reload 
> b/rhel/usr_share_openvswitch_scripts_ovs-reload
> new file mode 100755
> index 0..3ac1a46c6
> --- /dev/null
> +++ b/rhel/usr_share_openvswitch_scripts_ovs-reload
> @@ -0,0 +1,36 @@
> +#! /bin/sh
> +
> +# Copyright (c) 2017 Red Hat, Inc.
> +#
> +# Licensed under the Apache License, Version 2.0 (the "License");
> +# you may not use this file except in compliance with the License.
> +# You may obtain a copy of the License at:
> +#
> +# http://www.apache.org/licenses/LICENSE-2.0
> +#
> +# Unless required by applicable law or agreed to in writing, software
> +# distributed under the License is distributed on an "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> +# See the License for the specific language governing permissions and
> +# limitations under the License.
> +
> +# Save flows
> +bridges=$(ovs-vsctl -- --real list-br)
> +flows=$(/usr/share/openvswitch/scripts/ovs-save save-flows $bridges)
> +
> +# Restart the database first, since a large database may take a
> +# while to load, and we want to 

[ovs-dev] [PATCH] rhel: Add support for "systemctl reload openvswitch"

[Timothy Redaelli]

The reload procedure will trigger a script that saves the flows and tlv
maps (using ovs-save) 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).

Signed-off-by: Timothy Redaelli 
---
 rhel/automake.mk |  1 +
 rhel/openvswitch-fedora.spec.in  |  5 
 rhel/usr_lib_systemd_system_openvswitch.service  |  2 +-
 rhel/usr_lib_systemd_system_ovsdb-server.service |  1 -
 rhel/usr_share_openvswitch_scripts_ovs-reload| 36 
 5 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100755 rhel/usr_share_openvswitch_scripts_ovs-reload

diff --git a/rhel/automake.mk b/rhel/automake.mk
index 9336f0912..0955dceed 100644
--- a/rhel/automake.mk
+++ b/rhel/automake.mk
@@ -24,6 +24,7 @@ EXTRA_DIST += \
rhel/openvswitch.spec.in \
rhel/openvswitch-fedora.spec \
rhel/openvswitch-fedora.spec.in \
+   rhel/usr_share_openvswitch_scripts_ovs-reload \
rhel/usr_share_openvswitch_scripts_sysconfig.template \
rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
rhel/usr_lib_udev_rules.d_91-vfio.rules \
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index fb7d918c6..87bec39c9 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -314,6 +314,10 @@ install -d -m 0755 
$RPM_BUILD_ROOT%{_prefix}/lib/ocf/resource.d/ovn
 ln -s %{_datadir}/openvswitch/scripts/ovndb-servers.ocf \
   $RPM_BUILD_ROOT%{_prefix}/lib/ocf/resource.d/ovn/ovndb-servers
 
+install -p -D -m 0755 \
+rhel/usr_share_openvswitch_scripts_ovs-reload \
+$RPM_BUILD_ROOT%{_datadir}/openvswitch/scripts/ovs-reload
+
 # remove unpackaged files
 rm -f $RPM_BUILD_ROOT%{_bindir}/ovs-parse-backtrace \
 $RPM_BUILD_ROOT%{_sbindir}/ovs-vlan-bug-workaround \
@@ -539,6 +543,7 @@ fi
 %{_datadir}/openvswitch/scripts/ovs-save
 %{_datadir}/openvswitch/scripts/ovs-vtep
 %{_datadir}/openvswitch/scripts/ovs-ctl
+%{_datadir}/openvswitch/scripts/ovs-reload
 %config %{_datadir}/openvswitch/vswitch.ovsschema
 %config %{_datadir}/openvswitch/vtep.ovsschema
 %{_bindir}/ovs-appctl
diff --git a/rhel/usr_lib_systemd_system_openvswitch.service 
b/rhel/usr_lib_systemd_system_openvswitch.service
index faca44b54..2cf29f0e9 100644
--- a/rhel/usr_lib_systemd_system_openvswitch.service
+++ b/rhel/usr_lib_systemd_system_openvswitch.service
@@ -9,7 +9,7 @@ Requires=ovs-vswitchd.service
 [Service]
 Type=oneshot
 ExecStart=/bin/true
-ExecReload=/bin/true
+ExecReload=/usr/share/openvswitch/scripts/ovs-reload
 ExecStop=/bin/true
 RemainAfterExit=yes
 
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 5baac822d..234d39355 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -3,7 +3,6 @@ Description=Open vSwitch Database Unit
 After=syslog.target network-pre.target
 Before=network.target network.service
 Wants=ovs-delete-transient-ports.service
-ReloadPropagatedFrom=openvswitch.service
 PartOf=openvswitch.service
 
 [Service]
diff --git a/rhel/usr_share_openvswitch_scripts_ovs-reload 
b/rhel/usr_share_openvswitch_scripts_ovs-reload
new file mode 100755
index 0..3ac1a46c6
--- /dev/null
+++ b/rhel/usr_share_openvswitch_scripts_ovs-reload
@@ -0,0 +1,36 @@
+#! /bin/sh
+
+# Copyright (c) 2017 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Save flows
+bridges=$(ovs-vsctl -- --real list-br)
+flows=$(/usr/share/openvswitch/scripts/ovs-save save-flows $bridges)
+
+# Restart the database first, since a large database may take a
+# while to load, and we want to minimize forwarding disruption.
+systemctl --job-mode=ignore-dependencies restart ovsdb-server
+
+# Stop ovs-vswitchd.
+systemctl --job-mode=ignore-dependencies stop ovs-vswitchd
+
+# Start vswitchd by asking it to wait till flow restore is finished.
+ovs-vsctl --no-wait set open_vswitch . 

Re: [ovs-dev] [PATCH] rhel: Add support for "systemctl reload openvswitch"

[Markos Chandras]

On 08/08/2017 11:01 PM, Timothy M. Redaelli wrote:
> 
> 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.

For the record, SUSE uses the same approach

> [...]
> 
> 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?

The rest looks good to me. Thank you

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] rhel: Add support for "systemctl reload openvswitch"

[Timothy M. Redaelli]

On 08/08/2017 08:24 PM, Guru Shetty wrote:
> 
> 
> On 8 August 2017 at 11:05, Russell Bryant  > wrote:
> 
> On Tue, Aug 8, 2017 at 1:32 PM, Guru Shetty  > wrote:
> > On 8 August 2017 at 07:42, Timothy Redaelli  > 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

Re: [ovs-dev] [PATCH] rhel: Add support for "systemctl reload openvswitch"

[Guru Shetty]

On 8 August 2017 at 11:05, Russell Bryant  wrote:

> On Tue, Aug 8, 2017 at 1:32 PM, Guru Shetty  wrote:
> > On 8 August 2017 at 07:42, Timothy Redaelli 
> 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.
>
> 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.
>

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.

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?

>
> --
> Russell Bryant
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] rhel: Add support for "systemctl reload openvswitch"

[Timothy Redaelli]

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.

Signed-off-by: Timothy Redaelli 
---
 rhel/automake.mk |  1 +
 rhel/openvswitch-fedora.spec.in  |  5 ++
 rhel/usr_lib_systemd_system_openvswitch.service  |  2 +-
 rhel/usr_lib_systemd_system_ovsdb-server.service |  1 -
 rhel/usr_share_openvswitch_scripts_ovs-reload| 73 
 5 files changed, 80 insertions(+), 2 deletions(-)
 create mode 100755 rhel/usr_share_openvswitch_scripts_ovs-reload

diff --git a/rhel/automake.mk b/rhel/automake.mk
index 1265fa747..93dbeac0c 100644
--- a/rhel/automake.mk
+++ b/rhel/automake.mk
@@ -23,6 +23,7 @@ EXTRA_DIST += \
rhel/openvswitch.spec.in \
rhel/openvswitch-fedora.spec \
rhel/openvswitch-fedora.spec.in \
+   rhel/usr_share_openvswitch_scripts_ovs-reload \
rhel/usr_share_openvswitch_scripts_sysconfig.template \
rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
rhel/usr_lib_systemd_system_openvswitch.service \
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 1cad9406e..dc92ab3ca 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -289,6 +289,10 @@ install -d -m 0755 
$RPM_BUILD_ROOT%{_prefix}/lib/ocf/resource.d/ovn
 ln -s %{_datadir}/openvswitch/scripts/ovndb-servers.ocf \
   $RPM_BUILD_ROOT%{_prefix}/lib/ocf/resource.d/ovn/ovndb-servers
 
+install -p -D -m 0755 \
+rhel/usr_share_openvswitch_scripts_ovs-reload \
+$RPM_BUILD_ROOT%{_datadir}/openvswitch/scripts/ovs-reload
+
 # remove unpackaged files
 rm -f $RPM_BUILD_ROOT%{_bindir}/ovs-parse-backtrace \
 $RPM_BUILD_ROOT%{_sbindir}/ovs-vlan-bug-workaround \
@@ -493,6 +497,7 @@ fi
 %{_datadir}/openvswitch/scripts/ovs-save
 %{_datadir}/openvswitch/scripts/ovs-vtep
 %{_datadir}/openvswitch/scripts/ovs-ctl
+%{_datadir}/openvswitch/scripts/ovs-reload
 %config %{_datadir}/openvswitch/vswitch.ovsschema
 %config %{_datadir}/openvswitch/vtep.ovsschema
 %{_bindir}/ovs-appctl
diff --git a/rhel/usr_lib_systemd_system_openvswitch.service 
b/rhel/usr_lib_systemd_system_openvswitch.service
index faca44b54..2cf29f0e9 100644
--- a/rhel/usr_lib_systemd_system_openvswitch.service
+++ b/rhel/usr_lib_systemd_system_openvswitch.service
@@ -9,7 +9,7 @@ Requires=ovs-vswitchd.service
 [Service]
 Type=oneshot
 ExecStart=/bin/true
-ExecReload=/bin/true
+ExecReload=/usr/share/openvswitch/scripts/ovs-reload
 ExecStop=/bin/true
 RemainAfterExit=yes
 
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 68deace7c..b9814bae1 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -2,7 +2,6 @@
 Description=Open vSwitch Database Unit
 After=syslog.target network-pre.target
 Before=network.target network.service
-ReloadPropagatedFrom=openvswitch.service
 PartOf=openvswitch.service
 
 [Service]
diff --git a/rhel/usr_share_openvswitch_scripts_ovs-reload 
b/rhel/usr_share_openvswitch_scripts_ovs-reload
new file mode 100755
index 0..793257390
--- /dev/null
+++ b/rhel/usr_share_openvswitch_scripts_ovs-reload
@@ -0,0 +1,73 @@
+#! /bin/sh
+
+# Copyright (c) 2017 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Highest OpenFlow version enabled by default
+DEFAULT_OFP_VERSION=OpenFlow14
+
+get_highest_ofp_version() {
+ovs-vsctl get bridge "$1" protocols | \
+awk -v default_ofp_version="$DEFAULT_OFP_VERSION" \
+-F '"' '{ print (NF>1)? $(NF-1) : default_ofp_version }'
+}
+
+save_flows () {
+for bridge; do
+# Get the highest enabled OpenFlow version
+ofp_version=$(get_highest_ofp_version "$bridge")
+
+printf "ovs-ofctl -O $ofp_version add-tlv-map %s '"