From owner-freebsd-net@FreeBSD.ORG  Fri Nov  9 16:43:59 2012
Return-Path: <owner-freebsd-net@FreeBSD.ORG>
Delivered-To: freebsd-net@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52])
 by hub.freebsd.org (Postfix) with ESMTP id 975F23AA
 for <freebsd-net@freebsd.org>; Fri,  9 Nov 2012 16:43:59 +0000 (UTC)
 (envelope-from if@xip.at)
Received: from chile.gbit.at (ns1.xip.at [193.239.188.99])
 by mx1.freebsd.org (Postfix) with ESMTP id AB9858FC12
 for <freebsd-net@freebsd.org>; Fri,  9 Nov 2012 16:43:57 +0000 (UTC)
Received: (qmail 32289 invoked from network); 9 Nov 2012 17:43:51 +0100
Received: from fw.xip.at (HELO ?127.0.0.1?) (89.207.145.147)
 by chile.gbit.at with SMTP; 9 Nov 2012 17:43:51 +0100
Message-ID: <509D32CC.6000201@xip.at>
Date: Fri, 09 Nov 2012 17:43:56 +0100
From: Ingo Flaschberger <if@xip.at>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64;
 rv:16.0) Gecko/20121010 Thunderbird/16.0.1
MIME-Version: 1.0
To: freebsd-net@freebsd.org
Subject: Re: [patch] reducing arp locking
References: <509AEDAC.10002@FreeBSD.org> <509B884F.7040106@networx.ch>
 <509B88B1.3070905@FreeBSD.org>
 <49EE4F42-6162-40F4-9DE0-1ACA1289B225@netasq.com>
 <509CC776.9010200@FreeBSD.org>
 <37E1F76F-D951-4B36-AF00-039DA1CC5CF3@netasq.com>
 <509CE6A2.2040200@FreeBSD.org>
 <8169CD67-E444-46FC-A7C8-DD6FB59091E1@netasq.com>
In-Reply-To: <8169CD67-E444-46FC-A7C8-DD6FB59091E1@netasq.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
X-Content-Filtered-By: Mailman/MimeDel 2.1.14
X-BeenThere: freebsd-net@freebsd.org
X-Mailman-Version: 2.1.14
Precedence: list
List-Id: Networking and TCP/IP with FreeBSD <freebsd-net.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/freebsd-net>,
 <mailto:freebsd-net-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-net>
List-Post: <mailto:freebsd-net@freebsd.org>
List-Help: <mailto:freebsd-net-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-net>,
 <mailto:freebsd-net-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 09 Nov 2012 16:43:59 -0000

Am 09.11.2012 15:03, schrieb Fabien Thomas:
> In in_arpinput only exclusive access to the entry is taken during the update no IF_AFDATA_LOCK that's why i was surprised.

what about this:
--
--- /usr/src/sys/netinet/if_ether.c_org 2012-11-09 16:15:43.000000000 +0000
+++ /usr/src/sys/netinet/if_ether.c     2012-11-09 16:16:37.000000000 +0000
@@ -685,7 +685,7 @@
         flags |= LLE_EXCLUSIVE;
         IF_AFDATA_LOCK(ifp);
         la = lla_lookup(LLTABLE(ifp), flags, (struct sockaddr *)&sin);
-       IF_AFDATA_UNLOCK(ifp);
+
         if (la != NULL) {
                 /* the following is not an error when doing bridging */
                 if (!bridged && la->lle_tbl->llt_ifp != ifp && 
!carp_match) {
@@ -697,12 +697,14 @@
                                     ifp->if_addrlen, (u_char 
*)ar_sha(ah), ":",
                                     ifp->if_xname);
                         LLE_WUNLOCK(la);
+                       IF_AFDATA_UNLOCK(ifp);
                         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);
+                               IF_AFDATA_UNLOCK(ifp);
                                 if (log_arp_permanent_modify)
                                         log(LOG_ERR,
                                             "arp: %*D attempts to modify "
@@ -725,6 +727,7 @@

                 if (ifp->if_addrlen != ah->ar_hln) {
                         LLE_WUNLOCK(la);
+                       IF_AFDATA_UNLOCK(ifp);
                         log(LOG_WARNING, "arp from %*D: addr len: new %d, "
                             "i/f %d (ignored)\n", ifp->if_addrlen,
                             (u_char *) ar_sha(ah), ":", ah->ar_hln,
@@ -763,14 +766,19 @@
                         la->la_numheld = 0;
                         memcpy(&sa, L3_ADDR(la), sizeof(sa));
                         LLE_WUNLOCK(la);
+                       IF_AFDATA_UNLOCK(ifp);
                         for (; m_hold != NULL; m_hold = m_hold_next) {
                                 m_hold_next = m_hold->m_nextpkt;
                                 m_hold->m_nextpkt = NULL;
                                 (*ifp->if_output)(ifp, m_hold, &sa, NULL);
                         }
-               } else
+               } else {
                         LLE_WUNLOCK(la);
-       }
+                       IF_AFDATA_UNLOCK(ifp);
+                }
+       } else {
+               IF_AFDATA_UNLOCK(ifp);
+        }
  reply:
         if (op != ARPOP_REQUEST)
                 goto drop;
--

Kind regards,
     Ingo Flaschberger