[PATCH] mac80211: End the MPSP even if EOSP frame was not received

[PATCH] mac80211: End the MPSP even if EOSP frame was not received

[Masashi Honma]

The mesh STA sends QoS frame with EOSP (end of service period)
subfiled=1 to end the MPSP(mesh peer service period). Previously, if
the frame was not acked by peer, the mesh STA did not end the MPSP.
This patch ends the MPSP even if the QoS frame was no acked.

Signed-off-by: Masashi Honma <masashi.honma@gmail.com>
---
 net/mac80211/status.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index c6d5c72..a2a6826 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -771,6 +771,13 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 			clear_sta_flag(sta, WLAN_STA_SP);
 
 		acked = !!(info->flags & IEEE80211_TX_STAT_ACK);
+
+		/* mesh Peer Service Period support */
+		if (ieee80211_vif_is_mesh(&sta->sdata->vif) &&
+		    ieee80211_is_data_qos(fc))
+			ieee80211_mpsp_trigger_process(
+				ieee80211_get_qos_ctl(hdr), sta, true, acked);
+
 		if (!acked && test_sta_flag(sta, WLAN_STA_PS_STA)) {
 			/*
 			 * The STA is in power save mode, so assume
@@ -781,13 +788,6 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 			return;
 		}
 
-		/* mesh Peer Service Period support */
-		if (ieee80211_vif_is_mesh(&sta->sdata->vif) &&
-		    ieee80211_is_data_qos(fc))
-			ieee80211_mpsp_trigger_process(
-					ieee80211_get_qos_ctl(hdr),
-					sta, true, acked);
-
 		if (ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL) &&
 		    (ieee80211_is_data(hdr->frame_control)) &&
 		    (rates_idx != -1))
-- 
2.7.4

Re: [PATCH] mac80211: End the MPSP even if EOSP frame was not received

[Bob Copeland]

On Wed, Jul 13, 2016 at 04:04:35PM +0900, Masashi Honma wrote:
> The mesh STA sends QoS frame with EOSP (end of service period)
> subfiled=1 to end the MPSP(mesh peer service period). Previously, if
> the frame was not acked by peer, the mesh STA did not end the MPSP.
> This patch ends the MPSP even if the QoS frame was no acked.

Hmm, my recollection on this stuff is a little fuzzy.  So we only get
here if the peer sta is in doze state, but it shouldn't do that if we
are in an MPSP.  So, is the real problem that we don't wait for an ack
from the peer before marking ourselves in a MPSP?

-- 
Bob Copeland %% http://bobcopeland.com/

Re: [PATCH] mac80211: End the MPSP even if EOSP frame was not received

[Masashi Honma]

On 2016年07月13日 22:50, Bob Copeland wrote:
>
> Hmm, my recollection on this stuff is a little fuzzy.  So we only get
> here if the peer sta is in doze state, but it shouldn't do that if we
> are in an MPSP.
Yes. In the MPSP, local peer only transmits frame when opposite peer is in
AWAKE. This patch expects we lost opposite peer accidentally
(AC adapter unplugged).

>    So, is the real problem that we don't wait for an ack
> from the peer before marking ourselves in a MPSP?
>
This patch does not fix starting MPSP, this patch fixes ending of MPSP.
Without this patch, local peer continues MPSP even if we lost opposite peer
accidentally.

Re: [PATCH] mac80211: End the MPSP even if EOSP frame was not received

[Bob Copeland]

On Tue, Jul 19, 2016 at 11:16:24AM +0900, Masashi Honma wrote:
> This patch does not fix starting MPSP, this patch fixes ending of MPSP.
> Without this patch, local peer continues MPSP even if we lost opposite peer
> accidentally.

OK, do we need to also clear WLAN_STA_PS_STA flag for the peer in
ieee80211_mps_sta_status_update(), or does the node completely repeer?

ISTR running into a problem where rebooted peer (previously in power-save)
would send popen but we would buffer the response due to stale PS sta
flag.

-- 
Bob Copeland %% http://bobcopeland.com/

Re: [PATCH] mac80211: End the MPSP even if EOSP frame was not received

[Masashi Honma]

On 2016年07月19日 19:40, Bob Copeland wrote:
>
> OK, do we need to also clear WLAN_STA_PS_STA flag for the peer in
> ieee80211_mps_sta_status_update(), or does the node completely repeer?
>
> ISTR running into a problem where rebooted peer (previously in power-save)
> would send popen but we would buffer the response due to stale PS sta
> flag.
>
Thanks. I have tried your scenario.

I have disconnected remote peer by accidental shutdown.
(This means local peer does not recognize remote peer left.)

Then remote peer restarted and started to send Mesh Peering Open frames.

Local peer receives these frames but does not respond to these
because peer_lid mismatch.

After 3 times retrial, remote peer sent Mesh Peering Close frame
with reason code = 56(MESH-MAX-RETRIES).

And then, struct sta_info is freed and allocated.

So looks no problem.

Re: [PATCH] mac80211: End the MPSP even if EOSP frame was not received

[Bob Copeland]

On Wed, Jul 20, 2016 at 04:11:33PM +0900, Masashi Honma wrote:
> On 2016年07月19日 19:40, Bob Copeland wrote:
> >
> >OK, do we need to also clear WLAN_STA_PS_STA flag for the peer in
> >ieee80211_mps_sta_status_update(), or does the node completely repeer?
> >
> >ISTR running into a problem where rebooted peer (previously in power-save)
> >would send popen but we would buffer the response due to stale PS sta
> >flag.
[...]
>
> So looks no problem.

Ok, thanks for testing that - fwiw original patch

Acked-by: Bob Copeland <me@bobcopeland.com>

-- 
Bob Copeland %% http://bobcopeland.com/

Re: [PATCH] mac80211: End the MPSP even if EOSP frame was not received

[Johannes Berg]

On Wed, 2016-07-13 at 16:04 +0900, Masashi Honma wrote:
> The mesh STA sends QoS frame with EOSP (end of service period)
> subfiled=1 to end the MPSP(mesh peer service period). Previously, if
> the frame was not acked by peer, the mesh STA did not end the MPSP.
> This patch ends the MPSP even if the QoS frame was no acked.
> 
Can you elaborate why? Does it fix anything? Please resend the patch
with that (and perhaps the "subfield" typo fixed above).

johannes

Re: [PATCH] mac80211: End the MPSP even if EOSP frame was not received

[Masashi Honma]

On 2016年08月02日 16:47, Johannes Berg wrote:
> Can you elaborate why? Does it fix anything? Please resend the patch
> with that (and perhaps the "subfield" typo fixed above).

I see.

Masashi Honma.

[PATCH v2] mac80211: End the MPSP even if EOSP frame was not acked

[Masashi Honma]

If QoS frame with EOSP (end of service period) subfield=1 sent by local
peer was not acked by remote peer, local peer did not end the MPSP. This
prevents local peer from going to DOZE state. And if the remote peer
goes away without closing connection, local peer continues AWAKE state
and wastes battery.

Signed-off-by: Masashi Honma <masashi.honma@gmail.com>
---
 net/mac80211/status.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index c6d5c72..a2a6826 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -771,6 +771,13 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 			clear_sta_flag(sta, WLAN_STA_SP);
 
 		acked = !!(info->flags & IEEE80211_TX_STAT_ACK);
+
+		/* mesh Peer Service Period support */
+		if (ieee80211_vif_is_mesh(&sta->sdata->vif) &&
+		    ieee80211_is_data_qos(fc))
+			ieee80211_mpsp_trigger_process(
+				ieee80211_get_qos_ctl(hdr), sta, true, acked);
+
 		if (!acked && test_sta_flag(sta, WLAN_STA_PS_STA)) {
 			/*
 			 * The STA is in power save mode, so assume
@@ -781,13 +788,6 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 			return;
 		}
 
-		/* mesh Peer Service Period support */
-		if (ieee80211_vif_is_mesh(&sta->sdata->vif) &&
-		    ieee80211_is_data_qos(fc))
-			ieee80211_mpsp_trigger_process(
-					ieee80211_get_qos_ctl(hdr),
-					sta, true, acked);
-
 		if (ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL) &&
 		    (ieee80211_is_data(hdr->frame_control)) &&
 		    (rates_idx != -1))
-- 
2.7.4

Re: [PATCH v2] mac80211: End the MPSP even if EOSP frame was not acked

[Johannes Berg]

On Tue, 2016-08-02 at 17:16 +0900, Masashi Honma wrote:
> If QoS frame with EOSP (end of service period) subfield=1 sent by
> local
> peer was not acked by remote peer, local peer did not end the MPSP.
> This
> prevents local peer from going to DOZE state. And if the remote peer
> goes away without closing connection, local peer continues AWAKE
> state
> and wastes battery.
> 
Applied, thanks.

johannes