Commit e3685e03 authored by Johannes Berg's avatar Johannes Berg

mac80211: fix station wakeup powersave race

Consider the following (relatively unlikely) scenario:
 1) station goes to sleep while frames are buffered in driver
 2) driver blocks wakeup (until no more frames are buffered)
 3) station wakes up again
 4) driver unblocks wakeup

In this case, the current mac80211 code will do the following:
 1) WLAN_STA_PS_STA set
 2) WLAN_STA_PS_DRIVER set
 3) - nothing -
 4) WLAN_STA_PS_DRIVER cleared

As a result, no frames will be delivered to the client, even
though it is awake, until it sends another frame to us that
triggers ieee80211_sta_ps_deliver_wakeup() in sta_ps_end().

Since we now take the PS spinlock, we can fix this while at
the same time removing the complexity with the pending skb
queue function. This was broken since my commit 50a9432d
("mac80211: fix powersaving clients races") due to removing
the clearing of WLAN_STA_PS_STA in the RX path.

While at it, fix a cleanup path issue when a station is
removed while the driver is still blocking its wakeup.
Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
parent 5108ca82
......@@ -1692,14 +1692,8 @@ void ieee80211_stop_queue_by_reason(struct ieee80211_hw *hw, int queue,
void ieee80211_propagate_queue_wake(struct ieee80211_local *local, int queue);
void ieee80211_add_pending_skb(struct ieee80211_local *local,
struct sk_buff *skb);
void ieee80211_add_pending_skbs_fn(struct ieee80211_local *local,
struct sk_buff_head *skbs,
void (*fn)(void *data), void *data);
static inline void ieee80211_add_pending_skbs(struct ieee80211_local *local,
struct sk_buff_head *skbs)
{
ieee80211_add_pending_skbs_fn(local, skbs, NULL, NULL);
}
void ieee80211_add_pending_skbs(struct ieee80211_local *local,
struct sk_buff_head *skbs);
void ieee80211_flush_queues(struct ieee80211_local *local,
struct ieee80211_sub_if_data *sdata);
......
......@@ -1128,6 +1128,13 @@ static void sta_ps_end(struct sta_info *sta)
sta->sta.addr, sta->sta.aid);
if (test_sta_flag(sta, WLAN_STA_PS_DRIVER)) {
/*
* Clear the flag only if the other one is still set
* so that the TX path won't start TX'ing new frames
* directly ... In the case that the driver flag isn't
* set ieee80211_sta_ps_deliver_wakeup() will clear it.
*/
clear_sta_flag(sta, WLAN_STA_PS_STA);
ps_dbg(sta->sdata, "STA %pM aid %d driver-ps-blocked\n",
sta->sta.addr, sta->sta.aid);
return;
......
......@@ -99,7 +99,8 @@ static void __cleanup_single_sta(struct sta_info *sta)
struct ieee80211_local *local = sdata->local;
struct ps_data *ps;
if (test_sta_flag(sta, WLAN_STA_PS_STA)) {
if (test_sta_flag(sta, WLAN_STA_PS_STA) ||
test_sta_flag(sta, WLAN_STA_PS_DRIVER)) {
if (sta->sdata->vif.type == NL80211_IFTYPE_AP ||
sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
ps = &sdata->bss->ps;
......@@ -109,6 +110,7 @@ static void __cleanup_single_sta(struct sta_info *sta)
return;
clear_sta_flag(sta, WLAN_STA_PS_STA);
clear_sta_flag(sta, WLAN_STA_PS_DRIVER);
atomic_dec(&ps->num_sta_ps);
sta_info_recalc_tim(sta);
......@@ -1090,10 +1092,14 @@ struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_vif *vif,
}
EXPORT_SYMBOL(ieee80211_find_sta);
static void clear_sta_ps_flags(void *_sta)
/* powersave support code */
void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta)
{
struct sta_info *sta = _sta;
struct ieee80211_sub_if_data *sdata = sta->sdata;
struct ieee80211_local *local = sdata->local;
struct sk_buff_head pending;
int filtered = 0, buffered = 0, ac;
unsigned long flags;
struct ps_data *ps;
if (sdata->vif.type == NL80211_IFTYPE_AP ||
......@@ -1104,20 +1110,6 @@ static void clear_sta_ps_flags(void *_sta)
else
return;
clear_sta_flag(sta, WLAN_STA_PS_DRIVER);
if (test_and_clear_sta_flag(sta, WLAN_STA_PS_STA))
atomic_dec(&ps->num_sta_ps);
}
/* powersave support code */
void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta)
{
struct ieee80211_sub_if_data *sdata = sta->sdata;
struct ieee80211_local *local = sdata->local;
struct sk_buff_head pending;
int filtered = 0, buffered = 0, ac;
unsigned long flags;
clear_sta_flag(sta, WLAN_STA_SP);
BUILD_BUG_ON(BITS_TO_LONGS(IEEE80211_NUM_TIDS) > 1);
......@@ -1148,9 +1140,13 @@ void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta)
buffered += tmp - count;
}
ieee80211_add_pending_skbs_fn(local, &pending, clear_sta_ps_flags, sta);
ieee80211_add_pending_skbs(local, &pending);
clear_sta_flag(sta, WLAN_STA_PS_DRIVER);
clear_sta_flag(sta, WLAN_STA_PS_STA);
spin_unlock(&sta->ps_lock);
atomic_dec(&ps->num_sta_ps);
/* This station just woke up and isn't aware of our SMPS state */
if (!ieee80211_smps_is_restrictive(sta->known_smps_mode,
sdata->smps_mode) &&
......
......@@ -435,9 +435,8 @@ void ieee80211_add_pending_skb(struct ieee80211_local *local,
spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
}
void ieee80211_add_pending_skbs_fn(struct ieee80211_local *local,
struct sk_buff_head *skbs,
void (*fn)(void *data), void *data)
void ieee80211_add_pending_skbs(struct ieee80211_local *local,
struct sk_buff_head *skbs)
{
struct ieee80211_hw *hw = &local->hw;
struct sk_buff *skb;
......@@ -461,9 +460,6 @@ void ieee80211_add_pending_skbs_fn(struct ieee80211_local *local,
__skb_queue_tail(&local->pending[queue], skb);
}
if (fn)
fn(data);
for (i = 0; i < hw->queues; i++)
__ieee80211_wake_queue(hw, i,
IEEE80211_QUEUE_STOP_REASON_SKB_ADD);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment