From d3cdb7165559c1b98bec9b6a0145c85ac327710f Mon Sep 17 00:00:00 2001 From: "Alexander V. Chernikov" Date: Tue, 15 Sep 2015 06:48:19 +0000 Subject: [PATCH] * Require explicitl lle unlink prior to calling llentry_delete(). This one slightly decreases time of holding afdata wlock. * While here, make nd6_free() return void. No one has used its return value since r186119. --- sys/net/if_llatbl.c | 8 +------- sys/netinet/if_ether.c | 12 +++++------- sys/netinet6/nd6.c | 34 +++++++++++----------------------- 3 files changed, 17 insertions(+), 37 deletions(-) diff --git a/sys/net/if_llatbl.c b/sys/net/if_llatbl.c index 08937410c411..bb43be9a3955 100644 --- a/sys/net/if_llatbl.c +++ b/sys/net/if_llatbl.c @@ -291,17 +291,11 @@ lltable_drop_entry_queue(struct llentry *lle) size_t llentry_free(struct llentry *lle) { - struct lltable *llt; size_t pkts_dropped; LLE_WLOCK_ASSERT(lle); - if ((lle->la_flags & LLE_LINKED) != 0) { - llt = lle->lle_tbl; - - IF_AFDATA_WLOCK_ASSERT(llt->llt_ifp); - llt->llt_unlink_entry(lle); - } + KASSERT((lle->la_flags & LLE_LINKED) == 0, ("freeing linked lle")); pkts_dropped = lltable_drop_entry_queue(lle); diff --git a/sys/netinet/if_ether.c b/sys/netinet/if_ether.c index 39264a564149..bba546312a20 100644 --- a/sys/netinet/if_ether.c +++ b/sys/netinet/if_ether.c @@ -194,16 +194,14 @@ arptimer(void *arg) /* Guard against race with other llentry_free(). */ if (lle->la_flags & LLE_LINKED) { - - size_t pkts_dropped; LLE_REMREF(lle); - pkts_dropped = llentry_free(lle); - ARPSTAT_ADD(dropped, pkts_dropped); - } else - LLE_FREE_LOCKED(lle); - + lltable_unlink_entry(lle->lle_tbl, lle); + } IF_AFDATA_UNLOCK(ifp); + size_t pkts_dropped = llentry_free(lle); + + ARPSTAT_ADD(dropped, pkts_dropped); ARPSTAT_INC(timeouts); CURVNET_RESTORE(); diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c index cb6f7bbf07dc..ed906ff49d01 100644 --- a/sys/netinet6/nd6.c +++ b/sys/netinet6/nd6.c @@ -131,7 +131,7 @@ static int nd6_is_new_addr_neighbor(struct sockaddr_in6 *, static void nd6_setmtu0(struct ifnet *, struct nd_ifinfo *); static void nd6_slowtimo(void *); static int regen_tmpaddr(struct in6_ifaddr *); -static struct llentry *nd6_free(struct llentry *, int); +static void nd6_free(struct llentry *, int); static void nd6_free_redirect(const struct llentry *); static void nd6_llinfo_timer(void *); static void clear_llinfo_pqueue(struct llentry *); @@ -603,7 +603,7 @@ nd6_llinfo_timer(void *arg) } if (ln->la_flags & LLE_DELETED) { - (void)nd6_free(ln, 0); + nd6_free(ln, 0); ln = NULL; goto done; } @@ -630,7 +630,7 @@ nd6_llinfo_timer(void *arg) clear_llinfo_pqueue(ln); } EVENTHANDLER_INVOKE(lle_event, ln, LLENTRY_TIMEDOUT); - (void)nd6_free(ln, 0); + nd6_free(ln, 0); ln = NULL; if (m != NULL) icmp6_error2(m, ICMP6_DST_UNREACH, @@ -648,7 +648,7 @@ nd6_llinfo_timer(void *arg) /* Garbage Collection(RFC 2461 5.3) */ if (!ND6_LLINFO_PERMANENT(ln)) { EVENTHANDLER_INVOKE(lle_event, ln, LLENTRY_EXPIRED); - (void)nd6_free(ln, 1); + nd6_free(ln, 1); ln = NULL; } break; @@ -670,7 +670,7 @@ nd6_llinfo_timer(void *arg) send_ns = 1; } else { EVENTHANDLER_INVOKE(lle_event, ln, LLENTRY_EXPIRED); - (void)nd6_free(ln, 0); + nd6_free(ln, 0); ln = NULL; } break; @@ -1125,10 +1125,9 @@ nd6_is_addr_neighbor(struct sockaddr_in6 *addr, struct ifnet *ifp) * make it global, unless you have a strong reason for the change, and are sure * that the change is safe. */ -static struct llentry * +static void nd6_free(struct llentry *ln, int gc) { - struct llentry *next; struct nd_defrouter *dr; struct ifnet *ifp; @@ -1168,10 +1167,9 @@ nd6_free(struct llentry *ln, int gc) nd6_llinfo_settimer_locked(ln, (long)V_nd6_gctimer * hz); - next = LIST_NEXT(ln, lle_next); LLE_REMREF(ln); LLE_WUNLOCK(ln); - return (next); + return; } if (dr) { @@ -1235,14 +1233,6 @@ nd6_free(struct llentry *ln, int gc) LLE_WLOCK(ln); } - /* - * Before deleting the entry, remember the next entry as the - * return value. We need this because pfxlist_onlink_check() above - * might have freed other entries (particularly the old next entry) as - * a side effect (XXX). - */ - next = LIST_NEXT(ln, lle_next); - /* * Save to unlock. We still hold an extra reference and will not * free(9) in llentry_free() if someone else holds one as well. @@ -1250,17 +1240,15 @@ nd6_free(struct llentry *ln, int gc) LLE_WUNLOCK(ln); IF_AFDATA_LOCK(ifp); LLE_WLOCK(ln); - /* Guard against race with other llentry_free(). */ if (ln->la_flags & LLE_LINKED) { + /* Remove callout reference */ LLE_REMREF(ln); - llentry_free(ln); - } else - LLE_FREE_LOCKED(ln); - + lltable_unlink_entry(ln->lle_tbl, ln); + } IF_AFDATA_UNLOCK(ifp); - return (next); + llentry_free(ln); } /*