Do not queue crypto operations for IPsec. The packet entries in

task queues were unlimited and could overflow during havy traffic.
Even if we still use hardware drivers that sleep, softnet task
instead of soft interrupt can handle this now.  Without queues net
lock is inherited and kernel lock is only needed once per packet.
This results in less lock contention and faster IPsec.
Also protect tdb drop counters with net lock and avoid a leak in
crypto dispatch error handling.
intense testing Hrvoje Popovski; OK mpi@

diff --git a/sys/crypto/crypto.c b/sys/crypto/crypto.c
index 4f3f1f8..bff6dd4 100644 (file)
--- a/sys/crypto/crypto.c
+++ b/sys/crypto/crypto.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: crypto.c,v 1.84 2021/07/21 11:11:41 bluhm Exp $       */
+/*     $OpenBSD: crypto.c,v 1.85 2021/07/26 21:27:56 bluhm Exp $       */
 /*
  * The author of this code is Angelos D. Keromytis (angelos@cis.upenn.edu)
  *
@@ -387,23 +387,32 @@ crypto_unregister(u_int32_t driverid, int alg)
 int
 crypto_dispatch(struct cryptop *crp)
 {
-       struct taskq *tq = crypto_taskq;
-       int error = 0, s;
+       int error = 0, lock = 1, s;
        u_int32_t hid;
 
        s = splvm();
        hid = (crp->crp_sid >> 32) & 0xffffffff;
        if (hid < crypto_drivers_num) {
                if (crypto_drivers[hid].cc_flags & CRYPTOCAP_F_MPSAFE)
-                       tq = crypto_taskq_mpsafe;
+                       lock = 0;
        }
        splx(s);
 
-       if ((crp->crp_flags & CRYPTO_F_NOQUEUE) == 0) {
+       /* XXXSMP crypto_invoke() is not MP safe */
+       lock = 1;
+
+       if (crp->crp_flags & CRYPTO_F_NOQUEUE) {
+               if (lock)
+                       KERNEL_LOCK();
+               error = crypto_invoke(crp);
+               if (lock)
+                       KERNEL_UNLOCK();
+       } else {
+               struct taskq *tq;
+
+               tq = lock ? crypto_taskq : crypto_taskq_mpsafe;
                task_set(&crp->crp_task, (void (*))crypto_invoke, crp);
                task_add(tq, &crp->crp_task);
-       } else {
-               error = crypto_invoke(crp);
        }
 
        return error;
@@ -424,6 +433,8 @@ crypto_invoke(struct cryptop *crp)
        if (crp == NULL || crp->crp_callback == NULL)
                return EINVAL;
 
+       KERNEL_ASSERT_LOCKED();
+
        s = splvm();
        if (crp->crp_ndesc < 1 || crypto_drivers == NULL) {
                crp->crp_etype = EINVAL;
@@ -535,11 +546,16 @@ void
 crypto_done(struct cryptop *crp)
 {
        crp->crp_flags |= CRYPTO_F_DONE;
+
        if (crp->crp_flags & CRYPTO_F_NOQUEUE) {
                /* not from the crypto queue, wakeup the userland process */
                crp->crp_callback(crp);
        } else {
+               struct taskq *tq;
+
+               tq = (crp->crp_flags & CRYPTO_F_MPSAFE) ?
+                   crypto_taskq_mpsafe : crypto_taskq;
                task_set(&crp->crp_task, (void (*))crp->crp_callback, crp);
-               task_add(crypto_taskq, &crp->crp_task);
+               task_add(tq, &crp->crp_task);
        }
 }

diff --git a/sys/crypto/cryptodev.h b/sys/crypto/cryptodev.h
index 42ef0c5..10bb1a6 100644 (file)
--- a/sys/crypto/cryptodev.h
+++ b/sys/crypto/cryptodev.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: cryptodev.h,v 1.73 2021/07/09 20:43:28 mvs Exp $      */
+/*     $OpenBSD: cryptodev.h,v 1.74 2021/07/26 21:27:56 bluhm Exp $    */
 
 /*
  * The author of this code is Angelos D. Keromytis (angelos@cis.upenn.edu)
@@ -171,6 +171,7 @@ struct cryptop {
 
 #define CRYPTO_F_IMBUF 0x0001  /* Input/output are mbuf chains, otherwise contig */
 #define CRYPTO_F_IOV   0x0002  /* Input/output are uio */
+#define CRYPTO_F_MPSAFE        0x0004  /* Do not use kernel lock for callback */
 #define CRYPTO_F_NOQUEUE       0x0008  /* Don't use crypto queue/thread */
 #define CRYPTO_F_DONE  0x0010  /* request completed */
 

diff --git a/sys/netinet/ip_ah.c b/sys/netinet/ip_ah.c
index b9c4ffa..70a0c17 100644 (file)
--- a/sys/netinet/ip_ah.c
+++ b/sys/netinet/ip_ah.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_ah.c,v 1.151 2021/07/18 14:38:20 bluhm Exp $ */
+/*     $OpenBSD: ip_ah.c,v 1.152 2021/07/26 21:27:56 bluhm Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -684,7 +684,7 @@ ah_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
 
        /* Crypto operation descriptor. */
        crp->crp_ilen = m->m_pkthdr.len; /* Total input length. */
-       crp->crp_flags = CRYPTO_F_IMBUF;
+       crp->crp_flags = CRYPTO_F_IMBUF | CRYPTO_F_MPSAFE | CRYPTO_F_NOQUEUE;
        crp->crp_buf = (caddr_t)m;
        crp->crp_callback = ipsec_input_cb;
        crp->crp_sid = tdb->tdb_cryptoid;
@@ -699,9 +699,7 @@ ah_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
        memcpy(&tc->tc_dst, &tdb->tdb_dst, sizeof(union sockaddr_union));
        tc->tc_rpl = tdb->tdb_rpl;
 
-       KERNEL_LOCK();
        error = crypto_dispatch(crp);
-       KERNEL_UNLOCK();
        return error;
 
  drop:
@@ -1134,7 +1132,7 @@ ah_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 
        /* Crypto operation descriptor. */
        crp->crp_ilen = m->m_pkthdr.len; /* Total input length. */
-       crp->crp_flags = CRYPTO_F_IMBUF;
+       crp->crp_flags = CRYPTO_F_IMBUF | CRYPTO_F_MPSAFE | CRYPTO_F_NOQUEUE;
        crp->crp_buf = (caddr_t)m;
        crp->crp_callback = ipsec_output_cb;
        crp->crp_sid = tdb->tdb_cryptoid;
@@ -1148,9 +1146,7 @@ ah_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
        tc->tc_rdomain = tdb->tdb_rdomain;
        memcpy(&tc->tc_dst, &tdb->tdb_dst, sizeof(union sockaddr_union));
 
-       KERNEL_LOCK();
        error = crypto_dispatch(crp);
-       KERNEL_UNLOCK();
        return error;
 
  drop:

diff --git a/sys/netinet/ip_esp.c b/sys/netinet/ip_esp.c
index 11a323f..7865c9a 100644 (file)
--- a/sys/netinet/ip_esp.c
+++ b/sys/netinet/ip_esp.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_esp.c,v 1.169 2021/07/18 14:38:20 bluhm Exp $ */
+/*     $OpenBSD: ip_esp.c,v 1.170 2021/07/26 21:27:57 bluhm Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -498,7 +498,7 @@ esp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
 
        /* Crypto operation descriptor */
        crp->crp_ilen = m->m_pkthdr.len; /* Total input length */
-       crp->crp_flags = CRYPTO_F_IMBUF;
+       crp->crp_flags = CRYPTO_F_IMBUF | CRYPTO_F_MPSAFE | CRYPTO_F_NOQUEUE;
        crp->crp_buf = (caddr_t)m;
        crp->crp_callback = ipsec_input_cb;
        crp->crp_sid = tdb->tdb_cryptoid;
@@ -528,9 +528,7 @@ esp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
                        crde->crd_len = m->m_pkthdr.len - (skip + hlen + alen);
        }
 
-       KERNEL_LOCK();
        error = crypto_dispatch(crp);
-       KERNEL_UNLOCK();
        return error;
 
  drop:
@@ -983,7 +981,7 @@ esp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 
        /* Crypto operation descriptor. */
        crp->crp_ilen = m->m_pkthdr.len; /* Total input length. */
-       crp->crp_flags = CRYPTO_F_IMBUF;
+       crp->crp_flags = CRYPTO_F_IMBUF | CRYPTO_F_MPSAFE | CRYPTO_F_NOQUEUE;
        crp->crp_buf = (caddr_t)m;
        crp->crp_callback = ipsec_output_cb;
        crp->crp_opaque = (caddr_t)tc;
@@ -1015,9 +1013,7 @@ esp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
                        crda->crd_len = m->m_pkthdr.len - (skip + alen);
        }
 
-       KERNEL_LOCK();
        error = crypto_dispatch(crp);
-       KERNEL_UNLOCK();
        return error;
 
  drop:

diff --git a/sys/netinet/ip_ipcomp.c b/sys/netinet/ip_ipcomp.c
index 1b29d70..d7f53d1 100644 (file)
--- a/sys/netinet/ip_ipcomp.c
+++ b/sys/netinet/ip_ipcomp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ip_ipcomp.c,v 1.71 2021/07/08 21:07:19 bluhm Exp $ */
+/* $OpenBSD: ip_ipcomp.c,v 1.72 2021/07/26 21:27:57 bluhm Exp $ */
 
 /*
  * Copyright (c) 2001 Jean-Jacques Bernard-Gundol (jj@wabbitt.org)
@@ -174,7 +174,7 @@ ipcomp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
 
        /* Crypto operation descriptor */
        crp->crp_ilen = m->m_pkthdr.len - (skip + hlen);
-       crp->crp_flags = CRYPTO_F_IMBUF;
+       crp->crp_flags = CRYPTO_F_IMBUF | CRYPTO_F_MPSAFE | CRYPTO_F_NOQUEUE;
        crp->crp_buf = (caddr_t)m;
        crp->crp_callback = ipsec_input_cb;
        crp->crp_sid = tdb->tdb_cryptoid;
@@ -188,9 +188,7 @@ ipcomp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
        tc->tc_rdomain = tdb->tdb_rdomain;
        tc->tc_dst = tdb->tdb_dst;
 
-       KERNEL_LOCK();
        error = crypto_dispatch(crp);
-       KERNEL_UNLOCK();
        return error;
 }
 
@@ -479,15 +477,13 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 
        /* Crypto operation descriptor */
        crp->crp_ilen = m->m_pkthdr.len;        /* Total input length */
-       crp->crp_flags = CRYPTO_F_IMBUF;
+       crp->crp_flags = CRYPTO_F_IMBUF | CRYPTO_F_MPSAFE | CRYPTO_F_NOQUEUE;
        crp->crp_buf = (caddr_t)m;
        crp->crp_callback = ipsec_output_cb;
        crp->crp_opaque = (caddr_t)tc;
        crp->crp_sid = tdb->tdb_cryptoid;
 
-       KERNEL_LOCK();
        error = crypto_dispatch(crp);
-       KERNEL_UNLOCK();
        return error;
 
  drop:

diff --git a/sys/netinet/ipsec_input.c b/sys/netinet/ipsec_input.c
index ba4b92a..1271038 100644 (file)
--- a/sys/netinet/ipsec_input.c
+++ b/sys/netinet/ipsec_input.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ipsec_input.c,v 1.176 2021/07/21 12:23:32 bluhm Exp $ */
+/*     $OpenBSD: ipsec_input.c,v 1.177 2021/07/26 21:27:57 bluhm Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -380,21 +380,19 @@ ipsec_input_cb(struct cryptop *crp)
        struct tdb *tdb = NULL;
        int clen, error;
 
-       KERNEL_ASSERT_LOCKED();
+       NET_ASSERT_LOCKED();
 
        if (m == NULL) {
                DPRINTF("bogus returned buffer from crypto");
                ipsecstat_inc(ipsec_crypto);
-               goto droponly;
+               goto drop;
        }
 
-
-       NET_LOCK();
        tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto);
        if (tdb == NULL) {
                DPRINTF("TDB is expired while in crypto");
                ipsecstat_inc(ipsec_notdb);
-               goto baddone;
+               goto drop;
        }
 
        /* Check for crypto errors */
@@ -403,18 +401,16 @@ ipsec_input_cb(struct cryptop *crp)
                        /* Reset the session ID */
                        if (tdb->tdb_cryptoid != 0)
                                tdb->tdb_cryptoid = crp->crp_sid;
-                       NET_UNLOCK();
                        error = crypto_dispatch(crp);
                        if (error) {
                                DPRINTF("crypto dispatch error %d", error);
-                               ipsecstat_inc(ipsec_idrops);
-                               tdb->tdb_idrops++;
+                               goto drop;
                        }
                        return;
                }
                DPRINTF("crypto error %d", crp->crp_etype);
                ipsecstat_inc(ipsec_noxform);
-               goto baddone;
+               goto drop;
        }
 
        /* Length of data after processing */
@@ -438,16 +434,13 @@ ipsec_input_cb(struct cryptop *crp)
                    __func__, tdb->tdb_sproto);
        }
 
-       NET_UNLOCK();
        if (error) {
                ipsecstat_inc(ipsec_idrops);
                tdb->tdb_idrops++;
        }
        return;
 
- baddone:
-       NET_UNLOCK();
- droponly:
+ drop:
        ipsecstat_inc(ipsec_idrops);
        if (tdb != NULL)
                tdb->tdb_idrops++;

diff --git a/sys/netinet/ipsec_output.c b/sys/netinet/ipsec_output.c
index 9f39a53..5999770 100644 (file)
--- a/sys/netinet/ipsec_output.c
+++ b/sys/netinet/ipsec_output.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ipsec_output.c,v 1.83 2021/07/21 11:11:41 bluhm Exp $ */
+/*     $OpenBSD: ipsec_output.c,v 1.84 2021/07/26 21:27:57 bluhm Exp $ */
 /*
  * The author of this code is Angelos D. Keromytis (angelos@cis.upenn.edu)
  *
@@ -395,20 +395,19 @@ ipsec_output_cb(struct cryptop *crp)
        struct tdb *tdb = NULL;
        int error, ilen, olen;
 
-       KERNEL_ASSERT_LOCKED();
+       NET_ASSERT_LOCKED();
 
        if (m == NULL) {
                DPRINTF("bogus returned buffer from crypto");
                ipsecstat_inc(ipsec_crypto);
-               goto droponly;
+               goto drop;
        }
 
-       NET_LOCK();
        tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto);
        if (tdb == NULL) {
                DPRINTF("TDB is expired while in crypto");
                ipsecstat_inc(ipsec_notdb);
-               goto baddone;
+               goto drop;
        }
 
        /* Check for crypto errors. */
@@ -417,18 +416,16 @@ ipsec_output_cb(struct cryptop *crp)
                        /* Reset the session ID */
                        if (tdb->tdb_cryptoid != 0)
                                tdb->tdb_cryptoid = crp->crp_sid;
-                       NET_UNLOCK();
                        error = crypto_dispatch(crp);
                        if (error) {
                                DPRINTF("crypto dispatch error %d", error);
-                               ipsecstat_inc(ipsec_odrops);
-                               tdb->tdb_odrops++;
+                               goto drop;
                        }
                        return;
                }
                DPRINTF("crypto error %d", crp->crp_etype);
                ipsecstat_inc(ipsec_noxform);
-               goto baddone;
+               goto drop;
        }
 
        olen = crp->crp_olen;
@@ -452,16 +449,13 @@ ipsec_output_cb(struct cryptop *crp)
                    __func__, tdb->tdb_sproto);
        }
 
-       NET_UNLOCK();
        if (error) {
                ipsecstat_inc(ipsec_odrops);
                tdb->tdb_odrops++;
        }
        return;
 
- baddone:
-       NET_UNLOCK();
- droponly:
+ drop:
        if (tdb != NULL)
                tdb->tdb_odrops++;
        m_freem(m);
@@ -485,6 +479,8 @@ ipsp_process_done(struct mbuf *m, struct tdb *tdb)
        struct m_tag *mtag;
        int roff, error;
 
+       NET_ASSERT_LOCKED();
+
        tdb->tdb_last_used = gettime();
 
        if ((tdb->tdb_flags & TDBF_UDPENCAP) != 0) {