svn commit: r196738 - head/sys/netinet

[Bjoern A. Zeeb]

Author: bz
Date: Tue Sep  1 17:53:01 2009
New Revision: 196738
URL: http://svn.freebsd.org/changeset/base/196738

Log:
  In case an upper layer protocol tries to send a packet but the
  L2 code does not have the ethernet address for the destination
  within the broadcast domain in the table, we remember the
  original mbuf in `la_hold' in arpresolve() and send out a
  different packet with an arp request.
  In case there will be more upper layer packets to send we will
  free an earlier one held in `la_hold' and queue the new one.
  
  Once we get a packet in, with which we can perfect our arp table
  entry we send out the original 'on hold' packet, should there
  be any.
  Rather than continuing to process the packet that we received,
  we returned without freeing the packet that came in, which
  basically means that we leaked an mbuf for every arp request
  we sent.
  
  Rather than freeing the received packet and returning, continue
  to process the incoming arp packet as well.
  This should (a) improve some setups, also proxy-arp, in case it was an
  incoming arp request and (b) resembles the behaviour FreeBSD had
  from day 1, which alignes with RFC826 "Packet reception" (merge case).
  
  Rename 'm0' to 'hold' to make the code more understandable as
  well as diffable to earlier versions more easily.
  
  Handle the link-layer entry 'la' lock comepletely in the block
  where needed and release it as early as possible, rather than
  holding it longer, down to the end of the function.
  
  Found by:			pointyhat, ns1
  Bug hunting session with:	erwin, simon, rwatson
  Tested by:			simon on cluster machines
  Reviewed by:			ratson, kmacy, julian
  MFC after:			3 days

Modified:
  head/sys/netinet/if_ether.c

Modified: head/sys/netinet/if_ether.c
==============================================================================
--- head/sys/netinet/if_ether.c	Tue Sep  1 16:41:28 2009	(r196737)
+++ head/sys/netinet/if_ether.c	Tue Sep  1 17:53:01 2009	(r196738)
@@ -462,11 +462,11 @@ in_arpinput(struct mbuf *m)
 	struct rtentry *rt;
 	struct ifaddr *ifa;
 	struct in_ifaddr *ia;
+	struct mbuf *hold;
 	struct sockaddr sa;
 	struct in_addr isaddr, itaddr, myaddr;
 	u_int8_t *enaddr = NULL;
 	int op, flags;
-	struct mbuf *m0;
 	int req_len;
 	int bridged = 0, is_bridge = 0;
 #ifdef DEV_CARP
@@ -631,11 +631,13 @@ match:
 				    la->lle_tbl->llt_ifp->if_xname,
 				    ifp->if_addrlen, (u_char *)ar_sha(ah), ":",
 				    ifp->if_xname);
+			LLE_WUNLOCK(la);
 			goto reply;
 		}
 		if ((la->la_flags & LLE_VALID) &&
 		    bcmp(ar_sha(ah), &la->ll_addr, ifp->if_addrlen)) {
 			if (la->la_flags & LLE_STATIC) {
+				LLE_WUNLOCK(la);
 				log(LOG_ERR,
 				    "arp: %*D attempts to modify permanent "
 				    "entry for %s on %s\n",
@@ -655,6 +657,7 @@ match:
 		}
 		    
 		if (ifp->if_addrlen != ah->ar_hln) {
+			LLE_WUNLOCK(la);
 			log(LOG_WARNING,
 			    "arp from %*D: addr len: new %d, i/f %d (ignored)",
 			    ifp->if_addrlen, (u_char *) ar_sha(ah), ":",
@@ -671,15 +674,14 @@ match:
 		}
 		la->la_asked = 0;
 		la->la_preempt = V_arp_maxtries;
-		if (la->la_hold != NULL) {
-			m0 = la->la_hold;
-			la->la_hold = 0;
+		hold = la->la_hold;
+		if (hold != NULL) {
+			la->la_hold = NULL;
 			memcpy(&sa, L3_ADDR(la), sizeof(sa));
-			LLE_WUNLOCK(la);
-			
-			(*ifp->if_output)(ifp, m0, &sa, NULL);
-			return;
 		}
+		LLE_WUNLOCK(la);
+		if (hold != NULL)
+			(*ifp->if_output)(ifp, hold, &sa, NULL);
 	}
 reply:
 	if (op != ARPOP_REQUEST)
@@ -750,8 +752,6 @@ reply:
 #endif
 	}
 
-	if (la != NULL)
-		LLE_WUNLOCK(la);
 	if (itaddr.s_addr == myaddr.s_addr &&
 	    IN_LINKLOCAL(ntohl(itaddr.s_addr))) {
 		/* RFC 3927 link-local IPv4; always reply by broadcast. */
@@ -777,8 +777,6 @@ reply:
 	return;
 
 drop:
-	if (la != NULL)
-		LLE_WUNLOCK(la);
 	m_freem(m);
 }
 #endif