[PATCH 0/3] mac80211: HT rate reporting in RX status

[PATCH 0/3] mac80211: HT rate reporting in RX status

[Jouni Malinen]

This series enhances the driver API to allow HT rates to be reported
to mac80211. The first patch includes the needed changes for mac80211
and the second one changes ath9k to use the new variables. The third
patch updates the radiotap format based on the version used in
FreeBSD. This is also supported by wireshark.

I would like to get the first two patches into the upstream kernel
releases, but the third one could remain only in wireless-testing
until the radiotap fields have been standardized more officially.
Anyway, these have been used in FreeBSD for ages and I would also be
fine with just including this in Linux releases as-is.

-- 
Jouni Malinen                                            PGP id EFC895FA

[PATCH 1/3] mac80211: Add HT rates into RX status reporting

[Jouni Malinen]

This patch adds option for HT-enabled drivers to report HT rates
(HT20/HT40, short GI, MCS index) to mac80211. These rates are
currently not in the rate table, so the rate_idx is used to indicate
MCS index.

Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>
---
 include/net/mac80211.h |   11 +++++++--
 net/mac80211/mlme.c    |    7 +++++-
 net/mac80211/rx.c      |   56 ++++++++++++++++++++++++++++++++++++++++---------
 3 files changed, 61 insertions(+), 13 deletions(-)

--- wireless-testing.orig/include/net/mac80211.h	2008-12-11 18:15:49.000000000 +0200
+++ wireless-testing/include/net/mac80211.h	2008-12-12 14:09:52.000000000 +0200
@@ -441,6 +441,9 @@ ieee80211_tx_info_clear_status(struct ie
  *	is valid. This is useful in monitor mode and necessary for beacon frames
  *	to enable IBSS merging.
  * @RX_FLAG_SHORTPRE: Short preamble was used for this frame
+ * @RX_FLAG_HT: HT MCS was used and rate_idx is MCS index
+ * @RX_FLAG_40MHZ: HT40 (40 MHz) was used
+ * @RX_FLAG_SHORT_GI: Short guard interval was used
  */
 enum mac80211_rx_flags {
 	RX_FLAG_MMIC_ERROR	= 1<<0,
@@ -451,7 +454,10 @@ enum mac80211_rx_flags {
 	RX_FLAG_FAILED_FCS_CRC	= 1<<5,
 	RX_FLAG_FAILED_PLCP_CRC = 1<<6,
 	RX_FLAG_TSFT		= 1<<7,
-	RX_FLAG_SHORTPRE	= 1<<8
+	RX_FLAG_SHORTPRE	= 1<<8,
+	RX_FLAG_HT		= 1<<9,
+	RX_FLAG_40MHZ		= 1<<10,
+	RX_FLAG_SHORT_GI	= 1<<11,
 };
 
 /**
@@ -471,7 +477,8 @@ enum mac80211_rx_flags {
  * @noise: noise when receiving this frame, in dBm.
  * @qual: overall signal quality indication, in percent (0-100).
  * @antenna: antenna used
- * @rate_idx: index of data rate into band's supported rates
+ * @rate_idx: index of data rate into band's supported rates or MCS index if
+ *	HT rates are use (RX_FLAG_HT)
  * @flag: %RX_FLAG_*
  */
 struct ieee80211_rx_status {
--- wireless-testing.orig/net/mac80211/mlme.c	2008-12-12 00:26:04.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2008-12-12 14:09:52.000000000 +0200
@@ -1627,8 +1627,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]->
+			int rate;
+			if (rx_status->flag & RX_FLAG_HT) {
+				rate = 65; /* TODO: HT rates */
+			} else {
+				rate = local->hw.wiphy->bands[band]->
 					bitrates[rx_status->rate_idx].bitrate;
+			}
 			rx_timestamp = rx_status->mactime + (24 * 8 * 10 / rate);
 		} else if (local && local->ops && local->ops->get_tsf)
 			/* second best option: get current TSF */
--- wireless-testing.orig/net/mac80211/rx.c	2008-12-12 00:26:04.000000000 +0200
+++ wireless-testing/net/mac80211/rx.c	2008-12-12 14:09:52.000000000 +0200
@@ -149,7 +149,17 @@ ieee80211_add_rx_radiotap_header(struct 
 	pos++;
 
 	/* IEEE80211_RADIOTAP_RATE */
-	*pos = rate->bitrate / 5;
+	if (status->flag & RX_FLAG_HT) {
+		/*
+		 * TODO: add following information into radiotap header once
+		 * suitable fields are defined for it:
+		 * - MCS index (status->rate_idx)
+		 * - HT40 (status->flag & RX_FLAG_40MHZ)
+		 * - short-GI (status->flag & RX_FLAG_SHORT_GI)
+		 */
+		*pos = 0;
+	} else
+		*pos = rate->bitrate / 5;
 	pos++;
 
 	/* IEEE80211_RADIOTAP_CHANNEL */
@@ -1863,9 +1873,15 @@ 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_HT)
+				rate_idx = 0; /* TODO: HT rates */
+			else
+				rate_idx = rx->status->rate_idx;
 			rx->sta = ieee80211_ibss_add_sta(sdata, bssid, hdr->addr2,
-				BIT(rx->status->rate_idx));
+				BIT(rate_idx));
+		}
 		break;
 	case NL80211_IFTYPE_MESH_POINT:
 		if (!multicast &&
@@ -2071,7 +2087,13 @@ 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 & RX_FLAG_HT) {
+					/* TODO: HT rates */
+					rate = sband->bitrates;
+				} else {
+					rate = &sband->bitrates
+						[status.rate_idx];
+				}
 				__ieee80211_rx_handle_packet(hw,
 					tid_agg_rx->reorder_buf[index],
 					&status, rate);
@@ -2115,7 +2137,10 @@ static u8 ieee80211_sta_manage_reorder_b
 		memcpy(&status, 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 & RX_FLAG_HT)
+			rate = sband->bitrates; /* TODO: HT rates */
+		else
+			rate = &sband->bitrates[status.rate_idx];
 		__ieee80211_rx_handle_packet(hw, tid_agg_rx->reorder_buf[index],
 					     &status, rate);
 		tid_agg_rx->stored_mpdu_num--;
@@ -2203,15 +2228,26 @@ 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) {
 		WARN_ON(1);
 		return;
 	}
 
-	rate = &sband->bitrates[status->rate_idx];
+	if (status->flag & RX_FLAG_HT) {
+		/* rate_idx is MCS index */
+		if (WARN_ON(status->rate_idx < 0 ||
+			    status->rate_idx >= 76))
+			return;
+		/* 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];
+	} else {
+		if (WARN_ON(status->rate_idx < 0 ||
+			    status->rate_idx >= sband->n_bitrates))
+			return;
+		rate = &sband->bitrates[status->rate_idx];
+	}
 
 	/*
 	 * key references and virtual interfaces are protected using RCU

-- 

-- 
Jouni Malinen                                            PGP id EFC895FA

[PATCH 2/3] ath9k: Report HT rates in RX status

[Jouni Malinen]

Fix and clean 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.

Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>
---
 drivers/net/wireless/ath9k/recv.c |   61 ++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 35 deletions(-)

--- wireless-testing.orig/drivers/net/wireless/ath9k/recv.c	2008-12-12 00:26:04.000000000 +0200
+++ wireless-testing/drivers/net/wireless/ath9k/recv.c	2008-12-12 14:09:59.000000000 +0200
@@ -111,33 +111,6 @@ static struct sk_buff *ath_rxbuf_alloc(s
 	return skb;
 }
 
-static int ath_rate2idx(struct ath_softc *sc, int rate)
-{
-	int i = 0, cur_band, n_rates;
-	struct ieee80211_hw *hw = sc->hw;
-
-	cur_band = hw->conf.channel->band;
-	n_rates = sc->sbands[cur_band].n_bitrates;
-
-	for (i = 0; i < n_rates; i++) {
-		if (sc->sbands[cur_band].bitrates[i].bitrate == rate)
-			break;
-	}
-
-	/*
-	 * NB:mac80211 validates rx rate index against the supported legacy rate
-	 * index only (should be done against ht rates also), return the highest
-	 * legacy rate index for rx rate which does not match any one of the
-	 * supported basic and extended rates to make mac80211 happy.
-	 * The following hack will be cleaned up once the issue with
-	 * the rx rate index validation in mac80211 is fixed.
-	 */
-	if (i == n_rates)
-		return n_rates - 1;
-
-	return i;
-}
-
 /*
  * For Decrypt or Demic errors, we only mark packet status here and always push
  * up the frame up to let mac80211 handle the actual error case, be it no
@@ -147,9 +120,7 @@ static int ath_rx_prepare(struct sk_buff
 			  struct ieee80211_rx_status *rx_status, bool *decrypt_error,
 			  struct ath_softc *sc)
 {
-	struct ath_rate_table *rate_table = sc->cur_rate_table;
 	struct ieee80211_hdr *hdr;
-	int ratekbps, rix;
 	u8 ratecode;
 	__le16 fc;
 
@@ -204,15 +175,36 @@ static int ath_rx_prepare(struct sk_buff
 	}
 
 	ratecode = ds->ds_rxstat.rs_rate;
-	rix = rate_table->rateCodeToIndex[ratecode];
-	ratekbps = rate_table->info[rix].ratekbps;
 
-	/* HT rate */
 	if (ratecode & 0x80) {
+		/* HT rate */
+		rx_status->flag |= RX_FLAG_HT;
 		if (ds->ds_rxstat.rs_flags & ATH9K_RX_2040)
-			ratekbps = (ratekbps * 27) / 13;
+			rx_status->flag |= RX_FLAG_40MHZ;
 		if (ds->ds_rxstat.rs_flags & ATH9K_RX_GI)
-			ratekbps = (ratekbps * 10) / 9;
+			rx_status->flag |= RX_FLAG_SHORT_GI;
+		rx_status->rate_idx = ratecode & 0x7f;
+	} else {
+		int i = 0, cur_band, n_rates;
+		struct ieee80211_hw *hw = sc->hw;
+
+		cur_band = hw->conf.channel->band;
+		n_rates = sc->sbands[cur_band].n_bitrates;
+
+		for (i = 0; i < n_rates; i++) {
+			if (sc->sbands[cur_band].bitrates[i].hw_value ==
+			    ratecode) {
+				rx_status->rate_idx = i;
+				break;
+			}
+
+			if (sc->sbands[cur_band].bitrates[i].hw_value_short ==
+			    ratecode) {
+				rx_status->rate_idx = i;
+				rx_status->flag |= RX_FLAG_SHORTPRE;
+				break;
+			}
+		}
 	}
 
 	rx_status->mactime = ath_extend_tsf(sc, ds->ds_rxstat.rs_tstamp);
@@ -220,7 +212,6 @@ static int ath_rx_prepare(struct sk_buff
 	rx_status->freq =  sc->hw->conf.channel->center_freq;
 	rx_status->noise = sc->sc_ani.sc_noise_floor;
 	rx_status->signal = rx_status->noise + ds->ds_rxstat.rs_rssi;
-	rx_status->rate_idx = ath_rate2idx(sc, (ratekbps / 100));
 	rx_status->antenna = ds->ds_rxstat.rs_antenna;
 
 	/* at 45 you will be able to use MCS 15 reliably. A more elaborate

-- 

-- 
Jouni Malinen                                            PGP id EFC895FA

[PATCH 3/3] mac80211: Add HT rates into radiotap

[Jouni Malinen]

Update the radiotap definition based on the format used in FreeBSD to
allows MCS index, HT20/HT40, and short GI information to be reported
for received frames in monitor mode. This format is also supported by
wireshark.

Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>
---
 include/net/ieee80211_radiotap.h |    7 +++++--
 net/mac80211/rx.c                |   28 ++++++++++++----------------
 2 files changed, 17 insertions(+), 18 deletions(-)

--- wireless-testing.orig/include/net/ieee80211_radiotap.h	2008-12-04 12:56:59.000000000 +0200
+++ wireless-testing/include/net/ieee80211_radiotap.h	2008-12-12 14:10:02.000000000 +0200
@@ -100,9 +100,10 @@ struct ieee80211_radiotap_header {
  *      For frequency-hopping radios, the hop set (first byte)
  *      and pattern (second byte).
  *
- * IEEE80211_RADIOTAP_RATE              u8           500kb/s
+ * IEEE80211_RADIOTAP_RATE              u8           500kb/s or index
  *
- *      Tx/Rx data rate
+ *      Tx/Rx data rate.  If bit 0x80 is set then it represents an
+ *      an MCS index and not an IEEE rate.
  *
  * IEEE80211_RADIOTAP_DBM_ANTSIGNAL     s8           decibels from
  *                                                   one milliwatt (dBm)
@@ -230,6 +231,8 @@ enum ieee80211_radiotap_type {
 						 * 802.11 header and payload
 						 * (to 32-bit boundary)
 						 */
+#define	IEEE80211_RADIOTAP_F_SHORTGI	0x80	/* HT short GI */
+
 /* For IEEE80211_RADIOTAP_RX_FLAGS */
 #define IEEE80211_RADIOTAP_F_RX_BADFCS	0x0001	/* frame failed crc check */
 
--- wireless-testing.orig/net/mac80211/rx.c	2008-12-12 14:09:52.000000000 +0200
+++ wireless-testing/net/mac80211/rx.c	2008-12-12 14:10:02.000000000 +0200
@@ -116,6 +116,7 @@ ieee80211_add_rx_radiotap_header(struct 
 {
 	struct ieee80211_radiotap_header *rthdr;
 	unsigned char *pos;
+	u16 chan_flags;
 
 	rthdr = (struct ieee80211_radiotap_header *)skb_push(skb, rtap_len);
 	memset(rthdr, 0, rtap_len);
@@ -146,19 +147,14 @@ ieee80211_add_rx_radiotap_header(struct 
 		*pos |= IEEE80211_RADIOTAP_F_FCS;
 	if (status->flag & RX_FLAG_SHORTPRE)
 		*pos |= IEEE80211_RADIOTAP_F_SHORTPRE;
+	if (status->flag & RX_FLAG_SHORT_GI)
+		*pos |= IEEE80211_RADIOTAP_F_SHORTGI;
 	pos++;
 
 	/* IEEE80211_RADIOTAP_RATE */
-	if (status->flag & RX_FLAG_HT) {
-		/*
-		 * TODO: add following information into radiotap header once
-		 * suitable fields are defined for it:
-		 * - MCS index (status->rate_idx)
-		 * - HT40 (status->flag & RX_FLAG_40MHZ)
-		 * - short-GI (status->flag & RX_FLAG_SHORT_GI)
-		 */
-		*pos = 0;
-	} else
+	if (status->flag & RX_FLAG_HT)
+		*pos = 0x80 | status->rate_idx;
+	else
 		*pos = rate->bitrate / 5;
 	pos++;
 
@@ -166,14 +162,14 @@ ieee80211_add_rx_radiotap_header(struct 
 	*(__le16 *)pos = cpu_to_le16(status->freq);
 	pos += 2;
 	if (status->band == IEEE80211_BAND_5GHZ)
-		*(__le16 *)pos = cpu_to_le16(IEEE80211_CHAN_OFDM |
-					     IEEE80211_CHAN_5GHZ);
+		chan_flags = IEEE80211_CHAN_OFDM | IEEE80211_CHAN_5GHZ;
 	else if (rate->flags & IEEE80211_RATE_ERP_G)
-		*(__le16 *)pos = cpu_to_le16(IEEE80211_CHAN_OFDM |
-					     IEEE80211_CHAN_2GHZ);
+		chan_flags = IEEE80211_CHAN_OFDM | IEEE80211_CHAN_2GHZ;
 	else
-		*(__le16 *)pos = cpu_to_le16(IEEE80211_CHAN_CCK |
-					     IEEE80211_CHAN_2GHZ);
+		chan_flags = IEEE80211_CHAN_CCK | IEEE80211_CHAN_2GHZ;
+	if (status->flag & RX_FLAG_40MHZ)
+		chan_flags |= IEEE80211_CHAN_TURBO;
+	*(__le16 *) pos = cpu_to_le16(chan_flags);
 	pos += 2;
 
 	/* IEEE80211_RADIOTAP_DBM_ANTSIGNAL */

-- 

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH 3/3] mac80211: Add HT rates into radiotap

[Stefanik Gábor]

On Fri, Dec 12, 2008 at 1:38 PM, Jouni Malinen
<jouni.malinen@atheros.com> wrote:
> Update the radiotap definition based on the format used in FreeBSD to
> allows MCS index, HT20/HT40, and short GI information to be reported
> for received frames in monitor mode. This format is also supported by
> wireshark.
>
> Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>
> ---
>  include/net/ieee80211_radiotap.h |    7 +++++--
>  net/mac80211/rx.c                |   28 ++++++++++++----------------
>  2 files changed, 17 insertions(+), 18 deletions(-)
>
> --- wireless-testing.orig/include/net/ieee80211_radiotap.h      2008-12-04 12:56:59.000000000 +0200
> +++ wireless-testing/include/net/ieee80211_radiotap.h   2008-12-12 14:10:02.000000000 +0200
> @@ -100,9 +100,10 @@ struct ieee80211_radiotap_header {
>  *      For frequency-hopping radios, the hop set (first byte)
>  *      and pattern (second byte).
>  *
> - * IEEE80211_RADIOTAP_RATE              u8           500kb/s
> + * IEEE80211_RADIOTAP_RATE              u8           500kb/s or index
>  *
> - *      Tx/Rx data rate
> + *      Tx/Rx data rate.  If bit 0x80 is set then it represents an
> + *      an MCS index and not an IEEE rate.
>  *
>  * IEEE80211_RADIOTAP_DBM_ANTSIGNAL     s8           decibels from
>  *                                                   one milliwatt (dBm)
> @@ -230,6 +231,8 @@ enum ieee80211_radiotap_type {
>                                                 * 802.11 header and payload
>                                                 * (to 32-bit boundary)
>                                                 */
> +#define        IEEE80211_RADIOTAP_F_SHORTGI    0x80    /* HT short GI */
> +
>  /* For IEEE80211_RADIOTAP_RX_FLAGS */
>  #define IEEE80211_RADIOTAP_F_RX_BADFCS 0x0001  /* frame failed crc check */
>
> --- wireless-testing.orig/net/mac80211/rx.c     2008-12-12 14:09:52.000000000 +0200
> +++ wireless-testing/net/mac80211/rx.c  2008-12-12 14:10:02.000000000 +0200
> @@ -116,6 +116,7 @@ ieee80211_add_rx_radiotap_header(struct
>  {
>        struct ieee80211_radiotap_header *rthdr;
>        unsigned char *pos;
> +       u16 chan_flags;
>
>        rthdr = (struct ieee80211_radiotap_header *)skb_push(skb, rtap_len);
>        memset(rthdr, 0, rtap_len);
> @@ -146,19 +147,14 @@ ieee80211_add_rx_radiotap_header(struct
>                *pos |= IEEE80211_RADIOTAP_F_FCS;
>        if (status->flag & RX_FLAG_SHORTPRE)
>                *pos |= IEEE80211_RADIOTAP_F_SHORTPRE;
> +       if (status->flag & RX_FLAG_SHORT_GI)
> +               *pos |= IEEE80211_RADIOTAP_F_SHORTGI;
>        pos++;
>
>        /* IEEE80211_RADIOTAP_RATE */
> -       if (status->flag & RX_FLAG_HT) {
> -               /*
> -                * TODO: add following information into radiotap header once
> -                * suitable fields are defined for it:
> -                * - MCS index (status->rate_idx)
> -                * - HT40 (status->flag & RX_FLAG_40MHZ)
> -                * - short-GI (status->flag & RX_FLAG_SHORT_GI)
> -                */
> -               *pos = 0;
> -       } else
> +       if (status->flag & RX_FLAG_HT)
> +               *pos = 0x80 | status->rate_idx;
> +       else
>                *pos = rate->bitrate / 5;
>        pos++;
>
> @@ -166,14 +162,14 @@ ieee80211_add_rx_radiotap_header(struct
>        *(__le16 *)pos = cpu_to_le16(status->freq);
>        pos += 2;
>        if (status->band == IEEE80211_BAND_5GHZ)
> -               *(__le16 *)pos = cpu_to_le16(IEEE80211_CHAN_OFDM |
> -                                            IEEE80211_CHAN_5GHZ);
> +               chan_flags = IEEE80211_CHAN_OFDM | IEEE80211_CHAN_5GHZ;
>        else if (rate->flags & IEEE80211_RATE_ERP_G)
> -               *(__le16 *)pos = cpu_to_le16(IEEE80211_CHAN_OFDM |
> -                                            IEEE80211_CHAN_2GHZ);
> +               chan_flags = IEEE80211_CHAN_OFDM | IEEE80211_CHAN_2GHZ;
>        else
> -               *(__le16 *)pos = cpu_to_le16(IEEE80211_CHAN_CCK |
> -                                            IEEE80211_CHAN_2GHZ);
> +               chan_flags = IEEE80211_CHAN_CCK | IEEE80211_CHAN_2GHZ;
> +       if (status->flag & RX_FLAG_40MHZ)
> +               chan_flags |= IEEE80211_CHAN_TURBO;
> +       *(__le16 *) pos = cpu_to_le16(chan_flags);
>        pos += 2;
>
>        /* IEEE80211_RADIOTAP_DBM_ANTSIGNAL */
>
> --
>
> --
> Jouni Malinen                                            PGP id EFC895FA
> --
> 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
>

Thanks for the heads up, mail forwarded to Thomas d'Otreppe, as this
will need changes in airodump-ng.

-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [PATCH 3/3] mac80211: Add HT rates into radiotap

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]

On Fri, 2008-12-12 at 15:47 +0100, Stefanik Gábor wrote:

> Thanks for the heads up, mail forwarded to Thomas d'Otreppe, as this
> will need changes in airodump-ng.

Guys! This format is brain-dead, it means we don't know 40MHz or SGI,
let's define something proper in radiotap instead!

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] mac80211: Add HT rates into radiotap

[Jouni Malinen]

On Fri, Dec 12, 2008 at 03:48:47PM +0100, Johannes Berg wrote:
> On Fri, 2008-12-12 at 15:47 +0100, Stefanik G=C3=A1bor wrote:
>=20
> > Thanks for the heads up, mail forwarded to Thomas d'Otreppe, as thi=
s
> > will need changes in airodump-ng.
>=20
> Guys! This format is brain-dead, it means we don't know 40MHz or SGI,
> let's define something proper in radiotap instead!

While I'm not against defining something more officially for radiotap,
I do not agree with the part about this format being that brain-dead.
Both 40 MHz and short GI are indicated (IEEE80211_CHAN_TURBO and
IEEE80211_RADIOTAP_F_SHORTGI) and shown in the current wireshark
release.

--=20
Jouni Malinen                                            PGP id EFC895F=
A
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] mac80211: Add HT rates into radiotap

[Henning Rogge]

[-- Attachment #1: Type: text/plain, Size: 562 bytes --]

On Friday 12 December 2008 16:16:19 Jouni Malinen wrote:
> While I'm not against defining something more officially for radiotap,
> I do not agree with the part about this format being that brain-dead.
> Both 40 MHz and short GI are indicated (IEEE80211_CHAN_TURBO and
> IEEE80211_RADIOTAP_F_SHORTGI) and shown in the current wireshark
> release.
Changing the meaning of the old "txrate" field (by putting an index inside) is 
a great way to break the userspace interface to any existing program. :(

Just get a new field for the mcs index.

Henning

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/3] mac80211: Add HT rates into RX status reporting

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 985 bytes --]

On Fri, 2008-12-12 at 14:38 +0200, Jouni Malinen wrote:

> --- wireless-testing.orig/net/mac80211/rx.c	2008-12-12 00:26:04.000000000 +0200
> +++ wireless-testing/net/mac80211/rx.c	2008-12-12 14:09:52.000000000 +0200
> @@ -149,7 +149,17 @@ ieee80211_add_rx_radiotap_header(struct 
>  	pos++;
>  
>  	/* IEEE80211_RADIOTAP_RATE */
> -	*pos = rate->bitrate / 5;
> +	if (status->flag & RX_FLAG_HT) {
> +		/*
> +		 * TODO: add following information into radiotap header once
> +		 * suitable fields are defined for it:
> +		 * - MCS index (status->rate_idx)
> +		 * - HT40 (status->flag & RX_FLAG_40MHZ)
> +		 * - short-GI (status->flag & RX_FLAG_SHORT_GI)
> +		 */
> +		*pos = 0;
> +	} else
> +		*pos = rate->bitrate / 5;
>  	pos++;

I think we should also unset the 1<<IEEE80211_RADIOTAP_RATE bit in the
HT case so the *pos = 0 will just be padding to dissectors. That's
correct then because the field after this (channel) requires 2-byte
alignment.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] mac80211: Add HT rates into radiotap

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 665 bytes --]

On Fri, 2008-12-12 at 14:38 +0200, Jouni Malinen wrote:

> - *      Tx/Rx data rate
> + *      Tx/Rx data rate.  If bit 0x80 is set then it represents an
> + *      an MCS index and not an IEEE rate.

I really don't like this much because it means that old dissectors will
be confused, and the HT information is at like three different places...

We really should make an HT amendment for radiotap on the radiotap list.
Once that works again. grmbl.

For now I guess we can put this patch into -testing but I'd prefer not
letting it migrate to the official tree, so that we avoid tools starting
to rely on this totally weird information.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] mac80211: Add HT rates into radiotap

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 908 bytes --]

On Fri, 2008-12-12 at 17:16 +0200, Jouni Malinen wrote:
> On Fri, Dec 12, 2008 at 03:48:47PM +0100, Johannes Berg wrote:
> > On Fri, 2008-12-12 at 15:47 +0100, Stefanik Gábor wrote:
> > 
> > > Thanks for the heads up, mail forwarded to Thomas d'Otreppe, as this
> > > will need changes in airodump-ng.
> > 
> > Guys! This format is brain-dead, it means we don't know 40MHz or SGI,
> > let's define something proper in radiotap instead!
> 
> While I'm not against defining something more officially for radiotap,
> I do not agree with the part about this format being that brain-dead.
> Both 40 MHz and short GI are indicated (IEEE80211_CHAN_TURBO and
> IEEE80211_RADIOTAP_F_SHORTGI) and shown in the current wireshark
> release.

Oh, right, I noticed. Well, as I just said, it's still pretty bogus
because we reuse turbo and confuse older dissectors with the 0x80 for HT
bit.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] mac80211: Add HT rates into radiotap

[Jouni Malinen]

On Fri, Dec 12, 2008 at 04:31:51PM +0100, Henning Rogge wrote:
> On Friday 12 December 2008 16:16:19 Jouni Malinen wrote:

> Changing the meaning of the old "txrate" field (by putting an index inside) is 
> a great way to break the userspace interface to any existing program. :(

Well, depends on what you mean with breaking ;-). In this particular
case, the output may show incorrect rate in a case where the previous
version showed incorrect rate (e.g., ath9k just reported the highest
legacy rate when HT rates were used). In context of Linux and radiotap,
there wasn't really anything working before that this would break.

> Just get a new field for the mcs index.

Unfortunately, that does not seem to be that easy a task.. I tried to
subscribe to the radiotap mailing list, but the info page is returning
403 Forbidden. Then I tried looking at the archive, and that returned
404.. If I've understood correctly, that has been the state of the
mailing list for a long time. Furthermore, many (most?) people working
with radiotap for Linux/*BSD/wireshark/tcpdump seem to have more or less
given up on the effort to come up with a shared specification for the
format.

-- 
Jouni Malinen                                            PGP id EFC895FA