From owner-freebsd-wireless@freebsd.org Mon Nov 2 23:24:36 2015 Return-Path: Delivered-To: freebsd-wireless@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 3BDB8A25146 for ; Mon, 2 Nov 2015 23:24:36 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-ig0-x22b.google.com (mail-ig0-x22b.google.com [IPv6:2607:f8b0:4001:c05::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id EC1BD1AFA; Mon, 2 Nov 2015 23:24:35 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: by igbdj2 with SMTP id dj2so1845891igb.1; Mon, 02 Nov 2015 15:24:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type:content-transfer-encoding; bh=2H4fX9Dep97wVEmbrWlXxxRYpCucQb7PfG/sxIDk6EY=; b=kgvkHlmBj78Q5VSKfQc2g5DOhG1brMwc6cQnsH47qNPC0r/C8DcmlW5oFrYqpJJPDr 1JNUxVjdkL5ZlUuSEQTuxwm5x7D85OlfkSaWPynn6TIMPwc81PSDODV6ghJCKIhEZq4k fsviqZ6rJzicspC5CWhYjZWGcSTMxqjMn/j6q9xEZlMPg+35R8/Wo0c0g6Gq1zV6gyek IBC/dSXQGxxq64E+b/l8gbRhiIeX7Vh99rYSJ8w0n33ir+k9v0g9otYXEjKlYPsZlndM 6W5U4c/CFKWQ+osHNiL0A3dcutGSXLsiDwHcXPeUU0mtXWEK7h+cdI+qi4NIRzIUp3gK LrRg== MIME-Version: 1.0 X-Received: by 10.50.155.41 with SMTP id vt9mr13386816igb.22.1446506675475; Mon, 02 Nov 2015 15:24:35 -0800 (PST) Sender: adrian.chadd@gmail.com Received: by 10.36.46.66 with HTTP; Mon, 2 Nov 2015 15:24:35 -0800 (PST) In-Reply-To: References: <1446174922.809135.424262409.16BCE412@webmail.messagingengine.com> <1446474203.3014685.426732569.13D78488@webmail.messagingengine.com> Date: Mon, 2 Nov 2015 15:24:35 -0800 X-Google-Sender-Auth: Z1Jvgy93EBghNntECfo4c7gu1Og Message-ID: Subject: Re: bridge/wifi panic From: Adrian Chadd To: Andriy Voskoboinyk Cc: "freebsd-wireless@freebsd.org" , Mark Felder Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: freebsd-wireless@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "Discussions of 802.11 stack, tools device driver development." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Nov 2015 23:24:36 -0000 On 2 November 2015 at 14:49, Andriy Voskoboinyk wrote: > Tue, 03 Nov 2015 00:38:45 +0200 =D0=B1=D1=83=D0=BB=D0=BE =D0=BD=D0=B0=D0= =BF=D0=B8=D1=81=D0=B0=D0=BD=D0=BE Adrian Chadd > : > >> Actually, this is more annoying than I'd thought. There's a whole >> bunch of places where mbufs can change during processing in ath(4) and >> yeah, we can't guarantee the original mbuf is available. >> >> I wonder how many other drivers are doing this - stuff like >> m_collapse(), m_defrag(), etc. If this is called then the original >> mbuf can't be returned upon error. > > > I have thinked about that few hours ago - there are many ways to fix it: > > 1) add a guarantee for current m_collapse() / m_defrag() that current > mbuf pointer will not change (like it works in OpenBSD); > 2) implement third function with this guarantee (for backward > compatibility); > 3) pass pointer to mbuf pointer via ic_raw_xmit() / ic_transmit() - > then the caller can change it (that's the way I've seen in other > m_collapse() consumers); > 4) move m_defrag() / m_collapse() to the net80211 (to the ieee80211_encap= () > ?). > This will require an additional parameter, which will limit max number > of segments for device. > ... (something else?) > *) leave it 'as is'. I'm worried that there's a bunch of other places where this happens. I'm also worried about trying to modify the rest of the mbuf API right now just to fix things; I'd rather we revert things back to working state and do this work in a branch. in ath(4), the main place this gets unhappy is ath_tx_dmasetup() where it may do a defrag first, and then if the defrag succeeds but the load fails, then we're stuck with an mbuf that isn't the original one. I think the safest thing to do here is to revert it so the driver has to free the mbuf (ie, how it was.) We should sit down and figure out how to more cleanly solve this before we attempt it again. Do you have a problem with that? Any other ideas? -adrian