PERFORCE change 44105 for review

Sam Leffler sam at FreeBSD.org
Fri Dec 19 16:18:11 PST 2003


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

Change 44105 by sam at 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;


More information about the p4-projects mailing list