Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 10 Jun 2009 10:34:59 +0000 (UTC)
From:      Luigi Rizzo <luigi@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r193894 - head/sys/netinet/ipfw
Message-ID:  <200906101034.n5AAYxmm010265@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: luigi
Date: Wed Jun 10 10:34:59 2009
New Revision: 193894
URL: http://svn.freebsd.org/changeset/base/193894

Log:
  small simplifications to the code in charge of reaping deleted rules:
  - clear the head pointer immediately before using it, so there is
    no chance of mistakes;
  - call reap_rules() unconditionally. The function can handle a NULL
    argument just fine, and the cost of the extra call is hardly
    significant given that we do it rarely and outside the lock.
  
  MFC after:	3 days

Modified:
  head/sys/netinet/ipfw/ip_fw2.c

Modified: head/sys/netinet/ipfw/ip_fw2.c
==============================================================================
--- head/sys/netinet/ipfw/ip_fw2.c	Wed Jun 10 10:31:11 2009	(r193893)
+++ head/sys/netinet/ipfw/ip_fw2.c	Wed Jun 10 10:34:59 2009	(r193894)
@@ -3631,6 +3631,7 @@ remove_rule(struct ip_fw_chain *chain, s
 /*
  * Reclaim storage associated with a list of rules.  This is
  * typically the list created using remove_rule.
+ * A NULL pointer on input is handled correctly.
  */
 static void
 reap_rules(struct ip_fw *head)
@@ -3655,6 +3656,7 @@ free_chain(struct ip_fw_chain *chain, in
 
 	IPFW_WLOCK_ASSERT(chain);
 
+	chain->reap = NULL;
 	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)
@@ -3701,8 +3703,8 @@ del_entry(struct ip_fw_chain *chain, u_i
 	}
 
 	IPFW_WLOCK(chain);
-	rule = chain->rules;
-	chain->reap = NULL;
+	rule = chain->rules;	/* common starting point */
+	chain->reap = NULL;	/* prepare for deletions */
 	switch (cmd) {
 	case 0:	/* delete rules with given number */
 		/*
@@ -3726,18 +3728,17 @@ del_entry(struct ip_fw_chain *chain, u_i
 
 	case 1:	/* delete all rules with given set number */
 		flush_rule_ptrs(chain);
-		rule = chain->rules;
-		while (rule->rulenum < IPFW_DEFAULT_RULE)
+		while (rule->rulenum < IPFW_DEFAULT_RULE) {
 			if (rule->set == rulenum)
 				rule = remove_rule(chain, rule, prev);
 			else {
 				prev = rule;
 				rule = rule->next;
 			}
+		}
 		break;
 
 	case 2:	/* move rules with given number to new set */
-		rule = chain->rules;
 		for (; rule->rulenum < IPFW_DEFAULT_RULE; rule = rule->next)
 			if (rule->rulenum == rulenum)
 				rule->set = new_set;
@@ -3756,6 +3757,7 @@ del_entry(struct ip_fw_chain *chain, u_i
 			else if (rule->set == new_set)
 				rule->set = rulenum;
 		break;
+
 	case 5: /* delete rules with given number and with given set number.
 		 * rulenum - given rule number;
 		 * new_set - given set number.
@@ -3782,10 +3784,8 @@ del_entry(struct ip_fw_chain *chain, u_i
 	 * avoid a LOR with dummynet.
 	 */
 	rule = chain->reap;
-	chain->reap = NULL;
 	IPFW_WUNLOCK(chain);
-	if (rule)
-		reap_rules(rule);
+	reap_rules(rule);
 	return 0;
 }
 
@@ -4315,6 +4315,8 @@ ipfw_ctl(struct sockopt *sopt)
 		if (V_ipfw_dyn_v)		/* add size of dyn.rules */
 			size += (V_dyn_count * sizeof(ipfw_dyn_rule));
 
+		if (size >= sopt->sopt_valsize)
+			break;
 		/*
 		 * XXX todo: if the user passes a short length just to know
 		 * how much room is needed, do not bother filling up the
@@ -4341,13 +4343,10 @@ ipfw_ctl(struct sockopt *sopt)
 		 */
 
 		IPFW_WLOCK(&V_layer3_chain);
-		V_layer3_chain.reap = NULL;
 		free_chain(&V_layer3_chain, 0 /* keep default rule */);
 		rule = V_layer3_chain.reap;
-		V_layer3_chain.reap = NULL;
 		IPFW_WUNLOCK(&V_layer3_chain);
-		if (rule != NULL)
-			reap_rules(rule);
+		reap_rules(rule);
 		break;
 
 	case IP_FW_ADD:
@@ -4735,12 +4734,10 @@ ipfw_destroy(void)
 	callout_drain(&V_ipfw_timeout);
 	IPFW_WLOCK(&V_layer3_chain);
 	flush_tables(&V_layer3_chain);
-	V_layer3_chain.reap = NULL;
 	free_chain(&V_layer3_chain, 1 /* kill default rule */);
-	reap = V_layer3_chain.reap, V_layer3_chain.reap = NULL;
+	reap = V_layer3_chain.reap;
 	IPFW_WUNLOCK(&V_layer3_chain);
-	if (reap != NULL)
-		reap_rules(reap);
+	reap_rules(reap);
 	IPFW_DYN_LOCK_DESTROY();
 	uma_zdestroy(ipfw_dyn_rule_zone);
 	if (V_ipfw_dyn_v != NULL)

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200906101034.n5AAYxmm010265>