Commit 405a96d6 authored by David S. Miller's avatar David S. Miller

Merge branch 'bond_locking'

Ding Tianhong says:

====================
Jay Vosburgh said that the bond_3ad_adapter_speed_changed and
bond_3ad_adapter_duplex_changed is called with RTNL only, and
the functions will modify the port's information with no further
locking, they will not mutex against bond state machine and
incoming LACPDU which do not hold RTNL, So I add port lock to
protect the port information.

But they are not critical bugs, they exist since day one, and till
now they have never been hit and reported, because change for speed
and duplex is very rare, and will not occur critical problem.

The comments in the function is very old, cleanup the comments together.
====================
Signed-off-by: default avatarJay Vosburgh <fubar@us.ibm.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 35eecf05 108db736
...@@ -2201,20 +2201,25 @@ void bond_3ad_adapter_speed_changed(struct slave *slave) ...@@ -2201,20 +2201,25 @@ void bond_3ad_adapter_speed_changed(struct slave *slave)
port = &(SLAVE_AD_INFO(slave).port); port = &(SLAVE_AD_INFO(slave).port);
// if slave is null, the whole port is not initialized /* if slave is null, the whole port is not initialized */
if (!port->slave) { if (!port->slave) {
pr_warning("Warning: %s: speed changed for uninitialized port on %s\n", pr_warning("Warning: %s: speed changed for uninitialized port on %s\n",
slave->bond->dev->name, slave->dev->name); slave->bond->dev->name, slave->dev->name);
return; return;
} }
__get_state_machine_lock(port);
port->actor_admin_port_key &= ~AD_SPEED_KEY_BITS; port->actor_admin_port_key &= ~AD_SPEED_KEY_BITS;
port->actor_oper_port_key = port->actor_admin_port_key |= port->actor_oper_port_key = port->actor_admin_port_key |=
(__get_link_speed(port) << 1); (__get_link_speed(port) << 1);
pr_debug("Port %d changed speed\n", port->actor_port_number); pr_debug("Port %d changed speed\n", port->actor_port_number);
// there is no need to reselect a new aggregator, just signal the /* there is no need to reselect a new aggregator, just signal the
// state machines to reinitialize * state machines to reinitialize
*/
port->sm_vars |= AD_PORT_BEGIN; port->sm_vars |= AD_PORT_BEGIN;
__release_state_machine_lock(port);
} }
/** /**
...@@ -2229,20 +2234,25 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave) ...@@ -2229,20 +2234,25 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
port = &(SLAVE_AD_INFO(slave).port); port = &(SLAVE_AD_INFO(slave).port);
// if slave is null, the whole port is not initialized /* if slave is null, the whole port is not initialized */
if (!port->slave) { if (!port->slave) {
pr_warning("%s: Warning: duplex changed for uninitialized port on %s\n", pr_warning("%s: Warning: duplex changed for uninitialized port on %s\n",
slave->bond->dev->name, slave->dev->name); slave->bond->dev->name, slave->dev->name);
return; return;
} }
__get_state_machine_lock(port);
port->actor_admin_port_key &= ~AD_DUPLEX_KEY_BITS; port->actor_admin_port_key &= ~AD_DUPLEX_KEY_BITS;
port->actor_oper_port_key = port->actor_admin_port_key |= port->actor_oper_port_key = port->actor_admin_port_key |=
__get_duplex(port); __get_duplex(port);
pr_debug("Port %d changed duplex\n", port->actor_port_number); pr_debug("Port %d changed duplex\n", port->actor_port_number);
// there is no need to reselect a new aggregator, just signal the /* there is no need to reselect a new aggregator, just signal the
// state machines to reinitialize * state machines to reinitialize
*/
port->sm_vars |= AD_PORT_BEGIN; port->sm_vars |= AD_PORT_BEGIN;
__release_state_machine_lock(port);
} }
/** /**
...@@ -2258,15 +2268,21 @@ void bond_3ad_handle_link_change(struct slave *slave, char link) ...@@ -2258,15 +2268,21 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
port = &(SLAVE_AD_INFO(slave).port); port = &(SLAVE_AD_INFO(slave).port);
// if slave is null, the whole port is not initialized /* if slave is null, the whole port is not initialized */
if (!port->slave) { if (!port->slave) {
pr_warning("Warning: %s: link status changed for uninitialized port on %s\n", pr_warning("Warning: %s: link status changed for uninitialized port on %s\n",
slave->bond->dev->name, slave->dev->name); slave->bond->dev->name, slave->dev->name);
return; return;
} }
// on link down we are zeroing duplex and speed since some of the adaptors(ce1000.lan) report full duplex/speed instead of N/A(duplex) / 0(speed) __get_state_machine_lock(port);
// on link up we are forcing recheck on the duplex and speed since some of he adaptors(ce1000.lan) report /* on link down we are zeroing duplex and speed since
* some of the adaptors(ce1000.lan) report full duplex/speed
* instead of N/A(duplex) / 0(speed).
*
* on link up we are forcing recheck on the duplex and speed since
* some of he adaptors(ce1000.lan) report.
*/
if (link == BOND_LINK_UP) { if (link == BOND_LINK_UP) {
port->is_enabled = true; port->is_enabled = true;
port->actor_admin_port_key &= ~AD_DUPLEX_KEY_BITS; port->actor_admin_port_key &= ~AD_DUPLEX_KEY_BITS;
...@@ -2282,10 +2298,15 @@ void bond_3ad_handle_link_change(struct slave *slave, char link) ...@@ -2282,10 +2298,15 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
port->actor_oper_port_key = (port->actor_admin_port_key &= port->actor_oper_port_key = (port->actor_admin_port_key &=
~AD_SPEED_KEY_BITS); ~AD_SPEED_KEY_BITS);
} }
//BOND_PRINT_DBG(("Port %d changed link status to %s", port->actor_port_number, ((link == BOND_LINK_UP)?"UP":"DOWN"))); pr_debug("Port %d changed link status to %s",
// there is no need to reselect a new aggregator, just signal the port->actor_port_number,
// state machines to reinitialize (link == BOND_LINK_UP) ? "UP" : "DOWN");
/* there is no need to reselect a new aggregator, just signal the
* state machines to reinitialize
*/
port->sm_vars |= AD_PORT_BEGIN; port->sm_vars |= AD_PORT_BEGIN;
__release_state_machine_lock(port);
} }
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment