2009-03-17 03:24:23

by Sujith

[permalink] [raw]
Subject: [PATCH v3] mac80211: Tear down aggregation sessions for suspend/resume

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 <[email protected]>
---
v2:
---
* Deny ADDBA requests if suspend is in progress.

v3:
---
* Use RCU protection when shutting down BA sessions for STAs.

net/mac80211/agg-rx.c | 8 ++++++++
net/mac80211/agg-tx.c | 9 +++++++++
net/mac80211/pm.c | 25 +++++++++++++++++++++++++
net/mac80211/sta_info.h | 3 +++
4 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
index a95affc..07656d8 100644
--- a/net/mac80211/agg-rx.c
+++ b/net/mac80211/agg-rx.c
@@ -197,6 +197,14 @@ void ieee80211_process_addba_request(struct ieee80211_local *local,

status = WLAN_STATUS_REQUEST_DECLINED;

+ if (test_sta_flags(sta, WLAN_STA_SUSPEND)) {
+#ifdef CONFIG_MAC80211_HT_DEBUG
+ printk(KERN_DEBUG "Suspend in progress. "
+ "Denying ADDBA request\n");
+#endif
+ goto end_no_lock;
+ }
+
/* sanity check for incoming parameters:
* check if configuration can support the BA policy
* and if buffer size does not exceeds max value */
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index 1df116d..e5776ef 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -257,6 +257,15 @@ int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
goto unlock;
}

+ if (test_sta_flags(sta, WLAN_STA_SUSPEND)) {
+#ifdef CONFIG_MAC80211_HT_DEBUG
+ printk(KERN_DEBUG "Suspend in progress. "
+ "Denying BA session request\n");
+#endif
+ ret = -EINVAL;
+ goto unlock;
+ }
+
spin_lock_bh(&sta->lock);

sdata = sta->sdata;
diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
index f845601..0273023 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -21,6 +21,19 @@ int __ieee80211_suspend(struct ieee80211_hw *hw)
list_for_each_entry(sdata, &local->interfaces, list)
ieee80211_disable_keys(sdata);

+ /* Tear down aggregation sessions */
+
+ rcu_read_lock();
+
+ if (hw->flags & IEEE80211_HW_AMPDU_AGGREGATION) {
+ list_for_each_entry_rcu(sta, &local->sta_list, list) {
+ set_sta_flags(sta, WLAN_STA_SUSPEND);
+ ieee80211_sta_tear_down_BA_sessions(sta);
+ }
+ }
+
+ rcu_read_unlock();
+
/* remove STAs */
if (local->ops->sta_notify) {
spin_lock_irqsave(&local->sta_lock, flags);
@@ -102,6 +115,18 @@ int __ieee80211_resume(struct ieee80211_hw *hw)
spin_unlock_irqrestore(&local->sta_lock, flags);
}

+ /* Clear Suspend state so that ADDBA requests can be processed */
+
+ rcu_read_lock();
+
+ if (hw->flags & IEEE80211_HW_AMPDU_AGGREGATION) {
+ list_for_each_entry_rcu(sta, &local->sta_list, list) {
+ clear_sta_flags(sta, WLAN_STA_SUSPEND);
+ }
+ }
+
+ rcu_read_unlock();
+
/* add back keys */
list_for_each_entry(sdata, &local->interfaces, list)
if (netif_running(sdata->dev))
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 1f45573..5b223b2 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -35,6 +35,8 @@
* IEEE80211_TX_CTL_CLEAR_PS_FILT control flag) when the next
* frame to this station is transmitted.
* @WLAN_STA_MFP: Management frame protection is used with this STA.
+ * @WLAN_STA_SUSPEND: Set/cleared during a suspend/resume cycle.
+ * Used to deny ADDBA requests (both TX and RX).
*/
enum ieee80211_sta_info_flags {
WLAN_STA_AUTH = 1<<0,
@@ -48,6 +50,7 @@ enum ieee80211_sta_info_flags {
WLAN_STA_PSPOLL = 1<<8,
WLAN_STA_CLEAR_PS_FILT = 1<<9,
WLAN_STA_MFP = 1<<10,
+ WLAN_STA_SUSPEND = 1<<11
};

#define STA_TID_NUM 16
--
1.6.2



2009-03-17 07:47:08

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3] mac80211: Tear down aggregation sessions for suspend/resume

On Tue, 2009-03-17 at 08:50 +0530, Sujith wrote:
> 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 <[email protected]>
> ---
> v2:
> ---
> * Deny ADDBA requests if suspend is in progress.
>
> v3:
> ---
> * Use RCU protection when shutting down BA sessions for STAs.

Thanks for your patience :)

Acked-by: Johannes Berg <[email protected]>

> net/mac80211/agg-rx.c | 8 ++++++++
> net/mac80211/agg-tx.c | 9 +++++++++
> net/mac80211/pm.c | 25 +++++++++++++++++++++++++
> net/mac80211/sta_info.h | 3 +++
> 4 files changed, 45 insertions(+), 0 deletions(-)
>
> diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
> index a95affc..07656d8 100644
> --- a/net/mac80211/agg-rx.c
> +++ b/net/mac80211/agg-rx.c
> @@ -197,6 +197,14 @@ void ieee80211_process_addba_request(struct ieee80211_local *local,
>
> status = WLAN_STATUS_REQUEST_DECLINED;
>
> + if (test_sta_flags(sta, WLAN_STA_SUSPEND)) {
> +#ifdef CONFIG_MAC80211_HT_DEBUG
> + printk(KERN_DEBUG "Suspend in progress. "
> + "Denying ADDBA request\n");
> +#endif
> + goto end_no_lock;
> + }
> +
> /* sanity check for incoming parameters:
> * check if configuration can support the BA policy
> * and if buffer size does not exceeds max value */
> diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
> index 1df116d..e5776ef 100644
> --- a/net/mac80211/agg-tx.c
> +++ b/net/mac80211/agg-tx.c
> @@ -257,6 +257,15 @@ int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
> goto unlock;
> }
>
> + if (test_sta_flags(sta, WLAN_STA_SUSPEND)) {
> +#ifdef CONFIG_MAC80211_HT_DEBUG
> + printk(KERN_DEBUG "Suspend in progress. "
> + "Denying BA session request\n");
> +#endif
> + ret = -EINVAL;
> + goto unlock;
> + }
> +
> spin_lock_bh(&sta->lock);
>
> sdata = sta->sdata;
> diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
> index f845601..0273023 100644
> --- a/net/mac80211/pm.c
> +++ b/net/mac80211/pm.c
> @@ -21,6 +21,19 @@ int __ieee80211_suspend(struct ieee80211_hw *hw)
> list_for_each_entry(sdata, &local->interfaces, list)
> ieee80211_disable_keys(sdata);
>
> + /* Tear down aggregation sessions */
> +
> + rcu_read_lock();
> +
> + if (hw->flags & IEEE80211_HW_AMPDU_AGGREGATION) {
> + list_for_each_entry_rcu(sta, &local->sta_list, list) {
> + set_sta_flags(sta, WLAN_STA_SUSPEND);
> + ieee80211_sta_tear_down_BA_sessions(sta);
> + }
> + }
> +
> + rcu_read_unlock();
> +
> /* remove STAs */
> if (local->ops->sta_notify) {
> spin_lock_irqsave(&local->sta_lock, flags);
> @@ -102,6 +115,18 @@ int __ieee80211_resume(struct ieee80211_hw *hw)
> spin_unlock_irqrestore(&local->sta_lock, flags);
> }
>
> + /* Clear Suspend state so that ADDBA requests can be processed */
> +
> + rcu_read_lock();
> +
> + if (hw->flags & IEEE80211_HW_AMPDU_AGGREGATION) {
> + list_for_each_entry_rcu(sta, &local->sta_list, list) {
> + clear_sta_flags(sta, WLAN_STA_SUSPEND);
> + }
> + }
> +
> + rcu_read_unlock();
> +
> /* add back keys */
> list_for_each_entry(sdata, &local->interfaces, list)
> if (netif_running(sdata->dev))
> diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
> index 1f45573..5b223b2 100644
> --- a/net/mac80211/sta_info.h
> +++ b/net/mac80211/sta_info.h
> @@ -35,6 +35,8 @@
> * IEEE80211_TX_CTL_CLEAR_PS_FILT control flag) when the next
> * frame to this station is transmitted.
> * @WLAN_STA_MFP: Management frame protection is used with this STA.
> + * @WLAN_STA_SUSPEND: Set/cleared during a suspend/resume cycle.
> + * Used to deny ADDBA requests (both TX and RX).
> */
> enum ieee80211_sta_info_flags {
> WLAN_STA_AUTH = 1<<0,
> @@ -48,6 +50,7 @@ enum ieee80211_sta_info_flags {
> WLAN_STA_PSPOLL = 1<<8,
> WLAN_STA_CLEAR_PS_FILT = 1<<9,
> WLAN_STA_MFP = 1<<10,
> + WLAN_STA_SUSPEND = 1<<11
> };
>
> #define STA_TID_NUM 16


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2009-03-19 00:46:35

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH v3] mac80211: Tear down aggregation sessions for suspend/resume

On Tue, Mar 17, 2009 at 08:46:59AM +0100, Johannes Berg wrote:
> On Tue, 2009-03-17 at 08:50 +0530, Sujith wrote:
> > This can be fixed by tearing down the existing sessions on a suspend.
> >
> > v3:
> > ---
> > * Use RCU protection when shutting down BA sessions for STAs.
>
> Thanks for your patience :)
>
> Acked-by: Johannes Berg <[email protected]>

FWIW, I finally tested this formulation and it works fine on my hardware,
so I think the deadlock is long buried.

--
Bob Copeland %% http://www.bobcopeland.com