Date: Thu, 31 Mar 2011 15:23:32 +0000 (UTC) From: Fabien Thomas <fabient@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r220206 - head/sys/netipsec Message-ID: <201103311523.p2VFNWNf010554@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: fabient Date: Thu Mar 31 15:23:32 2011 New Revision: 220206 URL: http://svn.freebsd.org/changeset/base/220206 Log: Optimisation in IPSEC(4): - Remove contention on ISR during the crypto operation by using rwlock(9). - Remove a second lookup of the SA in the callback. Gain on 6 cores CPU with SHA1/AES128 can be up to 30%. Reviewed by: vanhu MFC after: 1 month Modified: head/sys/netipsec/ipsec.h head/sys/netipsec/key.c head/sys/netipsec/key.h head/sys/netipsec/xform.h head/sys/netipsec/xform_ah.c head/sys/netipsec/xform_esp.c head/sys/netipsec/xform_ipcomp.c Modified: head/sys/netipsec/ipsec.h ============================================================================== --- head/sys/netipsec/ipsec.h Thu Mar 31 15:12:40 2011 (r220205) +++ head/sys/netipsec/ipsec.h Thu Mar 31 15:23:32 2011 (r220206) @@ -123,7 +123,7 @@ struct ipsecrequest { struct secasvar *sav; /* place holder of SA for use */ struct secpolicy *sp; /* back pointer to SP */ - struct mtx lock; /* to interlock updates */ + struct rwlock lock; /* to interlock updates */ }; /* @@ -132,11 +132,15 @@ struct ipsecrequest { * hard it is to remove this... */ #define IPSECREQUEST_LOCK_INIT(_isr) \ - mtx_init(&(_isr)->lock, "ipsec request", NULL, MTX_DEF | MTX_RECURSE) -#define IPSECREQUEST_LOCK(_isr) mtx_lock(&(_isr)->lock) -#define IPSECREQUEST_UNLOCK(_isr) mtx_unlock(&(_isr)->lock) -#define IPSECREQUEST_LOCK_DESTROY(_isr) mtx_destroy(&(_isr)->lock) -#define IPSECREQUEST_LOCK_ASSERT(_isr) mtx_assert(&(_isr)->lock, MA_OWNED) + rw_init_flags(&(_isr)->lock, "ipsec request", RW_RECURSE) +#define IPSECREQUEST_LOCK(_isr) rw_rlock(&(_isr)->lock) +#define IPSECREQUEST_UNLOCK(_isr) rw_runlock(&(_isr)->lock) +#define IPSECREQUEST_WLOCK(_isr) rw_wlock(&(_isr)->lock) +#define IPSECREQUEST_WUNLOCK(_isr) rw_wunlock(&(_isr)->lock) +#define IPSECREQUEST_UPGRADE(_isr) rw_try_upgrade(&(_isr)->lock) +#define IPSECREQUEST_DOWNGRADE(_isr) rw_downgrade(&(_isr)->lock) +#define IPSECREQUEST_LOCK_DESTROY(_isr) rw_destroy(&(_isr)->lock) +#define IPSECREQUEST_LOCK_ASSERT(_isr) rw_assert(&(_isr)->lock, RA_LOCKED) /* security policy in PCB */ struct inpcbpolicy { Modified: head/sys/netipsec/key.c ============================================================================== --- head/sys/netipsec/key.c Thu Mar 31 15:12:40 2011 (r220205) +++ head/sys/netipsec/key.c Thu Mar 31 15:23:32 2011 (r220206) @@ -809,6 +809,7 @@ key_checkrequest(struct ipsecrequest *is { u_int level; int error; + struct secasvar *sav; IPSEC_ASSERT(isr != NULL, ("null isr")); IPSEC_ASSERT(saidx != NULL, ("null saidx")); @@ -826,45 +827,31 @@ key_checkrequest(struct ipsecrequest *is /* get current level */ level = ipsec_get_reqlevel(isr); -#if 0 - /* - * We do allocate new SA only if the state of SA in the holder is - * SADB_SASTATE_DEAD. The SA for outbound must be the oldest. - */ - if (isr->sav != NULL) { - if (isr->sav->sah == NULL) - panic("%s: sah is null.\n", __func__); - if (isr->sav == (struct secasvar *)LIST_FIRST( - &isr->sav->sah->savtree[SADB_SASTATE_DEAD])) { - KEY_FREESAV(&isr->sav); - isr->sav = NULL; - } - } -#else + /* - * we free any SA stashed in the IPsec request because a different + * We check new SA in the IPsec request because a different * SA may be involved each time this request is checked, either * because new SAs are being configured, or this request is * associated with an unconnected datagram socket, or this request * is associated with a system default policy. * - * The operation may have negative impact to performance. We may - * want to check cached SA carefully, rather than picking new SA - * every time. - */ - if (isr->sav != NULL) { - KEY_FREESAV(&isr->sav); - isr->sav = NULL; - } -#endif - - /* - * new SA allocation if no SA found. * key_allocsa_policy should allocate the oldest SA available. * See key_do_allocsa_policy(), and draft-jenkins-ipsec-rekeying-03.txt. */ - if (isr->sav == NULL) - isr->sav = key_allocsa_policy(saidx); + sav = key_allocsa_policy(saidx); + if (sav != isr->sav) { + /* SA need to be updated. */ + if (!IPSECREQUEST_UPGRADE(isr)) { + /* Kick everyone off. */ + IPSECREQUEST_UNLOCK(isr); + IPSECREQUEST_WLOCK(isr); + } + if (isr->sav != NULL) + KEY_FREESAV(&isr->sav); + isr->sav = sav; + IPSECREQUEST_DOWNGRADE(isr); + } else if (sav != NULL) + KEY_FREESAV(&sav); /* When there is SA. */ if (isr->sav != NULL) { @@ -1240,6 +1227,16 @@ key_freesp_so(struct secpolicy **sp) KEY_FREESP(sp); } +void +key_addrefsa(struct secasvar *sav, const char* where, int tag) +{ + + IPSEC_ASSERT(sav != NULL, ("null sav")); + IPSEC_ASSERT(sav->refcnt > 0, ("refcount must exist")); + + sa_addref(sav); +} + /* * Must be called after calling key_allocsa(). * This function is called by key_freesp() to free some SA allocated Modified: head/sys/netipsec/key.h ============================================================================== --- head/sys/netipsec/key.h Thu Mar 31 15:12:40 2011 (r220205) +++ head/sys/netipsec/key.h Thu Mar 31 15:23:32 2011 (r220206) @@ -76,10 +76,13 @@ extern void _key_freesp(struct secpolicy extern struct secasvar *key_allocsa(union sockaddr_union *, u_int, u_int32_t, const char*, int); +extern void key_addrefsa(struct secasvar *, const char*, int); extern void key_freesav(struct secasvar **, const char*, int); #define KEY_ALLOCSA(dst, proto, spi) \ key_allocsa(dst, proto, spi, __FILE__, __LINE__) +#define KEY_ADDREFSA(sav) \ + key_addrefsa(sav, __FILE__, __LINE__) #define KEY_FREESAV(psav) \ key_freesav(psav, __FILE__, __LINE__) Modified: head/sys/netipsec/xform.h ============================================================================== --- head/sys/netipsec/xform.h Thu Mar 31 15:12:40 2011 (r220205) +++ head/sys/netipsec/xform.h Thu Mar 31 15:23:32 2011 (r220206) @@ -75,6 +75,7 @@ struct tdb_crypto { int tc_protoff; /* current protocol offset */ int tc_skip; /* data offset */ caddr_t tc_ptr; /* associated crypto data */ + struct secasvar *tc_sav; /* related SA */ }; struct secasvar; Modified: head/sys/netipsec/xform_ah.c ============================================================================== --- head/sys/netipsec/xform_ah.c Thu Mar 31 15:12:40 2011 (r220205) +++ head/sys/netipsec/xform_ah.c Thu Mar 31 15:23:32 2011 (r220206) @@ -715,6 +715,8 @@ ah_input(struct mbuf *m, struct secasvar tc->tc_protoff = protoff; tc->tc_skip = skip; tc->tc_ptr = (caddr_t) mtag; /* Save the mtag we've identified. */ + KEY_ADDREFSA(sav); + tc->tc_sav = sav; if (mtag == NULL) return crypto_dispatch(crp); @@ -764,13 +766,8 @@ ah_input_cb(struct cryptop *crp) mtag = (struct m_tag *) tc->tc_ptr; m = (struct mbuf *) crp->crp_buf; - sav = KEY_ALLOCSA(&tc->tc_dst, tc->tc_proto, tc->tc_spi); - if (sav == NULL) { - V_ahstat.ahs_notdb++; - DPRINTF(("%s: SA expired while in crypto\n", __func__)); - error = ENOBUFS; /*XXX*/ - goto bad; - } + sav = tc->tc_sav; + IPSEC_ASSERT(sav != NULL, ("null SA!")); saidx = &sav->sah->saidx; IPSEC_ASSERT(saidx->dst.sa.sa_family == AF_INET || @@ -785,7 +782,6 @@ ah_input_cb(struct cryptop *crp) sav->tdb_cryptoid = crp->crp_sid; if (crp->crp_etype == EAGAIN) { - KEY_FREESAV(&sav); error = crypto_dispatch(crp); return error; } @@ -1111,6 +1107,8 @@ ah_output( /* These are passed as-is to the callback. */ tc->tc_isr = isr; + KEY_ADDREFSA(sav); + tc->tc_sav = sav; tc->tc_spi = sav->spi; tc->tc_dst = sav->sah->saidx.dst; tc->tc_proto = sav->sah->saidx.proto; @@ -1147,14 +1145,14 @@ ah_output_cb(struct cryptop *crp) isr = tc->tc_isr; IPSECREQUEST_LOCK(isr); - sav = KEY_ALLOCSA(&tc->tc_dst, tc->tc_proto, tc->tc_spi); - if (sav == NULL) { + sav = tc->tc_sav; + /* With the isr lock released SA pointer can be updated. */ + if (sav != isr->sav) { V_ahstat.ahs_notdb++; DPRINTF(("%s: SA expired while in crypto\n", __func__)); error = ENOBUFS; /*XXX*/ goto bad; } - IPSEC_ASSERT(isr->sav == sav, ("SA changed\n")); /* Check for crypto errors. */ if (crp->crp_etype) { @@ -1162,7 +1160,6 @@ ah_output_cb(struct cryptop *crp) sav->tdb_cryptoid = crp->crp_sid; if (crp->crp_etype == EAGAIN) { - KEY_FREESAV(&sav); IPSECREQUEST_UNLOCK(isr); error = crypto_dispatch(crp); return error; Modified: head/sys/netipsec/xform_esp.c ============================================================================== --- head/sys/netipsec/xform_esp.c Thu Mar 31 15:12:40 2011 (r220205) +++ head/sys/netipsec/xform_esp.c Thu Mar 31 15:23:32 2011 (r220206) @@ -429,6 +429,8 @@ esp_input(struct mbuf *m, struct secasva tc->tc_proto = sav->sah->saidx.proto; tc->tc_protoff = protoff; tc->tc_skip = skip; + KEY_ADDREFSA(sav); + tc->tc_sav = sav; /* Decryption descriptor */ if (espx) { @@ -490,15 +492,8 @@ esp_input_cb(struct cryptop *crp) mtag = (struct m_tag *) tc->tc_ptr; m = (struct mbuf *) crp->crp_buf; - sav = KEY_ALLOCSA(&tc->tc_dst, tc->tc_proto, tc->tc_spi); - if (sav == NULL) { - V_espstat.esps_notdb++; - DPRINTF(("%s: SA gone during crypto (SA %s/%08lx proto %u)\n", - __func__, ipsec_address(&tc->tc_dst), - (u_long) ntohl(tc->tc_spi), tc->tc_proto)); - error = ENOBUFS; /*XXX*/ - goto bad; - } + sav = tc->tc_sav; + IPSEC_ASSERT(sav != NULL, ("null SA!")); saidx = &sav->sah->saidx; IPSEC_ASSERT(saidx->dst.sa.sa_family == AF_INET || @@ -515,7 +510,6 @@ esp_input_cb(struct cryptop *crp) sav->tdb_cryptoid = crp->crp_sid; if (crp->crp_etype == EAGAIN) { - KEY_FREESAV(&sav); error = crypto_dispatch(crp); return error; } @@ -883,6 +877,8 @@ esp_output( /* Callback parameters */ tc->tc_isr = isr; + KEY_ADDREFSA(sav); + tc->tc_sav = sav; tc->tc_spi = sav->spi; tc->tc_dst = saidx->dst; tc->tc_proto = saidx->proto; @@ -932,8 +928,9 @@ esp_output_cb(struct cryptop *crp) isr = tc->tc_isr; IPSECREQUEST_LOCK(isr); - sav = KEY_ALLOCSA(&tc->tc_dst, tc->tc_proto, tc->tc_spi); - if (sav == NULL) { + sav = tc->tc_sav; + /* With the isr lock released SA pointer can be updated. */ + if (sav != isr->sav) { V_espstat.esps_notdb++; DPRINTF(("%s: SA gone during crypto (SA %s/%08lx proto %u)\n", __func__, ipsec_address(&tc->tc_dst), @@ -941,8 +938,6 @@ esp_output_cb(struct cryptop *crp) error = ENOBUFS; /*XXX*/ goto bad; } - IPSEC_ASSERT(isr->sav == sav, - ("SA changed was %p now %p\n", isr->sav, sav)); /* Check for crypto errors. */ if (crp->crp_etype) { @@ -951,7 +946,6 @@ esp_output_cb(struct cryptop *crp) sav->tdb_cryptoid = crp->crp_sid; if (crp->crp_etype == EAGAIN) { - KEY_FREESAV(&sav); IPSECREQUEST_UNLOCK(isr); error = crypto_dispatch(crp); return error; Modified: head/sys/netipsec/xform_ipcomp.c ============================================================================== --- head/sys/netipsec/xform_ipcomp.c Thu Mar 31 15:12:40 2011 (r220205) +++ head/sys/netipsec/xform_ipcomp.c Thu Mar 31 15:23:32 2011 (r220206) @@ -37,6 +37,7 @@ #include <sys/mbuf.h> #include <sys/lock.h> #include <sys/mutex.h> +#include <sys/rwlock.h> #include <sys/socket.h> #include <sys/kernel.h> #include <sys/protosw.h> @@ -185,6 +186,8 @@ ipcomp_input(struct mbuf *m, struct seca tc->tc_proto = sav->sah->saidx.proto; tc->tc_protoff = protoff; tc->tc_skip = skip; + KEY_ADDREFSA(sav); + tc->tc_sav = sav; return crypto_dispatch(crp); } @@ -228,13 +231,8 @@ ipcomp_input_cb(struct cryptop *crp) mtag = (struct mtag *) tc->tc_ptr; m = (struct mbuf *) crp->crp_buf; - sav = KEY_ALLOCSA(&tc->tc_dst, tc->tc_proto, tc->tc_spi); - if (sav == NULL) { - V_ipcompstat.ipcomps_notdb++; - DPRINTF(("%s: SA expired while in crypto\n", __func__)); - error = ENOBUFS; /*XXX*/ - goto bad; - } + sav = tc->tc_sav; + IPSEC_ASSERT(sav != NULL, ("null SA!")); saidx = &sav->sah->saidx; IPSEC_ASSERT(saidx->dst.sa.sa_family == AF_INET || @@ -248,7 +246,6 @@ ipcomp_input_cb(struct cryptop *crp) sav->tdb_cryptoid = crp->crp_sid; if (crp->crp_etype == EAGAIN) { - KEY_FREESAV(&sav); return crypto_dispatch(crp); } V_ipcompstat.ipcomps_noxform++; @@ -431,6 +428,8 @@ ipcomp_output( } tc->tc_isr = isr; + KEY_ADDREFSA(sav); + tc->tc_sav = sav; tc->tc_spi = sav->spi; tc->tc_dst = sav->sah->saidx.dst; tc->tc_proto = sav->sah->saidx.proto; @@ -471,14 +470,14 @@ ipcomp_output_cb(struct cryptop *crp) isr = tc->tc_isr; IPSECREQUEST_LOCK(isr); - sav = KEY_ALLOCSA(&tc->tc_dst, tc->tc_proto, tc->tc_spi); - if (sav == NULL) { + sav = tc->tc_sav; + /* With the isr lock released SA pointer can be updated. */ + if (sav != isr->sav) { V_ipcompstat.ipcomps_notdb++; DPRINTF(("%s: SA expired while in crypto\n", __func__)); error = ENOBUFS; /*XXX*/ goto bad; } - IPSEC_ASSERT(isr->sav == sav, ("SA changed\n")); /* Check for crypto errors */ if (crp->crp_etype) { @@ -487,7 +486,6 @@ ipcomp_output_cb(struct cryptop *crp) sav->tdb_cryptoid = crp->crp_sid; if (crp->crp_etype == EAGAIN) { - KEY_FREESAV(&sav); IPSECREQUEST_UNLOCK(isr); return crypto_dispatch(crp); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201103311523.p2VFNWNf010554>