shithub: riscv

Download patch

ref: e182ec11fb0b4e90d69f94dc5020deed505bf33a
parent: 354273938c80121158db1a9ac87556ddcb1d024c
author: cinap_lenrek <[email protected]>
date: Sat Sep 16 20:16:24 EDT 2023 

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.


--- a/sys/src/9/ip/ethermedium.c
+++ b/sys/src/9/ip/ethermedium.c
@@ -200,6 +200,12 @@
 	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,11 +217,11 @@
 	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();
 
@@ -222,7 +228,9 @@
 	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 @@
 	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 @@
 		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 @@
 	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 @@
 		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 @@
 
 	ebp = devtab[er->achan->type]->bread(er->achan, ifc->maxtu, 0);
 	if(ebp == nil)
-		return;
+		error(Ehungup);
 
 	rlock(ifc);
 
@@ -611,7 +625,8 @@
 
 	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(;;)
--- a/sys/src/9/ip/ip.h
+++ b/sys/src/9/ip/ip.h
@@ -741,6 +741,8 @@
 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
  */
--- a/sys/src/9/ip/ipifc.c
+++ b/sys/src/9/ip/ipifc.c
@@ -123,6 +123,7 @@
 
 /* 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 @@
 	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 @@
 	}
 	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 @@
 	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 @@
 	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 @@
 }
 
 /*
- *  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,12 +235,70 @@
 
 	/* 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"
--- a/sys/src/9/ip/netdevmedium.c
+++ b/sys/src/9/ip/netdevmedium.c
@@ -68,7 +68,7 @@
 	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();
 
@@ -75,7 +75,7 @@
 	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 @@
 		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 @@
 		runlock(ifc);
 		poperror();
 	}
-	er->readp = nil;
+	if(mediumunbindifc(ifc) != nil)
+		er->readp = nil;	/* someone else is doing the unbind */
 	pexit("hangup", 1);
 }