Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/netipsec Avoid data races on lifetime counters by using ...



details:   https://anonhg.NetBSD.org/src/rev/4e860e0dbd0e
branches:  trunk
changeset: 830242:4e860e0dbd0e
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Fri Mar 02 07:37:13 2018 +0000

description:
Avoid data races on lifetime counters by using percpu(9)

We don't make them percpu(9) directly because the structure is exposed to
userland and we don't want to break ABI.  So we add another member variable
for percpu(9) and use it internally.  When we export them to userland, they
are converted to the original format.

diffstat:

 sys/netipsec/key.c   |  88 ++++++++++++++++++++++++++++++++++++++++++++-------
 sys/netipsec/keydb.h |   6 ++-
 2 files changed, 80 insertions(+), 14 deletions(-)

diffs (226 lines):

diff -r 407608673ee7 -r 4e860e0dbd0e sys/netipsec/key.c
--- a/sys/netipsec/key.c        Fri Mar 02 06:41:18 2018 +0000
+++ b/sys/netipsec/key.c        Fri Mar 02 07:37:13 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: key.c,v 1.248 2018/02/08 20:57:41 maxv Exp $   */
+/*     $NetBSD: key.c,v 1.249 2018/03/02 07:37:13 ozaki-r Exp $        */
 /*     $FreeBSD: src/sys/netipsec/key.c,v 1.3.2.3 2004/02/14 22:23:23 bms Exp $        */
 /*     $KAME: key.c,v 1.191 2001/06/27 10:46:49 sakane Exp $   */
 
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.248 2018/02/08 20:57:41 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.249 2018/03/02 07:37:13 ozaki-r Exp $");
 
 /*
  * This code is referred to RFC 2367
@@ -785,6 +785,26 @@
 static struct workqueue        *key_timehandler_wq;
 static struct work     key_timehandler_wk;
 
+/*
+ * Utilities for percpu counters for sadb_lifetime_allocations and
+ * sadb_lifetime_bytes.
+ */
+#define LIFETIME_COUNTER_ALLOCATIONS   0
+#define LIFETIME_COUNTER_BYTES         1
+#define LIFETIME_COUNTER_SIZE          2
+
+typedef uint64_t lifetime_counters_t[LIFETIME_COUNTER_SIZE];
+
+static void
+key_sum_lifetime_counters(void *p, void *arg, struct cpu_info *ci __unused)
+{
+       lifetime_counters_t *one = p;
+       lifetime_counters_t *sum = arg;
+
+       (*sum)[LIFETIME_COUNTER_ALLOCATIONS] += (*one)[LIFETIME_COUNTER_ALLOCATIONS];
+       (*sum)[LIFETIME_COUNTER_BYTES] += (*one)[LIFETIME_COUNTER_BYTES];
+}
+
 u_int
 key_sp_refcnt(const struct secpolicy *sp)
 {
@@ -3257,6 +3277,8 @@
                /* We don't allow lft_c to be NULL */
                newsav->lft_c = kmem_zalloc(sizeof(struct sadb_lifetime),
                    KM_SLEEP);
+               newsav->lft_c_counters_percpu =
+                   percpu_alloc(sizeof(lifetime_counters_t));
        }
 
        /* reset created */
@@ -3467,6 +3489,10 @@
                kmem_intr_free(sav->key_auth, sav->key_auth_len);
        if (sav->key_enc != NULL)
                kmem_intr_free(sav->key_enc, sav->key_enc_len);
+       if (sav->lft_c_counters_percpu != NULL) {
+               percpu_free(sav->lft_c_counters_percpu,
+                   sizeof(lifetime_counters_t));
+       }
        if (sav->lft_c != NULL)
                kmem_intr_free(sav->lft_c, sizeof(*(sav->lft_c)));
        if (sav->lft_h != NULL)
@@ -3635,6 +3661,8 @@
        sav->lft_c->sadb_lifetime_addtime = time_uptime;
        sav->lft_c->sadb_lifetime_usetime = 0;
 
+       sav->lft_c_counters_percpu = percpu_alloc(sizeof(lifetime_counters_t));
+
        /* lifetimes for HARD and SOFT */
     {
        const struct sadb_lifetime *lft0;
@@ -3818,7 +3846,9 @@
                        p = sav->key_enc;
                        break;
 
-               case SADB_EXT_LIFETIME_CURRENT:
+               case SADB_EXT_LIFETIME_CURRENT: {
+                       lifetime_counters_t sum = {0};
+
                        KASSERT(sav->lft_c != NULL);
                        l = PFKEY_UNUNIT64(((struct sadb_ext *)sav->lft_c)->sadb_ext_len);
                        memcpy(&lt, sav->lft_c, sizeof(struct sadb_lifetime));
@@ -3826,8 +3856,15 @@
                            time_mono_to_wall(lt.sadb_lifetime_addtime);
                        lt.sadb_lifetime_usetime =
                            time_mono_to_wall(lt.sadb_lifetime_usetime);
+                       percpu_foreach(sav->lft_c_counters_percpu,
+                           key_sum_lifetime_counters, sum);
+                       lt.sadb_lifetime_allocations =
+                           sum[LIFETIME_COUNTER_ALLOCATIONS];
+                       lt.sadb_lifetime_bytes =
+                           sum[LIFETIME_COUNTER_BYTES];
                        p = &lt;
                        break;
+                   }
 
                case SADB_EXT_LIFETIME_HARD:
                        if (!sav->lft_h)
@@ -4857,9 +4894,17 @@
                         * when new SA is installed.  Caution when it's
                         * installed too big lifetime by time.
                         */
-                       else if (sav->lft_s->sadb_lifetime_bytes != 0 &&
-                                sav->lft_s->sadb_lifetime_bytes <
-                                sav->lft_c->sadb_lifetime_bytes) {
+                       else {
+                               uint64_t lft_c_bytes = 0;
+                               lifetime_counters_t sum = {0};
+
+                               percpu_foreach(sav->lft_c_counters_percpu,
+                                   key_sum_lifetime_counters, sum);
+                               lft_c_bytes = sum[LIFETIME_COUNTER_BYTES];
+
+                               if (sav->lft_s->sadb_lifetime_bytes == 0 ||
+                                   sav->lft_s->sadb_lifetime_bytes >= lft_c_bytes)
+                                       continue;
 
                                key_sa_chgstate(sav, SADB_SASTATE_DYING);
                                mutex_exit(&key_sad.lock);
@@ -4907,9 +4952,18 @@
                        }
 #endif
                        /* check HARD lifetime by bytes */
-                       else if (sav->lft_h->sadb_lifetime_bytes != 0 &&
-                                sav->lft_h->sadb_lifetime_bytes <
-                                sav->lft_c->sadb_lifetime_bytes) {
+                       else {
+                               uint64_t lft_c_bytes = 0;
+                               lifetime_counters_t sum = {0};
+
+                               percpu_foreach(sav->lft_c_counters_percpu,
+                                   key_sum_lifetime_counters, sum);
+                               lft_c_bytes = sum[LIFETIME_COUNTER_BYTES];
+
+                               if (sav->lft_h->sadb_lifetime_bytes == 0 ||
+                                   sav->lft_h->sadb_lifetime_bytes >= lft_c_bytes)
+                                       continue;
+
                                key_sa_chgstate(sav, SADB_SASTATE_DEAD);
                                goto restart_sav_DYING;
                        }
@@ -7178,6 +7232,7 @@
        int len;
        int error = -1;
        struct sadb_lifetime *lt;
+       lifetime_counters_t sum = {0};
 
        /* XXX: Why do we lock ? */
        s = splsoftnet();       /*called from softclock()*/
@@ -7210,8 +7265,10 @@
        lt = mtod(m, struct sadb_lifetime *);
        lt->sadb_lifetime_len = PFKEY_UNIT64(sizeof(struct sadb_lifetime));
        lt->sadb_lifetime_exttype = SADB_EXT_LIFETIME_CURRENT;
-       lt->sadb_lifetime_allocations = sav->lft_c->sadb_lifetime_allocations;
-       lt->sadb_lifetime_bytes = sav->lft_c->sadb_lifetime_bytes;
+       percpu_foreach(sav->lft_c_counters_percpu,
+           key_sum_lifetime_counters, sum);
+       lt->sadb_lifetime_allocations = sum[LIFETIME_COUNTER_ALLOCATIONS];
+       lt->sadb_lifetime_bytes = sum[LIFETIME_COUNTER_BYTES];
        lt->sadb_lifetime_addtime =
            time_mono_to_wall(sav->lft_c->sadb_lifetime_addtime);
        lt->sadb_lifetime_usetime =
@@ -8171,16 +8228,19 @@
 void
 key_sa_recordxfer(struct secasvar *sav, struct mbuf *m)
 {
+       lifetime_counters_t *counters;
 
        KASSERT(sav != NULL);
        KASSERT(sav->lft_c != NULL);
        KASSERT(m != NULL);
 
+       counters = percpu_getref(sav->lft_c_counters_percpu);
+
        /*
         * XXX Currently, there is a difference of bytes size
         * between inbound and outbound processing.
         */
-       sav->lft_c->sadb_lifetime_bytes += m->m_pkthdr.len;
+       (*counters)[LIFETIME_COUNTER_BYTES] += m->m_pkthdr.len;
        /* to check bytes lifetime is done in key_timehandler(). */
 
        /*
@@ -8188,9 +8248,11 @@
         * sadb_lifetime_allocations.  We increment the variable
         * whenever {esp,ah}_{in,out}put is called.
         */
-       sav->lft_c->sadb_lifetime_allocations++;
+       (*counters)[LIFETIME_COUNTER_ALLOCATIONS]++;
        /* XXX check for expires? */
 
+       percpu_putref(sav->lft_c_counters_percpu);
+
        /*
         * NOTE: We record CURRENT sadb_lifetime_usetime by using wall clock,
         * in seconds.  HARD and SOFT lifetime are measured by the time
diff -r 407608673ee7 -r 4e860e0dbd0e sys/netipsec/keydb.h
--- a/sys/netipsec/keydb.h      Fri Mar 02 06:41:18 2018 +0000
+++ b/sys/netipsec/keydb.h      Fri Mar 02 07:37:13 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: keydb.h,v 1.20 2017/08/09 09:48:11 ozaki-r Exp $       */
+/*     $NetBSD: keydb.h,v 1.21 2018/03/02 07:37:13 ozaki-r Exp $       */
 /*     $FreeBSD: src/sys/netipsec/keydb.h,v 1.1.4.1 2003/01/24 05:11:36 sam Exp $      */
 /*     $KAME: keydb.h,v 1.14 2000/08/02 17:58:26 sakane Exp $  */
 
@@ -37,6 +37,7 @@
 #ifdef _KERNEL
 
 #include <sys/localcount.h>
+#include <sys/percpu.h>
 
 #include <netipsec/key_var.h>
 #include <net/route.h>
@@ -118,6 +119,9 @@
        struct sadb_lifetime *lft_h;    /* HARD lifetime */
        struct sadb_lifetime *lft_s;    /* SOFT lifetime */
 
+       /* percpu counters for lft_c->sadb_lifetime_{allocations,bytes} */
+       percpu_t *lft_c_counters_percpu;
+
        u_int32_t seq;                  /* sequence number */
        pid_t pid;                      /* message's pid */
 



Home | Main Index | Thread Index | Old Index