Re: [PATCH v2] net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)

[Arvid Brodin <arvid.brodin@xdin.com>]

On 2013-07-02 01:48, Arvid Brodin wrote:
> On 2013-06-29 06:16, David Miller wrote:
>> From: Arvid Brodin <arvid.brodin@xdin.com>
>> Date: Wed, 26 Jun 2013 17:16:11 +0200
>>
>>> +static int hsr_dev_open(struct net_device *dev)
>>> +{
>>> +    struct hsr_priv *hsr_priv;
>>> +
>>> +    hsr_priv = netdev_priv(dev);
>>> +
>>> +    if (hsr_priv->slave[0])
>>> +        dev_open(hsr_priv->slave[0]);
>>> +    if (hsr_priv->slave[1])
>>> +        dev_open(hsr_priv->slave[1]);
>>> +
>>> +    return 0;
>>> +}
>>
>> dev_open() can and does fail, you must thus check for error returns, undo any
>> necessary state, and propagate that error to callers of hsr_dev_open.
>>
>
> I'm not sure it's an error (from the HSR interface's point of view) if the slave(s)
> won't come up here. The calls to dev_open() can be seen more like a convenience
> than a necessity (I actually left them out to begin with). If none of the slaves
> could go up the HSR interface would end up as admin UP but with operstate
> IF_OPER_LOWERLAYERDOWN until any of the slaves went up.
[snip]


Stephen Hemminger explained to me (in private email) that many of the errors
from dev_open() aren't soft errors - i.e. they should be propagated back up
even if the HSR driver doesn't care (out of memory, and hardware dead being
examples).

But HSR is often used in safety critical systems, and it may be important that
a hsr_dev->dev_open() does not fail just because one of the slave's cable
happens to be unplugged (or cut) - redundancy being the whole point of HSR.

I think the only reasonable thing to do is not to call dev_open() at all here,
and instead just inform the user if a slave is not already up. This way user
space must explicitly set both slaves UP and thus must also decide how to treat
different kinds of "slave errors".

I will send a new patch with this change.


Thanks to Stephen for helping me with this!


-- 
Arvid Brodin | Consultant (Linux)
XDIN AB | Knarrarnäsgatan 7 | SE-164 40 Kista | Sweden | xdin.com