bonding/802.3ad: fix link_failure_count tracking
Subsystems (4)
PATCH 1/1 bonding/802.3ad: fix link_failure_count tracking
Commit 4d2c0cda07448ea6980f00102dc3964eb25e241c set slave->link to BOND_LINK_DOWN for 802.3ad bonds whenever invalid speed/duplex values were read, to fix a problem with slaves getting into weird states, but in the process, broke tracking of link failures, as going straight to BOND_LINK_DOWN when a link is indeed down (cable pulled, switch rebooted) means we broke out of bond_miimon_inspect()'s BOND_LINK_DOWN case because !link_state was already true, we never incremented commit, and never got a chance to call bond_miimon_commit(), where slave->link_failure_count would be incremented. I believe the simple fix here is to mark the slave as BOND_LINK_FAIL, and let bond_miimon_inspect() transition the link from _FAIL to either _UP or _DOWN, and in the latter case, we now get proper incrementing of link_failure_count again.
--- drivers/net/bonding/bond_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/bonding/bond_main.c +2 -2
@@ -3112,13 +3112,13 @@ static int bond_slave_netdev_event(unsigned long event,case NETDEV_CHANGE :/* For 802.3ad mode only: * Getting invalid Speed/Duplex values here will put slave -* in weird state. So mark it as link-down for the time +* in weird state. So mark it as link-fail for the time * being and let link-monitoring (miimon) set it right when * correct speeds/duplex are available. */ if (bond_update_speed_duplex (slave) &&BOND_MODE (bond) ==BOND_MODE_8023AD ) - slave->link =BOND_LINK_DOWN ; + slave->link =BOND_LINK_FAIL ;if (BOND_MODE (bond) ==BOND_MODE_8023AD )bond_3ad_adapter_speed_duplex_changed (slave);
Signature · 1 line
2.16.1
David Miller
maintainerFrom: Jarod Wilson <[email protected]>
Date: Thu, 1 Nov 2018 17:22:40 -0400
Quoted 1 line
Fixes 4d2c0cda07448ea6980f00102dc3964eb25e241c
The Fixes tag format should be:
Where SHA1_ID has 12 digits of significance.
Thanks.
PATCH v2 1/1 bonding/802.3ad: fix link_failure_count tracking
In reply to Jarod WilsonCommit 4d2c0cda07448ea6980f00102dc3964eb25e241c set slave->link to BOND_LINK_DOWN for 802.3ad bonds whenever invalid speed/duplex values were read, to fix a problem with slaves getting into weird states, but in the process, broke tracking of link failures, as going straight to BOND_LINK_DOWN when a link is indeed down (cable pulled, switch rebooted) means we broke out of bond_miimon_inspect()'s BOND_LINK_DOWN case because !link_state was already true, we never incremented commit, and never got a chance to call bond_miimon_commit(), where slave->link_failure_count would be incremented. I believe the simple fix here is to mark the slave as BOND_LINK_FAIL, and let bond_miimon_inspect() transition the link from _FAIL to either _UP or _DOWN, and in the latter case, we now get proper incrementing of link_failure_count again.
---
v2: fix Fixes line
drivers/net/bonding/bond_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/bonding/bond_main.c +2 -2
@@ -3112,13 +3112,13 @@ static int bond_slave_netdev_event(unsigned long event,case NETDEV_CHANGE :/* For 802.3ad mode only: * Getting invalid Speed/Duplex values here will put slave -* in weird state. So mark it as link-down for the time +* in weird state. So mark it as link-fail for the time * being and let link-monitoring (miimon) set it right when * correct speeds/duplex are available. */ if (bond_update_speed_duplex (slave) &&BOND_MODE (bond) ==BOND_MODE_8023AD ) - slave->link =BOND_LINK_DOWN ; + slave->link =BOND_LINK_FAIL ;if (BOND_MODE (bond) ==BOND_MODE_8023AD )bond_3ad_adapter_speed_duplex_changed (slave);
Signature · 1 line
2.16.1
David Miller
maintainerFrom: Jarod Wilson <[email protected]>
Date: Sun, 4 Nov 2018 14:59:46 -0500
Quoted 13 lines
Commit
4d2c0cda07448ea6980f00102dc3964eb25e241csetslave->linktoBOND_LINK_DOWNfor 802.3ad bonds whenever invalid speed/duplex values were read, to fix a problem with slaves getting into weird states, but in the process, broke tracking of link failures, as going straight toBOND_LINK_DOWNwhen a link is indeed down (cable pulled, switch rebooted) means we broke out of bond_miimon_inspect()'sBOND_LINK_DOWNcase because !link_state was already true, we never incremented commit, and never got a chance to callbond_miimon_commit(), whereslave->link_failure_countwould be incremented. I believe the simple fix here is to mark the slave asBOND_LINK_FAIL, and letbond_miimon_inspect()transition the link from_FAILto either_UPor_DOWN, and in the latter case, we now get proper incrementing oflink_failure_countagain.
Quoted 8 lines
Fixes 4d2c0cda0744 ("bonding: speed/duplex update at NETDEV_UP event")CC Mahesh Bandewar <[email protected]>CC David S. Miller <[email protected]>Signed-off-by Jarod Wilson <[email protected]>---
v2: fix Fixes line
Applied and queued up for -stable.