From owner-freebsd-net@FreeBSD.ORG Fri Mar 9 09:30:12 2012 Return-Path: Delivered-To: freebsd-net@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D63DD106566C for ; Fri, 9 Mar 2012 09:30:12 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id C22908FC18 for ; Fri, 9 Mar 2012 09:30:12 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q299UCH2057953 for ; Fri, 9 Mar 2012 09:30:12 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id q299UCPX057950; Fri, 9 Mar 2012 09:30:12 GMT (envelope-from gnats) Date: Fri, 9 Mar 2012 09:30:12 GMT Message-Id: <201203090930.q299UCPX057950@freefall.freebsd.org> To: freebsd-net@FreeBSD.org From: Gleb Smirnoff Cc: Subject: kern/165863 X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Gleb Smirnoff List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 09 Mar 2012 09:30:12 -0000 The following reply was made to PR kern/165863; it has been noted by GNATS. From: Gleb Smirnoff To: Eric van Gyzen , Eric van Gyzen , emaste@FreeBSD.org Cc: bug-followup@FreeBSD.org Subject: kern/165863 Date: Fri, 9 Mar 2012 13:20:56 +0400 --BXVAT5kNtrzKuDFl Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Hello, Eric and Ed. Can you look at this patch? I decided to utilize newer callout API, that allows to delegate lock retrieval to the callout subsystem, and this makes things simplier. Hope that should work. Patch is against head. Eric, can you please send me your test programs, that you use to reproduce the bug? -- Totus tuus, Glebius. --BXVAT5kNtrzKuDFl Content-Type: text/x-diff; charset=koi8-r Content-Disposition: attachment; filename="kern165863.diff" Index: in.c =================================================================== --- in.c (revision 232689) +++ in.c (working copy) @@ -1279,11 +1279,12 @@ { struct in_llentry *lle; - lle = malloc(sizeof(struct in_llentry), M_LLTABLE, M_DONTWAIT | M_ZERO); + lle = malloc(sizeof(struct in_llentry), M_LLTABLE, M_NOWAIT | M_ZERO); if (lle == NULL) /* NB: caller generates msg */ return NULL; - callout_init(&lle->base.la_timer, CALLOUT_MPSAFE); + LLE_LOCK_INIT(&lle->base); + callout_init_rw(&lle->base.la_timer, &lle->base.lle_lock, 0); /* * For IPv4 this will trigger "arpresolve" to generate * an ARP request. @@ -1292,46 +1293,44 @@ lle->l3_addr4 = *(const struct sockaddr_in *)l3addr; lle->base.lle_refcnt = 1; lle->base.lle_free = in_lltable_free; - LLE_LOCK_INIT(&lle->base); - return &lle->base; + + return (&lle->base); } #define IN_ARE_MASKED_ADDR_EQUAL(d, a, m) ( \ (((ntohl((d)->sin_addr.s_addr) ^ (a)->sin_addr.s_addr) & (m)->sin_addr.s_addr)) == 0 ) static void -in_lltable_prefix_free(struct lltable *llt, - const struct sockaddr *prefix, - const struct sockaddr *mask, - u_int flags) +in_lltable_prefix_free(struct lltable *llt, const struct sockaddr *prefix, + const struct sockaddr *mask, u_int flags) { const struct sockaddr_in *pfx = (const struct sockaddr_in *)prefix; const struct sockaddr_in *msk = (const struct sockaddr_in *)mask; struct llentry *lle, *next; - register int i; + int i; size_t pkts_dropped; - for (i=0; i < LLTBL_HASHTBL_SIZE; i++) { + IF_AFDATA_WLOCK(llt->llt_ifp); + for (i = 0; i < LLTBL_HASHTBL_SIZE; i++) { LIST_FOREACH_SAFE(lle, &llt->lle_head[i], lle_next, next) { /* * (flags & LLE_STATIC) means deleting all entries - * including static ARP entries + * including static ARP entries. */ - if (IN_ARE_MASKED_ADDR_EQUAL((struct sockaddr_in *)L3_ADDR(lle), - pfx, msk) && - ((flags & LLE_STATIC) || !(lle->la_flags & LLE_STATIC))) { - int canceled; + if (IN_ARE_MASKED_ADDR_EQUAL(satosin(L3_ADDR(lle)), + pfx, msk) && ((flags & LLE_STATIC) || + !(lle->la_flags & LLE_STATIC))) { - canceled = callout_drain(&lle->la_timer); LLE_WLOCK(lle); - if (canceled) + if (callout_stop(&lle->la_timer)) LLE_REMREF(lle); pkts_dropped = llentry_free(lle); ARPSTAT_ADD(dropped, pkts_dropped); } } } + IF_AFDATA_WUNLOCK(llt->llt_ifp); } Index: if_ether.c =================================================================== --- if_ether.c (revision 232689) +++ if_ether.c (working copy) @@ -163,38 +163,23 @@ static void arptimer(void *arg) { + struct llentry *lle = (struct llentry *)arg; struct ifnet *ifp; - struct llentry *lle; - int pkts_dropped; + size_t pkts_dropped; - KASSERT(arg != NULL, ("%s: arg NULL", __func__)); - lle = (struct llentry *)arg; + LLE_WLOCK_ASSERT(lle); + + if (lle->la_flags & LLE_STATIC) + return; + ifp = lle->lle_tbl->llt_ifp; CURVNET_SET(ifp->if_vnet); - IF_AFDATA_LOCK(ifp); - LLE_WLOCK(lle); - if (lle->la_flags & LLE_STATIC) - LLE_WUNLOCK(lle); - else { - if (!callout_pending(&lle->la_timer) && - callout_active(&lle->la_timer)) { - callout_stop(&lle->la_timer); - LLE_REMREF(lle); - pkts_dropped = llentry_free(lle); - ARPSTAT_ADD(dropped, pkts_dropped); - ARPSTAT_INC(timeouts); - } else { -#ifdef DIAGNOSTIC - struct sockaddr *l3addr = L3_ADDR(lle); - log(LOG_INFO, - "arptimer issue: %p, IPv4 address: \"%s\"\n", lle, - inet_ntoa( - ((const struct sockaddr_in *)l3addr)->sin_addr)); -#endif - LLE_WUNLOCK(lle); - } - } - IF_AFDATA_UNLOCK(ifp); + + LLE_REMREF(lle); + pkts_dropped = llentry_free(lle); + ARPSTAT_ADD(dropped, pkts_dropped); + ARPSTAT_INC(timeouts); + CURVNET_RESTORE(); } --BXVAT5kNtrzKuDFl--