From 95708c5fe33c5b18e437dcf72bc4850e08d73f8d Mon Sep 17 00:00:00 2001
From: Pawel Jakub Dawidek <pjd@FreeBSD.org>
Date: Sat, 20 May 2006 15:35:36 +0000
Subject: [PATCH] Prevent disappearing SAD entries by implementing MPsafe
 refcounting. "Why didn't he use SECASVAR_LOCK()/SECASVAR_UNLOCK() macros to 
 synchronize access to the secasvar structure's fields?" one may ask. There
 were two reasons: 1. refcount(9) is faster then mutex(9) synchronization (one
 atomic    operation instead of two). 2. Those macros are not used now at all,
 so at some point we may decide    to remove them entirely.

OK'ed by:	gnn
MFC after:	2 weeks
---
 sys/netipsec/key.c | 53 +++++++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/sys/netipsec/key.c b/sys/netipsec/key.c
index 843e71cfb9a..be77509873b 100644
--- a/sys/netipsec/key.c
+++ b/sys/netipsec/key.c
@@ -54,6 +54,7 @@
 #include <sys/errno.h>
 #include <sys/proc.h>
 #include <sys/queue.h>
+#include <sys/refcount.h>
 #include <sys/syslog.h>
 
 #include <net/if.h>
@@ -502,14 +503,26 @@ static const char *key_getuserfqdn __P((void));
 static void key_sa_chgstate __P((struct secasvar *, u_int8_t));
 static struct mbuf *key_alloc_mbuf __P((int));
 
-#define	SA_ADDREF(p) do {						\
-	(p)->refcnt++;							\
-	IPSEC_ASSERT((p)->refcnt != 0, ("SA refcnt overflow"));		\
-} while (0)
-#define	SA_DELREF(p) do {						\
-	IPSEC_ASSERT((p)->refcnt > 0, ("SA refcnt underflow"));		\
-	(p)->refcnt--;							\
-} while (0)
+static __inline void
+sa_initref(struct secasvar *sav)
+{
+
+	refcount_init(&sav->refcnt, 1);
+}
+static __inline void
+sa_addref(struct secasvar *sav)
+{
+
+	refcount_acquire(&sav->refcnt);
+	IPSEC_ASSERT(sav->refcnt != 0, ("SA refcnt overflow"));
+}
+static __inline int
+sa_delref(struct secasvar *sav)
+{
+
+	IPSEC_ASSERT(sav->refcnt > 0, ("SA refcnt underflow"));
+	return (refcount_release(&sav->refcnt));
+}
 
 #define	SP_ADDREF(p) do {						\
 	(p)->refcnt++;							\
@@ -1000,7 +1013,7 @@ key_do_allocsa_policy(struct secashead *sah, u_int state)
 		}
 	}
 	if (candidate) {
-		SA_ADDREF(candidate);
+		sa_addref(candidate);
 		KEYDEBUG(KEYDEBUG_IPSEC_STAMP,
 			printf("DP %s cause refcnt++:%d SA:%p\n",
 				__func__, candidate->refcnt, candidate));
@@ -1079,7 +1092,7 @@ key_allocsa(
 				/* check dst address */
 				if (key_sockaddrcmp(&dst->sa, &sav->sah->saidx.dst.sa, 0) != 0)
 					continue;
-				SA_ADDREF(sav);
+				sa_addref(sav);
 				goto done;
 			}
 		}
@@ -1198,16 +1211,16 @@ key_freesav(struct secasvar **psav, const char* where, int tag)
 
 	IPSEC_ASSERT(sav != NULL, ("null sav"));
 
-	/* XXX unguarded? */
-	SA_DELREF(sav);
-
-	KEYDEBUG(KEYDEBUG_IPSEC_STAMP,
-		printf("DP %s SA:%p (SPI %u) from %s:%u; refcnt now %u\n",
-			__func__, sav, ntohl(sav->spi), where, tag, sav->refcnt));
-
-	if (sav->refcnt == 0) {
+	if (sa_delref(sav)) {
+		KEYDEBUG(KEYDEBUG_IPSEC_STAMP,
+			printf("DP %s SA:%p (SPI %u) from %s:%u; refcnt now %u\n",
+				__func__, sav, ntohl(sav->spi), where, tag, sav->refcnt));
 		*psav = NULL;
 		key_delsav(sav);
+	} else {
+		KEYDEBUG(KEYDEBUG_IPSEC_STAMP,
+			printf("DP %s SA:%p (SPI %u) from %s:%u; refcnt now %u\n",
+				__func__, sav, ntohl(sav->spi), where, tag, sav->refcnt));
 	}
 }
 
@@ -2764,7 +2777,7 @@ key_newsav(m, mhp, sah, errp, where, tag)
 
 	/* add to satree */
 	newsav->sah = sah;
-	newsav->refcnt = 1;
+	sa_initref(newsav);
 	newsav->state = SADB_SASTATE_LARVAL;
 
 	/* XXX locking??? */
@@ -4822,7 +4835,7 @@ key_getsavbyseq(sah, seq)
 		KEY_CHKSASTATE(state, sav->state, __func__);
 
 		if (sav->seq == seq) {
-			SA_ADDREF(sav);
+			sa_addref(sav);
 			KEYDEBUG(KEYDEBUG_IPSEC_STAMP,
 				printf("DP %s cause refcnt++:%d SA:%p\n",
 					__func__, sav->refcnt, sav));