| Message ID | 1435823996-32510-1-git-send-email-michal.kazior@tieto.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
On Thu, 2015-07-02 at 09:59 +0200, Michal Kazior wrote: > When acting as AP and a PS-Poll frame is received > associated station is marked as one in a Service > Period. This state is kept until Tx status for > released frame is reported. While a station is in > Service Period PS-Poll frames are ignored. > > However if PS-Poll was received during A-MPDU > teardown it was possible to have the to-be > released frame re-queued back to pending queue. > In such case the frame was stripped of 2 important > flags: > > (a) IEEE80211_TX_CTL_NO_PS_BUFFER > (b) IEEE80211_TX_STATUS_EOSP > > Stripping of (a) led to the frame that was to be > released to be queued back to ps_tx_buf queue. If > station remained to use only PS-Poll frames the > re-queued frame (and new ones) was never actually > transmitted because mac80211 would ignore > subsequent PS-Poll frames due to station being in > Service Period. There was nothing left to clear > the Service Period bit (no xmit -> no tx status -> > no SP end), i.e. the AP would have the station > stuck in Service Period. Beacon TIM would > repeatedly prompt station to poll for frames but > it would get none. > > Once (a) is not stripped (b) becomes important > because it's the main condition to clear the > Service Period bit of the station when Tx status > for the released frame is reported back. > > This problem was observed with ath9k acting as P2P > GO in some testing scenarios but isn't limited to > it. AP operation with mac80211 based Tx A-MPDU > control combined with clients using PS-Poll frames > is subject to this race. I'm not sure I quite understand - how is the aggregation teardown causing frame filtering? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2015-07-17 at 11:05 +0200, Johannes Berg wrote: > On Thu, 2015-07-02 at 09:59 +0200, Michal Kazior wrote: > > When acting as AP and a PS-Poll frame is received > > associated station is marked as one in a Service > > Period. This state is kept until Tx status for > > released frame is reported. While a station is in > > Service Period PS-Poll frames are ignored. > > > > However if PS-Poll was received during A-MPDU > > teardown it was possible to have the to-be > > released frame re-queued back to pending queue. > > In such case the frame was stripped of 2 important > > flags: > > > > (a) IEEE80211_TX_CTL_NO_PS_BUFFER > > (b) IEEE80211_TX_STATUS_EOSP > > > > Stripping of (a) led to the frame that was to be > > released to be queued back to ps_tx_buf queue. If > > station remained to use only PS-Poll frames the > > re-queued frame (and new ones) was never actually > > transmitted because mac80211 would ignore > > subsequent PS-Poll frames due to station being in > > Service Period. There was nothing left to clear > > the Service Period bit (no xmit -> no tx status -> > > no SP end), i.e. the AP would have the station > > stuck in Service Period. Beacon TIM would > > repeatedly prompt station to poll for frames but > > it would get none. > > > > Once (a) is not stripped (b) becomes important > > because it's the main condition to clear the > > Service Period bit of the station when Tx status > > for the released frame is reported back. > > > > This problem was observed with ath9k acting as P2P > > GO in some testing scenarios but isn't limited to > > it. AP operation with mac80211 based Tx A-MPDU > > control combined with clients using PS-Poll frames > > is subject to this race. > > I'm not sure I quite understand - how is the aggregation teardown > causing frame filtering? > Never mind, I was looking at the wrong code. I'll apply this. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2015-07-17 at 11:07 +0200, Johannes Berg wrote: > > > > Once (a) is not stripped (b) becomes important > > > because it's the main condition to clear the > > > Service Period bit of the station when Tx status > > > for the released frame is reported back. > > > > > > This problem was observed with ath9k acting as P2P > > > GO in some testing scenarios but isn't limited to > > > it. AP operation with mac80211 based Tx A-MPDU > > > control combined with clients using PS-Poll frames > > > is subject to this race. > > > > I'm not sure I quite understand - how is the aggregation teardown > > causing frame filtering? > > > > Never mind, I was looking at the wrong code. I'll apply this. > However, I'd like to ask you to look at this again - I can see how it fixes the problem now, but it seems like a fairly unreliable fix since the frame is sent through TX processing again, and you're relying on that preserving a flag that's otherwise marked temporary... So I think it may be better to adjust the station flags in this case to let a new (the same) frame be the ps-poll response again. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17 July 2015 at 11:09, Johannes Berg <johannes@sipsolutions.net> wrote: > On Fri, 2015-07-17 at 11:07 +0200, Johannes Berg wrote: >> >> > > Once (a) is not stripped (b) becomes important >> > > because it's the main condition to clear the >> > > Service Period bit of the station when Tx status >> > > for the released frame is reported back. >> > > >> > > This problem was observed with ath9k acting as P2P >> > > GO in some testing scenarios but isn't limited to >> > > it. AP operation with mac80211 based Tx A-MPDU >> > > control combined with clients using PS-Poll frames >> > > is subject to this race. >> > >> > I'm not sure I quite understand - how is the aggregation teardown >> > causing frame filtering? >> > >> >> Never mind, I was looking at the wrong code. I'll apply this. >> > > However, I'd like to ask you to look at this again - I can see how it > fixes the problem now, but it seems like a fairly unreliable fix since > the frame is sent through TX processing again, and you're relying on > that preserving a flag that's otherwise marked temporary... > > So I think it may be better to adjust the station flags in this case to > let a new (the same) frame be the ps-poll response again. Hmm.. I did contemplate on clearing the station flag. I didn't explore the idea thoroughly but my feeling was it could introduce new u-APSD corner-case bug(s) since you'd be changing the station state before it'd normally be done and hence would behave differently when handling Rx. I'll look into it more. Micha? -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index d14f3618069f..2079d480cd7b 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -1113,7 +1113,9 @@ 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; + info->flags &= ~IEEE80211_TX_TEMPORARY_FLAGS | + IEEE80211_TX_CTL_NO_PS_BUFFER | + IEEE80211_TX_STATUS_EOSP; __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);
When acting as AP and a PS-Poll frame is received associated station is marked as one in a Service Period. This state is kept until Tx status for released frame is reported. While a station is in Service Period PS-Poll frames are ignored. However if PS-Poll was received during A-MPDU teardown it was possible to have the to-be released frame re-queued back to pending queue. In such case the frame was stripped of 2 important flags: (a) IEEE80211_TX_CTL_NO_PS_BUFFER (b) IEEE80211_TX_STATUS_EOSP Stripping of (a) led to the frame that was to be released to be queued back to ps_tx_buf queue. If station remained to use only PS-Poll frames the re-queued frame (and new ones) was never actually transmitted because mac80211 would ignore subsequent PS-Poll frames due to station being in Service Period. There was nothing left to clear the Service Period bit (no xmit -> no tx status -> no SP end), i.e. the AP would have the station stuck in Service Period. Beacon TIM would repeatedly prompt station to poll for frames but it would get none. Once (a) is not stripped (b) becomes important because it's the main condition to clear the Service Period bit of the station when Tx status for the released frame is reported back. This problem was observed with ath9k acting as P2P GO in some testing scenarios but isn't limited to it. AP operation with mac80211 based Tx A-MPDU control combined with clients using PS-Poll frames is subject to this race. Signed-off-by: Michal Kazior <michal.kazior@tieto.com> --- net/mac80211/tx.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)