devip: automatically unbind interface on read errors

netdevmedium and ethermedium should unbind the interface
if one of the reader procs encounters an error,
such as a usb ethernet device being un-plugged.

netdevmedium already attempted something like this,
but without considering the locking as we need to
acquire the ipifc conv qlock for adjusting the
inuse ref counter.

this change adds a mediumunbindifc() function to
be used by the medium to unbind from its interface
asynchronously from its reader procs.

the call eigther succeeds (returning nil)
meaning the Medium.unbind function was executed
and the auxiolary structure has been freed,
or retunrs an error (when Medium.unbind was called
before) and we just clear our Proc* pointer.

sys/src/9/ip/ethermedium.c

@@ -200,6 +200,12 @@ etherbind(Ipifc *ifc, int argc, char **argv)
 	kproc("recvarpproc", recvarpproc, ifc);
 }
 
+/*
+ *  called from devip due to
+ *  manual unbind or from one of the reader procs
+ *  due to read error from the ethernet device.
+ *  this is called only once!
+ */
 static void
 etherunbind(Ipifc *ifc)
 {
@@ -211,18 +217,20 @@ etherunbind(Ipifc *ifc)
 	while(er->arpp == (void*)-1 || er->read4p == (void*)-1 || er->read6p == (void*)-1)
 		tsleep(&up->sleep, return0, 0, 300);
 
-	if(er->read4p != nil)
+	if(er->read4p != nil && er->read4p != up)
 		postnote(er->read4p, 1, "unbind", 0);
-	if(er->read6p != nil)
+	if(er->read6p != nil && er->read6p != up)
 		postnote(er->read6p, 1, "unbind", 0);
-	if(er->arpp != nil)
+	if(er->arpp != nil && er->arpp != up)
 		postnote(er->arpp, 1, "unbind", 0);
 	poperror();
 
 	while(waserror())
 		;
 	/* wait for readers to die */
-	while(er->arpp != nil || er->read4p != nil || er->read6p != nil)
+	while(er->arpp != nil && er->arpp != up
+	|| er->read4p != nil && er->read4p != up
+	|| er->read6p != nil && er->read6p != up)
 		tsleep(&up->sleep, return0, 0, 300);
 	poperror();
 
@@ -323,8 +331,10 @@ etherread4(void *a)
 	if(!waserror())
 	for(;;){
 		bp = devtab[er->mchan4->type]->bread(er->mchan4, ifc->maxtu, 0);
-		if(bp == nil)
+		if(bp == nil){
+			poperror();
 			break;
+		}
 		rlock(ifc);
 		if(waserror()){
 			runlock(ifc);
@@ -340,7 +350,8 @@ etherread4(void *a)
 		runlock(ifc);
 		poperror();
 	}
-	er->read4p = nil;
+	if(mediumunbindifc(ifc) != nil)
+		er->read4p = nil;	/* someone else is doing the unbind */
 	pexit("hangup", 1);
 }
 
@@ -361,8 +372,10 @@ etherread6(void *a)
 	if(!waserror())
 	for(;;){
 		bp = devtab[er->mchan6->type]->bread(er->mchan6, ifc->maxtu, 0);
-		if(bp == nil)
+		if(bp == nil){
+			poperror();
 			break;
+		}
 		rlock(ifc);
 		if(waserror()){
 			runlock(ifc);
@@ -378,7 +391,8 @@ etherread6(void *a)
 		runlock(ifc);
 		poperror();
 	}
-	er->read6p = nil;
+	if(mediumunbindifc(ifc) != nil)
+		er->read6p = nil;	/* someone else is doing the unbind */
 	pexit("hangup", 1);
 }
 
@@ -513,7 +527,7 @@ recvarp(Ipifc *ifc)
 
 	ebp = devtab[er->achan->type]->bread(er->achan, ifc->maxtu, 0);
 	if(ebp == nil)
-		return;
+		error(Ehungup);
 
 	rlock(ifc);
 
@@ -611,7 +625,8 @@ recvarpproc(void *v)
 
 	er->arpp = up;
 	if(waserror()){
-		er->arpp = nil;
+		if(mediumunbindifc(ifc) != nil)
+			er->arpp = nil;	/* someone else is doing the unbind */
 		pexit("hangup", 1);
 	}
 	for(;;)

sys/src/9/ip/ip.h

@@ -741,6 +741,8 @@ extern char*	ipifcadd(Ipifc *ifc, char **argv, int argc, int tentative, Iplifc *
 extern long	ipselftabread(Fs*, char *a, ulong offset, int n);
 extern char*	ipifcadd6(Ipifc *ifc, char**argv, int argc);
 extern char*	ipifcremove6(Ipifc *ifc, char**argv, int argc);
+extern char*	mediumunbindifc(Ipifc *ifc);
+
 /*
  *  ip.c
  */

sys/src/9/ip/ipifc.c

@@ -123,6 +123,7 @@ ipfindmedium(char *name)
 
 /* same as nullmedium, to prevent unbind while bind or unbind is in progress */
 extern Medium unboundmedium;
+static char *ipifcunbindmedium(Ipifc *ifc, Medium *m);
 
 /*
  *  attach a device (or pkt driver) to the interface.
@@ -137,13 +138,12 @@ ipifcbind(Conv *c, char **argv, int argc)
 	if(argc < 2)
 		return Ebadarg;
 
-	ifc = (Ipifc*)c->ptcl;
-
 	/* bind the device to the interface */
 	m = ipfindmedium(argv[1]);
 	if(m == nil)
 		return "unknown interface type";
 
+	ifc = (Ipifc*)c->ptcl;
 	wlock(ifc);
 	if(ifc->m != nil){
 		wunlock(ifc);
@@ -151,10 +151,15 @@ ipifcbind(Conv *c, char **argv, int argc)
 	}
 	ifc->m = &unboundmedium;	/* fake until bound */
 	ifc->arg = nil;
+	if(m->unbindonclose == 0)
+		c->inuse++;
+	snprint(ifc->dev, sizeof ifc->dev, "%s%d", ifc->m->name, c->x);
 	wunlock(ifc);
 
 	if(waserror()){
 		wlock(ifc);
+		if(m->unbindonclose == 0)
+			c->inuse--;
 		ifc->m = nil;
 		wunlock(ifc);
 		nexterror();
@@ -163,6 +168,9 @@ ipifcbind(Conv *c, char **argv, int argc)
 	poperror();
 
 	wlock(ifc);
+	/* switch to the real medium */
+	ifc->m = m;
+
 	/* set the bound device name */
 	if(argc > 2)
 		strncpy(ifc->dev, argv[2], sizeof(ifc->dev));
@@ -171,12 +179,9 @@ ipifcbind(Conv *c, char **argv, int argc)
 	ifc->dev[sizeof(ifc->dev)-1] = 0;
 
 	/* set up parameters */
-	ifc->m = m;
-	ifc->maxtu = ifc->m->maxtu;
+	ifc->maxtu = m->maxtu;
 	ifc->delay = 40;
 	ifc->speed = 0;
-	if(ifc->m->unbindonclose == 0)
-		ifc->conv->inuse++;
 
 	/* default router paramters */
 	ifc->rp = c->p->f->v6p->rp;
@@ -194,25 +199,19 @@ ipifcbind(Conv *c, char **argv, int argc)
 }
 
 /*
- *  detach a device from an interface, close the interface
+ *  detach a medium from an interface, close the interface
+ *  ifc->conv is locked, ifc is wlocked
  */
 static char*
-ipifcunbind(Ipifc *ifc)
+ipifcunbindmedium(Ipifc *ifc, Medium *m)
 {
-	Medium *m;
-
-	wlock(ifc);
-	m = ifc->m;
-	if(m == nil || m == &unboundmedium){
-		wunlock(ifc);
+	if(m == nil || m == &unboundmedium)
 		return Eunbound;
-	}
 
 	/* disassociate logical interfaces */
 	while(ifc->lifc != nil)
 		ipifcremlifc(ifc, &ifc->lifc);
 
-
 	/* disassociate device */
 	if(m->unbind != nil){
 		ifc->m = &unboundmedium;	/* fake until unbound */
@@ -236,14 +235,72 @@ ipifcunbind(Ipifc *ifc)
 
 	/* dissociate routes */
 	ifc->ifcid++;
+	ifc->m = nil;
+	ifc->arg = nil;
+
 	if(m->unbindonclose == 0)
 		ifc->conv->inuse--;
-	ifc->m = nil;
-	wunlock(ifc);
 
 	return nil;
 }
 
+/*
+ *  called from Ipifcproto->unbind,
+ *  with ifc->conv locked.
+ */
+static char*
+ipifcunbind(Ipifc *ifc)
+{
+	char *err;
+
+	wlock(ifc);
+	err = ipifcunbindmedium(ifc, ifc->m);
+	wunlock(ifc);
+
+	return err;
+}
+
+/*
+ *  called from medium at any time to unbind
+ *  an interface in case of an error such as
+ *  usb ethernet being un-plugged.
+ */
+char*
+mediumunbindifc(Ipifc *ifc)
+{
+	Medium *m;
+	Conv *conv;
+	char *err;
+
+	err = Eunbound;
+
+	rlock(ifc);
+	m = ifc->m;
+	if(m == &unboundmedium){
+		runlock(ifc);
+		return err;
+	}
+	conv = ifc->conv;
+	runlock(ifc);
+
+	assert(conv != nil);
+	assert(m != nil);
+	assert(m->unbindonclose == 0);
+
+	qlock(conv);
+
+	assert(conv->inuse > 0);
+	assert((Ipifc*)conv->ptcl == ifc);
+
+	wlock(ifc);
+	if(ifc->m == m)
+		err = ipifcunbindmedium(ifc, m);
+	wunlock(ifc);
+	qunlock(conv);
+
+	return err;
+}
+
 char sfixedformat[] = "device %s maxtu %d sendra %d recvra %d mflag %d oflag %d"
 " maxraint %d minraint %d linkmtu %d reachtime %d rxmitra %d ttl %d routerlt %d"
 " pktin %lud pktout %lud errin %lud errout %lud speed %d delay %d\n";

sys/src/9/ip/netdevmedium.c

@@ -68,14 +68,14 @@ netdevunbind(Ipifc *ifc)
 	while(er->readp == (void*)-1)
 		tsleep(&up->sleep, return0, 0, 300);
 
-	if(er->readp != nil)
+	if(er->readp != nil && er->readp != up)
 		postnote(er->readp, 1, "unbind", 0);
 	poperror();
 
 	while(waserror())
 		;
 	/* wait for reader to die */
-	while(er->readp != nil)
+	while(er->readp != nil && er->readp != up)
 		tsleep(&up->sleep, return0, 0, 300);
 	poperror();
 
@@ -115,10 +115,6 @@ netdevread(void *a)
 		bp = devtab[er->mchan->type]->bread(er->mchan, ifc->maxtu, 0);
 		if(bp == nil){
 			poperror();
-			if(!waserror()){
-				static char *argv[]  = { "unbind" };
-				ifc->conv->p->ctl(ifc->conv, argv, 1);
-			}
 			break;
 		}
 		rlock(ifc);
@@ -134,7 +130,8 @@ netdevread(void *a)
 		runlock(ifc);
 		poperror();
 	}
-	er->readp = nil;
+	if(mediumunbindifc(ifc) != nil)
+		er->readp = nil;	/* someone else is doing the unbind */
 	pexit("hangup", 1);
 }