From edf193186b53b2bd836f73142b2ec2025c64e0e0 Mon Sep 17 00:00:00 2001
From: Paul Stewart <pstew@chromium.org>
Date: Mon, 11 May 2015 11:45:41 -0700
Subject: [PATCH] Optionally ARP for gateway IP address

If the "arpgw" option is enabled in the config, we ARP for
the gateway provided in the DHCP response as part of the
process of testing our lease.  If this fails (ARP times
out) we DECLINE our lease in the hope that a new lease will
work better.  This can allow us to work around issues with
infrastructures where IP address / MAC pairs are placed on
a "dummy" VLAN under certain conditions.  Requesting a
different IP can sometimes help resolve this.

The code is setup so that for each dhcpcd instance, the
"arpgw" function is allowed to only fail once.  This is
to protect ourselves from mistakenly diagnosing a bad
system, or from looping endlessly if the system is truly
hosed.

BUG=chromium-os:16885
TEST=Manual -- confirm that positive case works, and
manufacture a negative case.  Packet dumps of ARP traffic.
ARP test adds ~0.02 seconds in the successful case.

Verify by using the WiFi testbed tweaked to have the
testbed server advertise default routes.  Recorded multiple
runs for old dhcpcd, dhpcd with arpgw disabled and with
arpgw enabled:

old:     0.879 0.065 0.052 0.877 0.057 0.075 0.920 0.360 0.057 0.964 0.055 0.063
new/off: 0.921 0.055 0.050 0.901 0.420 0.359 0.533 0.350 0.057 0.985 0.061 0.098
new/on:  2.309 0.088 0.087 0.262 0.086 0.083 0.997 0.072 0.076 0.215 0.089 0.077

Reviewed-on: http://gerrit.chromium.org/gerrit/3080
Reviewed-on: http://gerrit.chromium.org/gerrit/3531

---
 arp.c        |  3 ++-
 arp.h        |  1 +
 dhcp.c       | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 if-options.c |  6 ++++-
 if-options.h |  3 ++-
 5 files changed, 85 insertions(+), 3 deletions(-)

diff --git a/arp.c b/arp.c
index 01a8ba4..abb6d4e 100644
--- a/arp.c
+++ b/arp.c
@@ -279,7 +279,8 @@ arp_probe1(void *arg)
 	    ifp->name, inet_ntoa(astate->addr),
 	    astate->probes ? astate->probes : PROBE_NUM, PROBE_NUM,
 	    timespec_to_double(&tv));
-	if (arp_request(ifp, 0, astate->addr.s_addr) == -1)
+	if (arp_request(ifp, astate->src_addr.s_addr,
+			astate->addr.s_addr) == -1)
 		logger(ifp->ctx, LOG_ERR, "send_arp: %m");
 }
 
diff --git a/arp.h b/arp.h
index 83422fe..8c50092 100644
--- a/arp.h
+++ b/arp.h
@@ -58,6 +58,7 @@ struct arp_state {
 	void (*announced_cb)(struct arp_state *);
 	void (*conflicted_cb)(struct arp_state *, const struct arp_msg *);
 
+	struct in_addr src_addr;
 	struct in_addr addr;
 	int probes;
 	int claims;
diff --git a/dhcp.c b/dhcp.c
index abdfc78..9cb50a7 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -2342,6 +2342,71 @@ whitelisted_ip(const struct if_options *ifo, in_addr_t addr)
 }
 
 static void
+dhcp_probe_gw_timeout(struct arp_state *astate) {
+	struct dhcp_state *state = D_STATE(astate->iface);
+
+	/* Allow ourselves to fail only once this way */
+	logger(astate->iface->ctx, LOG_ERR,
+	       "%s: Probe gateway %s timed out ",
+	       astate->iface->name, inet_ntoa(astate->addr));
+	astate->iface->options->options &= ~DHCPCD_ARPGW;
+
+	unlink(state->leasefile);
+	if (!state->lease.frominfo)
+		dhcp_decline(astate->iface);
+#ifdef IN_IFF_DUPLICATED
+	ia = ipv4_iffindaddr(astate->iface, &astate->addr, NULL);
+	if (ia)
+		ipv4_deladdr(astate->iface, &ia->addr, &ia->net);
+#endif
+	eloop_timeout_delete(astate->iface->ctx->eloop, NULL,
+	    astate->iface);
+	eloop_timeout_add_sec(astate->iface->ctx->eloop,
+	    DHCP_RAND_MAX, dhcp_discover, astate->iface);
+}
+
+static void
+dhcp_probe_gw_response(struct arp_state *astate, const struct arp_msg *amsg)
+{
+	/* Verify this is a response for the gateway probe. */
+	if (astate->src_addr.s_addr != 0 &&
+	    amsg &&
+	    amsg->tip.s_addr == astate->src_addr.s_addr &&
+	    amsg->sip.s_addr == astate->addr.s_addr) {
+		dhcp_close(astate->iface);
+		eloop_timeout_delete(astate->iface->ctx->eloop,
+				     NULL, astate->iface);
+#ifdef IN_IFF_TENTATIVE
+		ipv4_finaliseaddr(astate->iface);
+#else
+		dhcp_bind(astate->iface, NULL);
+#endif
+		arp_close(astate->iface);
+	}
+}
+
+static int
+dhcp_probe_gw(struct interface *ifp)
+{
+	struct dhcp_state *state = D_STATE(ifp);
+	struct arp_state *astate;
+	struct in_addr gateway_addr;
+
+	if (get_option_addr(ifp->ctx, &gateway_addr,
+			    state->offer, DHO_ROUTER) == 0) {
+		astate = arp_new(ifp, &gateway_addr);
+		if (astate) {
+			astate->src_addr.s_addr = state->offer->yiaddr;
+			astate->probed_cb = dhcp_probe_gw_timeout;
+			astate->conflicted_cb = dhcp_probe_gw_response;
+			arp_probe(astate);
+			return 1;
+		}
+	}
+	return 0;
+}
+
+static void
 dhcp_arp_probed(struct arp_state *astate)
 {
 	struct dhcp_state *state;
@@ -2361,6 +2426,12 @@ dhcp_arp_probed(struct arp_state *astate)
 		dhcpcd_startinterface(astate->iface);
 		return;
 	}
+
+	/* Probe the gateway specified in the lease offer. */
+	if ((ifo->options & DHCPCD_ARPGW) && (dhcp_probe_gw(astate->iface))) {
+		return;
+	}
+
 	dhcp_close(astate->iface);
 	eloop_timeout_delete(astate->iface->ctx->eloop, NULL, astate->iface);
 #ifdef IN_IFF_TENTATIVE
@@ -2782,6 +2853,10 @@ dhcp_handledhcp(struct interface *ifp, struct dhcp_message **dhcpp,
 #endif
 	}
 
+	if ((ifo->options & DHCPCD_ARPGW) && (dhcp_probe_gw(ifp))) {
+		return;
+	}
+
 	dhcp_bind(ifp, astate);
 }
 
diff --git a/if-options.c b/if-options.c
index 85b6e8e..c6226dd 100644
--- a/if-options.c
+++ b/if-options.c
@@ -141,7 +141,8 @@ const struct option cf_options[] = {
 	{"noipv4ll",        no_argument,       NULL, 'L'},
 	{"master",          no_argument,       NULL, 'M'},
 	{"nooption",        optional_argument, NULL, 'O'},
-	{"require",         required_argument, NULL, 'Q'},
+	{"require",	    required_argument, NULL, 'Q'},
+	{"arpgw",	    no_argument,       NULL, 'R'},
 	{"static",          required_argument, NULL, 'S'},
 	{"test",            no_argument,       NULL, 'T'},
 	{"dumplease",       no_argument,       NULL, 'U'},
@@ -999,6 +1000,9 @@ parse_option(struct dhcpcd_ctx *ctx, const char *ifname, struct if_options *ifo,
 			return -1;
 		}
 		break;
+	case 'R':
+		ifo->options |= DHCPCD_ARPGW;
+		break;
 	case 'S':
 		p = strchr(arg, '=');
 		if (p == NULL) {
diff --git a/if-options.h b/if-options.h
index 4d1de15..153f9b1 100644
--- a/if-options.h
+++ b/if-options.h
@@ -42,7 +42,7 @@
 /* Don't set any optional arguments here so we retain POSIX
  * compatibility with getopt */
 #define IF_OPTS "46bc:de:f:gh:i:j:kl:m:no:pqr:s:t:u:v:wxy:z:" \
-		"ABC:DEF:GHI:JKLMO:Q:S:TUVW:X:Z:"
+		"ABC:DEF:GHI:JKLMO:Q:RS:TUVW:X:Z:"
 
 #define DEFAULT_TIMEOUT		30
 #define DEFAULT_REBOOT		5
@@ -111,6 +111,7 @@
 #define DHCPCD_ROUTER_HOST_ROUTE_WARNED	(1ULL << 55)
 #define DHCPCD_IPV6RA_ACCEPT_NOPUBLIC	(1ULL << 56)
 #define DHCPCD_BOOTP			(1ULL << 57)
+#define DHCPCD_ARPGW			(1ULL << 58)
 
 #define DHCPCD_WARNINGS (DHCPCD_CSR_WARNED | \
 		DHCPCD_ROUTER_HOST_ROUTE_WARNED)
-- 
2.2.0.rc0.207.ga3a616c