some AP PS buffering rework

@@ -2340,17 +2340,35 @@ static inline int ieee80211_sta_ps_trans
 #define IEEE80211_TX_STATUS_HEADROOM	13
 
 /**
- * ieee80211_sta_set_tim - set the TIM bit for a sleeping station
+ * ieee80211_sta_set_buffered - inform mac80211 about driver-buffered frames
  * @sta: &struct ieee80211_sta pointer for the sleeping station
+ * @tid: the TID that has buffered frames
+ * @buffered: indicates whether or not frames are buffered for this TID
  *
  * If a driver buffers frames for a powersave station instead of passing
- * them back to mac80211 for retransmission, the station needs to be told
- * to wake up using the TIM bitmap in the beacon.
+ * them back to mac80211 for retransmission, the station may still need
+ * to be told that there are buffered frames via the TIM bit.
  *
- * This function sets the station's TIM bit - it will be cleared when the
- * station wakes up.
+ * This function informs mac80211 whether or not there are frames that are
+ * buffered in the driver for a given TID; mac80211 can then use this data
+ * to set the TIM bit (NOTE: This may call back into the driver's set_tim
+ * call! Beware of the locking!)
+ *
+ * If all frames are released to the station (due to PS-poll or uAPSD)
+ * then the driver needs to inform mac80211 that there no longer are
+ * frames buffered. However, when the station wakes up mac80211 assumes
+ * that all buffered frames will be transmitted and clears this data,
+ * drivers need to make sure they inform mac80211 about all buffered
+ * frames on the sleep transition (sta_notify() with %STA_NOTIFY_SLEEP).
+ *
+ * Note that technically mac80211 only needs to know this per AC, not per
+ * TID, but since driver buffering will inevitably happen per TID (since
+ * it is related to aggregation) it is easier to make mac80211 map the
+ * TID to the AC as required instead of keeping track in all drivers that
+ * use this API.
  */
-void ieee80211_sta_set_tim(struct ieee80211_sta *sta);
+void ieee80211_sta_set_buffered(struct ieee80211_sta *sta,
+				u8 tid, bool buffered);
 
 /**
  * ieee80211_tx_status - transmit status callback

@@ -1117,11 +1117,15 @@ void ieee80211_sta_block_awake(struct ie
 }
 EXPORT_SYMBOL(ieee80211_sta_block_awake);
 
-void ieee80211_sta_set_tim(struct ieee80211_sta *pubsta)
+void ieee80211_sta_set_buffered(struct ieee80211_sta *pubsta,
+				u8 tid, bool buffered)
 {
 	struct sta_info *sta = container_of(pubsta, struct sta_info, sta);
 
+	if (!buffered)
+		return;
+
 	set_sta_flags(sta, WLAN_STA_PS_DRIVER_BUF);
 	sta_info_set_tim_bit(sta);
 }
-EXPORT_SYMBOL(ieee80211_sta_set_tim);
+EXPORT_SYMBOL(ieee80211_sta_set_buffered);

@@ -343,7 +343,8 @@ void ath_tx_aggr_stop(struct ath_softc *
 void ath_tx_aggr_resume(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid);
 
 void ath_tx_aggr_wakeup(struct ath_softc *sc, struct ath_node *an);
-bool ath_tx_aggr_sleep(struct ath_softc *sc, struct ath_node *an);
+void ath_tx_aggr_sleep(struct ieee80211_sta *sta, struct ath_softc *sc,
+		       struct ath_node *an);
 
 /********/
 /* VIFs */

@@ -1851,8 +1851,7 @@ static void ath9k_sta_notify(struct ieee
 	switch (cmd) {
 	case STA_NOTIFY_SLEEP:
 		an->sleeping = true;
-		if (ath_tx_aggr_sleep(sc, an))
-			ieee80211_sta_set_tim(sta);
+		ath_tx_aggr_sleep(sta, sc, an);
 		break;
 	case STA_NOTIFY_AWAKE:
 		an->sleeping = false;

@@ -545,7 +545,7 @@ static void ath_tx_complete_aggr(struct
 	/* prepend un-acked frames to the beginning of the pending frame queue */
 	if (!list_empty(&bf_pending)) {
 		if (an->sleeping)
-			ieee80211_sta_set_tim(sta);
+			ieee80211_sta_set_buffered(sta, tid->tidno, true);
 
 		spin_lock_bh(&txq->axq_lock);
 		if (clear_filter)

@@ -933,12 +933,13 @@ void ath_tx_aggr_stop(struct ath_softc *
 	ath_tx_flush_tid(sc, txtid);
 }
 
-bool ath_tx_aggr_sleep(struct ath_softc *sc, struct ath_node *an)
+void ath_tx_aggr_sleep(struct ieee80211_sta *sta, struct ath_softc *sc,
+		       struct ath_node *an)
 {
 	struct ath_atx_tid *tid;
 	struct ath_atx_ac *ac;
 	struct ath_txq *txq;
-	bool buffered = false;
+	bool buffered;
 	int tidno;
 
 	for (tidno = 0, tid = &an->tid[tidno];

@@ -952,8 +953,7 @@ bool ath_tx_aggr_sleep(struct ath_softc
 
 		spin_lock_bh(&txq->axq_lock);
 
-		if (!list_empty(&tid->buf_q))
-			buffered = true;
+		buffered = !list_empty(&tid->buf_q);
 
 		tid->sched = false;
 		list_del(&tid->list);

@@ -964,9 +964,9 @@ bool ath_tx_aggr_sleep(struct ath_softc
 		}
 
 		spin_unlock_bh(&txq->axq_lock);
-	}
 
-	return buffered;
+		ieee80211_sta_set_buffered(sta, tidno, buffered);
+	}
 }
 
 void ath_tx_aggr_wakeup(struct ath_softc *sc, struct ath_node *an)

@@ -518,8 +518,7 @@ int sta_info_destroy_addr(struct ieee802
 int sta_info_destroy_addr_bss(struct ieee80211_sub_if_data *sdata,
 			      const u8 *addr);
 
-void sta_info_set_tim_bit(struct sta_info *sta);
-void sta_info_clear_tim_bit(struct sta_info *sta);
+void sta_info_recalc_tim(struct sta_info *sta);
 
 void sta_info_init(struct ieee80211_local *local);
 void sta_info_stop(struct ieee80211_local *local);

@@ -470,13 +470,10 @@ ieee80211_tx_h_unicast_ps_buf(struct iee
 			tx->local->total_ps_buffered++;
 
 		/*
-		 * Queue frame to be sent after STA wakes up/polls,
-		 * but don't set the TIM bit if the driver is blocking
-		 * wakeup or poll response transmissions anyway.
+		 * We queued up some frames, so the TIM bit might
+		 * need to be set, recalculate it.
 		 */
-		if (skb_queue_empty(&sta->ps_tx_buf) &&
-		    !(staflags & WLAN_STA_PS_DRIVER))
-			sta_info_set_tim_bit(sta);
+		sta_info_recalc_tim(sta);
 
 		info->control.jiffies = jiffies;
 		info->control.vif = &tx->sdata->vif;

159 lines not on this page · raw message

@@ -106,6 +106,7 @@ static void ieee80211_handle_filtered_fr
 	if (test_sta_flags(sta, WLAN_STA_PS_STA) &&
 	    skb_queue_len(&sta->tx_filtered) < STA_MAX_TX_BUFFER) {
 		skb_queue_tail(&sta->tx_filtered, skb);
+		sta_info_recalc_tim(sta);
 		return;
 	}
 

@@ -698,6 +698,39 @@ static bool sta_info_cleanup_expire_buff
 	unsigned long flags;
 	struct sk_buff *skb;
 
+	/*
+	 * First check for frames that should expire on the filtered
+	 * queue. Frames here were rejected by the driver and are on
+	 * a separate queue to avoid reordering with normal PS-buffered
+	 * frames. They also aren't accounted for right now in the
+	 * total_ps_buffered counter.
+	 */
+	for (;;) {
+		spin_lock_irqsave(&sta->tx_filtered.lock, flags);
+		skb = skb_peek(&sta->tx_filtered);
+		if (sta_info_buffer_expired(sta, skb))
+			skb = __skb_dequeue(&sta->tx_filtered);
+		else
+			skb = NULL;
+		spin_unlock_irqrestore(&sta->tx_filtered.lock, flags);
+
+		/*
+		 * Frames are queued in order, so if this one
+		 * hasn't expired yet we can stop testing. If
+		 * we actually reached the end of the queue we
+		 * also need to stop, of course.
+		 */
+		if (!skb)
+			break;
+		dev_kfree_skb(skb);
+	}
+
+	/*
+	 * Now also check the normal PS-buffered queue, this will
+	 * only find something if the filtered queue was emptied
+	 * since the filtered frames are all before the normal PS
+	 * buffered frames.
+	 */
 	for (;;) {
 		spin_lock_irqsave(&sta->ps_tx_buf.lock, flags);
 		skb = skb_peek(&sta->ps_tx_buf);

@@ -707,6 +740,11 @@ static bool sta_info_cleanup_expire_buff
 			skb = NULL;
 		spin_unlock_irqrestore(&sta->ps_tx_buf.lock, flags);
 
+		/*
+		 * frames are queued in order, so if this one
+		 * hasn't expired yet (or we reached the end of
+		 * the queue) we can stop testing
+		 */
 		if (!skb)
 			break;
 

@@ -716,13 +754,22 @@ static bool sta_info_cleanup_expire_buff
 		       sta->sta.addr);
 #endif
 		dev_kfree_skb(skb);
-
-		/* if the queue is now empty recalc TIM bit */
-		if (skb_queue_empty(&sta->ps_tx_buf))
-			sta_info_recalc_tim(sta);
 	}
 
-	return !skb_queue_empty(&sta->ps_tx_buf);
+	/*
+	 * Finally, recalculate the TIM bit for this station -- it might
+	 * now be clear because the station was too slow to retrieve its
+	 * frames.
+	 */
+	sta_info_recalc_tim(sta);
+
+	/*
+	 * Return whether there are any frames still buffered, this is
+	 * used to check whether the cleanup timer still needs to run,
+	 * if there are no frames we don't need to rearm the timer.
+	 */
+	return !(skb_queue_empty(&sta->ps_tx_buf) &&
+		 skb_queue_empty(&sta->tx_filtered));
 }
 
 static int __must_check __sta_info_destroy(struct sta_info *sta)

@@ -107,6 +107,11 @@ static void ieee80211_handle_filtered_fr
 	    skb_queue_len(&sta->tx_filtered) < STA_MAX_TX_BUFFER) {
 		skb_queue_tail(&sta->tx_filtered, skb);
 		sta_info_recalc_tim(sta);
+
+		if (!timer_pending(&local->sta_cleanup))
+			mod_timer(&local->sta_cleanup,
+				  round_jiffies(jiffies +
+						STA_INFO_CLEANUP_INTERVAL));
 		return;
 	}