Thu, Dec 03, 2015 at 01:20:50AM IST, john.fastab...@gmail.com wrote:
>On 15-12-02 12:07 PM, Jiri Pirko wrote:
>> From: Ido Schimmel <ido...@mellanox.com>
>>
>> switchdev drivers reflect the newly requested topology to hardware when
>> CHANGEUPPER is received, after software links were already formed.
>> However, the operation can fail and user will not be notified, as the
>> return value of the notifier is not checked.
>>
>> Add this check and rollback software links if necessary.
>>
>> Signed-off-by: Ido Schimmel <ido...@mellanox.com>
>> Signed-off-by: Jiri Pirko <j...@mellanox.com>
>> ---
>> v1->v2:
>> -new patch
>> ---
>> net/core/dev.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 5df6cbc..df33f82 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -5490,8 +5490,11 @@ static int __netdev_upper_dev_link(struct net_device
>> *dev,
>> goto rollback_lower_mesh;
>> }
>>
>> - call_netdevice_notifiers_info(NETDEV_CHANGEUPPER, dev,
>> - &changeupper_info.info);
>> + ret = call_netdevice_notifiers_info(NETDEV_CHANGEUPPER, dev,
>> + &changeupper_info.info);
>> + if (ret)
>> + goto rollback_lower_mesh;
>> +
>
>hmm small nit (take it or leave it) but I think it would be more
>correct if this was
>
> if (ret == NOTIFY_BAD)
> goto rollback_lower_mesh;
>
>It seems that NOTIFY_DONE, NOTIFY_OK and NOTIFY_STOP_MASK would be
>valid return codes that don't indicate an error. However seeing I
>couldn't find any cases of NOTIFY_OK/NOTIFY_STOP_MASK from the
>CHANGEUPPER event it doesn't matter in practice.
>
Hi,
It's actually not a small nit. I forgot to call 'notifier_to_errno' and
then check its retrun value instead that of the notifier. This would
return 0 for NOTIFY_{DONE,OK,STOP_MASK} and -1 for NOTIFY_BAD.
Thank you.
>Thanks,
>John
>
>> return 0;
>>
>> rollback_lower_mesh:
>>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
