Re: [RFC v2] mac80211: follow 802.11-2007 11.5.3 Error recovery upon HT BA failure

[Christian Lamparter <chunkeey@googlemail.com>]

On Saturday 09 June 2012 09:59:27 Johannes Berg wrote:
> > introduced a TX BA session timeout timer. However the
> > timer was reset for each outgoing transmission instead
> > what 802.11-2007 11.5.3 specifies: 
> 
> You should probably reference 802.11-2012 now since some
> things were renumbered.
yeah, I should. But 802.11-2012 is not yet available for
free like the old 802.11-2007 through IEEE 802 Get. 
 
> > "The inactivity timer at the originator is reset when a
> > BlockAck frame corresponding to the TID for which the
> > Block Ack policy is set is received."
> 
> > This patch was only compile-tested and the design has 
> > some rather big problems: 
> 
> Maybe then we should just revert Nikolay's patch?
(I was referring to this RFC patch and not his patch)
I don't think we can/should revert it, just change it
a bit so the timeout reset is triggered by a received
BA from the peer.

> >  - The spec says we should test for BlockAcks, however
> >    that is not feasible because it is a control frame
> >    (so FIF_CONTROL might filter it on some hardware).
> 
> In some way, all devices are going to have to report these
> frames. Maybe not as the frame itself, but as some other
> notification, to allow cleaning up the TX queues
> accordingly, I think? There's a BA notification in iwlwifi
> for example.
Alright, I guess this means that we probably need a new HW
feature flag like:
	IEEE80211_HW_REPORTS_BA_NOTIFICATION
for hardware/firmware which filter BA frames, but have a BA
notification trap. And every other driver need to pass BA
to the stack where a new rx handler will update the last_tx
accordingly. 

(yeah, that IEEE80211_HW_HAS_RATE_CONTROL vs. ACK_REPORT mix-up
was a stupid error - anyway here's the last updated version
of this approach - v3 will use IEEE80211_HW_REPORTS_BA_NOTIFICATION)
---
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 3bb24a1..fbe84a7 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -103,7 +103,7 @@ enum ieee80211_sta_info_flags {
  * @dialog_token: dialog token for aggregation session
  * @timeout: session timeout value to be filled in ADDBA requests
  * @state: session state (see above)
- * @last_tx: jiffies of last tx activity
+ * @last_tx: jiffies of last successful tx activity
  * @stop_initiator: initiator of a session stop
  * @tx_stop: TX DelBA frame when stopping
  * @buf_size: reorder buffer size at receiver
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 6b4f425..dfc61de 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -439,6 +439,28 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 			}
 		}
 
+		if (ieee80211_is_data_qos(fc) && acked &&
+		    info->flags & IEEE80211_TX_CTL_AMPDU &&
+		    local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS) {
+			struct tid_ampdu_tx *tid_tx;
+			u8 *qc;
+			u16 tid;
+
+			qc = ieee80211_get_qos_ctl(hdr);
+			tid = qc[0] & 0xf;
+			tid_tx = rcu_dereference(sta->ampdu_mlme.tid_tx[tid]);
+			if (tid_tx) {
+				/*
+				 * 802.11-2007 11.5.3 Error recovery upon a peer failure
+				 *
+				 * The inactivity timer at the originator is reset when
+				 * a BlockAck frame corresponding to the TID for which
+				 * the Block Ack policy is set is received.
+				 */
+				tid_tx->last_tx = jiffies;
+			}
+		}
+
 		if (info->flags & IEEE80211_TX_STAT_TX_FILTERED) {
 			ieee80211_handle_filtered_frame(local, sta, skb);
 			rcu_read_unlock();
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index af25c4e..67887c5 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1098,7 +1098,9 @@ static bool ieee80211_tx_prep_agg(struct ieee80211_tx_data *tx,
 			/* do nothing, let packet pass through */
 		} else if (test_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state)) {
 			info->flags |= IEEE80211_TX_CTL_AMPDU;
-			reset_agg_timer = true;
+			if (!(tx->local->hw.flags &
+			      IEEE80211_HW_REPORTS_TX_ACK_STATUS))
+				reset_agg_timer = true;
 		} else {
 			queued = true;
 			info->control.vif = &tx->sdata->vif;