zcip: another code shrink
authorDenys Vlasenko <vda.linux@googlemail.com>
Tue, 4 Aug 2015 12:30:31 +0000 (14:30 +0200)
committerDenys Vlasenko <vda.linux@googlemail.com>
Tue, 4 Aug 2015 12:30:31 +0000 (14:30 +0200)
function                                             old     new   delta
send_arp_request                                       -     185    +185
zcip_main                                           1273    1272      -1
pick_nip                                              40       -     -40
arp                                                  185       -    -185
------------------------------------------------------------------------------
(add/remove: 1/2 grow/shrink: 0/1 up/down: 185/-226)          Total: -41 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
networking/zcip.c

index 5877a8317ba126bd4cc5ef76955b508e21f84584..a167c7e9168aa72f9292c7bbc5fd52a4934f1d6e 100644 (file)
@@ -90,7 +90,7 @@ enum {
 
 struct globals {
        struct sockaddr iface_sockaddr;
-       struct ether_addr eth_addr;
+       struct ether_addr our_ethaddr;
        uint32_t localnet_ip;
 } FIX_ALIASING;
 #define G (*(struct globals*)&bb_common_bufsiz1)
@@ -121,14 +121,14 @@ static const char *nip_to_a(uint32_t nip)
 /**
  * Broadcast an ARP packet.
  */
-static void arp(
+static void send_arp_request(
        /* int op, - always ARPOP_REQUEST */
-       /* const struct ether_addr *source_eth, - always &G.eth_addr */
+       /* const struct ether_addr *source_eth, - always &G.our_ethaddr */
                                        uint32_t source_nip,
        const struct ether_addr *target_eth, uint32_t target_nip)
 {
        enum { op = ARPOP_REQUEST };
-#define source_eth (&G.eth_addr)
+#define source_eth (&G.our_ethaddr)
 
        struct arp_packet p;
        memset(&p, 0, sizeof(p));
@@ -205,35 +205,34 @@ static ALWAYS_INLINE unsigned random_delay_ms(unsigned secs)
 int zcip_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int zcip_main(int argc UNUSED_PARAM, char **argv)
 {
-       int state;
        char *r_opt;
        const char *l_opt = "169.254.0.0";
+       int state;
+       int nsent;
        unsigned opts;
 
-       // ugly trick, but I want these zeroed in one go
+       // Ugly trick, but I want these zeroed in one go
        struct {
-               const struct ether_addr null_addr;
+               const struct ether_addr null_ethaddr;
                struct ifreq ifr;
                uint32_t chosen_nip;
+               int conflicts;
                int timeout_ms; // must be signed
-               unsigned conflicts;
-               unsigned nsent;
                int verbose;
        } L;
-#define null_addr  (L.null_addr )
-#define ifr        (L.ifr       )
-#define chosen_nip (L.chosen_nip)
-#define timeout_ms (L.timeout_ms)
-#define conflicts  (L.conflicts )
-#define nsent      (L.nsent     )
-#define verbose    (L.verbose   )
+#define null_ethaddr (L.null_ethaddr)
+#define ifr          (L.ifr         )
+#define chosen_nip   (L.chosen_nip  )
+#define conflicts    (L.conflicts   )
+#define timeout_ms   (L.timeout_ms  )
+#define verbose      (L.verbose     )
 
        memset(&L, 0, sizeof(L));
        INIT_G();
 
 #define FOREGROUND (opts & 1)
 #define QUIT       (opts & 2)
-       // parse commandline: prog [options] ifname script
+       // Parse commandline: prog [options] ifname script
        // exactly 2 args; -v accumulates and implies -f
        opt_complementary = "=2:vv:vf";
        opts = getopt32(argv, "fqr:l:v", &r_opt, &l_opt, &verbose);
@@ -242,7 +241,7 @@ int zcip_main(int argc UNUSED_PARAM, char **argv)
        if (!FOREGROUND)
                bb_daemonize_or_rexec(0 /*was: DAEMON_CHDIR_ROOT*/, argv);
 #endif
-       // open an ARP socket
+       // Open an ARP socket
        // (need to do it before openlog to prevent openlog from taking
        // fd 3 (sock_fd==3))
        xmove_fd(xsocket(AF_PACKET, SOCK_PACKET, htons(ETH_P_ARP)), sock_fd);
@@ -282,26 +281,26 @@ int zcip_main(int argc UNUSED_PARAM, char **argv)
 
        xsetenv("interface", argv_intf);
 
-       // initialize the interface (modprobe, ifup, etc)
+       // Initialize the interface (modprobe, ifup, etc)
        if (run(argv, "init", 0))
                return EXIT_FAILURE;
 
-       // initialize G.iface_sockaddr
+       // Initialize G.iface_sockaddr
        // G.iface_sockaddr is: { u16 sa_family; u8 sa_data[14]; }
        //memset(&G.iface_sockaddr, 0, sizeof(G.iface_sockaddr));
        //TODO: are we leaving sa_family == 0 (AF_UNSPEC)?!
        safe_strncpy(G.iface_sockaddr.sa_data, argv_intf, sizeof(G.iface_sockaddr.sa_data));
 
-       // bind to the interface's ARP socket
+       // Bind to the interface's ARP socket
        xbind(sock_fd, &G.iface_sockaddr, sizeof(G.iface_sockaddr));
 
-       // get the interface's ethernet address
+       // Get the interface's ethernet address
        //memset(&ifr, 0, sizeof(ifr));
        strncpy_IFNAMSIZ(ifr.ifr_name, argv_intf);
        xioctl(sock_fd, SIOCGIFHWADDR, &ifr);
-       memcpy(&G.eth_addr, &ifr.ifr_hwaddr.sa_data, ETH_ALEN);
+       memcpy(&G.our_ethaddr, &ifr.ifr_hwaddr.sa_data, ETH_ALEN);
 
-       // start with some stable ip address, either a function of
+       // Start with some stable ip address, either a function of
        // the hardware address or else the last address we used.
        // we are taking low-order four bytes, as top-order ones
        // aren't random enough.
@@ -309,17 +308,14 @@ int zcip_main(int argc UNUSED_PARAM, char **argv)
        // depending on when we detect conflicts.
        {
                uint32_t t;
-               move_from_unaligned32(t, ((char *)&G.eth_addr + 2));
+               move_from_unaligned32(t, ((char *)&G.our_ethaddr + 2));
                srand(t);
        }
-       if (chosen_nip == 0)
-               chosen_nip = pick_nip();
-
        // FIXME cases to handle:
        //  - zcip already running!
        //  - link already has local address... just defend/update
 
-       // daemonize now; don't delay system startup
+       // Daemonize now; don't delay system startup
        if (!FOREGROUND) {
 #if BB_MMU
                bb_daemonize(0 /*was: DAEMON_CHDIR_ROOT*/);
@@ -327,14 +323,14 @@ int zcip_main(int argc UNUSED_PARAM, char **argv)
                bb_info_msg("start, interface %s", argv_intf);
        }
 
-       // run the dynamic address negotiation protocol,
+       // Run the dynamic address negotiation protocol,
        // restarting after address conflicts:
        //  - start with some address we want to try
        //  - short random delay
        //  - arp probes to see if another host uses it
-       //    00:04:e2:64:23:c2 > ff:ff:ff:ff:ff:ff, ARP (0x0806): arp who-has 169.254.194.171 tell 0.0.0.0
+       //    00:04:e2:64:23:c2 > ff:ff:ff:ff:ff:ff: arp who-has 169.254.194.171 tell 0.0.0.0
        //  - arp announcements that we're claiming it
-       //    00:04:e2:64:23:c2 > ff:ff:ff:ff:ff:ff, ARP (0x0806): arp who-has 169.254.194.171 (00:04:e2:64:23:c2) tell 169.254.194.171
+       //    00:04:e2:64:23:c2 > ff:ff:ff:ff:ff:ff: arp who-has 169.254.194.171 (00:04:e2:64:23:c2) tell 169.254.194.171
        //  - use it
        //  - defend it, within limits
        // exit if:
@@ -342,73 +338,73 @@ int zcip_main(int argc UNUSED_PARAM, char **argv)
        //   run "<script> config", then exit with exitcode 0
        // - poll error (when does this happen?)
        // - read error (when does this happen?)
-       // - sendto error (in arp()) (when does this happen?)
+       // - sendto error (in send_arp_request()) (when does this happen?)
        // - revents & POLLERR (link down). run "<script> deconfig" first
+       if (chosen_nip == 0) {
+ new_nip_and_PROBE:
+               chosen_nip = pick_nip();
+       }
+       nsent = 0;
        state = PROBE;
        while (1) {
                struct pollfd fds[1];
                unsigned deadline_us;
                struct arp_packet p;
                int ip_conflict;
+               int n;
 
                fds[0].fd = sock_fd;
                fds[0].events = POLLIN;
                fds[0].revents = 0;
 
-               // poll, being ready to adjust current timeout
+               // Poll, being ready to adjust current timeout
                if (!timeout_ms) {
                        timeout_ms = random_delay_ms(PROBE_WAIT);
                        // FIXME setsockopt(sock_fd, SO_ATTACH_FILTER, ...) to
                        // make the kernel filter out all packets except
                        // ones we'd care about.
                }
-               // set deadline_us to the point in time when we timeout
+               // Set deadline_us to the point in time when we timeout
                deadline_us = MONOTONIC_US() + timeout_ms * 1000;
 
                VDBG("...wait %d %s nsent=%u\n",
                                timeout_ms, argv_intf, nsent);
 
-               switch (safe_poll(fds, 1, timeout_ms)) {
-
-               default:
+               n = safe_poll(fds, 1, timeout_ms);
+               if (n < 0) {
                        //bb_perror_msg("poll"); - done in safe_poll
                        return EXIT_FAILURE;
-
-               // timeout
-               case 0:
-                       VDBG("state = %d\n", state);
+               }
+               if (n == 0) { // timed out?
+                       VDBG("state:%d\n", state);
                        switch (state) {
                        case PROBE:
-                               // timeouts in the PROBE state mean no conflicting ARP packets
-                               // have been received, so we can progress through the states
+                               // No conflicting ARP packets were seen:
+                               // we can progress through the states
                                if (nsent < PROBE_NUM) {
                                        nsent++;
                                        VDBG("probe/%u %s@%s\n",
                                                        nsent, argv_intf, nip_to_a(chosen_nip));
                                        timeout_ms = PROBE_MIN * 1000;
                                        timeout_ms += random_delay_ms(PROBE_MAX - PROBE_MIN);
-                                       arp(/* ARPOP_REQUEST, */
-                                                       /* &G.eth_addr, */ 0,
-                                                       &null_addr, chosen_nip);
-                                       break;
+                                       send_arp_request(0, &null_ethaddr, chosen_nip);
+                                       continue;
                                }
                                // Switch to announce state
                                nsent = 0;
                                state = ANNOUNCE;
                                goto send_announce;
                        case ANNOUNCE:
-                               // timeouts in the ANNOUNCE state mean no conflicting ARP packets
-                               // have been received, so we can progress through the states
+                               // No conflicting ARP packets were seen:
+                               // we can progress through the states
                                if (nsent < ANNOUNCE_NUM) {
  send_announce:
                                        nsent++;
                                        VDBG("announce/%u %s@%s\n",
                                                        nsent, argv_intf, nip_to_a(chosen_nip));
                                        timeout_ms = ANNOUNCE_INTERVAL * 1000;
-                                       arp(/* ARPOP_REQUEST, */
-                                                       /* &G.eth_addr, */ chosen_nip,
-                                                       &G.eth_addr, chosen_nip);
-                                       break;
+                                       send_arp_request(chosen_nip, &G.our_ethaddr, chosen_nip);
+                                       continue;
                                }
                                // Switch to monitor state
                                // FIXME update filters
@@ -416,124 +412,117 @@ int zcip_main(int argc UNUSED_PARAM, char **argv)
                                // NOTE: all other exit paths should deconfig...
                                if (QUIT)
                                        return EXIT_SUCCESS;
-                               conflicts = 0;
-                               timeout_ms = -1; // Never timeout in the monitor state.
-                               state = MONITOR;
-                               break;
-                       case DEFEND:
+                               // fall through: switch_to_MONITOR
+                       default:
+                       // case DEFEND:
+                       // case MONITOR: (shouldn't happen, MONITOR timeout is infinite)
                                // Defend period ended with no ARP replies - we won
-                               conflicts = 0;
-                               timeout_ms = -1;
+                               timeout_ms = -1; // never timeout in monitor state
                                state = MONITOR;
-                               break;
-                       } // switch (state)
-                       break; // case 0 (timeout)
-
-               // packets arriving, or link went down
-               case 1:
-                       // We need to adjust the timeout in case we didn't receive
-                       // a conflicting packet.
-                       if (timeout_ms > 0) {
-                               unsigned diff = deadline_us - MONOTONIC_US();
-                               if ((int)(diff) < 0) {
-                                       // Current time is greater than the expected timeout time.
-                                       diff = 0;
-                               }
-                               VDBG("adjusting timeout\n");
-                               timeout_ms = (diff / 1000) | 1; // never 0
-                       }
-
-                       if ((fds[0].revents & POLLIN) == 0) {
-                               if (fds[0].revents & POLLERR) {
-                                       // FIXME: links routinely go down;
-                                       // this shouldn't necessarily exit.
-                                       bb_error_msg("iface %s is down", argv_intf);
-                                       if (state >= MONITOR) {
-                                               // only if we are in MONITOR or DEFEND
-                                               run(argv, "deconfig", chosen_nip);
-                                       }
-                                       return EXIT_FAILURE;
-                               }
                                continue;
                        }
+               }
 
-                       // read ARP packet
-                       if (safe_read(sock_fd, &p, sizeof(p)) < 0) {
-                               bb_perror_msg_and_die(bb_msg_read_error);
+               // Packet arrived, or link went down.
+               // We need to adjust the timeout in case we didn't receive
+               // a conflicting packet.
+               if (timeout_ms > 0) {
+                       unsigned diff = deadline_us - MONOTONIC_US();
+                       if ((int)(diff) < 0) {
+                               // Current time is greater than the expected timeout time.
+                               diff = 0;
                        }
+                       VDBG("adjusting timeout\n");
+                       timeout_ms = (diff / 1000) | 1; // never 0
+               }
 
-                       if (p.eth.ether_type != htons(ETHERTYPE_ARP))
-                               continue;
-                       if (p.arp.arp_op != htons(ARPOP_REQUEST)
-                        && p.arp.arp_op != htons(ARPOP_REPLY)
-                       ) {
-                               continue;
+               if ((fds[0].revents & POLLIN) == 0) {
+                       if (fds[0].revents & POLLERR) {
+                               // FIXME: links routinely go down;
+                               // this shouldn't necessarily exit.
+                               bb_error_msg("iface %s is down", argv_intf);
+                               if (state >= MONITOR) {
+                                       // Only if we are in MONITOR or DEFEND
+                                       run(argv, "deconfig", chosen_nip);
+                               }
+                               return EXIT_FAILURE;
                        }
+                       continue;
+               }
+
+               // Read ARP packet
+               if (safe_read(sock_fd, &p, sizeof(p)) < 0) {
+                       bb_perror_msg_and_die(bb_msg_read_error);
+               }
+
+               if (p.eth.ether_type != htons(ETHERTYPE_ARP))
+                       continue;
+               if (p.arp.arp_op != htons(ARPOP_REQUEST)
+                && p.arp.arp_op != htons(ARPOP_REPLY)
+               ) {
+                       continue;
+               }
 #ifdef DEBUG
-                       {
-                               struct ether_addr *sha = (struct ether_addr *) p.arp.arp_sha;
-                               struct ether_addr *tha = (struct ether_addr *) p.arp.arp_tha;
-                               struct in_addr *spa = (struct in_addr *) p.arp.arp_spa;
-                               struct in_addr *tpa = (struct in_addr *) p.arp.arp_tpa;
-                               VDBG("source=%s %s\n", ether_ntoa(sha), inet_ntoa(*spa));
-                               VDBG("target=%s %s\n", ether_ntoa(tha), inet_ntoa(*tpa));
-                       }
+               {
+                       struct ether_addr *sha = (struct ether_addr *) p.arp.arp_sha;
+                       struct ether_addr *tha = (struct ether_addr *) p.arp.arp_tha;
+                       struct in_addr *spa = (struct in_addr *) p.arp.arp_spa;
+                       struct in_addr *tpa = (struct in_addr *) p.arp.arp_tpa;
+                       VDBG("source=%s %s\n", ether_ntoa(sha), inet_ntoa(*spa));
+                       VDBG("target=%s %s\n", ether_ntoa(tha), inet_ntoa(*tpa));
+               }
 #endif
-                       ip_conflict = 0;
-                       if (memcmp(&p.arp.arp_sha, &G.eth_addr, ETH_ALEN) != 0) {
-                               if (memcmp(p.arp.arp_spa, &chosen_nip, 4) == 0) {
-                                       // A probe or reply with source_ip == chosen ip
-                                       ip_conflict = 1;
-                               }
-                               if (p.arp.arp_op == htons(ARPOP_REQUEST)
-                                && memcmp(p.arp.arp_spa, &const_int_0, 4) == 0
-                                && memcmp(p.arp.arp_tpa, &chosen_nip, 4) == 0
-                               ) {
-                                       // A probe with source_ip == 0.0.0.0, target_ip == chosen ip:
-                                       // another host trying to claim this ip!
-                                       ip_conflict |= 2;
-                               }
+               ip_conflict = 0;
+               if (memcmp(&p.arp.arp_sha, &G.our_ethaddr, ETH_ALEN) != 0) {
+                       if (memcmp(p.arp.arp_spa, &chosen_nip, 4) == 0) {
+                               // A probe or reply with source_ip == chosen ip
+                               ip_conflict = 1;
                        }
-                       VDBG("state:%d ip_conflict:%d\n", state, ip_conflict);
-                       if (!ip_conflict)
-                               break;
-
-                       // Either src or target IP conflict exists
-                       if (state <= ANNOUNCE) {
-                               // PROBE or ANNOUNCE
-                               conflicts++;
-                               timeout_ms = PROBE_MIN * 1000
-                                       + CONFLICT_MULTIPLIER * random_delay_ms(conflicts);
-                               chosen_nip = pick_nip();
-                               nsent = 0;
-                               state = PROBE;
-                               break;
+                       if (p.arp.arp_op == htons(ARPOP_REQUEST)
+                        && memcmp(p.arp.arp_spa, &const_int_0, 4) == 0
+                        && memcmp(p.arp.arp_tpa, &chosen_nip, 4) == 0
+                       ) {
+                               // A probe with source_ip == 0.0.0.0, target_ip == chosen ip:
+                               // another host trying to claim this ip!
+                               ip_conflict |= 2;
                        }
-                       // MONITOR or DEFEND: only src IP conflict is a problem
-                       if (ip_conflict & 1) {
-                               if (state == MONITOR) {
-                                       // Src IP conflict, defend with a single ARP probe
-                                       VDBG("monitor conflict - defending\n");
-                                       timeout_ms = DEFEND_INTERVAL * 1000;
-                                       state = DEFEND;
-                                       arp(/* ARPOP_REQUEST, */
-                                               /* &G.eth_addr, */ chosen_nip,
-                                               &G.eth_addr, chosen_nip);
-                                       break;
-                               }
-                               // state == DEFEND
-                               // Another src IP conflict, start over
-                               VDBG("defend conflict - starting over\n");
-                               run(argv, "deconfig", chosen_nip);
-
-                               // restart the whole protocol
-                               timeout_ms = 0;
-                               chosen_nip = pick_nip();
-                               nsent = 0;
-                               state = PROBE;
+               }
+               VDBG("state:%d ip_conflict:%d\n", state, ip_conflict);
+               if (!ip_conflict)
+                       continue;
+
+               // Either src or target IP conflict exists
+               if (state <= ANNOUNCE) {
+                       // PROBE or ANNOUNCE
+                       conflicts++;
+                       timeout_ms = PROBE_MIN * 1000
+                               + CONFLICT_MULTIPLIER * random_delay_ms(conflicts);
+                       goto new_nip_and_PROBE;
+               }
+
+               // MONITOR or DEFEND: only src IP conflict is a problem
+               if (ip_conflict & 1) {
+                       if (state == MONITOR) {
+                               // Src IP conflict, defend with a single ARP probe
+                               VDBG("monitor conflict - defending\n");
+                               timeout_ms = DEFEND_INTERVAL * 1000;
+                               state = DEFEND;
+                               send_arp_request(chosen_nip, &G.our_ethaddr, chosen_nip);
+                               continue;
                        }
-                       break; // case 1 (packet arrived)
-               } // switch (poll)
+                       // state == DEFEND
+                       // Another src IP conflict, start over
+                       VDBG("defend conflict - starting over\n");
+                       run(argv, "deconfig", chosen_nip);
+                       conflicts = 0;
+                       timeout_ms = 0;
+                       goto new_nip_and_PROBE;
+               }
+               // Note: if we only have a target IP conflict here (ip_conflict & 2),
+               // IOW: if we just saw this sort of ARP packet:
+               //  aa:bb:cc:dd:ee:ff > xx:xx:xx:xx:xx:xx: arp who-has <chosen_nip> tell 0.0.0.0
+               // we expect _kernel_ to respond to that, because <chosen_nip>
+               // is (expected to be) configured on this iface.
        } // while (1)
 #undef argv_intf
 }