Commit c8ad0f29 authored by David S. Miller's avatar David S. Miller
Browse files

Merge branch 'bonding_rcu' 



Eric Dumazet says:

====================
bonding: rcu cleanups

RCU was added to bonding in linux-3.12 but lacked proper sparse annotations.

Using __rcu annotation actually helps to spot all accesses to
bond->curr_active_slave & cond->current_arp_slave
are correctly protected, with full sparse & LOCKDEP support.

Lets clean the code.
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 725b7018 85741718

drivers/net/bonding/bond_alb.c

+28 −19
Original line number Diff line number Diff line
@@ -448,11 +448,13 @@ static struct slave *__rlb_next_rx_slave(struct bonding *bond) 
 */

static void rlb_teach_disabled_mac_on_primary(struct bonding *bond, u8 addr[])

{

	if (!bond->curr_active_slave)

	struct slave *curr_active = bond_deref_active_protected(bond);



	if (!curr_active)

		return;



	if (!bond->alb_info.primary_is_promisc) {

		if (!dev_set_promiscuity(bond->curr_active_slave->dev, 1))

		if (!dev_set_promiscuity(curr_active->dev, 1))

			bond->alb_info.primary_is_promisc = 1;

		else

			bond->alb_info.primary_is_promisc = 0;
@@ -460,7 +462,7 @@ static void rlb_teach_disabled_mac_on_primary(struct bonding *bond, u8 addr[]) 


	bond->alb_info.rlb_promisc_timeout_counter = 0;



	alb_send_learning_packets(bond->curr_active_slave, addr, true);

	alb_send_learning_packets(curr_active, addr, true);

}



/* slave being removed should not be active at this point
@@ -509,7 +511,7 @@ static void rlb_clear_slave(struct bonding *bond, struct slave *slave) 


	write_lock_bh(&bond->curr_slave_lock);



	if (slave != bond->curr_active_slave)

	if (slave != bond_deref_active_protected(bond))

		rlb_teach_disabled_mac_on_primary(bond, slave->dev->dev_addr);



	write_unlock_bh(&bond->curr_slave_lock);
@@ -684,7 +686,7 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon 
			 * move the old client to primary (curr_active_slave) so

			 * that the new client can be assigned to this entry.

			 */

			if (bond->curr_active_slave &&

			if (curr_active_slave &&

			    client_info->slave != curr_active_slave) {

				client_info->slave = curr_active_slave;

				rlb_update_client(client_info);
@@ -1221,7 +1223,7 @@ static void alb_change_hw_addr_on_detach(struct bonding *bond, struct slave *sla 
 */

static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slave *slave)

{

	struct slave *has_bond_addr = bond->curr_active_slave;

	struct slave *has_bond_addr = rcu_access_pointer(bond->curr_active_slave);

	struct slave *tmp_slave1, *free_mac_slave = NULL;

	struct list_head *iter;
@@ -1575,7 +1577,7 @@ void bond_alb_monitor(struct work_struct *work) 
			 * use mac of the slave device.

			 * In RLB mode, we always use strict matches.

			 */

			strict_match = (slave != bond->curr_active_slave ||

			strict_match = (slave != rcu_access_pointer(bond->curr_active_slave) ||

					bond_info->rlb_enabled);

			alb_send_learning_packets(slave, slave->dev->dev_addr,

						  strict_match);
@@ -1593,7 +1595,7 @@ void bond_alb_monitor(struct work_struct *work) 


		bond_for_each_slave_rcu(bond, slave, iter) {

			tlb_clear_slave(bond, slave, 1);

			if (slave == bond->curr_active_slave) {

			if (slave == rcu_access_pointer(bond->curr_active_slave)) {

				SLAVE_TLB_INFO(slave).load =

					bond_info->unbalanced_load /

						BOND_TLB_REBALANCE_INTERVAL;
@@ -1625,7 +1627,8 @@ void bond_alb_monitor(struct work_struct *work) 
			 * because a slave was disabled then

			 * it can now leave promiscuous mode.

			 */

			dev_set_promiscuity(bond->curr_active_slave->dev, -1);

			dev_set_promiscuity(rtnl_dereference(bond->curr_active_slave)->dev,

					    -1);

			bond_info->primary_is_promisc = 0;



			rtnl_unlock();
@@ -1742,17 +1745,21 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave 
	__acquires(&bond->curr_slave_lock)

{

	struct slave *swap_slave;

	struct slave *curr_active;



	if (bond->curr_active_slave == new_slave)

	curr_active = rcu_dereference_protected(bond->curr_active_slave,

						!new_slave ||

						lockdep_is_held(&bond->curr_slave_lock));

	if (curr_active == new_slave)

		return;



	if (bond->curr_active_slave && bond->alb_info.primary_is_promisc) {

		dev_set_promiscuity(bond->curr_active_slave->dev, -1);

	if (curr_active && bond->alb_info.primary_is_promisc) {

		dev_set_promiscuity(curr_active->dev, -1);

		bond->alb_info.primary_is_promisc = 0;

		bond->alb_info.rlb_promisc_timeout_counter = 0;

	}



	swap_slave = bond->curr_active_slave;

	swap_slave = curr_active;

	rcu_assign_pointer(bond->curr_active_slave, new_slave);



	if (!new_slave || !bond_has_slaves(bond))
@@ -1818,6 +1825,7 @@ int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr) 
{

	struct bonding *bond = netdev_priv(bond_dev);

	struct sockaddr *sa = addr;

	struct slave *curr_active;

	struct slave *swap_slave;

	int res;
@@ -1834,23 +1842,24 @@ int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr) 
	 * Otherwise we'll need to pass the new address to it and handle

	 * duplications.

	 */

	if (!bond->curr_active_slave)

	curr_active = rtnl_dereference(bond->curr_active_slave);

	if (!curr_active)

		return 0;



	swap_slave = bond_slave_has_mac(bond, bond_dev->dev_addr);



	if (swap_slave) {

		alb_swap_mac_addr(swap_slave, bond->curr_active_slave);

		alb_fasten_mac_swap(bond, swap_slave, bond->curr_active_slave);

		alb_swap_mac_addr(swap_slave, curr_active);

		alb_fasten_mac_swap(bond, swap_slave, curr_active);

	} else {

		alb_set_slave_mac_addr(bond->curr_active_slave, bond_dev->dev_addr);

		alb_set_slave_mac_addr(curr_active, bond_dev->dev_addr);



		read_lock(&bond->lock);

		alb_send_learning_packets(bond->curr_active_slave,

		alb_send_learning_packets(curr_active,

					  bond_dev->dev_addr, false);

		if (bond->alb_info.rlb_enabled) {

			/* inform clients mac address has changed */

			rlb_req_update_slave_clients(bond, bond->curr_active_slave);

			rlb_req_update_slave_clients(bond, curr_active);

		}

		read_unlock(&bond->lock);

	}

drivers/net/bonding/bond_main.c

+38 −30
Original line number Diff line number Diff line
@@ -498,11 +498,11 @@ static int bond_set_promiscuity(struct bonding *bond, int inc) 
	int err = 0;



	if (bond_uses_primary(bond)) {

		struct slave *curr_active = bond_deref_active_protected(bond);



		/* write lock already acquired */

		if (bond->curr_active_slave) {

			err = dev_set_promiscuity(bond->curr_active_slave->dev,

						  inc);

		}

		if (curr_active)

			err = dev_set_promiscuity(curr_active->dev, inc);

	} else {

		struct slave *slave;
@@ -524,11 +524,11 @@ static int bond_set_allmulti(struct bonding *bond, int inc) 
	int err = 0;



	if (bond_uses_primary(bond)) {

		struct slave *curr_active = bond_deref_active_protected(bond);



		/* write lock already acquired */

		if (bond->curr_active_slave) {

			err = dev_set_allmulti(bond->curr_active_slave->dev,

					       inc);

		}

		if (curr_active)

			err = dev_set_allmulti(curr_active->dev, inc);

	} else {

		struct slave *slave;
@@ -713,7 +713,7 @@ out: 
static bool bond_should_change_active(struct bonding *bond)

{

	struct slave *prim = bond->primary_slave;

	struct slave *curr = bond->curr_active_slave;

	struct slave *curr = bond_deref_active_protected(bond);



	if (!prim || !curr || curr->link != BOND_LINK_UP)

		return true;
@@ -792,7 +792,11 @@ static bool bond_should_notify_peers(struct bonding *bond) 
 */

void bond_change_active_slave(struct bonding *bond, struct slave *new_active)

{

	struct slave *old_active = bond->curr_active_slave;

	struct slave *old_active;



	old_active = rcu_dereference_protected(bond->curr_active_slave,

					       !new_active ||

					       lockdep_is_held(&bond->curr_slave_lock));



	if (old_active == new_active)

		return;
@@ -900,7 +904,7 @@ void bond_select_active_slave(struct bonding *bond) 
	int rv;



	best_slave = bond_find_best_slave(bond);

	if (best_slave != bond->curr_active_slave) {

	if (best_slave != bond_deref_active_protected(bond)) {

		bond_change_active_slave(bond, best_slave);

		rv = bond_set_carrier(bond);

		if (!rv)
@@ -1531,7 +1535,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) 
		 * anyway (it holds no special properties of the bond device),

		 * so we can change it without calling change_active_interface()

		 */

		if (!bond->curr_active_slave && new_slave->link == BOND_LINK_UP)

		if (!rcu_access_pointer(bond->curr_active_slave) &&

		    new_slave->link == BOND_LINK_UP)

			rcu_assign_pointer(bond->curr_active_slave, new_slave);



		break;
@@ -1602,7 +1607,7 @@ err_detach: 
	vlan_vids_del_by_dev(slave_dev, bond_dev);

	if (bond->primary_slave == new_slave)

		bond->primary_slave = NULL;

	if (bond->curr_active_slave == new_slave) {

	if (rcu_access_pointer(bond->curr_active_slave) == new_slave) {

		block_netpoll_tx();

		write_lock_bh(&bond->curr_slave_lock);

		bond_change_active_slave(bond, NULL);
@@ -1704,9 +1709,9 @@ static int __bond_release_one(struct net_device *bond_dev, 
		bond_is_active_slave(slave) ? "active" : "backup",

		slave_dev->name);



	oldcurrent = bond->curr_active_slave;

	oldcurrent = rcu_access_pointer(bond->curr_active_slave);



	bond->current_arp_slave = NULL;

	RCU_INIT_POINTER(bond->current_arp_slave, NULL);



	if (!all && (!bond->params.fail_over_mac ||

		     BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP)) {
@@ -1878,7 +1883,7 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in 


/*-------------------------------- Monitoring -------------------------------*/





/* called with rcu_read_lock() */

static int bond_miimon_inspect(struct bonding *bond)

{

	int link_state, commit = 0;
@@ -1886,7 +1891,7 @@ static int bond_miimon_inspect(struct bonding *bond) 
	struct slave *slave;

	bool ignore_updelay;



	ignore_updelay = !bond->curr_active_slave ? true : false;

	ignore_updelay = !rcu_dereference(bond->curr_active_slave);



	bond_for_each_slave_rcu(bond, slave, iter) {

		slave->new_link = BOND_LINK_NOCHANGE;
@@ -2046,7 +2051,7 @@ static void bond_miimon_commit(struct bonding *bond) 
				bond_alb_handle_link_change(bond, slave,

							    BOND_LINK_DOWN);



			if (slave == bond->curr_active_slave)

			if (slave == rcu_access_pointer(bond->curr_active_slave))

				goto do_failover;



			continue;
@@ -2416,7 +2421,7 @@ static void bond_loadbalance_arp_mon(struct work_struct *work) 


	rcu_read_lock();



	oldcurrent = ACCESS_ONCE(bond->curr_active_slave);

	oldcurrent = rcu_dereference(bond->curr_active_slave);

	/* see if any of the previous devices are up now (i.e. they have

	 * xmt and rcv traffic). the curr_active_slave does not come into

	 * the picture unless it is null. also, slave->last_link_up is not
@@ -2564,7 +2569,7 @@ static int bond_ab_arp_inspect(struct bonding *bond) 
		 * before being taken out

		 */

		if (!bond_is_active_slave(slave) &&

		    !bond->current_arp_slave &&

		    !rcu_access_pointer(bond->current_arp_slave) &&

		    !bond_time_in_interval(bond, last_rx, 3)) {

			slave->new_link = BOND_LINK_DOWN;

			commit++;
@@ -2607,21 +2612,24 @@ static void bond_ab_arp_commit(struct bonding *bond) 


		case BOND_LINK_UP:

			trans_start = dev_trans_start(slave->dev);

			if (bond->curr_active_slave != slave ||

			    (!bond->curr_active_slave &&

			if (rtnl_dereference(bond->curr_active_slave) != slave ||

			    (!rtnl_dereference(bond->curr_active_slave) &&

			     bond_time_in_interval(bond, trans_start, 1))) {

				struct slave *current_arp_slave;



				current_arp_slave = rtnl_dereference(bond->current_arp_slave);

				slave->link = BOND_LINK_UP;

				if (bond->current_arp_slave) {

				if (current_arp_slave) {

					bond_set_slave_inactive_flags(

						bond->current_arp_slave,

						current_arp_slave,

						BOND_SLAVE_NOTIFY_NOW);

					bond->current_arp_slave = NULL;

					RCU_INIT_POINTER(bond->current_arp_slave, NULL);

				}



				pr_info("%s: link status definitely up for interface %s\n",

					bond->dev->name, slave->dev->name);



				if (!bond->curr_active_slave ||

				if (!rtnl_dereference(bond->curr_active_slave) ||

				    (slave == bond->primary_slave))

					goto do_failover;
@@ -2640,8 +2648,8 @@ static void bond_ab_arp_commit(struct bonding *bond) 
			pr_info("%s: link status definitely down for interface %s, disabling it\n",

				bond->dev->name, slave->dev->name);



			if (slave == bond->curr_active_slave) {

				bond->current_arp_slave = NULL;

			if (slave == rtnl_dereference(bond->curr_active_slave)) {

				RCU_INIT_POINTER(bond->current_arp_slave, NULL);

				goto do_failover;

			}
@@ -3097,8 +3105,8 @@ static int bond_open(struct net_device *bond_dev) 
	if (bond_has_slaves(bond)) {

		read_lock(&bond->curr_slave_lock);

		bond_for_each_slave(bond, slave, iter) {

			if (bond_uses_primary(bond)

				&& (slave != bond->curr_active_slave)) {

			if (bond_uses_primary(bond) &&

			    slave != rcu_access_pointer(bond->curr_active_slave)) {

				bond_set_slave_inactive_flags(slave,

							      BOND_SLAVE_NOTIFY_NOW);

			} else {

drivers/net/bonding/bond_netlink.c

+16 −5
Original line number Diff line number Diff line
@@ -398,20 +398,31 @@ static size_t bond_get_size(const struct net_device *bond_dev) 
		0;

}



static int bond_option_active_slave_get_ifindex(struct bonding *bond)

{

	const struct net_device *slave;

	int ifindex;



	rcu_read_lock();

	slave = bond_option_active_slave_get_rcu(bond);

	ifindex = slave ? slave->ifindex : 0;

	rcu_read_unlock();

	return ifindex;

}



static int bond_fill_info(struct sk_buff *skb,

			  const struct net_device *bond_dev)

{

	struct bonding *bond = netdev_priv(bond_dev);

	struct net_device *slave_dev = bond_option_active_slave_get(bond);

	struct nlattr *targets;

	unsigned int packets_per_slave;

	int i, targets_added;

	int ifindex, i, targets_added;

	struct nlattr *targets;



	if (nla_put_u8(skb, IFLA_BOND_MODE, BOND_MODE(bond)))

		goto nla_put_failure;



	if (slave_dev &&

	    nla_put_u32(skb, IFLA_BOND_ACTIVE_SLAVE, slave_dev->ifindex))

	ifindex = bond_option_active_slave_get_ifindex(bond);

	if (ifindex && nla_put_u32(skb, IFLA_BOND_ACTIVE_SLAVE, ifindex))

		goto nla_put_failure;



	if (nla_put_u32(skb, IFLA_BOND_MIIMON, bond->params.miimon))

drivers/net/bonding/bond_options.c

+1 −6
Original line number Diff line number Diff line
@@ -704,11 +704,6 @@ struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond) 
	return __bond_option_active_slave_get(bond, slave);

}



struct net_device *bond_option_active_slave_get(struct bonding *bond)

{

	return __bond_option_active_slave_get(bond, bond->curr_active_slave);

}



static int bond_option_active_slave_set(struct bonding *bond,

					const struct bond_opt_value *newval)

{
@@ -748,7 +743,7 @@ static int bond_option_active_slave_set(struct bonding *bond, 
		RCU_INIT_POINTER(bond->curr_active_slave, NULL);

		bond_select_active_slave(bond);

	} else {

		struct slave *old_active = bond->curr_active_slave;

		struct slave *old_active = bond_deref_active_protected(bond);

		struct slave *new_active = bond_slave_get_rtnl(slave_dev);



		BUG_ON(!new_active);

drivers/net/bonding/bond_sysfs.c

+2 −1
Original line number Diff line number Diff line
@@ -492,8 +492,9 @@ static ssize_t bonding_show_mii_status(struct device *d, 
				       char *buf)

{

	struct bonding *bond = to_bond(d);

	bool active = !!rcu_access_pointer(bond->curr_active_slave);



	return sprintf(buf, "%s\n", bond->curr_active_slave ? "up" : "down");

	return sprintf(buf, "%s\n", active ? "up" : "down");

}

static DEVICE_ATTR(mii_status, S_IRUGO, bonding_show_mii_status, NULL);

CRA Git | Maintained and supported by SUSTech CRA and CCSE