Date: Mon, 2 Nov 2015 15:24:35 -0800 From: Adrian Chadd <adrian@freebsd.org> To: Andriy Voskoboinyk <avos@freebsd.org> Cc: "freebsd-wireless@freebsd.org" <freebsd-wireless@freebsd.org>, Mark Felder <feld@freebsd.org> Subject: Re: bridge/wifi panic Message-ID: <CAJ-Vmom4BP9XKGHS=qXK2p_Mg3aapOtVtbUVsOFRwpdgM%2BTRug@mail.gmail.com> In-Reply-To: <op.x7hwb5kl4dikkl@localhost> References: <1446174922.809135.424262409.16BCE412@webmail.messagingengine.com> <CAJ-Vmon-kwdeG2nVRGMoKBE654o0Yx8G8UB9=E6%2Ba=XyvDNrmg@mail.gmail.com> <BEB3A1D6-B877-4C65-B66E-9EFA39E2912C@FreeBSD.org> <CAJ-Vmo=exW_kmX0tnjQ81o8HxgJvG=5KtWf%2B3cQeAFteLPgtHw@mail.gmail.com> <A6634DC8-90CB-40C1-8B93-747E6AAC4A3D@FreeBSD.org> <CAJ-VmonrCfB_6jvFLMrJ6fD4ma=f8mHNmhQhum86wOPmzTNdoA@mail.gmail.com> <1446474203.3014685.426732569.13D78488@webmail.messagingengine.com> <CAJ-VmokBbh6Uh6FOaKhKPH_FxbNcHu7EdxP1SSh1ZzeQKqda_Q@mail.gmail.com> <op.x7hcb1o54dikkl@localhost> <CAJ-Vmo=uZbErxRudyoq6B7AG=SRb_PmoTQ4cD8-uhVPFHzbOEQ@mail.gmail.com> <CAJ-Vmo=tc%2BHvcLAVe_APVFkLUs76SmhpYJiPZT7BAyL5Jin-EQ@mail.gmail.com> <CAJ-Vmo=LgSeD%2BViEYdp4D2kBqQ=1KNOSnTTke9SqzkgJLtEHaw@mail.gmail.com> <op.x7hwb5kl4dikkl@localhost>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2 November 2015 at 14:49, Andriy Voskoboinyk <avos@freebsd.org> 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 > <adrian@freebsd.org>: > >> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-Vmom4BP9XKGHS=qXK2p_Mg3aapOtVtbUVsOFRwpdgM%2BTRug>