devip: fix packet loss when interface is wlocked

to prevent deadlock on media unbind (which is called with
the interface wlock()'ed), the medias reader processes
that unbind was waiting for used to discard packets when
the interface could not be rlocked.

this has the unfortunate side effect that when we change
addresses on a interface that packets are getting lost.
this is problematic for the processing of ipv6 router
advertisements when multiple RA's are getting received
in quick succession.

this change removes that packet dropping behaviour and
instead changes the unbind process to avoid the deadlock
by wunlock()ing the interface temporarily while waiting
for the reader processes to finish. the interface media
is also changed to the mullmedium before unlocking (see
the comment).

6 files changed, 85 insertions, 63 deletions

diff --git a/sys/src/9/ip/ethermedium.c b/sys/src/9/ip/ethermedium.c
index 0f0975fc2..2bb022dcf 100644
--- a/sys/src/9/ip/ethermedium.c
+++ b/sys/src/9/ip/ethermedium.c
@@ -218,6 +218,9 @@ etherunbind(Ipifc *ifc)
 {
 	Etherrock *er = ifc->arg;
 
+	while(waserror())
+		;
+
 	/* wait for readers to start */
 	while(er->arpp == (void*)-1 || er->read4p == (void*)-1 || er->read6p == (void*)-1)
 		tsleep(&up->sleep, return0, 0, 300);
@@ -229,10 +232,19 @@ etherunbind(Ipifc *ifc)
 	if(er->arpp != nil)
 		postnote(er->arpp, 1, "unbind", 0);
 
+	poperror();
+
+	wunlock(ifc);
+	while(waserror())
+		;
+
 	/* wait for readers to die */
 	while(er->arpp != nil || er->read4p != nil || er->read6p != nil)
 		tsleep(&up->sleep, return0, 0, 300);
 
+	poperror();
+	wlock(ifc);
+
 	if(er->mchan4 != nil)
 		cclose(er->mchan4);
 	if(er->cchan4 != nil)
@@ -319,16 +331,12 @@ etherread4(void *a)
 	ifc = a;
 	er = ifc->arg;
 	er->read4p = up;	/* hide identity under a rock for unbind */
-	if(waserror()){
-		er->read4p = nil;
-		pexit("hangup", 1);
-	}
+	if(!waserror())
 	for(;;){
 		bp = devtab[er->mchan4->type]->bread(er->mchan4, ifc->maxtu, 0);
-		if(!canrlock(ifc)){
-			freeb(bp);
-			continue;
-		}
+		if(bp == nil)
+			break;
+		rlock(ifc);
 		if(waserror()){
 			runlock(ifc);
 			nexterror();
@@ -343,6 +351,8 @@ etherread4(void *a)
 		runlock(ifc);
 		poperror();
 	}
+	er->read4p = nil;
+	pexit("hangup", 1);
 }
 
 
@@ -359,16 +369,12 @@ etherread6(void *a)
 	ifc = a;
 	er = ifc->arg;
 	er->read6p = up;	/* hide identity under a rock for unbind */
-	if(waserror()){
-		er->read6p = nil;
-		pexit("hangup", 1);
-	}
+	if(!waserror())
 	for(;;){
 		bp = devtab[er->mchan6->type]->bread(er->mchan6, ifc->maxtu, 0);
-		if(!canrlock(ifc)){
-			freeb(bp);
-			continue;
-		}
+		if(bp == nil)
+			break;
+		rlock(ifc);
 		if(waserror()){
 			runlock(ifc);
 			nexterror();
@@ -383,6 +389,8 @@ etherread6(void *a)
 		runlock(ifc);
 		poperror();
 	}
+	er->read6p = nil;
+	pexit("hangup", 1);
 }
 
 static void
@@ -559,10 +567,7 @@ recvarp(Ipifc *ifc)
 	if(ebp == nil)
 		return;
 
-	if(!canrlock(ifc)){
-		freeb(ebp);
-		return;
-	}
+	rlock(ifc);
 
 	e = (Etherarp*)ebp->rp;
 	switch(nhgets(e->op)) {
diff --git a/sys/src/9/ip/ip.c b/sys/src/9/ip/ip.c
index 0752cb6f0..9f7bfa0a0 100644
--- a/sys/src/9/ip/ip.c
+++ b/sys/src/9/ip/ip.c
@@ -123,8 +123,7 @@ ipoput4(Fs *f, Block *bp, int gating, int ttl, int tos, Routehint *rh)
 	else
 		gate = r->v4.gate;
 
-	if(!canrlock(ifc))
-		goto free;
+	rlock(ifc);
 	if(waserror()){
 		runlock(ifc);
 		nexterror();
diff --git a/sys/src/9/ip/ipifc.c b/sys/src/9/ip/ipifc.c
index 3a4380877..7af840991 100644
--- a/sys/src/9/ip/ipifc.c
+++ b/sys/src/9/ip/ipifc.c
@@ -201,8 +201,11 @@ ipifcbind(Conv *c, char **argv, int argc)
 static char*
 ipifcunbind(Ipifc *ifc)
 {
+	Medium *m;
+
 	wlock(ifc);
-	if(ifc->m == nil){
+	m = ifc->m;
+	if(m == nil){
 		wunlock(ifc);
 		return Eunbound;
 	}
@@ -212,9 +215,18 @@ ipifcunbind(Ipifc *ifc)
 		ipifcremlifc(ifc, &ifc->lifc);
 
 	/* disassociate device */
-	if(ifc->m->unbind != nil){
+	if(m->unbind != nil){
+		extern Medium nullmedium;
+
+		/*
+		 * unbind() might unlock the ifc, so change the medium
+		 * to the nullmedium to prevent packets from getting
+		 * sent while the medium is shutting down.
+		 */
+		ifc->m = &nullmedium;
+
 		if(!waserror()){
-			(*ifc->m->unbind)(ifc);
+			(*m->unbind)(ifc);
 			poperror();
 		}
 	}
@@ -232,7 +244,7 @@ ipifcunbind(Ipifc *ifc)
 
 	/* dissociate routes */
 	ifc->ifcid++;
-	if(ifc->m->unbindonclose == 0)
+	if(m->unbindonclose == 0)
 		ifc->conv->inuse--;
 	ifc->m = nil;
 	wunlock(ifc);
@@ -370,10 +382,7 @@ ipifckick(void *x)
 		return;
 
 	ifc = (Ipifc*)c->ptcl;
-	if(!canrlock(ifc)){
-		freeb(bp);
-		return;
-	}
+	rlock(ifc);
 	if(waserror()){
 		runlock(ifc);
 		nexterror();
@@ -622,10 +631,13 @@ ipifcadd(Ipifc *ifc, char **argv, int argc, int tentative, Iplifc *lifcp)
 	}
 
 done:
-	ipifcregisteraddr(f, ifc, lifc, ip);
 	wunlock(ifc);
 	poperror();
 
+	rlock(ifc);
+	ipifcregisteraddr(f, ifc, lifc, ip);
+	runlock(ifc);
+
 	return nil;
 }
 
diff --git a/sys/src/9/ip/ipv6.c b/sys/src/9/ip/ipv6.c
index 090d33460..7d7cdc09a 100644
--- a/sys/src/9/ip/ipv6.c
+++ b/sys/src/9/ip/ipv6.c
@@ -76,9 +76,7 @@ ipoput6(Fs *f, Block *bp, int gating, int ttl, int tos, Routehint *rh)
 	else
 		gate = r->v6.gate;
 
-	if(!canrlock(ifc))
-		goto free;
-
+	rlock(ifc);
 	if(waserror()){
 		runlock(ifc);
 		nexterror();
diff --git a/sys/src/9/ip/loopbackmedium.c b/sys/src/9/ip/loopbackmedium.c
index 26389f390..2bb4271be 100644
--- a/sys/src/9/ip/loopbackmedium.c
+++ b/sys/src/9/ip/loopbackmedium.c
@@ -42,6 +42,9 @@ loopbackunbind(Ipifc *ifc)
 {
 	LB *lb = ifc->arg;
 
+	while(waserror())
+		;
+
 	/* wat for reader to start */
 	while(lb->readp == (void*)-1)
 		tsleep(&up->sleep, return0, 0, 300);
@@ -49,10 +52,19 @@ loopbackunbind(Ipifc *ifc)
 	if(lb->readp != nil)
 		postnote(lb->readp, 1, "unbind", 0);
 
+	poperror();
+
+	wunlock(ifc);
+	while(waserror())
+		;
+
 	/* wait for reader to die */
 	while(lb->readp != nil)
 		tsleep(&up->sleep, return0, 0, 300);
 
+	poperror();
+	wlock(ifc);
+
 	/* clean up */
 	qfree(lb->q);
 	free(lb);
@@ -79,18 +91,9 @@ loopbackread(void *a)
 	ifc = a;
 	lb = ifc->arg;
 	lb->readp = up;	/* hide identity under a rock for unbind */
-	if(waserror()){
-		lb->readp = nil;
-		pexit("hangup", 1);
-	}
-	for(;;){
-		bp = qbread(lb->q, Maxtu);
-		if(bp == nil)
-			continue;
-		if(!canrlock(ifc)){
-			freeb(bp);
-			continue;
-		}
+	if(!waserror())
+	while((bp = qbread(lb->q, Maxtu)) != nil){
+		rlock(ifc);
 		if(waserror()){
 			runlock(ifc);
 			nexterror();
@@ -103,6 +106,8 @@ loopbackread(void *a)
 		runlock(ifc);
 		poperror();
 	}
+	lb->readp = nil;
+	pexit("hangup", 1);
 }
 
 Medium loopbackmedium =
diff --git a/sys/src/9/ip/netdevmedium.c b/sys/src/9/ip/netdevmedium.c
index f3d9ac1a4..6d56420fd 100644
--- a/sys/src/9/ip/netdevmedium.c
+++ b/sys/src/9/ip/netdevmedium.c
@@ -66,6 +66,9 @@ netdevunbind(Ipifc *ifc)
 {
 	Netdevrock *er = ifc->arg;
 
+	while(waserror())
+		;
+
 	/* wait for reader to start */
 	while(er->readp == (void*)-1)
 		tsleep(&up->sleep, return0, 0, 300);
@@ -73,10 +76,19 @@ netdevunbind(Ipifc *ifc)
 	if(er->readp != nil)
 		postnote(er->readp, 1, "unbind", 0);
 
+	poperror();
+
+	wunlock(ifc);
+	while(waserror())
+		;
+
 	/* wait for reader to die */
 	while(er->readp != nil)
 		tsleep(&up->sleep, return0, 0, 300);
 
+	poperror();
+	wlock(ifc);
+
 	if(er->mchan != nil)
 		cclose(er->mchan);
 
@@ -107,33 +119,22 @@ netdevread(void *a)
 	Ipifc *ifc;
 	Block *bp;
 	Netdevrock *er;
-	char *argv[1];
 
 	ifc = a;
 	er = ifc->arg;
 	er->readp = up;	/* hide identity under a rock for unbind */
-	if(waserror()){
-		er->readp = nil;
-		pexit("hangup", 1);
-	}
+	if(!waserror())
 	for(;;){
 		bp = devtab[er->mchan->type]->bread(er->mchan, ifc->maxtu, 0);
 		if(bp == nil){
-			/*
-			 * get here if mchan is a pipe and other side hangs up
-			 * clean up this interface & get out
-			 */
 			poperror();
-			er->readp = nil;
-			argv[0] = "unbind";
-			if(!waserror())
+			if(!waserror()){
+				static char *argv[]  = { "unbind" };
 				ifc->conv->p->ctl(ifc->conv, argv, 1);
-			pexit("hangup", 1);
-		}
-		if(!canrlock(ifc)){
-			freeb(bp);
-			continue;
+			}
+			break;
 		}
+		rlock(ifc);
 		if(waserror()){
 			runlock(ifc);
 			nexterror();
@@ -146,6 +147,8 @@ netdevread(void *a)
 		runlock(ifc);
 		poperror();
 	}
+	er->readp = nil;
+	pexit("hangup", 1);
 }
 
 void