mac80211: Add HT rates into RX status reporting

Radiotap code gets some of the additional knowledge so that it can determine the used bitrate for HT rates, too (though, the rate field is not large enough to fit all rates). It would be useful to add MCS index, HT20/HT40, and short GI information into radiotap definition at some point, too, and the needed information for filling in those field is now available in mac80211.

The ath9k change fixes and cleans up the RX status reporting by getting rid of code that used internal rate tables and ratekbps calculation. The correct value is now reported with MCS index instead of the old mechanism that defaulted to using the highest legacy rate.

net/mac80211/mlme.c +0 -1

@@ -1629,8 +1629,13 @@ static void ieee80211_rx_bss_info(struct
 			 * e.g: at 1 MBit that means mactime is 192 usec earlier
 			 * (=24 bytes * 8 usecs/byte) than the beacon timestamp.
 			 */
-			int rate = local->hw.wiphy->bands[band]->

net/mac80211/rx.c +28 -1

@@ -149,7 +149,41 @@ ieee80211_add_rx_radiotap_header(struct 
 	pos++;
 
 	/* IEEE80211_RADIOTAP_RATE */
-	*pos = rate->bitrate / 5;
+	if (status->flag & (RX_FLAG_HT20 | RX_FLAG_HT40)) {
+		int mcs_idx = status->rate_idx;
+		if (mcs_idx >= 16) {
+			/* TODO: MCS index 16.. */
+			*pos = 0;
+		} else {
+			int mcs2rate_ht40[16] = {
+				13500, 27500, 40500, 54000,
+				81500, 108000, 121500, 135000,
+				27000, 54000, 81000, 108000,
+				162000, 216000, 243000, 270000
+			};
+			int mcs2rate[16] = {
+				6500, 13000, 19500, 26000,
+				39000, 52000, 58500, 65000,
+				13000, 26000, 39000, 52000,
+				78000, 104000, 117000, 130000
+			};
+			int rate;
+			if (status->flag & RX_FLAG_HT40)
+				rate = mcs2rate_ht40[mcs_idx];
+			else
+				rate = mcs2rate[mcs_idx];
+
+			if (status->flag & RX_FLAG_SHORT_GI)
+				rate = rate * 10 / 9;
+
+			rate /= 500;

net/mac80211/rx.c +4 -1

@@ -1863,10 +1897,16 @@ static int prepare_for_handlers(struct i
 			if (!(sdata->dev->flags & IFF_PROMISC))
 				return 0;
 			rx->flags &= ~IEEE80211_RX_RA_MATCH;
-		} else if (!rx->sta)
+		} else if (!rx->sta) {
+			int rate_idx;
+			if (rx->status->flag & (RX_FLAG_HT20 | RX_FLAG_HT40))
+				rate_idx = 0; /* TODO: HT rates */

net/mac80211/rx.c +1 -1

@@ -2072,7 +2112,14 @@ static u8 ieee80211_sta_manage_reorder_b
 					tid_agg_rx->reorder_buf[index]->cb,
 					sizeof(status));
 				sband = local->hw.wiphy->bands[status.band];
-				rate = &sband->bitrates[status.rate_idx];
+				if (status.flag &

net/mac80211/rx.c +1 -4

@@ -2204,15 +2254,24 @@ void __ieee80211_rx(struct ieee80211_hw 
 	}
 
 	sband = local->hw.wiphy->bands[status->band];
-
-	if (!sband ||
-	    status->rate_idx < 0 ||
-	    status->rate_idx >= sband->n_bitrates) {
+	if (!sband) {

Quoted 6 lines · Jouni Malinen

Radiotap code gets some of the additional knowledge so that it can determine the used bitrate for HT rates, too (though, the rate field is not large enough to fit all rates). It would be useful to add MCS index, HT20/HT40, and short GI information into radiotap definition at some point, too, and the needed information for filling in those field is now available in mac80211.

Can we leave the rate calculation out here? I don't think we should do that much in this case, and we can extend radiotap later to include HT information.

Quoted 2 lines

The ath9k change fixes and cleans up the RX status reporting by getting rid of code that used internal rate tables

Yay. This is the last step for generic HT rate control with ath9k, I think.

Quoted 3 lines

+ * @RX_FLAG_HT20: HT20 MCS was used and rate_idx is MCS index
+ * @RX_FLAG_HT40: HT40 MCS was used and rate_idx is MCS index
+ * @RX_FLAG_SHORT_GI: Short guard interval was used

I think we should do RX_FLAG_HT and RX_FLAG_40MHZ so that

Quoted 1 line

+ * HT rates are use (RX_FLAG_HT20 or RX_FLAG_HT40)

this becomes just checking for RATE_FLAG_HT instead of both, what do you think?

Quoted 4 lines

--- wireless-testing.orig/net/mac80211/mlme.c 2008-12-11 16:48:40.000000000 +0200

net/mac80211/mlme.c +0 -0

@@ -1629,8 +1629,13 @@ static void ieee80211_rx_bss_info(struct
 			 * e.g: at 1 MBit that means mactime is 192 usec earlier
 			 * (=24 bytes * 8 usecs/byte) than the beacon timestamp.

net/mac80211/mlme.c static void ieee80211_rx_bss_info(struct · line 1639

Quoted 3 lines

net/mac80211/mlme.c +2 -0

+			if (rx_status->flag & (RX_FLAG_HT20 | RX_FLAG_HT40)) {
+				rate = 65; /* TODO: HT rates */
 			rx_timestamp = rx_status->mactime + (24 * 8 * 10 / rate);

Ick, ok, I give up, I guess we do need a function to calculate the raw bitrate after all. Let's put that into cfg80211 though, and then just call it here. Henning's function is nice, although it might be too expensive due to divisions...

Quoted 5 lines

net/mac80211/rx.c +4 -0

@@ -149,7 +149,41 @@ ieee80211_add_rx_radiotap_header(struct 
+			if (status->flag & RX_FLAG_HT40)
+				rate = mcs2rate_ht40[mcs_idx];
+			else
+				rate = mcs2rate[mcs_idx];

Can we just leave off the bitrate for HT for now?

Quoted 5 lines

net/mac80211/rx.c +4 -0

@@ -1863,10 +1897,16 @@ static int prepare_for_handlers(struct i
+			if (rx->status->flag & (RX_FLAG_HT20 | RX_FLAG_HT40))
+				rate_idx = 0; /* TODO: HT rates */
+			else
+				rate_idx = rx->status->rate_idx;

net/mac80211/rx.c static int prepare_for_handlers(struct i · line 1908

Quoted 4 lines

net/mac80211/rx.c +1 -1

 			rx->sta = ieee80211_ibss_add_sta(sdata, rx->skb,
 						bssid, hdr->addr2,
-						BIT(rx->status->rate_idx));
+						BIT(rate_idx));

Ok, that's icky, but you can at least use rate_idx = <mandatory rates> if you actually receive an HT frame for an IBSS peer, no? Of course, that is expensive to calculate, we might want to precalculate it...

net/mac80211/rx.c static u8 ieee80211_sta_manage_reorder_b · line 2114

Quoted 4 lines

net/mac80211/rx.c +0 -0

@@ -2072,7 +2112,14 @@ static u8 ieee80211_sta_manage_reorder_b
 				__ieee80211_rx_handle_packet(hw,
 					tid_agg_rx->reorder_buf[index],
 					&status, rate);

I think we need to change the internal rate representation... Or review it and see if we can allow NULL values. I.e. in struct ieee80211_rx_data we need to do MCS stuff too instead of using the first rate here.

net/mac80211/rx.c void __ieee80211_rx(struct ieee80211_hw · line 2266

Quoted 5 lines

net/mac80211/rx.c +5 -0

+	if (status->flag & (RX_FLAG_HT20 | RX_FLAG_HT40)) {
+		/* HT rates are not in the table - use the highest legacy rate
+		 * for now since other parts of mac80211 may not yet be fully
+		 * MCS aware. */
+		rate = &sband->bitrates[sband->n_bitrates - 1];

That's inconsistent, you're using the highest here but the lowest after the reorder buffer, no?

net/mac80211/rx.c void __ieee80211_rx(struct ieee80211_hw · line 2272

Quoted 7 lines

net/mac80211/rx.c +6 -0

+	} else {
+		if (status->rate_idx < 0 ||
+		    status->rate_idx >= sband->n_bitrates) {
+			WARN_ON(1);
+			return;
+		}

Could change to if(WARN_ON(...)) return :)

Quoted 1 line

Can we just leave off the bitrate for HT for now?

Yes. FreeBSD radiotap definition uses 0x80 as a flag in the rate field for indicating that it is an MCS index, not bitrate. That works fine here, too, and I'll update my patch to use the same style. No more bitrate calculation for HT rates in mac80211 after that..

I did not want to think about anything for IBSS.. ;-) One should just probe the other STA if you want to learn what it can do..

Quoted 2 lines · Jouni Malinen

Yes. FreeBSD radiotap definition uses 0x80 as a flag in the rate field for indicating that it is an MCS index, not bitrate.

Ugh. Nobody will dissect that properly.

Do you actually need this
information in userland _now_? Otherwise, Dave Young is moving the list elsewhere and then we can properly define this in radiotap rather than going with some other set. 0x80 in flags (not rate) is used to indicate short-GI in wireshark, and it's all very confusing... I'd rather see a new "HT rate" field added to radiotap.

Quoted 5 lines · Johannes Berg

Quoted 2 lines · Jouni Malinen

Yes. FreeBSD radiotap definition uses 0x80 as a flag in the rate field for indicating that it is an MCS index, not bitrate.

Ugh. Nobody will dissect that properly.

Actually, wireshark seems to have code to dissect that..

Well, yes, I do need it like yesterday. I really want to have a reliable sniffer that I can trust on providing correct information about received frames. I can obviously handle a personal patch that adds this, but it gets more complex when asking others to capture some frames for you..

Anyway, I will move the radiotap parts into a separate patch and leave it to others to decide whether that should go in now or not. The best option (if John is willing) would likely be to get the first part (mac80211 driver API addition and ath9k fix/cleanup) in and pushed upstream while the radiotap parts would be maintained only in wireless-testing for the time being.