From owner-p4-projects@FreeBSD.ORG Thu Nov 6 16:34:23 2003 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id AA27116A4D0; Thu, 6 Nov 2003 16:34:23 -0800 (PST) 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 7E90016A4CE for ; Thu, 6 Nov 2003 16:34:23 -0800 (PST) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id D76BF43FCB for ; Thu, 6 Nov 2003 16:34:22 -0800 (PST) (envelope-from sam@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.12.9/8.12.9) with ESMTP id hA70YMXJ076823 for ; Thu, 6 Nov 2003 16:34:22 -0800 (PST) (envelope-from sam@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.12.9/8.12.9/Submit) id hA70YMsO076820 for perforce@freebsd.org; Thu, 6 Nov 2003 16:34:22 -0800 (PST) (envelope-from sam@freebsd.org) Date: Thu, 6 Nov 2003 16:34:22 -0800 (PST) Message-Id: <200311070034.hA70YMsO076820@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 41602 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: Fri, 07 Nov 2003 00:34:24 -0000 http://perforce.freebsd.org/chv.cgi?CH=41602 Change 41602 by sam@sam_ebb on 2003/11/06 16:34:12 Fix locking snafu of the ip forwarding cache. We were holding a refrence to a routing table entry w/o bumping the reference count or locking against the entry being free'd. This caused major havoc (for some reason it appeared most frequently for folks running natd). Fix is to bump the reference count whenever we copy the route cache contents into a private copy so the entry cannot be reclaimed out from under us. This is a short term fix as the forthcoming routing table changes will eliminate this cache entirely. Affected files ... .. //depot/projects/netperf/sys/netinet/ip_input.c#19 edit Differences ... ==== //depot/projects/netperf/sys/netinet/ip_input.c#19 (text+ko) ==== @@ -213,7 +213,9 @@ ip_dn_io_t *ip_dn_io_ptr; /* - * One deep route cache for ip forwarding. + * One deep route cache for ip forwarding. This is done + * very inefficiently. We don't care as it's about to be + * replaced by something better. */ static struct rtcache { struct route rc_ro; /* most recently used route */ @@ -223,28 +225,37 @@ #define RTCACHE_LOCK() mtx_lock(&ip_fwdcache.rc_mtx) #define RTCACHE_UNLOCK() mtx_unlock(&ip_fwdcache.rc_mtx) #define RTCACHE_LOCK_INIT() \ - mtx_init(&ip_fwdcache.rc_mtx, "route cache", NULL, MTX_DEF); + mtx_init(&ip_fwdcache.rc_mtx, "route cache", NULL, MTX_DEF) #define RTCACHE_LOCK_ASSERT() mtx_assert(&ip_fwdcache.rc_mtx, MA_OWNED) /* - * Get the current route cache contents. + * Get a copy of the current route cache contents. */ #define RTCACHE_GET(_ro) do { \ + struct rtentry *rt; \ RTCACHE_LOCK(); \ *(_ro) = ip_fwdcache.rc_ro; \ + if ((rt = (_ro)->ro_rt) != NULL) { \ + RT_LOCK(rt); \ + RT_ADDREF(rt); \ + RT_UNLOCK(rt); \ + } \ RTCACHE_UNLOCK(); \ } while (0) /* - * Update the cache contents. We optimize this using - * the routing table reference. XXX is this safe? + * Update the cache contents. */ #define RTCACHE_UPDATE(_ro) do { \ - if ((_ro)->ro_rt != ip_fwdcache.rc_ro.ro_rt) { \ - RTCACHE_LOCK(); \ + struct rtentry *rt; \ + RTCACHE_LOCK(); \ + rt = ip_fwdcache.rc_ro.ro_rt; \ + if ((_ro)->ro_rt != rt) { \ ip_fwdcache.rc_ro = *(_ro); \ - RTCACHE_UNLOCK(); \ + if (rt) \ + RTFREE(rt); \ } \ + RTCACHE_UNLOCK(); \ } while (0) /* @@ -332,15 +343,14 @@ void ip_forward_cacheinval(void) { - struct rtentry *rt = NULL; + struct rtentry *rt; RTCACHE_LOCK(); rt = ip_fwdcache.rc_ro.ro_rt; ip_fwdcache.rc_ro.ro_rt = 0; - RTCACHE_UNLOCK(); - if (rt != NULL) RTFREE(rt); + RTCACHE_UNLOCK(); } /*