| Message ID | 1389700179-12723-1-git-send-email-vfalico@redhat.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
Veaceslav Falico <vfalico@redhat.com> wrote: >Currently, if a slave's name change, we just pass it by. However, if the >slave is a current primary_slave, then we end up with using a slave, whose >name != params.primary, for primary_slave. And vice-versa, if we don't have >a primary_slave but have params.primary set - we will not detected a new >primary_slave. > >Fix this by catching the NETDEV_CHANGENAME event and setting primary_slave >accordingly. Also, if the primary_slave was changed, issue a reselection of >the active slave, cause the priorities have changed. > >Reported-by: Ding Tianhong <dingtianhong@huawei.com> >CC: Ding Tianhong <dingtianhong@huawei.com> >CC: Jay Vosburgh <fubar@us.ibm.com> >CC: Andy Gospodarek <andy@greyhouse.net> >Signed-off-by: Veaceslav Falico <vfalico@redhat.com> >--- > drivers/net/bonding/bond_main.c | 23 ++++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) > >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >index e06c445..8077199 100644 >--- a/drivers/net/bonding/bond_main.c >+++ b/drivers/net/bonding/bond_main.c >@@ -2860,9 +2860,26 @@ static int bond_slave_netdev_event(unsigned long event, > */ > break; > case NETDEV_CHANGENAME: >- /* >- * TODO: handle changing the primary's name >- */ >+ /* we don't care if we don't have primary set */ >+ if (!USES_PRIMARY(bond->params.mode) || >+ !bond->params.primary[0]) >+ break; >+ >+ if (slave == bond->primary_slave) { >+ /* slave's name changed - he's no longer primary */ >+ bond->primary_slave = NULL; >+ } else if (!strcmp(slave_dev->name, bond->params.primary)) { >+ /* we have a new primary slave */ >+ bond->primary_slave = slave; >+ } else /* we didn't change primary - exit */ >+ break; >+ >+ pr_info("%s: Primary slave changed to %s, re-electing.\n", I suspect you mean "reselecting" here, not "re-electing." I'd add a couple more words, e.g., "reselecting active slave" to make it clearer. -J >+ bond->dev->name, bond->primary_slave ? slave_dev->name : >+ "none"); >+ write_lock_bh(&bond->curr_slave_lock); >+ bond_select_active_slave(bond); >+ write_unlock_bh(&bond->curr_slave_lock); > break; > case NETDEV_FEAT_CHANGE: > bond_compute_features(bond); >-- >1.8.4 > --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 14, 2014 at 09:32:59AM -0800, Jay Vosburgh wrote: >Veaceslav Falico <vfalico@redhat.com> wrote: > >>Currently, if a slave's name change, we just pass it by. However, if the >>slave is a current primary_slave, then we end up with using a slave, whose >>name != params.primary, for primary_slave. And vice-versa, if we don't have >>a primary_slave but have params.primary set - we will not detected a new >>primary_slave. >> >>Fix this by catching the NETDEV_CHANGENAME event and setting primary_slave >>accordingly. Also, if the primary_slave was changed, issue a reselection of >>the active slave, cause the priorities have changed. >> >>Reported-by: Ding Tianhong <dingtianhong@huawei.com> >>CC: Ding Tianhong <dingtianhong@huawei.com> >>CC: Jay Vosburgh <fubar@us.ibm.com> >>CC: Andy Gospodarek <andy@greyhouse.net> >>Signed-off-by: Veaceslav Falico <vfalico@redhat.com> >>--- >> drivers/net/bonding/bond_main.c | 23 ++++++++++++++++++++--- >> 1 file changed, 20 insertions(+), 3 deletions(-) >> >>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >>index e06c445..8077199 100644 >>--- a/drivers/net/bonding/bond_main.c >>+++ b/drivers/net/bonding/bond_main.c >>@@ -2860,9 +2860,26 @@ static int bond_slave_netdev_event(unsigned long event, >> */ >> break; >> case NETDEV_CHANGENAME: >>- /* >>- * TODO: handle changing the primary's name >>- */ >>+ /* we don't care if we don't have primary set */ >>+ if (!USES_PRIMARY(bond->params.mode) || >>+ !bond->params.primary[0]) >>+ break; >>+ >>+ if (slave == bond->primary_slave) { >>+ /* slave's name changed - he's no longer primary */ >>+ bond->primary_slave = NULL; >>+ } else if (!strcmp(slave_dev->name, bond->params.primary)) { >>+ /* we have a new primary slave */ >>+ bond->primary_slave = slave; >>+ } else /* we didn't change primary - exit */ >>+ break; >>+ >>+ pr_info("%s: Primary slave changed to %s, re-electing.\n", > > I suspect you mean "reselecting" here, not "re-electing." I'd >add a couple more words, e.g., "reselecting active slave" to make it >clearer. Yep, sure, will reword and send v3. Thank you! > > -J > >>+ bond->dev->name, bond->primary_slave ? slave_dev->name : >>+ "none"); >>+ write_lock_bh(&bond->curr_slave_lock); >>+ bond_select_active_slave(bond); >>+ write_unlock_bh(&bond->curr_slave_lock); >> break; >> case NETDEV_FEAT_CHANGE: >> bond_compute_features(bond); >>-- >>1.8.4 >> > >--- > -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index e06c445..8077199 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2860,9 +2860,26 @@ static int bond_slave_netdev_event(unsigned long event, */ break; case NETDEV_CHANGENAME: - /* - * TODO: handle changing the primary's name - */ + /* we don't care if we don't have primary set */ + if (!USES_PRIMARY(bond->params.mode) || + !bond->params.primary[0]) + break; + + if (slave == bond->primary_slave) { + /* slave's name changed - he's no longer primary */ + bond->primary_slave = NULL; + } else if (!strcmp(slave_dev->name, bond->params.primary)) { + /* we have a new primary slave */ + bond->primary_slave = slave; + } else /* we didn't change primary - exit */ + break; + + pr_info("%s: Primary slave changed to %s, re-electing.\n", + bond->dev->name, bond->primary_slave ? slave_dev->name : + "none"); + write_lock_bh(&bond->curr_slave_lock); + bond_select_active_slave(bond); + write_unlock_bh(&bond->curr_slave_lock); break; case NETDEV_FEAT_CHANGE: bond_compute_features(bond);
Currently, if a slave's name change, we just pass it by. However, if the slave is a current primary_slave, then we end up with using a slave, whose name != params.primary, for primary_slave. And vice-versa, if we don't have a primary_slave but have params.primary set - we will not detected a new primary_slave. Fix this by catching the NETDEV_CHANGENAME event and setting primary_slave accordingly. Also, if the primary_slave was changed, issue a reselection of the active slave, cause the priorities have changed. Reported-by: Ding Tianhong <dingtianhong@huawei.com> CC: Ding Tianhong <dingtianhong@huawei.com> CC: Jay Vosburgh <fubar@us.ibm.com> CC: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Veaceslav Falico <vfalico@redhat.com> --- drivers/net/bonding/bond_main.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)