Re: [PATCH net-next 1/4] bonding: update the primary when slave name changed

[Veaceslav Falico <vfalico@redhat.com>]

On Fri, Jan 10, 2014 at 12:20:45PM +0800, Ding Tianhong wrote:
>On 2014/1/9 20:30, Veaceslav Falico wrote:
>> On Thu, Jan 09, 2014 at 08:23:58PM +0800, Ding Tianhong wrote:
>>> On 2014/1/9 19:46, Veaceslav Falico wrote:
>>>> On Thu, Jan 09, 2014 at 07:20:36PM +0800, Ding Tianhong wrote:
>>>>> If the primary_slave's name changed, but the bond->prams.primay was
>>>>> still using the old name, it is conflict with the meaning of the
>>>>> primary, so update the primary when the slave change its name.
>>>>
>>>> Nope, the bonding parameter, which is set by the user, shouldn't change
>>>> because of an interface name change.
>>>>
>>> Yes, I know what you mean, but it is not bug fix, just make it more better,
>>> do not you feel it strange that the primary was different with primary_slave's name?
>>
>> Yep, that's an issue - that's why there is the TODO. We shouldn't, though,
>> change the primary param, but rather check if the slave (that changed name)
>> is (already not) eligible for primary_slave.
>>
>
>Ok，So，summarize your and my opinion, I think there are two ways to fix this:
>
>1. just like my patch said.

No, the primary string is user-set, and it should *not* be changed by
kernel.

>2. check if the primary is not the primary_slave, make the primary_slave = NULL, this means
>   the primary_slave is no valid.

Check the slave that changed name - if it's the primary slave, remove it,
and see if we need to select the new active slave. If it's not the primray
slave, and we don't have one - select it as a new primary and, again, see
if we need to select a new active slave.

>3. ?? did you have any better ideas?
>
>Regards
>Ding
>
>>>
>>> Regards
>>> Ding
>>>
>>>
>>>>>
>>>>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>>>>> ---
>>>>> drivers/net/bonding/bond_main.c | 14 ++++++++++++--
>>>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>>>> index e06c445..de646e2 100644
>>>>> --- a/drivers/net/bonding/bond_main.c
>>>>> +++ b/drivers/net/bonding/bond_main.c
>>>>> @@ -2860,9 +2860,19 @@ static int bond_slave_netdev_event(unsigned long event,
>>>>>          */
>>>>>         break;
>>>>>     case NETDEV_CHANGENAME:
>>>>> -        /*
>>>>> -         * TODO: handle changing the primary's name
>>>>> +        /* if the primary's name changed,
>>>>> +         * save the new name for primary.
>>>>>          */
>>>>> +        if (USES_PRIMARY(bond->params.mode) &&
>>>>> +            bond->params.primary[0]) {
>>>>> +            if (bond->primary_slave &&
>>>>> +                strcmp(bond->params.primary,
>>>>> +                   bond->primary_slave->dev->name)) {
>>>>> +                strncpy(bond->params.primary,
>>>>> +                    bond->primary_slave->dev->name,
>>>>> +                    IFNAMSIZ);
>>>>> +            }
>>>>> +        }
>>>>>         break;
>>>>>     case NETDEV_FEAT_CHANGE:
>>>>>         bond_compute_features(bond);
>>>>> --
>>>>> 1.8.0
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>> .
>>
>
>