Re: [PATCH] bonding: prevent deadlock on slave store with alb mode

["Nicolas de Pesloüan" <nicolas.2p.debian@gmail.com>]

Le 24/05/2011 22:51, Nicolas de Pesloüan a écrit :
> Le 24/05/2011 22:37, Neil Horman a écrit :
>
>>>>> + return -EINVAL;
>>>
>>> This will turn a warning into an error.
>>>
>> Yes, because it should have been an error all along.
>>
>>> This warning existed for long, but never caused the bonding setup to
>>> fail. This patch cause some regression for user space. For example,
>>> current ifenslave-2.6 package in Debian doesn't ensure bond is UP
>>> before enslaving, because this was never required.
>>>
>> Thats not a regression, thats the kernel returning an error where it should have
>> done so all along. Just because a utility got away with it for awhile and it
>> didn't always cause a lockup, doesn't grandfather that application in to a
>> situation where the kernel has to support its broken behavior in perpituity.
>>
>> Besides, iirc, the ifsenslave utility still uses the ioctl path, which this
>> patch doesn't touch, so ifenslave is currently unaffected (although I should
>> look in the ioctl path to see if we have already added such a check, lest you be
>> able to deadlock your system as previously indicated using that tool).
>
> Unfortunately, no. Recent versions of ifenslave-2.6 on Debian don't use ioctl (ifenslave binary)
> anymore, but only sysfs.
>
> Documentation/bonding.txt should be updated to reflect this change.
> pr_warning should be changed to pr_ err.
> Bonding version should be bumped.
>
> Anyway, I will fix this package, but I suspect there exist many user scripts that don't ensure bond
> is up before enslaving.

Well, still thinking about it...

On Ubuntu, due to the usage of upstart, the slaves are ifup before the master. On Debian, this is 
also true for hotplug slaves. I wonder whether this patch may cause troubles, because the master 
will have to be up before calling ifup for it.

	Nicolas.