Current-Users archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: if_addrflags6: Can't assign requested address



On 17/08/2018 10:08, Roy Marples wrote:
On 17/08/2018 09:04, Masanobu SAITOH wrote:
wm2: carrier lost
wm2: executing `/libexec/dhcpcd-run-hooks' NOCARRIER
wm2: deleting address fe80::1392:4012:56d8:a7a2
wm2: if_addrflags6: Can't assign requested address
wm2: if_addrflags6: Can't assign requested address
wm2: if_addrflags6: Can't assign requested address
wm2: if_addrflags6: Can't assign requested address
wm2: carrier acquired
wm2: executing `/libexec/dhcpcd-run-hooks' CARRIER

This helps.
I never saw this because on NetBSD-8, we have addrflags available in ifa_msghdr when sent over route(4). This does not exist on NetBSD-7 so we need to make an ioctl per address to work out the flags. Sadly, this is racy and this is what happens:

Something adds an address.
Kernel annnounces new address to route(4).
Something deletes this address.
Kernel announces the address deleted to route(4).

dhcpcd reads the address added message from route(4) *after* the address has been deleted from the kernel. Because dhcpcd needs the address flags at this point, an ioctl is made to the deleted address and boom, error.

Luckily dhcpcd handles it correctly and it's just noise.
Please test the attached patch to silence it.
If you can verify it works, let me know and I'll push a new version out.

Since then I've discovered two more critical issues with dhcpcd-7 on NetBSD-7.
1) Broken IP_PKTINFO implementation
2) Invalid RTA_BRD in RTM_NEWADDR messages for new addresses
Both of these have already been fixed in -8 and -current and neither looks suitable for a pullup and dhcpcd needs a workaround for both anyway.

A better patch attached and I'll hopefully get this pushed out over the weekend.

Roy
diff --git a/src/dhcp.c b/src/dhcp.c
index 7a6749d4..1e9fe186 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -86,6 +86,11 @@
 #define IPDEFTTL 64 /* RFC1340 */
 #endif
 
+/* NetBSD-7 has an incomplete IP_PKTINFO implementation. */
+#if defined(__NetBSD_Version__) && __NetBSD_Version__ < 800000000
+#undef IP_PKTINFO
+#endif
+
 /* Assert the correct structure size for on wire */
 __CTASSERT(sizeof(struct ip)		== 20);
 __CTASSERT(sizeof(struct udphdr)	== 8);
diff --git a/src/if-bsd.c b/src/if-bsd.c
index c3c95ba6..cdd959a6 100644
--- a/src/if-bsd.c
+++ b/src/if-bsd.c
@@ -1103,9 +1103,32 @@ if_ifa(struct dhcpcd_ctx *ctx, const struct ifa_msghdr *ifam)
 		sin = (const void *)rti_info[RTAX_NETMASK];
 		mask.s_addr = sin != NULL && sin->sin_family == AF_INET ?
 		    sin->sin_addr.s_addr : INADDR_ANY;
+
+#if defined(__NetBSD_Version__) && __NetBSD_Version__ < 800000000
+		/* NetBSD-7 and older send an invalid broadcast address.
+		 * So we need to query the actual address to get
+		 * the right one. */
+		{
+			struct in_aliasreq ifra;
+
+			memset(&ifra, 0, sizeof(ifra));
+			strlcpy(ifra.ifra_name, ifp->name,
+			    sizeof(ifra.ifra_name));
+			ifra.ifra_addr.sin_family = AF_INET;
+			ifra.ifra_addr.sin_len = sizeof(ifra.ifra_addr);
+			ifra.ifra_addr.sin_addr = addr;
+			if (ioctl(ctx->pf_inet_fd, SIOCGIFALIAS, &ifra) == -1) {
+				if (errno != EADDRNOTAVAIL)
+					logerr("%s: SIOCGIFALIAS", __func__);
+				break;
+			}
+			bcast = ifra.ifra_broadaddr.sin_addr;
+		}
+#else
 		sin = (const void *)rti_info[RTAX_BRD];
 		bcast.s_addr = sin != NULL && sin->sin_family == AF_INET ?
 		    sin->sin_addr.s_addr : INADDR_ANY;
+#endif
 
 #if defined(__FreeBSD__) || defined(__DragonFly__)
 		/* FreeBSD sends RTM_DELADDR for each assigned address
@@ -1134,8 +1157,8 @@ if_ifa(struct dhcpcd_ctx *ctx, const struct ifa_msghdr *ifam)
 		if (ifam->ifam_type == RTM_DELADDR)
 			addrflags = 0 ;
 		else if ((addrflags = if_addrflags(ifp, &addr, NULL)) == -1) {
-			logerr("%s: if_addrflags: %s",
-			    ifp->name, inet_ntoa(addr));
+			if (errno != EADDRNOTAVAIL)
+				logerr("%s: if_addrflags", __func__);
 			break;
 		}
 #endif
@@ -1160,7 +1183,8 @@ if_ifa(struct dhcpcd_ctx *ctx, const struct ifa_msghdr *ifam)
 		if (ifam->ifam_type == RTM_DELADDR)
 		    addrflags = 0;
 		else if ((addrflags = if_addrflags6(ifp, &addr6, NULL)) == -1) {
-			logerr("%s: if_addrflags6", ifp->name);
+			if (errno != EADDRNOTAVAIL)
+				logerr("%s: if_addrflags6", __func__);
 			break;
 		}
 #endif
diff --git a/src/if.c b/src/if.c
index eaebefa5..18356206 100644
--- a/src/if.c
+++ b/src/if.c
@@ -240,10 +240,8 @@ if_learnaddrs(struct dhcpcd_ctx *ctx, struct if_head *ifs,
 			addrflags = if_addrflags(ifp, &addr->sin_addr,
 			    ifa->ifa_name);
 			if (addrflags == -1) {
-				if (errno != EEXIST)
-					logerr("%s: if_addrflags: %s",
-					    __func__,
-					    inet_ntoa(addr->sin_addr));
+				if (errno != EEXIST && errno != EADDRNOTAVAIL)
+					logerr("%s: if_addrflags", __func__);
 				continue;
 			}
 #endif
@@ -266,7 +264,7 @@ if_learnaddrs(struct dhcpcd_ctx *ctx, struct if_head *ifs,
 			addrflags = if_addrflags6(ifp, &sin6->sin6_addr,
 			    ifa->ifa_name);
 			if (addrflags == -1) {
-				if (errno != EEXIST)
+				if (errno != EEXIST && errno != EADDRNOTAVAIL)
 					logerr("%s: if_addrflags6", __func__);
 				continue;
 			}
diff --git a/src/ipv4.c b/src/ipv4.c
index 0a2594b8..8008f9e6 100644
--- a/src/ipv4.c
+++ b/src/ipv4.c
@@ -816,9 +816,17 @@ ipv4_handleifa(struct dhcpcd_ctx *ctx,
 	bool ia_is_new;
 
 #if 0
-	logdebugx("%s: %s %s/%d %d", ifname,
-	    cmd == RTM_NEWADDR ? "RTM_NEWADDR" : cmd == RTM_DELADDR ? "RTM_DELADDR" : "???",
-	    inet_ntoa(*addr), inet_ntocidr(*mask), addrflags);
+	char sbrdbuf[INET_ADDRSTRLEN];
+	const char *sbrd;
+
+	if (brd)
+		sbrd = inet_ntop(AF_INET, brd, sbrdbuf, sizeof(sbrdbuf));
+	else
+		sbrd = NULL;
+	logdebugx("%s: %s %s/%d %s %d", ifname,
+	    cmd == RTM_NEWADDR ? "RTM_NEWADDR" :
+	    cmd == RTM_DELADDR ? "RTM_DELADDR" : "???",
+	    inet_ntoa(*addr), inet_ntocidr(*mask), sbrd, addrflags);
 #endif
 
 	if (ifs == NULL)
diff --git a/src/ipv6.c b/src/ipv6.c
index 2d516d99..8a2cb9f8 100644
--- a/src/ipv6.c
+++ b/src/ipv6.c
@@ -567,7 +567,8 @@ ipv6_checkaddrflags(void *arg)
 	alias = NULL;
 #endif
 	if ((flags = if_addrflags6(ia->iface, &ia->addr, alias)) == -1) {
-		logerr("%s: if_addrflags6", ia->iface->name);
+		if (errno != EEXIST && errno != EADDRNOTAVAIL)
+			logerr("%s: if_addrflags6", __func__);
 		return;
 	}
 


Home | Main Index | Thread Index | Old Index