diff options
author | Weiping Pan <wpan@redhat.com> | 2011-10-31 13:20:48 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2011-11-01 17:52:49 -0400 |
commit | 98f41f694f46085fda475cdee8cc0b6d2c5e6f1f (patch) | |
tree | aabd6b698ba6319c95eabe1a68f37207e94b51ec /drivers/net/bonding/bond_main.c | |
parent | deede2fabe24e00bd7e246eb81cd5767dc6fcfc7 (diff) |
bonding:update speed/duplex for NETDEV_CHANGE
Zheng Liang(lzheng@redhat.com) found a bug that if we config bonding with
arp monitor, sometimes bonding driver cannot get the speed and duplex from
its slaves, it will assume them to be 100Mb/sec and Full, please see
/proc/net/bonding/bond0.
But there is no such problem when uses miimon.
(Take igb for example)
I find that the reason is that after dev_open() in bond_enslave(),
bond_update_speed_duplex() will call igb_get_settings()
, but in that function,
it runs ethtool_cmd_speed_set(ecmd, -1); ecmd->duplex = -1;
because igb get an error value of status.
So even dev_open() is called, but the device is not really ready to get its
settings.
Maybe it is safe for us to call igb_get_settings() only after
this message shows up, that is "igb: p4p1 NIC Link is Up 1000 Mbps Full Duplex,
Flow Control: RX".
So I prefer to update the speed and duplex for a slave when reseices
NETDEV_CHANGE/NETDEV_UP event.
Changelog
V2:
1 remove the "fake 100/Full" logic in bond_update_speed_duplex(),
set speed and duplex to -1 when it gets error value of speed and duplex.
2 delete the warning in bond_enslave() if bond_update_speed_duplex() returns
error.
3 make bond_info_show_slave() handle bad values of speed and duplex.
Signed-off-by: Weiping Pan <wpan@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers/net/bonding/bond_main.c')
-rw-r--r-- | drivers/net/bonding/bond_main.c | 37 |
1 files changed, 12 insertions, 25 deletions
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index c34cc1e7c6f6..b2b9109b6712 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c | |||
@@ -550,7 +550,7 @@ down: | |||
550 | /* | 550 | /* |
551 | * Get link speed and duplex from the slave's base driver | 551 | * Get link speed and duplex from the slave's base driver |
552 | * using ethtool. If for some reason the call fails or the | 552 | * using ethtool. If for some reason the call fails or the |
553 | * values are invalid, fake speed and duplex to 100/Full | 553 | * values are invalid, set speed and duplex to -1, |
554 | * and return error. | 554 | * and return error. |
555 | */ | 555 | */ |
556 | static int bond_update_speed_duplex(struct slave *slave) | 556 | static int bond_update_speed_duplex(struct slave *slave) |
@@ -560,9 +560,8 @@ static int bond_update_speed_duplex(struct slave *slave) | |||
560 | u32 slave_speed; | 560 | u32 slave_speed; |
561 | int res; | 561 | int res; |
562 | 562 | ||
563 | /* Fake speed and duplex */ | 563 | slave->speed = -1; |
564 | slave->speed = SPEED_100; | 564 | slave->duplex = -1; |
565 | slave->duplex = DUPLEX_FULL; | ||
566 | 565 | ||
567 | res = __ethtool_get_settings(slave_dev, &ecmd); | 566 | res = __ethtool_get_settings(slave_dev, &ecmd); |
568 | if (res < 0) | 567 | if (res < 0) |
@@ -1751,16 +1750,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) | |||
1751 | new_slave->link = BOND_LINK_DOWN; | 1750 | new_slave->link = BOND_LINK_DOWN; |
1752 | } | 1751 | } |
1753 | 1752 | ||
1754 | if (bond_update_speed_duplex(new_slave) && | 1753 | bond_update_speed_duplex(new_slave); |
1755 | (new_slave->link != BOND_LINK_DOWN)) { | ||
1756 | pr_warning("%s: Warning: failed to get speed and duplex from %s, assumed to be 100Mb/sec and Full.\n", | ||
1757 | bond_dev->name, new_slave->dev->name); | ||
1758 | |||
1759 | if (bond->params.mode == BOND_MODE_8023AD) { | ||
1760 | pr_warning("%s: Warning: Operation of 802.3ad mode requires ETHTOOL support in base driver for proper aggregator selection.\n", | ||
1761 | bond_dev->name); | ||
1762 | } | ||
1763 | } | ||
1764 | 1754 | ||
1765 | if (USES_PRIMARY(bond->params.mode) && bond->params.primary[0]) { | 1755 | if (USES_PRIMARY(bond->params.mode) && bond->params.primary[0]) { |
1766 | /* if there is a primary slave, remember it */ | 1756 | /* if there is a primary slave, remember it */ |
@@ -3220,6 +3210,7 @@ static int bond_slave_netdev_event(unsigned long event, | |||
3220 | { | 3210 | { |
3221 | struct net_device *bond_dev = slave_dev->master; | 3211 | struct net_device *bond_dev = slave_dev->master; |
3222 | struct bonding *bond = netdev_priv(bond_dev); | 3212 | struct bonding *bond = netdev_priv(bond_dev); |
3213 | struct slave *slave = NULL; | ||
3223 | 3214 | ||
3224 | switch (event) { | 3215 | switch (event) { |
3225 | case NETDEV_UNREGISTER: | 3216 | case NETDEV_UNREGISTER: |
@@ -3230,20 +3221,16 @@ static int bond_slave_netdev_event(unsigned long event, | |||
3230 | bond_release(bond_dev, slave_dev); | 3221 | bond_release(bond_dev, slave_dev); |
3231 | } | 3222 | } |
3232 | break; | 3223 | break; |
3224 | case NETDEV_UP: | ||
3233 | case NETDEV_CHANGE: | 3225 | case NETDEV_CHANGE: |
3234 | if (bond->params.mode == BOND_MODE_8023AD || bond_is_lb(bond)) { | 3226 | slave = bond_get_slave_by_dev(bond, slave_dev); |
3235 | struct slave *slave; | 3227 | if (slave) { |
3228 | u32 old_speed = slave->speed; | ||
3229 | u8 old_duplex = slave->duplex; | ||
3236 | 3230 | ||
3237 | slave = bond_get_slave_by_dev(bond, slave_dev); | 3231 | bond_update_speed_duplex(slave); |
3238 | if (slave) { | ||
3239 | u32 old_speed = slave->speed; | ||
3240 | u8 old_duplex = slave->duplex; | ||
3241 | |||
3242 | bond_update_speed_duplex(slave); | ||
3243 | |||
3244 | if (bond_is_lb(bond)) | ||
3245 | break; | ||
3246 | 3232 | ||
3233 | if (bond->params.mode == BOND_MODE_8023AD) { | ||
3247 | if (old_speed != slave->speed) | 3234 | if (old_speed != slave->speed) |
3248 | bond_3ad_adapter_speed_changed(slave); | 3235 | bond_3ad_adapter_speed_changed(slave); |
3249 | if (old_duplex != slave->duplex) | 3236 | if (old_duplex != slave->duplex) |