Lennert Buytenhek noticed that delBA handling in mac80211
was broken and has remotely triggerable problems, some of
which are due to some code shuffling I did that ended up
changing the order in which things were done -- this was
commit d75636ef9c
Author: Johannes Berg <johannes@sipsolutions.net>
Date: Tue Feb 10 21:25:53 2009 +0100
mac80211: RX aggregation: clean up stop session
and other parts were already present in the original
commit d92684e660
Author: Ron Rindjunsky <ron.rindjunsky@intel.com>
Date: Mon Jan 28 14:07:22 2008 +0200
mac80211: A-MPDU Tx add delBA from recipient support
The first problem is that I moved a BUG_ON before various
checks -- thereby making it possible to hit. As the comment
indicates, the BUG_ON can be removed since the ampdu_action
callback must already exist when the state is != IDLE.
The second problem isn't easily exploitable but there's a
race condition due to unconditionally setting the state to
OPERATIONAL when a delBA frame is received, even when no
aggregation session was ever initiated. All the drivers
accept stopping the session even then, but that opens a
race window where crashes could happen before the driver
accepts it. Right now, a WARN_ON may happen with non-HT
drivers, while the race opens only for HT drivers.
For this case, there are two things necessary to fix it:
1) don't process spurious delBA frames, and be more careful
about the session state; don't drop the lock
2) HT drivers need to be prepared to handle a session stop
even before the session was really started -- this is
true for all drivers (that support aggregation) but
iwlwifi which can be fixed easily. The other HT drivers
(ath9k and ar9170) are behaving properly already.
Reported-by: Lennert Buytenhek <buytenh@marvell.com>
Cc: stable@kernel.org
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Lennert Buytenhek noticed a remotely triggerable problem
in mac80211, which is due to some code shuffling I did
that ended up changing the order in which things were
done -- this was in
commit d75636ef9c
Author: Johannes Berg <johannes@sipsolutions.net>
Date: Tue Feb 10 21:25:53 2009 +0100
mac80211: RX aggregation: clean up stop session
The problem is that the BUG_ON moved before the various
checks, and as such can be triggered.
As the comment indicates, the BUG_ON can be removed since
the ampdu_action callback must already exist when the
state is OPERATIONAL.
A similar code path leads to a WARN_ON in
ieee80211_stop_tx_ba_session, which can also be removed.
Cc: stable@kernel.org [2.6.29+]
Cc: Lennert Buytenhek <buytenh@marvell.com>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Since the flags moved into skb->cb, there's no
longer a need to have the encrypt bool passed
into the function, anyone who requires it set
to 0 (false) can just set the flag directly.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Not assigning the vif pointer causes an oops.
This patch fixes it.
Signed-off-by: Sujith <Sujith.Manoharan@atheros.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
The entire aggregation code currently operates on the
hw pointer and station addresses, but that needs to
change to make stations purely per-vif; As one step
preparing for that make the aggregation code callable
with the station, or by the combination of virtual
interface and station address.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
commit 2171abc586
Author: Johannes Berg <johannes@sipsolutions.net>
Date: Thu Oct 29 08:34:00 2009 +0100
mac80211: fix addba timer
left a problem in there, even if the timer was
never started it could be deleted and then added.
Linus pointed out that del_timer_sync() isn't
actually needed if we make the timer able to
deal with no longer being needed when it gets
queued _while_ we're in the locked section that
also deletes it. For that the timer function only
needs to check the HT_ADDBA_RECEIVED_MSK bit as
well as the HT_ADDBA_REQUESTED_MSK bit, only if
the former is clear should it do anything.
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
The addba timer function acquires the sta spinlock,
but at the same time we try to del_timer_sync() it
under the spinlock which can produce deadlocks.
To fix this, always del_timer_sync() the timer in
ieee80211_process_addba_resp() and add it again
after checking the conditions, if necessary.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
We splice skbs from the pending queue for a TID
onto the local pending queue when tearing down a
block ack request. This is not necessary unless we
actually have received a request to start a block ack
request (rate control, for example). If we never received
that request we should not be splicing the tid pending
queue as it would be null, causing a panic.
Not sure yet how exactly we allowed through a call when the
tid state does not have at least HT_ADDBA_REQUESTED_MSK set,
that will require some further review as it is not quite
obvious.
For more information see the bug report:
http://bugzilla.kernel.org/show_bug.cgi?id=13922
This fixes this oops:
BUG: unable to handle kernel NULL pointer dereference at 00000030
IP: [<f8806c70>] ieee80211_agg_splice_packets+0x40/0xc0 [mac80211]
*pdpt = 0000000002d1e001 *pde = 0000000000000000
Thread overran stack, or stack corrupted
Oops: 0000 [#1] SMP
last sysfs file: /sys/module/aes_generic/initstate
Modules linked in: <bleh>
Pid: 0, comm: swapper Not tainted (2.6.31-rc5-wl #2) Dell DV051
EIP: 0060:[<f8806c70>] EFLAGS: 00010292 CPU: 0
EIP is at ieee80211_agg_splice_packets+0x40/0xc0 [mac80211]
EAX: 00000030 EBX: 0000004c ECX: 00000003 EDX: 00000000
ESI: c1c98000 EDI: f745a1c0 EBP: c076be58 ESP: c076be38
DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
Process swapper (pid: 0, ti=c076a000 task=c0709160 task.ti=c076a000)
Stack: <bleh2>
Call Trace:
[<f8806edb>] ? ieee80211_stop_tx_ba_cb+0xab/0x150 [mac80211]
[<f8802f1e>] ? ieee80211_tasklet_handler+0xce/0x110 [mac80211]
[<c04862ff>] ? net_rx_action+0xef/0x1d0
[<c0149378>] ? tasklet_action+0x58/0xc0
[<c014a0f2>] ? __do_softirq+0xc2/0x190
[<c018eb48>] ? handle_IRQ_event+0x58/0x140
[<c01205fe>] ? ack_apic_level+0x7e/0x270
[<c014a1fd>] ? do_softirq+0x3d/0x40
[<c014a345>] ? irq_exit+0x65/0x90
[<c010a6af>] ? do_IRQ+0x4f/0xc0
[<c014a35d>] ? irq_exit+0x7d/0x90
[<c011d547>] ? smp_apic_timer_interrupt+0x57/0x90
[<c01094a9>] ? common_interrupt+0x29/0x30
[<c010fd9e>] ? mwait_idle+0xbe/0x100
[<c0107e42>] ? cpu_idle+0x52/0x90
[<c054b1a5>] ? rest_init+0x55/0x60
[<c077492d>] ? start_kernel+0x315/0x37d
[<c07743ce>] ? unknown_bootoption+0x0/0x1f9
[<c0774099>] ? i386_start_kernel+0x79/0x81
Code: <bleh3>
EIP: [<f8806c70>] ieee80211_agg_splice_packets+0x40/0xc0 [mac80211] SS:ESP 0068:c076be38
CR2: 0000000000000030
Cc: stable@kernel.org
Testedy-by: Jack Lau <jackelectronics@hotmail.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
With the internal 'pending' queue system in place, we can simply
put packets there instead of pushing them off to the master dev,
getting rid of the master interface completely.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
addba_req_num[tid] is supposed to have the count of consecutive
addba request attempts on 'tid' which failed. This count is checked
against a retry threshold (3 times) before starting the addba negotiation.
This patch fixes the way this addba count is incremented/reset and thereby
avoids indefinite addba attempts.
Signed-off-by: Vasanthakumar Thiagarajan <vasanth@atheros.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
In order to later add tracing or verifications to the driver
calls mac80211 makes, this patch adds static inline wrappers
for all operations.
All calls are now written as
drv_<op>(local, ...);
instead of
local->ops-><op>(&local->hw, ...);
Where necessary, the wrappers also do existence checking and
return default values as appropriate.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
This patch removes all the virtual A-MPDU-queue bookkeeping from
mac80211. Curiously, iwlwifi already does its own bookkeeping, so
it doesn't require much changes except where it needs to handle
starting and stopping the queues in mac80211.
To handle the queue stop/wake properly, we rewrite the software
queue number for aggregation frames and internally to iwlwifi keep
track of the queues that map into the same AC queue, and only talk
to mac80211 about the AC queue. The implementation requires calling
two new functions, iwl_stop_queue and iwl_wake_queue instead of the
mac80211 counterparts.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: Reinette Chattre <reinette.chatre@intel.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Instead of stopping the entire AC queue when enabling aggregation
(which was only done for hardware with aggregation queues) buffer
the packets for each station, and release them to the pending skb
queue once aggregation is turned on successfully.
We get a little more code, but it becomes conceptually simpler and
we can remove the entire virtual queue mechanism from mac80211 in
a follow-up patch.
This changes how mac80211 behaves towards drivers that support
aggregation but have no hardware queues -- those drivers will now
not be handed packets while the aggregation session is being
established, but only after it has been fully established.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
When TX aggregation becomes operational, we do a number of steps:
1) print a debug message
2) wake the virtual queue
3) notify the driver
Unfortunately, 1) and 3) are only done if the driver is first to
reply to the aggregation request, it is, however, possible that the
remote station replies before the driver! Thus, unify the code for
this and call the new function ieee80211_agg_tx_operational in both
places where TX aggregation can become operational.
Additionally, rename the driver notification from
IEEE80211_AMPDU_TX_RESUME to IEEE80211_AMPDU_TX_OPERATIONAL.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
When the driver has been notified with a STA_REMOVE, it tears down
the internal ADDBA state. On resume, trying to initiate aggregation would
fail because mac80211 has not cleared the operational state for that <TID,STA>.
This can be fixed by tearing down the existing sessions on a suspend.
Also, the driver can initiate a new BA session when suspend is in progress.
This is fixed by marking the station as being in suspend state and
denying ADDBA requests for such STAs.
Signed-off-by: Sujith <Sujith.Manoharan@atheros.com>
Acked-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
This patch splits out the ibss code and data from managed (station) mode.
The reason to do this is to better separate the state machines, and have
the code be contained better so it gets easier to determine what exactly
a given change will affect, that in turn makes it easier to understand.
This is quite some churn, especially because I split sdata->u.sta into
sdata->u.mgd and sdata->u.ibss, but I think it's easier to maintain that
way. I've also shuffled around some code -- null function sending is only
applicable to managed interfaces so put that into that file, some other
functions are needed from various places so put them into util, and also
rearranged the prototypes in ieee80211_i.h accordingly.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Hardware with AMPDU queues currently has broken aggregation.
This patch fixes it by making all A-MPDUs go over the regular AC queues,
but keeping track of the hardware queues in mac80211. As a first rough
version, it actually stops the AC queue for extended periods of time,
which can be removed by adding buffering internal to mac80211, but is
currently not a huge problem because people rarely use multiple TIDs
that are in the same AC (and iwlwifi currently doesn't operate as AP).
This is a short-term fix, my current medium-term plan, which I hope to
execute soon as well, but am not sure can finish before .30, looks like
this:
1) rework the internal queuing layer in mac80211 that we use for
fragments if the driver stopped queue in the middle of a fragmented
frame to be able to queue more frames at once (rather than just a
single frame with its fragments)
2) instead of stopping the entire AC queue, queue up the frames in a
per-station/per-TID queue during aggregation session initiation,
when the session has come up take all those frames and put them
onto the queue from 1)
3) push the ampdu queue layer abstraction this patch introduces in
mac80211 into the driver, and remove the virtual queue stuff from
mac80211 again
This plan will probably also affect ath9k in that mac80211 queues the
frames instead of passing them down, even when there are no ampdu queues.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Essentially consisting of passing the sta_info pointer around,
instead of repeatedly doing hash lookups.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
When disabling TX aggregation because it was rejected or from
the timer (it was not accepted), there is a window where we
first set the state to operation, unlock, and then undo the
whole thing. Avoid that by splitting up the stop function.
Also get rid of the pointless sta_info indirection in the timer.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Add documentation and move ieee80211_start_tx_ba_cb_irqsafe to right
after ieee80211_start_tx_ba_cb.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Doing so would be an MLME protocol violation when the peer disabled
the aggregation session. Quick driver review indicates that there are
error codes passed all over the drivers but cannot ever be nonzero
except in error conditions that would indicate mac80211 bugs.
No real changes here, since no drivers currently can return -EBUSY.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
We can only support aggregation on AP/STA right now. HT isn't defined
for IBSS, WDS or MESH. In the WDS/MESH cases it's not clear what to
put into the IBSS field, and we don't handle that in the code at all.
Also fix the code to handle VLAN correctly.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Create two new files, agg-tx.c and agg-rx.c to make it clearer
which code is common (ht.c) and which is specific (agg-*.c).
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
