Author: luigi
Date: Tue Dec 22 13:53:34 2009
New Revision: 200838
URL: http://svn.freebsd.org/changeset/base/200838
Log:
some mostly cosmetic changes in preparation for upcoming work:
+ in many places, replace &V_layer3_chain with a local
variable chain;
+ bring the counter of rules and static_len within ip_fw_chain
replacing static variables;
+ remove some spurious comments and extern declaration;
+ document which lock protects certain data structures
Modified:
head/sys/netinet/ipfw/ip_fw2.c
head/sys/netinet/ipfw/ip_fw_private.h
head/sys/netinet/ipfw/ip_fw_sockopt.c
head/sys/netinet/ipfw/ip_fw_table.c
Modified: head/sys/netinet/ipfw/ip_fw2.c
==============================================================================
--- head/sys/netinet/ipfw/ip_fw2.c Tue Dec 22 13:49:37 2009
(r200837)
+++ head/sys/netinet/ipfw/ip_fw2.c Tue Dec 22 13:53:34 2009
(r200838)
@@ -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);
@@ -739,6 +738,17 @@ check_uidgid(ipfw_insn_u32 *insn, int pr
}
/*
+ * 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)
+{
+ args->rule = f;
+ args->rule_id = f->id;
+ args->chain_id = chain->id;
+}
+
+/*
* The main check routine for the firewall.
*
* All arguments are in args so we can modify them and return them
@@ -1942,13 +1952,9 @@ do {
\
case O_PIPE:
case O_QUEUE:
- args->rule = f; /* report matching rule */
- args->rule_id = f->id;
- args->chain_id = chain->id;
- if (cmd->arg1 == IP_FW_TABLEARG)
- args->cookie = tablearg;
- else
- args->cookie = cmd->arg1;
+ set_match(args, f, chain);
+ args->cookie = (cmd->arg1 == IP_FW_TABLEARG) ?
+ tablearg : cmd->arg1;
retval = IP_FW_DUMMYNET;
l = 0; /* exit inner loop */
done = 1; /* exit outer loop */
@@ -2077,13 +2083,9 @@ do {
\
case O_NETGRAPH:
case O_NGTEE:
- args->rule = f; /* report matching rule */
- args->rule_id = f->id;
- args->chain_id = chain->id;
- if (cmd->arg1 == IP_FW_TABLEARG)
- args->cookie = tablearg;
- else
- args->cookie = cmd->arg1;
+ set_match(args, 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 */
@@ -2106,14 +2108,12 @@ do {
\
struct cfg_nat *t;
int nat_id;
- args->rule = f; /* Report matching rule. */
- args->rule_id = f->id;
- args->chain_id = chain->id;
+ set_match(args, f, chain);
t = ((ipfw_insn_nat *)cmd)->nat;
if (t == NULL) {
nat_id = (cmd->arg1 == IP_FW_TABLEARG) ?
tablearg : cmd->arg1;
- t =
(*lookup_nat_ptr)(&V_layer3_chain.nat, nat_id);
+ t = (*lookup_nat_ptr)(&chain->nat,
nat_id);
if (t == NULL) {
retval = IP_FW_DENY;
@@ -2175,9 +2175,7 @@ do {
\
else
ip->ip_sum = in_cksum(m, hlen);
retval = IP_FW_REASS;
- args->rule = f;
- args->rule_id = f->id;
- args->chain_id = chain->id;
+ set_match(args, f, chain);
}
done = 1; /* exit outer loop */
break;
@@ -2311,8 +2309,13 @@ vnet_ipfw_init(const void *unused)
{
int error;
struct ip_fw default_rule;
+ struct ip_fw_chain *chain;
+
+ chain = &V_layer3_chain;
/* First set up some values that are compile time options */
+ V_autoinc_step = 100; /* bounded to 1..1000 in add_rule() */
+ V_fw_deny_unknown_exthdrs = 1;
#ifdef IPFIREWALL_VERBOSE
V_fw_verbose = 1;
#endif
@@ -2320,20 +2323,17 @@ vnet_ipfw_init(const void *unused)
V_verbose_limit = IPFIREWALL_VERBOSE_LIMIT;
#endif
- error = ipfw_init_tables(&V_layer3_chain);
+ error = ipfw_init_tables(chain);
if (error) {
panic("init_tables"); /* XXX Marko fix this ! */
}
#ifdef IPFIREWALL_NAT
- LIST_INIT(&V_layer3_chain.nat);
+ LIST_INIT(&chain->nat);
#endif
- V_autoinc_step = 100; /* bounded to 1..1000 in add_rule() */
-
- V_fw_deny_unknown_exthdrs = 1;
- V_layer3_chain.rules = NULL;
- IPFW_LOCK_INIT(&V_layer3_chain);
+ chain->rules = NULL;
+ IPFW_LOCK_INIT(chain);
bzero(&default_rule, sizeof default_rule);
default_rule.act_ofs = 0;
@@ -2342,17 +2342,17 @@ vnet_ipfw_init(const void *unused)
default_rule.set = RESVD_SET;
default_rule.cmd[0].len = 1;
default_rule.cmd[0].opcode = default_to_accept ? O_ACCEPT : O_DENY;
- error = ipfw_add_rule(&V_layer3_chain, &default_rule);
+ error = ipfw_add_rule(chain, &default_rule);
if (error != 0) {
printf("ipfw2: error %u initializing default rule "
"(support disabled)\n", error);
- IPFW_LOCK_DESTROY(&V_layer3_chain);
+ IPFW_LOCK_DESTROY(chain);
printf("leaving ipfw_iattach (1) with error %d\n", error);
return (error);
}
- V_layer3_chain.default_rule = V_layer3_chain.rules;
+ chain->default_rule = chain->rules;
ipfw_dyn_init();
@@ -2386,6 +2386,7 @@ static int
vnet_ipfw_uninit(const void *unused)
{
struct ip_fw *reap;
+ struct ip_fw_chain *chain = &V_layer3_chain;
V_ipfw_vnet_ready = 0; /* tell new callers to go away */
/*
@@ -2400,21 +2401,21 @@ vnet_ipfw_uninit(const void *unused)
V_ip_fw_chk_ptr = NULL;
V_ip_fw_ctl_ptr = NULL;
- IPFW_WLOCK(&V_layer3_chain);
+ IPFW_WLOCK(chain);
/* We wait on the wlock here until the last user leaves */
- IPFW_WUNLOCK(&V_layer3_chain);
- IPFW_WLOCK(&V_layer3_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);
+ chain->reap = NULL;
+ ipfw_free_chain(chain, 1 /* kill default rule */);
+ reap = chain->reap;
+ chain->reap = NULL;
+ IPFW_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: head/sys/netinet/ipfw/ip_fw_private.h
==============================================================================
--- head/sys/netinet/ipfw/ip_fw_private.h Tue Dec 22 13:49:37 2009
(r200837)
+++ head/sys/netinet/ipfw/ip_fw_private.h Tue Dec 22 13:53:34 2009
(r200838)
@@ -78,9 +78,11 @@ 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 */
+ 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 */
@@ -174,6 +176,8 @@ struct ip_fw_chain {
struct ip_fw *rules; /* list of rules */
struct ip_fw *reap; /* list of rules to reap */
struct ip_fw *default_rule;
+ int n_rules; /* number of static rules */
+ int static_len; /* total len of static rules */
LIST_HEAD(nat_list, cfg_nat) nat; /* list of nat entries */
struct radix_node_head *tables[IPFW_TABLES_MAX];
struct rwlock rwmtx;
Modified: head/sys/netinet/ipfw/ip_fw_sockopt.c
==============================================================================
--- head/sys/netinet/ipfw/ip_fw_sockopt.c Tue Dec 22 13:49:37 2009
(r200837)
+++ head/sys/netinet/ipfw/ip_fw_sockopt.c Tue Dec 22 13:53:34 2009
(r200838)
@@ -77,23 +77,9 @@ __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)
*/
-static VNET_DEFINE(u_int32_t, static_count); /* # of static rules */
-#define V_static_count VNET(static_count)
-
-static VNET_DEFINE(u_int32_t, static_len); /* bytes of static rules */
-#define V_static_len VNET(static_len)
-
-#ifdef SYSCTL_NODE
-SYSCTL_DECL(_net_inet_ip_fw);
-SYSCTL_VNET_INT(_net_inet_ip_fw, OID_AUTO, static_count,
- CTLFLAG_RD, &VNET_NAME(static_count), 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.
@@ -187,11 +173,9 @@ ipfw_add_rule(struct ip_fw_chain *chain,
/* chain->id incremented inside flush_rule_ptrs() */
rule->id = chain->id;
done:
- V_static_count++;
- V_static_len += l;
+ chain->n_rules++;
+ chain->static_len += l;
IPFW_WUNLOCK(chain);
- DEB(printf("ipfw: installed rule %d, static count now %d\n",
- rule->rulenum, V_static_count);)
return (0);
}
@@ -218,8 +202,8 @@ remove_rule(struct ip_fw_chain *chain, s
chain->rules = n;
else
prev->next = n;
- V_static_count--;
- V_static_len -= l;
+ chain->n_rules--;
+ chain->static_len -= l;
rule->next = chain->reap;
chain->reap = rule;
@@ -839,6 +823,7 @@ ipfw_ctl(struct sockopt *sopt)
int error;
size_t size;
struct ip_fw *buf, *rule;
+ struct ip_fw_chain *chain;
u_int32_t rulenum[2];
error = priv_check(sopt->sopt_td, PRIV_NETINET_IPFW);
@@ -856,6 +841,7 @@ ipfw_ctl(struct sockopt *sopt)
return (error);
}
+ chain = &V_layer3_chain;
error = 0;
switch (sopt->sopt_name) {
@@ -871,7 +857,7 @@ ipfw_ctl(struct sockopt *sopt)
* change between calculating the size and returning the
* data in which case we'll just return what fits.
*/
- size = V_static_len; /* size of static rules */
+ size = chain->static_len; /* size of static rules */
size += ipfw_dyn_len();
if (size >= sopt->sopt_valsize)
@@ -883,7 +869,7 @@ ipfw_ctl(struct sockopt *sopt)
*/
buf = malloc(size, M_TEMP, M_WAITOK);
error = sooptcopyout(sopt, buf,
- ipfw_getrules(&V_layer3_chain, buf, size));
+ ipfw_getrules(chain, buf, size));
free(buf, M_TEMP);
break;
@@ -901,10 +887,10 @@ ipfw_ctl(struct sockopt *sopt)
* the old list without the need for a lock.
*/
- IPFW_WLOCK(&V_layer3_chain);
- ipfw_free_chain(&V_layer3_chain, 0 /* keep default rule */);
- rule = V_layer3_chain.reap;
- IPFW_WUNLOCK(&V_layer3_chain);
+ IPFW_WLOCK(chain);
+ ipfw_free_chain(chain, 0 /* keep default rule */);
+ rule = chain->reap;
+ IPFW_WUNLOCK(chain);
ipfw_reap_rules(rule);
break;
@@ -915,7 +901,7 @@ ipfw_ctl(struct sockopt *sopt)
if (error == 0)
error = check_ipfw_struct(rule, sopt->sopt_valsize);
if (error == 0) {
- error = ipfw_add_rule(&V_layer3_chain, rule);
+ error = ipfw_add_rule(chain, rule);
size = RULESIZE(rule);
if (!error && sopt->sopt_dir == SOPT_GET)
error = sooptcopyout(sopt, rule, size);
@@ -941,13 +927,14 @@ ipfw_ctl(struct sockopt *sopt)
if (error)
break;
size = sopt->sopt_valsize;
- if (size == sizeof(u_int32_t)) /* delete or reassign */
- error = del_entry(&V_layer3_chain, rulenum[0]);
- else if (size == 2*sizeof(u_int32_t)) /* set enable/disable */
+ if (size == sizeof(u_int32_t) && rulenum[0] != 0) {
+ /* delete or reassign, locking done in del_entry() */
+ error = del_entry(chain, rulenum[0]);
+ } else if (size == 2*sizeof(u_int32_t)) { /* set enable/disable
*/
V_set_disable =
(V_set_disable | rulenum[0]) & ~rulenum[1] &
~(1<<RESVD_SET); /* set RESVD_SET always enabled */
- else
+ } else
error = EINVAL;
break;
@@ -960,10 +947,11 @@ ipfw_ctl(struct sockopt *sopt)
if (error)
break;
}
- error = zero_entry(&V_layer3_chain, rulenum[0],
+ error = zero_entry(chain, rulenum[0],
sopt->sopt_name == IP_FW_RESETLOG);
break;
+ /*--- TABLE manipulations are protected by the IPFW_LOCK ---*/
case IP_FW_TABLE_ADD:
{
ipfw_table_entry ent;
@@ -972,7 +960,7 @@ ipfw_ctl(struct sockopt *sopt)
sizeof(ent), sizeof(ent));
if (error)
break;
- error = ipfw_add_table_entry(&V_layer3_chain, ent.tbl,
+ error = ipfw_add_table_entry(chain, ent.tbl,
ent.addr, ent.masklen, ent.value);
}
break;
@@ -985,7 +973,7 @@ ipfw_ctl(struct sockopt *sopt)
sizeof(ent), sizeof(ent));
if (error)
break;
- error = ipfw_del_table_entry(&V_layer3_chain, ent.tbl,
+ error = ipfw_del_table_entry(chain, ent.tbl,
ent.addr, ent.masklen);
}
break;
@@ -998,9 +986,9 @@ ipfw_ctl(struct sockopt *sopt)
sizeof(tbl), sizeof(tbl));
if (error)
break;
- IPFW_WLOCK(&V_layer3_chain);
- error = ipfw_flush_table(&V_layer3_chain, tbl);
- IPFW_WUNLOCK(&V_layer3_chain);
+ IPFW_WLOCK(chain);
+ error = ipfw_flush_table(chain, tbl);
+ IPFW_WUNLOCK(chain);
}
break;
@@ -1011,9 +999,9 @@ ipfw_ctl(struct sockopt *sopt)
if ((error = sooptcopyin(sopt, &tbl, sizeof(tbl),
sizeof(tbl))))
break;
- IPFW_RLOCK(&V_layer3_chain);
- error = ipfw_count_table(&V_layer3_chain, tbl, &cnt);
- IPFW_RUNLOCK(&V_layer3_chain);
+ IPFW_RLOCK(chain);
+ error = ipfw_count_table(chain, tbl, &cnt);
+ IPFW_RUNLOCK(chain);
if (error)
break;
error = sooptcopyout(sopt, &cnt, sizeof(cnt));
@@ -1037,9 +1025,9 @@ ipfw_ctl(struct sockopt *sopt)
}
tbl->size = (size - sizeof(*tbl)) /
sizeof(ipfw_table_entry);
- IPFW_RLOCK(&V_layer3_chain);
- error = ipfw_dump_table(&V_layer3_chain, tbl);
- IPFW_RUNLOCK(&V_layer3_chain);
+ IPFW_RLOCK(chain);
+ error = ipfw_dump_table(chain, tbl);
+ IPFW_RUNLOCK(chain);
if (error) {
free(tbl, M_TEMP);
break;
@@ -1049,6 +1037,7 @@ ipfw_ctl(struct sockopt *sopt)
}
break;
+ /*--- NAT operations are protected by the IPFW_LOCK ---*/
case IP_FW_NAT_CFG:
if (IPFW_NAT_LOADED)
error = ipfw_nat_cfg_ptr(sopt);
Modified: head/sys/netinet/ipfw/ip_fw_table.c
==============================================================================
--- head/sys/netinet/ipfw/ip_fw_table.c Tue Dec 22 13:49:37 2009
(r200837)
+++ head/sys/netinet/ipfw/ip_fw_table.c Tue Dec 22 13:53:34 2009
(r200838)
@@ -34,6 +34,9 @@ __FBSDID("$FreeBSD$");
* keys are network prefixes (addr/masklen), and values are integers.
* As a degenerate case we can interpret keys as 32-bit integers
* (with a /32 mask).
+ *
+ * The table is protected by the IPFW lock even for manipulation coming
+ * from userland, because operations are typically fast.
*/
#if !defined(KLD_MODULE)
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"
