Source-Changes-HG archive

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

[src/trunk]: src/sys Fix race condition on ifqueue used by traditional netisr



details:   https://anonhg.NetBSD.org/src/rev/f5be3d3c064d
branches:  trunk
changeset: 348064:f5be3d3c064d
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Mon Oct 03 11:06:06 2016 +0000

description:
Fix race condition on ifqueue used by traditional netisr

If a underlying network device driver supports MSI/MSI-X, RX interrupts
can be delivered to arbitrary CPUs. This means that Layer 2 subroutines
such as ether_input (softint) and subsequent Layer 3 subroutines (softint)
which are called via traditional netisr can be dispatched on an arbitrary
CPU. Layer 2 subroutines now run without any locks (expected) and so a
Layer 2 subroutine and a Layer 3 subroutine can run in parallel.

There is a shared data between a Layer 2 routine and a Layer 3 routine,
that is ifqueue and IF_ENQUEUE (from L2) and IF_DEQUEUE (from L3) on it
are racy now.

To fix the race condition, use ifqueue#ifq_lock to protect ifqueue
instead of splnet that is meaningless now.

The same race condition exists in route_intr. Fix it as well.

Reviewed by knakahara@

diffstat:

 sys/net/if.c              |   6 +++---
 sys/net/if.h              |   7 ++++++-
 sys/net/if_arcsubr.c      |  12 ++++++------
 sys/net/if_atmsubr.c      |   9 +++++----
 sys/net/if_ecosubr.c      |  11 +++++------
 sys/net/if_ethersubr.c    |   8 ++++++--
 sys/net/if_fddisubr.c     |  10 +++++-----
 sys/net/if_ieee1394subr.c |  10 +++++-----
 sys/net/if_mpls.c         |  12 ++++++------
 sys/net/if_pppoe.c        |  20 +++++++++++++-------
 sys/net/if_spppsubr.c     |  11 +++++------
 sys/net/if_tokensubr.c    |  11 ++++++-----
 sys/net/rtsock.c          |  24 ++++++++++++++----------
 sys/netatalk/ddp_input.c  |  17 ++++++-----------
 sys/netatalk/ddp_usrreq.c |   6 ++++--
 sys/netinet/if_arp.c      |  16 ++++++++++------
 sys/netmpls/mpls_proto.c  |   5 +++--
 sys/netnatm/natm.c        |   8 ++++----
 sys/netnatm/natm_proto.c  |   5 +++--
 19 files changed, 115 insertions(+), 93 deletions(-)

diffs (truncated from 795 to 300 lines):

diff -r 068981fb937f -r f5be3d3c064d sys/net/if.c
--- a/sys/net/if.c      Mon Oct 03 08:20:12 2016 +0000
+++ b/sys/net/if.c      Mon Oct 03 11:06:06 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if.c,v 1.357 2016/08/01 03:15:30 ozaki-r Exp $ */
+/*     $NetBSD: if.c,v 1.358 2016/10/03 11:06:06 ozaki-r Exp $ */
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc.
@@ -90,7 +90,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.357 2016/08/01 03:15:30 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.358 2016/10/03 11:06:06 ozaki-r Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_inet.h"
@@ -657,7 +657,7 @@
        ifp->if_snd.altq_ifp  = ifp;
 #endif
 
-       ifp->if_snd.ifq_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NET);
+       IFQ_LOCK_INIT(&ifp->if_snd);
 
        ifp->if_pfil = pfil_head_create(PFIL_TYPE_IFNET, ifp);
        (void)pfil_run_hooks(if_pfil,
diff -r 068981fb937f -r f5be3d3c064d sys/net/if.h
--- a/sys/net/if.h      Mon Oct 03 08:20:12 2016 +0000
+++ b/sys/net/if.h      Mon Oct 03 11:06:06 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if.h,v 1.226 2016/09/21 10:50:22 roy Exp $     */
+/*     $NetBSD: if.h,v 1.227 2016/10/03 11:06:06 ozaki-r Exp $ */
 
 /*-
  * Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc.
@@ -917,6 +917,11 @@
 
 #endif /* ALTQ */
 
+#define IFQ_LOCK_INIT(ifq)     (ifq)->ifq_lock =                       \
+           mutex_obj_alloc(MUTEX_DEFAULT, IPL_NET)
+#define IFQ_LOCK(ifq)          mutex_enter((ifq)->ifq_lock)
+#define IFQ_UNLOCK(ifq)                mutex_exit((ifq)->ifq_lock)
+
 #define        IFQ_IS_EMPTY(ifq)               IF_IS_EMPTY((ifq))
 #define        IFQ_INC_LEN(ifq)                ((ifq)->ifq_len++)
 #define        IFQ_DEC_LEN(ifq)                (--(ifq)->ifq_len)
diff -r 068981fb937f -r f5be3d3c064d sys/net/if_arcsubr.c
--- a/sys/net/if_arcsubr.c      Mon Oct 03 08:20:12 2016 +0000
+++ b/sys/net/if_arcsubr.c      Mon Oct 03 11:06:06 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_arcsubr.c,v 1.73 2016/04/28 14:40:09 ozaki-r Exp $  */
+/*     $NetBSD: if_arcsubr.c,v 1.74 2016/10/03 11:06:06 ozaki-r Exp $  */
 
 /*
  * Copyright (c) 1994, 1995 Ignatios Souvatzis
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_arcsubr.c,v 1.73 2016/04/28 14:40:09 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_arcsubr.c,v 1.74 2016/10/03 11:06:06 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -505,7 +505,6 @@
        struct ifqueue *inq;
        uint8_t atype;
        int isr = 0;
-       int s;
 
        if ((ifp->if_flags & IFF_UP) == 0) {
                m_freem(m);
@@ -568,22 +567,23 @@
                return;
        }
 
-       s = splnet();
        if (__predict_true(pktq)) {
                if (__predict_false(!pktq_enqueue(pktq, m, 0))) {
                        m_freem(m);
                }
-               splx(s);
                return;
        }
+
+       IFQ_LOCK(inq);
        if (IF_QFULL(inq)) {
                IF_DROP(inq);
+               IFQ_UNLOCK(inq);
                m_freem(m);
        } else {
                IF_ENQUEUE(inq, m);
+               IFQ_UNLOCK(inq);
                schednetisr(isr);
        }
-       splx(s);
 }
 
 /*
diff -r 068981fb937f -r f5be3d3c064d sys/net/if_atmsubr.c
--- a/sys/net/if_atmsubr.c      Mon Oct 03 08:20:12 2016 +0000
+++ b/sys/net/if_atmsubr.c      Mon Oct 03 11:06:06 2016 +0000
@@ -1,4 +1,4 @@
-/*      $NetBSD: if_atmsubr.c,v 1.59 2016/06/10 13:27:15 ozaki-r Exp $       */
+/*      $NetBSD: if_atmsubr.c,v 1.60 2016/10/03 11:06:06 ozaki-r Exp $       */
 
 /*
  * Copyright (c) 1996 Charles D. Cranor and Washington University.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_atmsubr.c,v 1.59 2016/06/10 13:27:15 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_atmsubr.c,v 1.60 2016/10/03 11:06:06 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -216,15 +216,16 @@
                inq = &natmintrq;
                m_set_rcvif(m, rxhand); /* XXX: overload */
 
-               s = splnet();
+               IFQ_LOCK(inq);
                if (IF_QFULL(inq)) {
                        IF_DROP(inq);
+                       IFQ_UNLOCK(inq);
                        m_freem(m);
                } else {
                        IF_ENQUEUE(inq, m);
+                       IFQ_UNLOCK(inq);
                        schednetisr(isr);
                }
-               splx(s);
 #else
                printf("%s: NATM detected but not configured in kernel\n",
                    __func__);
diff -r 068981fb937f -r f5be3d3c064d sys/net/if_ecosubr.c
--- a/sys/net/if_ecosubr.c      Mon Oct 03 08:20:12 2016 +0000
+++ b/sys/net/if_ecosubr.c      Mon Oct 03 11:06:06 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_ecosubr.c,v 1.48 2016/06/20 08:30:58 knakahara Exp $        */
+/*     $NetBSD: if_ecosubr.c,v 1.49 2016/10/03 11:06:06 ozaki-r Exp $  */
 
 /*-
  * Copyright (c) 2001 Ben Harris
@@ -58,7 +58,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_ecosubr.c,v 1.48 2016/06/20 08:30:58 knakahara Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_ecosubr.c,v 1.49 2016/10/03 11:06:06 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -326,7 +326,6 @@
        struct ifqueue *inq;
        struct eco_header ehdr, *eh;
        int isr = 0;
-       int s;
 #ifdef INET
        int i;
        struct arphdr *ah;
@@ -449,15 +448,15 @@
                return;
        }
 
-       s = splnet();
+       IFQ_LOCK(inq);
        if (IF_QFULL(inq)) {
-               IF_DROP(inq);
+               IFQ_UNLOCK(inq);
                m_freem(m);
        } else {
                IF_ENQUEUE(inq, m);
+               IFQ_UNLOCK(inq);
                schednetisr(isr);
        }
-       splx(s);
 }
 
 static void
diff -r 068981fb937f -r f5be3d3c064d sys/net/if_ethersubr.c
--- a/sys/net/if_ethersubr.c    Mon Oct 03 08:20:12 2016 +0000
+++ b/sys/net/if_ethersubr.c    Mon Oct 03 11:06:06 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_ethersubr.c,v 1.227 2016/08/01 03:15:30 ozaki-r Exp $       */
+/*     $NetBSD: if_ethersubr.c,v 1.228 2016/10/03 11:06:06 ozaki-r Exp $       */
 
 /*
  * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@@ -61,7 +61,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.227 2016/08/01 03:15:30 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.228 2016/10/03 11:06:06 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -896,11 +896,15 @@
                m_freem(m);
                return;
        }
+
+       IFQ_LOCK(inq);
        if (IF_QFULL(inq)) {
                IF_DROP(inq);
+               IFQ_UNLOCK(inq);
                m_freem(m);
        } else {
                IF_ENQUEUE(inq, m);
+               IFQ_UNLOCK(inq);
                schednetisr(isr);
        }
 }
diff -r 068981fb937f -r f5be3d3c064d sys/net/if_fddisubr.c
--- a/sys/net/if_fddisubr.c     Mon Oct 03 08:20:12 2016 +0000
+++ b/sys/net/if_fddisubr.c     Mon Oct 03 11:06:06 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_fddisubr.c,v 1.101 2016/10/03 07:13:29 ozaki-r Exp $        */
+/*     $NetBSD: if_fddisubr.c,v 1.102 2016/10/03 11:06:06 ozaki-r Exp $        */
 
 /*
  * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@@ -96,7 +96,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_fddisubr.c,v 1.101 2016/10/03 07:13:29 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_fddisubr.c,v 1.102 2016/10/03 11:06:06 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_gateway.h"
@@ -435,7 +435,6 @@
 #if defined(NETATALK)
        struct ifqueue *inq = NULL;
        int isr = 0;
-       int s;
 #endif
 
        struct llc *l;
@@ -587,15 +586,16 @@
                m_freem(m);
                return;
        }
-       s = splnet();
+       IFQ_LOCK(inq);
        if (IF_QFULL(inq)) {
                IF_DROP(inq);
+               IFQ_UNLOCK(inq);
                m_freem(m);
        } else {
                IF_ENQUEUE(inq, m);
+               IFQ_UNLOCK(inq);
                schednetisr(isr);
        }
-       splx(s);
 #endif
 }
 
diff -r 068981fb937f -r f5be3d3c064d sys/net/if_ieee1394subr.c
--- a/sys/net/if_ieee1394subr.c Mon Oct 03 08:20:12 2016 +0000
+++ b/sys/net/if_ieee1394subr.c Mon Oct 03 11:06:06 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_ieee1394subr.c,v 1.57 2016/08/15 09:14:12 maxv Exp $        */
+/*     $NetBSD: if_ieee1394subr.c,v 1.58 2016/10/03 11:06:06 ozaki-r Exp $     */
 
 /*
  * Copyright (c) 2000 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_ieee1394subr.c,v 1.57 2016/08/15 09:14:12 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_ieee1394subr.c,v 1.58 2016/10/03 11:06:06 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -318,7 +318,6 @@
        pktqueue_t *pktq = NULL;
        struct ifqueue *inq;
        uint16_t etype;
-       int s;
        struct ieee1394_unfraghdr *iuh;
        int isr = 0;
 
@@ -395,15 +394,16 @@
                return;
        }
 
-       s = splnet();
+       IFQ_LOCK(inq);
        if (IF_QFULL(inq)) {
                IF_DROP(inq);
+               IFQ_UNLOCK(inq);
                m_freem(m);
        } else {
                IF_ENQUEUE(inq, m);



Home | Main Index | Thread Index | Old Index