bonding: move active_slave setting into separate function

Do a bit of refactoring on the way.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: David S. Miller <davem@davemloft.net>

drivers/net/bonding/bond_options.c

@@ -12,6 +12,9 @@
 
 #include <linux/errno.h>
 #include <linux/if.h>
+#include <linux/netdevice.h>
+#include <linux/rwlock.h>
+#include <linux/rcupdate.h>
 #include "bonding.h"
 
 static bool bond_mode_is_valid(int mode)
@@ -53,3 +56,69 @@ int bond_option_mode_set(struct bonding *bond, int mode)
 	bond->params.mode = mode;
 	return 0;
 }
+
+int bond_option_active_slave_set(struct bonding *bond,
+				 struct net_device *slave_dev)
+{
+	int ret = 0;
+
+	if (slave_dev) {
+		if (!netif_is_bond_slave(slave_dev)) {
+			pr_err("Device %s is not bonding slave.\n",
+			       slave_dev->name);
+			return -EINVAL;
+		}
+
+		if (bond->dev != netdev_master_upper_dev_get(slave_dev)) {
+			pr_err("%s: Device %s is not our slave.\n",
+			       bond->dev->name, slave_dev->name);
+			return -EINVAL;
+		}
+	}
+
+	if (!USES_PRIMARY(bond->params.mode)) {
+		pr_err("%s: Unable to change active slave; %s is in mode %d\n",
+		       bond->dev->name, bond->dev->name, bond->params.mode);
+		return -EINVAL;
+	}
+
+	block_netpoll_tx();
+	read_lock(&bond->lock);
+	write_lock_bh(&bond->curr_slave_lock);
+
+	/* check to see if we are clearing active */
+	if (!slave_dev) {
+		pr_info("%s: Clearing current active slave.\n",
+		bond->dev->name);
+		rcu_assign_pointer(bond->curr_active_slave, NULL);
+		bond_select_active_slave(bond);
+	} else {
+		struct slave *old_active = bond->curr_active_slave;
+		struct slave *new_active = bond_slave_get_rtnl(slave_dev);
+
+		BUG_ON(!new_active);
+
+		if (new_active == old_active) {
+			/* do nothing */
+			pr_info("%s: %s is already the current active slave.\n",
+				bond->dev->name, new_active->dev->name);
+		} else {
+			if (old_active && (new_active->link == BOND_LINK_UP) &&
+			    IS_UP(new_active->dev)) {
+				pr_info("%s: Setting %s as active slave.\n",
+					bond->dev->name, new_active->dev->name);
+				bond_change_active_slave(bond, new_active);
+			} else {
+				pr_err("%s: Could not set %s as active slave; either %s is down or the link is down.\n",
+				       bond->dev->name, new_active->dev->name,
+				       new_active->dev->name);
+				ret = -EINVAL;
+			}
+		}
+	}
+
+	write_unlock_bh(&bond->curr_slave_lock);
+	read_unlock(&bond->lock);
+	unblock_netpoll_tx();
+	return ret;
+}

drivers/net/bonding/bond_sysfs.c

@@ -1235,81 +1235,33 @@ static ssize_t bonding_store_active_slave(struct device *d,
 					  struct device_attribute *attr,
 					  const char *buf, size_t count)
 {
-	struct slave *slave, *old_active, *new_active;
+	int ret;
 	struct bonding *bond = to_bond(d);
-	struct list_head *iter;
 	char ifname[IFNAMSIZ];
+	struct net_device *dev;
 
 	if (!rtnl_trylock())
 		return restart_syscall();
 
-	old_active = new_active = NULL;
-	block_netpoll_tx();
-	read_lock(&bond->lock);
-	write_lock_bh(&bond->curr_slave_lock);
-
-	if (!USES_PRIMARY(bond->params.mode)) {
-		pr_info("%s: Unable to change active slave; %s is in mode %d\n",
-			bond->dev->name, bond->dev->name, bond->params.mode);
-		goto out;
-	}
-
 	sscanf(buf, "%15s", ifname); /* IFNAMSIZ */
-
-	/* check to see if we are clearing active */
 	if (!strlen(ifname) || buf[0] == '\n') {
-		pr_info("%s: Clearing current active slave.\n",
+		dev = NULL;
-			bond->dev->name);
+	} else {
-		rcu_assign_pointer(bond->curr_active_slave, NULL);
+		dev = __dev_get_by_name(dev_net(bond->dev), ifname);
-		bond_select_active_slave(bond);
+		if (!dev) {
-		goto out;
+			ret = -ENODEV;
-	}
+			goto out;
-
-	bond_for_each_slave(bond, slave, iter) {
-		if (strncmp(slave->dev->name, ifname, IFNAMSIZ) == 0) {
-			old_active = bond->curr_active_slave;
-			new_active = slave;
-			if (new_active == old_active) {
-				/* do nothing */
-				pr_info("%s: %s is already the current"
-					" active slave.\n",
-					bond->dev->name,
-					slave->dev->name);
-				goto out;
-			} else {
-				if ((new_active) &&
-				    (old_active) &&
-				    (new_active->link == BOND_LINK_UP) &&
-				    IS_UP(new_active->dev)) {
-					pr_info("%s: Setting %s as active"
-						" slave.\n",
-						bond->dev->name,
-						slave->dev->name);
-					bond_change_active_slave(bond,
-								 new_active);
-				} else {
-					pr_info("%s: Could not set %s as"
-						" active slave; either %s is"
-						" down or the link is down.\n",
-						bond->dev->name,
-						slave->dev->name,
-						slave->dev->name);
-				}
-				goto out;
-			}
 		}
 	}
 
-	pr_info("%s: Unable to set %.*s as active slave.\n",
+	ret = bond_option_active_slave_set(bond, dev);
-		bond->dev->name, (int)strlen(buf) - 1, buf);
+	if (!ret)
- out:
+		ret = count;
-	write_unlock_bh(&bond->curr_slave_lock);
-	read_unlock(&bond->lock);
-	unblock_netpoll_tx();
 
+ out:
 	rtnl_unlock();
 
-	return count;
+	return ret;
 
 }
 static DEVICE_ATTR(active_slave, S_IRUGO | S_IWUSR,

drivers/net/bonding/bonding.h

@@ -427,6 +427,7 @@ unsigned int bond_get_num_tx_queues(void);
 int bond_netlink_init(void);
 void bond_netlink_fini(void);
 int bond_option_mode_set(struct bonding *bond, int mode);
+int bond_option_active_slave_set(struct bonding *bond, struct net_device *slave_dev);
 
 struct bond_net {
 	struct net *		net;	/* Associated network namespace */