aboutsummaryrefslogtreecommitdiffstats
path: root/net/mac80211
diff options
context:
space:
mode:
authorJohannes Berg <johannes.berg@intel.com>2014-01-08 11:45:07 -0500
committerJohannes Berg <johannes.berg@intel.com>2014-01-10 03:43:34 -0500
commitf9f760b4883d7fbfb463a67267e2be6b440d1aeb (patch)
treed026bb050177719db0c35c77f4bf1c72f2ccd3e9 /net/mac80211
parent0a1cb80975b67e29d572b28c1621203d1d74f4d3 (diff)
mac80211: release multiple ACs in uAPSD, fix more-data bug
When a response for PS-Poll or a uAPSD trigger frame is sent, the more-data bit should be set according to 802.11-2012 11.2.1.5 h), meaning that it should indicate more data on the relevant ACs (delivery-enabled or nondelivery-enabled for uAPSD or PS-Poll.) In, for example, the following scenario: * 1 frame on VO queue (either in driver or in mac80211) * at least 1 frame on VI queue (in the driver) * both VO/VI are delivery-enabled * uAPSD trigger frame received The more-data flag to the driver would not be set, even though it should be. While fixing this, I noticed that we should really release frames from multiple ACs where there's data buffered in the driver for the corresponding TIDs. To address all this, restructure the code a bit to consider all ACs if we only release driver frames or only buffered frames. This also addresses the more-data bug described above as now the TIDs will all be marked as released, so the driver will have to check the number of frames. While at it, clarify some code and comments and remove the found variable, replacing it with the appropriate sw/hw release check. Reported-by: Eliad Peller <eliad@wizery.com> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Diffstat (limited to 'net/mac80211')
-rw-r--r--net/mac80211/sta_info.c82
1 files changed, 40 insertions, 42 deletions
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 93bfd6700cbf..93e2157a5b7b 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1245,7 +1245,6 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
1245{ 1245{
1246 struct ieee80211_sub_if_data *sdata = sta->sdata; 1246 struct ieee80211_sub_if_data *sdata = sta->sdata;
1247 struct ieee80211_local *local = sdata->local; 1247 struct ieee80211_local *local = sdata->local;
1248 bool found = false;
1249 bool more_data = false; 1248 bool more_data = false;
1250 int ac; 1249 int ac;
1251 unsigned long driver_release_tids = 0; 1250 unsigned long driver_release_tids = 0;
@@ -1256,9 +1255,7 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
1256 1255
1257 __skb_queue_head_init(&frames); 1256 __skb_queue_head_init(&frames);
1258 1257
1259 /* 1258 /* Get response frame(s) and more data bit for the last one. */
1260 * Get response frame(s) and more data bit for it.
1261 */
1262 for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) { 1259 for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
1263 unsigned long tids; 1260 unsigned long tids;
1264 1261
@@ -1267,33 +1264,17 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
1267 1264
1268 tids = ieee80211_tids_for_ac(ac); 1265 tids = ieee80211_tids_for_ac(ac);
1269 1266
1270 if (!found) { 1267 /* if we already have frames from software, then we can't also
1271 driver_release_tids = sta->driver_buffered_tids & tids; 1268 * release from hardware queues
1272 if (driver_release_tids) { 1269 */
1273 found = true; 1270 if (skb_queue_empty(&frames))
1274 } else { 1271 driver_release_tids |= sta->driver_buffered_tids & tids;
1275 struct sk_buff *skb;
1276
1277 while (n_frames > 0) {
1278 skb = skb_dequeue(&sta->tx_filtered[ac]);
1279 if (!skb) {
1280 skb = skb_dequeue(
1281 &sta->ps_tx_buf[ac]);
1282 if (skb)
1283 local->total_ps_buffered--;
1284 }
1285 if (!skb)
1286 break;
1287 n_frames--;
1288 found = true;
1289 __skb_queue_tail(&frames, skb);
1290 }
1291 }
1292 1272
1293 /* 1273 if (driver_release_tids) {
1294 * If the driver has data on more than one TID then 1274 /* If the driver has data on more than one TID then
1295 * certainly there's more data if we release just a 1275 * certainly there's more data if we release just a
1296 * single frame now (from a single TID). 1276 * single frame now (from a single TID). This will
1277 * only happen for PS-Poll.
1297 */ 1278 */
1298 if (reason == IEEE80211_FRAME_RELEASE_PSPOLL && 1279 if (reason == IEEE80211_FRAME_RELEASE_PSPOLL &&
1299 hweight16(driver_release_tids) > 1) { 1280 hweight16(driver_release_tids) > 1) {
@@ -1303,8 +1284,28 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
1303 driver_release_tids)); 1284 driver_release_tids));
1304 break; 1285 break;
1305 } 1286 }
1287 } else {
1288 struct sk_buff *skb;
1289
1290 while (n_frames > 0) {
1291 skb = skb_dequeue(&sta->tx_filtered[ac]);
1292 if (!skb) {
1293 skb = skb_dequeue(
1294 &sta->ps_tx_buf[ac]);
1295 if (skb)
1296 local->total_ps_buffered--;
1297 }
1298 if (!skb)
1299 break;
1300 n_frames--;
1301 __skb_queue_tail(&frames, skb);
1302 }
1306 } 1303 }
1307 1304
1305 /* If we have more frames buffered on this AC, then set the
1306 * more-data bit and abort the loop since we can't send more
1307 * data from other ACs before the buffered frames from this.
1308 */
1308 if (!skb_queue_empty(&sta->tx_filtered[ac]) || 1309 if (!skb_queue_empty(&sta->tx_filtered[ac]) ||
1309 !skb_queue_empty(&sta->ps_tx_buf[ac])) { 1310 !skb_queue_empty(&sta->ps_tx_buf[ac])) {
1310 more_data = true; 1311 more_data = true;
@@ -1312,7 +1313,7 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
1312 } 1313 }
1313 } 1314 }
1314 1315
1315 if (!found) { 1316 if (skb_queue_empty(&frames) && !driver_release_tids) {
1316 int tid; 1317 int tid;
1317 1318
1318 /* 1319 /*
@@ -1334,10 +1335,7 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
1334 tid = 7 - ((ffs(~ignored_acs) - 1) << 1); 1335 tid = 7 - ((ffs(~ignored_acs) - 1) << 1);
1335 1336
1336 ieee80211_send_null_response(sdata, sta, tid, reason); 1337 ieee80211_send_null_response(sdata, sta, tid, reason);
1337 return; 1338 } else if (!driver_release_tids) {
1338 }
1339
1340 if (!driver_release_tids) {
1341 struct sk_buff_head pending; 1339 struct sk_buff_head pending;
1342 struct sk_buff *skb; 1340 struct sk_buff *skb;
1343 int num = 0; 1341 int num = 0;
@@ -1403,12 +1401,12 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
1403 /* 1401 /*
1404 * We need to release a frame that is buffered somewhere in the 1402 * We need to release a frame that is buffered somewhere in the
1405 * driver ... it'll have to handle that. 1403 * driver ... it'll have to handle that.
1406 * Note that, as per the comment above, it'll also have to see 1404 * Note that the driver also has to check the number of frames
1407 * if there is more than just one frame on the specific TID that 1405 * on the TIDs we're releasing from - if there are more than
1408 * we're releasing from, and it needs to set the more-data bit 1406 * n_frames it has to set the more-data bit (if we didn't ask
1409 * accordingly if we tell it that there's no more data. If we do 1407 * it to set it anyway due to other buffered frames); if there
1410 * tell it there's more data, then of course the more-data bit 1408 * are fewer than n_frames it has to make sure to adjust that
1411 * needs to be set anyway. 1409 * to allow the service period to end properly.
1412 */ 1410 */
1413 drv_release_buffered_frames(local, sta, driver_release_tids, 1411 drv_release_buffered_frames(local, sta, driver_release_tids,
1414 n_frames, reason, more_data); 1412 n_frames, reason, more_data);
@@ -1416,9 +1414,9 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
1416 /* 1414 /*
1417 * Note that we don't recalculate the TIM bit here as it would 1415 * Note that we don't recalculate the TIM bit here as it would
1418 * most likely have no effect at all unless the driver told us 1416 * most likely have no effect at all unless the driver told us
1419 * that the TID became empty before returning here from the 1417 * that the TID(s) became empty before returning here from the
1420 * release function. 1418 * release function.
1421 * Either way, however, when the driver tells us that the TID 1419 * Either way, however, when the driver tells us that the TID(s)
1422 * became empty we'll do the TIM recalculation. 1420 * became empty we'll do the TIM recalculation.
1423 */ 1421 */
1424 } 1422 }