Locking for updates to routing table entries. Each rtentry gets a mutex

that covers updates to the contents.  Note this is separate from holding
a reference and/or locking the routing table itself.

Other/related changes:

o rtredirect loses the final parameter by which an rtentry reference
  may be returned; this was never used and added unwarranted complexity
  for locking.
o minor style cleanups to routing code (e.g. ansi-fy function decls)
o remove the logic to bump the refcnt on the parent of cloned routes,
  we assume the parent will remain as long as the clone; doing this avoids
  a circularity in locking during delete
o convert some timeouts to MPSAFE callouts

Notes:

1. rt_mtx in struct rtentry is guarded by #ifdef _KERNEL as user-level
   applications cannot/do-no know about mutex's.  Doing this requires
   that the mutex be the last element in the structure.  A better solution
   is to introduce an externalized version of struct rtentry but this is
   a major task because of the intertwining of rtentry and other data
   structures that are visible to user applications.
2. There are known LOR's that are expected to go away with forthcoming
   work to eliminate many held references.  If not these will be resolved
   prior to release.
3. ATM changes are untested.

Sponsored by:	FreeBSD Foundation
Obtained from:	BSD/OS (partly)

1 files changed, 53 insertions, 32 deletions

diff --git a/sys/netinet/if_ether.c b/sys/netinet/if_ether.c
index 239c616..4cc1d4e 100644
--- a/sys/netinet/if_ether.c
+++ b/sys/netinet/if_ether.c
@@ -105,11 +105,13 @@ struct llinfo_arp {
 static	LIST_HEAD(, llinfo_arp) llinfo_arp;
 
 static struct	ifqueue arpintrq;
-static int	arp_inuse, arp_allocated, arpinit_done;
+static int	arp_allocated;
+static int	arpinit_done;
 
 static int	arp_maxtries = 5;
 static int	useloopback = 1; /* use loopback interface for local traffic */
 static int	arp_proxyall = 0;
+static struct callout arp_callout;
 
 SYSCTL_INT(_net_link_ether_inet, OID_AUTO, maxtries, CTLFLAG_RW,
 	   &arp_maxtries, 0, "");
@@ -140,7 +142,6 @@ arptimer(ignored_arg)
 	void *ignored_arg;
 {
 	struct llinfo_arp *la, *ola;
-	int s = splnet();
 
 	RADIX_NODE_HEAD_LOCK(rt_tables[AF_INET]);
 	la = LIST_FIRST(&llinfo_arp);
@@ -152,8 +153,8 @@ arptimer(ignored_arg)
 			arptfree(ola);		/* timer has expired, clear */
 	}
 	RADIX_NODE_HEAD_UNLOCK(rt_tables[AF_INET]);
-	splx(s);
-	timeout(arptimer, NULL, arpt_prune * hz);
+
+	callout_reset(&arp_callout, arpt_prune * hz, arptimer, NULL);
 }
 
 /*
@@ -165,16 +166,20 @@ arp_rtrequest(req, rt, info)
 	register struct rtentry *rt;
 	struct rt_addrinfo *info;
 {
-	register struct sockaddr *gate = rt->rt_gateway;
-	register struct llinfo_arp *la = (struct llinfo_arp *)rt->rt_llinfo;
+	register struct sockaddr *gate;
+	register struct llinfo_arp *la;
 	static struct sockaddr_dl null_sdl = {sizeof(null_sdl), AF_LINK};
 
+	RT_LOCK_ASSERT(rt);
+
 	if (!arpinit_done) {
 		arpinit_done = 1;
-		timeout(arptimer, (caddr_t)0, hz);
+		callout_reset(&arp_callout, hz, arptimer, NULL);
 	}
 	if (rt->rt_flags & RTF_GATEWAY)
 		return;
+	gate = rt->rt_gateway;
+	la = (struct llinfo_arp *)rt->rt_llinfo;
 	switch (req) {
 
 	case RTM_ADD:
@@ -208,7 +213,7 @@ arp_rtrequest(req, rt, info)
 	case RTM_RESOLVE:
 		if (gate->sa_family != AF_LINK ||
 		    gate->sa_len < sizeof(null_sdl)) {
-			log(LOG_DEBUG, "arp_rtrequest: bad gateway %s%s\n",
+			log(LOG_DEBUG, "%s: bad gateway %s%s\n", __func__,
 			    inet_ntoa(SIN(rt_key(rt))->sin_addr),
 			    (gate->sa_family != AF_LINK) ?
 			    " (!AF_LINK)": "");
@@ -222,14 +227,13 @@ arp_rtrequest(req, rt, info)
 		 * Case 2:  This route may come from cloning, or a manual route
 		 * add with a LL address.
 		 */
-		R_Malloc(la, struct llinfo_arp *, sizeof(*la));
+		R_Zalloc(la, struct llinfo_arp *, sizeof(*la));
 		rt->rt_llinfo = (caddr_t)la;
 		if (la == 0) {
-			log(LOG_DEBUG, "arp_rtrequest: malloc failed\n");
+			log(LOG_DEBUG, "%s: malloc failed\n", __func__);
 			break;
 		}
-		arp_inuse++, arp_allocated++;
-		Bzero(la, sizeof(*la));
+		arp_allocated++;
 		la->la_rt = rt;
 		rt->rt_flags |= RTF_LLINFO;
 		RADIX_NODE_HEAD_LOCK_ASSERT(rt_tables[AF_INET]);
@@ -280,7 +284,6 @@ arp_rtrequest(req, rt, info)
 	case RTM_DELETE:
 		if (la == 0)
 			break;
-		arp_inuse--;
 		RADIX_NODE_HEAD_LOCK_ASSERT(rt_tables[AF_INET]);
 		LIST_REMOVE(la, la_le);
 		rt->rt_llinfo = 0;
@@ -475,6 +478,7 @@ arpresolve(ifp, rt, m, dst, desten, rt0)
 		m_freem(la->la_hold);
 	la->la_hold = m;
 	if (rt->rt_expire) {
+		RT_LOCK(rt);
 		rt->rt_flags &= ~RTF_REJECT;
 		if (la->la_asked == 0 || rt->rt_expire != time_second) {
 			rt->rt_expire = time_second;
@@ -491,6 +495,7 @@ arpresolve(ifp, rt, m, dst, desten, rt0)
 			}
 
 		}
+		RT_UNLOCK(rt);
 	}
 	return (0);
 }
@@ -505,8 +510,9 @@ arpintr(struct mbuf *m)
 	struct arphdr *ar;
 
 	if (!arpinit_done) {
+		/* NB: this race should not matter */
 		arpinit_done = 1;
-		timeout(arptimer, (caddr_t)0, hz);
+		callout_reset(&arp_callout, hz, arptimer, NULL);
 	}
 	if (m->m_len < sizeof(struct arphdr) &&
 	    ((m = m_pullup(m, sizeof(struct arphdr))) == NULL)) {
@@ -736,9 +742,11 @@ match:
 			m->m_pkthdr.len += 8;
 			th->rcf = trld->trld_rcf;
 		}
+		RT_LOCK(rt);
 		if (rt->rt_expire)
 			rt->rt_expire = time_second + arpt_keep;
 		rt->rt_flags &= ~RTF_REJECT;
+		RT_UNLOCK(rt);
 		la->la_asked = 0;
 		la->la_preempt = arp_maxtries;
 		if (la->la_hold) {
@@ -885,13 +893,16 @@ arptfree(la)
 {
 	register struct rtentry *rt = la->la_rt;
 	register struct sockaddr_dl *sdl;
+
 	if (rt == 0)
 		panic("arptfree");
 	if (rt->rt_refcnt > 0 && (sdl = SDL(rt->rt_gateway)) &&
 	    sdl->sdl_family == AF_LINK) {
 		sdl->sdl_alen = 0;
 		la->la_preempt = la->la_asked = 0;
+		RT_LOCK(rt);		/* XXX needed or move higher? */
 		rt->rt_flags &= ~RTF_REJECT;
+		RT_UNLOCK(rt);
 		return;
 	}
 	rtrequest(RTM_DELETE, rt_key(rt), (struct sockaddr *)0, rt_mask(rt),
@@ -906,15 +917,18 @@ arplookup(addr, create, proxy)
 	int create, proxy;
 {
 	register struct rtentry *rt;
-	static struct sockaddr_inarp sin = {sizeof(sin), AF_INET };
+	struct sockaddr_inarp sin;
 	const char *why = 0;
 
+	bzero(&sin, sizeof(sin));
+	sin.sin_len = sizeof(sin);
+	sin.sin_family = AF_INET;
 	sin.sin_addr.s_addr = addr;
-	sin.sin_other = proxy ? SIN_PROXY : 0;
+	if (proxy)
+		sin.sin_other = SIN_PROXY;
 	rt = rtalloc1((struct sockaddr *)&sin, create, 0UL);
 	if (rt == 0)
 		return (0);
-	rt->rt_refcnt--;
 
 	if (rt->rt_flags & RTF_GATEWAY)
 		why = "host is not on local network";
@@ -924,25 +938,32 @@ arplookup(addr, create, proxy)
 		why = "gateway route is not ours";
 
 	if (why) {
-		if (create) {
+#define	ISDYNCLONE(_rt) \
+	(((_rt)->rt_flags & (RTF_STATIC | RTF_WASCLONED)) == RTF_WASCLONED)
+		if (create)
 			log(LOG_DEBUG, "arplookup %s failed: %s\n",
 			    inet_ntoa(sin.sin_addr), why);
-			/*
-			 * If there are no references to this Layer 2 route,
-			 * and it is a cloned route, and not static, and
-			 * arplookup() is creating the route, then purge
-			 * it from the routing table as it is probably bogus.
-			 */
-			if (((rt->rt_flags & (RTF_STATIC | RTF_WASCLONED)) ==
-			    RTF_WASCLONED) && (rt->rt_refcnt == 0))
-				rtrequest(RTM_DELETE,
-				    (struct sockaddr *)rt_key(rt),
-				    rt->rt_gateway, rt_mask(rt),
-				    rt->rt_flags, 0);
+		/*
+		 * If there are no references to this Layer 2 route,
+		 * and it is a cloned route, and not static, and
+		 * arplookup() is creating the route, then purge
+		 * it from the routing table as it is probably bogus.
+		 */
+		RT_UNLOCK(rt);
+		if (rt->rt_refcnt == 1 && ISDYNCLONE(rt)) {
+			rtrequest(RTM_DELETE,
+					(struct sockaddr *)rt_key(rt),
+					rt->rt_gateway, rt_mask(rt),
+					rt->rt_flags, 0);
 		}
+		RTFREE(rt);
 		return (0);
+#undef ISDYNCLONE
+	} else {
+		rt->rt_refcnt--;
+		RT_UNLOCK(rt);
+		return ((struct llinfo_arp *)rt->rt_llinfo);
 	}
-	return ((struct llinfo_arp *)rt->rt_llinfo);
 }
 
 void
@@ -964,7 +985,7 @@ arp_init(void)
 	arpintrq.ifq_maxlen = 50;
 	mtx_init(&arpintrq.ifq_mtx, "arp_inq", NULL, MTX_DEF);
 	LIST_INIT(&llinfo_arp);
+	callout_init(&arp_callout, CALLOUT_MPSAFE);
 	netisr_register(NETISR_ARP, arpintr, &arpintrq);
 }
-
 SYSINIT(arp, SI_SUB_PROTO_DOMAIN, SI_ORDER_ANY, arp_init, 0);