Skip to contentSkip to conversation

mac80211: Enable power save after receiving NULL packet ACK

PATCH In review Bassem Dawood 2 participants 4 messages Feb 27, 2021 – Apr 20, 2021 mac80211

lore
Share & tools
lore

Bassem Dawood

author
#0 lore raw

PATCH 1/1 mac80211: Enable power save after receiving NULL packet ACK

Trigger dynamic_ps_timer to re-evaluate power saving once a null function packet (with PM = 1) is ACKed.

Signed-off-by Bassem Dawood <[email protected]>
---
 net/mac80211/status.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
net/mac80211/status.c +3 -3
index 3485610755ef..c106f81478df 100644
@@ -1013,9 +1013,9 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
 		if (info->flags & IEEE80211_TX_STAT_ACK) {
 			local->ps_sdata->u.mgd.flags |=
 					IEEE80211_STA_NULLFUNC_ACKED;
-		} else
-			mod_timer(&local->dynamic_ps_timer, jiffies +
-					msecs_to_jiffies(10));
+		}
+		mod_timer(&local->dynamic_ps_timer, jiffies +
+				msecs_to_jiffies(10));
 	}
 
 	ieee80211_report_used_skb(local, skb, false);
Signature · 1 line
2.17.1
maintainer
#1 lore raw
Quoted 2 lines · Bassem Dawood

Trigger dynamic_ps_timer to re-evaluate power saving once a null function packet (with PM = 1) is ACKed.

Can you please say what this fixes/why the change is needed?

johannes

author
#2 lore raw
Quoted 8 lines · Johannes Berg
Quoted 2 lines · Bassem Dawood

Trigger dynamic_ps_timer to re-evaluate power saving once a null function packet (with PM = 1) is ACKed.

Can you please say what this fixes/why the change is needed?

johannes

Sure.

For drivers that declare PS_NULLFUNC_STACK, when sleeping, mac80211 would send a nullfunc (with PS bit set). It won't notify the driver yet (waiting for the NULLFUNC ACK).

a/net/mac80211/mlme.c: void ieee80211_dynamic_ps_enable_work(struct work_struct *work)

	if (ieee80211_hw_check(&local->hw, PS_NULLFUNC_STACK) &&
	    !(ifmgd->flags & IEEE80211_STA_NULLFUNC_ACKED)) {
		if (drv_tx_frames_pending(local)) {
			mod_timer(&local->dynamic_ps_timer, jiffies +
				  msecs_to_jiffies(
				  local->hw.conf.dynamic_ps_timeout));
		} else {
			ieee80211_send_nullfunc(local, sdata, true);
			/* Flush to get the tx status of nullfunc frame */
			ieee80211_flush_queues(local, sdata, false);
		}
	}

	if (!(ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS) &&
	      ieee80211_hw_check(&local->hw, PS_NULLFUNC_STACK)) ||
	    (ifmgd->flags & IEEE80211_STA_NULLFUNC_ACKED)) {
		ifmgd->flags &= ~IEEE80211_STA_NULLFUNC_ACKED;
		local->hw.conf.flags |= IEEE80211_CONF_PS;
		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
	}

On reception of the ACK, mac80211 will set the IEEE80211_STA_NULLFUNC_ACKED flag, but won't re-evaluate dynamic_ps_enable_work. So drivers won't get notified to sleep (until ieee80211_dynamic_ps_enable_work is re-evaluated through any other path).

a/net/mac80211/status.c:static void __ieee80211_tx_status(struct ieee80211_hw *hw,

				  struct ieee80211_tx_status *status,
				  int rates_idx, int retry_count) 

	if (ieee80211_is_any_nullfunc(fc) &&
	    ieee80211_has_pm(fc) &&
	    ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS) &&
	    !(info->flags & IEEE80211_TX_CTL_INJECTED) &&
	    local->ps_sdata && !(local->scanning)) {
		if (info->flags & IEEE80211_TX_STAT_ACK) {
			local->ps_sdata->u.mgd.flags |=
					IEEE80211_STA_NULLFUNC_ACKED;
		} else
			mod_timer(&local->dynamic_ps_timer, jiffies +
					msecs_to_jiffies(10));
	}

This change makes sure ieee80211_dynamic_ps_enable_work is evaluated once a NULLFUNC is ACKED for a more conssitent notification of the driver.

Regards,
Bassem

author
#3 lore raw
Quoted 57 lines · Bassem Fahmy
Quoted 8 lines · Johannes Berg
Quoted 2 lines · Bassem Dawood

Trigger dynamic_ps_timer to re-evaluate power saving once a null function packet (with PM = 1) is ACKed.

Can you please say what this fixes/why the change is needed?

johannes

Sure.

For drivers that declare PS_NULLFUNC_STACK, when sleeping, mac80211 would send a nullfunc (with PS bit set). It won't notify the driver yet (waiting for the NULLFUNC ACK).

a/net/mac80211/mlme.c: void ieee80211_dynamic_ps_enable_work(struct work_struct *work)

	if (ieee80211_hw_check(&local->hw, PS_NULLFUNC_STACK) &&
	    !(ifmgd->flags & IEEE80211_STA_NULLFUNC_ACKED)) {
		if (drv_tx_frames_pending(local)) {
			mod_timer(&local->dynamic_ps_timer, jiffies +
				  msecs_to_jiffies(
				  local->hw.conf.dynamic_ps_timeout));
		} else {
			ieee80211_send_nullfunc(local, sdata, true);
			/* Flush to get the tx status of nullfunc frame */
			ieee80211_flush_queues(local, sdata, false);
		}
	}

	if (!(ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS) &&
	      ieee80211_hw_check(&local->hw, PS_NULLFUNC_STACK)) ||
	    (ifmgd->flags & IEEE80211_STA_NULLFUNC_ACKED)) {
		ifmgd->flags &= ~IEEE80211_STA_NULLFUNC_ACKED;
		local->hw.conf.flags |= IEEE80211_CONF_PS;
		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
	}

On reception of the ACK, mac80211 will set the IEEE80211_STA_NULLFUNC_ACKED flag, but won't re-evaluate dynamic_ps_enable_work. So drivers won't get notified to sleep (until ieee80211_dynamic_ps_enable_work is re-evaluated through any other path).

a/net/mac80211/status.c:static void __ieee80211_tx_status(struct ieee80211_hw *hw,

				  struct ieee80211_tx_status *status,
				  int rates_idx, int retry_count) 

	if (ieee80211_is_any_nullfunc(fc) &&
	    ieee80211_has_pm(fc) &&
	    ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS) &&
	    !(info->flags & IEEE80211_TX_CTL_INJECTED) &&
	    local->ps_sdata && !(local->scanning)) {
		if (info->flags & IEEE80211_TX_STAT_ACK) {
			local->ps_sdata->u.mgd.flags |=
					IEEE80211_STA_NULLFUNC_ACKED;
		} else
			mod_timer(&local->dynamic_ps_timer, jiffies +
					msecs_to_jiffies(10));
	}
Quoted 4 lines

This change makes sure ieee80211_dynamic_ps_enable_work is evaluated once a NULLFUNC is ACKED for a more conssitent notification of the driver.

Regards,
Bassem

Hi Johannes,

Following up on this. Is the explaination clear or you need more description?

Regareds,
Bassem