[ofa-general] Re: [PATCH v2] ipoib: fix a deadlock between ipoib start/stop and child interface create/delete

Yossi Etigin yosefe at Voltaire.COM
Wed Jan 14 11:04:39 PST 2009


Roland Dreier wrote:
> I think this almost works, but:
> 
>  > +	list_for_each_entry(cpriv, &priv->child_intfs, list) {
>  > +		flags = cpriv->dev->flags;
>  > +		new_flags = (flags & ~IFF_UP) | iffup_value;
>  > +		if (flags != new_flags) {
>  > +			rtnl_lock();
>  > +			dev_change_flags(cpriv->dev, new_flags);
>  > +			rtnl_unlock();
>  > +		}
>  > +	}
> 
> taking flags outside of the rtnl lock looks dubious to me, since it
> could change before we get to the dev_change_flags() call.

Yes, you are right. All bitwise stuff should be inside the lock.

> 
> Looking at all this old code, I have to wonder whether anyone is
> depending on bringing up the main interface also bringing up all the
> subinterfaces ... the simplest solution would be to let the
> subinterfaces be independent.  Is there anything wrong with just
> deleting the code to bring subinterfaces up/down?
> 

Deleting the code would be the simplest thing to do, but on the other
hand the fact the the subinterfaces are up when the master is brought up
is a behaviour that the users/userspace scripts got used to so I didn't
want to change that.
Vlad, what do you think? How will openibd script work with this?




More information about the general mailing list