Skip to content

Commit

Permalink
mac80211: fix station wakeup powersave race
Browse files Browse the repository at this point in the history
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 50a9432
("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: Johannes Berg <johannes.berg@intel.com>
  • Loading branch information
Johannes Berg committed Feb 20, 2014
1 parent 5108ca8 commit e3685e0
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 32 deletions.
10 changes: 2 additions & 8 deletions net/mac80211/ieee80211_i.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
7 changes: 7 additions & 0 deletions net/mac80211/rx.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
32 changes: 14 additions & 18 deletions net/mac80211/sta_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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 ||
Expand All @@ -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);
Expand Down Expand Up @@ -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) &&
Expand Down
8 changes: 2 additions & 6 deletions net/mac80211/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down

0 comments on commit e3685e0

Please sign in to comment.