15 Jun, 2010

10 commits

  • Even before the recent changes, the documentation
    for TX aggregation was somewhat out of date. Update
    it and also add documentation for the RX side.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • To prepare for allowing drivers to sleep in
    ampdu_action, change the locking in the TX
    aggregation code to use the mutex the RX part
    already uses. The spinlock is still necessary
    around some code to avoid races with TX, but
    now we can also synchronize_net() to avoid
    getting an inconsistent sequence number.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • Since we want the code to be able to sleep
    in the future, it must not be called from
    the timer directly. To achieve that, simply
    call the function drivers would call, and
    also use RCU in the timer to get the struct
    so we don't need to rely on the spinlock in
    the future.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • Move the block-ack session works into common
    code, since it will be needed for RX agg too
    in the next patches.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • When the driver or rate control requests starting
    or stopping an aggregation session, that currently
    causes a direct callback into the driver, which
    could potentially cause locking problems. Also,
    the functions need to be callable from contexts
    that cannot sleep, and thus will interfere with
    making the ampdu_action callback sleeping.

    To address these issues, add a new work item for
    each station that will process any start or stop
    requests out of line.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • mac80211 currently maintains the ampdu_lock to
    avoid starting a queue due to one aggregation
    session while another aggregation session needs
    the queue stopped.

    We can do better, however, and instead refcount
    the queue stops for this particular purpose,
    thus removing the need for the lock. This will
    help making ampdu_action able to sleep.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • The non-irqsafe aggregation start/stop done
    callbacks are currently only used by ath9k_htc,
    and can cause callbacks into the driver again.
    This might lead to locking issues, which will
    only get worse as we modify locking. To avoid
    trouble, remove the non-irqsafe versions and
    change ath9k_htc to use those instead.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • Currently we allocate some memory for each TX
    aggregation session and additionally keep a
    state bitmap indicating the state it is in.
    By using RCU to protect the pointer, moving
    the state into the structure and some locking
    trickery we can avoid locking when the TX agg
    session is fully operational.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • This moves the aggregation callback processing
    to the per-sdata skb queue and a work function
    rather than the tasklet.

    Unfortunately, this means that it extends the
    pkt_type hack to that skb queue. However, it
    will enable making ampdu_action API changes
    gradually, my current plan is to get rid of
    this again by forcing drivers to only return
    from ampdu_action() when everything is done,
    thus removing the callbacks completely.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • A number of places use RCU locking for accessing
    the station list, even though they do not need
    to. Use mutex locking instead to prepare for the
    locking changes I want to make. The mlme code is
    also using a WLAN_STA_DISASSOC flag that has the
    same meaning as WLAN_STA_BLOCK_BA, so use that.

    While doing so, combine places where we loop
    over stations twice, and optimise away some of
    the loops by checking if the hardware supports
    aggregation at all first.

    Also fix a more theoretical race condition: right
    now we could resume, set up an aggregation session,
    and right after tear it down again due to the code
    that is needed for hardware reconfiguration here.
    Also mark add a comment to that code marking it as
    a workaround.

    Finally, remove a pointless aggregation disabling
    loop when an interface is stopped, directly after
    that we remove all stations from it which will also
    disable all aggregation sessions that may still be
    active, and does so in a race-free way unlike the
    current loop that doesn't block new sessions.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     

08 Jun, 2010

1 commit


04 Jun, 2010

1 commit

  • There's no sense in letting anything but internal
    mac80211 functions set the initiator to anything
    but WLAN_BACK_INITIATOR, since WLAN_BACK_RECIPIENT
    is only valid when we have received a frame from
    the peer, which we react to directly in mac80211.

    The debugfs code I recently added got this wrong
    as well.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     

02 Jun, 2010

1 commit


24 Apr, 2010

1 commit


21 Apr, 2010

2 commits


20 Apr, 2010

2 commits


16 Apr, 2010

1 commit


09 Apr, 2010

1 commit

  • Enhance tracing by adding tracing for a variety of
    callbacks that the drivers call, and also for
    internal calls (currently limited to queue status).
    This can aid debugging what is going on in mac80211
    in interaction with drivers, since we can now see
    what drivers call and not just what mac80211 calls
    in the driver.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     

08 Apr, 2010

1 commit


01 Apr, 2010

1 commit


30 Mar, 2010

1 commit

  • …it slab.h inclusion from percpu.h

    percpu.h is included by sched.h and module.h and thus ends up being
    included when building most .c files. percpu.h includes slab.h which
    in turn includes gfp.h making everything defined by the two files
    universally available and complicating inclusion dependencies.

    percpu.h -> slab.h dependency is about to be removed. Prepare for
    this change by updating users of gfp and slab facilities include those
    headers directly instead of assuming availability. As this conversion
    needs to touch large number of source files, the following script is
    used as the basis of conversion.

    http://userweb.kernel.org/~tj/misc/slabh-sweep.py

    The script does the followings.

    * Scan files for gfp and slab usages and update includes such that
    only the necessary includes are there. ie. if only gfp is used,
    gfp.h, if slab is used, slab.h.

    * When the script inserts a new include, it looks at the include
    blocks and try to put the new include such that its order conforms
    to its surrounding. It's put in the include block which contains
    core kernel includes, in the same order that the rest are ordered -
    alphabetical, Christmas tree, rev-Xmas-tree or at the end if there
    doesn't seem to be any matching order.

    * If the script can't find a place to put a new include (mostly
    because the file doesn't have fitting include block), it prints out
    an error message indicating which .h file needs to be added to the
    file.

    The conversion was done in the following steps.

    1. The initial automatic conversion of all .c files updated slightly
    over 4000 files, deleting around 700 includes and adding ~480 gfp.h
    and ~3000 slab.h inclusions. The script emitted errors for ~400
    files.

    2. Each error was manually checked. Some didn't need the inclusion,
    some needed manual addition while adding it to implementation .h or
    embedding .c file was more appropriate for others. This step added
    inclusions to around 150 files.

    3. The script was run again and the output was compared to the edits
    from #2 to make sure no file was left behind.

    4. Several build tests were done and a couple of problems were fixed.
    e.g. lib/decompress_*.c used malloc/free() wrappers around slab
    APIs requiring slab.h to be added manually.

    5. The script was run on all .h files but without automatically
    editing them as sprinkling gfp.h and slab.h inclusions around .h
    files could easily lead to inclusion dependency hell. Most gfp.h
    inclusion directives were ignored as stuff from gfp.h was usually
    wildly available and often used in preprocessor macros. Each
    slab.h inclusion directive was examined and added manually as
    necessary.

    6. percpu.h was updated not to include slab.h.

    7. Build test were done on the following configurations and failures
    were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my
    distributed build env didn't work with gcov compiles) and a few
    more options had to be turned off depending on archs to make things
    build (like ipr on powerpc/64 which failed due to missing writeq).

    * x86 and x86_64 UP and SMP allmodconfig and a custom test config.
    * powerpc and powerpc64 SMP allmodconfig
    * sparc and sparc64 SMP allmodconfig
    * ia64 SMP allmodconfig
    * s390 SMP allmodconfig
    * alpha SMP allmodconfig
    * um on x86_64 SMP allmodconfig

    8. percpu.h modifications were reverted so that it could be applied as
    a separate patch and serve as bisection point.

    Given the fact that I had only a couple of failures from tests on step
    6, I'm fairly confident about the coverage of this conversion patch.
    If there is a breakage, it's likely to be something in one of the arch
    headers which should be easily discoverable easily on most builds of
    the specific arch.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>

    Tejun Heo
     

11 Feb, 2010

1 commit

  • In associated state, when bringing an interface down, existing
    BA sessions are torn down. When this is in progress, nothing
    prevents mac80211 from accepting another BA session start request.

    Use a new station flag to fix this.

    Signed-off-by: Sujith
    Acked-by: Johannes Berg
    Signed-off-by: John W. Linville

    Sujith
     

13 Jan, 2010

1 commit

  • Make addba_resp_timer aware the HT_AGG_STATE_REQ_STOP_BA_MSK mask
    so that when ___ieee80211_stop_tx_ba_session() is issued the timer
    will quit. Otherwise when suspend happens before the timer expired,
    the timer handler will be called immediately after resume and
    messes up driver status.

    Signed-off-by: Zhu Yi
    Acked-by: Johannes Berg
    Signed-off-by: John W. Linville

    Zhu Yi
     

06 Jan, 2010

1 commit

  • The start_seq_num is taken from the station's tid_seq[tid].
    This is fine, except tid_seq sequence counter is shifted
    by 4 bits to accommodate for frame fragmentation.

    Both (iwlagn & ath9k) were unaffected by this minor glitch,
    because they don't read the *ssn for the AMPDU_TX_START action.

    Signed-off-by: Christian Lamparter
    Signed-off-by: John W. Linville

    Christian Lamparter
     

22 Dec, 2009

3 commits

  • It's not all that useful to have the vif/sdata pointer,
    we'd rather refer to the interfaces by their name.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • For bluetooth 3, we will most likely not have
    a netdev for a virtual interface (sdata), so
    prepare for that by reducing the reliance on
    having a netdev. This patch moves the name
    and address fields into the sdata struct and
    uses them from there all over. Some work is
    needed to keep them sync'ed, but that's not
    a lot of work and in slow paths anyway.

    In doing so, this also reduces the number of
    pointer dereferences in many places, because
    of things like sdata->dev->dev_addr becoming
    sdata->vif.addr.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • The station management currently uses the virtual
    interface, but you cannot add the same station to
    multiple virtual interfaces if you're communicating
    with it in multiple ways.

    This restriction should be lifted so that in the
    future we can, for instance, support bluetooth 3
    with an access point that mac80211 is already
    associated to.

    We can do that by requiring all sta_info_get users
    to provide the virtual interface and making the RX
    code aware that an address may match more than one
    station struct. Thanks to the previous patches this
    one isn't all that large and except for the RX and
    TX status paths changes has low complexity.

    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     

02 Dec, 2009

1 commit


01 Dec, 2009

2 commits

  • 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 d75636ef9c1af224f1097941879d5a8db7cd04e5
    Author: Johannes Berg
    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 d92684e66091c0f0101819619b315b4bb8b5bcc5
    Author: Ron Rindjunsky
    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
    Cc: stable@kernel.org
    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • 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 d75636ef9c1af224f1097941879d5a8db7cd04e5
    Author: Johannes Berg
    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
    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     

29 Nov, 2009

1 commit


19 Nov, 2009

4 commits

  • 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
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • Not assigning the vif pointer causes an oops.
    This patch fixes it.

    Signed-off-by: Sujith
    Signed-off-by: John W. Linville

    Sujith
     
  • 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
    Signed-off-by: John W. Linville

    Johannes Berg
     
  • commit 2171abc58644e09dbba546d91366b12743115396
    Author: Johannes Berg
    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
    Signed-off-by: Johannes Berg
    Signed-off-by: John W. Linville

    Johannes Berg
     

31 Oct, 2009

1 commit

  • 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
    Signed-off-by: John W. Linville

    Johannes Berg
     

15 Aug, 2009

1 commit


14 Aug, 2009

1 commit

  • 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: [] 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:

    Pid: 0, comm: swapper Not tainted (2.6.31-rc5-wl #2) Dell DV051
    EIP: 0060:[] 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:
    Call Trace:
    [] ? ieee80211_stop_tx_ba_cb+0xab/0x150 [mac80211]
    [] ? ieee80211_tasklet_handler+0xce/0x110 [mac80211]
    [] ? net_rx_action+0xef/0x1d0
    [] ? tasklet_action+0x58/0xc0
    [] ? __do_softirq+0xc2/0x190
    [] ? handle_IRQ_event+0x58/0x140
    [] ? ack_apic_level+0x7e/0x270
    [] ? do_softirq+0x3d/0x40
    [] ? irq_exit+0x65/0x90
    [] ? do_IRQ+0x4f/0xc0
    [] ? irq_exit+0x7d/0x90
    [] ? smp_apic_timer_interrupt+0x57/0x90
    [] ? common_interrupt+0x29/0x30
    [] ? mwait_idle+0xbe/0x100
    [] ? cpu_idle+0x52/0x90
    [] ? rest_init+0x55/0x60
    [] ? start_kernel+0x315/0x37d
    [] ? unknown_bootoption+0x0/0x1f9
    [] ? i386_start_kernel+0x79/0x81
    Code:
    EIP: [] ieee80211_agg_splice_packets+0x40/0xc0 [mac80211] SS:ESP 0068:c076be38
    CR2: 0000000000000030

    Cc: stable@kernel.org
    Testedy-by: Jack Lau
    Signed-off-by: Luis R. Rodriguez
    Signed-off-by: John W. Linville

    Luis R. Rodriguez