Date: Thu, 20 Aug 2015 14:13:28 +0100 From: "George Neville-Neil" <gnn@neville-neil.com> To: "Alexander V. Chernikov" <melifaro@FreeBSD.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r286955 - in head/sys: net netinet netinet6 Message-ID: <46D09C16-85BD-4E99-8600-16280AC9397A@neville-neil.com> In-Reply-To: <201508201205.t7KC5IoT012658@repo.freebsd.org> References: <201508201205.t7KC5IoT012658@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Why was this work not in Phabricator? This is a large change that has not been reviewed, as far as I can tell, by anyone else on the project. I am tempted to ask that this be backed out and reviewed BEFORE it goes into the tree. Best, George On 20 Aug 2015, at 13:05, Alexander V. Chernikov wrote: > Author: melifaro > Date: Thu Aug 20 12:05:17 2015 > New Revision: 286955 > URL: https://svnweb.freebsd.org/changeset/base/286955 > > Log: > * Split allocation and table linking for lle's. > Before that, the logic besides lle_create() was the following: > return existing if found, create if not. This behaviour was > error-prone > since we had to deal with 'sudden' static<>dynamic lle changes. > This commit fixes bunch of different issues like: > - refcount leak when lle is converted to static. > Simple check case: > console 1: > while true; > do for i in `arp -an|awk '$4~/incomp/{print$2}'|tr -d '()'`; > do arp -s $i 00:22:44:66:88:00 ; arp -d $i; > done; > done > console 2: > ping -f any-dead-host-in-L2 > console 3: > # watch for memory consumption: > vmstat -m | awk '$1~/lltable/{print$2}' > - possible problems in arptimer() / nd6_timer() when > dropping/reacquiring > lock. > New logic explicitly handles use-or-create cases in every lla_create > user. Basically, most of the changes are purely mechanical. However, > we explicitly avoid using existing lle's for interface/static LLE > records. > * While here, call lle_event handlers on all real table lle change. > * Create lltable_free_entry() calling existing per-lltable > lle_free_t callback for entry deletion > > Modified: > head/sys/net/if_llatbl.c > head/sys/net/if_llatbl.h > head/sys/netinet/if_ether.c > head/sys/netinet/in.c > head/sys/netinet/toecore.c > head/sys/netinet6/in6.c > head/sys/netinet6/nd6.c > head/sys/netinet6/nd6.h > > Modified: head/sys/net/if_llatbl.c > ============================================================================== > --- head/sys/net/if_llatbl.c Thu Aug 20 11:26:26 2015 (r286954) > +++ head/sys/net/if_llatbl.c Thu Aug 20 12:05:17 2015 (r286955) > @@ -228,7 +228,7 @@ htable_prefix_free(struct lltable *llt, > IF_AFDATA_WUNLOCK(llt->llt_ifp); > > LIST_FOREACH_SAFE(lle, &pmd.dchain, lle_chain, next) > - llt->llt_free_entry(llt, lle); > + lltable_free_entry(llt, lle); > } > > static void > @@ -278,10 +278,13 @@ lltable_drop_entry_queue(struct llentry > } > > /* > - * Deletes an address from the address table. > - * This function is called by the timer functions > - * such as arptimer() and nd6_llinfo_timer(), and > - * the caller does the locking. > + * > + * Performes generic cleanup routines and frees lle. > + * > + * Called for non-linked entries, with callouts and > + * other AF-specific cleanups performed. > + * > + * @lle must be passed WLOCK'ed > * > * Returns the number of held packets, if any, that were dropped. > */ > @@ -316,21 +319,35 @@ struct llentry * > llentry_alloc(struct ifnet *ifp, struct lltable *lt, > struct sockaddr_storage *dst) > { > - struct llentry *la; > + struct llentry *la, *la_tmp; > > IF_AFDATA_RLOCK(ifp); > la = lla_lookup(lt, LLE_EXCLUSIVE, (struct sockaddr *)dst); > IF_AFDATA_RUNLOCK(ifp); > - if ((la == NULL) && > - (ifp->if_flags & (IFF_NOARP | IFF_STATICARP)) == 0) { > - IF_AFDATA_WLOCK(ifp); > - la = lla_create(lt, 0, (struct sockaddr *)dst); > - IF_AFDATA_WUNLOCK(ifp); > - } > > if (la != NULL) { > LLE_ADDREF(la); > LLE_WUNLOCK(la); > + return (la); > + } > + > + if ((ifp->if_flags & (IFF_NOARP | IFF_STATICARP)) == 0) { > + la = lltable_alloc_entry(lt, 0, (struct sockaddr *)dst); > + if (la == NULL) > + return (NULL); > + IF_AFDATA_WLOCK(ifp); > + LLE_WLOCK(la); > + /* Prefer any existing LLE over newly-created one */ > + la_tmp = lla_lookup(lt, LLE_EXCLUSIVE, (struct sockaddr *)dst); > + if (la_tmp == NULL) > + lltable_link_entry(lt, la); > + IF_AFDATA_WUNLOCK(ifp); > + if (la_tmp != NULL) { > + lltable_free_entry(lt, la); > + la = la_tmp; > + } > + LLE_ADDREF(la); > + LLE_WUNLOCK(la); > } > > return (la); > @@ -483,6 +500,21 @@ lltable_foreach_lle(struct lltable *llt, > return (llt->llt_foreach_entry(llt, f, farg)); > } > > +struct llentry * > +lltable_alloc_entry(struct lltable *llt, u_int flags, > + const struct sockaddr *l3addr) > +{ > + > + return (llt->llt_alloc_entry(llt, flags, l3addr)); > +} > + > +void > +lltable_free_entry(struct lltable *llt, struct llentry *lle) > +{ > + > + llt->llt_free_entry(llt, lle); > +} > + > void > lltable_link_entry(struct lltable *llt, struct llentry *lle) > { > @@ -531,7 +563,7 @@ lla_rt_output(struct rt_msghdr *rtm, str > struct sockaddr *dst = (struct sockaddr *)info->rti_info[RTAX_DST]; > struct ifnet *ifp; > struct lltable *llt; > - struct llentry *lle; > + struct llentry *lle, *lle_tmp; > u_int laflags = 0; > int error; > > @@ -560,13 +592,9 @@ lla_rt_output(struct rt_msghdr *rtm, str > switch (rtm->rtm_type) { > case RTM_ADD: > /* Add static LLE */ > - IF_AFDATA_WLOCK(ifp); > - lle = lla_create(llt, 0, dst); > - if (lle == NULL) { > - IF_AFDATA_WUNLOCK(ifp); > + lle = lltable_alloc_entry(llt, 0, dst); > + if (lle == NULL) > return (ENOMEM); > - } > - > > bcopy(LLADDR(dl), &lle->ll_addr, ifp->if_addrlen); > if ((rtm->rtm_flags & RTF_ANNOUNCE)) > @@ -589,8 +617,39 @@ lla_rt_output(struct rt_msghdr *rtm, str > } else > lle->la_expire = rtm->rtm_rmx.rmx_expire; > laflags = lle->la_flags; > - LLE_WUNLOCK(lle); > + > + /* Try to link new entry */ > + lle_tmp = NULL; > + IF_AFDATA_WLOCK(ifp); > + LLE_WLOCK(lle); > + lle_tmp = lla_lookup(llt, LLE_EXCLUSIVE, dst); > + if (lle_tmp != NULL) { > + /* Check if we are trying to replace immutable entry */ > + if ((lle_tmp->la_flags & LLE_IFADDR) != 0) { > + IF_AFDATA_WUNLOCK(ifp); > + LLE_WUNLOCK(lle_tmp); > + lltable_free_entry(llt, lle); > + return (EPERM); > + } > + /* Unlink existing entry from table */ > + lltable_unlink_entry(llt, lle_tmp); > + } > + lltable_link_entry(llt, lle); > IF_AFDATA_WUNLOCK(ifp); > + > + if (lle_tmp != NULL) { > + EVENTHANDLER_INVOKE(lle_event, lle_tmp,LLENTRY_EXPIRED); > + lltable_free_entry(llt, lle_tmp); > + } > + > + /* > + * By invoking LLE handler here we might get > + * two events on static LLE entry insertion > + * in routing socket. However, since we might have > + * other subscribers we need to generate this event. > + */ > + EVENTHANDLER_INVOKE(lle_event, lle, LLENTRY_RESOLVED); > + LLE_WUNLOCK(lle); > #ifdef INET > /* gratuitous ARP */ > if ((laflags & LLE_PUB) && dst->sa_family == AF_INET) > > Modified: head/sys/net/if_llatbl.h > ============================================================================== > --- head/sys/net/if_llatbl.h Thu Aug 20 11:26:26 2015 (r286954) > +++ head/sys/net/if_llatbl.h Thu Aug 20 12:05:17 2015 (r286955) > @@ -133,7 +133,7 @@ struct llentry { > > typedef struct llentry *(llt_lookup_t)(struct lltable *, u_int flags, > const struct sockaddr *l3addr); > -typedef struct llentry *(llt_create_t)(struct lltable *, u_int flags, > +typedef struct llentry *(llt_alloc_t)(struct lltable *, u_int flags, > const struct sockaddr *l3addr); > typedef int (llt_delete_t)(struct lltable *, u_int flags, > const struct sockaddr *l3addr); > @@ -161,7 +161,7 @@ struct lltable { > struct ifnet *llt_ifp; > > llt_lookup_t *llt_lookup; > - llt_create_t *llt_create; > + llt_alloc_t *llt_alloc_entry; > llt_delete_t *llt_delete; > llt_prefix_free_t *llt_prefix_free; > llt_dump_entry_t *llt_dump_entry; > @@ -209,8 +209,9 @@ struct llentry *llentry_alloc(struct if > /* helper functions */ > size_t lltable_drop_entry_queue(struct llentry *); > > -struct llentry *lltable_create_lle(struct lltable *llt, u_int flags, > - const void *paddr); > +struct llentry *lltable_alloc_entry(struct lltable *llt, u_int flags, > + const struct sockaddr *l4addr); > +void lltable_free_entry(struct lltable *llt, struct llentry *lle); > void lltable_link_entry(struct lltable *llt, struct llentry *lle); > void lltable_unlink_entry(struct lltable *llt, struct llentry *lle); > void lltable_fill_sa_entry(const struct llentry *lle, struct sockaddr > *sa); > @@ -229,13 +230,6 @@ lla_lookup(struct lltable *llt, u_int fl > return (llt->llt_lookup(llt, flags, l3addr)); > } > > -static __inline struct llentry * > -lla_create(struct lltable *llt, u_int flags, const struct sockaddr > *l3addr) > -{ > - > - return (llt->llt_create(llt, flags, l3addr)); > -} > - > static __inline int > lla_delete(struct lltable *llt, u_int flags, const struct sockaddr > *l3addr) > { > > Modified: head/sys/netinet/if_ether.c > ============================================================================== > --- head/sys/netinet/if_ether.c Thu Aug 20 11:26:26 2015 (r286954) > +++ head/sys/netinet/if_ether.c Thu Aug 20 12:05:17 2015 (r286955) > @@ -218,8 +218,8 @@ arptimer(void *arg) > > /* Guard against race with other llentry_free(). */ > if (lle->la_flags & LLE_LINKED) { > - size_t pkts_dropped; > > + size_t pkts_dropped; > LLE_REMREF(lle); > pkts_dropped = llentry_free(lle); > ARPSTAT_ADD(dropped, pkts_dropped); > @@ -323,7 +323,7 @@ static int > arpresolve_full(struct ifnet *ifp, int is_gw, int create, struct mbuf > *m, > const struct sockaddr *dst, u_char *desten, uint32_t *pflags) > { > - struct llentry *la = NULL; > + struct llentry *la = NULL, *la_tmp; > struct mbuf *curr = NULL; > struct mbuf *next = NULL; > int error, renew; > @@ -337,16 +337,28 @@ arpresolve_full(struct ifnet *ifp, int i > IF_AFDATA_RUNLOCK(ifp); > } > if (la == NULL && (ifp->if_flags & (IFF_NOARP | IFF_STATICARP)) == 0) > { > - create = 1; > + la = lltable_alloc_entry(LLTABLE(ifp), 0, dst); > + if (la == NULL) { > + log(LOG_DEBUG, > + "arpresolve: can't allocate llinfo for %s on %s\n", > + inet_ntoa(SIN(dst)->sin_addr), if_name(ifp)); > + m_freem(m); > + return (EINVAL); > + } > + > IF_AFDATA_WLOCK(ifp); > - la = lla_create(LLTABLE(ifp), 0, dst); > + LLE_WLOCK(la); > + la_tmp = lla_lookup(LLTABLE(ifp), LLE_EXCLUSIVE, dst); > + /* Prefer ANY existing lle over newly-created one */ > + if (la_tmp == NULL) > + lltable_link_entry(LLTABLE(ifp), la); > IF_AFDATA_WUNLOCK(ifp); > + if (la_tmp != NULL) { > + lltable_free_entry(LLTABLE(ifp), la); > + la = la_tmp; > + } > } > if (la == NULL) { > - if (create != 0) > - log(LOG_DEBUG, > - "arpresolve: can't allocate llinfo for %s on %s\n", > - inet_ntoa(SIN(dst)->sin_addr), ifp->if_xname); > m_freem(m); > return (EINVAL); > } > @@ -608,7 +620,7 @@ in_arpinput(struct mbuf *m) > struct rm_priotracker in_ifa_tracker; > struct arphdr *ah; > struct ifnet *ifp = m->m_pkthdr.rcvif; > - struct llentry *la = NULL; > + struct llentry *la = NULL, *la_tmp; > struct rtentry *rt; > struct ifaddr *ifa; > struct in_ifaddr *ia; > @@ -620,6 +632,7 @@ in_arpinput(struct mbuf *m) > int bridged = 0, is_bridge = 0; > int carped; > struct sockaddr_in sin; > + struct sockaddr *dst; > sin.sin_len = sizeof(struct sockaddr_in); > sin.sin_family = AF_INET; > sin.sin_addr.s_addr = 0; > @@ -778,8 +791,9 @@ match: > sin.sin_len = sizeof(struct sockaddr_in); > sin.sin_family = AF_INET; > sin.sin_addr = isaddr; > + dst = (struct sockaddr *)&sin; > IF_AFDATA_RLOCK(ifp); > - la = lla_lookup(LLTABLE(ifp), LLE_EXCLUSIVE, (struct sockaddr > *)&sin); > + la = lla_lookup(LLTABLE(ifp), LLE_EXCLUSIVE, dst); > IF_AFDATA_RUNLOCK(ifp); > if (la != NULL) > arp_check_update_lle(ah, isaddr, ifp, bridged, la); > @@ -788,15 +802,47 @@ match: > * Reply to our address, but no lle exists yet. > * do we really have to create an entry? > */ > + la = lltable_alloc_entry(LLTABLE(ifp), 0, dst); > + if (la == NULL) > + goto drop; > + arp_update_lle(ah, ifp, la); > + > IF_AFDATA_WLOCK(ifp); > - la = lla_create(LLTABLE(ifp), 0, (struct sockaddr *)&sin); > - if (la != NULL) > - arp_update_lle(ah, ifp, la); > + LLE_WLOCK(la); > + la_tmp = lla_lookup(LLTABLE(ifp), LLE_EXCLUSIVE, dst); > + > + /* > + * Check if lle still does not exists. > + * If it does, that means that we either > + * 1) have configured it explicitly, via > + * 1a) 'arp -s' static entry or > + * 1b) interface address static record > + * or > + * 2) it was the result of sending first packet to-host > + * or > + * 3) it was another arp reply packet we handled in > + * different thread. > + * > + * In all cases except 3) we definitely need to prefer > + * existing lle. For the sake of simplicity, prefer any > + * existing lle over newly-create one. > + */ > + if (la_tmp == NULL) > + lltable_link_entry(LLTABLE(ifp), la); > IF_AFDATA_WUNLOCK(ifp); > - if (la != NULL) { > + > + if (la_tmp == NULL) { > arp_mark_lle_reachable(la); > LLE_WUNLOCK(la); > + } else { > + /* Free newly-create entry and handle packet */ > + lltable_free_entry(LLTABLE(ifp), la); > + la = la_tmp; > + la_tmp = NULL; > + arp_check_update_lle(ah, isaddr, ifp, bridged, la); > + /* arp_check_update_lle() returns @la unlocked */ > } > + la = NULL; > } > reply: > if (op != ARPOP_REQUEST) > @@ -1044,30 +1090,53 @@ arp_mark_lle_reachable(struct llentry *l > void > arp_ifinit(struct ifnet *ifp, struct ifaddr *ifa) > { > - struct llentry *lle; > + struct llentry *lle, *lle_tmp; > + struct sockaddr_in *dst_in; > + struct sockaddr *dst; > > if (ifa->ifa_carp != NULL) > return; > > - if (ntohl(IA_SIN(ifa)->sin_addr.s_addr) != INADDR_ANY) { > - arprequest(ifp, &IA_SIN(ifa)->sin_addr, > - &IA_SIN(ifa)->sin_addr, IF_LLADDR(ifp)); > - /* > - * interface address is considered static entry > - * because the output of the arp utility shows > - * that L2 entry as permanent > - */ > - IF_AFDATA_LOCK(ifp); > - lle = lla_create(LLTABLE(ifp), LLE_IFADDR | LLE_STATIC, > - (struct sockaddr *)IA_SIN(ifa)); > - IF_AFDATA_UNLOCK(ifp); > - if (lle == NULL) > - log(LOG_INFO, "arp_ifinit: cannot create arp " > - "entry for interface address\n"); > - else > - LLE_WUNLOCK(lle); > - } > ifa->ifa_rtrequest = NULL; > + > + dst_in = IA_SIN(ifa); > + dst = (struct sockaddr *)dst_in; > + > + if (ntohl(IA_SIN(ifa)->sin_addr.s_addr) == INADDR_ANY) > + return; > + > + arprequest(ifp, &IA_SIN(ifa)->sin_addr, > + &IA_SIN(ifa)->sin_addr, IF_LLADDR(ifp)); > + > + /* > + * Interface address LLE record is considered static > + * because kernel code relies on LLE_STATIC flag to check > + * if these entries can be rewriten by arp updates. > + */ > + lle = lltable_alloc_entry(LLTABLE(ifp), LLE_IFADDR | LLE_STATIC, > dst); > + if (lle == NULL) { > + log(LOG_INFO, "arp_ifinit: cannot create arp " > + "entry for interface address\n"); > + return; > + } > + > + IF_AFDATA_WLOCK(ifp); > + LLE_WLOCK(lle); > + /* Unlink any entry if exists */ > + lle_tmp = lla_lookup(LLTABLE(ifp), LLE_EXCLUSIVE, dst); > + if (lle_tmp != NULL) > + lltable_unlink_entry(LLTABLE(ifp), lle_tmp); > + > + lltable_link_entry(LLTABLE(ifp), lle); > + IF_AFDATA_WUNLOCK(ifp); > + > + if (lle_tmp != NULL) > + EVENTHANDLER_INVOKE(lle_event, lle_tmp, LLENTRY_EXPIRED); > + > + EVENTHANDLER_INVOKE(lle_event, lle, LLENTRY_RESOLVED); > + LLE_WUNLOCK(lle); > + if (lle_tmp != NULL) > + lltable_free_entry(LLTABLE(ifp), lle_tmp); > } > > void > > Modified: head/sys/netinet/in.c > ============================================================================== > --- head/sys/netinet/in.c Thu Aug 20 11:26:26 2015 (r286954) > +++ head/sys/netinet/in.c Thu Aug 20 12:05:17 2015 (r286955) > @@ -1202,25 +1202,15 @@ in_lltable_delete(struct lltable *llt, u > } > > static struct llentry * > -in_lltable_create(struct lltable *llt, u_int flags, const struct > sockaddr *l3addr) > +in_lltable_alloc(struct lltable *llt, u_int flags, const struct > sockaddr *l3addr) > { > const struct sockaddr_in *sin = (const struct sockaddr_in *)l3addr; > struct ifnet *ifp = llt->llt_ifp; > struct llentry *lle; > > - IF_AFDATA_WLOCK_ASSERT(ifp); > KASSERT(l3addr->sa_family == AF_INET, > ("sin_family %d", l3addr->sa_family)); > > - lle = in_lltable_find_dst(llt, sin->sin_addr); > - > - if (lle != NULL) { > - LLE_WLOCK(lle); > - return (lle); > - } > - > - /* no existing record, we need to create new one */ > - > /* > * A route that covers the given address must have > * been installed 1st because we are doing a resolution, > @@ -1241,9 +1231,6 @@ in_lltable_create(struct lltable *llt, u > lle->la_flags |= (LLE_VALID | LLE_STATIC); > } > > - lltable_link_entry(llt, lle); > - LLE_WLOCK(lle); > - > return (lle); > } > > @@ -1346,7 +1333,7 @@ in_lltattach(struct ifnet *ifp) > llt->llt_ifp = ifp; > > llt->llt_lookup = in_lltable_lookup; > - llt->llt_create = in_lltable_create; > + llt->llt_alloc_entry = in_lltable_alloc; > llt->llt_delete = in_lltable_delete; > llt->llt_dump_entry = in_lltable_dump_entry; > llt->llt_hash = in_lltable_hash; > > Modified: head/sys/netinet/toecore.c > ============================================================================== > --- head/sys/netinet/toecore.c Thu Aug 20 11:26:26 2015 (r286954) > +++ head/sys/netinet/toecore.c Thu Aug 20 12:05:17 2015 (r286955) > @@ -456,7 +456,7 @@ toe_route_redirect_event(void *arg __unu > static int > toe_nd6_resolve(struct ifnet *ifp, struct sockaddr *sa, uint8_t > *lladdr) > { > - struct llentry *lle; > + struct llentry *lle, *lle_tmp; > struct sockaddr_in6 *sin6 = (void *)sa; > int rc, flags = 0; > > @@ -465,19 +465,32 @@ restart: > lle = lla_lookup(LLTABLE6(ifp), flags, sa); > IF_AFDATA_RUNLOCK(ifp); > if (lle == NULL) { > - IF_AFDATA_LOCK(ifp); > - lle = nd6_create(&sin6->sin6_addr, 0, ifp); > - IF_AFDATA_UNLOCK(ifp); > + lle = nd6_alloc(&sin6->sin6_addr, 0, ifp); > if (lle == NULL) > return (ENOMEM); /* Couldn't create entry in cache. */ > lle->ln_state = ND6_LLINFO_INCOMPLETE; > - nd6_llinfo_settimer_locked(lle, > - (long)ND_IFINFO(ifp)->retrans * hz / 1000); > - LLE_WUNLOCK(lle); > + IF_AFDATA_WLOCK(ifp); > + LLE_WLOCK(lle); > + lle_tmp = nd6_lookup(&sin6->sin6_addr, ND6_EXCLUSIVE, ifp); > + /* Prefer any existing lle over newly-created one */ > + if (lle_tmp == NULL) > + lltable_link_entry(LLTABLE6(ifp), lle); > + IF_AFDATA_WUNLOCK(ifp); > + if (lle_tmp == NULL) { > + /* Arm timer for newly-created entry and send NS */ > + nd6_llinfo_settimer_locked(lle, > + (long)ND_IFINFO(ifp)->retrans * hz / 1000); > + LLE_WUNLOCK(lle); > > - nd6_ns_output(ifp, NULL, &sin6->sin6_addr, NULL, 0); > + nd6_ns_output(ifp, NULL, &sin6->sin6_addr, NULL, 0); > > - return (EWOULDBLOCK); > + return (EWOULDBLOCK); > + } else { > + /* Drop newly-created lle and switch to existing one */ > + lltable_free_entry(LLTABLE6(ifp), lle); > + lle = lle_tmp; > + lle_tmp = NULL; > + } > } > > if (lle->ln_state == ND6_LLINFO_STALE) { > > Modified: head/sys/netinet6/in6.c > ============================================================================== > --- head/sys/netinet6/in6.c Thu Aug 20 11:26:26 2015 (r286954) > +++ head/sys/netinet6/in6.c Thu Aug 20 12:05:17 2015 (r286955) > @@ -2239,24 +2239,16 @@ in6_lltable_delete(struct lltable *llt, > } > > static struct llentry * > -in6_lltable_create(struct lltable *llt, u_int flags, > +in6_lltable_alloc(struct lltable *llt, u_int flags, > const struct sockaddr *l3addr) > { > const struct sockaddr_in6 *sin6 = (const struct sockaddr_in6 > *)l3addr; > struct ifnet *ifp = llt->llt_ifp; > struct llentry *lle; > > - IF_AFDATA_WLOCK_ASSERT(ifp); > KASSERT(l3addr->sa_family == AF_INET6, > ("sin_family %d", l3addr->sa_family)); > > - lle = in6_lltable_find_dst(llt, &sin6->sin6_addr); > - > - if (lle != NULL) { > - LLE_WLOCK(lle); > - return (lle); > - } > - > /* > * A route that covers the given address must have > * been installed 1st because we are doing a resolution, > @@ -2277,9 +2269,6 @@ in6_lltable_create(struct lltable *llt, > lle->la_flags |= (LLE_VALID | LLE_STATIC); > } > > - lltable_link_entry(llt, lle); > - LLE_WLOCK(lle); > - > return (lle); > } > > @@ -2382,7 +2371,7 @@ in6_lltattach(struct ifnet *ifp) > llt->llt_ifp = ifp; > > llt->llt_lookup = in6_lltable_lookup; > - llt->llt_create = in6_lltable_create; > + llt->llt_alloc_entry = in6_lltable_alloc; > llt->llt_delete = in6_lltable_delete; > llt->llt_dump_entry = in6_lltable_dump_entry; > llt->llt_hash = in6_lltable_hash; > > Modified: head/sys/netinet6/nd6.c > ============================================================================== > --- head/sys/netinet6/nd6.c Thu Aug 20 11:26:26 2015 (r286954) > +++ head/sys/netinet6/nd6.c Thu Aug 20 12:05:17 2015 (r286955) > @@ -939,12 +939,8 @@ nd6_lookup(struct in6_addr *addr6, int f > return (ln); > } > > -/* > - * the caller acquires and releases the lock on the lltbls > - * Returns the llentry wlocked > - */ > struct llentry * > -nd6_create(struct in6_addr *addr6, int flags, struct ifnet *ifp) > +nd6_alloc(struct in6_addr *addr6, int flags, struct ifnet *ifp) > { > struct sockaddr_in6 sin6; > struct llentry *ln; > @@ -954,9 +950,7 @@ nd6_create(struct in6_addr *addr6, int f > sin6.sin6_family = AF_INET6; > sin6.sin6_addr = *addr6; > > - IF_AFDATA_WLOCK_ASSERT(ifp); > - > - ln = lla_create(LLTABLE6(ifp), 0, (struct sockaddr *)&sin6); > + ln = lltable_alloc_entry(LLTABLE6(ifp), 0, (struct sockaddr > *)&sin6); > if (ln != NULL) > ln->ln_state = ND6_LLINFO_NOSTATE; > > @@ -1640,7 +1634,7 @@ struct llentry * > nd6_cache_lladdr(struct ifnet *ifp, struct in6_addr *from, char > *lladdr, > int lladdrlen, int type, int code) > { > - struct llentry *ln = NULL; > + struct llentry *ln = NULL, *ln_tmp; > int is_newentry; > int do_update; > int olladdr; > @@ -1674,22 +1668,33 @@ nd6_cache_lladdr(struct ifnet *ifp, stru > IF_AFDATA_RLOCK(ifp); > ln = nd6_lookup(from, flags, ifp); > IF_AFDATA_RUNLOCK(ifp); > + is_newentry = 0; > if (ln == NULL) { > flags |= ND6_EXCLUSIVE; > - IF_AFDATA_LOCK(ifp); > - ln = nd6_create(from, 0, ifp); > - IF_AFDATA_UNLOCK(ifp); > - is_newentry = 1; > - } else { > - /* do nothing if static ndp is set */ > - if (ln->la_flags & LLE_STATIC) { > + ln = nd6_alloc(from, 0, ifp); > + if (ln == NULL) > + return (NULL); > + IF_AFDATA_WLOCK(ifp); > + LLE_WLOCK(ln); > + /* Prefer any existing lle over newly-created one */ > + ln_tmp = nd6_lookup(from, ND6_EXCLUSIVE, ifp); > + if (ln_tmp == NULL) > + lltable_link_entry(LLTABLE6(ifp), ln); > + IF_AFDATA_WUNLOCK(ifp); > + if (ln_tmp == NULL) > + /* No existing lle, mark as new entry */ > + is_newentry = 1; > + else { > + lltable_free_entry(LLTABLE6(ifp), ln); > + ln = ln_tmp; > + ln_tmp = NULL; > + } > + } > + /* do nothing if static ndp is set */ > + if ((ln->la_flags & LLE_STATIC)) { > static_route = 1; > goto done; > - } > - is_newentry = 0; > } > - if (ln == NULL) > - return (NULL); > > olladdr = (ln->la_flags & LLE_VALID) ? 1 : 0; > if (olladdr && lladdr) { > @@ -2032,7 +2037,7 @@ static int > nd6_output_lle(struct ifnet *ifp, struct ifnet *origifp, struct mbuf > *m, > struct sockaddr_in6 *dst) > { > - struct llentry *lle = NULL; > + struct llentry *lle = NULL, *lle_tmp; > > KASSERT(m != NULL, ("NULL mbuf, nothing to send")); > /* discard the packet if IPv6 operation is disabled on the interface > */ > @@ -2063,19 +2068,35 @@ nd6_output_lle(struct ifnet *ifp, struct > * the condition below is not very efficient. But we believe > * it is tolerable, because this should be a rare case. > */ > - IF_AFDATA_LOCK(ifp); > - lle = nd6_create(&dst->sin6_addr, 0, ifp); > - IF_AFDATA_UNLOCK(ifp); > + lle = nd6_alloc(&dst->sin6_addr, 0, ifp); > + if (lle == NULL) { > + char ip6buf[INET6_ADDRSTRLEN]; > + log(LOG_DEBUG, > + "nd6_output: can't allocate llinfo for %s " > + "(ln=%p)\n", > + ip6_sprintf(ip6buf, &dst->sin6_addr), lle); > + m_freem(m); > + return (ENOBUFS); > + } > + lle->ln_state = ND6_LLINFO_NOSTATE; > + > + IF_AFDATA_WLOCK(ifp); > + LLE_WLOCK(lle); > + /* Prefer any existing entry over newly-created one */ > + lle_tmp = nd6_lookup(&dst->sin6_addr, ND6_EXCLUSIVE, ifp); > + if (lle_tmp == NULL) > + lltable_link_entry(LLTABLE6(ifp), lle); > + IF_AFDATA_WUNLOCK(ifp); > + if (lle_tmp != NULL) { > + lltable_free_entry(LLTABLE6(ifp), lle); > + lle = lle_tmp; > + lle_tmp = NULL; > + } > } > } > if (lle == NULL) { > if ((ifp->if_flags & IFF_POINTOPOINT) == 0 && > !(ND_IFINFO(ifp)->flags & ND6_IFF_PERFORMNUD)) { > - char ip6buf[INET6_ADDRSTRLEN]; > - log(LOG_DEBUG, > - "nd6_output: can't allocate llinfo for %s " > - "(ln=%p)\n", > - ip6_sprintf(ip6buf, &dst->sin6_addr), lle); > m_freem(m); > return (ENOBUFS); > } > @@ -2241,24 +2262,40 @@ int > nd6_add_ifa_lle(struct in6_ifaddr *ia) > { > struct ifnet *ifp; > - struct llentry *ln; > + struct llentry *ln, *ln_tmp; > + struct sockaddr *dst; > > ifp = ia->ia_ifa.ifa_ifp; > if (nd6_need_cache(ifp) == 0) > return (0); > - IF_AFDATA_LOCK(ifp); > + > ia->ia_ifa.ifa_rtrequest = nd6_rtrequest; > - ln = lla_create(LLTABLE6(ifp), LLE_IFADDR, > - (struct sockaddr *)&ia->ia_addr); > - IF_AFDATA_UNLOCK(ifp); > - if (ln != NULL) { > - ln->la_expire = 0; /* for IPv6 this means permanent */ > - ln->ln_state = ND6_LLINFO_REACHABLE; > - LLE_WUNLOCK(ln); > - return (0); > - } > + dst = (struct sockaddr *)&ia->ia_addr; > + ln = lltable_alloc_entry(LLTABLE6(ifp), LLE_IFADDR, dst); > + if (ln == NULL) > + return (ENOBUFS); > > - return (ENOBUFS); > + ln->la_expire = 0; /* for IPv6 this means permanent */ > + ln->ln_state = ND6_LLINFO_REACHABLE; > + > + IF_AFDATA_WLOCK(ifp); > + LLE_WLOCK(ln); > + /* Unlink any entry if exists */ > + ln_tmp = lla_lookup(LLTABLE6(ifp), LLE_EXCLUSIVE, dst); > + if (ln_tmp != NULL) > + lltable_unlink_entry(LLTABLE6(ifp), ln_tmp); > + lltable_link_entry(LLTABLE6(ifp), ln); > + IF_AFDATA_WUNLOCK(ifp); > + > + if (ln_tmp != NULL) > + EVENTHANDLER_INVOKE(lle_event, ln_tmp, LLENTRY_EXPIRED); > + EVENTHANDLER_INVOKE(lle_event, ln, LLENTRY_RESOLVED); > + > + LLE_WUNLOCK(ln); > + if (ln_tmp != NULL) > + llentry_free(ln_tmp); > + > + return (0); > } > > /* > > Modified: head/sys/netinet6/nd6.h > ============================================================================== > --- head/sys/netinet6/nd6.h Thu Aug 20 11:26:26 2015 (r286954) > +++ head/sys/netinet6/nd6.h Thu Aug 20 12:05:17 2015 (r286955) > @@ -407,7 +407,7 @@ void nd6_option_init(void *, int, union > struct nd_opt_hdr *nd6_option(union nd_opts *); > int nd6_options(union nd_opts *); > struct llentry *nd6_lookup(struct in6_addr *, int, struct ifnet *); > -struct llentry *nd6_create(struct in6_addr *, int, struct ifnet *); > +struct llentry *nd6_alloc(struct in6_addr *, int, struct ifnet *); > void nd6_setmtu(struct ifnet *); > void nd6_llinfo_settimer(struct llentry *, long); > void nd6_llinfo_settimer_locked(struct llentry *, long);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?46D09C16-85BD-4E99-8600-16280AC9397A>