Add new lock for stageq (part of ieee80211_superg structure) and
ni_tx_superg (part of ieee80211_node structure);
drop com_lock protection where it is used to protect them.
While here, drop duplicate OPACKETS counter incrementation.
ni_tx_ampdu is not protected with it (however, it is also used without
locking in other places; probably, it requires some other solution
to be thread-safe).
Tested with RTL8188CUS (AP) and RTL8188EU (STA).
NOTE: Since this change breaks KBI, all wireless drivers need to be
recompiled.
Reviewed by: adrian
Approved by: re (gjb)
Differential Revision: https://reviews.freebsd.org/D6958
Drop scan generation number and node table scan lock - the only place
where ni_scangen is checked is in ieee80211_timeout_stations() (and it
is used to prevent duplicate checking of the same node); node scan lock
protects only this variable + node table scan generation number.
This will fix (at least) next LOR (hostap mode):
lock order reversal:
1st 0xc175f84c urtwm0_scan_loc (urtwm0_scan_loc) @ /usr/src/sys/modules/wlan/../../net80211/ieee80211_node.c:2019
2nd 0xc175e018 urtwm0_com_lock (urtwm0_com_lock) @ /usr/src/sys/modules/wlan/../../net80211/ieee80211_node.c:2693
stack backtrace:
#0 0xa070d1c5 at witness_debugger+0x75
#1 0xa070d0f6 at witness_checkorder+0xd46
#2 0xa0694cce at __mtx_lock_flags+0x9e
#3 0xb03ad9ef at ieee80211_node_leave+0x12f
#4 0xb03afd13 at ieee80211_timeout_stations+0x483
#5 0xb03aa1c2 at ieee80211_node_timeout+0x42
#6 0xa06c6fa1 at softclock_call_cc+0x1e1
#7 0xa06c7518 at softclock+0xc8
#8 0xa06789ae at intr_event_execute_handlers+0x8e
#9 0xa0678fa0 at ithread_loop+0x90
#10 0xa0675fbe at fork_exit+0x7e
#11 0xa08af910 at fork_trampoline+0x8
In addition to the above:
* switch to ieee80211_iterate_nodes();
* do not assert that node table lock is held, while calling node_age();
that's not really needed (there are no resources, which can be protected
by this lock) + this fixes LOR/deadlock between ieee80211_timeout_stations()
and ieee80211_set_tim() (easy to reproduce in HOSTAP mode while
sending something to an STA with enabled power management).
Tested:
* (avos) urtwn0, hostap mode
* (adrian) AR9380, STA mode
* (adrian) AR9380, AR9331, AR9580, hostap mode
Notes:
* This changes the net80211 internals, so you have to recompile all of it
and the wifi drivers.
Submitted by: avos
Approved by: re (delphij)
Differential Revision: https://reviews.freebsd.org/D6833
ieee80211.c:
add_chanlist(): 'error' variable will be uninitialized if
no channels were passed; return '0' instead.
ieee80211_action.c:
ieee80211_send_action_register(): drop 'break' after 'return'.
ieee80211_crypto_none.c:
none_encap(): 'keyid' is not used in non-debug builds; hide it
behind IEEE80211_DEBUG ifdef.
ieee80211_freebsd.c:
Staticize global 'ieee80211_debug' variable (used only in this
file).
ieee80211_hostap.c:
Fix a comment (associatio -> association).
ieee80211_ht.c:
ieee80211_setup_htrates(): initialize 'maxunequalmcs' to 0 to mute
compiler warning.
ieee80211_hwmp.c:
hwmp_recv_preq(): copy 'prep' between conditional blocks to fix
-Wshadow warning.
ieee80211_mesh.c:
mesh_newstate(): remove duplicate 'ni' definition.
mesh_recv_group_data(): fix -Wempty-body warning in non-debug
builds.
ieee80211_phy.c:
ieee80211_compute_duration(): remove 'break' after panic() call.
ieee80211_scan_sta.c:
Hide some TDMA-specific macros under IEEE80211_SUPPORT_TDMA ifdef
adhoc_pick_bss(): remove 'ic' pointer redefinition.
ieee80211_sta.c:
sta_beacon_miss(): remove 'ic' pointer redefinition.
ieee80211_superg.c:
superg_ioctl_set80211(): drop unreachable return.
Tested with clang 3.8.0, gcc 4.6.4 and gcc 5.3.0.
le*dec / le*enc functions.
Replace net80211 specific macros with system-wide bytestream
encoding/decoding functions:
- LE_READ_2 -> le16dec
- LE_READ_4 -> le32dec
- LE_WRITE_2 -> le16enc
- LE_WRITE_4 -> le32enc
+ drop ieee80211_input.h include, where it was included for these
operations only.
Reviewed by: adrian
Differential Revision: https://reviews.freebsd.org/D6030
A-MSDU is another 11n aggregation mechanism where multiple ethernet
frames get LLC encapsulated (so they have a length field), padded,
and put in a single MPDU (802.11 MAC frame.) This means it gets sent
out as a single frame, with a single seqno, it's acked as one frame, etc.
It turns out that, hah, atheros fast frames is almost but not quite
like this, so I'm reusing all of the current superg/fast-frames stuff
in order to actually transmit A-MSDU. Yes, this means that A-MSDU
frames are also only aggregated two at a time, so it's not necessarily
a huge win, but it's better than nothing.
This doesn't do anything by default - the driver needs to say it does
A-MSDU as well as set the AMSDU software TX capability so this code path
gets exercised.
For now, the only driver that enables this is urtwn. I'll enable it
for rsu at some point soon.
Tested:
* Add an amsdu encap path to aggregate two frames, same as the
fast-frames path.
* Always do the superg init/teardown and node init/teardown stuff,
regardless of whether the nodes are doing fast-frames (the ATH
capability stuff.) That way we can reuse it for amsdu.
* Don't do AMSDU for multicast/broadcast and EAPOL frames.
* If we're doing A-MPDU, then don't bother doing FF/A-MSDU.
We can likely do both together, but I don't want to change
behaviour.
* Teach the fast frames approx txtime logic to support the 11n
rates. But, since we don't currently have a full "current rate"
support, assume it's HT20, long-gi, etc. That way we overshoot
on the TX time estimation, so we're always inside the requirements.
(And we only aggregate two frames for now, so we're not really
going to exceed that.)
* Drop the maximum FF age default down to 2ms, otherwise we end up
with some very annoyingly large latencies.
TODO:
* We only aggregate two ethernet frames, so I'm not checking the max
A-MSDU size. But when it comes time to support >2 frames, we should
obey that.
Tested:
* urtwn(4)
Move error handling into ieee80211_parent_xmitpkt() instead of spreading it
between functions.
Submitted by: <s3erios@gmail.com>
Differential Revision: https://reviews.freebsd.org/D3772
field and into a separate fast-frames staging pointer in ieee80211_node.
The A-MPDU TX path allows txa_private to be used by drivers. So it will
clash with any attempt to use fast-frames. Now, fast-frames is not really
anything special - it's just a custom ethernet frame type that contains
two MSDUs into one MPDU. So all the NIC has to support doing is transmitting
up to a 4KiB frame with an arbitrary ethertype and bam! Fast-frames.
However, using txa_private means we can /either/ do fast-frames or A-MPDU TX,
so fast frames has been turned off in the Atheros HAL for 11n chipsets.
This is a bit silly - it actually means that 802.11 performance to/from
11abg Atheros chips is actually better than between an 11abg atheros device
and an 11n Atheros device.
So:
* create a new mbuf staging queue for fast frames. It only queues a single
frame in the staging queue (and there's a top-level ic staging queue
used for expiry/tracking) so it's just an mbuf pointer per TID.
* Still use the ampdu TX packet counter to determine whether to do
aggregation or not. It'll double count if we start doing both A-MPDU TX
and fast frames, but that's not all that important right now.
* Initialise the pps tracker so ticks isn't zero. This ensures that
fast-frames actually gets used - without it, the ticks math overflows
and the pps math always sets txa_pps=0. This is the same bug that
plagued A-MPDU TX starting logic.
This actually allows fast-frames transmit to occur between the AR9331
(in 11n HT/20 mode) and AR9170 (if_otus) in 11bg mode.
Now, this is a great big no-op on atheros 11n hardware, so don't worry.
It may mean you start seeing more reliable fast-frames transmission on
11abg hardware which may expose some more amusing bugs.
TODO:
* further testing and debugging of all of this before flipping on
fast-frames in if_ath (for 11n) and if_otus.
DragonflyBSD uses the FreeBSD wireless stack and drivers. Their malloc()
API is named differently, so they don't have userland/kernel symbol
clashes like we do (think libuinet.)
So, to make it easier for them and to port to other BSDs/other operating
systems, start hiding the malloc specific bits behind defines in
ieee80211_freebsd.h.
DragonflyBSD can now put these portability defines in their local
ieee80211_dragonflybsd.h.
This should be a great big no-op for everyone running wifi.
TODO:
* kill M_WAITOK - some platforms just don't want you to use it
* .. and/or handle it returning NULL rather than waiting forever.
* MALLOC_DEFINE() ?
* Migrate the well-known malloc names (eg M_TEMP) to net80211
namespace defines.
- Wrong integer type was specified.
- Wrong or missing "access" specifier. The "access" specifier
sometimes included the SYSCTL type, which it should not, except for
procedural SYSCTL nodes.
- Logical OR where binary OR was expected.
- Properly assert the "access" argument passed to all SYSCTL macros,
using the CTASSERT macro. This applies to both static- and dynamically
created SYSCTLs.
- Properly assert the the data type for both static and dynamic
SYSCTLs. In the case of static SYSCTLs we only assert that the data
pointed to by the SYSCTL data pointer has the correct size, hence
there is no easy way to assert types in the C language outside a
C-function.
- Rewrote some code which doesn't pass a constant "access" specifier
when creating dynamic SYSCTL nodes, which is now a requirement.
- Updated "EXAMPLES" section in SYSCTL manual page.
MFC after: 3 days
Sponsored by: Mellanox Technologies
upper layer(s).
This eliminates the if_snd queue from net80211. Yay!
This unfortunately has a few side effects:
* It breaks ALTQ to net80211 for now - sorry everyone, but fixing
parallelism and eliminating the if_snd queue is more important
than supporting this broken traffic scheduling model. :-)
* There's no VAP and IC flush methods just yet - I think I'll add
some NULL methods for now just as placeholders.
* It reduces throughput a little because now net80211 will drop packets
rather than buffer them if the driver doesn't do its own buffering.
This will be addressed in the future as I implement per-node software
queues.
Tested:
* ath(4) and iwn(4) in STA operation
This patchset implements a new TX lock, covering both the per-VAP (and
thus per-node) TX locking and the serialisation through to the underlying
physical device.
This implements the hard requirement that frames to the underlying physical
device are scheduled to the underlying device in the same order that they
are processed at the VAP layer. This includes adding extra encapsulation
state (such as sequence numbers and CCMP IV numbers.) Any order mismatch
here will result in dropped packets at the receiver.
There are multiple transmit contexts from the upper protocol layers as well
as the "raw" interface via the management and BPF transmit paths.
All of these need to be correctly serialised or bad behaviour will result
under load.
The specifics:
* add a new TX IC lock - it will eventually just be used for serialisation
to the underlying physical device but for now it's used for both the
VAP encapsulation/serialisation and the physical device dispatch.
This lock is specifically non-recursive.
* Methodize the parent transmit, vap transmit and ic_raw_xmit function
pointers; use lock assertions in the parent/vap transmit routines.
* Add a lock assertion in ieee80211_encap() - the TX lock must be held
here to guarantee sensible behaviour.
* Refactor out the packet sending code from ieee80211_start() - now
ieee80211_start() is just a loop over the ifnet queue and it dispatches
each VAP packet send through ieee80211_start_pkt().
Yes, I will likely rename ieee80211_start_pkt() to something that
better reflects its status as a VAP packet transmit path. More on
that later.
* Add locking around the management and BAR TX sending - to ensure that
encapsulation and TX are done hand-in-hand.
* Add locking in the mesh code - again, to ensure that encapsulation
and mesh transmit are done hand-in-hand.
* Add locking around the power save queue and ageq handling, when
dispatching to the parent interface.
* Add locking around the WDS handoff.
* Add a note in the mesh dispatch code that the TX path needs to be
re-thought-out - right now it's doing a direct parent device transmit
rather than going via the vap layer. It may "work", but it's likely
incorrect (as it bypasses any possible per-node power save and
aggregation handling.)
Why not a per-VAP or per-node lock?
Because in order to ensure per-VAP ordering, we'd have to hold the
VAP lock across parent->if_transmit(). There are a few problems
with this:
* There's some state being setup during each driver transmit - specifically,
the encryption encap / CCMP IV setup. That should eventually be dragged
back into the encapsulation phase but for now it lives in the driver TX path.
This should be locked.
* Two drivers (ath, iwn) re-use the node->ni_txseqs array in order to
allocate sequence numbers when doing transmit aggregation. This should
also be locked.
* Drivers may have multiple frames queued already - so when one calls
if_transmit(), it may end up dispatching multiple frames for different
VAPs/nodes, each needing a different lock when handling that particular
end destination.
So to be "correct" locking-wise, we'd end up needing to grab a VAP or
node lock inside the driver TX path when setting up crypto / AMPDU sequence
numbers, and we may already _have_ a TX lock held - mostly for the same
destination vap/node, but sometimes it'll be for others. That could lead
to LORs and thus deadlocks.
So for now, I'm sticking with an IC TX lock. It has the advantage of
papering over the above and it also has the added advantage that I can
assert that it's being held when doing a parent device transmit.
I'll look at splitting the locks out a bit more later on.
General outstanding net80211 TX path issues / TODO:
* Look into separating out the VAP serialisation and the IC handoff.
It's going to be tricky as parent->if_transmit() doesn't give me the
opportunity to split queuing from driver dispatch. See above.
* Work with monthadar to fix up the mesh transmit path so it doesn't go via
the parent interface when retransmitting frames.
* Push the encryption handling back into the driver, if it's at all
architectually sane to do so. I know it's possible - it's what mac80211
in Linux does.
* Make ieee80211_raw_xmit() queue a frame into VAP or parent queue rather
than doing a short-cut direct into the driver. There are QoS issues
here - you do want your management frames to be encapsulated and pushed
onto the stack sooner than the (large, bursty) amount of data frames
that are queued. But there has to be a saner way to do this.
* Fragments are still broken - drivers need to be upgraded to an if_transmit()
implementation and then fragmentation handling needs to be properly fixed.
Tested:
* STA - AR5416, AR9280, Intel 5300 abgn wifi
* Hostap - AR5416, AR9160, AR9280
* Mesh - some testing by monthadar@, more to come.
The stageqdepth (global, over all staging queues) was being kept
incorrectly. It was being incremented whenever things were added,
but only decremented during a flush. During active fast frames activity
it wasn't being decremented, resulting in it always having a non-zero
value during normal fast-frames operation.
It was only used when checking if the aging queue should be checked;
we may as well just defer to each of those staging queue counters (which
look correct, thankfully.)
Whilst I'm here, add locking assertions in the staging queue add/remove
functions. The current crash shows that the staging queue has one frame,
but only has a tail pointer set (the head pointer being set to NULL.)
I'd like to grab a few more crashes where these locking assertions are
in place so I can narrow down the issue between "somehow locking is
messed up and things are racy" and "the stage queue head/tail pointer
manipulation logic is subtly wrong."
Tested:
* AR5416 STA, AR5413 AP; with FastFrames enabled in the AR5416 HAL.
PR: kern/174283
pointers and leave the stage queue flush routine to just do nothing
(since both head and tail here will be NULL.)
This should quieten the "stageq empty" panic where the stageq itself
is empty, but it won't fix the second KASSERT() here "staging queue empty"
as that's likely a different underlying problem.
PR: kern/174283
o replace DLT_IEEE802_11 support in net80211 with DLT_IEEE802_11_RADIO
and remove explicit bpf support from wireless drivers; drivers now
use ieee80211_radiotap_attach to setup shared data structures that
hold the radiotap header for each packet tx/rx
o remove rx timestamp from the rx path; it was used only by the tdma support
for debugging and was mostly useless due to it being 32-bits and mostly
unavailable
o track DLT_IEEE80211_RADIO bpf attachments and maintain per-vap and
per-com state when there are active taps
o track the number of monitor mode vaps
o use bpf tap and monitor mode vap state to decide when to collect radiotap
state and dispatch frames; drivers no longer explicitly directly check
bpf state or use bpf calls to tap frames
o handle radiotap state updates on channel change in net80211; drivers
should not do this (unless they bypass net80211 which is almost always
a mistake)
o update various drivers to be more consistent/correct in handling radiotap
o update ral to include TSF in radiotap'd frames
o add promisc mode callback to wi
Reviewed by: cbzimmer, rpaulo, thompsa
o call ieee80211_encap in ieee80211_start so frames passed down to drivers
are already encapsulated
o remove ieee80211_encap calls in drivers
o fixup wi so it recreates the 802.3 head it requires from the 802.11
header contents
o move fast-frame aggregation from ath to net80211 (conditional on
IEEE80211_SUPPORT_SUPERG):
- aggregation is now done in ieee80211_start; it is enabled when the
packets/sec exceeds ieee80211_ffppsmin (net.wlan.ffppsmin) and frames
are held on a staging queue according to ieee80211_ffagemax
(net.wlan.ffagemax) to wait for a frame to combine with
- drivers must call back to age/flush the staging queue (ath does this
on tx done, at swba, and on rx according to the state of the tx queues
and/or the contents of the staging queue)
- remove fast-frame-related data structures from ath
- add ieee80211_ff_node_init and ieee80211_ff_node_cleanup to handle
per-node fast-frames state (we reuse 11n tx ampdu state)
o change ieee80211_encap calling convention to include an explicit vap
so frames coming through a WDS vap are recognized w/o setting M_WDS
With these changes any device able to tx/rx 3Kbyte+ frames can use fast-frames.
Reviewed by: thompsa, rpaulo, avatar, imp, sephe
