Author: fabient
Date: Fri May  6 13:24:10 2011
New Revision: 221525
URL: http://svn.freebsd.org/changeset/base/221525

Log:
  MFC r220206:
  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.

Modified:
  stable/8/sys/netipsec/ipsec.h
  stable/8/sys/netipsec/key.c
  stable/8/sys/netipsec/key.h
  stable/8/sys/netipsec/xform.h
  stable/8/sys/netipsec/xform_ah.c
  stable/8/sys/netipsec/xform_esp.c
  stable/8/sys/netipsec/xform_ipcomp.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)

Modified: stable/8/sys/netipsec/ipsec.h
==============================================================================
--- stable/8/sys/netipsec/ipsec.h       Fri May  6 13:12:45 2011        
(r221524)
+++ stable/8/sys/netipsec/ipsec.h       Fri May  6 13:24:10 2011        
(r221525)
@@ -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: stable/8/sys/netipsec/key.c
==============================================================================
--- stable/8/sys/netipsec/key.c Fri May  6 13:12:45 2011        (r221524)
+++ stable/8/sys/netipsec/key.c Fri May  6 13:24:10 2011        (r221525)
@@ -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) {
@@ -1239,6 +1226,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: stable/8/sys/netipsec/key.h
==============================================================================
--- stable/8/sys/netipsec/key.h Fri May  6 13:12:45 2011        (r221524)
+++ stable/8/sys/netipsec/key.h Fri May  6 13:24:10 2011        (r221525)
@@ -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: stable/8/sys/netipsec/xform.h
==============================================================================
--- stable/8/sys/netipsec/xform.h       Fri May  6 13:12:45 2011        
(r221524)
+++ stable/8/sys/netipsec/xform.h       Fri May  6 13:24:10 2011        
(r221525)
@@ -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: stable/8/sys/netipsec/xform_ah.c
==============================================================================
--- stable/8/sys/netipsec/xform_ah.c    Fri May  6 13:12:45 2011        
(r221524)
+++ stable/8/sys/netipsec/xform_ah.c    Fri May  6 13:24:10 2011        
(r221525)
@@ -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: stable/8/sys/netipsec/xform_esp.c
==============================================================================
--- stable/8/sys/netipsec/xform_esp.c   Fri May  6 13:12:45 2011        
(r221524)
+++ stable/8/sys/netipsec/xform_esp.c   Fri May  6 13:24:10 2011        
(r221525)
@@ -423,6 +423,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) {
@@ -484,15 +486,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 ||
@@ -509,7 +504,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;
                }
@@ -877,6 +871,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;
@@ -926,8 +922,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),
@@ -935,8 +932,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) {
@@ -945,7 +940,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: stable/8/sys/netipsec/xform_ipcomp.c
==============================================================================
--- stable/8/sys/netipsec/xform_ipcomp.c        Fri May  6 13:12:45 2011        
(r221524)
+++ stable/8/sys/netipsec/xform_ipcomp.c        Fri May  6 13:24:10 2011        
(r221525)
@@ -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>
@@ -206,6 +207,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);
 }
@@ -249,13 +252,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 ||
@@ -269,7 +267,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++;
@@ -452,6 +449,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;
@@ -492,14 +491,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) {
@@ -508,7 +507,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);
                }
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to