[src/trunk]: src/sys/net l2tp(4): avoid having struct ifqueue directly in a p...

[knakahara <knakahara%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/bf9daf43e0bc
branches:  trunk
changeset: 965494:bf9daf43e0bc
user:      knakahara <knakahara%NetBSD.org@localhost>
date:      Thu Sep 19 06:09:52 2019 +0000

description:
l2tp(4): avoid having struct ifqueue directly in a percpu storage.

percpu(9) has a certain memory storage for each CPU and provides it by the piece
to users.  If the storages went short, percpu(9) enlarges them by allocating new
larger memory areas, replacing old ones with them and destroying the old ones.
A percpu storage referenced by a pointer gotten via percpu_getref can be
destroyed by the mechanism after a running thread sleeps even if percpu_putref
has not been called.

Tx processing of l2tp(4) uses normally involves sleepable operations so we
must avoid dereferencing a percpu data (struct ifqueue) after executing Tx
processing.  Address this situation by having just a pointer to the data in
a percpu storage instead.

Reviewed by ozaki-r@ and yamaguchi@

diffstat:

 sys/net/if_l2tp.c |  50 +++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 37 insertions(+), 13 deletions(-)

diffs (130 lines):

diff -r 8ede8734fd3b -r bf9daf43e0bc sys/net/if_l2tp.c
--- a/sys/net/if_l2tp.c Thu Sep 19 06:07:24 2019 +0000
+++ b/sys/net/if_l2tp.c Thu Sep 19 06:09:52 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_l2tp.c,v 1.38 2019/09/19 06:07:24 knakahara Exp $   */
+/*     $NetBSD: if_l2tp.c,v 1.39 2019/09/19 06:09:52 knakahara Exp $   */
 
 /*
  * Copyright (c) 2017 Internet Initiative Japan Inc.
@@ -31,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_l2tp.c,v 1.38 2019/09/19 06:07:24 knakahara Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_l2tp.c,v 1.39 2019/09/19 06:09:52 knakahara Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -116,6 +116,7 @@
 struct psref_class *lv_psref_class __read_mostly;
 
 static void    l2tp_ifq_init_pc(void *, void *, struct cpu_info *);
+static void    l2tp_ifq_fini_pc(void *, void *, struct cpu_info *);
 
 static int     l2tp_clone_create(struct if_clone *, int);
 static int     l2tp_clone_destroy(struct ifnet *);
@@ -151,6 +152,20 @@
 static int     l2tp_encap_attach(struct l2tp_variant *);
 static int     l2tp_encap_detach(struct l2tp_variant *);
 
+static inline struct ifqueue *
+l2tp_ifq_percpu_getref(percpu_t *pc)
+{
+
+       return *(struct ifqueue **)percpu_getref(pc);
+}
+
+static inline void
+l2tp_ifq_percpu_putref(percpu_t *pc)
+{
+
+       percpu_putref(pc);
+}
+
 #ifndef MAX_L2TP_NEST
 /*
  * This macro controls the upper limitation on nesting of l2tp tunnels.
@@ -252,7 +267,7 @@
 
        sc->l2tp_ro_percpu = if_tunnel_alloc_ro_percpu();
 
-       sc->l2tp_ifq_percpu = percpu_alloc(sizeof(struct ifqueue));
+       sc->l2tp_ifq_percpu = percpu_alloc(sizeof(struct ifqueue *));
        percpu_foreach(sc->l2tp_ifq_percpu, l2tp_ifq_init_pc, NULL);
        sc->l2tp_si = softint_establish(si_flags, l2tpintr_softint, sc);
 
@@ -319,10 +334,18 @@
 void
 l2tp_ifq_init_pc(void *p, void *arg __unused, struct cpu_info *ci __unused)
 {
-       struct ifqueue *ifq = p;
+       struct ifqueue **ifqp = p;
+
+       *ifqp = kmem_zalloc(sizeof(**ifqp), KM_SLEEP);
+       (*ifqp)->ifq_maxlen = IFQ_MAXLEN;
+}
 
-       memset(ifq, 0, sizeof(*ifq));
-       ifq->ifq_maxlen = IFQ_MAXLEN;
+void
+l2tp_ifq_fini_pc(void *p, void *arg __unused, struct cpu_info *ci __unused)
+{
+       struct ifqueue **ifqp = p;
+
+       kmem_free(*ifqp, sizeof(**ifqp));
 }
 
 static int
@@ -344,7 +367,8 @@
        mutex_exit(&sc->l2tp_lock);
 
        softint_disestablish(sc->l2tp_si);
-       percpu_free(sc->l2tp_ifq_percpu, sizeof(struct ifqueue));
+       percpu_foreach(sc->l2tp_ifq_percpu, l2tp_ifq_fini_pc, NULL);
+       percpu_free(sc->l2tp_ifq_percpu, sizeof(struct ifqueue *));
 
        mutex_enter(&l2tp_softcs.lock);
        LIST_REMOVE(sc, l2tp_list);
@@ -378,10 +402,10 @@
        ifp = &sc->l2tp_ec.ec_if;
 
        s = splsoftnet();
-       ifq = percpu_getref(sc->l2tp_ifq_percpu);
+       ifq = l2tp_ifq_percpu_getref(sc->l2tp_ifq_percpu);
        if (IF_QFULL(ifq)) {
                ifp->if_oerrors++;
-               percpu_putref(sc->l2tp_ifq_percpu);
+               l2tp_ifq_percpu_putref(sc->l2tp_ifq_percpu);
                splx(s);
                m_freem(m);
                return ENOBUFS;
@@ -503,16 +527,16 @@
 
        /* output processing */
        if (var->lv_my_sess_id == 0 || var->lv_peer_sess_id == 0) {
-               ifq = percpu_getref(sc->l2tp_ifq_percpu);
+               ifq = l2tp_ifq_percpu_getref(sc->l2tp_ifq_percpu);
                IF_PURGE(ifq);
-               percpu_putref(sc->l2tp_ifq_percpu);
+               l2tp_ifq_percpu_putref(sc->l2tp_ifq_percpu);
                if (cpuid == 0)
                        IFQ_PURGE(&ifp->if_snd);
                return;
        }
 
        /* Currently, l2tpintr() is always called in softint context. */
-       ifq = percpu_getref(sc->l2tp_ifq_percpu);
+       ifq = l2tp_ifq_percpu_getref(sc->l2tp_ifq_percpu);
        for (;;) {
                IF_DEQUEUE(ifq, m);
                if (m != NULL)
@@ -520,7 +544,7 @@
                else
                        break;
        }
-       percpu_putref(sc->l2tp_ifq_percpu);
+       l2tp_ifq_percpu_putref(sc->l2tp_ifq_percpu);
 
        if (cpuid == 0) {
                for (;;) {