Date: Sat, 22 Nov 2008 01:30:20 GMT From: Qing Li <qingli@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 153315 for review Message-ID: <200811220130.mAM1UKon030837@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=153315 Change 153315 by qingli@FreeBSD-newarp on 2008/11/22 01:29:34 1. fixed the callout issue where enabling callout code for timing out entries would trigger kernel panic 2. introduced a mutex for handling callout 3. fixed the interface table locking issue, the locking code was actually disabled (oops!) 4. fixed the recursive-lock panic, typo (oops!) Unit testing: 1. running "ping x.x.x.255" and "ping6 fe80::???" continuous that used to cause kernel panics immediately 2. continuous running "netperf" TCP_STREAM tests, which also triggered kernel panics immediately Affected files ... .. //depot/projects/arp-v2/src/sys/net/if_llatbl.c#3 edit .. //depot/projects/arp-v2/src/sys/net/if_llatbl.h#2 edit .. //depot/projects/arp-v2/src/sys/net/if_var.h#7 edit .. //depot/projects/arp-v2/src/sys/net/route.c#11 edit .. //depot/projects/arp-v2/src/sys/netinet/if_ether.c#14 edit .. //depot/projects/arp-v2/src/sys/netinet6/nd6.c#7 edit .. //depot/projects/arp-v2/src/sys/netinet6/nd6_nbr.c#6 edit Differences ... ==== //depot/projects/arp-v2/src/sys/net/if_llatbl.c#3 (text+ko) ==== @@ -185,6 +185,7 @@ KASSERT(lle != NULL, ("%s: lle is NULL", __func__)); LIST_REMOVE(lle, lle_next); + IF_LLE_LOCK_DESTROY(lle); if (lle->la_hold) m_freem(lle->la_hold); uma_zfree(llezone, lle); @@ -362,6 +363,10 @@ log(LOG_INFO, "lla_lookup: new lle malloc failed\n"); return (NULL); } + + IF_LLE_LOCK_INIT(lle); + callout_init_mtx(&lle->la_timer, &lle->lle_mtx, 0); + /* qing * For IPv4 this will trigger "arpresolve" to generate * an ARP request ==== //depot/projects/arp-v2/src/sys/net/if_llatbl.h#2 (text+ko) ==== @@ -60,6 +60,7 @@ struct callout ln_timer_ch; struct callout la_timer; } lle_timer; + struct mtx lle_mtx; /* mutex for lle entry */ }; #define ln_timer_ch lle_timer.ln_timer_ch @@ -97,6 +98,12 @@ #define LLATBL_HASH(key, mask) (((((((key >> 8) ^ key) >> 8) ^ key) >> 8) ^ key) & mask) +#define IF_LLE_LOCK_INIT(lle) mtx_init(&(lle)->lle_mtx, \ + "if_llentry_mtx", NULL, MTX_DEF | MTX_RECURSE) +#define IF_LLE_LOCK_DESTROY(lle) mtx_destroy(&(lle)->lle_mtx) +#define IF_LLE_LOCK(lle) mtx_lock(&(lle)->lle_mtx) +#define IF_LLE_UNLOCK(lle) mtx_unlock(&(lle)->lle_mtx) + extern struct llentry *lla_lookup(struct ifnet *ifp, u_int flags, struct sockaddr *l3addr); extern int lla_rt_output(struct rt_msghdr *rtm, struct rt_addrinfo *info); extern int llentry_free(struct llentry *lle); ==== //depot/projects/arp-v2/src/sys/net/if_var.h#7 (text+ko) ==== @@ -244,13 +244,8 @@ #define IF_LLTBLS_LOCK_INIT(if) mtx_init(&(if)->if_lltbls_mtx, \ "if_lltbls_mtx", NULL, MTX_DEF | MTX_RECURSE) #define IF_LLTBLS_LOCK_DESTROY(if) mtx_destroy(&(if)->if_lltbls_mtx) -#if 0 #define IF_LLTBLS_LOCK(if) mtx_lock(&(if)->if_lltbls_mtx) #define IF_LLTBLS_UNLOCK(if) mtx_unlock(&(if)->if_lltbls_mtx) -#else -#define IF_LLTBLS_LOCK(if) -#define IF_LLTBLS_UNLOCK(if) -#endif #define IF_LLTBLS_LOCK_ASSERT(if) mtx_assert(&(if)->if_lltbls_mtx, MA_OWNED) ==== //depot/projects/arp-v2/src/sys/net/route.c#11 (text+ko) ==== @@ -41,6 +41,7 @@ #include <sys/param.h> #include <sys/systm.h> +#include <sys/syslog.h> #include <sys/malloc.h> #include <sys/mbuf.h> #include <sys/socket.h> @@ -379,12 +380,7 @@ */ RT_REMREF(rt); if (rt->rt_refcnt > 0) { - printf("%s: %p has %lu refs\t", __func__, rt, rt->rt_refcnt); - printf("route dst = %s", inet_ntoa(((struct sockaddr_in *)rt_key(rt))->sin_addr)); - if (rt->rt_flags & RTF_GATEWAY) - printf(", gw = %s\n", inet_ntoa(((struct sockaddr_in *)rt->rt_gateway)->sin_addr)); - else - printf("\n"); + log(LOG_DEBUG, "%s: %p has %lu refs\t", __func__, rt, rt->rt_refcnt); goto done; } @@ -1613,7 +1609,6 @@ * we don't actually need a reference. */ RT_REMREF(rt); - printf("rtref = %ld\n", rt->rt_refcnt); } RT_UNLOCK(rt); } ==== //depot/projects/arp-v2/src/sys/netinet/if_ether.c#14 (text+ko) ==== @@ -149,12 +149,18 @@ struct ifnet *ifp; struct llentry *lle = (struct llentry *)arg; + if (lle == NULL) { + panic("arptimer: NULL entry!\n"); + return; + } ifp = lle->lle_tbl->llt_ifp; IF_LLTBLS_LOCK(ifp); if ((lle->la_flags & LLE_DELETED) && !(lle->la_flags & LLE_STATIC)) { - callout_stop(&lle->la_timer); - (void)llentry_free(lle); + if (!callout_pending(&lle->la_timer) && + (callout_active(&lle->la_timer))) { + (void)llentry_free(lle); + } } IF_LLTBLS_UNLOCK(ifp); } @@ -278,8 +284,6 @@ m_freem(m); return (EINVAL); } - if (flags & LLE_CREATE) - callout_init(&la->la_timer, 0); if (la->la_flags & LLE_VALID && (la->la_flags & LLE_STATIC || la->la_expire > time_uptime)) { @@ -574,9 +578,6 @@ goto reply; } - if (flag & LLE_CREATE) - callout_init(&la->la_timer, 0); - if (la->la_flags & LLE_VALID && bcmp(ar_sha(ah), &la->ll_addr, ifp->if_addrlen)) { if (la->la_flags & LLE_STATIC) { @@ -743,7 +744,6 @@ IF_LLTBLS_LOCK(ifp); lle = lla_lookup(ifp, (LLE_CREATE | LLE_IFADDR | LLE_STATIC), (struct sockaddr *)IA_SIN(ifa)); - callout_init(&lle->la_timer, 0); IF_LLTBLS_UNLOCK(ifp); if (lle == NULL) log(LOG_INFO, "arp_ifinit: cannot create arp " ==== //depot/projects/arp-v2/src/sys/netinet6/nd6.c#7 (text+ko) ==== @@ -1043,7 +1043,7 @@ { INIT_VNET_INET6(curvnet); struct llentry *ln; -/* struct ifnet *ifp = rt->rt_ifp;*/ + struct ifnet *ifp = rt->rt_ifp; if (dst6 == NULL) return; ==== //depot/projects/arp-v2/src/sys/netinet6/nd6_nbr.c#6 (text+ko) ==== @@ -879,7 +879,7 @@ nd6_output(ifp, ifp, m_hold, &ln->l3_addr6, NULL); } } - IF_LLTBLS_LOCK(ifp); + IF_LLTBLS_UNLOCK(ifp); freeit: m_freem(m);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200811220130.mAM1UKon030837>