[PATCH 1/2] mac80211: don't send deferred frames outside the SP

[PATCH 1/2] mac80211: don't send deferred frames outside the SP

[Emmanuel Grumbach]

Frames that are sent between
ampdu_action(IEEE80211_AMPDU_TX_START) and the move to the
HT_AGG_STATE_OPERATIONAL state are buffered.
If we try to start an A-MPDU session while the peer is
sleeping and polling frames with U-APSD, we may have frames
that will be buffered by ieee80211_tx_prep_agg. These frames
have IEEE80211_TX_CTL_NO_PS_BUFFER set since they are sent to
a sleeping client and possibly IEEE80211_TX_STATUS_EOSP.
If the frame is buffered, we need clear these two flags
since they will be re-sent after the move to
HT_AGG_STATE_OPERATIONAL state which is very likely to
happen after the SP ends.

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 net/mac80211/tx.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 485e30a..114509a 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1118,9 +1118,7 @@ static bool ieee80211_tx_prep_agg(struct ieee80211_tx_data *tx,
 			queued = true;
 			info->control.vif = &tx->sdata->vif;
 			info->flags |= IEEE80211_TX_INTFL_NEED_TXPROCESSING;
-			info->flags &= ~IEEE80211_TX_TEMPORARY_FLAGS |
-					IEEE80211_TX_CTL_NO_PS_BUFFER |
-					IEEE80211_TX_STATUS_EOSP;
+			info->flags &= ~IEEE80211_TX_TEMPORARY_FLAGS;
 			__skb_queue_tail(&tid_tx->pending, skb);
 			if (skb_queue_len(&tid_tx->pending) > STA_MAX_TX_BUFFER)
 				purge_skb = __skb_dequeue(&tid_tx->pending);
-- 
2.5.0

[PATCH 2/2] mac80211: close the SP when we enqueue frames during the SP

[Emmanuel Grumbach]

Since we enqueued the frame that was supposed to be sent
during the SP, and that frame may very well cary the
IEEE80211_TX_STATUS_EOSP bit, we may never close the SP
(WLAN_STA_SP will never be cleared). If that happens, we
will not open any new SP and will never respond to any poll
frame from the client.
Clear WLAN_STA_SP manually if a frame that was polled during
the SP is queued because of a starting A-MPDU session. The
client may not see the EOSP bit, but it will at least be
able to poll new frames in another SP.

Reported-by: Alesya Shapira <alesya.shapira@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 net/mac80211/tx.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 114509a..5473281 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1116,6 +1116,13 @@ static bool ieee80211_tx_prep_agg(struct ieee80211_tx_data *tx,
 			reset_agg_timer = true;
 		} else {
 			queued = true;
+			/* XXX: should I test IEEE80211_TX_STATUS_EOSP here? */
+			if (info->flags & IEEE80211_TX_CTL_NO_PS_BUFFER) {
+				clear_sta_flag(tx->sta, WLAN_STA_SP);
+				ps_dbg(tx->sta->sdata,
+				       "STA %pM aid %d: SP frame queued, close the SP w/o telling the peer\n",
+				       tx->sta->sta.addr, tx->sta->sta.aid);
+			}
 			info->control.vif = &tx->sdata->vif;
 			info->flags |= IEEE80211_TX_INTFL_NEED_TXPROCESSING;
 			info->flags &= ~IEEE80211_TX_TEMPORARY_FLAGS;
-- 
2.5.0

Re: [PATCH 1/2] mac80211: don't send deferred frames outside the SP

[Johannes Berg]

On Thu, 2016-03-17 at 16:51 +0200, Emmanuel Grumbach wrote:
> Frames that are sent between
> ampdu_action(IEEE80211_AMPDU_TX_START) and the move to the
> HT_AGG_STATE_OPERATIONAL state are buffered.
> If we try to start an A-MPDU session while the peer is
> sleeping and polling frames with U-APSD, we may have frames
> that will be buffered by ieee80211_tx_prep_agg. These frames
> have IEEE80211_TX_CTL_NO_PS_BUFFER set since they are sent to
> a sleeping client and possibly IEEE80211_TX_STATUS_EOSP.
> If the frame is buffered, we need clear these two flags
> since they will be re-sent after the move to
> HT_AGG_STATE_OPERATIONAL state which is very likely to
> happen after the SP ends.
> 
Applied.

johannes

Re: [PATCH 2/2] mac80211: close the SP when we enqueue frames during the SP

[Johannes Berg]

On Thu, 2016-03-17 at 16:51 +0200, Emmanuel Grumbach wrote:
> Since we enqueued the frame that was supposed to be sent
> during the SP, and that frame may very well cary the
> IEEE80211_TX_STATUS_EOSP bit, we may never close the SP
> (WLAN_STA_SP will never be cleared). If that happens, we
> will not open any new SP and will never respond to any poll
> frame from the client.
> Clear WLAN_STA_SP manually if a frame that was polled during
> the SP is queued because of a starting A-MPDU session. The
> client may not see the EOSP bit, but it will at least be
> able to poll new frames in another SP.
> 
Also applied.

johannes

[PATCH v2] mac80211: close the SP when we enqueue frames during the SP

[Emmanuel Grumbach]

Since we enqueued the frame that was supposed to be sent
during the SP, and that frame may very well cary the
IEEE80211_TX_STATUS_EOSP bit, we may never close the SP
(WLAN_STA_SP will never be cleared). If that happens, we
will not open any new SP and will never respond to any poll
frame from the client.
Clear WLAN_STA_SP manually if a frame that was polled during
the SP is queued because of a starting A-MPDU session. The
client may not see the EOSP bit, but it will at least be
able to poll new frames in another SP.

Reported-by: Alesya Shapira <alesya.shapira@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
v2: remove TODO comment
---
 net/mac80211/tx.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 597c8fe..9fbc1b6 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1116,6 +1116,12 @@ static bool ieee80211_tx_prep_agg(struct ieee80211_tx_data *tx,
 			reset_agg_timer = true;
 		} else {
 			queued = true;
+			if (info->flags & IEEE80211_TX_CTL_NO_PS_BUFFER) {
+				clear_sta_flag(tx->sta, WLAN_STA_SP);
+				ps_dbg(tx->sta->sdata,
+				       "STA %pM aid %d: SP frame queued, close the SP w/o telling the peer\n",
+				       tx->sta->sta.addr, tx->sta->sta.aid);
+			}
 			info->control.vif = &tx->sdata->vif;
 			info->flags |= IEEE80211_TX_INTFL_NEED_TXPROCESSING;
 			info->flags &= ~IEEE80211_TX_TEMPORARY_FLAGS |
-- 
2.5.0