svn commit: r200850 - in user/luigi/ipfw3-head/sys: net netgraph netinet netinet/ipfw

Luigi Rizzo luigi at FreeBSD.org
Tue Dec 22 16:06:39 UTC 2009


Author: luigi
Date: Tue Dec 22 16:06:38 2009
New Revision: 200850
URL: http://svn.freebsd.org/changeset/base/200850

Log:
  complete the code for improved rule lookup in ipfw.
  Now we should have the following (Old is the code in HEAD;
  "Now" means the code in this commit; "Planned" is what i
  expect to achieve with small changes):
  
    Function				Old	Now	  Planned
  -------------------------------------------------------------------
    + skipto X, non cached		O(N)	O(log N)
  XXX skipto X, cached			O(1)	O(log N)  O(1) NOTE 1
  XXX dynamic rule lookup			O(1)	O(log N)  O(1) NOTE 1
    + skipto tablearg			O(N)	O(log N)  O(1) NOTE 2
    + reinject from dummynet, non cached	O(N)	O(log N)
    + reinject from dummynet, cached	O(1)	O(1)
    + kernel blocked during setsockopt()	O(N)	O(1)
  
  NOTE 1: validating the cached entries requires an extra field
  	in the rule structure, which in turn requires a bit of work
  	to preserve the kernel-userland ABI.
  
  NOTE 2: currently there is no cache for skipto tablearg.
  	It can be implemented rather trivially to achieve
  	O(1) time on repeated jumps to the same target.
  
  As you can see there are a couple of places where we have a bit
  of regression, but in most cases there is a huge improvement,
  especially in the interference between userland and kernel.

Modified:
  user/luigi/ipfw3-head/sys/net/if_bridge.c
  user/luigi/ipfw3-head/sys/net/if_ethersubr.c
  user/luigi/ipfw3-head/sys/netgraph/ng_ipfw.c
  user/luigi/ipfw3-head/sys/netgraph/ng_ipfw.h
  user/luigi/ipfw3-head/sys/netinet/ip_dummynet.h
  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c
  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw2.c
  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_pfil.c
  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_private.h
  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_sockopt.c

Modified: user/luigi/ipfw3-head/sys/net/if_bridge.c
==============================================================================
--- user/luigi/ipfw3-head/sys/net/if_bridge.c	Tue Dec 22 16:05:28 2009	(r200849)
+++ user/luigi/ipfw3-head/sys/net/if_bridge.c	Tue Dec 22 16:06:38 2009	(r200850)
@@ -3039,20 +3039,23 @@ bridge_pfil(struct mbuf **mp, struct ifn
 			goto bad;
 	}
 
+	/* XXX this section is also in if_ethersubr.c */
 	if (V_ip_fw_chk_ptr && pfil_ipfw != 0 && dir == PFIL_OUT && ifp != NULL) {
 		struct dn_pkt_tag *dn_tag;
 
 		error = -1;
 		dn_tag = ip_dn_claim_tag(*mp);
-		if (dn_tag != NULL) {
-			if (dn_tag->rule != NULL && V_fw_one_pass)
+		if (dn_tag == NULL) {
+			args.slot = 0;
+		} else {
+			if (dn_tag->slot != 0 && V_fw_one_pass)
 				/* packet already partially processed */
 				goto ipfwpass;
-			args.rule = dn_tag->rule; /* matching rule to restart */
-			args.rule_id = dn_tag->rule_id;
+			args.slot = dn_tag->slot; /* next rule to use */
 			args.chain_id = dn_tag->chain_id;
-		} else
-			args.rule = NULL;
+			args.rulenum = dn_tag->rulenum;
+			args.rule_id = dn_tag->rule_id;
+		}
 
 		args.m = *mp;
 		args.oif = ifp;

Modified: user/luigi/ipfw3-head/sys/net/if_ethersubr.c
==============================================================================
--- user/luigi/ipfw3-head/sys/net/if_ethersubr.c	Tue Dec 22 16:05:28 2009	(r200849)
+++ user/luigi/ipfw3-head/sys/net/if_ethersubr.c	Tue Dec 22 16:06:38 2009	(r200850)
@@ -471,15 +471,17 @@ ether_ipfw_chk(struct mbuf **m0, struct 
 
 	dn_tag = ip_dn_claim_tag(*m0);
 
-	if (dn_tag != NULL) {
-		if (dn_tag->rule != NULL && V_fw_one_pass)
+	if (dn_tag == NULL) {
+		args.slot = 0;
+	} else {
+		if (dn_tag->slot != 0 && V_fw_one_pass)
 			/* dummynet packet, already partially processed */
 			return (1);
-		args.rule = dn_tag->rule;	/* matching rule to restart */
+		args.slot = dn_tag->slot;	/* matching rule to restart */
+		args.rulenum = dn_tag->rulenum;
 		args.rule_id = dn_tag->rule_id;
 		args.chain_id = dn_tag->chain_id;
-	} else
-		args.rule = NULL;
+	}
 
 	/*
 	 * I need some amt of data to be contiguous, and in case others need

Modified: user/luigi/ipfw3-head/sys/netgraph/ng_ipfw.c
==============================================================================
--- user/luigi/ipfw3-head/sys/netgraph/ng_ipfw.c	Tue Dec 22 16:05:28 2009	(r200849)
+++ user/luigi/ipfw3-head/sys/netgraph/ng_ipfw.c	Tue Dec 22 16:06:38 2009	(r200850)
@@ -293,7 +293,8 @@ ng_ipfw_input(struct mbuf **m0, int dir,
 			m_freem(m);
 			return (ENOMEM);
 		}
-		ngit->rule = fwa->rule;
+		ngit->slot = fwa->slot;
+		ngit->rulenum = fwa->rulenum;
 		ngit->rule_id = fwa->rule_id;
 		ngit->chain_id = fwa->chain_id;
 		ngit->dir = dir;

Modified: user/luigi/ipfw3-head/sys/netgraph/ng_ipfw.h
==============================================================================
--- user/luigi/ipfw3-head/sys/netgraph/ng_ipfw.h	Tue Dec 22 16:05:28 2009	(r200849)
+++ user/luigi/ipfw3-head/sys/netgraph/ng_ipfw.h	Tue Dec 22 16:06:38 2009	(r200850)
@@ -37,7 +37,8 @@ extern	ng_ipfw_input_t	*ng_ipfw_input_p;
 
 struct ng_ipfw_tag {
 	struct m_tag	mt;		/* tag header */
-	struct ip_fw	*rule;		/* matching rule */
+	uint32_t	slot;		/* slot for next rule */
+	uint32_t	rulenum;	/* matching rule number */
 	uint32_t	rule_id;	/* matching rule id */
 	uint32_t	chain_id;	/* ruleset id */
 	struct ifnet	*ifp;		/* interface, for ip_output */

Modified: user/luigi/ipfw3-head/sys/netinet/ip_dummynet.h
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ip_dummynet.h	Tue Dec 22 16:05:28 2009	(r200849)
+++ user/luigi/ipfw3-head/sys/netinet/ip_dummynet.h	Tue Dec 22 16:06:38 2009	(r200850)
@@ -112,7 +112,8 @@ struct dn_heap {
  * processing requirements.
  */
 struct dn_pkt_tag {
-    struct ip_fw *rule;		/* matching rule */
+    uint32_t slot;		/* slot of next rule to use */
+    uint32_t rulenum;		/* matching rule number */
     uint32_t rule_id;		/* matching rule id */
     uint32_t chain_id;		/* ruleset id */
     int dn_dir;			/* action when packet comes out. */

Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c	Tue Dec 22 16:05:28 2009	(r200849)
+++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_dummynet.c	Tue Dec 22 16:06:38 2009	(r200850)
@@ -1367,20 +1367,11 @@ dummynet_io(struct mbuf **m0, int dir, s
 	struct dn_pipe *pipe;
 	uint64_t len = m->m_pkthdr.len;
 	struct dn_flow_queue *q = NULL;
-	int is_pipe;
-	ipfw_insn *cmd = ACTION_PTR(fwa->rule);
+	int is_pipe = fwa->cookie & 0x8000000 ? 0 : 1;
 
 	KASSERT(m->m_nextpkt == NULL,
 	    ("dummynet_io: mbuf queue passed to dummynet"));
 
-	if (cmd->opcode == O_LOG)
-		cmd += F_LEN(cmd);
-	if (cmd->opcode == O_ALTQ)
-		cmd += F_LEN(cmd);
-	if (cmd->opcode == O_TAG)
-		cmd += F_LEN(cmd);
-	is_pipe = (cmd->opcode == O_PIPE);
-
 	DUMMYNET_LOCK();
 	io_pkt++;
 	/*
@@ -1390,11 +1381,11 @@ dummynet_io(struct mbuf **m0, int dir, s
 	 * below can be simplified.
 	 */
 	if (is_pipe) {
-		pipe = locate_pipe(fwa->cookie);
+		pipe = locate_pipe(fwa->cookie & 0xffff);
 		if (pipe != NULL)
 			fs = &(pipe->fs);
 	} else
-		fs = locate_flowset(fwa->cookie);
+		fs = locate_flowset(fwa->cookie & 0xffff);
 
 	if (fs == NULL)
 		goto dropit;	/* This queue/pipe does not exist! */
@@ -1440,7 +1431,8 @@ dummynet_io(struct mbuf **m0, int dir, s
 	 * Ok, i can handle the pkt now...
 	 * Build and enqueue packet + parameters.
 	 */
-	pkt->rule = fwa->rule;
+	pkt->slot = fwa->slot;
+	pkt->rulenum = fwa->rulenum;
 	pkt->rule_id = fwa->rule_id;
 	pkt->chain_id = fwa->chain_id;
 	pkt->dn_dir = dir;

Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw2.c
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw2.c	Tue Dec 22 16:05:28 2009	(r200849)
+++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw2.c	Tue Dec 22 16:06:38 2009	(r200850)
@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2002 Luigi Rizzo, Universita` di Pisa
+ * Copyright (c) 2002-2009 Luigi Rizzo, Universita` di Pisa
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -131,12 +131,10 @@ VNET_DEFINE(u_int32_t, set_disable);
 #define	V_set_disable			VNET(set_disable)
 
 VNET_DEFINE(int, fw_verbose);
-//#define	V_verbose_limit			VNET(verbose_limit)
 /* counter for ipfw_log(NULL...) */
 VNET_DEFINE(u_int64_t, norule_counter);
 VNET_DEFINE(int, verbose_limit);
 
-
 /* layer3_chain contains the list of rules for layer 3 */
 VNET_DEFINE(struct ip_fw_chain, layer3_chain);
 
@@ -147,8 +145,6 @@ ipfw_nat_cfg_t *ipfw_nat_del_ptr;
 ipfw_nat_cfg_t *ipfw_nat_get_cfg_ptr;
 ipfw_nat_cfg_t *ipfw_nat_get_log_ptr;
 
-extern int ipfw_chg_hook(SYSCTL_HANDLER_ARGS);
-
 #ifdef SYSCTL_NODE
 SYSCTL_NODE(_net_inet_ip, OID_AUTO, fw, CTLFLAG_RW, 0, "Firewall");
 SYSCTL_VNET_INT(_net_inet_ip_fw, OID_AUTO, one_pass,
@@ -173,6 +169,9 @@ SYSCTL_INT(_net_inet_ip_fw, OID_AUTO, de
     &default_to_accept, 0,
     "Make the default rule accept all packets.");
 TUNABLE_INT("net.inet.ip.fw.default_to_accept", &default_to_accept);
+SYSCTL_VNET_INT(_net_inet_ip_fw, OID_AUTO, static_count,
+    CTLFLAG_RD, &VNET_NAME(layer3_chain.n_rules), 0,
+    "Number of static rules");
 
 #ifdef INET6
 SYSCTL_DECL(_net_inet6_ip6);
@@ -713,12 +712,17 @@ check_uidgid(ipfw_insn_u32 *insn, int pr
 	return match;
 }
 
+/*
+ * Helper function to write the matching rule into args
+ */
 static inline void
-set_match(struct ip_fw_args *args, struct ip_fw *f, struct ip_fw_chain *chain)
+set_match(struct ip_fw_args *args, int slot,
+	struct ip_fw *f, struct ip_fw_chain *chain)
 {
-	args->rule = (void *)(uintptr_t)f->rulenum;
-	args->rule_id = f->id;
 	args->chain_id = chain->id;
+	args->slot = slot + 1; /* we use 0 as a marker */
+	args->rule_id = f->id;
+	args->rulenum = f->rulenum;
 }
 
 /*
@@ -812,8 +816,7 @@ ipfw_chk(struct ip_fw_args *args)
 	 */
 	struct ifnet *oif = args->oif;
 
-	struct ip_fw *f = NULL;		/* matching rule */
-	int f_pos = 0;			/* index of f in the array */
+	int f_pos = 0;		/* index of current rule in the array */
 	int retval = 0;
 
 	/*
@@ -1142,23 +1145,21 @@ do {								\
 		return (IP_FW_PASS);	/* accept */
 	}
 	mtag = m_tag_find(m, PACKET_TAG_DIVERT, NULL);
-	if (args->rule) {
+	if (args->slot) {
 		/*
 		 * Packet has already been tagged as a result of a previous
 		 * match on rule args->rule aka args->rule_id (PIPE, QUEUE,
 		 * REASS, NETGRAPH and similar, never a skipto).
-		 * Validate the pointer and continue from args->rule->next
-		 * if still present, otherwise use the default rule.
-		 * XXX If fw_one_pass != 0 then just accept it, though
-		 * the caller should never pass us such packets.
-		 * XXX rule is now the rule number so we can do a lookup
+		 * Validate the slot and continue from the next one
+		 * if still present, otherwise do a lookup.
 		 */
 		if (V_fw_one_pass) {
 			IPFW_RUNLOCK(chain);
 			return (IP_FW_PASS);
 		}
-		f_pos = ipfw_find_rule(chain, (uint32_t)(args->rule), args->rule_id+1);
-		f = chain->map[f_pos];
+		f_pos = (args->chain_id == chain->id) ?
+		    args->slot /* already incremented */ :
+		    ipfw_find_rule(chain, args->rulenum, args->rule_id+1);
 	} else {
 		/*
 		 * Find the starting rule. It can be either the first
@@ -1166,7 +1167,6 @@ do {								\
 		 */
 		int skipto = mtag ? divert_cookie(mtag) : 0;
 
-		f = chain->rules;
 		f_pos = 0;
 		if (args->eh == NULL && skipto != 0) {
 			if (skipto >= IPFW_DEFAULT_RULE) {
@@ -1174,7 +1174,6 @@ do {								\
 				return (IP_FW_DENY); /* invalid */
 			}
 			f_pos = ipfw_find_rule(chain, skipto, 0);
-			f = chain->map[f_pos];
 		}
 	}
 	/* reset divert rule to avoid confusion later */
@@ -1206,8 +1205,8 @@ do {								\
 		ipfw_insn *cmd;
 		uint32_t tablearg = 0;
 		int l, cmdlen, skip_or; /* skip rest of OR block */
+		struct ip_fw *f;
 
-/* again: */
 		f = chain->map[f_pos];
 		if (V_set_disable & (1 << f->set) )
 			continue;
@@ -1890,9 +1889,14 @@ do {								\
 					 */
 					q->pcnt++;
 					q->bcnt += pktlen;
+					/* XXX we would like to have f_pos
+					 * readily accessible in the dynamic
+				         * rule, instead of having to
+					 * looku q->rule.
+					 */
 					f = q->rule;
-					/* the pointer is valid so we can do a lookup */
-					f_pos = ipfw_find_rule(chain, f->rulenum, f->id);
+					f_pos = ipfw_find_rule(chain,
+						f->rulenum, f->id);
 					cmd = ACTION_PTR(f);
 					l = f->cmd_len - f->act_ofs;
 					ipfw_dyn_unlock();
@@ -1918,11 +1922,11 @@ do {								\
 
 			case O_PIPE:
 			case O_QUEUE:
-				set_match(args, f, chain);
-				if (cmd->arg1 == IP_FW_TABLEARG)
-					args->cookie = tablearg;
-				else
-					args->cookie = cmd->arg1;
+				set_match(args, f_pos, f, chain);
+				args->cookie = (cmd->arg1 == IP_FW_TABLEARG) ?
+					tablearg : cmd->arg1;
+				if (cmd->opcode == O_QUEUE)
+					args->cookie |= 0x80000000;
 				retval = IP_FW_DUMMYNET;
 				l = 0;          /* exit inner loop */
 				done = 1;       /* exit outer loop */
@@ -1956,6 +1960,9 @@ do {								\
 
 			case O_COUNT:
 			case O_SKIPTO:
+			    {
+				int i;
+
 				f->pcnt++;	/* update stats */
 				f->bcnt += pktlen;
 				f->timestamp = time_uptime;
@@ -1964,12 +1971,13 @@ do {								\
 					break;
 				}
 				/* skipto: */
-				{
-					int i = (cmd->arg1 == IP_FW_TABLEARG) ? tablearg : cmd->arg1;
-					if (i <= f->rulenum)
-						i = f->rulenum + 1;	/* no backward jumps */
-					f_pos = ipfw_find_rule(chain, i, 0);
-				}
+				i = (cmd->arg1 == IP_FW_TABLEARG) ?
+					tablearg : cmd->arg1;
+				/* make sure we do not jump backward */
+				if (i <= f->rulenum)
+					i = f->rulenum + 1;
+				f_pos = ipfw_find_rule(chain, i, 0);
+			    }
 				/*
 				 * Skip disabled rules, and
 				 * re-enter the inner loop
@@ -1978,7 +1986,6 @@ do {								\
 				 */
 				for (; f_pos < chain->n_rules - 1 && (V_set_disable & (1 << chain->map[f_pos]->set)); f_pos++)
 					;
-				f = chain->map[f_pos];
 				l = f->cmd_len;
 				cmd = f->cmd;
 				match = 1;
@@ -2045,11 +2052,9 @@ do {								\
 
 			case O_NETGRAPH:
 			case O_NGTEE:
-				set_match(args, f, chain);
-				if (cmd->arg1 == IP_FW_TABLEARG)
-					args->cookie = tablearg;
-				else
-					args->cookie = cmd->arg1;
+				set_match(args, f_pos, f, chain);
+				args->cookie = (cmd->arg1 == IP_FW_TABLEARG) ?
+					tablearg : cmd->arg1;
 				retval = (cmd->opcode == O_NETGRAPH) ?
 				    IP_FW_NETGRAPH : IP_FW_NGTEE;
 				l = 0;          /* exit inner loop */
@@ -2072,7 +2077,7 @@ do {								\
 				    struct cfg_nat *t;
 				    int nat_id;
 
-				    set_match(args, f, chain);
+				    set_match(args, f_pos, f, chain);
 				    t = ((ipfw_insn_nat *)cmd)->nat;
 				    if (t == NULL) {
 					nat_id = (cmd->arg1 == IP_FW_TABLEARG) ?
@@ -2139,7 +2144,7 @@ do {								\
 				    else
 					ip->ip_sum = in_cksum(m, hlen);
 				    retval = IP_FW_REASS;
-				    set_match(args, f, chain);
+				    set_match(args, f_pos, f, chain);
 				}
 				done = 1;	/* exit outer loop */
 				break;
@@ -2173,10 +2178,11 @@ do {								\
 	}		/* end of outer for, scan rules */
 
 	if (done) {
+		struct ip_fw *rule = chain->map[f_pos];
 		/* Update statistics */
-		f->pcnt++;
-		f->bcnt += pktlen;
-		f->timestamp = time_uptime;
+		rule->pcnt++;
+		rule->bcnt += pktlen;
+		rule->timestamp = time_uptime;
 	} else {
 		retval = IP_FW_DENY;
 		printf("ipfw: ouch!, skip past end of rules, denying packet\n");
@@ -2287,7 +2293,7 @@ vnet_ipfw_init(const void *unused)
 	V_verbose_limit = IPFIREWALL_VERBOSE_LIMIT;
 #endif
 #ifdef IPFIREWALL_NAT
-	LIST_INIT(chain->nat);
+	LIST_INIT(&chain->nat);
 #endif
 
 	/* insert the default rule and create the initial map */
@@ -2350,7 +2356,9 @@ vnet_ipfw_init(const void *unused)
 static int
 vnet_ipfw_uninit(const void *unused)
 {
-	struct ip_fw *reap;
+	struct ip_fw *reap, *rule;
+	struct ip_fw_chain *chain = &V_layer3_chain;
+	int i;
 
 	V_ipfw_vnet_ready = 0; /* tell new callers to go away */
 	/*
@@ -2364,22 +2372,29 @@ vnet_ipfw_uninit(const void *unused)
 #endif
 	V_ip_fw_chk_ptr = NULL;
 	V_ip_fw_ctl_ptr = NULL;
-
-	IPFW_WLOCK(&V_layer3_chain);
-	/* We wait on the wlock here until the last user leaves */
-	IPFW_WUNLOCK(&V_layer3_chain);
-	IPFW_WLOCK(&V_layer3_chain);
+	IPFW_UH_WLOCK(chain);
+	IPFW_UH_WUNLOCK(chain);
+	IPFW_UH_WLOCK(chain);
+
+	IPFW_WLOCK(chain);
+	IPFW_WUNLOCK(chain);
+	IPFW_WLOCK(chain);
 
 	ipfw_dyn_uninit(0);	/* run the callout_drain */
-	ipfw_flush_tables(&V_layer3_chain);
-	V_layer3_chain.reap = NULL;
-	ipfw_free_chain(&V_layer3_chain, 1 /* kill default rule */);
-	reap = V_layer3_chain.reap;
-	V_layer3_chain.reap = NULL;
-	IPFW_WUNLOCK(&V_layer3_chain);
+	ipfw_flush_tables(chain);
+	reap = NULL;
+	for (i = 0; i < chain->n_rules; i++) {
+		rule = chain->map[i];
+		rule->x_next = reap;
+		reap = rule;
+	}
+	if (chain->map)
+		free(chain->map, M_IPFW);
+	IPFW_WUNLOCK(chain);
+	IPFW_UH_WUNLOCK(chain);
 	if (reap != NULL)
 		ipfw_reap_rules(reap);
-	IPFW_LOCK_DESTROY(&V_layer3_chain);
+	IPFW_LOCK_DESTROY(chain);
 	ipfw_dyn_uninit(1);	/* free the remaining parts */
 	return 0;
 }

Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_pfil.c
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_pfil.c	Tue Dec 22 16:05:28 2009	(r200849)
+++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_pfil.c	Tue Dec 22 16:06:38 2009	(r200850)
@@ -125,7 +125,8 @@ ipfw_check_in(void *arg, struct mbuf **m
 	if (ng_tag != NULL) {
 		KASSERT(ng_tag->dir == NG_IPFW_IN,
 		    ("ng_ipfw tag with wrong direction"));
-		args.rule = ng_tag->rule;
+		args.slot = ng_tag->slot;
+		args.rulenum = ng_tag->rulenum;
 		args.rule_id = ng_tag->rule_id;
 		args.chain_id = ng_tag->chain_id;
 		m_tag_delete(*m0, (struct m_tag *)ng_tag);
@@ -137,7 +138,8 @@ again:
 		struct dn_pkt_tag *dt;
 
 		dt = (struct dn_pkt_tag *)(dn_tag+1);
-		args.rule = dt->rule;
+		args.slot = dt->slot;
+		args.rulenum = dt->rulenum;
 		args.rule_id = dt->rule_id;
 		args.chain_id = dt->chain_id;
 		m_tag_delete(*m0, dn_tag);
@@ -147,7 +149,7 @@ again:
 	args.inp = inp;
 	tee = 0;
 
-	if (V_fw_one_pass == 0 || args.rule == NULL) {
+	if (V_fw_one_pass == 0 || args.slot == 0) {
 		ipfw = ipfw_chk(&args);
 		*m0 = args.m;
 	} else
@@ -200,7 +202,7 @@ again:
 			*m0 = NULL;
 			return 0;	/* packet consumed */
 		} else {
-			args.rule = NULL;
+			args.slot = 0;
 			goto again;	/* continue with packet */
 		}
 
@@ -257,7 +259,8 @@ ipfw_check_out(void *arg, struct mbuf **
 	if (ng_tag != NULL) {
 		KASSERT(ng_tag->dir == NG_IPFW_OUT,
 		    ("ng_ipfw tag with wrong direction"));
-		args.rule = ng_tag->rule;
+		args.slot = ng_tag->slot;
+		args.rulenum = ng_tag->rulenum;
 		args.rule_id = ng_tag->rule_id;
 		args.chain_id = ng_tag->chain_id;
 		m_tag_delete(*m0, (struct m_tag *)ng_tag);
@@ -269,7 +272,8 @@ again:
 		struct dn_pkt_tag *dt;
 
 		dt = (struct dn_pkt_tag *)(dn_tag+1);
-		args.rule = dt->rule;
+		args.slot = dt->slot;
+		args.rulenum = dt->rulenum;
 		args.rule_id = dt->rule_id;
 		args.chain_id = dt->chain_id;
 		m_tag_delete(*m0, dn_tag);
@@ -280,7 +284,7 @@ again:
 	args.inp = inp;
 	tee = 0;
 
-	if (V_fw_one_pass == 0 || args.rule == NULL) {
+	if (V_fw_one_pass == 0 || args.slot == 0) {
 		ipfw = ipfw_chk(&args);
 		*m0 = args.m;
 	} else
@@ -339,7 +343,7 @@ again:
 			*m0 = NULL;
 			return 0;	/* packet consumed */
 		} else {
-			args.rule = NULL;
+			args.slot = 0;
 			goto again;	/* continue with packet */
 		}
 

Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_private.h
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_private.h	Tue Dec 22 16:05:28 2009	(r200849)
+++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_private.h	Tue Dec 22 16:06:38 2009	(r200850)
@@ -78,9 +78,16 @@ struct ip_fw_args {
 	struct mbuf	*m;		/* the mbuf chain		*/
 	struct ifnet	*oif;		/* output interface		*/
 	struct sockaddr_in *next_hop;	/* forward address		*/
-	struct ip_fw	*rule;		/* matching rule		*/
-	uint32_t	rule_id;	/* matching rule id */
-	uint32_t	chain_id;	/* ruleset id */
+
+	/* chain_id validates 'slot', the location of the pointer to
+	 * a matching rule.
+	 * If invalid, we can lookup the rule using rule_id and rulenum
+	 */
+	uint32_t	slot;		/* slot for matching rule	*/
+	uint32_t	rulenum;	/* matching rule number		*/
+	uint32_t	rule_id;	/* matching rule id		*/
+	uint32_t	chain_id;	/* ruleset id			*/
+
 	struct ether_header *eh;	/* for bridged packets		*/
 
 	struct ipfw_flow_id f_id;	/* grabbed from IP header	*/
@@ -212,13 +219,13 @@ struct sockopt;	/* used by tcp_var.h */
 #define IPFW_UH_RUNLOCK(p) rw_runlock(&(p)->uh_lock)
 #define IPFW_UH_WLOCK(p) rw_wlock(&(p)->uh_lock)
 #define IPFW_UH_WUNLOCK(p) rw_wunlock(&(p)->uh_lock)
+
 /* In ip_fw_sockopt.c */
 int ipfw_find_rule(struct ip_fw_chain *chain, uint32_t key, uint32_t id);
 int ipfw_add_rule(struct ip_fw_chain *chain, struct ip_fw *input_rule);
 int ipfw_ctl(struct sockopt *sopt);
 int ipfw_chk(struct ip_fw_args *args);
 void ipfw_reap_rules(struct ip_fw *head);
-void ipfw_free_chain(struct ip_fw_chain *chain, int kill_default);
 
 /* In ip_fw_table.c */
 struct radix_node;

Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_sockopt.c
==============================================================================
--- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_sockopt.c	Tue Dec 22 16:05:28 2009	(r200849)
+++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_sockopt.c	Tue Dec 22 16:06:38 2009	(r200850)
@@ -77,38 +77,11 @@ __FBSDID("$FreeBSD$");
 MALLOC_DEFINE(M_IPFW, "IpFw/IpAcct", "IpFw/IpAcct chain's");
 
 /*
- * static variables followed by global ones
+ * static variables followed by global ones (none in this file)
  */
 
-#ifdef SYSCTL_NODE
-SYSCTL_DECL(_net_inet_ip_fw);
-SYSCTL_VNET_INT(_net_inet_ip_fw, OID_AUTO, static_count,
-    CTLFLAG_RD, &VNET_NAME(layer3_chain.n_rules), 0,
-    "Number of static rules");
-
-#endif /* SYSCTL_NODE */
-
 /*
- * When a rule is added/deleted, clear the next_rule pointers in all rules.
- * These will be reconstructed on the fly as packets are matched.
- */
-static void
-flush_rule_ptrs(struct ip_fw_chain *chain)
-{
-#if 0
-	struct ip_fw *rule;
-
-	IPFW_WLOCK_ASSERT(chain);
-
-	for (rule = chain->rules; rule; rule = rule->next)
-		rule->next_rule = NULL;
-#endif
-	chain->id++;
-}
-
-/*
- * Find the smallest rule >= key, id. 
- * In case of multiple rules with the same number finds the first one.
+ * Find the smallest rule >= key, id.
  * We could use bsearch but it is so simple that we code it directly
  */
 int
@@ -117,28 +90,18 @@ ipfw_find_rule(struct ip_fw_chain *chain
 	int i, lo, hi;
 	struct ip_fw *r;
 
-	printf("looking for rule %d:%d\n", key, id);
   	for (lo = 0, hi = chain->n_rules - 1; lo < hi;) {
-		printf(" -- looking for rule %d:%d [%d-%d]\n", key, id, lo, hi);
 		i = (lo + hi) / 2;
 		r = chain->map[i];
 		if (r->rulenum < key)
-			lo = i + 1;
+			lo = i + 1;	/* continue from the next one */
 		else if (r->rulenum > key)
-			hi = i; // i - 1;
+			hi = i;		/* this might be good */
 		else if (r->id < id)
-			lo = i + 1;
-		else if (r->id > id)
-			hi = i; // i - 1;
-		else
-			hi = i;
+			lo = i + 1;	/* continue from the next one */
+		else /* r->id >= id */
+			hi = i;		/* this might be good */
 	};
-	printf("search end lo %d hi %d\n",lo, hi);
-	if (hi < 0)
-		hi = 0;
-	if (lo >= chain->n_rules - 1)
-		lo = chain->n_rules - 1;
-	printf(" -- looking for rule %d:%d found at %d\n", key, id, hi);
 	return hi;
 }
 
@@ -149,48 +112,38 @@ ipfw_find_rule(struct ip_fw_chain *chain
 static struct ip_fw **
 get_map(struct ip_fw_chain *chain, int extra, int locked)
 {
-	struct ip_fw **map;
 
 	for (;;) {
-		int i = chain->n_rules + extra;
-		map = malloc( (1 + i) * sizeof(struct ip_fw *), M_IPFW, M_WAITOK);
+		struct ip_fw **map;
+		int i;
+
+		i = chain->n_rules + extra;
+		map = malloc(i * sizeof(struct ip_fw *), M_IPFW, M_WAITOK);
 		if (map == NULL) {
 			printf("%s: cannot allocate map\n", __FUNCTION__);
-			break;
+			return NULL;
 		}
 		if (!locked)
 			IPFW_UH_WLOCK(chain);
-		if (i >= chain->n_rules + extra)
-			break;
+		if (i >= chain->n_rules + extra) /* good */
+			return map;
 		/* otherwise we lost the race, free and retry */
 		if (!locked)
 			IPFW_UH_WUNLOCK(chain);
 		free(map, M_IPFW);
 	}
-	return map;
 }
 
-/* swap the maps. It is supposed to be called with IPFW_UH_WLOCK
- * so we can do the remaining housekeepking outside
+/*
+ * swap the maps. It is supposed to be called with IPFW_UH_WLOCK
  */
 static struct ip_fw **
 swap_map(struct ip_fw_chain *chain, struct ip_fw **new_map, int new_len)
 {
 	struct ip_fw **old_map;
-	int i, lim;
 
 	IPFW_WLOCK(chain);
-	printf("%s %p %d --> %p %d\n", __FUNCTION__,
-		chain->map, chain->n_rules,
-		new_map, new_len);
-	flush_rule_ptrs(chain);
-	lim = chain->n_rules < new_len ? new_len : chain->n_rules;
-	for (i=0; i < lim; i++)
-		printf("%3d %p %p\n", i,
-			(i<chain->n_rules) ? chain->map[i] : NULL,
-			(i<new_len) ? new_map[i] : NULL);
-			
-	/* chain->id incremented inside flush_rule_ptrs() */
+	chain->id++;
 	chain->n_rules = new_len;
 	old_map = chain->map;
 	chain->map = new_map;
@@ -202,25 +155,27 @@ swap_map(struct ip_fw_chain *chain, stru
  * Add a new rule to the list. Copy the rule into a malloc'ed area, then
  * possibly create a rule number and add the rule to the list.
  * Update the rule_number in the input struct so the caller knows it as well.
- * XXX not good for the default rule.
+ * XXX DO NOT USE FOR THE DEFAULT RULE.
+ * Must be called without IPFW_UH held
  */
 int
 ipfw_add_rule(struct ip_fw_chain *chain, struct ip_fw *input_rule)
 {
 	struct ip_fw *rule;
-	int l = RULESIZE(input_rule);
-	int i, insert_before;
+	int i, l, insert_before;
 	struct ip_fw **map;	/* the new array of pointers */
 
 	if (chain->rules == NULL || input_rule->rulenum > IPFW_DEFAULT_RULE-1)
 		return (EINVAL);
 
+	l = RULESIZE(input_rule);
 	rule = malloc(l, M_IPFW, M_WAITOK | M_ZERO);
 	if (rule == NULL)
 		return (ENOSPC);
+	/* get_map returns with IPFW_UH_WLOCK if successful */
 	map = get_map(chain, 1, 0 /* not locked */);
 	if (map == NULL) {
-		free(map, M_IPFW);
+		free(rule, M_IPFW);
 		return ENOSPC;
 	}
 
@@ -236,21 +191,19 @@ ipfw_add_rule(struct ip_fw_chain *chain,
 		V_autoinc_step = 1;
 	else if (V_autoinc_step > 1000)
 		V_autoinc_step = 1000;
-	/* find the insertion point */
+	/* find the insertion point, we will insert before */
 	insert_before = rule->rulenum ? rule->rulenum + 1 : IPFW_DEFAULT_RULE;
 	i = ipfw_find_rule(chain, insert_before, 0);
 	/* duplicate first part */
-	printf("insert %d before %d (%d) at pos %d, +copy %d\n",
-		rule->rulenum, insert_before,
-		i>= 0 ? chain->map[i]->rulenum : -1, i, chain->n_rules - i);
 	if (i > 0)
 		bcopy(chain->map, map, i * sizeof(struct ip_fw *));
 	map[i] = rule;
 	/* duplicate remaining part, we always have the default rule */
-	bcopy(chain->map + i, map + i + 1, sizeof(struct ip_fw *) *(chain->n_rules - i));
+	bcopy(chain->map + i, map + i + 1,
+		sizeof(struct ip_fw *) *(chain->n_rules - i));
 	if (rule->rulenum == 0) {
-		/* set the number */
-		rule->rulenum = map[i]->rulenum;
+		/* write back the number */
+		rule->rulenum = i > 0 ? map[i-1]->rulenum : 0;
 		if (rule->rulenum < IPFW_DEFAULT_RULE - V_autoinc_step)
 			rule->rulenum += V_autoinc_step;
 		input_rule->rulenum = rule->rulenum;
@@ -262,30 +215,9 @@ ipfw_add_rule(struct ip_fw_chain *chain,
 	IPFW_UH_WUNLOCK(chain);
 	if (map)
 		free(map, M_IPFW);
-	DEB(printf("ipfw: installed rule %d, static count now %d\n",
-		rule->rulenum, chain->n_rules);)
 	return (0);
 }
 
-/**
- * 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 void
-remove_rule(struct ip_fw_chain *chain, struct ip_fw *rule)
-{
-	int l = RULESIZE(rule);
-
-	chain->static_len -= l;
-	ipfw_remove_dyn_children(rule);
-	rule->x_next = chain->reap;
-	chain->reap = rule;
-}
-
 /*
  * Reclaim storage associated with a list of rules.  This is
  * typically the list created using remove_rule.
@@ -302,31 +234,6 @@ ipfw_reap_rules(struct ip_fw *head)
 	}
 }
 
-/*
- * 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.
- */
-void
-ipfw_free_chain(struct ip_fw_chain *chain, int kill_default)
-{
-#if 0 // XXX
-	struct ip_fw *prev, *rule;
-
-	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)
-			rule = remove_rule(chain, rule);
-		else {
-			prev = rule;
-			rule = rule->next;
-		}
-#endif
-}
-
 /**
  * Remove all rules with given number, and also do set manipulation.
  * Assumes chain != NULL && *chain != NULL.
@@ -345,9 +252,9 @@ static int
 del_entry(struct ip_fw_chain *chain, u_int32_t arg)
 {
 	struct ip_fw *rule;
-	u_int16_t rulenum;	/* rule or old_set */
-	u_int8_t cmd, new_set;
-	int start, end, i, ofs, n;
+	uint32_t rulenum;	/* rule or old_set */
+	uint8_t cmd, new_set;
+	int start, end = 0, i, ofs, n;
 	struct ip_fw **map = NULL;
 	int error = 0;
 
@@ -367,81 +274,54 @@ del_entry(struct ip_fw_chain *chain, u_i
 
 	IPFW_UH_WLOCK(chain); /* prevent conflicts among the writers */
 	chain->reap = NULL;	/* prepare for deletions */
-	switch (cmd) {
-	case 0:	/* delete rules with given number */
-		/* locate first rule to delete */
-		start = ipfw_find_rule(chain, rulenum, 0);
-		/* count matching rules and end of range */
-		for (end = start, n = 0; end < chain->n_rules; end++) {
-			rule = chain->map[end];
-			if (rule->rulenum != rulenum)
-				break;
-			if (rule->set != RESVD_SET)
-				n++;
-		}
-		printf("must delete %d rules between %d and %d\n", n, start, end);
-		/* allocate the map, if needed */
-		if (n > 0)
-			map = get_map(chain, -n, 1 /* locked */);
-		if (n == 0 || map == NULL) {
-			error = EINVAL;
-			goto done;
-		}
-		/* copy the initial part of the map */
-		if (start > 0)
-			bcopy(chain->map, map, start * sizeof(struct ip_fw *));
-		/* copy active rules between start and end */
-		for (i = ofs = start; i < end; i++) {
-			rule = chain->map[i];
-			if (rule->set == RESVD_SET)
-				map[ofs++] = chain->map[i];
-		}
-		/* finally the tail */
-		bcopy(chain->map + end, map + ofs,
-			(chain->n_rules - end) * sizeof(struct ip_fw *));
-		map = swap_map(chain, map, chain->n_rules - n);
-		/* now remove the rules deleted */
-		for (i = start; i < end; i++) {
-			rule = map[i];
-			printf("about to remove rule %p at %d\n",
-				rule, i);
-			if (rule->set != RESVD_SET)
-				remove_rule(chain, rule);
-		}
-		printf("done with removals\n");
-		break;
 
-	case 1:	/* delete all rules with given set number */
-		IPFW_UH_WLOCK(chain);
-		/* locate first rule to delete */
-		for (start = 0; start < chain->n_rules; start++) {
-			rule = chain->map[start];
-			if (rule->set != rulenum)
-				break;
-		}
-		for (n = 0, end = i = start; i < chain->n_rules; i++) {
-			rule = chain->map[i];
-			if (rule->set == rulenum) {
+	switch (cmd) {
+	case 0:	/* delete rules with given number (0 is special means all) */
+	case 1:	/* delete all rules with given set number, rule->set == rulenum */
+	case 5: /* delete rules with given number and with given set number.
+		 * rulenum - given rule number;
+		 * new_set - given set number.
+		 */
+		/* locate first rule to delete (start), the one after the
+		 * last one (end), and count how many rules to delete (n)
+		 */
+		n = 0;
+		if (cmd == 1) { /* look for a specific set, must scan all */
+			for (start = -1, i = 0; i < chain->n_rules; i++) {
+				if (chain->map[start]->set != rulenum)
+					continue;
+				if (start < 0)
+					start = i;
 				end = i;
 				n++;
 			}
+			end++;	/* first non-matching */
+		} else {
+			start = ipfw_find_rule(chain, rulenum, 0);
+			for (end = start; end < chain->n_rules; end++) {
+				rule = chain->map[end];
+				if (rulenum > 0 && rule->rulenum != rulenum)
+					break;
+				if (rule->set != RESVD_SET &&
+				    (cmd == 0 || rule->set == new_set) )
+					n++;
+			}
 		}
-		end++;	/* first non-matching */
-		printf("must delete %d rules between %d and %d\n", n, start, end);
 		/* allocate the map, if needed */
 		if (n > 0)
 			map = get_map(chain, -n, 1 /* locked */);
 		if (n == 0 || map == NULL) {
 			error = EINVAL;
-			goto done;
+			break;
 		}
 		/* copy the initial part of the map */
 		if (start > 0)
 			bcopy(chain->map, map, start * sizeof(struct ip_fw *));
-		/* copy reserved rules */
+		/* copy active rules between start and end */
 		for (i = ofs = start; i < end; i++) {
 			rule = chain->map[i];
-			if (rule->set != rulenum)
+			if (!(rule->set != RESVD_SET &&
+			    (cmd == 0 || rule->set == new_set) ))
 				map[ofs++] = chain->map[i];
 		}
 		/* finally the tail */
@@ -451,8 +331,15 @@ del_entry(struct ip_fw_chain *chain, u_i
 		/* now remove the rules deleted */
 		for (i = start; i < end; i++) {
 			rule = map[i];
-			if (rule->set == rulenum)
-				remove_rule(chain, map[i]);
+			if (rule->set != RESVD_SET &&
+			    (cmd == 0 || rule->set == new_set) ) {
+				int l = RULESIZE(rule);
+
+				chain->static_len -= l;
+				ipfw_remove_dyn_children(rule);
+				rule->x_next = chain->reap;
+				chain->reap = rule;
+			}
 		}
 		break;
 
@@ -487,36 +374,9 @@ del_entry(struct ip_fw_chain *chain, u_i
 		}
 		IPFW_UH_WUNLOCK(chain);
 		break;
-
-#if 0 // XXX case 5 is a restriction of 1
-	case 5: /* delete rules with given number and with given set number.
-		 * rulenum - given rule number;
-		 * new_set - given set number.
-		 */
-		for (; rule->rulenum < rulenum; prev = rule, rule = rule->next)
-			;
-		if (rule->rulenum != rulenum) {
-			IPFW_WUNLOCK(chain);
-			return (EINVAL);
-		}
-		flush_rule_ptrs(chain);
-		while (rule->rulenum == rulenum) {
-			if (rule->set == new_set)
-				rule = remove_rule(chain, rule, prev);
-			else {
-				prev = rule;
-				rule = rule->next;
-			}
-		}
-#endif
 	}
-	/*
-	 * Look for rules to reclaim.  We grab the list before
-	 * releasing the lock then reclaim them w/o the lock to

*** DIFF OUTPUT TRUNCATED AT 1000 LINES ***


More information about the svn-src-user mailing list