From owner-p4-projects@FreeBSD.ORG Wed Sep 3 21:27:50 2003 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 4ABE516A4C1; Wed, 3 Sep 2003 21:27:50 -0700 (PDT) Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D5E7216A4BF for ; Wed, 3 Sep 2003 21:27:49 -0700 (PDT) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 4413343F75 for ; Wed, 3 Sep 2003 21:27:49 -0700 (PDT) (envelope-from sam@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.12.6/8.12.6) with ESMTP id h844Rn0U058559 for ; Wed, 3 Sep 2003 21:27:49 -0700 (PDT) (envelope-from sam@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.12.6/8.12.6/Submit) id h844RmdQ058556 for perforce@freebsd.org; Wed, 3 Sep 2003 21:27:48 -0700 (PDT) Date: Wed, 3 Sep 2003 21:27:48 -0700 (PDT) Message-Id: <200309040427.h844RmdQ058556@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to sam@freebsd.org using -f From: Sam Leffler To: Perforce Change Reviews Subject: PERFORCE change 37476 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Sep 2003 04:27:50 -0000 http://perforce.freebsd.org/chv.cgi?CH=37476 Change 37476 by sam@sam_ebb on 2003/09/03 21:27:48 rtrequest(RTM_DELETE, ...) locks the rtentry found by looking up the destination address that's specified. This means the caller cannot be holding a locked reference when doing this operation so (for now) drop the lock and reaquire it after the delete. Various alternatives were considered, including adding a delete interface that takes a locked rtentry, but none are as clean as this. It appears that dropping+reacquiring the lock is safe in all these cases. This problem appeared when ejecting a cardbus network card configured with IPv6. Affected files ... .. //depot/projects/netperf/sys/netinet/in_rmx.c#4 edit .. //depot/projects/netperf/sys/netinet6/in6_ifattach.c#3 edit .. //depot/projects/netperf/sys/netinet6/in6_rmx.c#4 edit Differences ... ==== //depot/projects/netperf/sys/netinet/in_rmx.c#4 (text+ko) ==== @@ -72,7 +72,6 @@ struct sockaddr_in *sin = (struct sockaddr_in *)rt_key(rt); struct radix_node *ret; - /*XXX locking? */ /* * For IP, all unicast non-host routes are automatically cloning. */ @@ -126,12 +125,15 @@ rt2->rt_flags & RTF_HOST && rt2->rt_gateway && rt2->rt_gateway->sa_family == AF_LINK) { + /* NB: must unlock to avoid recursion */ + RT_UNLOCK(rt2); rtrequest(RTM_DELETE, (struct sockaddr *)rt_key(rt2), rt2->rt_gateway, rt_mask(rt2), rt2->rt_flags, 0); ret = rn_addroute(v_arg, n_arg, head, treenodes); + RT_LOCK(rt2); } RTFREE_LOCKED(rt2); } @@ -212,10 +214,13 @@ rt->rt_flags |= RTPRF_OURS; rt->rt_rmx.rmx_expire = time_second + rtq_reallyold; } else { + /* NB: must unlock to avoid recursion */ + RT_UNLOCK(rt); rtrequest(RTM_DELETE, (struct sockaddr *)rt_key(rt), rt->rt_gateway, rt_mask(rt), rt->rt_flags, 0); + RT_LOCK(rt); } } ==== //depot/projects/netperf/sys/netinet6/in6_ifattach.c#3 (text+ko) ==== @@ -986,10 +986,13 @@ sin6.sin6_addr.s6_addr16[1] = htons(ifp->if_index); rt = rtalloc1((struct sockaddr *)&sin6, 0, 0UL); if (rt) { - if (rt->rt_ifp == ifp) + if (rt->rt_ifp == ifp) { + RT_UNLOCK(rt); rtrequest(RTM_DELETE, (struct sockaddr *)rt_key(rt), rt->rt_gateway, rt_mask(rt), rt->rt_flags, 0); - rtfree(rt); + RTFREE(rt); + } else + rtfree(rt); } } ==== //depot/projects/netperf/sys/netinet6/in6_rmx.c#4 (text+ko) ==== @@ -166,12 +166,15 @@ rt2->rt_flags & RTF_HOST && rt2->rt_gateway && rt2->rt_gateway->sa_family == AF_LINK) { + /* NB: must unlock to avoid recursion */ + RT_UNLOCK(rt2); rtrequest(RTM_DELETE, (struct sockaddr *)rt_key(rt2), rt2->rt_gateway, rt_mask(rt2), rt2->rt_flags, 0); ret = rn_addroute(v_arg, n_arg, head, treenodes); + RT_LOCK(rt2); } RTFREE_LOCKED(rt2); } @@ -272,10 +275,13 @@ rt->rt_flags |= RTPRF_OURS; rt->rt_rmx.rmx_expire = time_second + rtq_reallyold; } else { + /* NB: must unlock to avoid recursion */ + RT_UNLOCK(rt); rtrequest(RTM_DELETE, (struct sockaddr *)rt_key(rt), rt->rt_gateway, rt_mask(rt), rt->rt_flags, 0); + RT_LOCK(rt); } }