From: Roy Marples <roy@marples.name>
Date: Thu, 2 May 2019 20:25:57 +0000 (+0100)
Subject: BSD: Simplify carrier detection once more.
X-Git-Tag: v7.2.2~9
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fbfa48bcfc205f3ab607ef0f9cd65e3e8948bc43;p=thirdparty%2Fdhcpcd.git

BSD: Simplify carrier detection once more.

RTM_IFINFO messages now send the un-molested link status to
the main carrier handler which no longer polls on LINK_UNKNOWN.
Instead, we check carrier status directly, which if unsupported
we instead interface flags.

This is 2019, all interface drivers should report link status
correctly via RTM_IFINFO messages and trying to constantly work
around this is error prone and time consuming.
---

diff --git a/src/dhcpcd.c b/src/dhcpcd.c
index fa5d5d59..ecff2e57 100644
--- a/src/dhcpcd.c
+++ b/src/dhcpcd.c
@@ -652,25 +652,6 @@ configure_interface(struct interface *ifp, int argc, char **argv,
 	}
 }
 
-static void
-dhcpcd_pollup(void *arg)
-{
-	struct interface *ifp = arg;
-	int carrier;
-
-	carrier = if_carrier(ifp); /* will set ifp->flags */
-	if (carrier == LINK_UP && !(ifp->flags & IFF_UP)) {
-		struct timespec tv;
-
-		tv.tv_sec = 0;
-		tv.tv_nsec = IF_POLL_UP * NSEC_PER_MSEC;
-		eloop_timeout_add_tv(ifp->ctx->eloop, &tv, dhcpcd_pollup, ifp);
-		return;
-	}
-
-	dhcpcd_handlecarrier(ifp->ctx, carrier, ifp->flags, ifp->name);
-}
-
 static void
 dhcpcd_initstate2(struct interface *ifp, unsigned long long options)
 {
@@ -724,35 +705,19 @@ dhcpcd_handlecarrier(struct dhcpcd_ctx *ctx, int carrier, unsigned int flags,
 	    !ifp->active)
 		return;
 
-	switch(carrier) {
-	case LINK_UNKNOWN:
-		carrier = if_carrier(ifp); /* will set ifp->flags */
-		break;
-	case LINK_UP:
-		/* we have a carrier! Still need to check for IFF_UP */
-		if (flags & IFF_UP)
+	if (carrier == LINK_UNKNOWN) {
+		if (ifp->wireless) {
+			carrier = LINK_DOWN;
 			ifp->flags = flags;
-		else {
-			/* So we need to poll for IFF_UP as there is no
-			 * kernel notification when it's set. */
-			dhcpcd_pollup(ifp);
-			return;
-		}
-		break;
-	default:
+		} else
+			carrier = if_carrier(ifp);
+	} else
 		ifp->flags = flags;
-	}
-
-	/* If we here, we don't need to poll for IFF_UP any longer
-	 * if generated by a kernel event. */
-	eloop_timeout_delete(ifp->ctx->eloop, dhcpcd_pollup, ifp);
+	if (carrier == LINK_UNKNOWN)
+		carrier = (ifp->flags & (IFF_UP | IFF_RUNNING)) ==
+		    (IFF_UP & IFF_RUNNING) ? LINK_UP : LINK_DOWN;
 
-	if (carrier == LINK_UNKNOWN) {
-		if (errno != ENOTTY && errno != ENXIO) {
-			/* Don't log an error if interface departed */
-			logerr("%s: %s", ifp->name, __func__);
-		}
-	} else if (carrier == LINK_DOWN || (ifp->flags & IFF_UP) == 0) {
+	if (carrier == LINK_DOWN || (ifp->flags & IFF_UP) == 0) {
 		if (ifp->carrier != LINK_DOWN) {
 			if (ifp->carrier == LINK_UP)
 				loginfox("%s: carrier lost", ifp->name);
@@ -987,20 +952,6 @@ dhcpcd_prestartinterface(void *arg)
 	    )
 		logerr("%s: %s", __func__, ifp->name);
 
-	if (ifp->options->options & DHCPCD_LINK &&
-	    ifp->carrier == LINK_UNKNOWN)
-	{
-		int carrier;
-
-		if ((carrier = if_carrier(ifp)) != LINK_UNKNOWN) {
-			dhcpcd_handlecarrier(ifp->ctx, carrier,
-			    ifp->flags, ifp->name);
-			return;
-		}
-		loginfox("%s: unknown carrier, waiting for interface flags",
-		    ifp->name);
-	}
-
 	dhcpcd_startinterface(ifp);
 }
 
diff --git a/src/if-bsd.c b/src/if-bsd.c
index e608c8e6..cb7cce59 100644
--- a/src/if-bsd.c
+++ b/src/if-bsd.c
@@ -203,28 +203,19 @@ if_closesockets_os(struct dhcpcd_ctx *ctx)
 		close(priv->pf_inet6_fd);
 }
 
-static int
-if_carrier_flags(struct interface *ifp, unsigned int flags)
+int
+if_carrier(struct interface *ifp)
 {
 	struct ifmediareq ifmr = { .ifm_status = 0 };
 
 	strlcpy(ifmr.ifm_name, ifp->name, sizeof(ifmr.ifm_name));
 	if (ioctl(ifp->ctx->pf_inet_fd, SIOCGIFMEDIA, &ifmr) == -1 ||
 	    !(ifmr.ifm_status & IFM_AVALID))
-		return flags & IFF_RUNNING ? LINK_UP : LINK_UNKNOWN;
+		return LINK_UNKNOWN;
 
 	return (ifmr.ifm_status & IFM_ACTIVE) ? LINK_UP : LINK_DOWN;
 }
 
-int
-if_carrier(struct interface *ifp)
-{
-
-	if (if_getflags(ifp) == -1)
-		return LINK_UNKNOWN;
-	return if_carrier_flags(ifp, ifp->flags);
-}
-
 static void
 if_linkaddr(struct sockaddr_dl *sdl, const struct interface *ifp)
 {
@@ -1009,33 +1000,24 @@ if_ifinfo(struct dhcpcd_ctx *ctx, const struct if_msghdr *ifm)
 {
 	struct interface *ifp;
 	int link_state;
-	unsigned int flags;
 
 	if ((ifp = if_findindex(ctx->ifaces, ifm->ifm_index)) == NULL)
 		return;
 
-	flags = (unsigned int)ifm->ifm_flags;
 	switch (ifm->ifm_data.ifi_link_state) {
 	case LINK_STATE_UNKNOWN:
-		/* In theory this is only set when an interface is first
-		 * initiaised.
-		 * However whilst some drivers report an active link
-		 * via SIOCGIFMEDIA, they don't bother to announce it
-		 * via a routing message. */
-		if (ifp->wireless) /* Wireless needs to work correctly. */
-			link_state = LINK_DOWN;
-		else
-			link_state = if_carrier_flags(ifp, flags);
+		link_state = LINK_UNKNOWN;
 		break;
 	case LINK_STATE_UP:
-		link_state = ifm->ifm_flags & IFF_UP ? LINK_UP : LINK_DOWN;
+		link_state = LINK_UP;
 		break;
 	default:
 		link_state = LINK_DOWN;
 		break;
 	}
 
-	dhcpcd_handlecarrier(ctx, link_state, flags, ifp->name);
+	dhcpcd_handlecarrier(ctx, link_state,
+	    (unsigned int)ifm->ifm_flags, ifp->name);
 }
 
 static void