Purge routes attached to an address when this address is removed.

This is done to stop using stale ifa attached to routes, which is
the easiest way to make rtisvalid(9) MP-safe.

sthen@ and henning@ like it, ok claudio@

5 files changed, 63 insertions, 143 deletions

diff --git a/sys/net/if.c b/sys/net/if.c
index 8f734148156..185978d06e8 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if.c,v 1.440 2016/09/03 10:05:19 mpi Exp $	*/
+/*	$OpenBSD: if.c,v 1.441 2016/09/04 10:32:01 mpi Exp $	*/
 /*	$NetBSD: if.c,v 1.35 1996/05/07 05:26:04 thorpej Exp $	*/
 
 /*
@@ -954,7 +954,6 @@ if_detach(struct ifnet *ifp)
 #ifdef INET6
 	in6_ifdetach(ifp);
 #endif
-	rt_if_remove(ifp);
 #if NPF > 0
 	pfi_detach_ifnet(ifp);
 #endif
@@ -1951,7 +1950,6 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p)
 #ifdef INET6
 			in6_ifdetach(ifp);
 #endif
-			rt_if_remove(ifp);
 			splx(s);
 		}
 
diff --git a/sys/net/route.c b/sys/net/route.c
index 4f4f57b8035..ed828c835ad 100644
--- a/sys/net/route.c
+++ b/sys/net/route.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: route.c,v 1.323 2016/09/04 09:39:01 claudio Exp $	*/
+/*	$OpenBSD: route.c,v 1.324 2016/09/04 10:32:01 mpi Exp $	*/
 /*	$NetBSD: route.c,v 1.14 1996/02/13 22:00:46 christos Exp $	*/
 
 /*
@@ -152,6 +152,7 @@ extern unsigned int	rtmap_limit;
 
 struct rtstat		rtstat;
 int			rttrash;	/* routes not in table but not freed */
+int			ifatrash;	/* ifas not in ifp list but not free */
 
 struct pool		rtentry_pool;	/* pool for rtentry structures */
 struct pool		rttimer_pool;	/* pool for rttimer structures */
@@ -159,10 +160,9 @@ struct pool		rttimer_pool;	/* pool for rttimer structures */
 void	rt_timer_init(void);
 int	rt_setgwroute(struct rtentry *, u_int);
 void	rt_putgwroute(struct rtentry *);
-int	rt_fixgwroute(struct rtentry *, void *, unsigned int);
 int	rtflushclone1(struct rtentry *, void *, u_int);
 void	rtflushclone(unsigned int, struct rtentry *);
-int	rt_if_remove_rtdelete(struct rtentry *, void *, u_int);
+int	rt_ifa_purge_walker(struct rtentry *, void *, unsigned int);
 struct rtentry *rt_match(struct sockaddr *, uint32_t *, int, unsigned int);
 struct sockaddr *rt_plentosa(sa_family_t, int, struct sockaddr_in6 *);
 
@@ -217,10 +217,6 @@ rtisvalid(struct rtentry *rt)
 	if (!ISSET(rt->rt_flags, RTF_UP))
 		return (0);
 
-	/* Routes attached to stale ifas should be freed. */
-	if (rt->rt_ifa == NULL || rt->rt_ifa->ifa_ifp == NULL)
-		return (0);
-
 	if (ISSET(rt->rt_flags, RTF_GATEWAY)) {
 		KASSERT(rt->rt_gwroute != NULL);
 	    	KASSERT(!ISSET(rt->rt_gwroute->rt_flags, RTF_GATEWAY));
@@ -453,41 +449,6 @@ rt_putgwroute(struct rtentry *rt)
 	rt->rt_gwroute = NULL;
 }
 
-/*
- * Refresh cached entries of RTF_GATEWAY routes for a given interface.
- *
- * This clever logic is necessary to try to fix routes linked to stale
- * ifas.
- */
-int
-rt_fixgwroute(struct rtentry *rt, void *arg, unsigned int id)
-{
-	struct ifnet *ifp = arg;
-
-	KERNEL_ASSERT_LOCKED();
-
-	if (rt->rt_ifidx != ifp->if_index || !ISSET(rt->rt_flags, RTF_GATEWAY))
-		return (0);
-
-	/*
-	 * If the gateway route is not stale, its associated cached
-	 * is also not stale.
-	 */
-	if (rt->rt_ifa->ifa_ifp != NULL)
-		return (0);
-
-	/* If we can fix the cached next hop entry, we can fix the ifa. */
-	if (rt_setgate(rt, rt->rt_gateway, ifp->if_rdomain) == 0) {
-		struct ifaddr *ifa = rt->rt_gwroute->rt_ifa;
-
-		ifafree(rt->rt_ifa);
-		ifa->ifa_refcnt++;
-		rt->rt_ifa = ifa;
-	}
-
-	return (0);
-}
-
 void
 rtref(struct rtentry *rt)
 {
@@ -558,9 +519,10 @@ ifafree(struct ifaddr *ifa)
 {
 	if (ifa == NULL)
 		panic("ifafree");
-	if (ifa->ifa_refcnt == 0)
+	if (ifa->ifa_refcnt == 0) {
+		ifatrash--;
 		free(ifa, M_IFADDR, 0);
-	else
+	} else
 		ifa->ifa_refcnt--;
 }
 
@@ -735,7 +697,7 @@ rtflushclone1(struct rtentry *rt, void *arg, u_int id)
 	/*
 	 * This happens when an interface with a RTF_CLONING route is
 	 * being detached.  In this case it's safe to bail because all
-	 * the routes are being purged by rt_if_remove().
+	 * the routes are being purged by rt_ifa_purge().
 	 */
 	if (ifp == NULL)
 	        return 0;
@@ -993,22 +955,8 @@ rtrequest(int req, struct rt_addrinfo *info, u_int8_t prio,
 			return (EINVAL);
 		if ((rt->rt_flags & RTF_CLONING) == 0)
 			return (EINVAL);
-		if (rt->rt_ifa->ifa_ifp) {
-			info->rti_ifa = rt->rt_ifa;
-		} else {
-			/*
-			 * The address of the cloning route is not longer
-			 * configured on an interface, but its descriptor
-			 * is still there because of reference counting.
-			 *
-			 * Try to find a similar active address and use
-			 * it for the cloned route.  The cloning route
-			 * will get the new address and interface later.
-			 */
-			info->rti_ifa = NULL;
-			info->rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr;
-		}
-
+		KASSERT(rt->rt_ifa->ifa_ifp != NULL);
+		info->rti_ifa = rt->rt_ifa;
 		info->rti_flags = rt->rt_flags | (RTF_CLONED|RTF_HOST);
 		info->rti_flags &= ~(RTF_CLONING|RTF_CONNECTED|RTF_STATIC);
 		info->rti_info[RTAX_GATEWAY] = sdltosa(&sa_dl);
@@ -1101,29 +1049,6 @@ rtrequest(int req, struct rt_addrinfo *info, u_int8_t prio,
 		rt->rt_ifidx = ifp->if_index;
 		if (rt->rt_flags & RTF_CLONED) {
 			/*
-			 * If the ifa of the cloning route was stale, a
-			 * successful lookup for an ifa with the same address
-			 * has been made.  Use this ifa also for the cloning
-			 * route.
-			 */
-			if ((*ret_nrt)->rt_ifa->ifa_ifp == NULL) {
-				struct ifnet *ifp0;
-
-				printf("%s RTM_RESOLVE: wrong ifa (%p) was (%p)"
-				    "\n", __func__, ifa, (*ret_nrt)->rt_ifa);
-
-				ifp0 = if_get((*ret_nrt)->rt_ifidx);
-				KASSERT(ifp0 != NULL);
-				ifp0->if_rtrequest(ifp0, RTM_DELETE, *ret_nrt);
-				ifafree((*ret_nrt)->rt_ifa);
-				if_put(ifp0);
-
-				ifa->ifa_refcnt++;
-				(*ret_nrt)->rt_ifa = ifa;
-				(*ret_nrt)->rt_ifidx = ifp->if_index;
-				ifp->if_rtrequest(ifp, RTM_ADD, *ret_nrt);
-			}
-			/*
 			 * Copy both metrics and a back pointer to the cloned
 			 * route's parent.
 			 */
@@ -1297,8 +1222,6 @@ rt_ifa_add(struct ifaddr *ifa, int flags, struct sockaddr *dst)
 
 	error = rtrequest(RTM_ADD, &info, prio, &rt, rtableid);
 	if (error == 0) {
-		unsigned int i;
-
 		/*
 		 * A local route is created for every address configured
 		 * on an interface, so use this information to notify
@@ -1308,18 +1231,6 @@ rt_ifa_add(struct ifaddr *ifa, int flags, struct sockaddr *dst)
 			rt_sendaddrmsg(rt, RTM_NEWADDR, ifa);
 		rt_sendmsg(rt, RTM_ADD, rtableid);
 		rtfree(rt);
-
-		/*
-		 * Userland inserted routes stay in the table even
-		 * if their corresponding ``ifa'' is no longer valid.
-		 *
-		 * Try to fix the stale RTF_GATEWAY entries in case
-		 * their gateway match the newly inserted route.
-		 */
-		for (i = 0; i <= RT_TABLEID_MAX; i++) {
-			rtable_walk(i, ifa->ifa_addr->sa_family,
-			    rt_fixgwroute, ifp);
-		}
 	}
 	return (error);
 }
@@ -1476,6 +1387,46 @@ rt_ifa_dellocal(struct ifaddr *ifa)
 }
 
 /*
+ * Remove all addresses attached to ``ifa''.
+ */
+void
+rt_ifa_purge(struct ifaddr *ifa)
+{
+	struct ifnet		*ifp = ifa->ifa_ifp;
+	unsigned int		 rtableid;
+	int			 i;
+
+	KASSERT(ifp != NULL);
+
+	for (rtableid = 0; rtableid < rtmap_limit; rtableid++) {
+		/* skip rtables that are not in the rdomain of the ifp */
+		if (rtable_l2(rtableid) != ifp->if_rdomain)
+			continue;
+		for (i = 1; i <= AF_MAX; i++) {
+			rtable_walk(rtableid, i, rt_ifa_purge_walker, ifa);
+		}
+	}
+}
+
+int
+rt_ifa_purge_walker(struct rtentry *rt, void *vifa, unsigned int rtableid)
+{
+	struct ifaddr		*ifa = vifa;
+	struct ifnet		*ifp = ifa->ifa_ifp;
+	int			 error;
+
+	if (rt->rt_ifa != ifa)
+		return (0);
+
+	if ((error = rtdeletemsg(rt, ifp, rtableid))) {
+		return (error);
+	}
+
+	return (EAGAIN);
+
+}
+
+/*
  * Route timer routines.  These routes allow functions to be called
  * for various routes at any time.  This is useful in supporting
  * path MTU discovery and redirect route deletion.
@@ -1758,38 +1709,6 @@ rtlabel_unref(u_int16_t id)
 	}
 }
 
-void
-rt_if_remove(struct ifnet *ifp)
-{
-	int			 i;
-	u_int			 tid;
-
-	for (tid = 0; tid < rtmap_limit; tid++) {
-		/* skip rtables that are not in the rdomain of the ifp */
-		if (rtable_l2(tid) != ifp->if_rdomain)
-			continue;
-		for (i = 1; i <= AF_MAX; i++) {
-			rtable_walk(tid, i, rt_if_remove_rtdelete, ifp);
-		}
-	}
-}
-
-int
-rt_if_remove_rtdelete(struct rtentry *rt, void *vifp, u_int id)
-{
-	struct ifnet	*ifp = vifp;
-	int		 error;
-
-	if (rt->rt_ifidx != ifp->if_index)
-		return (0);
-
-	if ((error = rtdeletemsg(rt, ifp, id)))
-		return (error);
-
-	return (EAGAIN);
-
-}
-
 #ifndef SMALL_KERNEL
 void
 rt_if_track(struct ifnet *ifp)
diff --git a/sys/net/route.h b/sys/net/route.h
index bb49114ab1e..359f6967237 100644
--- a/sys/net/route.h
+++ b/sys/net/route.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: route.h,v 1.146 2016/09/04 09:39:01 claudio Exp $	*/
+/*	$OpenBSD: route.h,v 1.147 2016/09/04 10:32:01 mpi Exp $	*/
 /*	$NetBSD: route.h,v 1.9 1996/02/13 22:00:49 christos Exp $	*/
 
 /*
@@ -398,13 +398,13 @@ void	 rtfree(struct rtentry *);
 int	 rt_getifa(struct rt_addrinfo *, u_int);
 int	 rt_ifa_add(struct ifaddr *, int, struct sockaddr *);
 int	 rt_ifa_del(struct ifaddr *, int, struct sockaddr *);
+void	 rt_ifa_purge(struct ifaddr *);
 int	 rt_ifa_addlocal(struct ifaddr *);
 int	 rt_ifa_dellocal(struct ifaddr *);
 int	 rtioctl(u_long, caddr_t, struct proc *);
 void	 rtredirect(struct sockaddr *, struct sockaddr *, struct sockaddr *, struct rtentry **, unsigned int);
 int	 rtrequest(int, struct rt_addrinfo *, u_int8_t, struct rtentry **,
 	     u_int);
-void	 rt_if_remove(struct ifnet *);
 #ifndef SMALL_KERNEL
 void	 rt_if_track(struct ifnet *);
 int	 rt_if_linkstate_change(struct rtentry *, void *, u_int);
diff --git a/sys/netinet/in.c b/sys/netinet/in.c
index 7d8bd510578..b70abc895bf 100644
--- a/sys/netinet/in.c
+++ b/sys/netinet/in.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in.c,v 1.128 2016/06/13 10:34:40 mpi Exp $	*/
+/*	$OpenBSD: in.c,v 1.129 2016/09/04 10:32:01 mpi Exp $	*/
 /*	$NetBSD: in.c,v 1.26 1996/02/13 23:41:39 christos Exp $	*/
 
 /*
@@ -699,12 +699,14 @@ in_purgeaddr(struct ifaddr *ifa)
 {
 	struct ifnet *ifp = ifa->ifa_ifp;
 	struct in_ifaddr *ia = ifatoia(ifa);
+	extern int ifatrash;
 
 	splsoftassert(IPL_SOFTNET);
 
 	in_ifscrub(ifp, ia);
 
 	rt_ifa_dellocal(&ia->ia_ifa);
+	rt_ifa_purge(&ia->ia_ifa);
 	ifa_del(ifp, &ia->ia_ifa);
 
 	if (ia->ia_allhosts != NULL) {
@@ -712,6 +714,7 @@ in_purgeaddr(struct ifaddr *ifa)
 		ia->ia_allhosts = NULL;
 	}
 
+	ifatrash++;
 	ia->ia_ifp = NULL;
 	ifafree(&ia->ia_ifa);
 }
diff --git a/sys/netinet6/in6.c b/sys/netinet6/in6.c
index 8ecdd65cc0a..74e2327dc50 100644
--- a/sys/netinet6/in6.c
+++ b/sys/netinet6/in6.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in6.c,v 1.191 2016/08/22 10:33:22 mpi Exp $	*/
+/*	$OpenBSD: in6.c,v 1.192 2016/09/04 10:32:01 mpi Exp $	*/
 /*	$KAME: in6.c,v 1.372 2004/06/14 08:14:21 itojun Exp $	*/
 
 /*
@@ -895,12 +895,11 @@ void
 in6_unlink_ifa(struct in6_ifaddr *ia6, struct ifnet *ifp)
 {
 	struct ifaddr *ifa = &ia6->ia_ifa;
+	extern int ifatrash;
 	int plen;
 
 	splsoftassert(IPL_SOFTNET);
 
-	ifa_del(ifp, ifa);
-
 	TAILQ_REMOVE(&in6_ifaddr, ia6, ia_list);
 
 	/* Release the reference to the base prefix. */
@@ -918,10 +917,11 @@ in6_unlink_ifa(struct in6_ifaddr *ia6, struct ifnet *ifp)
 		ia6->ia6_ndpr = NULL;
 	}
 
-	/*
-	 * release another refcnt for the link from in6_ifaddr.
-	 * Note that we should decrement the refcnt at least once for all *BSD.
-	 */
+	rt_ifa_purge(ifa);
+	ifa_del(ifp, ifa);
+
+	ifatrash++;
+	ia6->ia_ifp = NULL;
 	ifafree(&ia6->ia_ifa);
 }