2021-02-27 06:04:32

by Bassem Dawood

[permalink] [raw]
Subject: [PATCH] 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(-)

diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 3485610755ef..c106f81478df 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -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);
--
2.17.1


2021-04-08 13:27:08

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Enable power save after receiving NULL packet ACK

On Sat, 2021-02-27 at 16:58 +1100, Bassem Dawood wrote:
> 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

2021-04-09 01:32:57

by Bassem Dawood

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Enable power save after receiving NULL packet ACK

On Thu, Apr 08, 2021 at 03:26:06PM +0200, Johannes Berg wrote:
> On Sat, 2021-02-27 at 16:58 +1100, Bassem Dawood wrote:
> > 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