dhcpcd-discuss

Hi Peter

On 05/12/16 15:39, Piers O'Hanlon wrote:
> It seems that there’s still issues with my fix below as
> dhcp_arp_probed() then fails to function normally, as does
> dhcp_arp_conflicted(). So one could add state->arping_index++  just
> before dhcpcd_startinterface() is called in both functions.

Yes :)

I've found some time to do some testing have made the attached patch
which appears to work fine for me and passed valgrind testing.
Can you verify it fixes the issue for you please?

Thanks

Roy

Index: dhcp.c
==================================================================
--- dhcp.c
+++ dhcp.c
@@ -2021,11 +2021,11 @@
 		/* We didn't find a profile for this
 		 * address or hwaddr, so move to the next
 		 * arping profile */
 		if (++state->arping_index < ifo->arping_len) {
 			astate->addr.s_addr =
-			    ifo->arping[state->arping_index - 1];
+			    ifo->arping[state->arping_index];
 			arp_probe(astate);
 			return;
 		}
 		arp_free(astate);
 #ifdef KERNEL_RFC5227
@@ -2085,20 +2085,20 @@
 	ifp = astate->iface;
 	state = D_STATE(ifp);
 
 #ifdef ARPING
 	ifo = ifp->options;
-	if (state->arping_index &&
-	    state->arping_index <= ifo->arping_len &&
+	if (state->arping_index != -1 &&
+	    state->arping_index < ifo->arping_len &&
 	    amsg &&
-	    (amsg->sip.s_addr == ifo->arping[state->arping_index - 1] ||
+	    (amsg->sip.s_addr == ifo->arping[state->arping_index] ||
 	    (amsg->sip.s_addr == 0 &&
-	    amsg->tip.s_addr == ifo->arping[state->arping_index - 1])))
+	    amsg->tip.s_addr == ifo->arping[state->arping_index])))
 	{
 		char buf[HWADDR_LEN * 3];
 
-		astate->failed.s_addr = ifo->arping[state->arping_index - 1];
+		astate->failed.s_addr = ifo->arping[state->arping_index];
 		arp_report_conflicted(astate, amsg);
 		hwaddr_ntoa(amsg->sha, ifp->hwlen, buf, sizeof(buf));
 		if (dhcpcd_selectprofile(ifp, buf) == -1 &&
 		    dhcpcd_selectprofile(ifp,
 		        inet_ntoa(astate->failed)) == -1)
@@ -2594,10 +2594,13 @@
 	if (state == NULL) {
 		eloop_timeout_delete(ifp->ctx->eloop, NULL, ifp);
 		return;
 	}
 
+#ifdef ARPING
+	state->arping_index = -1;
+#endif
 	if (ifp->options->options & DHCPCD_RELEASE &&
 	    !(ifp->options->options & DHCPCD_INFORM))
 	{
 		/* Failure to send the release may cause this function to
 		 * re-enter so guard by setting the state. */
@@ -3408,10 +3411,13 @@
 		if (state == NULL)
 			return -1;
 		/* 0 is a valid fd, so init to -1 */
 		state->raw_fd = -1;
 
+#ifdef ARPING
+		state->arping_index = -1;
+#endif
 		/* Now is a good time to find IPv4 routes */
 		if_initrt(ifp->ctx, AF_INET);
 	}
 
 	state->state = DHS_INIT;
@@ -3661,10 +3667,13 @@
 
 void
 dhcp_start(struct interface *ifp)
 {
 	struct timespec tv;
+#ifdef ARPING
+	const struct dhcp_state *state;
+#endif
 
 	if (!(ifp->options->options & DHCPCD_IPV4))
 		return;
 
 	/* If we haven't been given a netmask for our requested address,
@@ -3703,10 +3712,19 @@
 	    !(ifp->options->options & DHCPCD_INITIAL_DELAY))
 	{
 		dhcp_start1(ifp);
 		return;
 	}
+
+#ifdef ARPING
+	/* If we have arpinged then we have already delayed. */
+	state = D_CSTATE(ifp);
+	if (state != NULL && state->arping_index != -1) {
+		dhcp_start1(ifp);
+		return;
+	}
+#endif
 
 	tv.tv_sec = DHCP_MIN_DELAY;
 	tv.tv_nsec = (suseconds_t)arc4random_uniform(
 	    (DHCP_MAX_DELAY - DHCP_MIN_DELAY) * NSEC_PER_SEC);
 	timespecnorm(&tv);
@@ -3718,10 +3736,17 @@
 }
 
 void
 dhcp_abort(struct interface *ifp)
 {
+#ifdef ARPING
+	struct dhcp_state *state;
+
+	state = D_STATE(ifp);
+	if (state != NULL)
+		state->arping_index = -1;
+#endif
 
 	eloop_timeout_delete(ifp->ctx->eloop, dhcp_start1, ifp);
 }
 
 void

Index: dhcp.h
==================================================================
--- dhcp.h
+++ dhcp.h
@@ -207,11 +207,11 @@
 	char leasefile[sizeof(LEASEFILE) + IF_NAMESIZE + (IF_SSIDLEN * 4)];
 	struct timespec started;
 	unsigned char *clientid;
 	struct authstate auth;
 #ifdef ARPING
-	size_t arping_index;
+	ssize_t arping_index;
 #endif
 };
 
 #define D_STATE(ifp)							       \
 	((struct dhcp_state *)(ifp)->if_data[IF_DATA_DHCP])

Index: if-options.c
==================================================================
--- if-options.c
+++ if-options.c
@@ -1236,11 +1236,11 @@
 			if (fp)
 				*fp++ = '\0';
 			if (parse_addr(ctx, &addr, NULL, arg) != 0)
 				return -1;
 			naddr = realloc(ifo->arping,
-			    sizeof(in_addr_t) * (ifo->arping_len + 1));
+			    sizeof(in_addr_t) * ((size_t)ifo->arping_len + 1));
 			if (naddr == NULL) {
 				logger(ctx, LOG_ERR, "%s: %m", __func__);
 				return -1;
 			}
 			ifo->arping = naddr;

Index: if-options.h
==================================================================
--- if-options.h
+++ if-options.h
@@ -195,11 +195,11 @@
 
 	size_t blacklist_len;
 	in_addr_t *blacklist;
 	size_t whitelist_len;
 	in_addr_t *whitelist;
-	size_t arping_len;
+	ssize_t arping_len;
 	in_addr_t *arping;
 	char *fallback;
 
 	struct if_ia *ia;
 	size_t ia_len;

Attachment: signature.asc
Description: OpenPGP digital signature