So it's not a good idea to perform long lasting operations in the kernel.
The scheduler doesn't deal well with it and nobody else gets to run.
One of those long loops is loading a large table into pf. If you're
lucky, you'll run out of memory and pool will finally sleep.
I stuck a couple yield() calls into the long loops after sufficient
iteration.
I also zapped PFR_FLAG_ATOMIC because it's not really atomic anyway. I
also couldn't find any callers. Leftover?
Another thing to fix at some point is that we call splsoftnet and splx
multiple times per address in some cases, but fixing that was getting too
complicated and requires some more code shuffling.
Index: pf_table.c
===================================================================
RCS file: /home/tedu/cvs/src/sys/net/pf_table.c,v
retrieving revision 1.86
diff -u -r1.86 pf_table.c
--- pf_table.c 30 Sep 2010 07:14:02 -0000 1.86
+++ pf_table.c 13 Oct 2010 23:47:01 -0000
@@ -213,9 +213,8 @@
{
struct pfr_ktable *kt;
struct pfr_kentryworkq workq;
- int s;
- ACCEPT_FLAGS(flags, PFR_FLAG_ATOMIC | PFR_FLAG_DUMMY);
+ ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY);
if (pfr_validate_table(tbl, 0, flags & PFR_FLAG_USERIOCTL))
return (EINVAL);
kt = pfr_lookup_table(tbl);
@@ -226,11 +225,7 @@
pfr_enqueue_addrs(kt, &workq, ndel, 0);
if (!(flags & PFR_FLAG_DUMMY)) {
- if (flags & PFR_FLAG_ATOMIC)
- s = splsoftnet();
pfr_remove_kentries(kt, &workq);
- if (flags & PFR_FLAG_ATOMIC)
- splx(s);
if (kt->pfrkt_cnt) {
DPFPRINTF(LOG_NOTICE,
"pfr_clr_addrs: corruption detected (%d).",
@@ -249,11 +244,10 @@
struct pfr_kentryworkq workq;
struct pfr_kentry *p, *q;
struct pfr_addr ad;
- int i, rv, s, xadd = 0;
+ int i, rv, xadd = 0;
long tzero = time_second;
- ACCEPT_FLAGS(flags, PFR_FLAG_ATOMIC | PFR_FLAG_DUMMY |
- PFR_FLAG_FEEDBACK);
+ ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY | PFR_FLAG_FEEDBACK);
if (pfr_validate_table(tbl, 0, flags & PFR_FLAG_USERIOCTL))
return (EINVAL);
kt = pfr_lookup_table(tbl);
@@ -267,6 +261,8 @@
return (ENOMEM);
SLIST_INIT(&workq);
for (i = 0; i < size; i++) {
+ if (i % 1000 == 0)
+ yield();
if (COPYIN(addr+i, &ad, sizeof(ad), flags))
senderr(EFAULT);
if (pfr_validate_addr(&ad))
@@ -302,11 +298,7 @@
}
pfr_clean_node_mask(tmpkt, &workq);
if (!(flags & PFR_FLAG_DUMMY)) {
- if (flags & PFR_FLAG_ATOMIC)
- s = splsoftnet();
pfr_insert_kentries(kt, &workq, tzero);
- if (flags & PFR_FLAG_ATOMIC)
- splx(s);
} else
pfr_destroy_kentries(&workq);
if (nadd != NULL)
@@ -330,10 +322,9 @@
struct pfr_kentryworkq workq;
struct pfr_kentry *p;
struct pfr_addr ad;
- int i, rv, s, xdel = 0, log = 1;
+ int i, rv, xdel = 0, log = 1;
- ACCEPT_FLAGS(flags, PFR_FLAG_ATOMIC | PFR_FLAG_DUMMY |
- PFR_FLAG_FEEDBACK);
+ ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY | PFR_FLAG_FEEDBACK);
if (pfr_validate_table(tbl, 0, flags & PFR_FLAG_USERIOCTL))
return (EINVAL);
kt = pfr_lookup_table(tbl);
@@ -399,11 +390,7 @@
senderr(EFAULT);
}
if (!(flags & PFR_FLAG_DUMMY)) {
- if (flags & PFR_FLAG_ATOMIC)
- s = splsoftnet();
pfr_remove_kentries(kt, &workq);
- if (flags & PFR_FLAG_ATOMIC)
- splx(s);
}
if (ndel != NULL)
*ndel = xdel;
@@ -423,11 +410,10 @@
struct pfr_kentryworkq addq, delq, changeq;
struct pfr_kentry *p, *q;
struct pfr_addr ad;
- int i, rv, s, xadd = 0, xdel = 0, xchange = 0;
+ int i, rv, xadd = 0, xdel = 0, xchange = 0;
long tzero = time_second;
- ACCEPT_FLAGS(flags, PFR_FLAG_ATOMIC | PFR_FLAG_DUMMY |
- PFR_FLAG_FEEDBACK);
+ ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY | PFR_FLAG_FEEDBACK);
if (pfr_validate_table(tbl, ignore_pfrt_flags, flags &
PFR_FLAG_USERIOCTL))
return (EINVAL);
@@ -502,13 +488,9 @@
}
pfr_clean_node_mask(tmpkt, &addq);
if (!(flags & PFR_FLAG_DUMMY)) {
- if (flags & PFR_FLAG_ATOMIC)
- s = splsoftnet();
pfr_insert_kentries(kt, &addq, tzero);
pfr_remove_kentries(kt, &delq);
pfr_clstats_kentries(&changeq, tzero, INVERT_NEG_FLAG);
- if (flags & PFR_FLAG_ATOMIC)
- splx(s);
} else
pfr_destroy_kentries(&addq);
if (nadd != NULL)
@@ -615,11 +597,9 @@
struct pfr_ktable *kt;
struct pfr_walktree w;
struct pfr_kentryworkq workq;
- int rv, s;
+ int rv;
long tzero = time_second;
- /* XXX PFR_FLAG_CLSTATS disabled */
- ACCEPT_FLAGS(flags, PFR_FLAG_ATOMIC);
if (pfr_validate_table(tbl, 0, 0))
return (EINVAL);
kt = pfr_lookup_table(tbl);
@@ -635,8 +615,6 @@
w.pfrw_astats = addr;
w.pfrw_free = kt->pfrkt_cnt;
w.pfrw_flags = flags;
- if (flags & PFR_FLAG_ATOMIC)
- s = splsoftnet();
rv = rn_walktree(kt->pfrkt_ip4, pfr_walktree, &w);
if (!rv)
rv = rn_walktree(kt->pfrkt_ip6, pfr_walktree, &w);
@@ -644,8 +622,6 @@
pfr_enqueue_addrs(kt, &workq, NULL, 0);
pfr_clstats_kentries(&workq, tzero, 0);
}
- if (flags & PFR_FLAG_ATOMIC)
- splx(s);
if (rv)
return (rv);
@@ -666,10 +642,9 @@
struct pfr_kentryworkq workq;
struct pfr_kentry *p;
struct pfr_addr ad;
- int i, rv, s, xzero = 0;
+ int i, rv, xzero = 0;
- ACCEPT_FLAGS(flags, PFR_FLAG_ATOMIC | PFR_FLAG_DUMMY |
- PFR_FLAG_FEEDBACK);
+ ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY | PFR_FLAG_FEEDBACK);
if (pfr_validate_table(tbl, 0, 0))
return (EINVAL);
kt = pfr_lookup_table(tbl);
@@ -695,11 +670,7 @@
}
if (!(flags & PFR_FLAG_DUMMY)) {
- if (flags & PFR_FLAG_ATOMIC)
- s = splsoftnet();
pfr_clstats_kentries(&workq, 0, 0);
- if (flags & PFR_FLAG_ATOMIC)
- splx(s);
}
if (nzero != NULL)
*nzero = xzero;
@@ -854,10 +825,13 @@
pfr_destroy_kentries(struct pfr_kentryworkq *workq)
{
struct pfr_kentry *p, *q;
+ int n = 0;
for (p = SLIST_FIRST(workq); p != NULL; p = q) {
q = SLIST_NEXT(p, pfrke_workq);
pfr_destroy_kentry(p);
+ if (++n % 1000 == 0)
+ yield();
}
}
@@ -885,7 +859,8 @@
break;
}
p->pfrke_tzero = tzero;
- n++;
+ if (++n % 1000 == 0)
+ yield();
}
kt->pfrkt_cnt += n;
}
@@ -922,7 +897,8 @@
SLIST_FOREACH(p, workq, pfrke_workq) {
pfr_unroute_kentry(kt, p);
- n++;
+ if (++n % 1000 == 0)
+ yield();
}
kt->pfrkt_cnt -= n;
pfr_destroy_kentries(workq);
@@ -933,9 +909,13 @@
struct pfr_kentryworkq *workq)
{
struct pfr_kentry *p;
+ int n = 0;
- SLIST_FOREACH(p, workq, pfrke_workq)
+ SLIST_FOREACH(p, workq, pfrke_workq) {
pfr_unroute_kentry(kt, p);
+ if (++n % 1000 == 0)
+ yield();
+ }
}
void
@@ -1161,10 +1141,9 @@
{
struct pfr_ktableworkq workq;
struct pfr_ktable *p;
- int s, xdel = 0;
+ int xdel = 0;
- ACCEPT_FLAGS(flags, PFR_FLAG_ATOMIC | PFR_FLAG_DUMMY |
- PFR_FLAG_ALLRSETS);
+ ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY | PFR_FLAG_ALLRSETS);
if (pfr_fix_anchor(filter->pfrt_anchor))
return (EINVAL);
if (pfr_table_count(filter, flags) < 0)
@@ -1183,11 +1162,7 @@
xdel++;
}
if (!(flags & PFR_FLAG_DUMMY)) {
- if (flags & PFR_FLAG_ATOMIC)
- s = splsoftnet();
pfr_setflags_ktables(&workq);
- if (flags & PFR_FLAG_ATOMIC)
- splx(s);
}
if (ndel != NULL)
*ndel = xdel;
@@ -1199,10 +1174,10 @@
{
struct pfr_ktableworkq addq, changeq;
struct pfr_ktable *p, *q, *r, key;
- int i, rv, s, xadd = 0;
+ int i, rv, xadd = 0;
long tzero = time_second;
- ACCEPT_FLAGS(flags, PFR_FLAG_ATOMIC | PFR_FLAG_DUMMY);
+ ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY);
SLIST_INIT(&addq);
SLIST_INIT(&changeq);
for (i = 0; i < size; i++) {
@@ -1260,12 +1235,8 @@
;
}
if (!(flags & PFR_FLAG_DUMMY)) {
- if (flags & PFR_FLAG_ATOMIC)
- s = splsoftnet();
pfr_insert_ktables(&addq);
pfr_setflags_ktables(&changeq);
- if (flags & PFR_FLAG_ATOMIC)
- splx(s);
} else
pfr_destroy_ktables(&addq, 0);
if (nadd != NULL)
@@ -1281,9 +1252,9 @@
{
struct pfr_ktableworkq workq;
struct pfr_ktable *p, *q, key;
- int i, s, xdel = 0;
+ int i, xdel = 0;
- ACCEPT_FLAGS(flags, PFR_FLAG_ATOMIC | PFR_FLAG_DUMMY);
+ ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY);
SLIST_INIT(&workq);
for (i = 0; i < size; i++) {
if (COPYIN(tbl+i, &key.pfrkt_t, sizeof(key.pfrkt_t), flags))
@@ -1305,11 +1276,7 @@
}
if (!(flags & PFR_FLAG_DUMMY)) {
- if (flags & PFR_FLAG_ATOMIC)
- s = splsoftnet();
pfr_setflags_ktables(&workq);
- if (flags & PFR_FLAG_ATOMIC)
- splx(s);
}
if (ndel != NULL)
*ndel = xdel;
@@ -1360,7 +1327,7 @@
long tzero = time_second;
/* XXX PFR_FLAG_CLSTATS disabled */
- ACCEPT_FLAGS(flags, PFR_FLAG_ATOMIC | PFR_FLAG_ALLRSETS);
+ ACCEPT_FLAGS(flags, PFR_FLAG_ALLRSETS);
if (pfr_fix_anchor(filter->pfrt_anchor))
return (EINVAL);
n = nn = pfr_table_count(filter, flags);
@@ -1371,28 +1338,22 @@
return (0);
}
SLIST_INIT(&workq);
- if (flags & PFR_FLAG_ATOMIC)
- s = splsoftnet();
RB_FOREACH(p, pfr_ktablehead, &pfr_ktables) {
if (pfr_skip_table(filter, p, flags))
continue;
if (n-- <= 0)
continue;
- if (!(flags & PFR_FLAG_ATOMIC))
- s = splsoftnet();
+ s = splsoftnet();
if (COPYOUT(&p->pfrkt_ts, tbl++, sizeof(*tbl), flags)) {
splx(s);
return (EFAULT);
}
- if (!(flags & PFR_FLAG_ATOMIC))
- splx(s);
+ splx(s);
SLIST_INSERT_HEAD(&workq, p, pfrkt_workq);
}
if (flags & PFR_FLAG_CLSTATS)
pfr_clstats_ktables(&workq, tzero,
flags & PFR_FLAG_ADDRSTOO);
- if (flags & PFR_FLAG_ATOMIC)
- splx(s);
if (n) {
DPFPRINTF(LOG_ERR,
"pfr_get_tstats: corruption detected (%d).", n);
@@ -1407,11 +1368,10 @@
{
struct pfr_ktableworkq workq;
struct pfr_ktable *p, key;
- int i, s, xzero = 0;
+ int i, xzero = 0;
long tzero = time_second;
- ACCEPT_FLAGS(flags, PFR_FLAG_ATOMIC | PFR_FLAG_DUMMY |
- PFR_FLAG_ADDRSTOO);
+ ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY | PFR_FLAG_ADDRSTOO);
SLIST_INIT(&workq);
for (i = 0; i < size; i++) {
if (COPYIN(tbl+i, &key.pfrkt_t, sizeof(key.pfrkt_t), flags))
@@ -1425,11 +1385,7 @@
}
}
if (!(flags & PFR_FLAG_DUMMY)) {
- if (flags & PFR_FLAG_ATOMIC)
- s = splsoftnet();
pfr_clstats_ktables(&workq, tzero, flags & PFR_FLAG_ADDRSTOO);
- if (flags & PFR_FLAG_ATOMIC)
- splx(s);
}
if (nzero != NULL)
*nzero = xzero;
@@ -1442,9 +1398,9 @@
{
struct pfr_ktableworkq workq;
struct pfr_ktable *p, *q, key;
- int i, s, xchange = 0, xdel = 0;
+ int i, xchange = 0, xdel = 0;
- ACCEPT_FLAGS(flags, PFR_FLAG_ATOMIC | PFR_FLAG_DUMMY);
+ ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY);
if ((setflag & ~PFR_TFLAG_USRMASK) ||
(clrflag & ~PFR_TFLAG_USRMASK) ||
(setflag & clrflag))
@@ -1477,11 +1433,7 @@
;
}
if (!(flags & PFR_FLAG_DUMMY)) {
- if (flags & PFR_FLAG_ATOMIC)
- s = splsoftnet();
pfr_setflags_ktables(&workq);
- if (flags & PFR_FLAG_ATOMIC)
- splx(s);
}
if (nchange != NULL)
*nchange = xchange;
@@ -1663,10 +1615,10 @@
struct pfr_ktable *p, *q;
struct pfr_ktableworkq workq;
struct pf_ruleset *rs;
- int s, xadd = 0, xchange = 0;
+ int xadd = 0, xchange = 0;
long tzero = time_second;
- ACCEPT_FLAGS(flags, PFR_FLAG_ATOMIC | PFR_FLAG_DUMMY);
+ ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY);
rs = pf_find_ruleset(trs->pfrt_anchor);
if (rs == NULL || !rs->topen || ticket != rs->tticket)
return (EBUSY);
@@ -1684,14 +1636,10 @@
}
if (!(flags & PFR_FLAG_DUMMY)) {
- if (flags & PFR_FLAG_ATOMIC)
- s = splsoftnet();
for (p = SLIST_FIRST(&workq); p != NULL; p = q) {
q = SLIST_NEXT(p, pfrkt_workq);
pfr_commit_ktable(p, tzero);
}
- if (flags & PFR_FLAG_ATOMIC)
- splx(s);
rs->topen = 0;
pf_remove_if_empty_ruleset(rs);
}
Index: pfvar.h
===================================================================
RCS file: /home/tedu/cvs/src/sys/net/pfvar.h,v
retrieving revision 1.315
diff -u -r1.315 pfvar.h
--- pfvar.h 22 Sep 2010 05:58:29 -0000 1.315
+++ pfvar.h 13 Oct 2010 23:34:23 -0000
@@ -1562,7 +1562,6 @@
} *array;
};
-#define PFR_FLAG_ATOMIC 0x00000001
#define PFR_FLAG_DUMMY 0x00000002
#define PFR_FLAG_FEEDBACK 0x00000004
#define PFR_FLAG_CLSTATS 0x00000008
