Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 24 Dec 2014 18:34:57 +0000 (UTC)
From:      "Andrey V. Elsukov" <ae@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r276188 - head/sys/netipsec
Message-ID:  <201412241834.sBOIYvrL078222@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ae
Date: Wed Dec 24 18:34:56 2014
New Revision: 276188
URL: https://svnweb.freebsd.org/changeset/base/276188

Log:
  Rename ip4_def_policy variable to def_policy. It is used by both IPv4 and
  IPv6. Initialize it only once in def_policy_init(). Remove its
  initialization from key_init() and make it static.
  
  Remove several fields from struct secpolicy:
  * lock - it isn't so useful having mutex in the structure, but the only
    thing we do with it is initialization and destroying.
  * state - it has only two values - DEAD and ALIVE. Instead of take a lock
    and change the state to DEAD, then take lock again in GC function and
    delete policy from the chain - keep in the chain only ALIVE policies.
  * scangen - it was used in GC function to protect from sending several
    SADB_SPDEXPIRE messages for one SPD entry. Now we don't keep DEAD entries
    in the chain and there is no need to have scangen variable.
  
  Use TAILQ to implement SPD entries chain. Use rmlock to protect access
  to SPD entries chain. Protect all SP lookup with RLOCK, and use WLOCK
  when we are inserting (or removing) SP entry in the chain.
  
  Instead of using pattern "LOCK(); refcnt++; UNLOCK();", use refcount(9)
  API to implement refcounting in SPD. Merge code from key_delsp() and
  _key_delsp() into _key_freesp(). And use KEY_FREESP() macro in all cases
  when we want to release reference or just delete SP entry.
  
  Obtained from:	Yandex LLC
  Sponsored by:	Yandex LLC

Modified:
  head/sys/netipsec/ipsec.c
  head/sys/netipsec/ipsec.h
  head/sys/netipsec/key.c
  head/sys/netipsec/key_debug.c

Modified: head/sys/netipsec/ipsec.c
==============================================================================
--- head/sys/netipsec/ipsec.c	Wed Dec 24 17:12:51 2014	(r276187)
+++ head/sys/netipsec/ipsec.c	Wed Dec 24 18:34:56 2014	(r276188)
@@ -118,11 +118,12 @@ VNET_DEFINE(int, ip4_esp_trans_deflev) =
 VNET_DEFINE(int, ip4_esp_net_deflev) = IPSEC_LEVEL_USE;
 VNET_DEFINE(int, ip4_ah_trans_deflev) = IPSEC_LEVEL_USE;
 VNET_DEFINE(int, ip4_ah_net_deflev) = IPSEC_LEVEL_USE;
-VNET_DEFINE(struct secpolicy, ip4_def_policy);
 /* ECN ignore(-1)/forbidden(0)/allowed(1) */
 VNET_DEFINE(int, ip4_ipsec_ecn) = 0;
 VNET_DEFINE(int, ip4_esp_randpad) = -1;
 
+static VNET_DEFINE(struct secpolicy, def_policy);
+#define	V_def_policy	VNET(def_policy)
 /*
  * Crypto support requirements:
  *
@@ -141,7 +142,7 @@ SYSCTL_DECL(_net_inet_ipsec);
 
 /* net.inet.ipsec */
 SYSCTL_INT(_net_inet_ipsec, IPSECCTL_DEF_POLICY, def_policy,
-	CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(ip4_def_policy).policy, 0,
+	CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(def_policy).policy, 0,
 	"IPsec default policy.");
 SYSCTL_INT(_net_inet_ipsec, IPSECCTL_DEF_ESP_TRANSLEV, esp_trans_deflev,
 	CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(ip4_esp_trans_deflev), 0,
@@ -213,7 +214,7 @@ SYSCTL_DECL(_net_inet6_ipsec6);
 
 /* net.inet6.ipsec6 */
 SYSCTL_INT(_net_inet6_ipsec6, IPSECCTL_DEF_POLICY, def_policy,
-	CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(ip4_def_policy).policy, 0,
+	CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(def_policy).policy, 0,
 	"IPsec default policy.");
 SYSCTL_INT(_net_inet6_ipsec6, IPSECCTL_DEF_ESP_TRANSLEV, esp_trans_deflev,
 	CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(ip6_esp_trans_deflev), 0,
@@ -262,7 +263,7 @@ key_allocsp_default(const char* where, i
 	KEYDEBUG(KEYDEBUG_IPSEC_STAMP,
 		printf("DP key_allocsp_default from %s:%u\n", where, tag));
 
-	sp = &V_ip4_def_policy;
+	sp = &V_def_policy;
 	if (sp->policy != IPSEC_POLICY_DISCARD &&
 	    sp->policy != IPSEC_POLICY_NONE) {
 		ipseclog((LOG_INFO, "fixed system default policy: %d->%d\n",
@@ -828,17 +829,13 @@ ipsec_init_policy(struct socket *so, str
 		ipsec_delpcbpolicy(new);
 		return (ENOBUFS);
 	}
-	new->sp_in->state = IPSEC_SPSTATE_ALIVE;
 	new->sp_in->policy = IPSEC_POLICY_ENTRUST;
-
 	if ((new->sp_out = KEY_NEWSP()) == NULL) {
 		KEY_FREESP(&new->sp_in);
 		ipsec_delpcbpolicy(new);
 		return (ENOBUFS);
 	}
-	new->sp_out->state = IPSEC_SPSTATE_ALIVE;
 	new->sp_out->policy = IPSEC_POLICY_ENTRUST;
-
 	*pcb_sp = new;
 
 	return (0);
@@ -927,7 +924,6 @@ ipsec_deepcopy_policy(struct secpolicy *
 	}
 
 	dst->req = newchain;
-	dst->state = src->state;
 	dst->policy = src->policy;
 	/* Do not touch the refcnt fields. */
 
@@ -979,8 +975,6 @@ ipsec_set_policy_internal(struct secpoli
 	if ((newsp = key_msg2sp(xpl, len, &error)) == NULL)
 		return (error);
 
-	newsp->state = IPSEC_SPSTATE_ALIVE;
-
 	/* Clear old SP and set new SP. */
 	KEY_FREESP(pcb_sp);
 	*pcb_sp = newsp;
@@ -1693,14 +1687,15 @@ ipsec_dumpmbuf(struct mbuf *m)
 }
 
 static void
-ipsec_init(const void *unused __unused)
+def_policy_init(const void *unused __unused)
 {
 
-	SECPOLICY_LOCK_INIT(&V_ip4_def_policy);
-	V_ip4_def_policy.refcnt = 1;			/* NB: disallow free. */
+	bzero(&V_def_policy, sizeof(struct secpolicy));
+	V_def_policy.policy = IPSEC_POLICY_NONE;
+	V_def_policy.refcnt = 1;
 }
-VNET_SYSINIT(ipsec_init, SI_SUB_PROTO_DOMAININIT, SI_ORDER_ANY, ipsec_init,
-    NULL);
+VNET_SYSINIT(def_policy_init, SI_SUB_PROTO_DOMAININIT, SI_ORDER_ANY,
+    def_policy_init, NULL);
 
 
 /* XXX This stuff doesn't belong here... */

Modified: head/sys/netipsec/ipsec.h
==============================================================================
--- head/sys/netipsec/ipsec.h	Wed Dec 24 17:12:51 2014	(r276187)
+++ head/sys/netipsec/ipsec.h	Wed Dec 24 18:34:56 2014	(r276188)
@@ -81,21 +81,15 @@ struct secpolicyindex {
 
 /* Security Policy Data Base */
 struct secpolicy {
-	LIST_ENTRY(secpolicy) chain;
-	struct mtx lock;
+	TAILQ_ENTRY(secpolicy) chain;
 
-	u_int refcnt;			/* reference count */
 	struct secpolicyindex spidx;	/* selector */
-	u_int32_t id;			/* It's unique number on the system. */
-	u_int state;			/* 0: dead, others: alive */
-#define IPSEC_SPSTATE_DEAD	0
-#define IPSEC_SPSTATE_ALIVE	1
-	u_int policy;			/* policy_type per pfkeyv2.h */
-	u_int16_t scangen;		/* scan generation # */
 	struct ipsecrequest *req;
 				/* pointer to the ipsec request tree, */
 				/* if policy == IPSEC else this value == NULL.*/
-
+	u_int refcnt;			/* reference count */
+	u_int policy;			/* policy_type per pfkeyv2.h */
+	u_int32_t id;			/* It's unique number on the system. */
 	/*
 	 * lifetime handler.
 	 * the policy can be used without limitiation if both lifetime and
@@ -109,13 +103,6 @@ struct secpolicy {
 	long validtime;		/* duration this policy is valid without use */
 };
 
-#define	SECPOLICY_LOCK_INIT(_sp) \
-	mtx_init(&(_sp)->lock, "ipsec policy", NULL, MTX_DEF)
-#define	SECPOLICY_LOCK(_sp)		mtx_lock(&(_sp)->lock)
-#define	SECPOLICY_UNLOCK(_sp)		mtx_unlock(&(_sp)->lock)
-#define	SECPOLICY_LOCK_DESTROY(_sp)	mtx_destroy(&(_sp)->lock)
-#define	SECPOLICY_LOCK_ASSERT(_sp)	mtx_assert(&(_sp)->lock, MA_OWNED)
-
 /* Request for IPsec */
 struct ipsecrequest {
 	struct ipsecrequest *next;
@@ -279,7 +266,6 @@ VNET_DECLARE(int, ipsec_integrity);
 #endif
 
 VNET_PCPUSTAT_DECLARE(struct ipsecstat, ipsec4stat);
-VNET_DECLARE(struct secpolicy, ip4_def_policy);
 VNET_DECLARE(int, ip4_esp_trans_deflev);
 VNET_DECLARE(int, ip4_esp_net_deflev);
 VNET_DECLARE(int, ip4_ah_trans_deflev);
@@ -292,7 +278,6 @@ VNET_DECLARE(int, crypto_support);
 
 #define	IPSECSTAT_INC(name)	\
     VNET_PCPUSTAT_ADD(struct ipsecstat, ipsec4stat, name, 1)
-#define	V_ip4_def_policy	VNET(ip4_def_policy)
 #define	V_ip4_esp_trans_deflev	VNET(ip4_esp_trans_deflev)
 #define	V_ip4_esp_net_deflev	VNET(ip4_esp_net_deflev)
 #define	V_ip4_ah_trans_deflev	VNET(ip4_ah_trans_deflev)

Modified: head/sys/netipsec/key.c
==============================================================================
--- head/sys/netipsec/key.c	Wed Dec 24 17:12:51 2014	(r276187)
+++ head/sys/netipsec/key.c	Wed Dec 24 18:34:56 2014	(r276188)
@@ -48,6 +48,7 @@
 #include <sys/domain.h>
 #include <sys/protosw.h>
 #include <sys/malloc.h>
+#include <sys/rmlock.h>
 #include <sys/socket.h>
 #include <sys/socketvar.h>
 #include <sys/sysctl.h>
@@ -141,16 +142,19 @@ static VNET_DEFINE(u_int32_t, acq_seq) =
 #define	V_acq_seq		VNET(acq_seq)
 
 								/* SPD */
-static VNET_DEFINE(LIST_HEAD(_sptree, secpolicy), sptree[IPSEC_DIR_MAX]);
+static VNET_DEFINE(TAILQ_HEAD(_sptree, secpolicy), sptree[IPSEC_DIR_MAX]);
+static struct rmlock sptree_lock;
 #define	V_sptree		VNET(sptree)
-static struct mtx sptree_lock;
-#define	SPTREE_LOCK_INIT() \
-	mtx_init(&sptree_lock, "sptree", \
-		"fast ipsec security policy database", MTX_DEF)
-#define	SPTREE_LOCK_DESTROY()	mtx_destroy(&sptree_lock)
-#define	SPTREE_LOCK()		mtx_lock(&sptree_lock)
-#define	SPTREE_UNLOCK()	mtx_unlock(&sptree_lock)
-#define	SPTREE_LOCK_ASSERT()	mtx_assert(&sptree_lock, MA_OWNED)
+#define	SPTREE_LOCK_INIT()      rm_init(&sptree_lock, "sptree")
+#define	SPTREE_LOCK_DESTROY()   rm_destroy(&sptree_lock)
+#define	SPTREE_RLOCK_TRACKER    struct rm_priotracker sptree_tracker
+#define	SPTREE_RLOCK()          rm_rlock(&sptree_lock, &sptree_tracker)
+#define	SPTREE_RUNLOCK()        rm_runlock(&sptree_lock, &sptree_tracker)
+#define	SPTREE_RLOCK_ASSERT()   rm_assert(&sptree_lock, RA_RLOCKED)
+#define	SPTREE_WLOCK()          rm_wlock(&sptree_lock)
+#define	SPTREE_WUNLOCK()        rm_wunlock(&sptree_lock)
+#define	SPTREE_WLOCK_ASSERT()   rm_assert(&sptree_lock, RA_WLOCKED)
+#define	SPTREE_UNLOCK_ASSERT()  rm_assert(&sptree_lock, RA_UNLOCKED)
 
 static VNET_DEFINE(LIST_HEAD(_sahtree, secashead), sahtree);	/* SAD */
 #define	V_sahtree		VNET(sahtree)
@@ -416,9 +420,8 @@ static struct callout key_timer;
 static struct secasvar *key_allocsa_policy(const struct secasindex *);
 static void key_freesp_so(struct secpolicy **);
 static struct secasvar *key_do_allocsa_policy(struct secashead *, u_int);
-static void key_delsp(struct secpolicy *);
+static void key_unlink(struct secpolicy *);
 static struct secpolicy *key_getsp(struct secpolicyindex *);
-static void _key_delsp(struct secpolicy *sp);
 static struct secpolicy *key_getspbyid(u_int32_t);
 static u_int32_t key_newreqid(void);
 static struct mbuf *key_gather_mbuf(struct mbuf *,
@@ -575,15 +578,8 @@ sa_delref(struct secasvar *sav)
 	return (refcount_release(&sav->refcnt));
 }
 
-#define	SP_ADDREF(p) do {						\
-	(p)->refcnt++;							\
-	IPSEC_ASSERT((p)->refcnt != 0, ("SP refcnt overflow"));		\
-} while (0)
-#define	SP_DELREF(p) do {						\
-	IPSEC_ASSERT((p)->refcnt > 0, ("SP refcnt underflow"));		\
-	(p)->refcnt--;							\
-} while (0)
- 
+#define	SP_ADDREF(p)	refcount_acquire(&(p)->refcnt)
+#define	SP_DELREF(p)	refcount_release(&(p)->refcnt)
 
 /*
  * Update the refcnt while holding the SPTREE lock.
@@ -591,9 +587,8 @@ sa_delref(struct secasvar *sav)
 void
 key_addref(struct secpolicy *sp)
 {
-	SPTREE_LOCK();
+
 	SP_ADDREF(sp);
-	SPTREE_UNLOCK();
 }
 
 /*
@@ -606,7 +601,7 @@ key_havesp(u_int dir)
 {
 
 	return (dir == IPSEC_DIR_INBOUND || dir == IPSEC_DIR_OUTBOUND ?
-		LIST_FIRST(&V_sptree[dir]) != NULL : 1);
+		TAILQ_FIRST(&V_sptree[dir]) != NULL : 1);
 }
 
 /* %%% IPsec policy management */
@@ -620,6 +615,7 @@ struct secpolicy *
 key_allocsp(struct secpolicyindex *spidx, u_int dir, const char* where,
     int tag)
 {
+	SPTREE_RLOCK_TRACKER;
 	struct secpolicy *sp;
 
 	IPSEC_ASSERT(spidx != NULL, ("null spidx"));
@@ -634,14 +630,11 @@ key_allocsp(struct secpolicyindex *spidx
 		printf("*** objects\n");
 		kdebug_secpolicyindex(spidx));
 
-	SPTREE_LOCK();
-	LIST_FOREACH(sp, &V_sptree[dir], chain) {
+	SPTREE_RLOCK();
+	TAILQ_FOREACH(sp, &V_sptree[dir], chain) {
 		KEYDEBUG(KEYDEBUG_IPSEC_DATA,
 			printf("*** in SPD\n");
 			kdebug_secpolicyindex(&sp->spidx));
-
-		if (sp->state == IPSEC_SPSTATE_DEAD)
-			continue;
 		if (key_cmpspidx_withmask(&sp->spidx, spidx))
 			goto found;
 	}
@@ -655,7 +648,7 @@ found:
 		sp->lastused = time_second;
 		SP_ADDREF(sp);
 	}
-	SPTREE_UNLOCK();
+	SPTREE_RUNLOCK();
 
 	KEYDEBUG(KEYDEBUG_IPSEC_STAMP,
 		printf("DP %s return SP:%p (ID=%u) refcnt %u\n", __func__,
@@ -673,6 +666,7 @@ struct secpolicy *
 key_allocsp2(u_int32_t spi, union sockaddr_union *dst, u_int8_t proto,
     u_int dir, const char* where, int tag)
 {
+	SPTREE_RLOCK_TRACKER;
 	struct secpolicy *sp;
 
 	IPSEC_ASSERT(dst != NULL, ("null dst"));
@@ -688,14 +682,11 @@ key_allocsp2(u_int32_t spi, union sockad
 		printf("spi %u proto %u dir %u\n", spi, proto, dir);
 		kdebug_sockaddr(&dst->sa));
 
-	SPTREE_LOCK();
-	LIST_FOREACH(sp, &V_sptree[dir], chain) {
+	SPTREE_RLOCK();
+	TAILQ_FOREACH(sp, &V_sptree[dir], chain) {
 		KEYDEBUG(KEYDEBUG_IPSEC_DATA,
 			printf("*** in SPD\n");
 			kdebug_secpolicyindex(&sp->spidx));
-
-		if (sp->state == IPSEC_SPSTATE_DEAD)
-			continue;
 		/* compare simple values, then dst address */
 		if (sp->spidx.ul_proto != proto)
 			continue;
@@ -715,7 +706,7 @@ found:
 		sp->lastused = time_second;
 		SP_ADDREF(sp);
 	}
-	SPTREE_UNLOCK();
+	SPTREE_RUNLOCK();
 
 	KEYDEBUG(KEYDEBUG_IPSEC_STAMP,
 		printf("DP %s return SP:%p (ID=%u) refcnt %u\n", __func__,
@@ -1174,22 +1165,41 @@ done:
 void
 _key_freesp(struct secpolicy **spp, const char* where, int tag)
 {
+	struct ipsecrequest *isr, *nextisr;
 	struct secpolicy *sp = *spp;
 
 	IPSEC_ASSERT(sp != NULL, ("null sp"));
-
-	SPTREE_LOCK();
-	SP_DELREF(sp);
-
 	KEYDEBUG(KEYDEBUG_IPSEC_STAMP,
 		printf("DP %s SP:%p (ID=%u) from %s:%u; refcnt now %u\n",
 			__func__, sp, sp->id, where, tag, sp->refcnt));
 
-	if (sp->refcnt == 0) {
-		*spp = NULL;
-		key_delsp(sp);
+	if (SP_DELREF(sp) == 0)
+		return;
+	*spp = NULL;
+	for (isr = sp->req; isr != NULL; isr = nextisr) {
+		if (isr->sav != NULL) {
+			KEY_FREESAV(&isr->sav);
+			isr->sav = NULL;
+		}
+		nextisr = isr->next;
+		ipsec_delisr(isr);
 	}
-	SPTREE_UNLOCK();
+	free(sp, M_IPSEC_SP);
+}
+
+static void
+key_unlink(struct secpolicy *sp)
+{
+
+	IPSEC_ASSERT(sp != NULL, ("null sp"));
+	IPSEC_ASSERT(sp->spidx.dir == IPSEC_DIR_INBOUND ||
+	    sp->spidx.dir == IPSEC_DIR_OUTBOUND,
+	    ("invalid direction %u", sp->spidx.dir));
+	SPTREE_UNLOCK_ASSERT();
+
+	SPTREE_WLOCK();
+	TAILQ_REMOVE(&V_sptree[sp->spidx.dir], sp, chain);
+	SPTREE_WUNLOCK();
 }
 
 /*
@@ -1278,38 +1288,6 @@ key_freesav(struct secasvar **psav, cons
 
 /* %%% SPD management */
 /*
- * free security policy entry.
- */
-static void
-key_delsp(struct secpolicy *sp)
-{
-	struct ipsecrequest *isr, *nextisr;
-
-	IPSEC_ASSERT(sp != NULL, ("null sp"));
-	SPTREE_LOCK_ASSERT();
-
-	sp->state = IPSEC_SPSTATE_DEAD;
-
-	IPSEC_ASSERT(sp->refcnt == 0,
-		("SP with references deleted (refcnt %u)", sp->refcnt));
-
-	/* remove from SP index */
-	if (__LIST_CHAINED(sp))
-		LIST_REMOVE(sp, chain);
-
-	for (isr = sp->req; isr != NULL; isr = nextisr) {
-		if (isr->sav != NULL) {
-			KEY_FREESAV(&isr->sav);
-			isr->sav = NULL;
-		}
-
-		nextisr = isr->next;
-		ipsec_delisr(isr);
-	}
-	_key_delsp(sp);
-}
-
-/*
  * search SPD
  * OUT:	NULL	: not found
  *	others	: found, pointer to a SP.
@@ -1317,20 +1295,19 @@ key_delsp(struct secpolicy *sp)
 static struct secpolicy *
 key_getsp(struct secpolicyindex *spidx)
 {
+	SPTREE_RLOCK_TRACKER;
 	struct secpolicy *sp;
 
 	IPSEC_ASSERT(spidx != NULL, ("null spidx"));
 
-	SPTREE_LOCK();
-	LIST_FOREACH(sp, &V_sptree[spidx->dir], chain) {
-		if (sp->state == IPSEC_SPSTATE_DEAD)
-			continue;
+	SPTREE_RLOCK();
+	TAILQ_FOREACH(sp, &V_sptree[spidx->dir], chain) {
 		if (key_cmpspidx_exactly(spidx, &sp->spidx)) {
 			SP_ADDREF(sp);
 			break;
 		}
 	}
-	SPTREE_UNLOCK();
+	SPTREE_RUNLOCK();
 
 	return sp;
 }
@@ -1343,28 +1320,25 @@ key_getsp(struct secpolicyindex *spidx)
 static struct secpolicy *
 key_getspbyid(u_int32_t id)
 {
+	SPTREE_RLOCK_TRACKER;
 	struct secpolicy *sp;
 
-	SPTREE_LOCK();
-	LIST_FOREACH(sp, &V_sptree[IPSEC_DIR_INBOUND], chain) {
-		if (sp->state == IPSEC_SPSTATE_DEAD)
-			continue;
+	SPTREE_RLOCK();
+	TAILQ_FOREACH(sp, &V_sptree[IPSEC_DIR_INBOUND], chain) {
 		if (sp->id == id) {
 			SP_ADDREF(sp);
 			goto done;
 		}
 	}
 
-	LIST_FOREACH(sp, &V_sptree[IPSEC_DIR_OUTBOUND], chain) {
-		if (sp->state == IPSEC_SPSTATE_DEAD)
-			continue;
+	TAILQ_FOREACH(sp, &V_sptree[IPSEC_DIR_OUTBOUND], chain) {
 		if (sp->id == id) {
 			SP_ADDREF(sp);
 			goto done;
 		}
 	}
 done:
-	SPTREE_UNLOCK();
+	SPTREE_RUNLOCK();
 
 	return sp;
 }
@@ -1376,11 +1350,8 @@ key_newsp(const char* where, int tag)
 
 	newsp = (struct secpolicy *)
 		malloc(sizeof(struct secpolicy), M_IPSEC_SP, M_NOWAIT|M_ZERO);
-	if (newsp) {
-		SECPOLICY_LOCK_INIT(newsp);
-		newsp->refcnt = 1;
-		newsp->req = NULL;
-	}
+	if (newsp)
+		refcount_init(&newsp->refcnt, 1);
 
 	KEYDEBUG(KEYDEBUG_IPSEC_STAMP,
 		printf("DP %s from %s:%u return SP:%p\n", __func__,
@@ -1388,13 +1359,6 @@ key_newsp(const char* where, int tag)
 	return newsp;
 }
 
-static void
-_key_delsp(struct secpolicy *sp)
-{
-	SECPOLICY_LOCK_DESTROY(sp);
-	free(sp, M_IPSEC_SP);
-}
-
 /*
  * create secpolicy structure from sadb_x_policy structure.
  * NOTE: `state', `secpolicyindex' in secpolicy structure are not set,
@@ -1874,9 +1838,7 @@ key_spdadd(struct socket *so, struct mbu
 	newsp = key_getsp(&spidx);
 	if (mhp->msg->sadb_msg_type == SADB_X_SPDUPDATE) {
 		if (newsp) {
-			SPTREE_LOCK();
-			newsp->state = IPSEC_SPSTATE_DEAD;
-			SPTREE_UNLOCK();
+			key_unlink(newsp);
 			KEY_FREESP(&newsp);
 		}
 	} else {
@@ -1888,13 +1850,15 @@ key_spdadd(struct socket *so, struct mbu
 		}
 	}
 
+	/* XXX: there is race between key_getsp and key_msg2sp. */
+
 	/* allocation new SP entry */
 	if ((newsp = key_msg2sp(xpl0, PFKEY_EXTLEN(xpl0), &error)) == NULL) {
 		return key_senderror(so, m, error);
 	}
 
 	if ((newsp->id = key_getnewspid()) == 0) {
-		_key_delsp(newsp);
+		KEY_FREESP(&newsp);
 		return key_senderror(so, m, ENOBUFS);
 	}
 
@@ -1910,18 +1874,20 @@ key_spdadd(struct socket *so, struct mbu
 	/* sanity check on addr pair */
 	if (((struct sockaddr *)(src0 + 1))->sa_family !=
 			((struct sockaddr *)(dst0+ 1))->sa_family) {
-		_key_delsp(newsp);
+		KEY_FREESP(&newsp);
 		return key_senderror(so, m, EINVAL);
 	}
 	if (((struct sockaddr *)(src0 + 1))->sa_len !=
 			((struct sockaddr *)(dst0+ 1))->sa_len) {
-		_key_delsp(newsp);
+		KEY_FREESP(&newsp);
 		return key_senderror(so, m, EINVAL);
 	}
 #if 1
-	if (newsp->req && newsp->req->saidx.src.sa.sa_family && newsp->req->saidx.dst.sa.sa_family) {
-		if (newsp->req->saidx.src.sa.sa_family != newsp->req->saidx.dst.sa.sa_family) {
-			_key_delsp(newsp);
+	if (newsp->req && newsp->req->saidx.src.sa.sa_family &&
+	    newsp->req->saidx.dst.sa.sa_family) {
+		if (newsp->req->saidx.src.sa.sa_family !=
+		    newsp->req->saidx.dst.sa.sa_family) {
+			KEY_FREESP(&newsp);
 			return key_senderror(so, m, EINVAL);
 		}
 	}
@@ -1932,9 +1898,9 @@ key_spdadd(struct socket *so, struct mbu
 	newsp->lifetime = lft ? lft->sadb_lifetime_addtime : 0;
 	newsp->validtime = lft ? lft->sadb_lifetime_usetime : 0;
 
-	newsp->refcnt = 1;	/* do not reclaim until I say I do */
-	newsp->state = IPSEC_SPSTATE_ALIVE;
-	LIST_INSERT_TAIL(&V_sptree[newsp->spidx.dir], newsp, secpolicy, chain);
+	SPTREE_WLOCK();
+	TAILQ_INSERT_TAIL(&V_sptree[newsp->spidx.dir], newsp, chain);
+	SPTREE_WUNLOCK();
 
 	/* delete the entry in spacqtree */
 	if (mhp->msg->sadb_msg_type == SADB_X_SPDUPDATE) {
@@ -2109,9 +2075,7 @@ key_spddelete(struct socket *so, struct 
 	/* save policy id to buffer to be returned. */
 	xpl0->sadb_x_policy_id = sp->id;
 
-	SPTREE_LOCK();
-	sp->state = IPSEC_SPSTATE_DEAD;
-	SPTREE_UNLOCK();
+	key_unlink(sp);
 	KEY_FREESP(&sp);
 
     {
@@ -2176,9 +2140,7 @@ key_spddelete2(struct socket *so, struct
 		return key_senderror(so, m, EINVAL);
 	}
 
-	SPTREE_LOCK();
-	sp->state = IPSEC_SPSTATE_DEAD;
-	SPTREE_UNLOCK();
+	key_unlink(sp);
 	KEY_FREESP(&sp);
 
     {
@@ -2356,8 +2318,9 @@ key_spdacquire(struct secpolicy *sp)
 static int
 key_spdflush(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 {
+	TAILQ_HEAD(, secpolicy) drainq;
 	struct sadb_msg *newmsg;
-	struct secpolicy *sp;
+	struct secpolicy *sp, *nextsp;
 	u_int dir;
 
 	IPSEC_ASSERT(so != NULL, ("null socket"));
@@ -2368,11 +2331,17 @@ key_spdflush(struct socket *so, struct m
 	if (m->m_len != PFKEY_ALIGN8(sizeof(struct sadb_msg)))
 		return key_senderror(so, m, EINVAL);
 
+	TAILQ_INIT(&drainq);
+	SPTREE_WLOCK();
 	for (dir = 0; dir < IPSEC_DIR_MAX; dir++) {
-		SPTREE_LOCK();
-		LIST_FOREACH(sp, &V_sptree[dir], chain)
-			sp->state = IPSEC_SPSTATE_DEAD;
-		SPTREE_UNLOCK();
+		TAILQ_CONCAT(&drainq, &V_sptree[dir], chain);
+	}
+	SPTREE_WUNLOCK();
+	sp = TAILQ_FIRST(&drainq);
+	while (sp != NULL) {
+		nextsp = TAILQ_NEXT(sp, chain);
+		KEY_FREESP(&sp);
+		sp = nextsp;
 	}
 
 	if (sizeof(struct sadb_msg) > m->m_len + M_TRAILINGSPACE(m)) {
@@ -2405,6 +2374,7 @@ key_spdflush(struct socket *so, struct m
 static int
 key_spddump(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp)
 {
+	SPTREE_RLOCK_TRACKER;
 	struct secpolicy *sp;
 	int cnt;
 	u_int dir;
@@ -2417,20 +2387,20 @@ key_spddump(struct socket *so, struct mb
 
 	/* search SPD entry and get buffer size. */
 	cnt = 0;
-	SPTREE_LOCK();
+	SPTREE_RLOCK();
 	for (dir = 0; dir < IPSEC_DIR_MAX; dir++) {
-		LIST_FOREACH(sp, &V_sptree[dir], chain) {
+		TAILQ_FOREACH(sp, &V_sptree[dir], chain) {
 			cnt++;
 		}
 	}
 
 	if (cnt == 0) {
-		SPTREE_UNLOCK();
+		SPTREE_RUNLOCK();
 		return key_senderror(so, m, ENOENT);
 	}
 
 	for (dir = 0; dir < IPSEC_DIR_MAX; dir++) {
-		LIST_FOREACH(sp, &V_sptree[dir], chain) {
+		TAILQ_FOREACH(sp, &V_sptree[dir], chain) {
 			--cnt;
 			n = key_setdumpsp(sp, SADB_X_SPDDUMP, cnt,
 			    mhp->msg->sadb_msg_pid);
@@ -2440,7 +2410,7 @@ key_spddump(struct socket *so, struct mb
 		}
 	}
 
-	SPTREE_UNLOCK();
+	SPTREE_RUNLOCK();
 	m_freem(m);
 	return 0;
 }
@@ -2452,6 +2422,8 @@ key_setdumpsp(struct secpolicy *sp, u_in
 	struct mbuf *result = NULL, *m;
 	struct seclifetime lt;
 
+	SPTREE_RLOCK_ASSERT();
+
 	m = key_setsadbmsg(type, 0, SADB_SATYPE_UNSPEC, seq, pid, sp->refcnt);
 	if (!m)
 		goto fail;
@@ -4226,47 +4198,29 @@ key_bbcmp(const void *a1, const void *a2
 static void
 key_flush_spd(time_t now)
 {
-	static u_int16_t sptree_scangen = 0;
-	u_int16_t gen = sptree_scangen++;
+	SPTREE_RLOCK_TRACKER;
 	struct secpolicy *sp;
 	u_int dir;
 
 	/* SPD */
 	for (dir = 0; dir < IPSEC_DIR_MAX; dir++) {
 restart:
-		SPTREE_LOCK();
-		LIST_FOREACH(sp, &V_sptree[dir], chain) {
-			if (sp->scangen == gen)		/* previously handled */
-				continue;
-			sp->scangen = gen;
-			if (sp->state == IPSEC_SPSTATE_DEAD &&
-			    sp->refcnt == 1) {
-				/*
-				 * Ensure that we only decrease refcnt once,
-				 * when we're the last consumer.
-				 * Directly call SP_DELREF/key_delsp instead
-				 * of KEY_FREESP to avoid unlocking/relocking
-				 * SPTREE_LOCK before key_delsp: may refcnt
-				 * be increased again during that time ?
-				 * NB: also clean entries created by
-				 * key_spdflush
-				 */
-				SP_DELREF(sp);
-				key_delsp(sp);
-				SPTREE_UNLOCK();
-				goto restart;
-			}
+		SPTREE_RLOCK();
+		TAILQ_FOREACH(sp, &V_sptree[dir], chain) {
 			if (sp->lifetime == 0 && sp->validtime == 0)
 				continue;
-			if ((sp->lifetime && now - sp->created > sp->lifetime)
-			 || (sp->validtime && now - sp->lastused > sp->validtime)) {
-				sp->state = IPSEC_SPSTATE_DEAD;
-				SPTREE_UNLOCK();
+			if ((sp->lifetime &&
+			    now - sp->created > sp->lifetime) ||
+			    (sp->validtime &&
+			    now - sp->lastused > sp->validtime)) {
+				SPTREE_RUNLOCK();
+				key_unlink(sp);
 				key_spdexpire(sp);
+				KEY_FREESP(&sp);
 				goto restart;
 			}
 		}
-		SPTREE_UNLOCK();
+		SPTREE_RUNLOCK();
 	}
 }
 
@@ -7609,7 +7563,7 @@ key_init(void)
 	int i;
 
 	for (i = 0; i < IPSEC_DIR_MAX; i++)
-		LIST_INIT(&V_sptree[i]);
+		TAILQ_INIT(&V_sptree[i]);
 
 	LIST_INIT(&V_sahtree);
 
@@ -7619,10 +7573,6 @@ key_init(void)
 	LIST_INIT(&V_acqtree);
 	LIST_INIT(&V_spacqtree);
 
-	/* system default */
-	V_ip4_def_policy.policy = IPSEC_POLICY_NONE;
-	V_ip4_def_policy.refcnt++;	/*never reclaim this*/
-
 	if (!IS_DEFAULT_VNET(curvnet))
 		return;
 
@@ -7647,6 +7597,7 @@ key_init(void)
 void
 key_destroy(void)
 {
+	TAILQ_HEAD(, secpolicy) drainq;
 	struct secpolicy *sp, *nextsp;
 	struct secacq *acq, *nextacq;
 	struct secspacq *spacq, *nextspacq;
@@ -7654,18 +7605,18 @@ key_destroy(void)
 	struct secreg *reg;
 	int i;
 
-	SPTREE_LOCK();
+	TAILQ_INIT(&drainq);
+	SPTREE_WLOCK();
 	for (i = 0; i < IPSEC_DIR_MAX; i++) {
-		for (sp = LIST_FIRST(&V_sptree[i]); 
-		    sp != NULL; sp = nextsp) {
-			nextsp = LIST_NEXT(sp, chain);
-			if (__LIST_CHAINED(sp)) {
-				LIST_REMOVE(sp, chain);
-				free(sp, M_IPSEC_SP);
-			}
-		}
+		TAILQ_CONCAT(&drainq, &V_sptree[dir], chain);
+	}
+	SPTREE_WUNLOCK();
+	sp = TAILQ_FIRST(&drainq);
+	while (sp != NULL) {
+		nextsp = TAILQ_NEXT(sp, chain);
+		KEY_FREESP(&sp);
+		sp = nextsp;
 	}
-	SPTREE_UNLOCK();
 
 	SAHTREE_LOCK();
 	for (sah = LIST_FIRST(&V_sahtree); sah != NULL; sah = nextsah) {

Modified: head/sys/netipsec/key_debug.c
==============================================================================
--- head/sys/netipsec/key_debug.c	Wed Dec 24 17:12:51 2014	(r276187)
+++ head/sys/netipsec/key_debug.c	Wed Dec 24 18:34:56 2014	(r276188)
@@ -463,8 +463,8 @@ kdebug_secpolicy(struct secpolicy *sp)
 	if (sp == NULL)
 		panic("%s: NULL pointer was passed.\n", __func__);
 
-	printf("secpolicy{ refcnt=%u state=%u policy=%u\n",
-		sp->refcnt, sp->state, sp->policy);
+	printf("secpolicy{ refcnt=%u policy=%u\n",
+		sp->refcnt, sp->policy);
 
 	kdebug_secpolicyindex(&sp->spidx);
 



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