From owner-svn-src-projects@FreeBSD.ORG Sat Dec 13 10:19:29 2008 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A1DED1065672; Sat, 13 Dec 2008 10:19:29 +0000 (UTC) (envelope-from kmacy@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 8EFEA8FC13; Sat, 13 Dec 2008 10:19:29 +0000 (UTC) (envelope-from kmacy@FreeBSD.org) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id mBDAJTWJ025871; Sat, 13 Dec 2008 10:19:29 GMT (envelope-from kmacy@svn.freebsd.org) Received: (from kmacy@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id mBDAJTkF025863; Sat, 13 Dec 2008 10:19:29 GMT (envelope-from kmacy@svn.freebsd.org) Message-Id: <200812131019.mBDAJTkF025863@svn.freebsd.org> From: Kip Macy Date: Sat, 13 Dec 2008 10:19:29 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org X-SVN-Group: projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r186035 - in projects/arpv2_merge_1/sys: amd64/conf net netinet netinet6 X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Dec 2008 10:19:29 -0000 Author: kmacy Date: Sat Dec 13 10:19:28 2008 New Revision: 186035 URL: http://svn.freebsd.org/changeset/base/186035 Log: - add AFDATA and LLE lock asserts - fix LLE_REMREF - add nd6_llinfo_settimer_locked and nd6_output_lle for case where lle lock is already held - remove inappropriate AFDATA lock in ip6_forward - change "Qing" to "XXX QL" Modified: projects/arpv2_merge_1/sys/amd64/conf/GENERIC projects/arpv2_merge_1/sys/net/if_llatbl.h projects/arpv2_merge_1/sys/netinet/in.c projects/arpv2_merge_1/sys/netinet6/in6.c projects/arpv2_merge_1/sys/netinet6/ip6_forward.c projects/arpv2_merge_1/sys/netinet6/nd6.c projects/arpv2_merge_1/sys/netinet6/nd6.h projects/arpv2_merge_1/sys/netinet6/nd6_nbr.c Modified: projects/arpv2_merge_1/sys/amd64/conf/GENERIC ============================================================================== --- projects/arpv2_merge_1/sys/amd64/conf/GENERIC Sat Dec 13 09:33:03 2008 (r186034) +++ projects/arpv2_merge_1/sys/amd64/conf/GENERIC Sat Dec 13 10:19:28 2008 (r186035) @@ -313,3 +313,5 @@ device fwe # Ethernet over FireWire (n device fwip # IP over FireWire (RFC 2734,3146) device dcons # Dumb console driver device dcons_crom # Configuration ROM for dcons + +options ALT_BREAK_TO_DEBUGGER Modified: projects/arpv2_merge_1/sys/net/if_llatbl.h ============================================================================== --- projects/arpv2_merge_1/sys/net/if_llatbl.h Sat Dec 13 09:33:03 2008 (r186034) +++ projects/arpv2_merge_1/sys/net/if_llatbl.h Sat Dec 13 10:19:28 2008 (r186035) @@ -94,9 +94,9 @@ struct llentry { #define LLE_REMREF(lle) do { \ LLE_WLOCK_ASSERT(lle); \ - KASSERT((lle)->rt_refcnt > 0, \ - ("bogus refcnt %ld", (lle)->rt_refcnt)); \ - (lle)->rt_refcnt--; \ + KASSERT((lle)->lle_refcnt > 1, \ + ("bogus refcnt %d", (lle)->lle_refcnt)); \ + (lle)->lle_refcnt--; \ } while (0) #define LLE_FREE_LOCKED(lle) do { \ Modified: projects/arpv2_merge_1/sys/netinet/in.c ============================================================================== --- projects/arpv2_merge_1/sys/netinet/in.c Sat Dec 13 09:33:03 2008 (r186034) +++ projects/arpv2_merge_1/sys/netinet/in.c Sat Dec 13 10:19:28 2008 (r186035) @@ -1097,6 +1097,7 @@ in_lltable_lookup(struct lltable *llt, u struct llentries *lleh; u_int hashkey; + IF_AFDATA_LOCK_ASSERT(ifp); KASSERT(l3addr->sa_family == AF_INET, ("sin_family %d", l3addr->sa_family)); Modified: projects/arpv2_merge_1/sys/netinet6/in6.c ============================================================================== --- projects/arpv2_merge_1/sys/netinet6/in6.c Sat Dec 13 09:33:03 2008 (r186034) +++ projects/arpv2_merge_1/sys/netinet6/in6.c Sat Dec 13 10:19:28 2008 (r186035) @@ -1533,7 +1533,7 @@ in6_ifinit(struct ifnet *ifp, struct in6 * XXX: the logic below rejects assigning multiple addresses on a p2p * interface that share the same destination. */ -#if 0 /* QING - verify */ +#if 0 /* QL - verify */ plen = in6_mask2len(&ia->ia_prefixmask.sin6_addr, NULL); /* XXX */ if (!(ia->ia_flags & IFA_ROUTE) && plen == 128 && ia->ia_dstaddr.sin6_family == AF_INET6) { @@ -1587,7 +1587,7 @@ in6_ifinit(struct ifnet *ifp, struct in6 IF_AFDATA_LOCK(ifp); ia->ia_ifa.ifa_rtrequest = NULL; - /* Qing + /* XXX QL * we need to report rt_newaddrmsg */ ln = lla_lookup(LLTABLE6(ifp), (LLE_CREATE | LLE_IFADDR | LLE_EXCLUSIVE), @@ -2162,6 +2162,7 @@ in6_lltable_lookup(struct lltable *llt, struct llentries *lleh; u_int hashkey; + IF_AFDATA_LOCK_ASSERT(ifp); KASSERT(l3addr->sa_family == AF_INET6, ("sin_family %d", l3addr->sa_family)); Modified: projects/arpv2_merge_1/sys/netinet6/ip6_forward.c ============================================================================== --- projects/arpv2_merge_1/sys/netinet6/ip6_forward.c Sat Dec 13 09:33:03 2008 (r186034) +++ projects/arpv2_merge_1/sys/netinet6/ip6_forward.c Sat Dec 13 10:19:28 2008 (r186035) @@ -610,9 +610,7 @@ ip6_forward(struct mbuf *m, int srcrt) ip6 = mtod(m, struct ip6_hdr *); pass: - IF_AFDATA_LOCK(rt->rt_ifp); error = nd6_output(rt->rt_ifp, origifp, m, dst, rt); - IF_AFDATA_UNLOCK(rt->rt_ifp); if (error) { in6_ifstat_inc(rt->rt_ifp, ifs6_out_discard); V_ip6stat.ip6s_cantforward++; Modified: projects/arpv2_merge_1/sys/netinet6/nd6.c ============================================================================== --- projects/arpv2_merge_1/sys/netinet6/nd6.c Sat Dec 13 09:33:03 2008 (r186034) +++ projects/arpv2_merge_1/sys/netinet6/nd6.c Sat Dec 13 10:19:28 2008 (r186035) @@ -434,13 +434,20 @@ skip1: * ND6 timer routine to handle ND6 entries */ void -nd6_llinfo_settimer(struct llentry *ln, long tick) +nd6_llinfo_settimer_locked(struct llentry *ln, long tick) { - LLE_WLOCK(ln); if (tick < 0) { ln->la_expire = 0; ln->ln_ntick = 0; callout_stop(&ln->ln_timer_ch); + /* + * XXX - do we know that there is + * callout installed? i.e. are we + * guaranteed that we're not dropping + * a reference that we did not add? + * KMM + */ + LLE_REMREF(ln); } else { ln->la_expire = time_second + tick / hz; LLE_ADDREF(ln); @@ -454,6 +461,14 @@ nd6_llinfo_settimer(struct llentry *ln, nd6_llinfo_timer, ln); } } +} + +void +nd6_llinfo_settimer(struct llentry *ln, long tick) +{ + + LLE_WLOCK(ln); + nd6_llinfo_settimer_locked(ln, tick); LLE_WUNLOCK(ln); } @@ -477,12 +492,6 @@ nd6_llinfo_timer(void *arg) CURVNET_SET(ifp->if_vnet); INIT_VNET_INET6(curvnet); - /* - * llentry is refcounted - we shouldn't need to protect it - * with IF_AFDATA - */ - IF_AFDATA_LOCK(ifp); - if (ln->ln_ntick > 0) { if (ln->ln_ntick > INT_MAX) { ln->ln_ntick -= INT_MAX; @@ -491,21 +500,17 @@ nd6_llinfo_timer(void *arg) ln->ln_ntick = 0; nd6_llinfo_settimer(ln, ln->ln_ntick); } - IF_AFDATA_UNLOCK(ifp); goto done; } ndi = ND_IFINFO(ifp); dst = &L3_ADDR_SIN6(ln)->sin6_addr; - if ((ln->la_flags & LLE_STATIC) || (ln->la_expire > time_second)) { - IF_AFDATA_UNLOCK(ifp); goto done; } if (ln->la_flags & LLE_DELETED) { (void)nd6_free(ln, 0); - IF_AFDATA_UNLOCK(ifp); goto done; } @@ -574,10 +579,9 @@ nd6_llinfo_timer(void *arg) } break; } - IF_AFDATA_UNLOCK(ifp); CURVNET_RESTORE(); done: - LLE_FREE_LOCKED(ln); + LLE_FREE(ln); } @@ -1427,6 +1431,7 @@ nd6_cache_lladdr(struct ifnet *ifp, stru if (ln) IF_AFDATA_UNLOCK(ifp); if (ln == NULL) { + flags |= LLE_EXCLUSIVE; ln = nd6_lookup(from, flags |ND6_CREATE, ifp); IF_AFDATA_UNLOCK(ifp); is_newentry = 1; @@ -1436,7 +1441,6 @@ nd6_cache_lladdr(struct ifnet *ifp, stru goto done; is_newentry = 0; } - if (ln == NULL) return (NULL); @@ -1495,7 +1499,7 @@ nd6_cache_lladdr(struct ifnet *ifp, stru * we must set the timer now, although it is actually * meaningless. */ - nd6_llinfo_settimer(ln, (long)V_nd6_gctimer * hz); + nd6_llinfo_settimer_locked(ln, (long)V_nd6_gctimer * hz); if (ln->la_hold) { struct mbuf *m_hold, *m_hold_next; @@ -1515,12 +1519,12 @@ nd6_cache_lladdr(struct ifnet *ifp, stru * just set the 2nd argument as the * 1st one. */ - nd6_output(ifp, ifp, m_hold, L3_ADDR_SIN6(ln), NULL); + nd6_output_lle(ifp, ifp, m_hold, L3_ADDR_SIN6(ln), NULL, ln); } } } else if (ln->ln_state == ND6_LLINFO_INCOMPLETE) { /* probe right away */ - nd6_llinfo_settimer((void *)ln, 0); + nd6_llinfo_settimer_locked((void *)ln, 0); } } @@ -1660,6 +1664,15 @@ nd6_slowtimo(void *arg) CURVNET_RESTORE(); } +int +nd6_output(struct ifnet *ifp, struct ifnet *origifp, struct mbuf *m0, + struct sockaddr_in6 *dst, struct rtentry *rt0) +{ + + return (nd6_output_lle(ifp, origifp, m0, dst, rt0, NULL)); +} + + /* * Note that I'm not enforcing any global serialization * lle state or asked changes here as the logic is too @@ -1669,17 +1682,22 @@ nd6_slowtimo(void *arg) * */ #define senderr(e) { error = (e); goto bad;} + int -nd6_output(struct ifnet *ifp, struct ifnet *origifp, struct mbuf *m0, - struct sockaddr_in6 *dst, struct rtentry *rt0) +nd6_output_lle(struct ifnet *ifp, struct ifnet *origifp, struct mbuf *m0, + struct sockaddr_in6 *dst, struct rtentry *rt0, struct llentry *lle) { INIT_VNET_INET6(curvnet); struct mbuf *m = m0; struct rtentry *rt = rt0; - struct llentry *ln = NULL; + struct llentry *ln = lle; int error = 0; int flags = 0; +#ifdef INVARIANTS + if (lle) + LLE_WLOCK_ASSERT(lle); +#endif if (IN6_IS_ADDR_MULTICAST(&dst->sin6_addr)) goto sendpkt; @@ -1696,21 +1714,25 @@ nd6_output(struct ifnet *ifp, struct ifn * At this point, the destination of the packet must be a unicast * or an anycast address(i.e. not a multicast). */ - flags = m ? LLE_EXCLUSIVE : 0; - IF_AFDATA_LOCK(rt->rt_ifp); - ln = lla_lookup(LLTABLE6(ifp), flags, (struct sockaddr *)dst); - IF_AFDATA_UNLOCK(rt->rt_ifp); - if ((ln == NULL) && nd6_is_addr_neighbor(dst, ifp)) { - /* - * Since nd6_is_addr_neighbor() internally calls nd6_lookup(), - * the condition below is not very efficient. But we believe - * it is tolerable, because this should be a rare case. - */ - flags = ND6_CREATE | (m ? ND6_EXCLUSIVE : 0); + + flags = (m || lle) ? LLE_EXCLUSIVE : 0; + if (ln == NULL) { + retry: IF_AFDATA_LOCK(rt->rt_ifp); - ln = nd6_lookup(&dst->sin6_addr, flags, ifp); + ln = lla_lookup(LLTABLE6(ifp), flags, (struct sockaddr *)dst); IF_AFDATA_UNLOCK(rt->rt_ifp); - } + if ((ln == NULL) && nd6_is_addr_neighbor(dst, ifp)) { + /* + * Since nd6_is_addr_neighbor() internally calls nd6_lookup(), + * the condition below is not very efficient. But we believe + * it is tolerable, because this should be a rare case. + */ + flags = ND6_CREATE | (m ? ND6_EXCLUSIVE : 0); + IF_AFDATA_LOCK(rt->rt_ifp); + ln = nd6_lookup(&dst->sin6_addr, flags, ifp); + IF_AFDATA_UNLOCK(rt->rt_ifp); + } + } if (ln == NULL) { if ((ifp->if_flags & IFF_POINTOPOINT) == 0 && !(ND_IFINFO(ifp)->flags & ND6_IFF_PERFORMNUD)) { @@ -1727,8 +1749,12 @@ nd6_output(struct ifnet *ifp, struct ifn /* We don't have to do link-layer address resolution on a p2p link. */ if ((ifp->if_flags & IFF_POINTOPOINT) != 0 && ln->ln_state < ND6_LLINFO_REACHABLE) { + if ((flags & LLE_EXCLUSIVE) == 0) { + flags |= LLE_EXCLUSIVE; + goto retry; + } ln->ln_state = ND6_LLINFO_STALE; - nd6_llinfo_settimer(ln, (long)V_nd6_gctimer * hz); + nd6_llinfo_settimer_locked(ln, (long)V_nd6_gctimer * hz); } /* @@ -1739,9 +1765,14 @@ nd6_output(struct ifnet *ifp, struct ifn * (RFC 2461 7.3.3) */ if (ln->ln_state == ND6_LLINFO_STALE) { + if ((flags & LLE_EXCLUSIVE) == 0) { + flags |= LLE_EXCLUSIVE; + LLE_RUNLOCK(ln); + goto retry; + } ln->la_asked = 0; ln->ln_state = ND6_LLINFO_DELAY; - nd6_llinfo_settimer(ln, (long)V_nd6_delay * hz); + nd6_llinfo_settimer_locked(ln, (long)V_nd6_delay * hz); } /* @@ -1761,10 +1792,16 @@ nd6_output(struct ifnet *ifp, struct ifn */ if (ln->ln_state == ND6_LLINFO_NOSTATE) ln->ln_state = ND6_LLINFO_INCOMPLETE; + + if ((flags & LLE_EXCLUSIVE) == 0) { + flags |= LLE_EXCLUSIVE; + LLE_RUNLOCK(ln); + goto retry; + } if (ln->la_hold) { struct mbuf *m_hold; int i; - + i = 0; for (m_hold = ln->la_hold; m_hold; m_hold = m_hold->m_nextpkt) { i++; @@ -1782,11 +1819,16 @@ nd6_output(struct ifnet *ifp, struct ifn } else { ln->la_hold = m; } - - if (flags & LLE_EXCLUSIVE) - LLE_WUNLOCK(ln); - else - LLE_RUNLOCK(ln); + /* + * We did the lookup (no lle arg) so we + * need to do the unlock here + */ + if (lle == NULL) { + if (flags & LLE_EXCLUSIVE) + LLE_WUNLOCK(ln); + else + LLE_RUNLOCK(ln); + } /* * If there has been no NS for the neighbor after entering the @@ -1807,7 +1849,11 @@ nd6_output(struct ifnet *ifp, struct ifn error = ENETDOWN; /* better error? */ goto bad; } - if (ln) { + /* + * ln is valid and the caller did not pass in + * an llentry + */ + if (ln && (lle == NULL)) { if (flags & LLE_EXCLUSIVE) LLE_WUNLOCK(ln); else @@ -1825,7 +1871,11 @@ nd6_output(struct ifnet *ifp, struct ifn return (error); bad: - if (ln) { + /* + * ln is valid and the caller did not pass in + * an llentry + */ + if (ln && (lle == NULL)) { if (flags & LLE_EXCLUSIVE) LLE_WUNLOCK(ln); else Modified: projects/arpv2_merge_1/sys/netinet6/nd6.h ============================================================================== --- projects/arpv2_merge_1/sys/netinet6/nd6.h Sat Dec 13 09:33:03 2008 (r186034) +++ projects/arpv2_merge_1/sys/netinet6/nd6.h Sat Dec 13 10:19:28 2008 (r186035) @@ -380,6 +380,7 @@ int nd6_options __P((union nd_opts *)); struct llentry *nd6_lookup __P((struct in6_addr *, int, struct ifnet *)); void nd6_setmtu __P((struct ifnet *)); void nd6_llinfo_settimer __P((struct llentry *, long)); +void nd6_llinfo_settimer_locked __P((struct llentry *, long)); void nd6_timer __P((void *)); void nd6_purge __P((struct ifnet *)); void nd6_nud_hint __P((struct rtentry *, struct in6_addr *, int)); @@ -390,6 +391,8 @@ struct llentry *nd6_cache_lladdr __P((st char *, int, int, int)); int nd6_output __P((struct ifnet *, struct ifnet *, struct mbuf *, struct sockaddr_in6 *, struct rtentry *)); +int nd6_output_lle __P((struct ifnet *, struct ifnet *, struct mbuf *, + struct sockaddr_in6 *, struct rtentry *, struct llentry *)); int nd6_need_cache __P((struct ifnet *)); int nd6_storelladdr __P((struct ifnet *, struct rtentry *, struct mbuf *, struct sockaddr *, u_char *, struct llentry **)); Modified: projects/arpv2_merge_1/sys/netinet6/nd6_nbr.c ============================================================================== --- projects/arpv2_merge_1/sys/netinet6/nd6_nbr.c Sat Dec 13 09:33:03 2008 (r186034) +++ projects/arpv2_merge_1/sys/netinet6/nd6_nbr.c Sat Dec 13 10:19:28 2008 (r186035) @@ -41,6 +41,8 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include +#include #include #include #include @@ -603,7 +605,7 @@ nd6_na_input(struct mbuf *m, int off, in char *lladdr = NULL; int lladdrlen = 0; struct ifaddr *ifa; - struct llentry *ln; + struct llentry *ln = NULL; union nd_opts ndopts; char ip6bufs[INET6_ADDRSTRLEN], ip6bufd[INET6_ADDRSTRLEN]; @@ -699,7 +701,7 @@ nd6_na_input(struct mbuf *m, int off, in * discarded. */ IF_AFDATA_LOCK(ifp); - ln = nd6_lookup(&taddr6, 0, ifp); + ln = nd6_lookup(&taddr6, LLE_EXCLUSIVE, ifp); IF_AFDATA_UNLOCK(ifp); if (ln == NULL) { goto freeit; @@ -723,12 +725,12 @@ nd6_na_input(struct mbuf *m, int off, in ln->ln_state = ND6_LLINFO_REACHABLE; ln->ln_byhint = 0; if (!ND6_LLINFO_PERMANENT(ln)) { - nd6_llinfo_settimer(ln, + nd6_llinfo_settimer_locked(ln, (long)ND_IFINFO(ln->lle_tbl->llt_ifp)->reachable * hz); } } else { ln->ln_state = ND6_LLINFO_STALE; - nd6_llinfo_settimer(ln, (long)V_nd6_gctimer * hz); + nd6_llinfo_settimer_locked(ln, (long)V_nd6_gctimer * hz); } if ((ln->ln_router = is_router) != 0) { /* @@ -782,7 +784,7 @@ nd6_na_input(struct mbuf *m, int off, in */ if (ln->ln_state == ND6_LLINFO_REACHABLE) { ln->ln_state = ND6_LLINFO_STALE; - nd6_llinfo_settimer(ln, (long)V_nd6_gctimer * hz); + nd6_llinfo_settimer_locked(ln, (long)V_nd6_gctimer * hz); } goto freeit; } else if (is_override /* (2a) */ @@ -805,13 +807,13 @@ nd6_na_input(struct mbuf *m, int off, in ln->ln_state = ND6_LLINFO_REACHABLE; ln->ln_byhint = 0; if (!ND6_LLINFO_PERMANENT(ln)) { - nd6_llinfo_settimer(ln, + nd6_llinfo_settimer_locked(ln, (long)ND_IFINFO(ifp)->reachable * hz); } } else { if (lladdr != NULL && llchange) { ln->ln_state = ND6_LLINFO_STALE; - nd6_llinfo_settimer(ln, + nd6_llinfo_settimer_locked(ln, (long)V_nd6_gctimer * hz); } } @@ -825,7 +827,6 @@ nd6_na_input(struct mbuf *m, int off, in */ struct nd_defrouter *dr; struct in6_addr *in6; -/* int s;*/ in6 = &L3_ADDR_SIN6(ln)->sin6_addr; @@ -835,9 +836,6 @@ nd6_na_input(struct mbuf *m, int off, in * is only called under the network software interrupt * context. However, we keep it just for safety. */ -/* Qing - removing - s = splnet(); -*/ dr = defrouter_lookup(in6, ln->lle_tbl->llt_ifp); if (dr) defrtrlist_del(dr); @@ -851,15 +849,13 @@ nd6_na_input(struct mbuf *m, int off, in */ rt6_flush(&ip6->ip6_src, ifp); } -/* Qing - removing - splx(s); -*/ } ln->ln_router = is_router; } - /* Qing - do we care ? - rt->rt_flags &= ~RTF_REJECT; - */ + /* XXX - QL + * Does this matter? + * rt->rt_flags &= ~RTF_REJECT; + */ ln->la_asked = 0; if (ln->la_hold) { struct mbuf *m_hold, *m_hold_next; @@ -877,14 +873,20 @@ nd6_na_input(struct mbuf *m, int off, in * we assume ifp is not a loopback here, so just set * the 2nd argument as the 1st one. */ - nd6_output(ifp, ifp, m_hold, L3_ADDR_SIN6(ln), NULL); + nd6_output_lle(ifp, ifp, m_hold, L3_ADDR_SIN6(ln), NULL, ln); } } freeit: + if (ln) + LLE_WUNLOCK(ln); + m_freem(m); return; bad: + if (ln) + LLE_WUNLOCK(ln); + V_icmp6stat.icp6s_badna++; m_freem(m); }