Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

Weiping Pan and committed by
David S. Miller
98f41f69 deede2fa

+22 -27
+12 -25
drivers/net/bonding/bond_main.c
··· 550 550 /* 551 551 * Get link speed and duplex from the slave's base driver 552 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 554 * and return error. 555 555 */ 556 556 static int bond_update_speed_duplex(struct slave *slave) ··· 560 560 u32 slave_speed; 561 561 int res; 562 562 563 - /* Fake speed and duplex */ 564 - slave->speed = SPEED_100; 565 - slave->duplex = DUPLEX_FULL; 563 + slave->speed = -1; 564 + slave->duplex = -1; 566 565 567 566 res = __ethtool_get_settings(slave_dev, &ecmd); 568 567 if (res < 0) ··· 1750 1751 new_slave->link = BOND_LINK_DOWN; 1751 1752 } 1752 1753 1753 - if (bond_update_speed_duplex(new_slave) && 1754 - (new_slave->link != BOND_LINK_DOWN)) { 1755 - pr_warning("%s: Warning: failed to get speed and duplex from %s, assumed to be 100Mb/sec and Full.\n", 1756 - bond_dev->name, new_slave->dev->name); 1757 - 1758 - if (bond->params.mode == BOND_MODE_8023AD) { 1759 - pr_warning("%s: Warning: Operation of 802.3ad mode requires ETHTOOL support in base driver for proper aggregator selection.\n", 1760 - bond_dev->name); 1761 - } 1762 - } 1754 + bond_update_speed_duplex(new_slave); 1763 1755 1764 1756 if (USES_PRIMARY(bond->params.mode) && bond->params.primary[0]) { 1765 1757 /* if there is a primary slave, remember it */ ··· 3210 3220 { 3211 3221 struct net_device *bond_dev = slave_dev->master; 3212 3222 struct bonding *bond = netdev_priv(bond_dev); 3223 + struct slave *slave = NULL; 3213 3224 3214 3225 switch (event) { 3215 3226 case NETDEV_UNREGISTER: ··· 3221 3230 bond_release(bond_dev, slave_dev); 3222 3231 } 3223 3232 break; 3233 + case NETDEV_UP: 3224 3234 case NETDEV_CHANGE: 3225 - if (bond->params.mode == BOND_MODE_8023AD || bond_is_lb(bond)) { 3226 - struct slave *slave; 3235 + slave = bond_get_slave_by_dev(bond, slave_dev); 3236 + if (slave) { 3237 + u32 old_speed = slave->speed; 3238 + u8 old_duplex = slave->duplex; 3227 3239 3228 - slave = bond_get_slave_by_dev(bond, slave_dev); 3229 - if (slave) { 3230 - u32 old_speed = slave->speed; 3231 - u8 old_duplex = slave->duplex; 3240 + bond_update_speed_duplex(slave); 3232 3241 3233 - bond_update_speed_duplex(slave); 3234 - 3235 - if (bond_is_lb(bond)) 3236 - break; 3237 - 3242 + if (bond->params.mode == BOND_MODE_8023AD) { 3238 3243 if (old_speed != slave->speed) 3239 3244 bond_3ad_adapter_speed_changed(slave); 3240 3245 if (old_duplex != slave->duplex)
+10 -2
drivers/net/bonding/bond_procfs.c
··· 157 157 seq_printf(seq, "\nSlave Interface: %s\n", slave->dev->name); 158 158 seq_printf(seq, "MII Status: %s\n", 159 159 (slave->link == BOND_LINK_UP) ? "up" : "down"); 160 - seq_printf(seq, "Speed: %d Mbps\n", slave->speed); 161 - seq_printf(seq, "Duplex: %s\n", slave->duplex ? "full" : "half"); 160 + if (slave->speed == -1) 161 + seq_printf(seq, "Speed: %s\n", "Unknown"); 162 + else 163 + seq_printf(seq, "Speed: %d Mbps\n", slave->speed); 164 + 165 + if (slave->duplex == -1) 166 + seq_printf(seq, "Duplex: %s\n", "Unknown"); 167 + else 168 + seq_printf(seq, "Duplex: %s\n", slave->duplex ? "full" : "half"); 169 + 162 170 seq_printf(seq, "Link Failure Count: %u\n", 163 171 slave->link_failure_count); 164 172