[ovs-dev] [PATCH 1/3] ovs-ctl: Unload & reload vport modules on force-reload-kmod

[Flavio Leitner]

On Tue,  7 Apr 2015 12:43:41 +0200
Thomas Graf <tgraf at noironetworks.com> wrote:

> We manually rmmod the loaded vports as using modprobe -r
> only works if the modules are available through modules.dep
> 
> We do not treat failures to load vports as a fatal error in case
> the vport module has been renamed. Bringing the bridge back up is
> considered more important. The error is still reported though.
> 
> Reported-by: Pravin Shelar <pshelar at nicira.com>
> Signed-off-by: Thomas Graf <tgraf at noironetworks.com>
> ---
> v2:
>  - Try to load previously unloaded vports as suggested by Pravin
> 
>  utilities/ovs-ctl.in | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
> index 6d2e938..413dd37 100755
> --- a/utilities/ovs-ctl.in
> +++ b/utilities/ovs-ctl.in
> @@ -30,6 +30,19 @@ done
>  ## start ##
>  ## ----- ##
>  
> +# Keep track of removed vports so we can reload them if needed
> +removed_vports=""
> +
> +insert_mods () {
> +    # Try loading openvswitch again.
> +    action "Inserting openvswitch module" modprobe openvswitch
> +
> +    for vport in $removed_vports; do
> +        # Don't treat failures to load vports as fatal error
> +        action "Inserting $vport module" modprobe $vport || true
> +    done
> +}
> +
>  insert_mod_if_required () {
>      # If this kernel has no module support, expect we're done.
>      if test ! -e /proc/modules
> @@ -43,7 +56,7 @@ insert_mod_if_required () {
>       return 0
>  
>      # Load openvswitch.  If that's successful then we're done.
> -    action "Inserting openvswitch module" modprobe openvswitch &&
> return 0
> +    insert_mods && return 0
>  
>      # If the bridge module is loaded, then that might be blocking
>      # openvswitch.  Try to unload it, if there are no bridges.
> @@ -56,7 +69,7 @@ insert_mod_if_required () {
>      action "removing bridge module" rmmod bridge || return 1
>  
>      # Try loading openvswitch again.
> -    action "Inserting openvswitch module" modprobe openvswitch
> +    insert_mods
>  }
>  
>  ovs_vsctl () {
> @@ -388,6 +401,11 @@ force_reload_kmod () {
>          action "Removing datapath: $dp" ovs-dpctl del-dp "$dp"
>      done
>  
> +    for vport in `lsmod | grep ^vport_ | awk '{print $1;}'`; do
> +        removed_vports="$removed_vports $vport"
> +        action "Removing $vport module" rmmod $vport
> +    done

Minor nit: The action() returns the error code, so maybe it would be
good to check if that actually succeed before add to the removed list.
Otherwise if the module is busy for some reason, two errors will happen.

Or perhaps:
for vport in `awk '/^vport_/ { print $1 }' /proc/modules`; do
	action "Removing $vport module" rmmod $vport
	if ! grep -q $vport /proc/modules; then
		removed_vports="$removed_vports $vport"
	fi
done

fbl
> +
>      # try both old and new names in case this is post upgrade
>      if test -e /sys/module/openvswitch_mod; then
>          action "Removing openvswitch module" rmmod openvswitch_mod