zcip: fix link-local IP conflict detection

During link-local IP resolution, if a regular ARP request is received
during the ARP probe period, it will incorrectly cause a target IP
conflict.  This then leads to a new IP being picked unnecessarily.

Per RFC 3927, section 2.2.1, we should flag a target IP conflict only if
the source IP is null, the target IP matches our IP, and the source
hw addr does not match our hw addr.

function                                             old     new   delta
zcip_main                                           1354    1322     -32

Signed-off-by: Ken Sharp <ken.sharp@ni.com>
Signed-off-by: Ben Shelton <ben.shelton@ni.com>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

diff --git a/networking/zcip.c b/networking/zcip.c
index 7314ff8db7509b57f75eef48d50b2ade4cb911bb..45d1f7c1c25cf148825052075a0d20ea15eb9751 100644 (file)
--- a/networking/zcip.c
+++ b/networking/zcip.c
@@ -366,11 +366,11 @@ int zcip_main(int argc UNUSED_PARAM, char **argv)
                                        nprobes++;
                                        VDBG("probe/%u %s@%s\n",
                                                        nprobes, argv_intf, inet_ntoa(ip));
+                                       timeout_ms = PROBE_MIN * 1000;
+                                       timeout_ms += random_delay_ms(PROBE_MAX - PROBE_MIN);
                                        arp(/* ARPOP_REQUEST, */
                                                        /* &eth_addr, */ null_ip,
                                                        &null_addr, ip);
-                                       timeout_ms = PROBE_MIN * 1000;
-                                       timeout_ms += random_delay_ms(PROBE_MAX - PROBE_MIN);
                                }
                                else {
                                        // Switch to announce state.
@@ -378,10 +378,10 @@ int zcip_main(int argc UNUSED_PARAM, char **argv)
                                        nclaims = 0;
                                        VDBG("announce/%u %s@%s\n",
                                                        nclaims, argv_intf, inet_ntoa(ip));
+                                       timeout_ms = ANNOUNCE_INTERVAL * 1000;
                                        arp(/* ARPOP_REQUEST, */
                                                        /* &eth_addr, */ ip,
                                                        &eth_addr, ip);
-                                       timeout_ms = ANNOUNCE_INTERVAL * 1000;
                                }
                                break;
                        case RATE_LIMIT_PROBE:
@@ -391,10 +391,10 @@ int zcip_main(int argc UNUSED_PARAM, char **argv)
                                nclaims = 0;
                                VDBG("announce/%u %s@%s\n",
                                                nclaims, argv_intf, inet_ntoa(ip));
+                               timeout_ms = ANNOUNCE_INTERVAL * 1000;
                                arp(/* ARPOP_REQUEST, */
                                                /* &eth_addr, */ ip,
                                                &eth_addr, ip);
-                               timeout_ms = ANNOUNCE_INTERVAL * 1000;
                                break;
                        case ANNOUNCE:
                                // timeouts in the ANNOUNCE state mean no conflicting ARP packets
@@ -403,10 +403,10 @@ int zcip_main(int argc UNUSED_PARAM, char **argv)
                                        nclaims++;
                                        VDBG("announce/%u %s@%s\n",
                                                        nclaims, argv_intf, inet_ntoa(ip));
+                                       timeout_ms = ANNOUNCE_INTERVAL * 1000;
                                        arp(/* ARPOP_REQUEST, */
                                                        /* &eth_addr, */ ip,
                                                        &eth_addr, ip);
-                                       timeout_ms = ANNOUNCE_INTERVAL * 1000;
                                }
                                else {
                                        // Switch to monitor state.
@@ -495,22 +495,28 @@ int zcip_main(int argc UNUSED_PARAM, char **argv)
                        }
 #endif
                        if (p.arp.arp_op != htons(ARPOP_REQUEST)
-                        && p.arp.arp_op != htons(ARPOP_REPLY))
+                        && p.arp.arp_op != htons(ARPOP_REPLY)
+                       ) {
                                continue;
+                       }
 
                        source_ip_conflict = 0;
                        target_ip_conflict = 0;
 
-                       if (memcmp(p.arp.arp_spa, &ip.s_addr, sizeof(struct in_addr)) == 0
-                        && memcmp(&p.arp.arp_sha, &eth_addr, ETH_ALEN) != 0
-                       ) {
-                               source_ip_conflict = 1;
-                       }
-                       if (p.arp.arp_op == htons(ARPOP_REQUEST)
-                        && memcmp(p.arp.arp_tpa, &ip.s_addr, sizeof(struct in_addr)) == 0
-                        && memcmp(&p.arp.arp_tha, &eth_addr, ETH_ALEN) != 0
-                       ) {
-                               target_ip_conflict = 1;
+                       if (memcmp(&p.arp.arp_sha, &eth_addr, ETH_ALEN) != 0) {
+                               if (memcmp(p.arp.arp_spa, &ip.s_addr, sizeof(struct in_addr))) {
+                                       /* A probe or reply with source_ip == chosen ip */
+                                       source_ip_conflict = 1;
+                               }
+                               if (p.arp.arp_op == htons(ARPOP_REQUEST)
+                                && memcmp(p.arp.arp_spa, &null_ip, sizeof(struct in_addr)) == 0
+                                && memcmp(p.arp.arp_tpa, &ip.s_addr, sizeof(struct in_addr)) == 0
+                               ) {
+                                       /* A probe with source_ip == 0.0.0.0, target_ip == chosen ip:
+                                        * another host trying to claim this ip!
+                                        */
+                                       target_ip_conflict = 1;
+                               }
                        }
 
                        VDBG("state = %d, source ip conflict = %d, target ip conflict = %d\n",