From owner-p4-projects@FreeBSD.ORG Fri Dec 19 16:18:11 2003 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id D694116A4D0; Fri, 19 Dec 2003 16:18:10 -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 AFCCB16A4CE for ; Fri, 19 Dec 2003 16:18:10 -0800 (PST) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id D9EBC43D36 for ; Fri, 19 Dec 2003 16:18:04 -0800 (PST) (envelope-from sam@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.12.10/8.12.10) with ESMTP id hBK0I40B058389 for ; Fri, 19 Dec 2003 16:18:04 -0800 (PST) (envelope-from sam@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.12.10/8.12.10/Submit) id hBK0I4Nt058386 for perforce@freebsd.org; Fri, 19 Dec 2003 16:18:04 -0800 (PST) (envelope-from sam@freebsd.org) Date: Fri, 19 Dec 2003 16:18:04 -0800 (PST) Message-Id: <200312200018.hBK0I4Nt058386@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 44105 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: Sat, 20 Dec 2003 00:18:11 -0000 http://perforce.freebsd.org/chv.cgi?CH=44105 Change 44105 by sam@sam_ebb on 2003/12/19 16:17:28 Cleanup the mess I created when dealing with use as a module and add some additional fixups: o move mutex init/destroy logic to the module load/unload hooks; otherwise they are initialized twice when the code is statically configured in the kernel because the module load method gets invoked before the user application calls ip_mrouter_init o add a mutex to synchronize the module init/done operations; this sort of was done using the value of ip_mroute but X_ip_mrouter_done sets it to NULL very early on which can lead to a race against ip_mrouter_init--using the additional mutex means this is safe now o don't call ip_mrouter_reset from ip_mrouter_init; this now happens once at module load and X_ip_mrouter_done does the appropriate cleanup work to insure the data structures are in a consistent state so that a subsequent init operation inherits good state o cleanup debugging code so error conditions are signaled and to use __func__ o make the mrtdebug variable accessible as net.inet.ip.mrtdebug Thanks to juli for shaming me into fixing this. Affected files ... .. //depot/projects/netperf+sockets/sys/netinet/ip_mroute.c#4 edit Differences ... ==== //depot/projects/netperf+sockets/sys/netinet/ip_mroute.c#4 (text+ko) ==== @@ -74,6 +74,8 @@ #define DEBUG_EXPIRE 0x08 #define DEBUG_XMIT 0x10 #define DEBUG_PIM 0x20 +SYSCTL_INT(_net_inet_ip, OID_AUTO, mrtdebug, CTLFLAG_RW, + &mrtdebug, 0, "multicast routing debug control"); #define VIFI_INVALID ((vifi_t) -1) @@ -584,27 +586,8 @@ return 0; } -static void -ip_mrouter_reset(void) -{ - bzero((caddr_t)mfctable, sizeof(mfctable)); - MFC_LOCK_INIT(); - VIF_LOCK_INIT(); - bzero((caddr_t)nexpire, sizeof(nexpire)); - - pim_assert = 0; - mrt_api_config = 0; +static struct mtx mrouter_mtx; /* used to synch init/done work */ - callout_init(&expire_upcalls_ch, CALLOUT_MPSAFE); - - bw_upcalls_n = 0; - bzero((caddr_t)bw_meter_timers, sizeof(bw_meter_timers)); - callout_init(&bw_upcalls_ch, CALLOUT_MPSAFE); - callout_init(&bw_meter_ch, CALLOUT_MPSAFE); - - callout_init(&tbf_reprocess_ch, CALLOUT_MPSAFE); -} - /* * Enable multicast routing */ @@ -612,20 +595,32 @@ ip_mrouter_init(struct socket *so, int version) { if (mrtdebug) - log(LOG_DEBUG, "ip_mrouter_init: so_type = %d, pr_protocol = %d\n", + log(LOG_DEBUG, "%s: so_type = %d, pr_protocol = %d\n", __func__, so->so_type, so->so_proto->pr_protocol); - if (so->so_type != SOCK_RAW || so->so_proto->pr_protocol != IPPROTO_IGMP) + if (so->so_type != SOCK_RAW || so->so_proto->pr_protocol != IPPROTO_IGMP) { + if (mrtdebug) + log(LOG_DEBUG, "%s: invalid socket, type %u protocol %u\n", + __func__, so->so_type, so->so_proto->pr_protocol); return EOPNOTSUPP; + } - if (version != 1) + if (version != 1) { + if (mrtdebug) + log(LOG_DEBUG, "%s: bad version %u, expecting 1\n", + __func__, version); return ENOPROTOOPT; + } + + mtx_lock(&mrouter_mtx); - if (ip_mrouter != NULL) + if (ip_mrouter != NULL) { + if (mrtdebug) + log(LOG_DEBUG, "%s: already initialized\n", __func__); + mtx_unlock(&mrouter_mtx); return EADDRINUSE; + } - ip_mrouter_reset(); - callout_reset(&expire_upcalls_ch, EXPIRE_TIMEOUT, expire_upcalls, NULL); callout_reset(&bw_upcalls_ch, BW_UPCALLS_PERIOD, @@ -634,8 +629,10 @@ ip_mrouter = so; + mtx_unlock(&mrouter_mtx); + if (mrtdebug) - log(LOG_DEBUG, "ip_mrouter_init\n"); + log(LOG_DEBUG, "%s: done\n", __func__); return 0; } @@ -653,6 +650,18 @@ struct mfc *rt; struct rtdetq *rte; + if (mrtdebug) + log(LOG_DEBUG, "%s: start\n", __func__); + + mtx_lock(&mrouter_mtx); + + if (ip_mrouter == NULL) { + if (mrtdebug) + log(LOG_DEBUG, "%s: not initialized\n", __func__); + mtx_unlock(&mrouter_mtx); + return EINVAL; + } + /* * Detach/disable hooks to the reset of the system. */ @@ -690,7 +699,7 @@ bzero((caddr_t)viftable, sizeof(viftable)); numvifs = 0; pim_assert = 0; - VIF_LOCK_DESTROY(); + VIF_UNLOCK(); /* * Free all multicast forwarding cache entries. @@ -717,9 +726,10 @@ } } bzero((caddr_t)mfctable, sizeof(mfctable)); + bzero((caddr_t)nexpire, sizeof(nexpire)); bw_upcalls_n = 0; bzero(bw_meter_timers, sizeof(bw_meter_timers)); - MFC_LOCK_DESTROY(); + MFC_UNLOCK(); /* * Reset de-encapsulation cache @@ -730,8 +740,10 @@ reg_vif_num = VIFI_INVALID; #endif + mtx_unlock(&mrouter_mtx); + if (mrtdebug) - log(LOG_DEBUG, "ip_mrouter_done\n"); + log(LOG_DEBUG, "%s: end\n", __func__); return 0; } @@ -3352,16 +3364,34 @@ } #endif /* PIM */ +static void +ip_mrouter_reset(void) +{ + bzero((caddr_t)mfctable, sizeof(mfctable)); + bzero((caddr_t)nexpire, sizeof(nexpire)); + + pim_assert = 0; + mrt_api_config = 0; + + callout_init(&expire_upcalls_ch, CALLOUT_MPSAFE); + + bw_upcalls_n = 0; + bzero((caddr_t)bw_meter_timers, sizeof(bw_meter_timers)); + callout_init(&bw_upcalls_ch, CALLOUT_MPSAFE); + callout_init(&bw_meter_ch, CALLOUT_MPSAFE); + + callout_init(&tbf_reprocess_ch, CALLOUT_MPSAFE); +} + static int ip_mroute_modevent(module_t mod, int type, void *unused) { - int s; - switch (type) { case MOD_LOAD: - s = splnet(); + mtx_init(&mrouter_mtx, "mrouter initialization", NULL, MTX_DEF); + MFC_LOCK_INIT(); + VIF_LOCK_INIT(); ip_mrouter_reset(); - /* XXX synchronize setup */ ip_mcast_src = X_ip_mcast_src; ip_mforward = X_ip_mforward; ip_mrouter_done = X_ip_mrouter_done; @@ -3397,6 +3427,9 @@ legal_vif_num = NULL; mrt_ioctl = NULL; rsvp_input_p = NULL; + VIF_LOCK_DESTROY(); + MFC_LOCK_DESTROY(); + mtx_destroy(&mrouter_mtx); break; } return 0;