Ethan, will you review this?  It fixes a crash.

On Mon, Nov 25, 2013 at 02:14:33PM -0800, Ben Pfaff wrote:
> If one configured a controller which does not exist, waited for the switch
> to enter fail-open mode, and then deleted the controller, then
> ofproto_set_controllers() would take ofproto_mutex and call
> update_fail_open(), which would call fail_open_destroy(), which would call
> fail_open_recover(), which would call ofproto_delete_flow(), which requires
> ofproto_mutex not to be held since it eventually try to take it.  This
> caused OVS to abort.
> 
> This fixes the problem by releasing ofproto_mutex earlier, since nothing
> seems to require it being held so long (a comment in
> connmgr_set_controllers() says that this is likely to be the case).
> 
> Better annotations would have found this problem at compile time.  A later
> patch adds them.
> 
> Reported-by: ZhengLingyun <konghuaru...@163.com>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
>  ofproto/connmgr.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
> index da25930..a062772 100644
> --- a/ofproto/connmgr.c
> +++ b/ofproto/connmgr.c
> @@ -637,12 +637,13 @@ connmgr_set_controllers(struct connmgr *mgr,
>  
>      shash_destroy(&new_controllers);
>  
> +    ovs_mutex_unlock(&ofproto_mutex);
> +
>      update_in_band_remotes(mgr);
>      update_fail_open(mgr);
>      if (had_controllers != connmgr_has_controllers(mgr)) {
>          ofproto_flush_flows(mgr->ofproto);
>      }
> -    ovs_mutex_unlock(&ofproto_mutex);
>  }
>  
>  /* Drops the connections between 'mgr' and all of its primary and secondary
> -- 
> 1.7.10.4
> 
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to