Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 17 Sep 2010 08:54:56 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        freebsd-current@freebsd.org
Cc:        Marcin Cieslak <saper@saper.info>
Subject:   Re: tun(4) in -CURRENT: No buffer space available - race condition patch
Message-ID:  <201009170854.56516.jhb@freebsd.org>
In-Reply-To: <slrni945av.1rou.saper@saper.info>
References:  <slrnht8djj.1tsh.saper@saper.info> <201009151749.45038.jhb@freebsd.org> <slrni945av.1rou.saper@saper.info>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday, September 16, 2010 9:02:23 am Marcin Cieslak wrote:
> Dnia 15.09.2010 John Baldwin <jhb@freebsd.org> napisa=C5=82/a:
> > On Monday, September 13, 2010 9:10:01 pm Marcin Cieslak wrote:
> >> Output queue of tun(4) gets full after some time when sending lots of =
data.
> >> I have been observing this on -CURRENT at least since March this year.
> >>=20
> >> Looks like it's a race condition (same in tun(4) and tap(4)),=20
> >> the following patch seems to address the issue:
> >
> > This is a good find.  I actually went through these drivers a bit furth=
er and=20
> > have a bit of a larger patch to extend the locking some.  Would you car=
e to=20
> > test it?
>=20
> Do you think those drivers could be taken out of Giant after this change?
> I think that networking code path (via if_start etc.) is not using Giant
> at all, only cdevsw routines are. Am I right?

Oh, yes.  I've updated the patch to remove D_NEEDGIANT.

Index: if_tap.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
=2D-- if_tap.c	(revision 212732)
+++ if_tap.c	(working copy)
@@ -132,7 +132,7 @@
=20
 static struct cdevsw	tap_cdevsw =3D {
 	.d_version =3D	D_VERSION,
=2D	.d_flags =3D	D_PSEUDO | D_NEEDGIANT | D_NEEDMINOR,
+	.d_flags =3D	D_PSEUDO | D_NEEDMINOR,
 	.d_open =3D	tapopen,
 	.d_close =3D	tapclose,
 	.d_read =3D	tapread,
@@ -209,7 +209,6 @@
 tap_destroy(struct tap_softc *tp)
 {
 	struct ifnet *ifp =3D tp->tap_ifp;
=2D	int s;
=20
 	/* Unlocked read. */
 	KASSERT(!(tp->tap_flags & TAP_OPEN),
@@ -217,10 +216,8 @@
=20
 	knlist_destroy(&tp->tap_rsel.si_note);
 	destroy_dev(tp->tap_dev);
=2D	s =3D splimp();
 	ether_ifdetach(ifp);
 	if_free_type(ifp, IFT_ETHER);
=2D	splx(s);
=20
 	mtx_destroy(&tp->tap_mtx);
 	free(tp, M_TAP);
@@ -398,7 +395,7 @@
 	struct tap_softc	*tp =3D NULL;
 	unsigned short		 macaddr_hi;
 	uint32_t		 macaddr_mid;
=2D	int			 unit, s;
+	int			 unit;
 	char			*name =3D NULL;
 	u_char			eaddr[6];
=20
@@ -442,22 +439,20 @@
 	ifp->if_ioctl =3D tapifioctl;
 	ifp->if_mtu =3D ETHERMTU;
 	ifp->if_flags =3D (IFF_BROADCAST|IFF_SIMPLEX|IFF_MULTICAST);
=2D	ifp->if_snd.ifq_maxlen =3D ifqmaxlen;
+	IFQ_SET_MAXLEN(&ifp->if_snd, ifqmaxlen);
 	ifp->if_capabilities |=3D IFCAP_LINKSTATE;
 	ifp->if_capenable |=3D IFCAP_LINKSTATE;
=20
 	dev->si_drv1 =3D tp;
 	tp->tap_dev =3D dev;
=20
=2D	s =3D splimp();
 	ether_ifattach(ifp, eaddr);
=2D	splx(s);
=20
 	mtx_lock(&tp->tap_mtx);
 	tp->tap_flags |=3D TAP_INITED;
 	mtx_unlock(&tp->tap_mtx);
=20
=2D	knlist_init_mtx(&tp->tap_rsel.si_note, NULL);
+	knlist_init_mtx(&tp->tap_rsel.si_note, &tp->tap_mtx);
=20
 	TAPDEBUG("interface %s is created. minor =3D %#x\n",=20
 		ifp->if_xname, dev2unit(dev));
@@ -474,7 +469,7 @@
 {
 	struct tap_softc	*tp =3D NULL;
 	struct ifnet		*ifp =3D NULL;
=2D	int			 error, s;
+	int			 error;
=20
 	if (tapuopen =3D=3D 0) {
 		error =3D priv_check(td, PRIV_NET_TAP);
@@ -497,15 +492,13 @@
 	tp->tap_pid =3D td->td_proc->p_pid;
 	tp->tap_flags |=3D TAP_OPEN;
 	ifp =3D tp->tap_ifp;
=2D	mtx_unlock(&tp->tap_mtx);
=20
=2D	s =3D splimp();
 	ifp->if_drv_flags |=3D IFF_DRV_RUNNING;
 	ifp->if_drv_flags &=3D ~IFF_DRV_OACTIVE;
 	if (tapuponopen)
 		ifp->if_flags |=3D IFF_UP;
 	if_link_state_change(ifp, LINK_STATE_UP);
=2D	splx(s);
+	mtx_unlock(&tp->tap_mtx);
=20
 	TAPDEBUG("%s is open. minor =3D %#x\n", ifp->if_xname, dev2unit(dev));
=20
@@ -524,9 +517,9 @@
 	struct ifaddr		*ifa;
 	struct tap_softc	*tp =3D dev->si_drv1;
 	struct ifnet		*ifp =3D tp->tap_ifp;
=2D	int			s;
=20
 	/* junk all pending output */
+	mtx_lock(&tp->tap_mtx);
 	IF_DRAIN(&ifp->if_snd);
=20
 	/*
@@ -534,28 +527,26 @@
 	 * interface, if we are in VMnet mode. just close the device.
 	 */
=20
=2D	mtx_lock(&tp->tap_mtx);
 	if (((tp->tap_flags & TAP_VMNET) =3D=3D 0) && (ifp->if_flags & IFF_UP)) {
 		mtx_unlock(&tp->tap_mtx);
=2D		s =3D splimp();
 		if_down(ifp);
+		mtx_lock(&tp->tap_mtx);
 		if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
+			ifp->if_drv_flags &=3D ~IFF_DRV_RUNNING;
+			mtx_unlock(&tp->tap_mtx);
 			TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
 				rtinit(ifa, (int)RTM_DELETE, 0);
 			}
 			if_purgeaddrs(ifp);
=2D			ifp->if_drv_flags &=3D ~IFF_DRV_RUNNING;
+			mtx_lock(&tp->tap_mtx);
 		}
=2D		splx(s);
=2D	} else
=2D		mtx_unlock(&tp->tap_mtx);
+	}
=20
 	if_link_state_change(ifp, LINK_STATE_DOWN);
 	funsetown(&tp->tap_sigio);
 	selwakeuppri(&tp->tap_rsel, PZERO+1);
=2D	KNOTE_UNLOCKED(&tp->tap_rsel.si_note, 0);
+	KNOTE_LOCKED(&tp->tap_rsel.si_note, 0);
=20
=2D	mtx_lock(&tp->tap_mtx);
 	tp->tap_flags &=3D ~TAP_OPEN;
 	tp->tap_pid =3D 0;
 	mtx_unlock(&tp->tap_mtx);
@@ -580,8 +571,10 @@
=20
 	TAPDEBUG("initializing %s\n", ifp->if_xname);
=20
+	mtx_lock(&tp->tap_mtx);
 	ifp->if_drv_flags |=3D IFF_DRV_RUNNING;
 	ifp->if_drv_flags &=3D ~IFF_DRV_OACTIVE;
+	mtx_unlock(&tp->tap_mtx);
=20
 	/* attempt to start output */
 	tapifstart(ifp);
@@ -599,7 +592,7 @@
 	struct tap_softc	*tp =3D ifp->if_softc;
 	struct ifreq		*ifr =3D (struct ifreq *)data;
 	struct ifstat		*ifs =3D NULL;
=2D	int			 s, dummy;
+	int			 dummy;
=20
 	switch (cmd) {
 		case SIOCSIFFLAGS: /* XXX -- just like vmnet does */
@@ -612,7 +605,6 @@
 			break;
=20
 		case SIOCGIFSTATUS:
=2D			s =3D splimp();
 			ifs =3D (struct ifstat *)data;
 			dummy =3D strlen(ifs->ascii);
 			mtx_lock(&tp->tap_mtx);
@@ -621,14 +613,10 @@
 					sizeof(ifs->ascii) - dummy,
 					"\tOpened by PID %d\n", tp->tap_pid);
 			mtx_unlock(&tp->tap_mtx);
=2D			splx(s);
 			break;
=20
 		default:
=2D			s =3D splimp();
=2D			dummy =3D ether_ioctl(ifp, cmd, data);
=2D			splx(s);
=2D			return (dummy);
+			return (ether_ioctl(ifp, cmd, data));
 			/* NOT REACHED */
 	}
=20
@@ -645,7 +633,6 @@
 tapifstart(struct ifnet *ifp)
 {
 	struct tap_softc	*tp =3D ifp->if_softc;
=2D	int			 s;
=20
 	TAPDEBUG("%s starting\n", ifp->if_xname);
=20
@@ -657,32 +644,28 @@
 	mtx_lock(&tp->tap_mtx);
 	if (((tp->tap_flags & TAP_VMNET) =3D=3D 0) &&
 	    ((tp->tap_flags & TAP_READY) !=3D TAP_READY)) {
=2D		struct mbuf	*m =3D NULL;
+		struct mbuf *m;
=20
=2D		mtx_unlock(&tp->tap_mtx);
=2D
 		/* Unlocked read. */
 		TAPDEBUG("%s not ready, tap_flags =3D 0x%x\n", ifp->if_xname,=20
 		    tp->tap_flags);
=20
=2D		s =3D splimp();
=2D		do {
+		for (;;) {
 			IF_DEQUEUE(&ifp->if_snd, m);
=2D			if (m !=3D NULL)
+			if (m !=3D NULL) {
 				m_freem(m);
=2D			ifp->if_oerrors ++;
=2D		} while (m !=3D NULL);
=2D		splx(s);
+				ifp->if_oerrors++;
+			} else
+				break;
+		}
+		mtx_unlock(&tp->tap_mtx);
=20
 		return;
 	}
=2D	mtx_unlock(&tp->tap_mtx);
=20
=2D	s =3D splimp();
 	ifp->if_drv_flags |=3D IFF_DRV_OACTIVE;
=20
=2D	if (ifp->if_snd.ifq_len !=3D 0) {
=2D		mtx_lock(&tp->tap_mtx);
+	if (!IFQ_IS_EMPTY(&ifp->if_snd)) {
 		if (tp->tap_flags & TAP_RWAIT) {
 			tp->tap_flags &=3D ~TAP_RWAIT;
 			wakeup(tp);
@@ -691,16 +674,16 @@
 		if ((tp->tap_flags & TAP_ASYNC) && (tp->tap_sigio !=3D NULL)) {
 			mtx_unlock(&tp->tap_mtx);
 			pgsigio(&tp->tap_sigio, SIGIO, 0);
=2D		} else
=2D			mtx_unlock(&tp->tap_mtx);
+			mtx_lock(&tp->tap_mtx);
+		}
=20
 		selwakeuppri(&tp->tap_rsel, PZERO+1);
=2D		KNOTE_UNLOCKED(&tp->tap_rsel.si_note, 0);
+		KNOTE_LOCKED(&tp->tap_rsel.si_note, 0);
 		ifp->if_opackets ++; /* obytes are counted in ether_output */
 	}
=20
 	ifp->if_drv_flags &=3D ~IFF_DRV_OACTIVE;
=2D	splx(s);
+	mtx_unlock(&tp->tap_mtx);
 } /* tapifstart */
=20
=20
@@ -715,7 +698,6 @@
 	struct tap_softc	*tp =3D dev->si_drv1;
 	struct ifnet		*ifp =3D tp->tap_ifp;
 	struct tapinfo		*tapp =3D NULL;
=2D	int			 s;
 	int			 f;
 #if defined(COMPAT_FREEBSD6) || defined(COMPAT_FREEBSD5) || \
     defined(COMPAT_FREEBSD4)
@@ -724,19 +706,21 @@
=20
 	switch (cmd) {
 		case TAPSIFINFO:
=2D			s =3D splimp();
 			tapp =3D (struct tapinfo *)data;
+			mtx_lock(&tp->tap_mtx);
 			ifp->if_mtu =3D tapp->mtu;
 			ifp->if_type =3D tapp->type;
 			ifp->if_baudrate =3D tapp->baudrate;
=2D			splx(s);
+			mtx_unlock(&tp->tap_mtx);
 			break;
=20
 		case TAPGIFINFO:
 			tapp =3D (struct tapinfo *)data;
+			mtx_lock(&tp->tap_mtx);
 			tapp->mtu =3D ifp->if_mtu;
 			tapp->type =3D ifp->if_type;
 			tapp->baudrate =3D ifp->if_baudrate;
+			mtx_unlock(&tp->tap_mtx);
 			break;
=20
 		case TAPSDEBUG:
@@ -757,26 +741,26 @@
 			break;
=20
 		case FIOASYNC:
=2D			s =3D splimp();
 			mtx_lock(&tp->tap_mtx);
 			if (*(int *)data)
 				tp->tap_flags |=3D TAP_ASYNC;
 			else
 				tp->tap_flags &=3D ~TAP_ASYNC;
 			mtx_unlock(&tp->tap_mtx);
=2D			splx(s);
 			break;
=20
 		case FIONREAD:
=2D			s =3D splimp();
=2D			if (ifp->if_snd.ifq_head) {
=2D				struct mbuf	*mb =3D ifp->if_snd.ifq_head;
+			if (!IFQ_IS_EMPTY(&ifp->if_snd)) {
+				struct mbuf *mb;
=20
=2D				for(*(int *)data =3D 0;mb !=3D NULL;mb =3D mb->m_next)
+				IFQ_LOCK(&ifp->if_snd);
+				IFQ_POLL_NOLOCK(&ifp->if_snd, mb);
+				for (*(int *)data =3D 0; mb !=3D NULL;
+				     mb =3D mb->m_next)
 					*(int *)data +=3D mb->m_len;
+				IFQ_UNLOCK(&ifp->if_snd);
 			} else
 				*(int *)data =3D 0;
=2D			splx(s);
 			break;
=20
 		case FIOSETOWN:
@@ -797,10 +781,6 @@
=20
 		/* VMware/VMnet port ioctl's */
=20
=2D		case SIOCGIFFLAGS:	/* get ifnet flags */
=2D			bcopy(&ifp->if_flags, data, sizeof(ifp->if_flags));
=2D			break;
=2D
 #if defined(COMPAT_FREEBSD6) || defined(COMPAT_FREEBSD5) || \
     defined(COMPAT_FREEBSD4)
 		case _IO('V', 0):
@@ -814,9 +794,9 @@
 			f &=3D ~IFF_CANTCHANGE;
 			f |=3D IFF_UP;
=20
=2D			s =3D splimp();
+			mtx_lock(&tp->tap_mtx);
 			ifp->if_flags =3D f | (ifp->if_flags & IFF_CANTCHANGE);
=2D			splx(s);
+			mtx_unlock(&tp->tap_mtx);
 			break;
=20
 		case OSIOCGIFADDR:	/* get MAC address of the remote side */
@@ -851,7 +831,7 @@
 	struct tap_softc	*tp =3D dev->si_drv1;
 	struct ifnet		*ifp =3D tp->tap_ifp;
 	struct mbuf		*m =3D NULL;
=2D	int			 error =3D 0, len, s;
+	int			 error =3D 0, len;
=20
 	TAPDEBUG("%s reading, minor =3D %#x\n", ifp->if_xname, dev2unit(dev));
=20
@@ -867,26 +847,27 @@
 	}
=20
 	tp->tap_flags &=3D ~TAP_RWAIT;
=2D	mtx_unlock(&tp->tap_mtx);
=20
 	/* sleep until we get a packet */
 	do {
=2D		s =3D splimp();
 		IF_DEQUEUE(&ifp->if_snd, m);
=2D		splx(s);
=20
 		if (m =3D=3D NULL) {
=2D			if (flag & O_NONBLOCK)
+			if (flag & O_NONBLOCK) {
+				mtx_unlock(&tp->tap_mtx);
 				return (EWOULDBLOCK);
+			}
=20
=2D			mtx_lock(&tp->tap_mtx);
 			tp->tap_flags |=3D TAP_RWAIT;
=2D			mtx_unlock(&tp->tap_mtx);
=2D			error =3D tsleep(tp,PCATCH|(PZERO+1),"taprd",0);
=2D			if (error)
+			error =3D mtx_sleep(tp, &tp->tap_mtx, PCATCH | (PZERO + 1),
+			    "taprd", 0);
+			if (error) {
+				mtx_unlock(&tp->tap_mtx);
 				return (error);
+			}
 		}
 	} while (m =3D=3D NULL);
+	mtx_unlock(&tp->tap_mtx);
=20
 	/* feed packet to bpf */
 	BPF_MTAP(ifp, m);
@@ -982,14 +963,14 @@
 {
 	struct tap_softc	*tp =3D dev->si_drv1;
 	struct ifnet		*ifp =3D tp->tap_ifp;
=2D	int			 s, revents =3D 0;
+	int			 revents =3D 0;
=20
 	TAPDEBUG("%s polling, minor =3D %#x\n",=20
 		ifp->if_xname, dev2unit(dev));
=20
=2D	s =3D splimp();
 	if (events & (POLLIN | POLLRDNORM)) {
=2D		if (ifp->if_snd.ifq_len > 0) {
+		IFQ_LOCK(&ifp->if_snd);
+		if (!IFQ_IS_EMPTY(&ifp->if_snd)) {
 			TAPDEBUG("%s have data in queue. len =3D %d, " \
 				"minor =3D %#x\n", ifp->if_xname,
 				ifp->if_snd.ifq_len, dev2unit(dev));
@@ -1001,12 +982,12 @@
=20
 			selrecord(td, &tp->tap_rsel);
 		}
+		IFQ_UNLOCK(&ifp->if_snd);
 	}
=20
 	if (events & (POLLOUT | POLLWRNORM))
 		revents |=3D (events & (POLLOUT | POLLWRNORM));
=20
=2D	splx(s);
 	return (revents);
 } /* tappoll */
=20
@@ -1019,11 +1000,9 @@
 static int
 tapkqfilter(struct cdev *dev, struct knote *kn)
 {
=2D    	int			 s;
 	struct tap_softc	*tp =3D dev->si_drv1;
 	struct ifnet		*ifp =3D tp->tap_ifp;
=20
=2D	s =3D splimp();
 	switch (kn->kn_filter) {
 	case EVFILT_READ:
 		TAPDEBUG("%s kqfilter: EVFILT_READ, minor =3D %#x\n",
@@ -1040,13 +1019,11 @@
 	default:
 		TAPDEBUG("%s kqfilter: invalid filter, minor =3D %#x\n",
 			ifp->if_xname, dev2unit(dev));
=2D		splx(s);
 		return (EINVAL);
 		/* NOT REACHED */
 	}
=2D	splx(s);
=20
=2D	kn->kn_hook =3D (caddr_t) dev;
+	kn->kn_hook =3D tp;
 	knlist_add(&tp->tap_rsel.si_note, kn, 0);
=20
 	return (0);
@@ -1061,12 +1038,11 @@
 static int
 tapkqread(struct knote *kn, long hint)
 {
=2D	int			 ret, s;
=2D	struct cdev		*dev =3D (struct cdev *)(kn->kn_hook);
=2D	struct tap_softc	*tp =3D dev->si_drv1;
+	int			 ret;
+	struct tap_softc	*tp =3D kn->kn_hook;
+	struct cdev		*dev =3D tp->tap_dev;
 	struct ifnet		*ifp =3D tp->tap_ifp;
=20
=2D	s =3D splimp();
 	if ((kn->kn_data =3D ifp->if_snd.ifq_len) > 0) {
 		TAPDEBUG("%s have data in queue. len =3D %d, minor =3D %#x\n",
 			ifp->if_xname, ifp->if_snd.ifq_len, dev2unit(dev));
@@ -1076,7 +1052,6 @@
 			ifp->if_xname, dev2unit(dev));
 		ret =3D 0;
 	}
=2D	splx(s);
=20
 	return (ret);
 } /* tapkqread */
@@ -1090,13 +1065,10 @@
 static int
 tapkqwrite(struct knote *kn, long hint)
 {
=2D	int			 s;
=2D	struct tap_softc	*tp =3D ((struct cdev *) kn->kn_hook)->si_drv1;
+	struct tap_softc	*tp =3D kn->kn_hook;
 	struct ifnet		*ifp =3D tp->tap_ifp;
=20
=2D	s =3D splimp();
 	kn->kn_data =3D ifp->if_mtu;
=2D	splx(s);
=20
 	return (1);
 } /* tapkqwrite */
@@ -1105,7 +1077,7 @@
 static void
 tapkqdetach(struct knote *kn)
 {
=2D	struct tap_softc	*tp =3D ((struct cdev *) kn->kn_hook)->si_drv1;
+	struct tap_softc	*tp =3D kn->kn_hook;
=20
 	knlist_remove(&tp->tap_rsel.si_note, kn, 0);
 } /* tapkqdetach */
Index: if_tun.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
=2D-- if_tun.c	(revision 212732)
+++ if_tun.c	(working copy)
@@ -165,7 +165,7 @@
=20
 static struct cdevsw tun_cdevsw =3D {
 	.d_version =3D	D_VERSION,
=2D	.d_flags =3D	D_PSEUDO | D_NEEDGIANT | D_NEEDMINOR,
+	.d_flags =3D	D_PSEUDO | D_NEEDMINOR,
 	.d_open =3D	tunopen,
 	.d_close =3D	tunclose,
 	.d_read =3D	tunread,
@@ -344,13 +344,13 @@
 		tp->tun_flags &=3D ~TUN_RWAIT;
 		wakeup(tp);
 	}
+	selwakeuppri(&tp->tun_rsel, PZERO + 1);
+	KNOTE_LOCKED(&tp->tun_rsel.si_note, 0);
 	if (tp->tun_flags & TUN_ASYNC && tp->tun_sigio) {
 		mtx_unlock(&tp->tun_mtx);
 		pgsigio(&tp->tun_sigio, SIGIO, 0);
 	} else
 		mtx_unlock(&tp->tun_mtx);
=2D	selwakeuppri(&tp->tun_rsel, PZERO + 1);
=2D	KNOTE_UNLOCKED(&tp->tun_rsel.si_note, 0);
 }
=20
 /* XXX: should return an error code so it can fail. */
@@ -385,7 +385,7 @@
 	IFQ_SET_MAXLEN(&ifp->if_snd, ifqmaxlen);
 	ifp->if_snd.ifq_drv_maxlen =3D 0;
 	IFQ_SET_READY(&ifp->if_snd);
=2D	knlist_init_mtx(&sc->tun_rsel.si_note, NULL);
+	knlist_init_mtx(&sc->tun_rsel.si_note, &sc->tun_mtx);
 	ifp->if_capabilities |=3D IFCAP_LINKSTATE;
 	ifp->if_capenable |=3D IFCAP_LINKSTATE;
=20
@@ -426,10 +426,10 @@
 	tp->tun_pid =3D td->td_proc->p_pid;
=20
 	tp->tun_flags |=3D TUN_OPEN;
=2D	mtx_unlock(&tp->tun_mtx);
 	ifp =3D TUN2IFP(tp);
 	if_link_state_change(ifp, LINK_STATE_UP);
 	TUNDEBUG(ifp, "open\n");
+	mtx_unlock(&tp->tun_mtx);
=20
 	return (0);
 }
@@ -443,7 +443,6 @@
 {
 	struct tun_softc *tp;
 	struct ifnet *ifp;
=2D	int s;
=20
 	tp =3D dev->si_drv1;
 	ifp =3D TUN2IFP(tp);
@@ -451,27 +450,25 @@
 	mtx_lock(&tp->tun_mtx);
 	tp->tun_flags &=3D ~TUN_OPEN;
 	tp->tun_pid =3D 0;
=2D	mtx_unlock(&tp->tun_mtx);
=20
 	/*
 	 * junk all pending output
 	 */
 	CURVNET_SET(ifp->if_vnet);
=2D	s =3D splimp();
 	IFQ_PURGE(&ifp->if_snd);
=2D	splx(s);
=20
 	if (ifp->if_flags & IFF_UP) {
=2D		s =3D splimp();
+		mtx_unlock(&tp->tun_mtx);
 		if_down(ifp);
=2D		splx(s);
+		mtx_lock(&tp->tun_mtx);
 	}
=20
 	/* Delete all addresses and routes which reference this interface. */
 	if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
 		struct ifaddr *ifa;
=20
=2D		s =3D splimp();
+		ifp->if_drv_flags &=3D ~IFF_DRV_RUNNING;
+		mtx_unlock(&tp->tun_mtx);
 		TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
 			/* deal w/IPv4 PtP destination; unlocked read */
 			if (ifa->ifa_addr->sa_family =3D=3D AF_INET) {
@@ -482,16 +479,14 @@
 			}
 		}
 		if_purgeaddrs(ifp);
=2D		ifp->if_drv_flags &=3D ~IFF_DRV_RUNNING;
=2D		splx(s);
+		mtx_lock(&tp->tun_mtx);
 	}
 	if_link_state_change(ifp, LINK_STATE_DOWN);
 	CURVNET_RESTORE();
=20
=2D	mtx_lock(&tp->tun_mtx);
 	funsetown(&tp->tun_sigio);
 	selwakeuppri(&tp->tun_rsel, PZERO + 1);
=2D	KNOTE_UNLOCKED(&tp->tun_rsel.si_note, 0);
+	KNOTE_LOCKED(&tp->tun_rsel.si_note, 0);
 	TUNDEBUG (ifp, "closed\n");
=20
 	cv_broadcast(&tp->tun_cv);
@@ -510,6 +505,7 @@
=20
 	TUNDEBUG(ifp, "tuninit\n");
=20
+	mtx_lock(&tp->tun_mtx);
 	ifp->if_flags |=3D IFF_UP;
 	ifp->if_drv_flags |=3D IFF_DRV_RUNNING;
 	getmicrotime(&ifp->if_lastchange);
@@ -521,18 +517,17 @@
 			struct sockaddr_in *si;
=20
 			si =3D (struct sockaddr_in *)ifa->ifa_addr;
=2D			mtx_lock(&tp->tun_mtx);
 			if (si->sin_addr.s_addr)
 				tp->tun_flags |=3D TUN_IASET;
=20
 			si =3D (struct sockaddr_in *)ifa->ifa_dstaddr;
 			if (si && si->sin_addr.s_addr)
 				tp->tun_flags |=3D TUN_DSTADDR;
=2D			mtx_unlock(&tp->tun_mtx);
 		}
 	}
 	if_addr_runlock(ifp);
 #endif
+	mtx_unlock(&tp->tun_mtx);
 	return (error);
 }
=20
@@ -545,9 +540,8 @@
 	struct ifreq *ifr =3D (struct ifreq *)data;
 	struct tun_softc *tp =3D ifp->if_softc;
 	struct ifstat *ifs;
=2D	int		error =3D 0, s;
+	int		error =3D 0;
=20
=2D	s =3D splimp();
 	switch(cmd) {
 	case SIOCGIFSTATUS:
 		ifs =3D (struct ifstat *)data;
@@ -576,7 +570,6 @@
 	default:
 		error =3D EINVAL;
 	}
=2D	splx(s);
 	return (error);
 }
=20
@@ -682,7 +675,6 @@
 static	int
 tunioctl(struct cdev *dev, u_long cmd, caddr_t data, int flag, struct thre=
ad *td)
 {
=2D	int		s;
 	int		error;
 	struct tun_softc *tp =3D dev->si_drv1;
 	struct tuninfo *tunp;
@@ -697,15 +689,19 @@
 			if (error)
 				return (error);
 		}
+		mtx_lock(&tp->tun_mtx);
 		TUN2IFP(tp)->if_mtu =3D tunp->mtu;
 		TUN2IFP(tp)->if_type =3D tunp->type;
 		TUN2IFP(tp)->if_baudrate =3D tunp->baudrate;
+		mtx_unlock(&tp->tun_mtx);
 		break;
 	case TUNGIFINFO:
 		tunp =3D (struct tuninfo *)data;
+		mtx_lock(&tp->tun_mtx);
 		tunp->mtu =3D TUN2IFP(tp)->if_mtu;
 		tunp->type =3D TUN2IFP(tp)->if_type;
 		tunp->baudrate =3D TUN2IFP(tp)->if_baudrate;
+		mtx_unlock(&tp->tun_mtx);
 		break;
 	case TUNSDEBUG:
 		tundebug =3D *(int *)data;
@@ -732,7 +728,6 @@
 		mtx_unlock(&tp->tun_mtx);
 		break;
 	case TUNGIFHEAD:
=2D		/* Could be unlocked read? */
 		mtx_lock(&tp->tun_mtx);
 		*(int *)data =3D (tp->tun_flags & TUN_IFHEAD) ? 1 : 0;
 		mtx_unlock(&tp->tun_mtx);
@@ -745,9 +740,11 @@
 		switch (*(int *)data & ~IFF_MULTICAST) {
 		case IFF_POINTOPOINT:
 		case IFF_BROADCAST:
+			mtx_lock(&tp->tun_mtx);
 			TUN2IFP(tp)->if_flags &=3D
 			    ~(IFF_BROADCAST|IFF_POINTOPOINT|IFF_MULTICAST);
 			TUN2IFP(tp)->if_flags |=3D *(int *)data;
+			mtx_unlock(&tp->tun_mtx);
 			break;
 		default:
 			return(EINVAL);
@@ -769,17 +766,15 @@
 		mtx_unlock(&tp->tun_mtx);
 		break;
 	case FIONREAD:
=2D		s =3D splimp();
 		if (!IFQ_IS_EMPTY(&TUN2IFP(tp)->if_snd)) {
 			struct mbuf *mb;
 			IFQ_LOCK(&TUN2IFP(tp)->if_snd);
 			IFQ_POLL_NOLOCK(&TUN2IFP(tp)->if_snd, mb);
=2D			for( *(int *)data =3D 0; mb !=3D 0; mb =3D mb->m_next)
+			for (*(int *)data =3D 0; mb !=3D NULL; mb =3D mb->m_next)
 				*(int *)data +=3D mb->m_len;
 			IFQ_UNLOCK(&TUN2IFP(tp)->if_snd);
 		} else
 			*(int *)data =3D 0;
=2D		splx(s);
 		break;
 	case FIOSETOWN:
 		return (fsetown(*(int *)data, &tp->tun_sigio));
@@ -813,7 +808,7 @@
 	struct tun_softc *tp =3D dev->si_drv1;
 	struct ifnet	*ifp =3D TUN2IFP(tp);
 	struct mbuf	*m;
=2D	int		error=3D0, len, s;
+	int		error=3D0, len;
=20
 	TUNDEBUG (ifp, "read\n");
 	mtx_lock(&tp->tun_mtx);
@@ -824,27 +819,24 @@
 	}
=20
 	tp->tun_flags &=3D ~TUN_RWAIT;
=2D	mtx_unlock(&tp->tun_mtx);
=20
=2D	s =3D splimp();
 	do {
 		IFQ_DEQUEUE(&ifp->if_snd, m);
 		if (m =3D=3D NULL) {
 			if (flag & O_NONBLOCK) {
=2D				splx(s);
+				mtx_unlock(&tp->tun_mtx);
 				return (EWOULDBLOCK);
 			}
=2D			mtx_lock(&tp->tun_mtx);
 			tp->tun_flags |=3D TUN_RWAIT;
=2D			mtx_unlock(&tp->tun_mtx);
=2D			if ((error =3D tsleep(tp, PCATCH | (PZERO + 1),
=2D					"tunread", 0)) !=3D 0) {
=2D				splx(s);
+			error =3D mtx_sleep(tp, &tp->tun_mtx, PCATCH | (PZERO + 1),
+			    "tunread", 0);
+			if (error !=3D 0) {
+				mtx_unlock(&tp->tun_mtx);
 				return (error);
 			}
 		}
 	} while (m =3D=3D NULL);
=2D	splx(s);
+	mtx_unlock(&tp->tun_mtx);
=20
 	while (m && uio->uio_resid > 0 && error =3D=3D 0) {
 		len =3D min(uio->uio_resid, m->m_len);
@@ -957,13 +949,11 @@
 static	int
 tunpoll(struct cdev *dev, int events, struct thread *td)
 {
=2D	int		s;
 	struct tun_softc *tp =3D dev->si_drv1;
 	struct ifnet	*ifp =3D TUN2IFP(tp);
 	int		revents =3D 0;
 	struct mbuf	*m;
=20
=2D	s =3D splimp();
 	TUNDEBUG(ifp, "tunpoll\n");
=20
 	if (events & (POLLIN | POLLRDNORM)) {
@@ -981,7 +971,6 @@
 	if (events & (POLLOUT | POLLWRNORM))
 		revents |=3D events & (POLLOUT | POLLWRNORM);
=20
=2D	splx(s);
 	return (revents);
 }
=20
@@ -991,11 +980,9 @@
 static int
 tunkqfilter(struct cdev *dev, struct knote *kn)
 {
=2D	int			s;
 	struct tun_softc	*tp =3D dev->si_drv1;
 	struct ifnet	*ifp =3D TUN2IFP(tp);
=20
=2D	s =3D splimp();
 	switch(kn->kn_filter) {
 	case EVFILT_READ:
 		TUNDEBUG(ifp, "%s kqfilter: EVFILT_READ, minor =3D %#x\n",
@@ -1012,12 +999,10 @@
 	default:
 		TUNDEBUG(ifp, "%s kqfilter: invalid filter, minor =3D %#x\n",
 		    ifp->if_xname, dev2unit(dev));
=2D		splx(s);
 		return(EINVAL);
 	}
=2D	splx(s);
=20
=2D	kn->kn_hook =3D (caddr_t) dev;
+	kn->kn_hook =3D tp;
 	knlist_add(&tp->tun_rsel.si_note, kn, 0);
=20
 	return (0);
@@ -1029,12 +1014,11 @@
 static int
 tunkqread(struct knote *kn, long hint)
 {
=2D	int			ret, s;
=2D	struct cdev		*dev =3D (struct cdev *)(kn->kn_hook);
=2D	struct tun_softc	*tp =3D dev->si_drv1;
+	int			ret;
+	struct tun_softc	*tp =3D kn->kn_hook;
+	struct cdev		*dev =3D tp->tun_dev;
 	struct ifnet	*ifp =3D TUN2IFP(tp);
=20
=2D	s =3D splimp();
 	if ((kn->kn_data =3D ifp->if_snd.ifq_len) > 0) {
 		TUNDEBUG(ifp,
 		    "%s have data in the queue.  Len =3D %d, minor =3D %#x\n",
@@ -1046,7 +1030,6 @@
 		    dev2unit(dev));
 		ret =3D 0;
 	}
=2D	splx(s);
=20
 	return (ret);
 }
@@ -1057,13 +1040,10 @@
 static int
 tunkqwrite(struct knote *kn, long hint)
 {
=2D	int			s;
=2D	struct tun_softc	*tp =3D ((struct cdev *)kn->kn_hook)->si_drv1;
+	struct tun_softc	*tp =3D kn->kn_hook;
 	struct ifnet	*ifp =3D TUN2IFP(tp);
=20
=2D	s =3D splimp();
 	kn->kn_data =3D ifp->if_mtu;
=2D	splx(s);
=20
 	return (1);
 }
@@ -1071,7 +1051,7 @@
 static void
 tunkqdetach(struct knote *kn)
 {
=2D	struct tun_softc	*tp =3D ((struct cdev *)kn->kn_hook)->si_drv1;
+	struct tun_softc	*tp =3D kn->kn_hook;
=20
 	knlist_remove(&tp->tun_rsel.si_note, kn, 0);
 }

=2D-=20
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201009170854.56516.jhb>