kern/165863

[Gleb Smirnoff]

  Thanks, Ryan!

On Thu, Mar 29, 2012 at 05:59:38PM -0400, Ryan Stone wrote:
R> Ok, I think that I have an approach that will work.  This is heavily
R> based off of glebius' proposal.  The big difference is that instead of
R> initializing the arptimer callout with the ll_entry's lock, I
R> initialize it with the if_afdata_lock.  This eliminates the LOR
R> problem in arptimer.  It also nicely prevents any races between
R> arptimer and in_lltable_prefix_free.  Either arptimer will run and
R> ensure that prefix_free can never see an entry that arptimer is in the
R> process of destroying, or prefix_free will stop the callout before
R> arptimer gets a chance to start.
R> 
R> I'll admit that it feels a bit dirty to be doing anything if the ifp
R> at this level, but I'd argue that is an artifact of using a lock in
R> the ifp to protect the lltable.
R> 
R> The patch below is not yet complete; it doesn't fix the IPv6 case.
R> IPv6 is looking much trickier as when an NDP entry is timed out
R> nd6_free() will drop the LLE_WLOCK, acquire the IF_AFDATA_LOCK and
R> then re-acquire the LLE_WLOCK.  It's not immediately clear to me what
R> the best way to handle the race between in6_lltable_prefix_free and
R> nd6_llinfo_timer is.  Holding the afdata_lock across all of
R> nd6_llinfo_timer feels like overkill.
R> 
R> Any comments on this approach?  Am I going in the wrong direction?

Looks okay from my viewpoint. Have you stress tested it? My snap patch
definitely had problems, AFAIR.

If this patch fixes panics observed by kern/165863 and passes stress
testing, then it should be committed ASAP, and shouldn't depend on
IPv6 part.

-- 
Totus tuus, Glebius.