wifi: mac80211: Power-save state and TIM handling fixes

Following up on the v1 discussion [0], this two-patch series:

1. Makes AP-side Power Management (PM) bit handling in ieee80211_rx_h_sta_process() symmetric across both wake-to-doze and doze-to-wake transitions, ignoring the reserved PM bit in non-bufferable management frames while still waking sleeping stations on Authentication and (Re)Association Request frames per commit 9fef65443388 ("mac80211: always update the PM state of a peer on MGMT / DATA frames").

2. Ensures an awake client station sends a wake nullfunc frame in ieee80211_rx_mgmt_beacon() when the Beacon TIM element indicates buffered unicast traffic for its AID, even when dynamic power-save is disabled (dynamic_ps_timeout == 0) and IEEE80211_CONF_PS is not set.

v1 => v2:
- Make non-bufferable MMPDU PM-bit handling symmetric in both directions and expand the commit description.
- Add client-side wake nullfunc transmission in ieee80211_rx_mgmt_beacon() when the TIM bit is set while awake.

[0] https://lore.kernel.org/r/[email protected]

Lee Jones (2):
wifi: mac80211: ignore PM bit in non-bufferable MMPDUs
wifi: mac80211: send wake nullfunc when TIM bit is set while awake

 net/mac80211/mlme.c |  3 ++-
 net/mac80211/rx.c   | 22 ++++++++++++++++------
 2 files changed, 18 insertions(+), 7 deletions(-)

2.56.0.313.g2479322e68-goog

In AP mode, the Power Management (PM) subfield in the Frame Control header is reserved in non-bufferable management frames (such as Probe Request, Authentication, and (Re)Association Request). Transmitters set reserved subfields to zero and receivers are expected to ignore them.

Commit 9fef65443388 ("mac80211: always update the PM state of a peer on MGMT / DATA frames") removed the bufferable MMPDU check from ieee80211_rx_h_sta_process() so that a sleeping station that disappears and later re-authenticates transitions back to awake (sta_ps_end()). However, removing the check altogether caused the AP to honour the reserved PM bit on all non-bufferable management frames in both directions:

- A sleeping station transmitting a non-bufferable MMPDU with the reserved PM bit set to 0 (for example, a Probe Request) is spuriously transitioned out of power-save mode.

- An awake station for which a non-bufferable MMPDU with the reserved PM bit set to 1 is received (for example, a spoofed Association Request) is transitioned into power-save mode, causing the AP to buffer subsequent frames such as SA Query Requests.

Only inspect the PM bit on data frames and bufferable management frame subtypes (Action, Disassociation, and Deauthentication) via hdr->frame_control, avoiding inspection of encrypted Action payloads prior to ieee80211_rx_h_decrypt(). For Authentication and (Re)Association Request frames, where a station is always in Active mode per IEEE 802.11, transition a sleeping station to awake without consulting the reserved PM bit.

---

v1 => v2:
- Make non-bufferable MMPDU PM-bit handling symmetric across both wake-to-doze and doze-to-wake transitions
- Expand commit description explaining both transitions and why Authentication/(Re)Association Request still wake sleeping stations

 net/mac80211/rx.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

index ecee9348c70d..29383fc85a63 100644
@@ -1920,12 +1920,22 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
 	    !(status->rx_flags & IEEE80211_RX_DEFERRED_RELEASE) &&
 	    (rx->sdata->vif.type == NL80211_IFTYPE_AP ||
 	     rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN)) {
-		if (test_sta_flag(sta, WLAN_STA_PS_STA)) {
-			if (!ieee80211_has_pm(hdr->frame_control))
-				sta_ps_end(sta);
-		} else {
-			if (ieee80211_has_pm(hdr->frame_control))
-				sta_ps_start(sta);
+		if (ieee80211_is_data(hdr->frame_control) ||
+		    ieee80211_is_action(hdr->frame_control) ||
+		    ieee80211_is_disassoc(hdr->frame_control) ||
+		    ieee80211_is_deauth(hdr->frame_control)) {
+			if (test_sta_flag(sta, WLAN_STA_PS_STA)) {
+				if (!ieee80211_has_pm(hdr->frame_control))
+					sta_ps_end(sta);
+			} else {
+				if (ieee80211_has_pm(hdr->frame_control))
+					sta_ps_start(sta);
+			}
+		} else if (test_sta_flag(sta, WLAN_STA_PS_STA) &&
+			   (ieee80211_is_auth(hdr->frame_control) ||
+			    ieee80211_is_assoc_req(hdr->frame_control) ||
+			    ieee80211_is_reassoc_req(hdr->frame_control))) {
+			sta_ps_end(sta);
 		}
 	}
 

2.56.0.313.g2479322e68-goog

When an AP's Beacon TIM element indicates buffered unicast traffic for a station's AID, the AP believes the station is in power-save mode. If the station is actually awake (IEEE80211_CONF_PS is not set) and dynamic power-save is disabled (dynamic_ps_timeout == 0, for instance when power-save is disabled on the interface), ieee80211_rx_mgmt_beacon() currently takes neither the dynamic-PS wakeup branch nor the PS-Poll branch.

As a result, if the AP's power-save state for the station becomes desynchronised (for example, due to an unauthenticated frame with the PM bit set), an awake client with power-save disabled ignores its AID bit in the TIM and never transmits a nullfunc frame with PM=0 to flush the AP's buffered frames.

Send a wake nullfunc frame in ieee80211_rx_mgmt_beacon() whenever the TIM indicates buffered traffic for the station while IEEE80211_CONF_PS is not set.

---

v1 => v2: New patch per Johannes Berg's review suggestion

 net/mac80211/mlme.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 6ec45a9f7221..802c714ba852 100644
@@ -8286,7 +8286,8 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_link_data *link,
 	if (ieee80211_hw_check(&local->hw, PS_NULLFUNC_STACK) &&
 	    ieee80211_check_tim(elems->tim, elems->tim_len, vif_cfg->aid,
 				vif_cfg->s1g)) {
-		if (local->hw.conf.dynamic_ps_timeout > 0) {
+		if (local->hw.conf.dynamic_ps_timeout > 0 ||
+		    !(local->hw.conf.flags & IEEE80211_CONF_PS)) {
 			if (local->hw.conf.flags & IEEE80211_CONF_PS) {
 				local->hw.conf.flags &= ~IEEE80211_CONF_PS;
 				ieee80211_hw_config(local, -1,

2.56.0.313.g2479322e68-goog

I'm somewhat worried that this is going to cause another issue on broken APs that just never clear the TIM and we'd send NDPs all the time, but I guess if that were the case then we'd have figured it out in iwlwifi maybe.

I might still not want to put it in wireless/stable until we have more experience with it though.

(Also not sure about the other patch for stable, it's a bit more of a cleanup really?)

johannes

Your call bud.

If you want to strip the Cc: and Fixes: tags, I have no issue with it.

Lee Jones