Commit 16d79d7d authored by Nils Carlson's avatar Nils Carlson Committed by David S. Miller

bonding 802.3ad: Fix the state machine locking v2

Changes since v1:
* Clarify an unclear comment
* Move a (possible) name change to a separate patch

The ad_rx_machine, ad_periodic_machine and ad_port_selection_logic
functions all inspect and alter common fields within the port structure.
Previous to this patch, only the ad_rx_machines were mutexed, and the
periodic and port_selection could run unmutexed against an ad_rx_machine
trigged by an arriving LACPDU.

This patch remedies the situation by protecting all the state machines
from concurrency. This is accomplished by locking around all the state
machines for a given port, which are executed at regular intervals; and
the ad_rx_machine when handling an incoming LACPDU.
Signed-off-by: default avatarNils Carlson <nils.carlson@ericsson.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent ce3c8692
...@@ -1025,9 +1025,6 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port) ...@@ -1025,9 +1025,6 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
{ {
rx_states_t last_state; rx_states_t last_state;
// Lock to prevent 2 instances of this function to run simultaneously(rx interrupt and periodic machine callback)
__get_rx_machine_lock(port);
// keep current State Machine state to compare later if it was changed // keep current State Machine state to compare later if it was changed
last_state = port->sm_rx_state; last_state = port->sm_rx_state;
...@@ -1133,7 +1130,6 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port) ...@@ -1133,7 +1130,6 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
pr_err("%s: An illegal loopback occurred on adapter (%s).\n" pr_err("%s: An illegal loopback occurred on adapter (%s).\n"
"Check the configuration to verify that all adapters are connected to 802.3ad compliant switch ports\n", "Check the configuration to verify that all adapters are connected to 802.3ad compliant switch ports\n",
port->slave->dev->master->name, port->slave->dev->name); port->slave->dev->master->name, port->slave->dev->name);
__release_rx_machine_lock(port);
return; return;
} }
__update_selected(lacpdu, port); __update_selected(lacpdu, port);
...@@ -1153,7 +1149,6 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port) ...@@ -1153,7 +1149,6 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
break; break;
} }
} }
__release_rx_machine_lock(port);
} }
/** /**
...@@ -2155,6 +2150,12 @@ void bond_3ad_state_machine_handler(struct work_struct *work) ...@@ -2155,6 +2150,12 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
goto re_arm; goto re_arm;
} }
/* Lock around state machines to protect data accessed
* by all (e.g., port->sm_vars). ad_rx_machine may run
* concurrently due to incoming LACPDU.
*/
__get_rx_machine_lock(port);
ad_rx_machine(NULL, port); ad_rx_machine(NULL, port);
ad_periodic_machine(port); ad_periodic_machine(port);
ad_port_selection_logic(port); ad_port_selection_logic(port);
...@@ -2164,6 +2165,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work) ...@@ -2164,6 +2165,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
// turn off the BEGIN bit, since we already handled it // turn off the BEGIN bit, since we already handled it
if (port->sm_vars & AD_PORT_BEGIN) if (port->sm_vars & AD_PORT_BEGIN)
port->sm_vars &= ~AD_PORT_BEGIN; port->sm_vars &= ~AD_PORT_BEGIN;
__release_rx_machine_lock(port);
} }
re_arm: re_arm:
...@@ -2200,7 +2203,10 @@ static void bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave, u ...@@ -2200,7 +2203,10 @@ static void bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave, u
case AD_TYPE_LACPDU: case AD_TYPE_LACPDU:
pr_debug("Received LACPDU on port %d\n", pr_debug("Received LACPDU on port %d\n",
port->actor_port_number); port->actor_port_number);
/* Protect against concurrent state machines */
__get_rx_machine_lock(port);
ad_rx_machine(lacpdu, port); ad_rx_machine(lacpdu, port);
__release_rx_machine_lock(port);
break; break;
case AD_TYPE_MARKER: case AD_TYPE_MARKER:
......
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