Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 5 Oct 2012 23:44:10 -0700
From:      Adrian Chadd <adrian@freebsd.org>
To:        freebsd-wireless@freebsd.org
Subject:   [RFC] How the TX path works; I'd like to fix this before 10.x
Message-ID:  <CAJ-VmomS=7Udyixqqss_1261EXzEpeYBuAWqwiKEah=5V4rrWw@mail.gmail.com>

next in thread | raw e-mail | index | archive | help
Hi all,

It seems the net80211 TX path is a bit .. racey.

Specifically:

* There's three entry points - the vap netif start at
ieee80211_start(), and ic_raw_xmit() method, and ieee80211_output()
for raw frames
* the vap netif path will do 802.11 encapsulation, including seqno allocation
* raw_xmit (iirc) doesn't do seqno allocation, but it's for management frames
* ieee80211_output() does the 802.11 encap, seqno allocation and
encryption if needed.

* Then, the driver does the encryption, including CCMP IV.

Now, there's problems:

* there's multiple, overlapping entry points into ieee80211_start(),
as the if_start() method doesn't _at all_ guarantee only one
if_start() method is running at once. Sigh.
* ic_raw_xmit() can be called at any time, and bypasses the vap ifp entirely.
* ieee80211_output() can either use or bypass the vap ifp queue,
depending upon whether it's a raw 802.11 or 802.3 frame.

Then, to make matters worse, the whole TX path shift in the 7.x days
broke fragment TX handling, as the method of using m_nextpt doesn't
work at all with the ifnet macros. That's definitely a problem as it
means we can't get any wifi certification _at all_.

Before I continue - if_transmit() is not the answer. if_transmit()
guarantess _no_ serialisation at all. So we'd stlil have to stuff
things into queues. And ensure only one thing is running at once.

So the question is - how do we fix this?

We could do the dfbsd approach and just giant-lock the whole wifi
subsystem and drivers. I don't want to do that.

The ic_raw_xmit() interface needs to go away. Wireless frames should
just be pushed onto the output queue like any other frames and the
queue discipline should ensure management frames don't get dropped.
But as long as they're not encrypted (for now, there _is_ management
frame encryption as an optional part of 802.11) and don't require
sequence/CCMP IVs, we're ok. Still; I think we should attach an
optional "tx params" to the mbuf and send that along. Then we teach
the driver to check for that particular mbuf parameter set and use it,
just like how ic_raw_xmit() works right at the moment.

The ic_raw_xmit() stuff is dangerous and scary, mostly because it
right now does bypass a full ifnet queue and lets us send those
management frames doirectly.

I could wrap a big VAP TX lock around ieee80211_output() and
ieee80211_start(), ensuring they don't run over each other. But
long-held locks need to go away and die. yes, even the ones in the
ath(4) driver that I've introduced. They're there because of a lack of
synchronisation and queue design. A lot of the gige/10ge drivers use
long-held locks.. sigh.

I could create a net80211 TX tasklet for each vap (or ic, _maybe_)
which serialises the TX path. ieee80211_start() would just schedule
the tasklet to run. That would serialise all of the parallel TX entry
points and solve a bunch of the subtle sequence number and other TX
state races that are occuring. That doesn't solve the ic_raw_xmit() or
ieee80211_output() problem, as both of those can also do TX 802.11
encapsulation and kick off parts of the state machine.

It doesn't solve the same issues with the drivers .Yes, even if we
converted them to use if_transmit(). iwn(4) solves this by just having
a big IWN_LOCK() but it releases it when doing anything stack related.
I'm not sure if it holds it across the whole TX path through
iwn_start(). In any case, it's a big lock. ath(4) can and does have
multiple ath_start() instances running in multiple kernel threads,
fighting to dequeue frames from the ifnet. This still can cause issues
with non-aggregate sequence number and CCMP IV counter allocation.
Sigh.

God even knows what to do about USB devices in all of this.

So, now that I've ranted about the problem and some poking about
solutions, I'm opening the floor for comment. I'd like to have this
worked on parallel (whilst I'm doing things to the net80211 stack to
support better 802.11n legacy power save and fix ps/ps-poll support)
and get it implemented/debugged before 10.0.

... your turn. :)



Adrian



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-VmomS=7Udyixqqss_1261EXzEpeYBuAWqwiKEah=5V4rrWw>