> ------------------------------
> Message: 3
> Date: Thu, 27 Dec 2012 22:24:33 -0800
> From: Adrian Chadd <adr...@freebsd.org>
> To: Monthadar Al Jaberi <montha...@gmail.com>, Bernhard Schmidt
>         <bschm...@freebsd.org>, freebsd-wireless@freebsd.org
> Subject: .. if_transmit() and the quirky issues with node referencing
> Message-ID:
>         <caj-vmom6jnniy16h7np3pbaj1ljaagqtvanmkegzlsypxtj...@mail.gmail.com>
> Content-Type: text/plain; charset=ISO-8859-1
> So I finally figured out my problem with if_transmit()'ifying the
> ath(4) code. (Yes, this means I have TX fragmentation working with
> ath(4), as well as some non-trivial performance improvements in TX TCP
> tests.)
> In quick summary: net80211's call to the driver will decrement the
> node ref if the driver if_transmit() call fails.
> So I went looking for instances of where if_transmit() is called
> without correct error checking.
> One is in ieee80211_hwmp.c . It pops frames off of the ageq and calls
> if_transmit(). It should deref the node ref the same way that the ageq
> code does.
> The other is in ieee80211_hostap.c - my fault. It uses the psq; I
> should deref the node ref the same way that the psq code does.
> This just highlights some of the rather quirky corners of net80211:
> * if_transmit() can be called on the driver facing interface (ie, the
> driver ifp) or the vap ifp.
> * for driver ifp, the frame should be encapsulated and there must be a
> node reference held/given when it's called. If if_transmit() fails
> here, it must deref the node.
> * for non-encapsulated frames (eg perhaps some stuff in the ageq or
> one of the psqs), if_transmit() will be called on the VAP interface.
> * .. now, when if_transmit() is called on the vap interface, the mbuf
> m_pkthdr.recvif will point to the vap ifp;
> * .. when if_transmit() is called on the physical interface, the mbuf
> m_pkthdr.recvif will point to the node reference.
> This is all a bit kooky and very prone to people making mistakes,
> especially when mbufs may be pushed up, down and throughout all kinds
> of weird places. I also bet the re-entrant parts of the codebase (ie
> stuff that uses the ageq, psq, wds .. maybe the mesh stuff) could do
> with a bit of a re-review.
> In any case, what this means is:
> * we need to be really, really careful with how we route mbufs through
> net80211 here, aiee!
> * if a frame has M_ENCAP tagged and it's pushed into an ageq or psq,
> it _must_ have the TX node ref held;
>   .. thus, if we raw construct an encapsulated frame (locally in
> net80211, or in a driver, or via ieee80211_output, etc) then when we
> set M_ENCAP, we must hold the TX node reference and we must set recvif
> correctly!
> What I'd really like to do here:
> * re-review all the if_transmit() error handling - make sure that if
> it fails, we correctly handle dereferencing the node or things will
> leak;
> * make "get/set recvif from ifp" and "get/set recvif from node" as methods;
> * have those methods check whether M_ENCAP is correctly set/cleared,
> and complain loudly if we ever try to get the wrong reference for the
> given situation (eg, if we try to read the node reference from an
> mbuf, but M_ENCAP isn't set, then complain);
> * .. figure out some alternate way to store the node reference (mbuf
> tags?) and start migrating the code that way.

Just to add another thing before I forget.

When tearing a ba session is somehow failed, a node could still
receive ampdu packets from another node which ni has already freed.

I don't know exactly how that happens, yet. I will let you know when I
figured it out.

freebsd-wireless@freebsd.org mailing list
To unsubscribe, send any mail to "freebsd-wireless-unsubscr...@freebsd.org"

Reply via email to