From owner-freebsd-net@FreeBSD.ORG Sat Apr 7 08:02:49 2012 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DE9ED1065670 for ; Sat, 7 Apr 2012 08:02:48 +0000 (UTC) (envelope-from andy@fud.org.nz) Received: from mail-lb0-f182.google.com (mail-lb0-f182.google.com [209.85.217.182]) by mx1.freebsd.org (Postfix) with ESMTP id 57E868FC08 for ; Sat, 7 Apr 2012 08:02:48 +0000 (UTC) Received: by lbok6 with SMTP id k6so1424785lbo.13 for ; Sat, 07 Apr 2012 01:02:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding:x-gm-message-state; bh=SR4Ic4RioPLVNS/VvTinqBt2iYPQgRoZV5q53l5pEU0=; b=gJNIRu3iz/bimlVcUq3lGR8OvrrI1U5bxMZ0iWjKWDnUez0RdYuWJSrwtNYXXzhmGr uGqXNNr/UFNVS0yKn586WCTgkOqNjbfSnWyzwG5jI89onkAFrOxYFHcDP3ThCCpByf8D jHDuTtu0KvcL/N2zfYRUdxlU1Bc3wicUt8nmlYVdpawQXa2FbVQduSy44wivwJkTF84L dzXpoDWPHJa951HGy6raNwP3UWt/+iuDiReTVUqbtS+/sx6NpKj3zQUuQJjrmlsu59To Gzlgd45qsSzGMdet0WgeGZigV83PJ2oKZ4ysCxdVjBG+w45uTnRxcOQD7fvpESX0C5Q+ GPLA== MIME-Version: 1.0 Received: by 10.152.129.69 with SMTP id nu5mr1180010lab.9.1333785767007; Sat, 07 Apr 2012 01:02:47 -0700 (PDT) Sender: andy@fud.org.nz Received: by 10.112.150.72 with HTTP; Sat, 7 Apr 2012 01:02:46 -0700 (PDT) In-Reply-To: <201204020835.00357.jhb@freebsd.org> References: <201204020835.00357.jhb@freebsd.org> Date: Sat, 7 Apr 2012 20:02:46 +1200 X-Google-Sender-Auth: sJflwS9OItCYhSofu7YOecKpRS4 Message-ID: From: Andrew Thompson To: John Baldwin Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-Gm-Message-State: ALoCoQnhE2haoxfj1PEtKdmnUBKb7YP64J97+uVLst7GV/89wI3cPOiTzz8c3Ge67Qy1olr9it3P Cc: freebsd-net@freebsd.org, Andrew Boyer 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: Sat, 07 Apr 2012 08:02:49 -0000 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 kern= el. > 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 incomi= ng > packets to be dropped until the lagg gets reconfigured. =A0Thoughts? > > This looks sensible. 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. Does this look ok? 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) /