Commit a176ded3 authored by David S. Miller's avatar David S. Miller

Merge branch 'bonding-actor-updates'

Mahesh Bandewar says:

====================
re-org actor admin/oper key updates

I was observing machines entering into weird LACP state when the
partner is in passive mode. This issue is not because of the partners
in passive state but probably because of some operational key update
which is pushing the state-machine is that weird state. This was
happening randomly on about 1% of the machine (when the sample size
is a large set of machines with a variety of NICs/ports bonded).

In this patch-series I'm attempting to unify the logic of actor-key
/ operational-key changes to one place to avoid possible errors in
update. Also this eliminates the need for the event-handler to decide
if the key needs update.

After this patch-set none of the machines (from same sample set) were
exhibiting LACP-weirdness that was observed earlier.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 12d43096 52bc6716
...@@ -127,6 +127,7 @@ static void ad_marker_info_received(struct bond_marker *marker_info, ...@@ -127,6 +127,7 @@ static void ad_marker_info_received(struct bond_marker *marker_info,
struct port *port); struct port *port);
static void ad_marker_response_received(struct bond_marker *marker, static void ad_marker_response_received(struct bond_marker *marker,
struct port *port); struct port *port);
static void ad_update_actor_keys(struct port *port, bool reset);
/* ================= api to bonding and kernel code ================== */ /* ================= api to bonding and kernel code ================== */
...@@ -327,14 +328,12 @@ static u16 __get_link_speed(struct port *port) ...@@ -327,14 +328,12 @@ static u16 __get_link_speed(struct port *port)
static u8 __get_duplex(struct port *port) static u8 __get_duplex(struct port *port)
{ {
struct slave *slave = port->slave; struct slave *slave = port->slave;
u8 retval; u8 retval = 0x0;
/* handling a special case: when the configuration starts with /* handling a special case: when the configuration starts with
* link down, it sets the duplex to 0. * link down, it sets the duplex to 0.
*/ */
if (slave->link != BOND_LINK_UP) { if (slave->link == BOND_LINK_UP) {
retval = 0x0;
} else {
switch (slave->duplex) { switch (slave->duplex) {
case DUPLEX_FULL: case DUPLEX_FULL:
retval = 0x1; retval = 0x1;
...@@ -1953,14 +1952,7 @@ void bond_3ad_bind_slave(struct slave *slave) ...@@ -1953,14 +1952,7 @@ void bond_3ad_bind_slave(struct slave *slave)
* user key * user key
*/ */
port->actor_admin_port_key = bond->params.ad_user_port_key << 6; port->actor_admin_port_key = bond->params.ad_user_port_key << 6;
port->actor_admin_port_key |= __get_duplex(port); ad_update_actor_keys(port, false);
port->actor_admin_port_key |= (__get_link_speed(port) << 1);
port->actor_oper_port_key = port->actor_admin_port_key;
/* if the port is not full duplex, then the port should be not
* lacp Enabled
*/
if (!(port->actor_oper_port_key & AD_DUPLEX_KEY_MASKS))
port->sm_vars &= ~AD_PORT_LACP_ENABLED;
/* actor system is the bond's system */ /* actor system is the bond's system */
port->actor_system = BOND_AD_INFO(bond).system.sys_mac_addr; port->actor_system = BOND_AD_INFO(bond).system.sys_mac_addr;
port->actor_system_priority = port->actor_system_priority =
...@@ -2310,45 +2302,60 @@ static int bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave, ...@@ -2310,45 +2302,60 @@ static int bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave,
} }
/** /**
* bond_3ad_adapter_speed_changed - handle a slave's speed change indication * ad_update_actor_keys - Update the oper / admin keys for a port based on
* @slave: slave struct to work on * its current speed and duplex settings.
* *
* Handle reselection of aggregator (if needed) for this port. * @port: the port we'are looking at
* @reset: Boolean to just reset the speed and the duplex part of the key
*
* The logic to change the oper / admin keys is:
* (a) A full duplex port can participate in LACP with partner.
* (b) When the speed is changed, LACP need to be reinitiated.
*/ */
void bond_3ad_adapter_speed_changed(struct slave *slave) static void ad_update_actor_keys(struct port *port, bool reset)
{ {
struct port *port; u8 duplex = 0;
u16 ospeed = 0, speed = 0;
port = &(SLAVE_AD_INFO(slave)->port); u16 old_oper_key = port->actor_oper_port_key;
/* if slave is null, the whole port is not initialized */ port->actor_admin_port_key &= ~(AD_SPEED_KEY_MASKS|AD_DUPLEX_KEY_MASKS);
if (!port->slave) { if (!reset) {
netdev_warn(slave->bond->dev, "speed changed for uninitialized port on %s\n", speed = __get_link_speed(port);
slave->dev->name); ospeed = (old_oper_key & AD_SPEED_KEY_MASKS) >> 1;
return; duplex = __get_duplex(port);
port->actor_admin_port_key |= (speed << 1) | duplex;
} }
spin_lock_bh(&slave->bond->mode_lock);
port->actor_admin_port_key &= ~AD_SPEED_KEY_MASKS;
port->actor_admin_port_key |= __get_link_speed(port) << 1;
port->actor_oper_port_key = port->actor_admin_port_key; port->actor_oper_port_key = port->actor_admin_port_key;
netdev_dbg(slave->bond->dev, "Port %d changed speed\n", port->actor_port_number);
/* there is no need to reselect a new aggregator, just signal the
* state machines to reinitialize
*/
port->sm_vars |= AD_PORT_BEGIN;
spin_unlock_bh(&slave->bond->mode_lock); if (old_oper_key != port->actor_oper_port_key) {
/* Only 'duplex' port participates in LACP */
if (duplex)
port->sm_vars |= AD_PORT_LACP_ENABLED;
else
port->sm_vars &= ~AD_PORT_LACP_ENABLED;
if (!reset) {
if (!speed) {
netdev_err(port->slave->dev,
"speed changed to 0 for port %s",
port->slave->dev->name);
} else if (duplex && ospeed != speed) {
/* Speed change restarts LACP state-machine */
port->sm_vars |= AD_PORT_BEGIN;
}
}
}
} }
/** /**
* bond_3ad_adapter_duplex_changed - handle a slave's duplex change indication * bond_3ad_adapter_speed_duplex_changed - handle a slave's speed / duplex
* change indication
*
* @slave: slave struct to work on * @slave: slave struct to work on
* *
* Handle reselection of aggregator (if needed) for this port. * Handle reselection of aggregator (if needed) for this port.
*/ */
void bond_3ad_adapter_duplex_changed(struct slave *slave) void bond_3ad_adapter_speed_duplex_changed(struct slave *slave)
{ {
struct port *port; struct port *port;
...@@ -2356,25 +2363,16 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave) ...@@ -2356,25 +2363,16 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
/* 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) {
netdev_warn(slave->bond->dev, "duplex changed for uninitialized port on %s\n", netdev_warn(slave->bond->dev,
"speed/duplex changed for uninitialized port %s\n",
slave->dev->name); slave->dev->name);
return; return;
} }
spin_lock_bh(&slave->bond->mode_lock); spin_lock_bh(&slave->bond->mode_lock);
ad_update_actor_keys(port, false);
port->actor_admin_port_key &= ~AD_DUPLEX_KEY_MASKS; netdev_dbg(slave->bond->dev, "Port %d slave %s changed speed/duplex\n",
port->actor_admin_port_key |= __get_duplex(port);
port->actor_oper_port_key = port->actor_admin_port_key;
netdev_dbg(slave->bond->dev, "Port %d slave %s changed duplex\n",
port->actor_port_number, slave->dev->name); port->actor_port_number, slave->dev->name);
if (port->actor_oper_port_key & AD_DUPLEX_KEY_MASKS)
port->sm_vars |= AD_PORT_LACP_ENABLED;
/* there is no need to reselect a new aggregator, just signal the
* state machines to reinitialize
*/
port->sm_vars |= AD_PORT_BEGIN;
spin_unlock_bh(&slave->bond->mode_lock); spin_unlock_bh(&slave->bond->mode_lock);
} }
...@@ -2406,26 +2404,17 @@ void bond_3ad_handle_link_change(struct slave *slave, char link) ...@@ -2406,26 +2404,17 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
* on link up we are forcing recheck on the duplex and speed since * on link up we are forcing recheck on the duplex and speed since
* some of he adaptors(ce1000.lan) report. * some of he adaptors(ce1000.lan) report.
*/ */
port->actor_admin_port_key &= ~(AD_DUPLEX_KEY_MASKS|AD_SPEED_KEY_MASKS);
if (link == BOND_LINK_UP) { if (link == BOND_LINK_UP) {
port->is_enabled = true; port->is_enabled = true;
port->actor_admin_port_key |= ad_update_actor_keys(port, false);
(__get_link_speed(port) << 1) | __get_duplex(port);
if (port->actor_admin_port_key & AD_DUPLEX_KEY_MASKS)
port->sm_vars |= AD_PORT_LACP_ENABLED;
} else { } else {
/* link has failed */ /* link has failed */
port->is_enabled = false; port->is_enabled = false;
port->sm_vars &= ~AD_PORT_LACP_ENABLED; ad_update_actor_keys(port, true);
} }
port->actor_oper_port_key = port->actor_admin_port_key;
netdev_dbg(slave->bond->dev, "Port %d changed link status to %s\n", netdev_dbg(slave->bond->dev, "Port %d changed link status to %s\n",
port->actor_port_number, port->actor_port_number,
link == BOND_LINK_UP ? "UP" : "DOWN"); 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;
spin_unlock_bh(&slave->bond->mode_lock); spin_unlock_bh(&slave->bond->mode_lock);
......
...@@ -2943,8 +2943,6 @@ static int bond_slave_netdev_event(unsigned long event, ...@@ -2943,8 +2943,6 @@ static int bond_slave_netdev_event(unsigned long event,
struct slave *slave = bond_slave_get_rtnl(slave_dev), *primary; struct slave *slave = bond_slave_get_rtnl(slave_dev), *primary;
struct bonding *bond; struct bonding *bond;
struct net_device *bond_dev; struct net_device *bond_dev;
u32 old_speed;
u8 old_duplex;
/* A netdev event can be generated while enslaving a device /* A netdev event can be generated while enslaving a device
* before netdev_rx_handler_register is called in which case * before netdev_rx_handler_register is called in which case
...@@ -2965,17 +2963,9 @@ static int bond_slave_netdev_event(unsigned long event, ...@@ -2965,17 +2963,9 @@ static int bond_slave_netdev_event(unsigned long event,
break; break;
case NETDEV_UP: case NETDEV_UP:
case NETDEV_CHANGE: case NETDEV_CHANGE:
old_speed = slave->speed;
old_duplex = slave->duplex;
bond_update_speed_duplex(slave); bond_update_speed_duplex(slave);
if (BOND_MODE(bond) == BOND_MODE_8023AD)
if (BOND_MODE(bond) == BOND_MODE_8023AD) { bond_3ad_adapter_speed_duplex_changed(slave);
if (old_speed != slave->speed)
bond_3ad_adapter_speed_changed(slave);
if (old_duplex != slave->duplex)
bond_3ad_adapter_duplex_changed(slave);
}
/* Fallthrough */ /* Fallthrough */
case NETDEV_DOWN: case NETDEV_DOWN:
/* Refresh slave-array if applicable! /* Refresh slave-array if applicable!
......
...@@ -297,8 +297,7 @@ void bond_3ad_bind_slave(struct slave *slave); ...@@ -297,8 +297,7 @@ void bond_3ad_bind_slave(struct slave *slave);
void bond_3ad_unbind_slave(struct slave *slave); void bond_3ad_unbind_slave(struct slave *slave);
void bond_3ad_state_machine_handler(struct work_struct *); void bond_3ad_state_machine_handler(struct work_struct *);
void bond_3ad_initiate_agg_selection(struct bonding *bond, int timeout); void bond_3ad_initiate_agg_selection(struct bonding *bond, int timeout);
void bond_3ad_adapter_speed_changed(struct slave *slave); void bond_3ad_adapter_speed_duplex_changed(struct slave *slave);
void bond_3ad_adapter_duplex_changed(struct slave *slave);
void bond_3ad_handle_link_change(struct slave *slave, char link); void bond_3ad_handle_link_change(struct slave *slave, char link);
int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info); int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info);
int __bond_3ad_get_active_agg_info(struct bonding *bond, int __bond_3ad_get_active_agg_info(struct bonding *bond,
......
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