* [PATCH net-next 1/4] bonding: update the primary when slave name changed @ 2014-01-09 11:20 Ding Tianhong 2014-01-09 11:46 ` Veaceslav Falico 0 siblings, 1 reply; 9+ messages in thread From: Ding Tianhong @ 2014-01-09 11:20 UTC (permalink / raw) To: Jay Vosburgh, Veaceslav Falico, David S. Miller, Netdev 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. 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/4] bonding: update the primary when slave name changed 2014-01-09 11:20 [PATCH net-next 1/4] bonding: update the primary when slave name changed Ding Tianhong @ 2014-01-09 11:46 ` Veaceslav Falico 2014-01-09 12:23 ` Ding Tianhong 0 siblings, 1 reply; 9+ messages in thread From: Veaceslav Falico @ 2014-01-09 11:46 UTC (permalink / raw) To: Ding Tianhong; +Cc: Jay Vosburgh, David S. Miller, Netdev 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. > >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 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/4] bonding: update the primary when slave name changed 2014-01-09 11:46 ` Veaceslav Falico @ 2014-01-09 12:23 ` Ding Tianhong 2014-01-09 12:30 ` Veaceslav Falico 0 siblings, 1 reply; 9+ messages in thread From: Ding Tianhong @ 2014-01-09 12:23 UTC (permalink / raw) To: Veaceslav Falico; +Cc: Jay Vosburgh, David S. Miller, Netdev 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? 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 >> >> > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/4] bonding: update the primary when slave name changed 2014-01-09 12:23 ` Ding Tianhong @ 2014-01-09 12:30 ` Veaceslav Falico 2014-01-10 4:20 ` Ding Tianhong 0 siblings, 1 reply; 9+ messages in thread From: Veaceslav Falico @ 2014-01-09 12:30 UTC (permalink / raw) To: Ding Tianhong; +Cc: Jay Vosburgh, David S. Miller, Netdev 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. > >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 >>> >>> >> >> > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/4] bonding: update the primary when slave name changed 2014-01-09 12:30 ` Veaceslav Falico @ 2014-01-10 4:20 ` Ding Tianhong 2014-01-10 7:44 ` Veaceslav Falico 0 siblings, 1 reply; 9+ messages in thread From: Ding Tianhong @ 2014-01-10 4:20 UTC (permalink / raw) To: Veaceslav Falico; +Cc: Jay Vosburgh, David S. Miller, Netdev 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. 2. check if the primary is not the primary_slave, make the primary_slave = NULL, this means the primary_slave is no valid. 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 >>>> >>>> >>> >>> >> >> > > . > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/4] bonding: update the primary when slave name changed 2014-01-10 4:20 ` Ding Tianhong @ 2014-01-10 7:44 ` Veaceslav Falico 2014-01-10 11:05 ` Ding Tianhong 0 siblings, 1 reply; 9+ messages in thread From: Veaceslav Falico @ 2014-01-10 7:44 UTC (permalink / raw) To: Ding Tianhong; +Cc: Jay Vosburgh, David S. Miller, Netdev 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 >>>>> >>>>> >>>> >>>> >>> >>> >> >> . >> > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/4] bonding: update the primary when slave name changed 2014-01-10 7:44 ` Veaceslav Falico @ 2014-01-10 11:05 ` Ding Tianhong 2014-01-10 11:11 ` Veaceslav Falico 0 siblings, 1 reply; 9+ messages in thread From: Ding Tianhong @ 2014-01-10 11:05 UTC (permalink / raw) To: Veaceslav Falico; +Cc: Jay Vosburgh, David S. Miller, Netdev On 2014/1/10 15:44, Veaceslav Falico wrote: > 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. Ok, agree. > 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. I don't think so , I think if it is not the primary slave and we don't have one, no need to do anything, just a normal slave change its name. Regards Ding > >> 3. ?? did you have any better ideas? >> >> Regards >> Ding >> >>>> >>> >>> . >>> >> >> > > . > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/4] bonding: update the primary when slave name changed 2014-01-10 11:05 ` Ding Tianhong @ 2014-01-10 11:11 ` Veaceslav Falico 2014-01-10 11:55 ` Ding Tianhong 0 siblings, 1 reply; 9+ messages in thread From: Veaceslav Falico @ 2014-01-10 11:11 UTC (permalink / raw) To: Ding Tianhong; +Cc: Jay Vosburgh, David S. Miller, Netdev On Fri, Jan 10, 2014 at 07:05:34PM +0800, Ding Tianhong wrote: >On 2014/1/10 15:44, Veaceslav Falico wrote: >> 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. > >I don't think so , I think if it is not the primary slave and we don't have one, >no need to do anything, just a normal slave change its name. If primary == "my_eth0", you have 2 slaves - "eth0" and "eth1", thus null primary_slave, and rename eth0 to my_eth0 - then you need to set primary_slave to my_eth0. If primary == "my_eth0", you have 2 slaves - "my_eth0" and "eth1", thus primary_slave == dev with name "my_eth0", and rename "my_eth0" to "eth0" - then you must set primary_slave to NULL. And after either of these you must see if you need to re-select the active slave, as it might have been forced by the primary_slave, which has been modified. You might also want to add some pr_info() about adding/removing primary_slave, as the user to be aware. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/4] bonding: update the primary when slave name changed 2014-01-10 11:11 ` Veaceslav Falico @ 2014-01-10 11:55 ` Ding Tianhong 0 siblings, 0 replies; 9+ messages in thread From: Ding Tianhong @ 2014-01-10 11:55 UTC (permalink / raw) To: Veaceslav Falico; +Cc: Jay Vosburgh, David S. Miller, Netdev On 2014/1/10 19:11, Veaceslav Falico wrote: > On Fri, Jan 10, 2014 at 07:05:34PM +0800, Ding Tianhong wrote: >> On 2014/1/10 15:44, Veaceslav Falico wrote: >>> 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. >> >> I don't think so , I think if it is not the primary slave and we don't have one, >> no need to do anything, just a normal slave change its name. > > If primary == "my_eth0", you have 2 slaves - "eth0" and "eth1", thus null > primary_slave, and rename eth0 to my_eth0 - then you need to set > primary_slave to my_eth0. > > If primary == "my_eth0", you have 2 slaves - "my_eth0" and "eth1", thus > primary_slave == dev with name "my_eth0", and rename "my_eth0" to "eth0" - > then you must set primary_slave to NULL. > > And after either of these you must see if you need to re-select the active > slave, as it might have been forced by the primary_slave, which has been > modified. > > You might also want to add some pr_info() about adding/removing > primary_slave, as the user to be aware. > Ok thanks. Regards Ding > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-01-10 11:56 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-01-09 11:20 [PATCH net-next 1/4] bonding: update the primary when slave name changed Ding Tianhong 2014-01-09 11:46 ` Veaceslav Falico 2014-01-09 12:23 ` Ding Tianhong 2014-01-09 12:30 ` Veaceslav Falico 2014-01-10 4:20 ` Ding Tianhong 2014-01-10 7:44 ` Veaceslav Falico 2014-01-10 11:05 ` Ding Tianhong 2014-01-10 11:11 ` Veaceslav Falico 2014-01-10 11:55 ` Ding Tianhong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).