Re: [PATCH] mac80211: delete addba_resp_timer when stop Tx BA session

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 1757 bytes --]

On Mon, 2009-12-28 at 11:18 +0800, Zhu Yi wrote:
> We start an one shot addba_resp_timer in ieee80211_start_tx_ba_session
> but don't delete it in __ieee80211_stop_tx_ba_session. This will cause
> problem in suspend and resume. When suspend happens without the timer
> deleted, the timer handler will be called immediately after resume and
> messes up driver status.
> 
> See http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2134
> comment #22 for more details.

Hmm. The timer checks

        if ((*state & (HT_ADDBA_REQUESTED_MSK | HT_ADDBA_RECEIVED_MSK)) !=
                                                HT_ADDBA_REQUESTED_MSK) {

which /should/ make it a no-op in the case you mention because we tear
down sessions, or does that not reset their state properly?

Ok, so ___ieee80211_stop_tx_ba_session() doesn't, in fact, clear the
mask right away, so I think we should add HT_AGG_STATE_REQ_STOP_BA_MSK
to the code I quoted above?

In any case the del_timer_sync isn't right there and we also shouldn't
really use it (per Linus)

johannes

> Cc: Johannes Berg <johannes@sipsolutions.net>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> ---
>  net/mac80211/agg-tx.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
> index ceda366..d122082 100644
> --- a/net/mac80211/agg-tx.c
> +++ b/net/mac80211/agg-tx.c
> @@ -509,6 +509,8 @@ int __ieee80211_stop_tx_ba_session(struct sta_info
> *sta, u16 tid,
>  	u8 *state;
>  	int ret;
>  
> +	del_timer_sync(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer);
> +
>  	/* check if the TID is in aggregation */
>  	state = &sta->ampdu_mlme.tid_state_tx[tid];
>  	spin_lock_bh(&sta->lock);


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]