From owner-svn-src-stable-8@FreeBSD.ORG Wed Sep 2 16:35:57 2009 Return-Path: Delivered-To: svn-src-stable-8@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C3A001065692; Wed, 2 Sep 2009 16:35:57 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id AFE488FC19; Wed, 2 Sep 2009 16:35:57 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id n82GZvNb001553; Wed, 2 Sep 2009 16:35:57 GMT (envelope-from bz@svn.freebsd.org) Received: (from bz@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id n82GZvjh001551; Wed, 2 Sep 2009 16:35:57 GMT (envelope-from bz@svn.freebsd.org) Message-Id: <200909021635.n82GZvjh001551@svn.freebsd.org> From: "Bjoern A. Zeeb" Date: Wed, 2 Sep 2009 16:35:57 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r196770 - in stable/8/sys: . amd64/include/xen cddl/contrib/opensolaris contrib/dev/acpica contrib/pf dev/xen/xenpci netinet X-BeenThere: svn-src-stable-8@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for only the 8-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 02 Sep 2009 16:35:58 -0000 Author: bz Date: Wed Sep 2 16:35:57 2009 New Revision: 196770 URL: http://svn.freebsd.org/changeset/base/196770 Log: MFC r196738: 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 Approved by: re (kib) Modified: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) stable/8/sys/dev/xen/xenpci/ (props changed) stable/8/sys/netinet/if_ether.c Modified: stable/8/sys/netinet/if_ether.c ============================================================================== --- stable/8/sys/netinet/if_ether.c Wed Sep 2 16:02:48 2009 (r196769) +++ stable/8/sys/netinet/if_ether.c Wed Sep 2 16:35:57 2009 (r196770) @@ -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