PERFORCE change 38148 for review

[Sam Leffler]

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

Change 38148 by sam at sam_ebb on 2003/09/16 14:36:02

	o fix LOR between ipfw and dummynet: when deleting rules place them
	  on a ``reap list'' and reclaim their storage after dropping the
	  IPFW lock (calling back into dummynet during reclamation to avoid
	  the LOR)
	o cleanup the module unload handling
	o always honor MOD_UNLOCK requests; not doing this when built as
	  a static kernel component is bogus

Affected files ...

.. //depot/projects/netperf/sys/netinet/ip_fw2.c#7 edit

Differences ...

==== //depot/projects/netperf/sys/netinet/ip_fw2.c#7 (text+ko) ====

@@ -107,6 +107,7 @@
 
 struct ip_fw_chain {
 	struct ip_fw	*rules;		/* list of rules */
+	struct ip_fw	*reap;		/* list of rules to reap */
 	struct mtx	mtx;		/* lock guarding rule list */
 };
 #define	IPFW_LOCK_INIT(_chain) \
@@ -2209,15 +2210,15 @@
 }
 
 /**
- * Free storage associated with a static rule (including derived
- * dynamic rules).
+ * Remove a static rule (including derived * dynamic rules)
+ * and place it on the ``reap list'' for later reclamation.
  * The caller is in charge of clearing rule pointers to avoid
  * dangling pointers.
  * @return a pointer to the next entry.
  * Arguments are not checked, so they better be correct.
  */
 static struct ip_fw *
-delete_rule(struct ip_fw_chain *chain, struct ip_fw *prev, struct ip_fw *rule)
+remove_rule(struct ip_fw_chain *chain, struct ip_fw *rule, struct ip_fw *prev)
 {
 	struct ip_fw *n;
 	int l = RULESIZE(rule);
@@ -2235,15 +2236,33 @@
 	static_count--;
 	static_len -= l;
 
-	if (DUMMYNET_LOADED)
-		ip_dn_ruledel_ptr(rule);
-	free(rule, M_IPFW);
+	rule->next = chain->reap;
+	chain->reap = rule;
+
 	return n;
 }
 
+/**
+ * Reclaim storage associated with a list of rules.  This is
+ * typically the list created using remove_rule.
+ */
+static void
+reap_rules(struct ip_fw *head)
+{
+	struct ip_fw *rule;
+
+	while ((rule = head) != NULL) {
+		head = head->next;
+		if (DUMMYNET_LOADED)
+			ip_dn_ruledel_ptr(rule);
+		free(rule, M_IPFW);
+	}
+}
+
 /*
- * Deletes all rules from a chain (except rules in set RESVD_SET
- * unless kill_default = 1).
+ * Remove all rules from a chain (except rules in set RESVD_SET
+ * unless kill_default = 1).  The caller is responsible for
+ * reclaiming storage for the rules left in chain->reap.
  */
 static void
 free_chain(struct ip_fw_chain *chain, int kill_default)
@@ -2255,7 +2274,7 @@
 	flush_rule_ptrs(chain); /* more efficient to do outside the loop */
 	for (prev = NULL, rule = chain->rules; rule ; )
 		if (kill_default || rule->set != RESVD_SET)
-			rule = delete_rule(chain, prev, rule);
+			rule = remove_rule(chain, rule, prev);
 		else {
 			prev = rule;
 			rule = rule->next;
@@ -2300,6 +2319,7 @@
 
 	IPFW_LOCK(chain);
 	rule = chain->rules;
+	chain->reap = NULL;
 	switch (cmd) {
 	case 0:	/* delete rules with given number */
 		/*
@@ -2318,7 +2338,7 @@
 		 */
 		flush_rule_ptrs(chain);
 		while (rule->rulenum == rulenum)
-			rule = delete_rule(chain, prev, rule);
+			rule = remove_rule(chain, rule, prev);
 		break;
 
 	case 1:	/* delete all rules with given set number */
@@ -2326,7 +2346,7 @@
 		rule = chain->rules;
 		while (rule->rulenum < IPFW_DEFAULT_RULE)
 			if (rule->set == rulenum)
-				rule = delete_rule(chain, prev, rule);
+				rule = remove_rule(chain, rule, prev);
 			else {
 				prev = rule;
 				rule = rule->next;
@@ -2354,7 +2374,16 @@
 				rule->set = rulenum;
 		break;
 	}
+	/*
+	 * Look for rules to reclaim.  We grab the list before
+	 * releasing the lock then reclaim them w/o the lock to
+	 * avoid a LOR with dummynet.
+	 */
+	rule = chain->reap;
+	chain->reap = NULL;
 	IPFW_UNLOCK(chain);
+	if (rule)
+		reap_rules(rule);
 	return 0;
 }
 
@@ -2744,8 +2773,12 @@
 		 */
 
 		IPFW_LOCK(&layer3_chain);
+		layer3_chain.reap = NULL;
 		free_chain(&layer3_chain, 0 /* keep default rule */);
+		rule = layer3_chain.reap, layer3_chain.reap = NULL;
 		IPFW_UNLOCK(&layer3_chain);
+		if (layer3_chain.reap != NULL)
+			reap_rules(rule);
 		break;
 
 	case IP_FW_ADD:
@@ -2924,6 +2957,27 @@
 	return (0);
 }
 
+static void
+ipfw_destroy(void)
+{
+	struct ip_fw *reap;
+
+	IPFW_LOCK(&layer3_chain);
+	callout_stop(&ipfw_timeout);
+	ip_fw_chk_ptr = NULL;
+	ip_fw_ctl_ptr = NULL;
+	layer3_chain.reap = NULL;
+	free_chain(&layer3_chain, 1 /* kill default rule */);
+	reap = layer3_chain.reap, layer3_chain.reap = NULL;
+	IPFW_UNLOCK(&layer3_chain);
+	if (reap != NULL)
+		reap_rules(reap);
+
+	IPFW_DYN_LOCK_DESTROY();
+	IPFW_LOCK_DESTROY(&layer3_chain);
+	printf("IP firewall unloaded\n");
+}
+
 static int
 ipfw_modevent(module_t mod, int type, void *unused)
 {
@@ -2940,21 +2994,8 @@
 		break;
 
 	case MOD_UNLOAD:
-#if !defined(KLD_MODULE)
-		printf("ipfw statically compiled, cannot unload\n");
-		err = EBUSY;
-#else
-		IPFW_LOCK(&layer3_chain);
-		IPFW_DYN_LOCK();
-		callout_stop(&ipfw_timeout);
-		ip_fw_chk_ptr = NULL;
-		ip_fw_ctl_ptr = NULL;
-		free_chain(&layer3_chain, 1 /* kill default rule */);
-		IPFW_DYN_LOCK_DESTROY();
-		IPFW_LOCK_DESTROY(&layer3_chain);
-		printf("IP firewall unloaded\n");
+		ipfw_destroy();
 		err = 0;
-#endif
 		break;
 	default:
 		break;