bonding: add helper function bond_get_targets_ip(targets, ip)

Add function bond_get_targets_ip(targets, ip) which searches through
targets array of ips (arp_targets) and returns the position of first
match. If ip == 0, returns the first free slot. On failure to find the
ip or free slot, return -1.

Use it to verify if the arp we've received is valid and in sysfs.

v1->v2:
Fix "[2/6] bonding: add helper function bond_get_targets_ip(targets, ip)",
per Nikolay's advice, to verify if source ip != 0.0.0.0, otherwise we might
update 'null' arp_ip_targets' last_rx. Also, address style.

Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

1 files changed, 21 insertions, 33 deletions

diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index f8bee4c0cbf1..ece57f146a60 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -591,7 +591,7 @@ static ssize_t bonding_store_arp_targets(struct device *d,
 					 const char *buf, size_t count)
 {
 	__be32 newtarget;
-	int i = 0, done = 0, ret = count;
+	int i = 0, ret = -EINVAL;
 	struct bonding *bond = to_bond(d);
 	__be32 *targets;
 
@@ -602,57 +602,44 @@ static ssize_t bonding_store_arp_targets(struct device *d,
 		if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) {
 			pr_err("%s: invalid ARP target %pI4 specified for addition\n",
 			       bond->dev->name, &newtarget);
-			ret = -EINVAL;
 			goto out;
 		}
-		/* look for an empty slot to put the target in, and check for dupes */
-		for (i = 0; (i < BOND_MAX_ARP_TARGETS) && !done; i++) {
-			if (targets[i] == newtarget) { /* duplicate */
-				pr_err("%s: ARP target %pI4 is already present\n",
-				       bond->dev->name, &newtarget);
-				ret = -EINVAL;
-				goto out;
-			}
-			if (targets[i] == 0) {
-				pr_info("%s: adding ARP target %pI4.\n",
-					bond->dev->name, &newtarget);
-				done = 1;
-				targets[i] = newtarget;
-			}
+
+		if (bond_get_targets_ip(targets, newtarget) != -1) { /* dup */
+			pr_err("%s: ARP target %pI4 is already present\n",
+			       bond->dev->name, &newtarget);
+			goto out;
 		}
-		if (!done) {
+
+		i = bond_get_targets_ip(targets, 0); /* first free slot */
+		if (i == -1) {
 			pr_err("%s: ARP target table is full!\n",
 			       bond->dev->name);
-			ret = -EINVAL;
 			goto out;
 		}
 
+		pr_info("%s: adding ARP target %pI4.\n", bond->dev->name,
+			 &newtarget);
+		targets[i] = newtarget;
 	} else if (buf[0] == '-')	{
 		if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) {
 			pr_err("%s: invalid ARP target %pI4 specified for removal\n",
 			       bond->dev->name, &newtarget);
-			ret = -EINVAL;
 			goto out;
 		}
 
-		for (i = 0; (i < BOND_MAX_ARP_TARGETS) && !done; i++) {
-			if (targets[i] == newtarget) {
-				int j;
-				pr_info("%s: removing ARP target %pI4.\n",
-					bond->dev->name, &newtarget);
-				for (j = i; (j < (BOND_MAX_ARP_TARGETS-1)) && targets[j+1]; j++)
-					targets[j] = targets[j+1];
-
-				targets[j] = 0;
-				done = 1;
-			}
-		}
-		if (!done) {
+		i = bond_get_targets_ip(targets, newtarget);
+		if (i == -1) {
 			pr_info("%s: unable to remove nonexistent ARP target %pI4.\n",
 				bond->dev->name, &newtarget);
-			ret = -EINVAL;
 			goto out;
 		}
+
+		pr_info("%s: removing ARP target %pI4.\n", bond->dev->name,
+			&newtarget);
+		for (; (i < BOND_MAX_ARP_TARGETS-1) && targets[i+1]; i++)
+			targets[i] = targets[i+1];
+		targets[i] = 0;
 	} else {
 		pr_err("no command found in arp_ip_targets file for bond %s. Use +<addr> or -<addr>.\n",
 		       bond->dev->name);
@@ -660,6 +647,7 @@ static ssize_t bonding_store_arp_targets(struct device *d,
 		goto out;
 	}
 
+	ret = count;
 out:
 	return ret;
 }