Module Name:    src
Committed By:   knakahara
Date:           Wed Jun 22 10:44:32 UTC 2016

Modified Files:
        src/sys/dev/usb: if_upl.c
        src/sys/net: if.c if.h if_bridge.c if_ieee1394subr.c if_loop.c
            if_vlan.c
        src/sys/netipsec: ipsec_osdep.h

Log Message:
fix: locking about IFQ_ENQUEUE and ALTQ

- If NET_MPSAFE is not defined, IFQ_LOCK is nop. Currently, that means
  IFQ_ENQUEUE() of some paths such as bridge_enqueue() is called parallel
  wrongly.
- If ALTQ is enabled, Tx processing should call if_transmit() (= IFQ_ENQUEUE
  + ifp->if_start()) instead of ifp->if_transmit() to call ALTQ_ENQUEUE()
  and ALTQ_DEQUEUE().
  Furthermore, ALTQ processing is always required KERNEL_LOCK currently.


To generate a diff of this commit:
cvs rdiff -u -r1.54 -r1.55 src/sys/dev/usb/if_upl.c
cvs rdiff -u -r1.344 -r1.345 src/sys/net/if.c
cvs rdiff -u -r1.214 -r1.215 src/sys/net/if.h
cvs rdiff -u -r1.128 -r1.129 src/sys/net/if_bridge.c
cvs rdiff -u -r1.55 -r1.56 src/sys/net/if_ieee1394subr.c
cvs rdiff -u -r1.88 -r1.89 src/sys/net/if_loop.c
cvs rdiff -u -r1.89 -r1.90 src/sys/net/if_vlan.c
cvs rdiff -u -r1.25 -r1.26 src/sys/netipsec/ipsec_osdep.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/usb/if_upl.c
diff -u src/sys/dev/usb/if_upl.c:1.54 src/sys/dev/usb/if_upl.c:1.55
--- src/sys/dev/usb/if_upl.c:1.54	Fri Jun 10 13:27:15 2016
+++ src/sys/dev/usb/if_upl.c	Wed Jun 22 10:44:31 2016
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_upl.c,v 1.54 2016/06/10 13:27:15 ozaki-r Exp $	*/
+/*	$NetBSD: if_upl.c,v 1.55 2016/06/22 10:44:31 knakahara Exp $	*/
 /*
  * Copyright (c) 2000 The NetBSD Foundation, Inc.
  * All rights reserved.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_upl.c,v 1.54 2016/06/10 13:27:15 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_upl.c,v 1.55 2016/06/22 10:44:31 knakahara Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -1003,7 +1003,7 @@ upl_output(struct ifnet *ifp, struct mbu
 	 * Queue message on interface, and start output if interface
 	 * not yet active.
 	 */
-	error = (*ifp->if_transmit)(ifp, m);
+	error = if_transmit_lock(ifp, m);
 
 	return error;
 }

Index: src/sys/net/if.c
diff -u src/sys/net/if.c:1.344 src/sys/net/if.c:1.345
--- src/sys/net/if.c:1.344	Tue Jun 21 10:25:27 2016
+++ src/sys/net/if.c	Wed Jun 22 10:44:32 2016
@@ -1,4 +1,4 @@
-/*	$NetBSD: if.c,v 1.344 2016/06/21 10:25:27 ozaki-r Exp $	*/
+/*	$NetBSD: if.c,v 1.345 2016/06/22 10:44:32 knakahara 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.344 2016/06/21 10:25:27 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.345 2016/06/22 10:44:32 knakahara Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_inet.h"
@@ -199,6 +199,7 @@ static void if_slowtimo(void *);
 static void if_free_sadl(struct ifnet *);
 static void if_attachdomain1(struct ifnet *);
 static int ifconf(u_long, void *);
+static int if_transmit(struct ifnet *, struct mbuf *);
 static int if_clone_create(const char *);
 static int if_clone_destroy(const char *);
 static void if_link_state_change_si(void *);
@@ -2773,14 +2774,24 @@ ifreq_setaddr(u_long cmd, struct ifreq *
 /*
  * wrapper function for the drivers which doesn't have if_transmit().
  */
-int
+static int
 if_transmit(struct ifnet *ifp, struct mbuf *m)
 {
 	int s, error;
 
 	s = splnet();
 
+	/*
+	 * If NET_MPSAFE is not defined , IFQ_LOCK() is nop.
+	 * use KERNEL_LOCK instead of ifq_lock.
+	 */
+#ifndef NET_MPSAFE
+	KERNEL_LOCK(1, NULL);
+#endif
 	IFQ_ENQUEUE(&ifp->if_snd, m, error);
+#ifndef NET_MPSAFE
+	KERNEL_UNLOCK_ONE(NULL);
+#endif
 	if (error != 0) {
 		/* mbuf is already freed */
 		goto out;
@@ -2798,6 +2809,27 @@ out:
 	return error;
 }
 
+int
+if_transmit_lock(struct ifnet *ifp, struct mbuf *m)
+{
+	int error;
+
+#ifdef ALTQ
+	KERNEL_LOCK(1, NULL);
+	if (ALTQ_IS_ENABLED(&ifp->if_snd)) {
+		error = if_transmit(ifp, m);
+		KERNEL_UNLOCK_ONE(NULL);
+	} else {
+		KERNEL_UNLOCK_ONE(NULL);
+		error = (*ifp->if_transmit)(ifp, m);
+	}
+#else /* !ALTQ */
+	error = (*ifp->if_transmit)(ifp, m);
+#endif /* !ALTQ */
+
+	return error;
+}
+
 /*
  * Queue message on interface, and start output if interface
  * not yet active.
@@ -2806,7 +2838,7 @@ int
 ifq_enqueue(struct ifnet *ifp, struct mbuf *m)
 {
 
-	return (*ifp->if_transmit)(ifp, m);
+	return if_transmit_lock(ifp, m);
 }
 
 /*

Index: src/sys/net/if.h
diff -u src/sys/net/if.h:1.214 src/sys/net/if.h:1.215
--- src/sys/net/if.h:1.214	Tue Jun 21 10:25:27 2016
+++ src/sys/net/if.h	Wed Jun 22 10:44:32 2016
@@ -1,4 +1,4 @@
-/*	$NetBSD: if.h,v 1.214 2016/06/21 10:25:27 ozaki-r Exp $	*/
+/*	$NetBSD: if.h,v 1.215 2016/06/22 10:44:32 knakahara Exp $	*/
 
 /*-
  * Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc.
@@ -992,7 +992,7 @@ void	p2p_rtrequest(int, struct rtentry *
 void	if_clone_attach(struct if_clone *);
 void	if_clone_detach(struct if_clone *);
 
-int	if_transmit(struct ifnet *, struct mbuf *);
+int	if_transmit_lock(struct ifnet *, struct mbuf *);
 
 int	ifq_enqueue(struct ifnet *, struct mbuf *);
 int	ifq_enqueue2(struct ifnet *, struct ifqueue *, struct mbuf *);

Index: src/sys/net/if_bridge.c
diff -u src/sys/net/if_bridge.c:1.128 src/sys/net/if_bridge.c:1.129
--- src/sys/net/if_bridge.c:1.128	Mon Jun 20 08:14:41 2016
+++ src/sys/net/if_bridge.c	Wed Jun 22 10:44:32 2016
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_bridge.c,v 1.128 2016/06/20 08:14:41 knakahara Exp $	*/
+/*	$NetBSD: if_bridge.c,v 1.129 2016/06/22 10:44:32 knakahara Exp $	*/
 
 /*
  * Copyright 2001 Wasabi Systems, Inc.
@@ -80,7 +80,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_bridge.c,v 1.128 2016/06/20 08:14:41 knakahara Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_bridge.c,v 1.129 2016/06/22 10:44:32 knakahara Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_bridge_ipf.h"
@@ -1396,7 +1396,7 @@ bridge_enqueue(struct bridge_softc *sc, 
 	len = m->m_pkthdr.len;
 	mflags = m->m_flags;
 
-	error = (*dst_ifp->if_transmit)(dst_ifp, m);
+	error = if_transmit_lock(dst_ifp, m);
 	if (error) {
 		/* mbuf is already freed */
 		sc->sc_if.if_oerrors++;

Index: src/sys/net/if_ieee1394subr.c
diff -u src/sys/net/if_ieee1394subr.c:1.55 src/sys/net/if_ieee1394subr.c:1.56
--- src/sys/net/if_ieee1394subr.c:1.55	Thu Apr 28 01:37:17 2016
+++ src/sys/net/if_ieee1394subr.c	Wed Jun 22 10:44:32 2016
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_ieee1394subr.c,v 1.55 2016/04/28 01:37:17 knakahara Exp $	*/
+/*	$NetBSD: if_ieee1394subr.c,v 1.56 2016/06/22 10:44:32 knakahara 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.55 2016/04/28 01:37:17 knakahara Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_ieee1394subr.c,v 1.56 2016/06/22 10:44:32 knakahara Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -221,7 +221,7 @@ ieee1394_output(struct ifnet *ifp, struc
 	while ((m = m0) != NULL) {
 		m0 = m->m_nextpkt;
 
-		error = (*ifp->if_transmit)(ifp, m);
+		error = if_transmit_lock(ifp, m);
 		if (error) {
 			/* mbuf is already freed */
 			goto bad;

Index: src/sys/net/if_loop.c
diff -u src/sys/net/if_loop.c:1.88 src/sys/net/if_loop.c:1.89
--- src/sys/net/if_loop.c:1.88	Mon Jun 20 06:52:44 2016
+++ src/sys/net/if_loop.c	Wed Jun 22 10:44:32 2016
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_loop.c,v 1.88 2016/06/20 06:52:44 knakahara Exp $	*/
+/*	$NetBSD: if_loop.c,v 1.89 2016/06/22 10:44:32 knakahara Exp $	*/
 
 /*
  * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@@ -65,7 +65,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_loop.c,v 1.88 2016/06/20 06:52:44 knakahara Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_loop.c,v 1.89 2016/06/22 10:44:32 knakahara Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -256,7 +256,7 @@ looutput(struct ifnet *ifp, struct mbuf 
 		}
 		*(mtod(m, uint32_t *)) = dst->sa_family;
 
-		error = ifp->if_transmit(ifp, m);
+		error = if_transmit_lock(ifp, m);
 		goto out;
 	}
 #endif /* ALTQ */

Index: src/sys/net/if_vlan.c
diff -u src/sys/net/if_vlan.c:1.89 src/sys/net/if_vlan.c:1.90
--- src/sys/net/if_vlan.c:1.89	Fri Jun 10 13:27:16 2016
+++ src/sys/net/if_vlan.c	Wed Jun 22 10:44:32 2016
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_vlan.c,v 1.89 2016/06/10 13:27:16 ozaki-r Exp $	*/
+/*	$NetBSD: if_vlan.c,v 1.90 2016/06/22 10:44:32 knakahara Exp $	*/
 
 /*-
  * Copyright (c) 2000, 2001 The NetBSD Foundation, Inc.
@@ -78,7 +78,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_vlan.c,v 1.89 2016/06/10 13:27:16 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_vlan.c,v 1.90 2016/06/22 10:44:32 knakahara Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -700,6 +700,10 @@ vlan_start(struct ifnet *ifp)
 
 #ifdef ALTQ
 		/*
+		 * KERNEL_LOCK is required for ALTQ even if NET_MPSAFE if defined.
+		 */
+		KERNEL_LOCK(1, NULL);
+		/*
 		 * If ALTQ is enabled on the parent interface, do
 		 * classification; the queueing discipline might
 		 * not require classification, but might require
@@ -716,6 +720,7 @@ vlan_start(struct ifnet *ifp)
 #endif
 			}
 		}
+		KERNEL_UNLOCK_ONE(NULL);
 #endif /* ALTQ */
 
 		bpf_mtap(ifp, m);
@@ -808,7 +813,7 @@ vlan_start(struct ifnet *ifp)
 		 * would have.  We are already running at splnet.
 		 */
 		if ((p->if_flags & IFF_RUNNING) != 0) {
-			error = (*p->if_transmit)(p, m);
+			error = if_transmit_lock(p, m);
 			if (error) {
 				/* mbuf is already freed */
 				ifp->if_oerrors++;

Index: src/sys/netipsec/ipsec_osdep.h
diff -u src/sys/netipsec/ipsec_osdep.h:1.25 src/sys/netipsec/ipsec_osdep.h:1.26
--- src/sys/netipsec/ipsec_osdep.h:1.25	Thu Apr 28 01:37:17 2016
+++ src/sys/netipsec/ipsec_osdep.h	Wed Jun 22 10:44:32 2016
@@ -1,4 +1,4 @@
-/*	$NetBSD: ipsec_osdep.h,v 1.25 2016/04/28 01:37:17 knakahara Exp $	*/
+/*	$NetBSD: ipsec_osdep.h,v 1.26 2016/06/22 10:44:32 knakahara Exp $	*/
 /*	$FreeBSD: /repoman/r/ncvs/src/sys/netipsec/ipsec_osdep.h,v 1.1 2003/09/29 22:47:45 sam Exp $	*/
 
 /*
@@ -152,7 +152,7 @@ if_handoff(struct ifqueue *ifq, struct m
 		return (0);
 	}
 	if (ifp != NULL)
-		(void)(*ifp->if_transmit)(ifp, m);
+		(void)if_transmit_lock(ifp, m);
 
 	KERNEL_UNLOCK_ONE(NULL);
 	splx(s);

Reply via email to