| Message ID | 5285F130.5060705@huawei.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Ding Tianhong <dingtianhong@huawei.com> wrote: >The primary_reselect only reselection for the primary slave, >but the primary slave only support for ALB, TLB and AB mode, >so we sould set the primary_reselect for these mode. > >to fix this: Add a check for ALB, TLB and AB mode in >bonding_store_primary_reselect, avoid to select active slave >again in other modes. I don't believe that setting primary_reselect in a !USES_PRIMARY() mode has any negative effects. It doesn't do anything, but also doesn't break anything. Is there a case that setting primary_reselect causes misbehavior in a !USES_PRIMARY() mode? Presuming that primary_reselect doesn't break things, this just adds an ordering limitation when configuring bonding (mode must be set prior to primary_reselect). I don't believe this change adds any value, and may break existing configuration scripts. -J >Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> >--- > drivers/net/bonding/bond_sysfs.c | 7 +++++++ > 1 file changed, 7 insertions(+) > >diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c >index 47749c9..cac2291 100644 >--- a/drivers/net/bonding/bond_sysfs.c >+++ b/drivers/net/bonding/bond_sysfs.c >@@ -1141,6 +1141,13 @@ static ssize_t bonding_store_primary_reselect(struct device *d, > if (!rtnl_trylock()) > return restart_syscall(); > >+ if (!USES_PRIMARY(bond->params.mode)) { >+ pr_info("%s: Unable to set primary reselect; %s is in mode %d\n", >+ bond->dev->name, bond->dev->name, bond->params.mode); >+ ret = -EINVAL; >+ goto out; >+ } >+ > new_value = bond_parse_parm(buf, pri_reselect_tbl); > if (new_value < 0) { > pr_err("%s: Ignoring invalid primary_reselect value %.*s.\n", >-- >1.7.12 --- -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
于 2013/11/16 2:02, Jay Vosburgh 写道: > Ding Tianhong <dingtianhong@huawei.com> wrote: > >> The primary_reselect only reselection for the primary slave, >> but the primary slave only support for ALB, TLB and AB mode, >> so we sould set the primary_reselect for these mode. >> >> to fix this: Add a check for ALB, TLB and AB mode in >> bonding_store_primary_reselect, avoid to select active slave >> again in other modes. > I don't believe that setting primary_reselect in a > !USES_PRIMARY() mode has any negative effects. It doesn't do anything, > but also doesn't break anything. Is there a case that setting > primary_reselect causes misbehavior in a !USES_PRIMARY() mode? I see it will call bond_select_active_slave and I will think more about it, if I could not find any problem about it, will miss this patch. thanks. Regards Ding > Presuming that primary_reselect doesn't break things, this just > adds an ordering limitation when configuring bonding (mode must be set > prior to primary_reselect). I don't believe this change adds any value, > and may break existing configuration scripts. > > -J > >> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> >> --- >> drivers/net/bonding/bond_sysfs.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c >> index 47749c9..cac2291 100644 >> --- a/drivers/net/bonding/bond_sysfs.c >> +++ b/drivers/net/bonding/bond_sysfs.c >> @@ -1141,6 +1141,13 @@ static ssize_t bonding_store_primary_reselect(struct device *d, >> if (!rtnl_trylock()) >> return restart_syscall(); >> >> + if (!USES_PRIMARY(bond->params.mode)) { >> + pr_info("%s: Unable to set primary reselect; %s is in mode %d\n", >> + bond->dev->name, bond->dev->name, bond->params.mode); >> + ret = -EINVAL; >> + goto out; >> + } >> + >> new_value = bond_parse_parm(buf, pri_reselect_tbl); >> if (new_value < 0) { >> pr_err("%s: Ignoring invalid primary_reselect value %.*s.\n", >> -- >> 1.7.12 > --- > -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 > -- 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
于 2013/11/16 2:02, Jay Vosburgh 写道: > Ding Tianhong <dingtianhong@huawei.com> wrote: > >> The primary_reselect only reselection for the primary slave, >> but the primary slave only support for ALB, TLB and AB mode, >> so we sould set the primary_reselect for these mode. >> >> to fix this: Add a check for ALB, TLB and AB mode in >> bonding_store_primary_reselect, avoid to select active slave >> again in other modes. > I don't believe that setting primary_reselect in a > !USES_PRIMARY() mode has any negative effects. It doesn't do anything, > but also doesn't break anything. Is there a case that setting > primary_reselect causes misbehavior in a !USES_PRIMARY() mode? > > Presuming that primary_reselect doesn't break things, this just > adds an ordering limitation when configuring bonding (mode must be set > prior to primary_reselect). I don't believe this change adds any value, > and may break existing configuration scripts. > > -J agree, I could not find any problem here, it is not bugfix, so miss it. Regards Ding >> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> >> --- >> drivers/net/bonding/bond_sysfs.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c >> index 47749c9..cac2291 100644 >> --- a/drivers/net/bonding/bond_sysfs.c >> +++ b/drivers/net/bonding/bond_sysfs.c >> @@ -1141,6 +1141,13 @@ static ssize_t bonding_store_primary_reselect(struct device *d, >> if (!rtnl_trylock()) >> return restart_syscall(); >> >> + if (!USES_PRIMARY(bond->params.mode)) { >> + pr_info("%s: Unable to set primary reselect; %s is in mode %d\n", >> + bond->dev->name, bond->dev->name, bond->params.mode); >> + ret = -EINVAL; >> + goto out; >> + } >> + >> new_value = bond_parse_parm(buf, pri_reselect_tbl); >> if (new_value < 0) { >> pr_err("%s: Ignoring invalid primary_reselect value %.*s.\n", >> -- >> 1.7.12 > --- > -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 > -- 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_sysfs.c b/drivers/net/bonding/bond_sysfs.c index 47749c9..cac2291 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -1141,6 +1141,13 @@ static ssize_t bonding_store_primary_reselect(struct device *d, if (!rtnl_trylock()) return restart_syscall(); + if (!USES_PRIMARY(bond->params.mode)) { + pr_info("%s: Unable to set primary reselect; %s is in mode %d\n", + bond->dev->name, bond->dev->name, bond->params.mode); + ret = -EINVAL; + goto out; + } + new_value = bond_parse_parm(buf, pri_reselect_tbl); if (new_value < 0) { pr_err("%s: Ignoring invalid primary_reselect value %.*s.\n",
The primary_reselect only reselection for the primary slave, but the primary slave only support for ALB, TLB and AB mode, so we sould set the primary_reselect for these mode. to fix this: Add a check for ALB, TLB and AB mode in bonding_store_primary_reselect, avoid to select active slave again in other modes. Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> --- drivers/net/bonding/bond_sysfs.c | 7 +++++++ 1 file changed, 7 insertions(+)