PERFORCE change 37475 for review

[Sam Leffler]

http://perforce.freebsd.org/chv.cgi?CH=37475

Change 37475 by sam at sam_ebb on 2003/09/03 21:23:33

	Don't hold a reference in rt_parent with a reference count,
	it is unnecessary and we cannot safely remove it.  Specifically
	when deleting an rtentry from which cloned entries were
	created rtrequest1 recurses down the tree deleting cloned
	entries.  But this is done while holding the parent rtentry
	lock. This means that when the backpointer in rt_parent is
	reclaimed we must recursively lock the parent (which fails
	because the mutex is non-recursive).  It's not safe to do
	the clone purging first or to allow recursion on the lock.
	The better solution appears to be to depend on the ordered
	relationship between the parent rtentry and all cloned items
	to insure rt_parent is valid.
	
	This was (briefly) discussed with Garrett who initially added
	the refcnt bump in rev 1.19.  FWIW BSD/OS does not hold use
	the refcnt.

Affected files ...

.. //depot/projects/netperf/sys/net/route.c#7 edit

Differences ...

==== //depot/projects/netperf/sys/net/route.c#7 (text+ko) ====

@@ -262,8 +262,7 @@
 		 */
 		if (rt->rt_ifa)
 			IFAFREE(rt->rt_ifa);
-		if (rt->rt_parent)
-			RTFREE(rt->rt_parent);
+		rt->rt_parent = NULL;		/* NB: no refcnt on parent */
 
 		/*
 		 * The key is separatly alloc'd so free it (see rt_setgate()).
@@ -729,9 +728,17 @@
 			rt->rt_rmx = (*ret_nrt)->rt_rmx; /* copy metrics */
 			rt->rt_rmx.rmx_pksent = 0; /* reset packet counter */
 			if ((*ret_nrt)->rt_flags & (RTF_CLONING | RTF_PRCLONING)) {
+				/*
+				 * NB: We do not bump the refcnt on the parent
+				 * entry under the assumption that it will
+				 * remain so long as we do.  This is
+				 * important when deleting the parent route
+				 * as this operation requires traversing
+				 * the tree to delete all clones and futzing
+				 * with refcnts requires us to double-lock
+				 * parent through this back reference.
+				 */
 				rt->rt_parent = *ret_nrt;
-				/* XXX locking */
-				(*ret_nrt)->rt_refcnt++;
 			}
 		}