Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 2 Apr 2012 08:35:00 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        freebsd-net@freebsd.org
Cc:        Andrew Boyer <aboyer@averesystems.com>
Subject:   Re: LACP kernel panics: /* unlocking is safe here */
Message-ID:  <201204020835.00357.jhb@freebsd.org>
In-Reply-To: <D02B1265-C1F4-4F6A-979D-E141565E813F@averesystems.com>
References:  <D02B1265-C1F4-4F6A-979D-E141565E813F@averesystems.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Friday, March 30, 2012 6:04:24 pm Andrew Boyer wrote:
> While investigating a LACP issue, I turned on LACP_DEBUG on a debug kernel.  
In this configuration it's easy to panic the kernel - just run 'ifconfig lagg0 
laggproto lacp' on a lagg that's already in LACP mode and receiving LACP 
messages.
> 
> The problem is that lagg_lacp_detach() drops the lagg wlock (with the 
comment in the title), which allows incoming LACP messages to get through 
lagg_input() while the structure is being destroyed in lacp_detach().
> 
> There's a very simple fix, but I don't know if it's the best way to fix it.  
Resetting the protocol before calling sc_detach causes any further incoming 
packets to be dropped until the lagg gets reconfigured.  Thoughts?

This looks sensible.

> Is it safe to just hold on to the lagg wlock across the callout_drain() 
calls in lacp_detach()?  That's what OpenBSD does.

No, callout_drain() can sleep.  Also, if this is using callout_init_mtx()
or callout_init_rwlock() (which it probably should), then holding the
lock across callout_drain() could deadlock.

> -Andrew
> 
> Index: sys/net/if_lagg.c
> ===================================================================
> --- sys/net/if_lagg.c	(revision 233707)
> +++ sys/net/if_lagg.c	(working copy)
> @@ -952,9 +952,10 @@
>  		}
>  		if (sc->sc_proto != LAGG_PROTO_NONE) {
>  			LAGG_WLOCK(sc);
> +			/* Reset protocol */
> +			sc->sc_proto = LAGG_PROTO_NONE;
>  			error = sc->sc_detach(sc);
> -			/* Reset protocol and pointers */
> -			sc->sc_proto = LAGG_PROTO_NONE;
> +			/* Reset pointers */
>  			sc->sc_detach = NULL;
>  			sc->sc_start = NULL;
>  			sc->sc_input = NULL;
> 
> --------------------------------------------------
> Andrew Boyer	aboyer@averesystems.com
> 
> 
> 
> 
> _______________________________________________
> freebsd-net@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"
> 

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201204020835.00357.jhb>