Patch "bonding: Don't update slave->link until ready to commit" has been added to the 3.18-stable tree
Subsystems (3)
This is a note to let you know that I've just added the patch titled
bonding: Don't update slave->link until ready to commit
to the 3.18-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
bonding-don-t-update-slave-link-until-ready-to-commit.patch
and it can be found in the queue-3.18 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
Quoted 1 line
From foo@baz Tue Apr 10 13:58:07 CEST 2018
From: Nithin Sujir <[email protected]>
Date: Wed, 24 May 2017 19:45:17 -0700
Subject: bonding: Don't update slave->link until ready to commit
From: Nithin Sujir <[email protected]>
[ Upstream commit 797a93647a48d6cb8a20641a86a71713a947f786 ]
In the loadbalance arp monitoring scheme, when a slave link change is detected, the slave->link is immediately updated and slave_state_changed is set. Later down the function, the rtnl_lock is acquired and the changes are committed, updating the bond link state.
However, the acquisition of the rtnl_lock can fail. The next time the monitor runs, since slave->link is already updated, it determines that link is unchanged. This results in the bond link state permanently out of sync with the slave link.
This patch modifies bond_loadbalance_arp_mon() to handle link changes identical to bond_ab_arp_{inspect/commit}(). The new link state is maintained in slave->new_link until we're ready to commit at which point it's copied into slave->link.
NOTE: miimon_{inspect/commit}() has a more complex state machine requiring the use of the bond_{propose,commit}_link_state() functions which maintains the intermediate state in slave->link_new_state. The arp monitors don't require that.
Testing: This bug is very easy to reproduce with the following steps.
1. In a loop, toggle a slave link of a bond slave interface.
2. In a separate loop, do ifconfig up/down of an unrelated interface to create contention for rtnl_lock.
Within a few iterations, the bond link goes out of sync with the slave link.
--- drivers/net/bonding/bond_main.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/net/bonding/bond_main.c +9 -2
@@ -2425,11 +2425,13 @@ static void bond_loadbalance_arp_mon(strbond_for_each_slave_rcu (bond, slave, iter) {unsigned long trans_start =dev_trans_start (slave->dev); + slave->new_link =BOND_LINK_NOCHANGE ; +if (slave->link !=BOND_LINK_UP ) {if (bond_time_in_interval (bond, trans_start,1 ) &&bond_time_in_interval (bond, slave->last_rx,1 )) { - slave->link =BOND_LINK_UP ; + slave->new_link =BOND_LINK_UP ; slave_state_changed =1 ;/* primary_slave has no meaning in round-robin
@@ -2456,7 +2458,7 @@ static void bond_loadbalance_arp_mon(strif (!bond_time_in_interval (bond, trans_start,2 ) || !bond_time_in_interval (bond, slave->last_rx,2 )) { - slave->link =BOND_LINK_DOWN ; + slave->new_link =BOND_LINK_DOWN ; slave_state_changed =1 ;if (slave->link_failure_count <UINT_MAX )
@@ -2487,6 +2489,11 @@ static void bond_loadbalance_arp_mon(strif (!rtnl_trylock ()) goto re_arm; +bond_for_each_slave (bond, slave, iter) { +if (slave->new_link !=BOND_LINK_NOCHANGE ) + slave->link = slave->new_link; + } +if (slave_state_changed) {bond_slave_state_change (bond);if (BOND_MODE (bond) ==BOND_MODE_XOR )
Patches currently in stable-queue which might be from [email protected] are
queue-3.18/bonding-don-t-update-slave-link-until-ready-to-commit.patch