From owner-freebsd-net@FreeBSD.ORG Mon Apr 9 17:06:35 2012 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 548421065674; Mon, 9 Apr 2012 17:06:35 +0000 (UTC) (envelope-from aboyer@averesystems.com) Received: from zimbra.averesystems.com (75-149-8-245-Pennsylvania.hfc.comcastbusiness.net [75.149.8.245]) by mx1.freebsd.org (Postfix) with ESMTP id 05AA68FC20; Mon, 9 Apr 2012 17:06:35 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by zimbra.averesystems.com (Postfix) with ESMTP id B3EFB446006; Mon, 9 Apr 2012 13:05:54 -0400 (EDT) X-Virus-Scanned: amavisd-new at averesystems.com Received: from zimbra.averesystems.com ([127.0.0.1]) by localhost (zimbra.averesystems.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id skriCG8kkTvX; Mon, 9 Apr 2012 13:05:52 -0400 (EDT) Received: from riven.arriad.com (fw.arriad.com [10.0.0.16]) by zimbra.averesystems.com (Postfix) with ESMTPSA id ABDC3446003; Mon, 9 Apr 2012 13:05:52 -0400 (EDT) Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: text/plain; charset=us-ascii From: Andrew Boyer In-Reply-To: Date: Mon, 9 Apr 2012 13:06:30 -0400 Content-Transfer-Encoding: quoted-printable Message-Id: <8B0D75C6-5918-4EFE-9929-4344D877B320@averesystems.com> References: <201204020835.00357.jhb@freebsd.org> To: Andrew Thompson X-Mailer: Apple Mail (2.1084) Cc: freebsd-net@freebsd.org, John Baldwin Subject: Re: LACP kernel panics: /* unlocking is safe here */ X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Apr 2012 17:06:35 -0000 Makes sense to me. -Andrew On Apr 7, 2012, at 4:02 AM, Andrew Thompson wrote: > On 3 April 2012 00:35, John Baldwin wrote: >> 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. >>>=20 >>> 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(). >>>=20 >>> 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? >>=20 >> This looks sensible. >=20 > Changing the order also needs an additional check as LAGG_PROTO_NONE > no longer means the detach is finished. If one ioctl sleeps then we > may nullify all the pointers upon wake that have already been set by > the other ioctl. >=20 > Does this look ok? >=20 > Index: if_lagg.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- if_lagg.c (revision 233252) > +++ if_lagg.c (working copy) > @@ -950,11 +950,11 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, = caddr_t > error =3D EPROTONOSUPPORT; > break; > } > + LAGG_WLOCK(sc); > if (sc->sc_proto !=3D LAGG_PROTO_NONE) { > - LAGG_WLOCK(sc); > + /* Reset protocol first in case detach unlocks = */ > + sc->sc_proto =3D LAGG_PROTO_NONE; > error =3D sc->sc_detach(sc); > - /* Reset protocol and pointers */ > - sc->sc_proto =3D LAGG_PROTO_NONE; > sc->sc_detach =3D NULL; > sc->sc_start =3D NULL; > sc->sc_input =3D NULL; > @@ -966,8 +966,11 @@ lagg_ioctl(struct ifnet *ifp, u_long cmd, caddr_t > sc->sc_lladdr =3D NULL; > sc->sc_req =3D NULL; > sc->sc_portreq =3D NULL; > - LAGG_WUNLOCK(sc); > + } else if (sc->sc_input !=3D NULL) { > + /* Still detaching */ > + error =3D EBUSY; > } > + LAGG_WUNLOCK(sc); > if (error !=3D 0) > break; > for (int i =3D 0; i < (sizeof(lagg_protos) / -------------------------------------------------- Andrew Boyer aboyer@averesystems.com