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>

3 files changed, 46 insertions, 45 deletions

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3b31c19972d4..976d28e3498a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2610,19 +2610,16 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 
 static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32 sip, __be32 tip)
 {
-        int i;
-        __be32 *targets = bond->params.arp_targets;
+        if (!sip || !bond_has_this_ip(bond, tip)) {
+                pr_debug("bva: sip %pI4 tip %pI4 not found\n", &sip, &tip);
+                return;
+        }
 
-        for (i = 0; (i < BOND_MAX_ARP_TARGETS) && targets[i]; i++) {
-                pr_debug("bva: sip %pI4 tip %pI4 t[%d] %pI4 bhti(tip) %d\n",
-                         &sip, &tip, i, &targets[i],
-                         bond_has_this_ip(bond, tip));
-                if (sip == targets[i]) {
-                        if (bond_has_this_ip(bond, tip))
-                                slave->last_arp_rx = jiffies;
-                        return;
-                }
+        if (bond_get_targets_ip(bond->params.arp_targets, sip) == -1) {
+                pr_debug("bva: sip %pI4 not found in targets\n", &sip);
+                return;
         }
+        slave->last_arp_rx = jiffies;
 }
 
 static int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
@@ -4839,7 +4836,7 @@ static int __net_init bond_net_init(struct net *net)
 
         bond_create_proc_dir(bn);
         bond_create_sysfs(bn);
-        
+
         return 0;
 }
 
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;
 }
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index c990b42cf7a1..486e532f77e4 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -464,6 +464,22 @@ static inline struct slave *bond_slave_has_mac(struct bonding *bond,
         return NULL;
 }
 
+/* Check if the ip is present in arp ip list, or first free slot if ip == 0
+ * Returns -1 if not found, index if found
+ */
+static inline int bond_get_targets_ip(__be32 *targets, __be32 ip)
+{
+        int i;
+
+        for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
+                if (targets[i] == ip)
+                        return i;
+                else if (targets[i] == 0)
+                        break;
+
+        return -1;
+}
+
 /* exported from bond_main.c */
 extern int bond_net_id;
 extern const struct bond_parm_tbl bond_lacp_tbl[];