[PATCH 0/2] add support for mcs masks

[PATCH 0/2] add support for mcs masks

[Simon Wunderlich]

This patchset adds support for fixed HT datarates. The configuration is
set through nl80211, and now accepts MCS masks next to the legacy masks.

A user may also choose to only use HT datarates and no legacy rates at all -
as always when fiddling with datarates, he should know what he's doing,
and a fallback in the rate matching functions allow to use the rate controls
selected rate if no matching rate was found. The rate matching was enhanced
to consider MCS rates as well.

I have tested this feature in IBSS mode with ath9k devices and with and without
NoAcks, and (at least for me) it works as expected.

There is an iw patch as well to make use of this feature.

It should apply well on the latest wireless-testing kernel.

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fokus.fraunhofer.de>

Simon Wunderlich (2):
  nl80211: add support for mcs masks
  mac80211: add support for mcs masks

 include/linux/nl80211.h       |    4 +
 include/net/cfg80211.h        |    3 +-
 include/net/mac80211.h        |    1 +
 net/mac80211/cfg.c            |    5 +-
 net/mac80211/debugfs_netdev.c |   34 +++++++++++
 net/mac80211/ieee80211_i.h    |    1 +
 net/mac80211/iface.c          |    3 +
 net/mac80211/rate.c           |  129 +++++++++++++++++++++++++++++++++++++----
 net/mac80211/tx.c             |    5 ++
 net/wireless/nl80211.c        |   54 +++++++++++++++++-
 10 files changed, 223 insertions(+), 16 deletions(-)

-- 
1.7.7.3

[PATCH 1/2] nl80211: add support for mcs masks

[Simon Wunderlich]

Allow to set mcs masks through nl80211. We also allow to set MCS
rates but no legacy rates (and vice versa).

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fokus.fraunhofer.de>
---
 include/linux/nl80211.h |    4 +++
 include/net/cfg80211.h  |    3 +-
 net/wireless/nl80211.c  |   54 ++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index a187606..f1eca8a 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1475,6 +1475,7 @@ enum nl80211_attrs {
 #define NL80211_ATTR_FEATURE_FLAGS NL80211_ATTR_FEATURE_FLAGS
 
 #define NL80211_MAX_SUPP_RATES			32
+#define NL80211_MAX_SUPP_HT_RATES		255
 #define NL80211_MAX_SUPP_REG_RULES		32
 #define NL80211_TKIP_DATA_OFFSET_ENCR_KEY	0
 #define NL80211_TKIP_DATA_OFFSET_TX_MIC_KEY	16
@@ -2395,12 +2396,15 @@ enum nl80211_key_attributes {
  *	in an array of rates as defined in IEEE 802.11 7.3.2.2 (u8 values with
  *	1 = 500 kbps) but without the IE length restriction (at most
  *	%NL80211_MAX_SUPP_RATES in a single array).
+ * @NL80211_TXRATE_MCS: HT (MCS) rates allowed for TX rate selection
+ *	in an array of MCS numbers.
  * @__NL80211_TXRATE_AFTER_LAST: internal
  * @NL80211_TXRATE_MAX: highest TX rate attribute
  */
 enum nl80211_tx_rate_attributes {
 	__NL80211_TXRATE_INVALID,
 	NL80211_TXRATE_LEGACY,
+	NL80211_TXRATE_MCS,
 
 	/* keep last */
 	__NL80211_TXRATE_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 150c0ee..4b6ff1c 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1223,8 +1223,7 @@ enum wiphy_params_flags {
 struct cfg80211_bitrate_mask {
 	struct {
 		u32 legacy;
-		/* TODO: add support for masking MCS rates; e.g.: */
-		/* u8 mcs[IEEE80211_HT_MCS_MASK_LEN]; */
+		u8 mcs[IEEE80211_HT_MCS_MASK_LEN];
 	} control[IEEE80211_NUM_BANDS];
 };
 /**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index ba43966..6ab1b35 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5324,9 +5324,39 @@ static u32 rateset_to_mask(struct ieee80211_supported_band *sband,
 	return mask;
 }
 
+static bool ht_rateset_to_mask(struct ieee80211_supported_band *sband,
+			       u8 *rates, u8 rates_len,
+			       u8 mcs[IEEE80211_HT_MCS_MASK_LEN])
+{
+	u8 i;
+
+	memset(mcs, 0, IEEE80211_HT_MCS_MASK_LEN);
+
+	for (i = 0; i < rates_len; i++) {
+		int ridx, rbit;
+
+		ridx = rates[i] / 8;
+		rbit = BIT(rates[i] % 8);
+
+		/* check validity */
+		if ((ridx < 0) || (ridx > IEEE80211_HT_MCS_MASK_LEN))
+			return false;
+
+		/* check availability */
+		if (sband->ht_cap.mcs.rx_mask[ridx] & rbit)
+			mcs[ridx] |= rbit;
+		else
+			return false;
+	}
+
+	return true;
+}
+
 static const struct nla_policy nl80211_txattr_policy[NL80211_TXRATE_MAX + 1] = {
 	[NL80211_TXRATE_LEGACY] = { .type = NLA_BINARY,
 				    .len = NL80211_MAX_SUPP_RATES },
+	[NL80211_TXRATE_MCS] = { .type = NLA_BINARY,
+				 .len = NL80211_MAX_SUPP_HT_RATES },
 };
 
 static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
@@ -5352,6 +5382,8 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
 		sband = rdev->wiphy.bands[i];
 		mask.control[i].legacy =
 			sband ? (1 << sband->n_bitrates) - 1 : 0;
+		memset(&mask.control[i].mcs, sband ? 0xff : 0,
+		       sizeof(mask.control[i].mcs));
 	}
 
 	/*
@@ -5373,7 +5405,27 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
 				sband,
 				nla_data(tb[NL80211_TXRATE_LEGACY]),
 				nla_len(tb[NL80211_TXRATE_LEGACY]));
-			if (mask.control[band].legacy == 0)
+		}
+		if (tb[NL80211_TXRATE_MCS]) {
+			if (!ht_rateset_to_mask(sband,
+				nla_data(tb[NL80211_TXRATE_MCS]),
+				nla_len(tb[NL80211_TXRATE_MCS]),
+				mask.control[band].mcs))
+				return -EINVAL;
+		}
+
+		if (mask.control[band].legacy == 0) {
+			/* don't allow empty legacy rates if HT
+			 * is not even supported. */
+			if (!rdev->wiphy.bands[band]->ht_cap.ht_supported)
+				return -EINVAL;
+
+			for (i = 0; i < IEEE80211_HT_MCS_MASK_LEN; i++)
+				if (mask.control[band].mcs[i])
+					break;
+
+			/* legacy and mcs rates may not be both empty */
+			if (i == IEEE80211_HT_MCS_MASK_LEN)
 				return -EINVAL;
 		}
 	}
-- 
1.7.7.3

Re: [PATCH 1/2] nl80211: add support for mcs masks

[Ben Greear]

On 12/19/2011 06:14 AM, Simon Wunderlich wrote:
> Allow to set mcs masks through nl80211. We also allow to set MCS
> rates but no legacy rates (and vice versa).

Can you not use the ht-caps logic that went in recently?  I was able
to successfully set station MCS rates with it.  It only works for
station devices at this point...but AP support could probably be
added without too much trouble.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 1/2] nl80211: add support for mcs masks

[Simon Wunderlich]

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

Hello Ben,

On Mon, Dec 19, 2011 at 07:28:52AM -0800, Ben Greear wrote:
> On 12/19/2011 06:14 AM, Simon Wunderlich wrote:
> >Allow to set mcs masks through nl80211. We also allow to set MCS
> >rates but no legacy rates (and vice versa).
> 
> Can you not use the ht-caps logic that went in recently?  I was able
> to successfully set station MCS rates with it.  It only works for
> station devices at this point...but AP support could probably be
> added without too much trouble.

I have not tried that out yet, but one important side effect we are
using here is to force HT mode, also for No Ack packets which are
usually sent with the lowest (legacy) rate. We are building fixed
links and want to set a fixed HT rate there, and only this rate should
be used whenever possible (and no legacy rates).

As far as I understand, the ht caps overrides will only override
HT configurations, but what I want is to also let the rate matcher
consider HT rates too and to disable legacy rates completely.

As the mcs masks support has been prepared a long time ago in iw and
mac80211, it did not seem to be the worst idea to finally fill this
gap and implement it. :)

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/2] nl80211: add support for mcs masks

[Ben Greear]

On 12/19/2011 01:14 PM, Simon Wunderlich wrote:
> Hello Ben,
>
> On Mon, Dec 19, 2011 at 07:28:52AM -0800, Ben Greear wrote:
>> On 12/19/2011 06:14 AM, Simon Wunderlich wrote:
>>> Allow to set mcs masks through nl80211. We also allow to set MCS
>>> rates but no legacy rates (and vice versa).
>>
>> Can you not use the ht-caps logic that went in recently?  I was able
>> to successfully set station MCS rates with it.  It only works for
>> station devices at this point...but AP support could probably be
>> added without too much trouble.
>
> I have not tried that out yet, but one important side effect we are
> using here is to force HT mode, also for No Ack packets which are
> usually sent with the lowest (legacy) rate. We are building fixed
> links and want to set a fixed HT rate there, and only this rate should
> be used whenever possible (and no legacy rates).

The ht-caps patch gives the ability to specify the available MCS rates
(again, for stations only at this point).  I didn't try forcing speeds
higher, but I did test that it could force speeds lower.  It requires
a patch to hostapd that I've posted, but it has not yet been accepted.

There is no way to disable legacy rates in the current code, as far
as I know.

>
> As far as I understand, the ht caps overrides will only override
> HT configurations, but what I want is to also let the rate matcher
> consider HT rates too and to disable legacy rates completely.
>
> As the mcs masks support has been prepared a long time ago in iw and
> mac80211, it did not seem to be the worst idea to finally fill this
> gap and implement it. :)

I haven't read your patch in detail, and at the least, the ht-caps code
would need some additional work to support your needs.  So, I'm not
sure whether working on top of the ht-caps logic is useful or not.

Likely Johannes will have more useful feedback when he gets the time.

I will try to read your patch in more detail when I get a chance.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 1/2] nl80211: add support for mcs masks

[Johannes Berg]

On Mon, 2011-12-19 at 15:14 +0100, Simon Wunderlich wrote:
> Allow to set mcs masks through nl80211. We also allow to set MCS
> rates but no legacy rates (and vice versa).
> 
> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
> Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fokus.fraunhofer.de>
> ---
>  include/linux/nl80211.h |    4 +++
>  include/net/cfg80211.h  |    3 +-
>  net/wireless/nl80211.c  |   54 ++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 58 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> index a187606..f1eca8a 100644
> --- a/include/linux/nl80211.h
> +++ b/include/linux/nl80211.h
> @@ -1475,6 +1475,7 @@ enum nl80211_attrs {
>  #define NL80211_ATTR_FEATURE_FLAGS NL80211_ATTR_FEATURE_FLAGS
>  
>  #define NL80211_MAX_SUPP_RATES			32
> +#define NL80211_MAX_SUPP_HT_RATES		255

That doesn't seem to make a lot of sense, the max is 77 anyway?

> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 150c0ee..4b6ff1c 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1223,8 +1223,7 @@ enum wiphy_params_flags {
>  struct cfg80211_bitrate_mask {
>  	struct {
>  		u32 legacy;
> -		/* TODO: add support for masking MCS rates; e.g.: */
> -		/* u8 mcs[IEEE80211_HT_MCS_MASK_LEN]; */
> +		u8 mcs[IEEE80211_HT_MCS_MASK_LEN];

And if the max above is 255, then this wouldn't be sufficient.

Maybe add

BUILD_BUG_ON(NL80211_MAX_SUPP_HT_RATES > IEEE80211_HT_MCS_MASK_LEN * 8);

somewhere.

> @@ -5352,6 +5382,8 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
>  		sband = rdev->wiphy.bands[i];
>  		mask.control[i].legacy =
>  			sband ? (1 << sband->n_bitrates) - 1 : 0;
> +		memset(&mask.control[i].mcs, sband ? 0xff : 0,
> +		       sizeof(mask.control[i].mcs));
>  	}

Would it make sense to copy the supported ones from the sband rather
than setting a mask that includes more than could possibly be supported?

> @@ -5373,7 +5405,27 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
>  				sband,
>  				nla_data(tb[NL80211_TXRATE_LEGACY]),
>  				nla_len(tb[NL80211_TXRATE_LEGACY]));
> -			if (mask.control[band].legacy == 0)
> +		}
> +		if (tb[NL80211_TXRATE_MCS]) {
> +			if (!ht_rateset_to_mask(sband,
> +				nla_data(tb[NL80211_TXRATE_MCS]),
> +				nla_len(tb[NL80211_TXRATE_MCS]),
> +				mask.control[band].mcs))
> +				return -EINVAL;

Indentation looks strange here.


johannes

[PATCH 2/2] mac80211: add support for mcs masks

[Simon Wunderlich]

* Handle MCS masks set by the user.
* Match rates provided by the rate control algorithm to the mask set,
  also in HT mode, and switch back to legacy mode if necessary.
* add debugfs files to observate the rate selection

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fokus.fraunhofer.de>
---
 include/net/mac80211.h        |    1 +
 net/mac80211/cfg.c            |    5 +-
 net/mac80211/debugfs_netdev.c |   34 +++++++++++
 net/mac80211/ieee80211_i.h    |    1 +
 net/mac80211/iface.c          |    3 +
 net/mac80211/rate.c           |  129 +++++++++++++++++++++++++++++++++++++----
 net/mac80211/tx.c             |    5 ++
 7 files changed, 165 insertions(+), 13 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 5b5c8a7..4bce803 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3537,6 +3537,7 @@ struct ieee80211_tx_rate_control {
 	bool rts, short_preamble;
 	u8 max_rate_idx;
 	u32 rate_idx_mask;
+	u8 rate_idx_mcs_mask[IEEE80211_HT_MCS_MASK_LEN];
 	bool bss;
 };
 
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 393b2a4..4862a73 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1865,8 +1865,11 @@ static int ieee80211_set_bitrate_mask(struct wiphy *wiphy,
 			return ret;
 	}
 
-	for (i = 0; i < IEEE80211_NUM_BANDS; i++)
+	for (i = 0; i < IEEE80211_NUM_BANDS; i++) {
 		sdata->rc_rateidx_mask[i] = mask->control[i].legacy;
+		memcpy(sdata->rc_rateidx_mcs_mask[i], mask->control[i].mcs,
+		       sizeof(mask->control[i].mcs));
+	}
 
 	return 0;
 }
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index 8df2891..6e2ff9c 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -87,6 +87,21 @@ static ssize_t ieee80211_if_fmt_##name(					\
 #define IEEE80211_IF_FMT_SIZE(name, field)				\
 		IEEE80211_IF_FMT(name, field, "%zd\n")
 
+#define IEEE80211_IF_FMT_HEXARRAY(name, field)				\
+static ssize_t ieee80211_if_fmt_##name(					\
+	const struct ieee80211_sub_if_data *sdata,			\
+	char *buf, int buflen)						\
+{									\
+	char *p = buf;							\
+	int i;								\
+	for (i = 0; i < sizeof(sdata->field); i++) {			\
+		p += scnprintf(p, buflen + buf - p, "%.2x ",		\
+				 sdata->field[i]);			\
+	}								\
+	p += scnprintf(p, buflen + buf - p, "\n");			\
+	return p - buf;							\
+}
+
 #define IEEE80211_IF_FMT_ATOMIC(name, field)				\
 static ssize_t ieee80211_if_fmt_##name(					\
 	const struct ieee80211_sub_if_data *sdata,			\
@@ -148,6 +163,11 @@ IEEE80211_IF_FILE(rc_rateidx_mask_2ghz, rc_rateidx_mask[IEEE80211_BAND_2GHZ],
 		  HEX);
 IEEE80211_IF_FILE(rc_rateidx_mask_5ghz, rc_rateidx_mask[IEEE80211_BAND_5GHZ],
 		  HEX);
+IEEE80211_IF_FILE(rc_rateidx_mcs_mask_2ghz,
+		  rc_rateidx_mcs_mask[IEEE80211_BAND_2GHZ], HEXARRAY);
+IEEE80211_IF_FILE(rc_rateidx_mcs_mask_5ghz,
+		  rc_rateidx_mcs_mask[IEEE80211_BAND_5GHZ], HEXARRAY);
+
 IEEE80211_IF_FILE(flags, flags, HEX);
 IEEE80211_IF_FILE(state, state, LHEX);
 IEEE80211_IF_FILE(channel_type, vif.bss_conf.channel_type, DEC);
@@ -440,6 +460,8 @@ static void add_sta_files(struct ieee80211_sub_if_data *sdata)
 	DEBUGFS_ADD(channel_type);
 	DEBUGFS_ADD(rc_rateidx_mask_2ghz);
 	DEBUGFS_ADD(rc_rateidx_mask_5ghz);
+	DEBUGFS_ADD(rc_rateidx_mcs_mask_2ghz);
+	DEBUGFS_ADD(rc_rateidx_mcs_mask_5ghz);
 
 	DEBUGFS_ADD(bssid);
 	DEBUGFS_ADD(aid);
@@ -457,6 +479,8 @@ static void add_ap_files(struct ieee80211_sub_if_data *sdata)
 	DEBUGFS_ADD(channel_type);
 	DEBUGFS_ADD(rc_rateidx_mask_2ghz);
 	DEBUGFS_ADD(rc_rateidx_mask_5ghz);
+	DEBUGFS_ADD(rc_rateidx_mcs_mask_2ghz);
+	DEBUGFS_ADD(rc_rateidx_mcs_mask_5ghz);
 
 	DEBUGFS_ADD(num_sta_ps);
 	DEBUGFS_ADD(dtim_count);
@@ -466,6 +490,12 @@ static void add_ap_files(struct ieee80211_sub_if_data *sdata)
 
 static void add_ibss_files(struct ieee80211_sub_if_data *sdata)
 {
+	DEBUGFS_ADD(channel_type);
+	DEBUGFS_ADD(rc_rateidx_mask_2ghz);
+	DEBUGFS_ADD(rc_rateidx_mask_5ghz);
+	DEBUGFS_ADD(rc_rateidx_mcs_mask_2ghz);
+	DEBUGFS_ADD(rc_rateidx_mcs_mask_5ghz);
+
 	DEBUGFS_ADD_MODE(tsf, 0600);
 }
 
@@ -477,6 +507,8 @@ static void add_wds_files(struct ieee80211_sub_if_data *sdata)
 	DEBUGFS_ADD(channel_type);
 	DEBUGFS_ADD(rc_rateidx_mask_2ghz);
 	DEBUGFS_ADD(rc_rateidx_mask_5ghz);
+	DEBUGFS_ADD(rc_rateidx_mcs_mask_2ghz);
+	DEBUGFS_ADD(rc_rateidx_mcs_mask_5ghz);
 
 	DEBUGFS_ADD(peer);
 }
@@ -489,6 +521,8 @@ static void add_vlan_files(struct ieee80211_sub_if_data *sdata)
 	DEBUGFS_ADD(channel_type);
 	DEBUGFS_ADD(rc_rateidx_mask_2ghz);
 	DEBUGFS_ADD(rc_rateidx_mask_5ghz);
+	DEBUGFS_ADD(rc_rateidx_mcs_mask_2ghz);
+	DEBUGFS_ADD(rc_rateidx_mcs_mask_5ghz);
 }
 
 static void add_monitor_files(struct ieee80211_sub_if_data *sdata)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 96fe754..bff632f 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -642,6 +642,7 @@ struct ieee80211_sub_if_data {
 
 	/* bitmap of allowed (non-MCS) rate indexes for rate control */
 	u32 rc_rateidx_mask[IEEE80211_NUM_BANDS];
+	u8  rc_rateidx_mcs_mask[IEEE80211_NUM_BANDS][IEEE80211_HT_MCS_MASK_LEN];
 
 	union {
 		struct ieee80211_if_ap ap;
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 3d3bb5e..93328c9 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1180,6 +1180,9 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
 		sband = local->hw.wiphy->bands[i];
 		sdata->rc_rateidx_mask[i] =
 			sband ? (1 << sband->n_bitrates) - 1 : 0;
+		memset(sdata->rc_rateidx_mcs_mask[i], sband ? 0xff : 0,
+		       sizeof(sdata->rc_rateidx_mcs_mask[i]));
+
 	}
 
 	/* setup type-dependent data */
diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 5a5a776..3b6ace2 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -267,10 +267,10 @@ bool rate_control_send_low(struct ieee80211_sta *sta,
 
 	if (!sta || !priv_sta || rc_no_data_or_no_ack_use_min(txrc)) {
 		if ((sband->band != IEEE80211_BAND_2GHZ) ||
-		    !(info->flags & IEEE80211_TX_CTL_NO_CCK_RATE))
+		    !(info->flags & IEEE80211_TX_CTL_NO_CCK_RATE)) {
 			info->control.rates[0].idx =
 				rate_lowest_index(txrc->sband, sta);
-		else
+		} else
 			info->control.rates[0].idx =
 				rate_lowest_non_cck_index(txrc->sband, sta);
 		info->control.rates[0].count =
@@ -293,25 +293,128 @@ bool rate_control_send_low(struct ieee80211_sta *sta,
 }
 EXPORT_SYMBOL(rate_control_send_low);
 
-static void rate_idx_match_mask(struct ieee80211_tx_rate *rate,
-				int n_bitrates, u32 mask)
+static bool rate_idx_match_legacy_mask(struct ieee80211_tx_rate *rate,
+				       int n_bitrates, u32 mask)
 {
 	int j;
 
-	/* See whether the selected rate or anything below it is allowed. */
+	/* See whether the selected rate or anything below
+	 * it is allowed. */
 	for (j = rate->idx; j >= 0; j--) {
 		if (mask & (1 << j)) {
 			/* Okay, found a suitable rate. Use it. */
 			rate->idx = j;
-			return;
+			return true;
 		}
 	}
-
 	/* Try to find a higher rate that would be allowed */
 	for (j = rate->idx + 1; j < n_bitrates; j++) {
 		if (mask & (1 << j)) {
 			/* Okay, found a suitable rate. Use it. */
 			rate->idx = j;
+			return true;
+		}
+	}
+	return false;
+}
+
+static bool rate_idx_match_mcs_mask(struct ieee80211_tx_rate *rate,
+				    u8 mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
+{
+	int i, j;
+	int ridx, rbit;
+
+	ridx = rate->idx / 8;
+	rbit = rate->idx % 8;
+
+	/* See whether the selected rate or anything below
+	 * it is allowed. */
+	for (i = ridx; i >= 0; i--) {
+		for (j = rbit; j >= 0; j--)
+			if (mcs_mask[i] & BIT(j)) {
+				rate->idx = i * 8 + j;
+				return true;
+			}
+		rbit = 7;
+	}
+
+	/* Try to find a higher rate that would be allowed */
+	ridx = (rate->idx + 1) / 8;
+	rbit = (rate->idx + 1) % 8;
+
+	for (i = ridx; i < IEEE80211_HT_MCS_MASK_LEN; i++) {
+		for (j = rbit; j < 8; j++)
+			if (mcs_mask[i] & BIT(j)) {
+				rate->idx = i * 8 + j;
+				return true;
+			}
+		rbit = 0;
+	}
+	return false;
+}
+
+
+
+static void rate_idx_match_mask(struct ieee80211_tx_rate *rate,
+				struct ieee80211_tx_rate_control *txrc,
+				u32 mask,
+				u8 mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
+{
+	struct ieee80211_tx_rate alt_rate;
+
+	/* handle HT rates */
+	if (rate->flags & IEEE80211_TX_RC_MCS) {
+		if (rate_idx_match_mcs_mask(rate, mcs_mask))
+			return;
+
+		/* also try the legacy rates. */
+		alt_rate.idx = 0;
+		/* keep protection flags */
+		alt_rate.flags = rate->flags &
+				 (IEEE80211_TX_RC_USE_RTS_CTS |
+				  IEEE80211_TX_RC_USE_CTS_PROTECT |
+				  IEEE80211_TX_RC_USE_SHORT_PREAMBLE);
+		alt_rate.count = rate->count;
+		if (rate_idx_match_legacy_mask(&alt_rate,
+					       txrc->sband->n_bitrates,
+					       mask)) {
+			*rate = alt_rate;
+			return;
+		}
+	} else {
+		struct sk_buff *skb = txrc->skb;
+		struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+		__le16 fc;
+
+		/* handle legacy rates */
+		if (rate_idx_match_legacy_mask(rate, txrc->sband->n_bitrates,
+					       mask))
+			return;
+
+		/* if HT BSS, and we handle a data frame, also try HT rates */
+		if (txrc->bss_conf->channel_type == NL80211_CHAN_NO_HT)
+			return;
+
+		fc = hdr->frame_control;
+		if (!ieee80211_is_data(fc))
+			return;
+
+		alt_rate.idx = 0;
+		/* keep protection flags */
+		alt_rate.flags = rate->flags &
+				 (IEEE80211_TX_RC_USE_RTS_CTS |
+				  IEEE80211_TX_RC_USE_CTS_PROTECT |
+				  IEEE80211_TX_RC_USE_SHORT_PREAMBLE);
+		alt_rate.count = rate->count;
+
+		alt_rate.flags |= IEEE80211_TX_RC_MCS;
+
+		if ((txrc->bss_conf->channel_type == NL80211_CHAN_HT40MINUS) ||
+		    (txrc->bss_conf->channel_type == NL80211_CHAN_HT40PLUS))
+			alt_rate.flags |= IEEE80211_TX_RC_40_MHZ_WIDTH;
+
+		if (rate_idx_match_mcs_mask(&alt_rate, mcs_mask)) {
+			*rate = alt_rate;
 			return;
 		}
 	}
@@ -335,6 +438,7 @@ void rate_control_get_rate(struct ieee80211_sub_if_data *sdata,
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(txrc->skb);
 	int i;
 	u32 mask;
+	u8 mcs_mask[IEEE80211_HT_MCS_MASK_LEN];
 
 	if (sta) {
 		ista = &sta->sta;
@@ -358,10 +462,14 @@ void rate_control_get_rate(struct ieee80211_sub_if_data *sdata,
 	 * the common case.
 	 */
 	mask = sdata->rc_rateidx_mask[info->band];
+	memcpy(mcs_mask, sdata->rc_rateidx_mcs_mask[info->band],
+	       sizeof(mcs_mask));
 	if (mask != (1 << txrc->sband->n_bitrates) - 1) {
 		if (sta) {
 			/* Filter out rates that the STA does not support */
 			mask &= sta->sta.supp_rates[info->band];
+			for (i = 0; i < sizeof(mcs_mask); i++)
+				mcs_mask[i] &= sta->sta.ht_cap.mcs.rx_mask[i];
 		}
 		/*
 		 * Make sure the rate index selected for each TX rate is
@@ -372,11 +480,8 @@ void rate_control_get_rate(struct ieee80211_sub_if_data *sdata,
 			/* Skip invalid rates */
 			if (info->control.rates[i].idx < 0)
 				break;
-			/* Rate masking supports only legacy rates for now */
-			if (info->control.rates[i].flags & IEEE80211_TX_RC_MCS)
-				continue;
-			rate_idx_match_mask(&info->control.rates[i],
-					    txrc->sband->n_bitrates, mask);
+			rate_idx_match_mask(&info->control.rates[i], txrc,
+					    mask, mcs_mask);
 		}
 	}
 
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index e74652d..5d8210c 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -639,6 +639,9 @@ ieee80211_tx_h_rate_ctrl(struct ieee80211_tx_data *tx)
 		txrc.max_rate_idx = -1;
 	else
 		txrc.max_rate_idx = fls(txrc.rate_idx_mask) - 1;
+	memcpy(txrc.rate_idx_mcs_mask,
+	       tx->sdata->rc_rateidx_mcs_mask[tx->channel->band],
+	       sizeof(txrc.rate_idx_mcs_mask));
 	txrc.bss = (tx->sdata->vif.type == NL80211_IFTYPE_AP ||
 		    tx->sdata->vif.type == NL80211_IFTYPE_MESH_POINT ||
 		    tx->sdata->vif.type == NL80211_IFTYPE_ADHOC);
@@ -2443,6 +2446,8 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
 		txrc.max_rate_idx = -1;
 	else
 		txrc.max_rate_idx = fls(txrc.rate_idx_mask) - 1;
+	memcpy(txrc.rate_idx_mcs_mask, sdata->rc_rateidx_mcs_mask[band],
+	       sizeof(txrc.rate_idx_mcs_mask));
 	txrc.bss = true;
 	rate_control_get_rate(sdata, NULL, &txrc);
 
-- 
1.7.7.3

Re: [PATCH 2/2] mac80211: add support for mcs masks

[Johannes Berg]

On Mon, 2011-12-19 at 15:14 +0100, Simon Wunderlich wrote:

> +++ b/net/mac80211/iface.c
> @@ -1180,6 +1180,9 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
>  		sband = local->hw.wiphy->bands[i];
>  		sdata->rc_rateidx_mask[i] =
>  			sband ? (1 << sband->n_bitrates) - 1 : 0;
> +		memset(sdata->rc_rateidx_mcs_mask[i], sband ? 0xff : 0,
> +		       sizeof(sdata->rc_rateidx_mcs_mask[i]));
> +
>  	}

Similar comment as before; there's also a spurious blank line here.

> --- a/net/mac80211/rate.c
> +++ b/net/mac80211/rate.c
> @@ -267,10 +267,10 @@ bool rate_control_send_low(struct ieee80211_sta *sta,
>  
>  	if (!sta || !priv_sta || rc_no_data_or_no_ack_use_min(txrc)) {
>  		if ((sband->band != IEEE80211_BAND_2GHZ) ||
> -		    !(info->flags & IEEE80211_TX_CTL_NO_CCK_RATE))
> +		    !(info->flags & IEEE80211_TX_CTL_NO_CCK_RATE)) {
>  			info->control.rates[0].idx =
>  				rate_lowest_index(txrc->sband, sta);
> -		else
> +		} else
>  			info->control.rates[0].idx =
>  				rate_lowest_non_cck_index(txrc->sband, sta);
>  		info->control.rates[0].count =

Hmm? I see no change here?

> @@ -293,25 +293,128 @@ bool rate_control_send_low(struct ieee80211_sta *sta,
>  }
>  EXPORT_SYMBOL(rate_control_send_low);
>  
> -static void rate_idx_match_mask(struct ieee80211_tx_rate *rate,
> -				int n_bitrates, u32 mask)
> +static bool rate_idx_match_legacy_mask(struct ieee80211_tx_rate *rate,
> +				       int n_bitrates, u32 mask)
>  {
>  	int j;
>  
> -	/* See whether the selected rate or anything below it is allowed. */
> +	/* See whether the selected rate or anything below
> +	 * it is allowed. */

why change the comment format (and do it wrong while at it)?

>  		}
>  	}
> -
>  	/* Try to find a higher rate that would be allowed */

?

> @@ -358,10 +462,14 @@ void rate_control_get_rate(struct ieee80211_sub_if_data *sdata,
>  	 * the common case.
>  	 */
>  	mask = sdata->rc_rateidx_mask[info->band];
> +	memcpy(mcs_mask, sdata->rc_rateidx_mcs_mask[info->band],
> +	       sizeof(mcs_mask));

Do we really have to do this? Might not a pointer be better?

>  	if (mask != (1 << txrc->sband->n_bitrates) - 1) {
>  		if (sta) {
>  			/* Filter out rates that the STA does not support */
>  			mask &= sta->sta.supp_rates[info->band];
> +			for (i = 0; i < sizeof(mcs_mask); i++)
> +				mcs_mask[i] &= sta->sta.ht_cap.mcs.rx_mask[i];

Oh, so it's filtered by station ... hm ok I guess unless we tie
lifetimes together we have to do this.

Maybe we could update all stations in the slow-path (changes in the HT
mask) and then just use the already masked version in the sta entry in
the fastpath here?

johannes

Re: [PATCH 2/2] mac80211: add support for mcs masks

[Simon Wunderlich]

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

Hello Johannes,

thanks for all the comments, I can agree to all points you made in this mail
and for the other one and will provide a PATCHv2 which should fix this.

One issue is left which I'd like to discuss, please see below.

On Tue, Jan 03, 2012 at 03:24:50PM +0100, Johannes Berg wrote:
> > @@ -358,10 +462,14 @@ void rate_control_get_rate(struct ieee80211_sub_if_data *sdata,
> >  	 * the common case.
> >  	 */
> >  	mask = sdata->rc_rateidx_mask[info->band];
> > +	memcpy(mcs_mask, sdata->rc_rateidx_mcs_mask[info->band],
> > +	       sizeof(mcs_mask));
> 
> Do we really have to do this? Might not a pointer be better?
> 
> >  	if (mask != (1 << txrc->sband->n_bitrates) - 1) {
> >  		if (sta) {
> >  			/* Filter out rates that the STA does not support */
> >  			mask &= sta->sta.supp_rates[info->band];
> > +			for (i = 0; i < sizeof(mcs_mask); i++)
> > +				mcs_mask[i] &= sta->sta.ht_cap.mcs.rx_mask[i];
> 
> Oh, so it's filtered by station ... hm ok I guess unless we tie
> lifetimes together we have to do this.
> 
> Maybe we could update all stations in the slow-path (changes in the HT
> mask) and then just use the already masked version in the sta entry in
> the fastpath here?

You are right that this definitly adds some load on the fastpath (Although
I don't know how much the compiler optimizes here anyway, its only 10 bytes
which are set and masked).

Currently I see two approaches here:

1.) move the masking into the station handling code (when a new station joins).
    We would also need to iterate over the connected station when changing
    the mcs_mask while the interface is up.
    --> this will probably need to change at quite some palces and might be race-risky
2.) keep the masking in the rate_control_get_rate() function, but mask only
    once (first time) and keep the result in the station struct. Next time we
    enter it, we use the already masked result. We could then use a flag to
    remember that we already computed the mask and just use it, and another
    flag to signalize if the mcs_mask was modified externally
    --> this is not racy and rather simple, but adds some flags

Maybe you have an even better idea, please let me know what you think. :)

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/2] mac80211: add support for mcs masks

[Johannes Berg]

Hi,

> > Maybe we could update all stations in the slow-path (changes in the HT
> > mask) and then just use the already masked version in the sta entry in
> > the fastpath here?
> 
> You are right that this definitly adds some load on the fastpath (Although
> I don't know how much the compiler optimizes here anyway, its only 10 bytes
> which are set and masked).
> 
> Currently I see two approaches here:
> 
> 1.) move the masking into the station handling code (when a new station joins).
>     We would also need to iterate over the connected station when changing
>     the mcs_mask while the interface is up.
>     --> this will probably need to change at quite some palces and might be race-risky

Yeah this is what I was thinking of, but you could well be right that it
might be quite hard to do.

> 2.) keep the masking in the rate_control_get_rate() function, but mask only
>     once (first time) and keep the result in the station struct. Next time we
>     enter it, we use the already masked result. We could then use a flag to
>     remember that we already computed the mask and just use it, and another
>     flag to signalize if the mcs_mask was modified externally
>     --> this is not racy and rather simple, but adds some flags

I don't really like that, it's just as much complexity really and you
need to still iterate all stations to invalidate the flag if mcs_mask
was changed.

Better just keep it as is unless we find it's a problem.

johannes

[PATCH 1/2] iw: add nl80211 bitrates

[Simon Wunderlich]

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fokus.fraunhofer.de>
---
 nl80211.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/nl80211.h b/nl80211.h
index a187606..445e0e4 100644
--- a/nl80211.h
+++ b/nl80211.h
@@ -2395,12 +2395,15 @@ enum nl80211_key_attributes {
  *	in an array of rates as defined in IEEE 802.11 7.3.2.2 (u8 values with
  *	1 = 500 kbps) but without the IE length restriction (at most
  *	%NL80211_MAX_SUPP_RATES in a single array).
+ * @NL80211_TXRATE_MCS: HT (MCS) rates allowed for TX rate selection
+ *      in an array of MCS numbers.
  * @__NL80211_TXRATE_AFTER_LAST: internal
  * @NL80211_TXRATE_MAX: highest TX rate attribute
  */
 enum nl80211_tx_rate_attributes {
 	__NL80211_TXRATE_INVALID,
 	NL80211_TXRATE_LEGACY,
+	NL80211_TXRATE_MCS,
 
 	/* keep last */
 	__NL80211_TXRATE_AFTER_LAST,
-- 
1.7.7.3

[PATCH 2/2] iw: remove ifdefs for mcs mask

[Simon Wunderlich]

These are enums, not defines. Therefore the ifdef check can never be
true.

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fokus.fraunhofer.de>
---
 bitrate.c |   20 ++------------------
 1 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/bitrate.c b/bitrate.c
index cefd150..22b863e 100644
--- a/bitrate.c
+++ b/bitrate.c
@@ -17,12 +17,10 @@ static int handle_bitrates(struct nl80211_state *state,
 	uint8_t *legacy = NULL;
 	int *n_legacy = NULL;
 	bool have_mcs_24 = false, have_mcs_5 = false;
-#ifdef NL80211_TXRATE_MCS
 	uint8_t mcs_24[77], mcs_5[77];
 	int n_mcs_24 = 0, n_mcs_5 = 0;
 	uint8_t *mcs = NULL;
 	int *n_mcs = NULL;
-#endif
 	enum {
 		S_NONE,
 		S_LEGACY,
@@ -32,9 +30,7 @@ static int handle_bitrates(struct nl80211_state *state,
 	for (i = 0; i < argc; i++) {
 		char *end;
 		double tmpd;
-#ifdef NL80211_TXRATE_MCS
 		long tmpl;
-#endif
 
 		if (strcmp(argv[i], "legacy-2.4") == 0) {
 			if (have_legacy_24)
@@ -51,7 +47,6 @@ static int handle_bitrates(struct nl80211_state *state,
 			n_legacy = &n_legacy_5;
 			have_legacy_5 = true;
 		}
-#ifdef NL80211_TXRATE_MCS
 		else if (strcmp(argv[i], "mcs-2.4") == 0) {
 			if (have_mcs_24)
 				return 1;
@@ -67,7 +62,6 @@ static int handle_bitrates(struct nl80211_state *state,
 			n_mcs = &n_mcs_5;
 			have_mcs_5 = true;
 		}
-#endif
 		else switch (parser_state) {
 		case S_LEGACY:
 			tmpd = strtod(argv[i], &end);
@@ -78,7 +72,6 @@ static int handle_bitrates(struct nl80211_state *state,
 			legacy[(*n_legacy)++] = tmpd * 2;
 			break;
 		case S_MCS:
-#ifdef NL80211_TXRATE_MCS
 			tmpl = strtol(argv[i], &end, 0);
 			if (*end != '\0')
 				return 1;
@@ -86,7 +79,6 @@ static int handle_bitrates(struct nl80211_state *state,
 				return 1;
 			mcs[(*n_mcs)++] = tmpl;
 			break;
-#endif
 		default:
 			return 1;
 		}
@@ -102,10 +94,8 @@ static int handle_bitrates(struct nl80211_state *state,
 			goto nla_put_failure;
 		if (have_legacy_24)
 			nla_put(msg, NL80211_TXRATE_LEGACY, n_legacy_24, legacy_24);
-#ifdef NL80211_TXRATE_MCS
 		if (have_mcs_24)
 			nla_put(msg, NL80211_TXRATE_MCS, n_mcs_24, mcs_24);
-#endif
 		nla_nest_end(msg, nl_band);
 	}
 
@@ -115,10 +105,8 @@ static int handle_bitrates(struct nl80211_state *state,
 			goto nla_put_failure;
 		if (have_legacy_5)
 			nla_put(msg, NL80211_TXRATE_LEGACY, n_legacy_5, legacy_5);
-#ifdef NL80211_TXRATE_MCS
 		if (have_mcs_5)
 			nla_put(msg, NL80211_TXRATE_MCS, n_mcs_5, mcs_5);
-#endif
 		nla_nest_end(msg, nl_band);
 	}
 
@@ -130,13 +118,9 @@ static int handle_bitrates(struct nl80211_state *state,
 }
 
 #define DESCR_LEGACY "[legacy-<2.4|5> <legacy rate in Mbps>*]"
-#ifdef NL80211_TXRATE_MCS
 #define DESCR DESCR_LEGACY " [mcs-<2.4|5> <MCS index>*]"
-#else
-#define DESCR DESCR_LEGACY
-#endif
 
-COMMAND(set, bitrates, DESCR, NL80211_CMD_SET_TX_BITRATE_MASK, 0, CIB_NETDEV,
-	handle_bitrates,
+COMMAND(set, bitrates, "[legacy-<2.4|5> <legacy rate in Mbps>*] [mcs-<2.4|5> <MCS index>*]",
+	NL80211_CMD_SET_TX_BITRATE_MASK, 0, CIB_NETDEV, handle_bitrates,
 	"Sets up the specified rate masks.\n"
 	"Not passing any arguments would clear the existing mask (if any).");
-- 
1.7.7.3