Date: Tue, 16 Sep 2003 14:36:51 -0700 (PDT) From: Sam Leffler <sam@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 38148 for review Message-ID: <200309162136.h8GLapQK049021@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=38148 Change 38148 by sam@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;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200309162136.h8GLapQK049021>