devip: implement ipv4 arp timeout with icmp host unreachable notification

The IPv4 ARP cache used to indefinitely buffer packets in the Arpent hold list.
This is bad in case of a router, because it opens a 1 second
(retransmit time) window to leak all the to be forwarded packets.

This change makes the ipv4 arp code path similar to the IPv6 neighbour
solicitation path, using the retransmit process to time out old entries
(after 3 arp retransmits => 3 seconds).

A new function arpcontinue() has been added that unifies the point when
we schedule the (ipv6 sol retransmit) / (ipv4 arp timeout) and reduce
the hold queue to the last packet and unlock the cache.

As a bonus, we also now send a icmp host unreachable notification
for the dropped packets.

6 files changed, 131 insertions, 119 deletions

diff --git a/sys/src/9/ip/arp.c b/sys/src/9/ip/arp.c
index 9c1a0cfc7..82e9eb04d 100644
--- a/sys/src/9/ip/arp.c
+++ b/sys/src/9/ip/arp.c
@@ -37,7 +37,7 @@ struct Arp
 	Fs	*f;
 	Arpent	*hash[NHASH];
 	Arpent	cache[NCACHE];
-	Arpent	*rxmt;
+	Arpent	*rxmt[2];
 	Proc	*rxmitp;	/* neib sol re-transmit proc */
 	Rendez	rxmtq;
 	Block 	*dropf, *dropl;
@@ -47,16 +47,15 @@ char *Ebadarp = "bad arp";
 
 #define haship(s) ((s)[IPaddrlen-1]%NHASH)
 
-int 	ReTransTimer = RETRANS_TIMER;
-
-static void 	rxmitproc(void *v);
+static void 	rxmitproc(void*);
 
 void
 arpinit(Fs *f)
 {
 	f->arp = smalloc(sizeof(Arp));
 	f->arp->f = f;
-	f->arp->rxmt = nil;
+	f->arp->rxmt[0] = nil;
+	f->arp->rxmt[1] = nil;
 	f->arp->dropf = f->arp->dropl = nil;
 	kproc("rxmitproc", rxmitproc, f->arp);
 }
@@ -79,7 +78,7 @@ rxmtunchain(Arp *arp, Arpent *a)
 {
 	Arpent **l;
 
-	for(l = &arp->rxmt; *l != nil; l = &((*l)->nextrxt)){
+	for(l = &arp->rxmt[isv4(a->ip) != 0]; *l != nil; l = &((*l)->nextrxt)){
 		if(*l == a){
 			*l = a->nextrxt;
 			break;
@@ -104,26 +103,20 @@ cleanarpent(Arp *arp, Arpent *a)
 	}
 	a->hash = nil;
 
-	/* dump waiting packets */
-	bp = a->hold;
-	a->hold = nil;
-	if(isv4(a->ip))
-		freeblistchain(bp);
-	else {
-		rxmtunchain(arp, a);
-
-		/* queue icmp unreachable for rxmitproc later on, w/o arp lock */
-		if(bp != nil){
-			if(arp->dropf == nil)
-				arp->dropf = bp;
-			else
-				arp->dropl->list = bp;
-			arp->dropl = a->last;
-
-			if(bp == arp->dropf)
-				wakeup(&arp->rxmtq);
-		}
+	/* remove from retransmit / timout chain */
+	rxmtunchain(arp, a);
+
+	/* queue packets for icmp unreachable for rxmitproc later on, w/o arp lock */
+	if((bp = a->hold) != nil){
+		if(arp->dropf == nil)
+			arp->dropf = bp;
+		else
+			arp->dropl->list = bp;
+		arp->dropl = a->last;
+		if(bp == arp->dropf)
+			wakeup(&arp->rxmtq);
 	}
+	a->hold = nil;
 	a->last = nil;
 
 	a->ifc = nil;
@@ -152,7 +145,7 @@ newarpent(Arp *arp, uchar *ip, Ipifc *ifc)
 	e = &arp->cache[NCACHE];
 	a = arp->cache;
 	t = a->utime;
-	for(f = a; f < e; f++){
+	for(f = a+1; t > 0 && f < e; f++){
 		if(f->utime < t){
 			t = f->utime;
 			a = f;
@@ -182,7 +175,6 @@ newarpent(Arp *arp, uchar *ip, Ipifc *ifc)
 Arpent*
 arpget(Arp *arp, Block *bp, int version, Ipifc *ifc, uchar *ip, uchar *mac)
 {
-	int hash;
 	Arpent *a;
 	uchar v6ip[IPaddrlen];
 
@@ -192,8 +184,7 @@ arpget(Arp *arp, Block *bp, int version, Ipifc *ifc, uchar *ip, uchar *mac)
 	}
 
 	qlock(arp);
-	hash = haship(ip);
-	for(a = arp->hash[hash]; a != nil; a = a->hash){
+	for(a = arp->hash[haship(ip)]; a != nil; a = a->hash){
 		if(a->ifc == ifc && a->ifcid == ifc->ifcid && ipcmp(ip, a->ip) == 0)
 			break;
 	}
@@ -225,6 +216,39 @@ arpget(Arp *arp, Block *bp, int version, Ipifc *ifc, uchar *ip, uchar *mac)
 }
 
 /*
+ * continue address resolution for the entry,
+ * schedule it on the retransmit / timeout chains
+ * and unlock the arp cache.
+ */
+void
+arpcontinue(Arp *arp, Arpent *a)
+{
+	Arpent **l;
+	Block *bp;
+
+	/* try to keep it around for a second more */
+	a->ctime = NOW;
+
+	/* remove all but the last message */
+	while((bp = a->hold) != nil){
+		if(bp == a->last)
+			break;
+		a->hold = bp->list;
+		freeblist(bp);
+	}
+
+	/* put on end of re-transmit / timeout chain */
+	for(l = rxmtunchain(arp, a); *l != nil; l = &(*l)->nextrxt)
+		;
+	*l = a;
+
+	if(l == &arp->rxmt[0] || l == &arp->rxmt[1])
+		wakeup(&arp->rxmtq);
+
+	qunlock(arp);
+}
+
+/*
  * called with arp locked
  */
 void
@@ -233,6 +257,7 @@ arprelease(Arp *arp, Arpent*)
 	qunlock(arp);
 }
 
+
 /*
  * Copy out the mac address from the Arpent.  Return the
  * block waiting to get sent to this mac address.
@@ -245,7 +270,7 @@ arpresolve(Arp *arp, Arpent *a, Medium *type, uchar *mac)
 	Block *bp;
 
 	memmove(a->mac, mac, type->maclen);
-	if(a->state == AWAIT && !isv4(a->ip)){
+	if(a->state == AWAIT) {
 		rxmtunchain(arp, a);
 		a->rxtsrem = 0;
 	}
@@ -363,7 +388,7 @@ arpwrite(Fs *fs, char *s, int len)
 		memset(arp->hash, 0, sizeof(arp->hash));
 		freeblistchain(arp->dropf);
 		arp->dropf = arp->dropl = nil;
-		arp->rxmt = nil;
+		arp->rxmt[0] = arp->rxmt[1] = nil;
 		qunlock(arp);
 	} else if(strcmp(f[0], "add") == 0){
 		switch(n){
@@ -480,32 +505,22 @@ void
 ndpsendsol(Fs *f, Ipifc *ifc, Arpent *a)
 {
 	uchar targ[IPaddrlen], src[IPaddrlen];
-	Arpent **l;
 
-	a->ctime = NOW;
 	if(a->rxtsrem == 0)
 		a->rxtsrem = MAX_MULTICAST_SOLICIT;
 	else
 		a->rxtsrem--;
 
-	/* put on end of re-transmit chain */
-	for(l = rxmtunchain(f->arp, a); *l != nil; l = &(*l)->nextrxt)
-		;
-	*l = a;
-
-	if(l == &f->arp->rxmt)
-		wakeup(&f->arp->rxmtq);
-
 	/* try to use source address of original packet */
 	ipmove(targ, a->ip);
 	if(a->last != nil){
 		ipmove(src, ((Ip6hdr*)a->last->rp)->src);
-		arprelease(f->arp, a);
+		arpcontinue(f->arp, a);
 
 		if(iplocalonifc(ifc, src) != nil || ipproxyifc(f, ifc, src))
 			goto send;
 	} else {
-		arprelease(f->arp, a);
+		arpcontinue(f->arp, a);
 	}
 	if(!ipv6local(ifc, src, 0, targ))
 		return;
@@ -516,16 +531,17 @@ send:
 	}
 }
 
-static void
-rxmitsols(Arp *arp)
+static Block*
+rxmt(Arp *arp)
 {
-	Block *next, *bp;
 	Arpent *a;
+	Block *bp;
 	Ipifc *ifc;
-	Route *r;
 
 	qlock(arp);
-	while((a = arp->rxmt) != nil && NOW - a->ctime > 3*ReTransTimer/4){
+
+	/* retransmit ipv6 solicitations */
+	while((a = arp->rxmt[0]) != nil && NOW - a->ctime > 3*RETRANS_TIMER/4){
 		if(a->rxtsrem > 0 && (ifc = a->ifc) != nil && canrlock(ifc)){
 			if(a->ifcid == ifc->ifcid){
 				ndpsendsol(arp->f, ifc, a);	/* unlocks arp */
@@ -537,17 +553,40 @@ rxmitsols(Arp *arp)
 		}
 		cleanarpent(arp, a);
 	}
+
+	/* timeout waiting ipv4 arp entries */
+	while((a = arp->rxmt[1]) != nil && NOW - a->ctime > 3*RETRANS_TIMER)
+		cleanarpent(arp, a);
+
 	bp = arp->dropf;
 	arp->dropf = arp->dropl = nil;
+
 	qunlock(arp);
 
+	return bp;
+}
+
+static void
+drop(Fs *f, Block *bp)
+{
+	Block *next;
+	Ipifc *ifc;
+	Route *r;
+
 	for(; bp != nil; bp = next){
 		next = bp->list;
 		bp->list = nil;
-		r = v6lookup(arp->f, ((Ip6hdr*)bp->rp)->src, ((Ip6hdr*)bp->rp)->dst, nil);
+
+		if((bp->rp[0]&0xF0) == IP_VER4)
+			r = v4lookup(f, ((Ip4hdr*)bp->rp)->src, ((Ip4hdr*)bp->rp)->dst, nil);
+		else
+			r = v6lookup(f, ((Ip6hdr*)bp->rp)->src, ((Ip6hdr*)bp->rp)->dst, nil);
 		if(r != nil && (ifc = r->ifc) != nil && canrlock(ifc)){
 			if(!waserror()){
-				icmphostunr6(arp->f, ifc, bp, Icmp6_adr_unreach, (r->type & Runi) != 0);
+				if((bp->rp[0]&0xF0) == IP_VER4)
+					icmpnohost(f, ifc, bp);
+				else
+					icmphostunr6(f, ifc, bp, Icmp6_adr_unreach, (r->type & Runi) != 0);
 				poperror();
 			}
 			runlock(ifc);
@@ -561,7 +600,7 @@ rxready(void *v)
 {
 	Arp *arp = (Arp *)v;
 
-	return arp->rxmt != nil || arp->dropf != nil;
+	return arp->rxmt[0] != nil || arp->rxmt[1] != nil || arp->dropf != nil;
 }
 
 static void
@@ -576,7 +615,7 @@ rxmitproc(void *v)
 	}
 	for(;;){
 		sleep(&arp->rxmtq, rxready, v);
-		rxmitsols(arp);
-		tsleep(&arp->rxmtq, return0, nil, ReTransTimer/4);
+		drop(arp->f, rxmt(arp));
+		tsleep(&arp->rxmtq, return0, nil, RETRANS_TIMER/4);
 	}
 }
diff --git a/sys/src/9/ip/ethermedium.c b/sys/src/9/ip/ethermedium.c
index 2bb022dcf..0bcd0c165 100644
--- a/sys/src/9/ip/ethermedium.c
+++ b/sys/src/9/ip/ethermedium.c
@@ -17,15 +17,6 @@ struct Etherhdr
 	uchar	t[2];
 };
 
-static uchar ipbroadcast[IPaddrlen] = {
-	0xff,0xff,0xff,0xff,
-	0xff,0xff,0xff,0xff,
-	0xff,0xff,0xff,0xff,
-	0xff,0xff,0xff,0xff,
-};
-
-static uchar etherbroadcast[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
-
 static void	etherread4(void *a);
 static void	etherread6(void *a);
 static void	etherbind(Ipifc *ifc, int argc, char **argv);
@@ -35,8 +26,7 @@ static void	etheraddmulti(Ipifc *ifc, uchar *a, uchar *ia);
 static void	etherremmulti(Ipifc *ifc, uchar *a, uchar *ia);
 static void	etherareg(Fs *f, Ipifc *ifc, Iplifc *lifc, uchar *ip);
 static Block*	multicastarp(Fs *f, Arpent *a, Medium*, uchar *mac);
-static void	sendarp(Ipifc *ifc, Arpent *a);
-static void	sendndp(Ipifc *ifc, Arpent *a);
+static void	sendarpreq(Ipifc *ifc, Arpent *a);
 static int	multicastea(uchar *ea, uchar *ip);
 static void	recvarpproc(void*);
 static void	etherpref2addr(uchar *pref, uchar *ea);
@@ -276,12 +266,17 @@ etherbwrite(Ipifc *ifc, Block *bp, int version, uchar *ip)
 		/* check for broadcast or multicast */
 		bp = multicastarp(er->f, a, ifc->m, mac);
 		if(bp == nil){
+			/* don't do anything if it's been less than a second since the last */
+			if(NOW - a->ctime < RETRANS_TIMER){
+				arprelease(er->f->arp, a);
+				return;
+			}
 			switch(version){
 			case V4:
-				sendarp(ifc, a);
+				sendarpreq(ifc, a);		/* unlocks arp */
 				break;
 			case V6:
-				sendndp(ifc, a);
+				ndpsendsol(er->f, ifc, a);	/* unlocks arp */
 				break;
 			default:
 				panic("etherbwrite: version %d", version);
@@ -442,7 +437,7 @@ etherremmulti(Ipifc *ifc, uchar *a, uchar *)
  *  (only v4, v6 uses the neighbor discovery, rfc1970)
  */
 static void
-sendarp(Ipifc *ifc, Arpent *a)
+sendarpreq(Ipifc *ifc, Arpent *a)
 {
 	int n;
 	Block *bp;
@@ -450,25 +445,8 @@ sendarp(Ipifc *ifc, Arpent *a)
 	Etherrock *er = ifc->arg;
 	uchar targ[IPv4addrlen], src[IPv4addrlen];
 
-	/* don't do anything if it's been less than a second since the last */
-	if(NOW - a->ctime < 1000){
-		arprelease(er->f->arp, a);
-		return;
-	}
-
-	/* try to keep it around for a second more */
-	a->ctime = NOW;
-
-	/* remove all but the last message */
-	while((bp = a->hold) != nil){
-		if(bp == a->last)
-			break;
-		a->hold = bp->list;
-		freeblist(bp);
-	}
-
 	memmove(targ, a->ip+IPv4off, IPv4addrlen);
-	arprelease(er->f->arp, a);
+	arpcontinue(er->f->arp, a);
 
 	if(!ipv4local(ifc, src, 0, targ))
 		return;
@@ -477,8 +455,8 @@ sendarp(Ipifc *ifc, Arpent *a)
 	if(n < ifc->m->mintu)
 		n = ifc->m->mintu;
 	bp = allocb(n);
-	memset(bp->rp, 0, n);
 	e = (Etherarp*)bp->rp;
+	memset(e, 0, n);
 	memmove(e->tpa, targ, sizeof(e->tpa));
 	memmove(e->spa, src, sizeof(e->spa));
 	memmove(e->sha, ifc->mac, sizeof(e->sha));
@@ -496,29 +474,6 @@ sendarp(Ipifc *ifc, Arpent *a)
 	devtab[er->achan->type]->bwrite(er->achan, bp, 0);
 }
 
-static void
-sendndp(Ipifc *ifc, Arpent *a)
-{
-	Block *bp;
-	Etherrock *er = ifc->arg;
-
-	/* don't do anything if it's been less than a second since the last */
-	if(NOW - a->ctime < ReTransTimer){
-		arprelease(er->f->arp, a);
-		return;
-	}
-
-	/* remove all but the last message */
-	while((bp = a->hold) != nil){
-		if(bp == a->last)
-			break;
-		a->hold = bp->list;
-		freeblist(bp);
-	}
-
-	ndpsendsol(er->f, ifc, a);	/* unlocks arp */
-}
-
 /*
  *  send a gratuitous arp to refresh arp caches
  */
@@ -534,8 +489,8 @@ sendgarp(Ipifc *ifc, uchar *ip)
 	if(n < ifc->m->mintu)
 		n = ifc->m->mintu;
 	bp = allocb(n);
-	memset(bp->rp, 0, n);
 	e = (Etherarp*)bp->rp;
+	memset(e, 0, n);
 	memmove(e->tpa, ip+IPv4off, sizeof(e->tpa));
 	memmove(e->spa, ip+IPv4off, sizeof(e->spa));
 	memmove(e->sha, ifc->mac, sizeof(e->sha));
diff --git a/sys/src/9/ip/icmp.c b/sys/src/9/ip/icmp.c
index 93f0cd173..b17054c4c 100644
--- a/sys/src/9/ip/icmp.c
+++ b/sys/src/9/ip/icmp.c
@@ -242,22 +242,34 @@ icmpttlexceeded(Fs *f, Ipifc *ifc, Block *bp)
 }
 
 static void
-icmpunreachable(Fs *f, Block *bp, int code, int seq)
+icmpunreachable(Fs *f, Ipifc *ifc, Block *bp, int code, int seq)
 {
 	Block	*nbp;
 	Icmp	*p, *np;
+	uchar	ia[IPv4addrlen];
 
 	p = (Icmp *)bp->rp;
-	if(!ip4me(f, p->dst) || !ip4reply(f, p->src))
+	if(!ip4reply(f, p->src))
 		return;
 
-	netlog(f, Logicmp, "sending icmpnoconv -> %V\n", p->src);
+	if(ifc == nil){
+		if(!ipforme(f, p->dst))
+			return;
+		memmove(ia, p->dst, sizeof(p->dst));
+	} else {
+		if(!ipv4local(ifc, ia, 0, p->src))
+			return;
+	}
+
+	netlog(f, Logicmp, "sending icmpunreachable %V -> src %V dst %V\n",
+		ia, p->src, p->dst);
+
 	nbp = allocb(ICMP_IPSIZE + ICMP_HDRSIZE + ICMP_IPSIZE + 8);
 	nbp->wp += ICMP_IPSIZE + ICMP_HDRSIZE + ICMP_IPSIZE + 8;
 	np = (Icmp *)nbp->rp;
 	np->vihl = IP_VER4;
+	memmove(np->src, ia, sizeof(np->src));
 	memmove(np->dst, p->src, sizeof(np->dst));
-	memmove(np->src, p->dst, sizeof(np->src));
 	memmove(np->data, bp->rp, ICMP_IPSIZE + 8);
 	np->type = Unreachable;
 	np->code = code;
@@ -270,15 +282,21 @@ icmpunreachable(Fs *f, Block *bp, int code, int seq)
 }
 
 void
+icmpnohost(Fs *f, Ipifc *ifc, Block *bp)
+{
+	icmpunreachable(f, ifc, bp, 1, 0);
+}
+
+void
 icmpnoconv(Fs *f, Block *bp)
 {
-	icmpunreachable(f, bp, 3, 0);
+	icmpunreachable(f, nil, bp, 3, 0);
 }
 
 void
 icmpcantfrag(Fs *f, Block *bp, int mtu)
 {
-	icmpunreachable(f, bp, 4, mtu);
+	icmpunreachable(f, nil, bp, 4, mtu);
 }
 
 static void
diff --git a/sys/src/9/ip/ip.h b/sys/src/9/ip/ip.h
index 01b294d67..c3be02f6b 100644
--- a/sys/src/9/ip/ip.h
+++ b/sys/src/9/ip/ip.h
@@ -613,6 +613,7 @@ extern int	arpread(Arp*, char*, ulong, int);
 extern int	arpwrite(Fs*, char*, int);
 extern Arpent*	arpget(Arp*, Block *bp, int version, Ipifc *ifc, uchar *ip, uchar *h);
 extern void	arprelease(Arp*, Arpent *a);
+extern void	arpcontinue(Arp*, Arpent *a);
 extern Block*	arpresolve(Arp*, Arpent *a, Medium *type, uchar *mac);
 extern int	arpenter(Fs*, int version, uchar *ip, uchar *mac, int n, uchar *ia, Ipifc *ifc, int refresh);
 extern void	ndpsendsol(Fs*, Ipifc*, Arpent*);
@@ -682,6 +683,7 @@ extern char*	ipifcremove6(Ipifc *ifc, char**argv, int argc);
  *  ip.c
  */
 extern void	iprouting(Fs*, int);
+extern void	icmpnohost(Fs*, Ipifc*, Block*);
 extern void	icmpnoconv(Fs*, Block*);
 extern void	icmpcantfrag(Fs*, Block*, int);
 extern void	icmpttlexceeded(Fs*, Ipifc*, Block*);
diff --git a/sys/src/9/ip/ipv6.c b/sys/src/9/ip/ipv6.c
index 1d8750cde..779499a47 100644
--- a/sys/src/9/ip/ipv6.c
+++ b/sys/src/9/ip/ipv6.c
@@ -32,7 +32,7 @@ ip_init_6(Fs *f)
 	v6p->rp.ttl		= MAXTTL;
 	v6p->rp.routerlt	= (3 * v6p->rp.maxraint) / 1000;
 
-	v6p->hp.rxmithost	= 1000;		/* v6 RETRANS_TIMER */
+	v6p->hp.rxmithost	= RETRANS_TIMER;
 
 	f->v6p			= v6p;
 }
diff --git a/sys/src/9/ip/ipv6.h b/sys/src/9/ip/ipv6.h
index 489d8903c..bbf4cacaf 100644
--- a/sys/src/9/ip/ipv6.h
+++ b/sys/src/9/ip/ipv6.h
@@ -178,8 +178,6 @@ extern int v6snpreflen;
 extern int v6aNpreflen;
 extern int v6aLpreflen;
 
-extern int ReTransTimer;
-
 void ipv62smcast(uchar *, uchar *);
 void icmpns(Fs *f, uchar* src, int suni, uchar* targ, int tuni, uchar* mac);
 void icmpna(Fs *f, uchar* src, uchar* dst, uchar* targ, uchar* mac, uchar flags);