PERFORCE change 38806 for review
Sam Leffler
sam at FreeBSD.org
Mon Sep 29 15:08:07 PDT 2003
http://perforce.freebsd.org/chv.cgi?CH=38806
Change 38806 by sam at sam_ebb on 2003/09/29 15:07:09
general cleanups and reorg to deal with recursive locking
problems:
o use LIST_FOREACH_SAFE instead of handrolled code
o change key_flush_spd to drop the sptree lock before purging
an entry to avoid lock recursion and to avoid holding the lock
over a long-running operation
o misc cleanups of tangled and twisty code
There is still much to do here but for now things look to be
working again.
Affected files ...
.. //depot/projects/netperf/sys/netipsec/key.c#10 edit
Differences ...
==== //depot/projects/netperf/sys/netipsec/key.c#10 (text+ko) ====
@@ -289,10 +289,6 @@
SYSCTL_INT(_net_key, KEYCTL_PREFERED_OLDSA, prefered_oldsa, CTLFLAG_RW,\
&key_prefered_oldsa, 0, "");
-#ifndef LIST_FOREACH
-#define LIST_FOREACH(elm, head, field) \
- for (elm = LIST_FIRST(head); elm; elm = LIST_NEXT(elm, field))
-#endif
#define __LIST_CHAINED(elm) \
(!((elm)->chain.le_next == NULL && (elm)->chain.le_prev == NULL))
#define LIST_INSERT_TAIL(head, elm, type, field) \
@@ -1168,6 +1164,7 @@
IPSEC_ASSERT(sav != NULL, ("null sav"));
+ /* XXX unguarded? */
SA_DELREF(sav);
KEYDEBUG(KEYDEBUG_IPSEC_STAMP,
@@ -2298,9 +2295,10 @@
return key_senderror(so, m, EINVAL);
for (dir = 0; dir < IPSEC_DIR_MAX; dir++) {
- LIST_FOREACH(sp, &sptree[dir], chain) {
+ SPTREE_LOCK();
+ LIST_FOREACH(sp, &sptree[dir], chain)
sp->state = IPSEC_SPSTATE_DEAD;
- }
+ SPTREE_UNLOCK();
}
if (sizeof(struct sadb_msg) > m->m_len + M_TRAILINGSPACE(m)) {
@@ -2610,7 +2608,7 @@
struct secashead *sah;
{
struct secasvar *sav, *nextsav;
- u_int stateidx, state;
+ u_int stateidx;
int zombie = 0;
IPSEC_ASSERT(sah != NULL, ("NULL sah"));
@@ -2620,14 +2618,8 @@
for (stateidx = 0;
stateidx < _ARRAYLEN(saorder_state_any);
stateidx++) {
-
- state = saorder_state_any[stateidx];
- for (sav = (struct secasvar *)LIST_FIRST(&sah->savtree[state]);
- sav != NULL;
- sav = nextsav) {
-
- nextsav = LIST_NEXT(sav, chain);
-
+ u_int state = saorder_state_any[stateidx];
+ LIST_FOREACH_SAFE(sav, &sah->savtree[state], chain, nextsav) {
if (sav->refcnt == 0) {
/* sanity check */
KEY_CHKSASTATE(state, sav->state, __func__);
@@ -2638,20 +2630,16 @@
}
}
}
- /* remove from tree of SA index */
- if (!zombie && __LIST_CHAINED(sah))
- LIST_REMOVE(sah, chain);
-
- /* don't delete sah only if there are savs. */
- if (zombie)
- return;
-
- if (sah->sa_route.ro_rt) {
- RTFREE(sah->sa_route.ro_rt);
- sah->sa_route.ro_rt = (struct rtentry *)NULL;
+ if (!zombie) { /* delete only if there are savs */
+ /* remove from tree of SA index */
+ if (__LIST_CHAINED(sah))
+ LIST_REMOVE(sah, chain);
+ if (sah->sa_route.ro_rt) {
+ RTFREE(sah->sa_route.ro_rt);
+ sah->sa_route.ro_rt = (struct rtentry *)NULL;
+ }
+ free(sah, M_IPSEC_SAH);
}
-
- free(sah, M_IPSEC_SAH);
}
/*
@@ -3973,32 +3961,34 @@
static void
key_flush_spd(time_t now)
{
- struct secpolicy *sp, *nextsp;
+ static u_int16_t sptree_scangen = 0;
+ u_int16_t gen = sptree_scangen++;
+ struct secpolicy *sp;
u_int dir;
/* SPD */
for (dir = 0; dir < IPSEC_DIR_MAX; dir++) {
+restart:
SPTREE_LOCK();
- for (sp = LIST_FIRST(&sptree[dir]);
- sp != NULL;
- sp = nextsp) {
-
- nextsp = LIST_NEXT(sp, chain);
-
+ LIST_FOREACH(sp, &sptree[dir], chain) {
+ if (sp->scangen == gen) /* previously handled */
+ continue;
+ sp->scangen = gen;
if (sp->state == IPSEC_SPSTATE_DEAD) {
+ /* NB: clean entries created by key_spdflush */
+ SPTREE_UNLOCK();
KEY_FREESP(&sp);
- continue;
+ goto restart;
}
-
if (sp->lifetime == 0 && sp->validtime == 0)
continue;
-
- /* the deletion will occur next time */
if ((sp->lifetime && now - sp->created > sp->lifetime)
|| (sp->validtime && now - sp->lastused > sp->validtime)) {
sp->state = IPSEC_SPSTATE_DEAD;
+ SPTREE_UNLOCK();
key_spdexpire(sp);
- continue;
+ KEY_FREESP(&sp);
+ goto restart;
}
}
SPTREE_UNLOCK();
@@ -4013,12 +4003,7 @@
/* SAD */
SAHTREE_LOCK();
- for (sah = LIST_FIRST(&sahtree);
- sah != NULL;
- sah = nextsah) {
-
- nextsah = LIST_NEXT(sah, chain);
-
+ LIST_FOREACH_SAFE(sah, &sahtree, chain, nextsah) {
/* if sah has been dead, then delete it and process next sah. */
if (sah->state == SADB_SASTATE_DEAD) {
key_delsah(sah);
@@ -4026,27 +4011,16 @@
}
/* if LARVAL entry doesn't become MATURE, delete it. */
- for (sav = LIST_FIRST(&sah->savtree[SADB_SASTATE_LARVAL]);
- sav != NULL;
- sav = nextsav) {
-
- nextsav = LIST_NEXT(sav, chain);
-
- if (now - sav->created > key_larval_lifetime) {
+ LIST_FOREACH_SAFE(sav, &sah->savtree[SADB_SASTATE_LARVAL], chain, nextsav) {
+ if (now - sav->created > key_larval_lifetime)
KEY_FREESAV(&sav);
- }
}
/*
* check MATURE entry to start to send expire message
* whether or not.
*/
- for (sav = LIST_FIRST(&sah->savtree[SADB_SASTATE_MATURE]);
- sav != NULL;
- sav = nextsav) {
-
- nextsav = LIST_NEXT(sav, chain);
-
+ LIST_FOREACH_SAFE(sav, &sah->savtree[SADB_SASTATE_MATURE], chain, nextsav) {
/* we don't need to check. */
if (sav->lft_s == NULL)
continue;
@@ -4059,8 +4033,8 @@
}
/* check SOFT lifetime */
- if (sav->lft_s->sadb_lifetime_addtime != 0
- && now - sav->created > sav->lft_s->sadb_lifetime_addtime) {
+ if (sav->lft_s->sadb_lifetime_addtime != 0 &&
+ now - sav->created > sav->lft_s->sadb_lifetime_addtime) {
/*
* check SA to be used whether or not.
* when SA hasn't been used, delete it.
@@ -4084,8 +4058,8 @@
* when new SA is installed. Caution when it's
* installed too big lifetime by time.
*/
- else if (sav->lft_s->sadb_lifetime_bytes != 0
- && sav->lft_s->sadb_lifetime_bytes < sav->lft_c->sadb_lifetime_bytes) {
+ else if (sav->lft_s->sadb_lifetime_bytes != 0 &&
+ sav->lft_s->sadb_lifetime_bytes < sav->lft_c->sadb_lifetime_bytes) {
key_sa_chgstate(sav, SADB_SASTATE_DYING);
/*
@@ -4098,12 +4072,7 @@
}
/* check DYING entry to change status to DEAD. */
- for (sav = LIST_FIRST(&sah->savtree[SADB_SASTATE_DYING]);
- sav != NULL;
- sav = nextsav) {
-
- nextsav = LIST_NEXT(sav, chain);
-
+ LIST_FOREACH_SAFE(sav, &sah->savtree[SADB_SASTATE_DYING], chain, nextsav) {
/* we don't need to check. */
if (sav->lft_h == NULL)
continue;
@@ -4115,8 +4084,8 @@
continue;
}
- if (sav->lft_h->sadb_lifetime_addtime != 0
- && now - sav->created > sav->lft_h->sadb_lifetime_addtime) {
+ if (sav->lft_h->sadb_lifetime_addtime != 0 &&
+ now - sav->created > sav->lft_h->sadb_lifetime_addtime) {
key_sa_chgstate(sav, SADB_SASTATE_DEAD);
KEY_FREESAV(&sav);
}
@@ -4137,20 +4106,15 @@
}
#endif
/* check HARD lifetime by bytes */
- else if (sav->lft_h->sadb_lifetime_bytes != 0
- && sav->lft_h->sadb_lifetime_bytes < sav->lft_c->sadb_lifetime_bytes) {
+ else if (sav->lft_h->sadb_lifetime_bytes != 0 &&
+ sav->lft_h->sadb_lifetime_bytes < sav->lft_c->sadb_lifetime_bytes) {
key_sa_chgstate(sav, SADB_SASTATE_DEAD);
KEY_FREESAV(&sav);
}
}
/* delete entry in DEAD */
- for (sav = LIST_FIRST(&sah->savtree[SADB_SASTATE_DEAD]);
- sav != NULL;
- sav = nextsav) {
-
- nextsav = LIST_NEXT(sav, chain);
-
+ LIST_FOREACH_SAFE(sav, &sah->savtree[SADB_SASTATE_DEAD], chain, nextsav) {
/* sanity check */
if (sav->state != SADB_SASTATE_DEAD) {
ipseclog((LOG_DEBUG, "%s: invalid sav->state "
@@ -4158,7 +4122,6 @@
__func__,
SADB_SASTATE_DEAD, sav->state));
}
-
/*
* do not call key_freesav() here.
* sav should already be freed, and sav->refcnt
More information about the p4-projects
mailing list