[PATCH 1/4] nl80211: don't clear bitrate_mask twice

[PATCH 1/4] nl80211: don't clear bitrate_mask twice

[Janusz Dziedzic]

Don't clear cfg80211_bitrate_mask twice in
nl80211_set_tx_bitrate_mask() function.

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 net/wireless/nl80211.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index c000449..c6401a8 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7321,15 +7321,14 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
 	/* Default to all rates enabled */
 	for (i = 0; i < IEEE80211_NUM_BANDS; i++) {
 		sband = rdev->wiphy.bands[i];
-		mask.control[i].legacy =
-			sband ? (1 << sband->n_bitrates) - 1 : 0;
-		if (sband)
-			memcpy(mask.control[i].mcs,
-			       sband->ht_cap.mcs.rx_mask,
-			       sizeof(mask.control[i].mcs));
-		else
-			memset(mask.control[i].mcs, 0,
-			       sizeof(mask.control[i].mcs));
+
+		if (!sband)
+			continue;
+
+		mask.control[i].legacy = (1 << sband->n_bitrates) - 1;
+		memcpy(mask.control[i].mcs,
+		       sband->ht_cap.mcs.rx_mask,
+		       sizeof(mask.control[i].mcs));
 	}
 
 	/*
-- 
1.7.9.5

[PATCH 2/4] nl80211: back to default bitrate_mask correctly

[Janusz Dziedzic]

In case of empty NL80211_ATTR_TX_RATES attribute
in nl80211_set_tx_bitrate_mask() function back to
default bitrate mask.

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 net/wireless/nl80211.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index c6401a8..072b60d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7311,9 +7311,6 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
 	struct nlattr *tx_rates;
 	struct ieee80211_supported_band *sband;
 
-	if (info->attrs[NL80211_ATTR_TX_RATES] == NULL)
-		return -EINVAL;
-
 	if (!rdev->ops->set_bitrate_mask)
 		return -EOPNOTSUPP;
 
@@ -7331,6 +7328,10 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
 		       sizeof(mask.control[i].mcs));
 	}
 
+	/* Back to default settings */
+	if (info->attrs[NL80211_ATTR_TX_RATES] == NULL)
+		goto out;
+
 	/*
 	 * The nested attribute uses enum nl80211_band as the index. This maps
 	 * directly to the enum ieee80211_band values used in cfg80211.
@@ -7380,6 +7381,7 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
 		}
 	}
 
+out:
 	return rdev_set_bitrate_mask(rdev, dev, NULL, &mask);
 }
 
-- 
1.7.9.5

Re: [PATCH 2/4] nl80211: back to default bitrate_mask correctly

[Johannes Berg]

On Tue, 2013-12-03 at 09:50 +0100, Janusz Dziedzic wrote:
> In case of empty NL80211_ATTR_TX_RATES attribute
> in nl80211_set_tx_bitrate_mask() function back to
> default bitrate mask.

Err what?

> +++ b/net/wireless/nl80211.c
> @@ -7311,9 +7311,6 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
>  	struct nlattr *tx_rates;
>  	struct ieee80211_supported_band *sband;
>  
> -	if (info->attrs[NL80211_ATTR_TX_RATES] == NULL)
> -		return -EINVAL;

Why?

johannes

Re: [PATCH 2/4] nl80211: back to default bitrate_mask correctly

[Janusz Dziedzic]

On 3 December 2013 15:20, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2013-12-03 at 09:50 +0100, Janusz Dziedzic wrote:
>> In case of empty NL80211_ATTR_TX_RATES attribute
>> in nl80211_set_tx_bitrate_mask() function back to
>> default bitrate mask.
>
> Err what?
>
>> +++ b/net/wireless/nl80211.c
>> @@ -7311,9 +7311,6 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
>>       struct nlattr *tx_rates;
>>       struct ieee80211_supported_band *sband;
>>
>> -     if (info->attrs[NL80211_ATTR_TX_RATES] == NULL)
>> -             return -EINVAL;
>
> Why?
In case we will execute:
iw wlanX set bitrate
with this patch we will back to default - defined by capabilities
(driver) startup values for both legacy and ht (and VHT with last
patch).

Without this patch I don't see an option how to do that from iw. Or I
miss something

BR
Janusz

Re: [PATCH 2/4] nl80211: back to default bitrate_mask correctly

[Johannes Berg]

On Tue, 2013-12-03 at 17:00 +0100, Janusz Dziedzic wrote:
> On 3 December 2013 15:20, Johannes Berg <johannes@sipsolutions.net> wrote:
> > On Tue, 2013-12-03 at 09:50 +0100, Janusz Dziedzic wrote:
> >> In case of empty NL80211_ATTR_TX_RATES attribute
> >> in nl80211_set_tx_bitrate_mask() function back to
> >> default bitrate mask.

> > Why?
> In case we will execute:
> iw wlanX set bitrate
> with this patch we will back to default - defined by capabilities
> (driver) startup values for both legacy and ht (and VHT with last
> patch).

Maybe add something like that to the commit log - just a single line
would be ok (e.g. "back to the default bitrate mask, to be able to reset
the state") or so?

> Without this patch I don't see an option how to do that from iw. Or I
> miss something

You're right - how did I ever take this code into the tree? Ugh. It
seems it should be reset more often, but I guess we're stuck with it.

johannes

[PATCH 3/4] nl80211/cfg80211: bitrate_mask, rename mcs to ht_mcs

[Janusz Dziedzic]

Rename NL80211_TXRATE_MCS into NL80211_TXRATE_HT_MCS.
Rename mcs to ht_mcs in cfg80211_bitrate_mask structure.
This is cleanup before we will introduce VHT_MCS.

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 drivers/net/wireless/ath/ath6kl/wmi.c   |    6 +++---
 drivers/net/wireless/mwifiex/cfg80211.c |    6 +++---
 include/net/cfg80211.h                  |    2 +-
 include/uapi/linux/nl80211.h            |    6 ++++--
 net/mac80211/cfg.c                      |    4 ++--
 net/wireless/nl80211.c                  |   18 +++++++++---------
 6 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index 546d5da..4f16d79 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -2754,9 +2754,9 @@ static int ath6kl_set_bitrate_mask64(struct wmi *wmi, u8 if_idx,
 				mask->control[band].legacy << 4;
 
 		/* copy mcs rate mask */
-		mcsrate = mask->control[band].mcs[1];
+		mcsrate = mask->control[band].ht_mcs[1];
 		mcsrate <<= 8;
-		mcsrate |= mask->control[band].mcs[0];
+		mcsrate |= mask->control[band].ht_mcs[0];
 		ratemask[band] |= mcsrate << 12;
 		ratemask[band] |= mcsrate << 28;
 	}
@@ -2806,7 +2806,7 @@ static int ath6kl_set_bitrate_mask32(struct wmi *wmi, u8 if_idx,
 				mask->control[band].legacy << 4;
 
 		/* copy mcs rate mask */
-		mcsrate = mask->control[band].mcs[0];
+		mcsrate = mask->control[band].ht_mcs[0];
 		ratemask[band] |= mcsrate << 12;
 		ratemask[band] |= mcsrate << 20;
 	}
diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c
index fbad00a..27033ab 100644
--- a/drivers/net/wireless/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/mwifiex/cfg80211.c
@@ -1170,10 +1170,10 @@ static int mwifiex_cfg80211_set_bitrate_mask(struct wiphy *wiphy,
 	else
 		bitmap_rates[1] = mask->control[band].legacy;
 
-	/* Fill MCS rates */
-	bitmap_rates[2] = mask->control[band].mcs[0];
+	/* Fill HT MCS rates */
+	bitmap_rates[2] = mask->control[band].ht_mcs[0];
 	if (priv->adapter->hw_dev_mcs_support == HT_STREAM_2X2)
-		bitmap_rates[2] |= mask->control[band].mcs[1] << 8;
+		bitmap_rates[2] |= mask->control[band].ht_mcs[1] << 8;
 
 	return mwifiex_send_cmd_sync(priv, HostCmd_CMD_TX_RATE_CFG,
 				     HostCmd_ACT_GEN_SET, 0, bitmap_rates);
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index f07840e..626087b 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1763,7 +1763,7 @@ enum wiphy_params_flags {
 struct cfg80211_bitrate_mask {
 	struct {
 		u32 legacy;
-		u8 mcs[IEEE80211_HT_MCS_MASK_LEN];
+		u8 ht_mcs[IEEE80211_HT_MCS_MASK_LEN];
 	} control[IEEE80211_NUM_BANDS];
 };
 /**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index f752e98..4d89492 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -3058,7 +3058,7 @@ 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
+ * @NL80211_TXRATE_HT_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
@@ -3066,13 +3066,15 @@ enum nl80211_key_attributes {
 enum nl80211_tx_rate_attributes {
 	__NL80211_TXRATE_INVALID,
 	NL80211_TXRATE_LEGACY,
-	NL80211_TXRATE_MCS,
+	NL80211_TXRATE_HT_MCS,
 
 	/* keep last */
 	__NL80211_TXRATE_AFTER_LAST,
 	NL80211_TXRATE_MAX = __NL80211_TXRATE_AFTER_LAST - 1
 };
 
+#define NL80211_TXRATE_MCS NL80211_TXRATE_HT_MCS
+
 /**
  * enum nl80211_band - Frequency band
  * @NL80211_BAND_2GHZ: 2.4 GHz ISM band
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 5232b01..57d25a0 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2565,8 +2565,8 @@ static int ieee80211_set_bitrate_mask(struct wiphy *wiphy,
 		int j;
 
 		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));
+		memcpy(sdata->rc_rateidx_mcs_mask[i], mask->control[i].ht_mcs,
+		       sizeof(mask->control[i].ht_mcs));
 
 		sdata->rc_has_mcs_mask[i] = false;
 		if (!sband)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 072b60d..ff65172 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7296,8 +7296,8 @@ static bool ht_rateset_to_mask(struct ieee80211_supported_band *sband,
 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 },
+	[NL80211_TXRATE_HT_MCS] = { .type = NLA_BINARY,
+				    .len = NL80211_MAX_SUPP_HT_RATES },
 };
 
 static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
@@ -7323,9 +7323,9 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
 			continue;
 
 		mask.control[i].legacy = (1 << sband->n_bitrates) - 1;
-		memcpy(mask.control[i].mcs,
+		memcpy(mask.control[i].ht_mcs,
 		       sband->ht_cap.mcs.rx_mask,
-		       sizeof(mask.control[i].mcs));
+		       sizeof(mask.control[i].ht_mcs));
 	}
 
 	/* Back to default settings */
@@ -7356,12 +7356,12 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
 			    nla_len(tb[NL80211_TXRATE_LEGACY]))
 				return -EINVAL;
 		}
-		if (tb[NL80211_TXRATE_MCS]) {
+		if (tb[NL80211_TXRATE_HT_MCS]) {
 			if (!ht_rateset_to_mask(
 					sband,
-					nla_data(tb[NL80211_TXRATE_MCS]),
-					nla_len(tb[NL80211_TXRATE_MCS]),
-					mask.control[band].mcs))
+					nla_data(tb[NL80211_TXRATE_HT_MCS]),
+					nla_len(tb[NL80211_TXRATE_HT_MCS]),
+					mask.control[band].ht_mcs))
 				return -EINVAL;
 		}
 
@@ -7372,7 +7372,7 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
 				return -EINVAL;
 
 			for (i = 0; i < IEEE80211_HT_MCS_MASK_LEN; i++)
-				if (mask.control[band].mcs[i])
+				if (mask.control[band].ht_mcs[i])
 					break;
 
 			/* legacy and mcs rates may not be both empty */
-- 
1.7.9.5

[PATCH 4/4] nl80211: add VHT support for set_bitrate_mask

[Janusz Dziedzic]

Add VHT mcs support for nl80211_set_tx_bitrate_mask().

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 include/linux/ieee80211.h    |    1 +
 include/net/cfg80211.h       |    1 +
 include/uapi/linux/nl80211.h |    4 ++
 net/wireless/nl80211.c       |   93 +++++++++++++++++++++++++++++++++++++++---
 4 files changed, 94 insertions(+), 5 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 8c3b26a..3c92dfd 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1124,6 +1124,7 @@ struct ieee80211_bar {
 #define IEEE80211_BAR_CTRL_TID_INFO_MASK	0xf000
 #define IEEE80211_BAR_CTRL_TID_INFO_SHIFT	12
 
+#define IEEE80211_VHT_NSS_MAX			8
 #define IEEE80211_HT_MCS_MASK_LEN		10
 
 /**
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 626087b..ffdf771 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1764,6 +1764,7 @@ struct cfg80211_bitrate_mask {
 	struct {
 		u32 legacy;
 		u8 ht_mcs[IEEE80211_HT_MCS_MASK_LEN];
+		u16 vht_mcs[IEEE80211_VHT_NSS_MAX];
 	} control[IEEE80211_NUM_BANDS];
 };
 /**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 4d89492..5b7ed5d 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1862,6 +1862,7 @@ enum nl80211_attrs {
 
 #define NL80211_MAX_SUPP_RATES			32
 #define NL80211_MAX_SUPP_HT_RATES		77
+#define NL80211_MAX_SUPP_VHT_RATES		80
 #define NL80211_MAX_SUPP_REG_RULES		32
 #define NL80211_TKIP_DATA_OFFSET_ENCR_KEY	0
 #define NL80211_TKIP_DATA_OFFSET_TX_MIC_KEY	16
@@ -3060,6 +3061,8 @@ enum nl80211_key_attributes {
  *	%NL80211_MAX_SUPP_RATES in a single array).
  * @NL80211_TXRATE_HT_MCS: HT (MCS) rates allowed for TX rate selection
  *	in an array of MCS numbers.
+ * @NL80211_TXRATE_VHT_MCS: VHT (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
  */
@@ -3067,6 +3070,7 @@ enum nl80211_tx_rate_attributes {
 	__NL80211_TXRATE_INVALID,
 	NL80211_TXRATE_LEGACY,
 	NL80211_TXRATE_HT_MCS,
+	NL80211_TXRATE_VHT_MCS,
 
 	/* keep last */
 	__NL80211_TXRATE_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index ff65172..3ab794e 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7293,11 +7293,74 @@ static bool ht_rateset_to_mask(struct ieee80211_supported_band *sband,
 	return true;
 }
 
+static void vht_mcs_map_to_mcs_mask(u16 vht_mcs_map,
+				    u16 vht_mcs_mask[IEEE80211_VHT_NSS_MAX])
+{
+	u8 nss;
+
+	for (nss = 0; nss < IEEE80211_VHT_NSS_MAX; nss++) {
+		switch (vht_mcs_map & 0x03) {
+		case IEEE80211_VHT_MCS_NOT_SUPPORTED:
+			vht_mcs_mask[nss] = 0;
+			break;
+		case IEEE80211_VHT_MCS_SUPPORT_0_7:
+			vht_mcs_mask[nss] = 0x00FF;
+			break;
+		case IEEE80211_VHT_MCS_SUPPORT_0_8:
+			vht_mcs_mask[nss] = 0x01FF;
+			break;
+		case IEEE80211_VHT_MCS_SUPPORT_0_9:
+			vht_mcs_mask[nss] = 0x03FF;
+			break;
+		}
+
+		vht_mcs_map >>= 2;
+	}
+}
+
+static bool vht_rateset_to_mask(struct ieee80211_supported_band *sband,
+				u8 *rates, u8 rates_len,
+				u16 mcs[IEEE80211_VHT_NSS_MAX])
+{
+	u16 tx_mcs_map = le16_to_cpu(sband->vht_cap.vht_mcs.tx_mcs_map);
+	u16 tx_mcs_mask[IEEE80211_VHT_NSS_MAX] = {};
+	u8 i;
+
+	if (!sband->vht_cap.vht_supported)
+		return false;
+
+	memset(mcs, 0, sizeof(mcs));
+
+	/* Build vht_mcs_mask from VHT capabilities */
+	vht_mcs_map_to_mcs_mask(tx_mcs_map, tx_mcs_mask);
+
+	for (i = 0; i < rates_len; i++) {
+		int ridx, rbit;
+
+		ridx = rates[i] / 10;
+		rbit = BIT(rates[i] % 10);
+
+		/* check validity */
+		if ((ridx < 0) || (ridx >= IEEE80211_VHT_NSS_MAX))
+			return false;
+
+		/* check availability */
+		if (!(tx_mcs_mask[ridx] & rbit))
+			return false;
+
+		mcs[ridx] |= rbit;
+	}
+
+	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_HT_MCS] = { .type = NLA_BINARY,
 				    .len = NL80211_MAX_SUPP_HT_RATES },
+	[NL80211_TXRATE_VHT_MCS] = { .type = NLA_BINARY,
+				     .len = NL80211_MAX_SUPP_VHT_RATES},
 };
 
 static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
@@ -7310,6 +7373,7 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
 	struct net_device *dev = info->user_ptr[1];
 	struct nlattr *tx_rates;
 	struct ieee80211_supported_band *sband;
+	u16 vht_tx_mcs_map;
 
 	if (!rdev->ops->set_bitrate_mask)
 		return -EOPNOTSUPP;
@@ -7326,6 +7390,13 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
 		memcpy(mask.control[i].ht_mcs,
 		       sband->ht_cap.mcs.rx_mask,
 		       sizeof(mask.control[i].ht_mcs));
+
+		if (!sband->vht_cap.vht_supported)
+			continue;
+
+		vht_tx_mcs_map = le16_to_cpu(sband->vht_cap.vht_mcs.tx_mcs_map);
+		vht_mcs_map_to_mcs_mask(vht_tx_mcs_map,
+					mask.control[i].vht_mcs);
 	}
 
 	/* Back to default settings */
@@ -7364,20 +7435,32 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
 					mask.control[band].ht_mcs))
 				return -EINVAL;
 		}
+		if (tb[NL80211_TXRATE_VHT_MCS]) {
+			if (!vht_rateset_to_mask(
+					sband,
+					nla_data(tb[NL80211_TXRATE_VHT_MCS]),
+					nla_len(tb[NL80211_TXRATE_VHT_MCS]),
+					mask.control[band].vht_mcs))
+				return -EINVAL;
+		}
 
 		if (mask.control[band].legacy == 0) {
-			/* don't allow empty legacy rates if HT
+			/* don't allow empty legacy rates if HT and VHT
 			 * is not even supported. */
-			if (!rdev->wiphy.bands[band]->ht_cap.ht_supported)
+			if (!(rdev->wiphy.bands[band]->ht_cap.ht_supported ||
+			      rdev->wiphy.bands[band]->vht_cap.vht_supported))
 				return -EINVAL;
 
 			for (i = 0; i < IEEE80211_HT_MCS_MASK_LEN; i++)
 				if (mask.control[band].ht_mcs[i])
-					break;
+					goto out;
+
+			for (i = 0; i < IEEE80211_VHT_NSS_MAX; i++)
+				if (mask.control[band].vht_mcs[i])
+					goto out;
 
 			/* legacy and mcs rates may not be both empty */
-			if (i == IEEE80211_HT_MCS_MASK_LEN)
-				return -EINVAL;
+			return -EINVAL;
 		}
 	}
 
-- 
1.7.9.5

Re: [PATCH 4/4] nl80211: add VHT support for set_bitrate_mask

[Johannes Berg]

On Tue, 2013-12-03 at 09:50 +0100, Janusz Dziedzic wrote:
> Add VHT mcs support for nl80211_set_tx_bitrate_mask().

Do we really need more fine-grained control than 0-7/8/9? Why do we need
this at all? :)

> +#define NL80211_MAX_SUPP_VHT_RATES		80

Where does 80 come from? That seems odd?

> + * @NL80211_TXRATE_VHT_MCS: VHT (MCS) rates allowed for TX rate selection
> + *	in an array of MCS numbers.

VHT has no "MCS numbers", so this doesn't make much sense?

>  		if (mask.control[band].legacy == 0) {
> -			/* don't allow empty legacy rates if HT
> +			/* don't allow empty legacy rates if HT and VHT
>  			 * is not even supported. */

probably should say "are not" now :)

johannes

Re: [PATCH 4/4] nl80211: add VHT support for set_bitrate_mask

[Janusz Dziedzic]

On 3 December 2013 15:24, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2013-12-03 at 09:50 +0100, Janusz Dziedzic wrote:
>> Add VHT mcs support for nl80211_set_tx_bitrate_mask().
>
> Do we really need more fine-grained control than 0-7/8/9? Why do we need
> this at all? :)
>
We need this for test purpose. Need to test different MCS/NSS
configurations using our HW (mainly for VHT).
Our HW supports setting TX fixed_rate for legacy/HT/VHT.
So, we decided to use set_tx_bitrate_mask() as the best interface for
that in nl80211.
And after that we realize VHT implementation/change will be needed in
nl80211/iw also.
Because of that I send this VHT patch.
So, we need this :)

>> +#define NL80211_MAX_SUPP_VHT_RATES           80
>
> Where does 80 come from? That seems odd?
>
NSS_MAX (8) * MCS_MAX (10)

Eg.
9 - NSS=1, MCS=9
10 - NSS=2, MCS=0
11 - NSS=2, MCS=1
...

>> + * @NL80211_TXRATE_VHT_MCS: VHT (MCS) rates allowed for TX rate selection
>> + *   in an array of MCS numbers.
>
> VHT has no "MCS numbers", so this doesn't make much sense?
>
You mean description only or mcs mask for vht (like above)?

>>               if (mask.control[band].legacy == 0) {
>> -                     /* don't allow empty legacy rates if HT
>> +                     /* don't allow empty legacy rates if HT and VHT
>>                        * is not even supported. */
>
> probably should say "are not" now :)
>
yes you right :)

> johannes
>

Re: [PATCH 4/4] nl80211: add VHT support for set_bitrate_mask

[Johannes Berg]

On Tue, 2013-12-03 at 17:21 +0100, Janusz Dziedzic wrote:

> We need this for test purpose. Need to test different MCS/NSS
> configurations using our HW (mainly for VHT).
> Our HW supports setting TX fixed_rate for legacy/HT/VHT.
> So, we decided to use set_tx_bitrate_mask() as the best interface for
> that in nl80211.
> And after that we realize VHT implementation/change will be needed in
> nl80211/iw also.
> Because of that I send this VHT patch.
> So, we need this :)

Fair enough - please add a line or two to the commit log.

> >> +#define NL80211_MAX_SUPP_VHT_RATES           80
> >
> > Where does 80 come from? That seems odd?
> >
> NSS_MAX (8) * MCS_MAX (10)
> 
> Eg.
> 9 - NSS=1, MCS=9
> 10 - NSS=2, MCS=0
> 11 - NSS=2, MCS=1
> ...

Can we please find a better way to encode this?

> >> + * @NL80211_TXRATE_VHT_MCS: VHT (MCS) rates allowed for TX rate selection
> >> + *   in an array of MCS numbers.
> >
> > VHT has no "MCS numbers", so this doesn't make much sense?
> >
> You mean description only or mcs mask for vht (like above)?

Well, for VHT, there's no single "MCS" identifier for a rate, rates are
identified using MCS (0-9) and NSS (1-8). In mac80211, we do some bit
tricks to fit them into a single byte, but I see no reason to do this in
the nl80211 API and would rather have an API that makes this obvious.
The easiest would be using a struct, but then you don't need 80 bytes,
only 8 bitmaps (one for each NSS), for example.

johannes

Re: [PATCH 4/4] nl80211: add VHT support for set_bitrate_mask

[Janusz Dziedzic]

2013/12/4 Johannes Berg <johannes@sipsolutions.net>:
> On Tue, 2013-12-03 at 17:21 +0100, Janusz Dziedzic wrote:
>
>> We need this for test purpose. Need to test different MCS/NSS
>> configurations using our HW (mainly for VHT).
>> Our HW supports setting TX fixed_rate for legacy/HT/VHT.
>> So, we decided to use set_tx_bitrate_mask() as the best interface for
>> that in nl80211.
>> And after that we realize VHT implementation/change will be needed in
>> nl80211/iw also.
>> Because of that I send this VHT patch.
>> So, we need this :)
>
> Fair enough - please add a line or two to the commit log.
>
>> >> +#define NL80211_MAX_SUPP_VHT_RATES           80
>> >
>> > Where does 80 come from? That seems odd?
>> >
>> NSS_MAX (8) * MCS_MAX (10)
>>
>> Eg.
>> 9 - NSS=1, MCS=9
>> 10 - NSS=2, MCS=0
>> 11 - NSS=2, MCS=1
>> ...
>
> Can we please find a better way to encode this?
>
>> >> + * @NL80211_TXRATE_VHT_MCS: VHT (MCS) rates allowed for TX rate selection
>> >> + *   in an array of MCS numbers.
>> >
>> > VHT has no "MCS numbers", so this doesn't make much sense?
>> >
>> You mean description only or mcs mask for vht (like above)?
>
> Well, for VHT, there's no single "MCS" identifier for a rate, rates are
> identified using MCS (0-9) and NSS (1-8). In mac80211, we do some bit
> tricks to fit them into a single byte, but I see no reason to do this in
> the nl80211 API and would rather have an API that makes this obvious.
> The easiest would be using a struct, but then you don't need 80 bytes,
> only 8 bitmaps (one for each NSS), for example.
>
Yes, and I added 8 bitmaps:

+ u16 vht_mcs[IEEE80211_VHT_NSS_MAX];

80 bytes is because of format we will from user mode (80 available
combinations for VHT mask).
I decided for:
>> 9 - NSS=1, MCS=9
>> 10 - NSS=2, MCS=0
>> 11 - NSS=2, MCS=1
To be compatible with ht-mcs set.
You mean better to send 8*u16 from user mode and do conversion in IW
(currently we send 80 bytes and do conversion in nl80211, same as for
HT)?

BR
Janusz

Re: [PATCH 4/4] nl80211: add VHT support for set_bitrate_mask

[Johannes Berg]

On Wed, 2013-12-04 at 10:08 +0100, Janusz Dziedzic wrote:

> Yes, and I added 8 bitmaps:
> 
> + u16 vht_mcs[IEEE80211_VHT_NSS_MAX];

But that was the internal API, inside the kernel.

> 80 bytes is because of format we will from user mode (80 available
> combinations for VHT mask).
> I decided for:
> >> 9 - NSS=1, MCS=9
> >> 10 - NSS=2, MCS=0
> >> 11 - NSS=2, MCS=1
> To be compatible with ht-mcs set.
> You mean better to send 8*u16 from user mode and do conversion in IW
> (currently we send 80 bytes and do conversion in nl80211, same as for
> HT)?

But HT MCS is specified that way, there are 77 MCSes in HT, which each
have implicit NSS etc. That's how the standard works for HT. For VHT,
the standard works completely differently.

So yes, I think sending 8 bitmaps from userspace would be much easier to
understand.

johannes

[PATCH] iw: add VHT MCS set support to set bitrates

[Janusz Dziedzic]

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 bitrate.c |   71 +++++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 46 insertions(+), 25 deletions(-)

diff --git a/bitrate.c b/bitrate.c
index 4da246f..2af75d5 100644
--- a/bitrate.c
+++ b/bitrate.c
@@ -17,15 +17,17 @@ static int handle_bitrates(struct nl80211_state *state,
 	int n_legacy_24 = 0, n_legacy_5 = 0;
 	uint8_t *legacy = NULL;
 	int *n_legacy = NULL;
-	bool have_mcs_24 = false, have_mcs_5 = false;
-	uint8_t mcs_24[77], mcs_5[77];
-	int n_mcs_24 = 0, n_mcs_5 = 0;
+	bool have_ht_mcs_24 = false, have_ht_mcs_5 = false;
+	bool have_vht_mcs_24 = false, have_vht_mcs_5 = false;
+	uint8_t ht_mcs_24[77], ht_mcs_5[77], vht_mcs_24[80], vht_mcs_5[80];
+	int n_ht_mcs_24 = 0, n_ht_mcs_5 = 0, n_vht_mcs_24 = 0, n_vht_mcs_5 = 0;
 	uint8_t *mcs = NULL;
 	int *n_mcs = NULL;
 	enum {
 		S_NONE,
 		S_LEGACY,
-		S_MCS,
+		S_HT_MCS,
+		S_VHT_MCS,
 	} parser_state = S_NONE;
 
 	for (i = 0; i < argc; i++) {
@@ -48,20 +50,34 @@ static int handle_bitrates(struct nl80211_state *state,
 			n_legacy = &n_legacy_5;
 			have_legacy_5 = true;
 		}
-		else if (strcmp(argv[i], "mcs-2.4") == 0) {
-			if (have_mcs_24)
+		else if (strcmp(argv[i], "ht-mcs-2.4") == 0) {
+			if (have_ht_mcs_24)
 				return 1;
-			parser_state = S_MCS;
-			mcs = mcs_24;
-			n_mcs = &n_mcs_24;
-			have_mcs_24 = true;
-		} else if (strcmp(argv[i], "mcs-5") == 0) {
-			if (have_mcs_5)
+			parser_state = S_HT_MCS;
+			mcs = ht_mcs_24;
+			n_mcs = &n_ht_mcs_24;
+			have_ht_mcs_24 = true;
+		} else if (strcmp(argv[i], "ht-mcs-5") == 0) {
+			if (have_ht_mcs_5)
 				return 1;
-			parser_state = S_MCS;
-			mcs = mcs_5;
-			n_mcs = &n_mcs_5;
-			have_mcs_5 = true;
+			parser_state = S_HT_MCS;
+			mcs = ht_mcs_5;
+			n_mcs = &n_ht_mcs_5;
+			have_ht_mcs_5 = true;
+		} else if (strcmp(argv[i], "vht-mcs-2.4") == 0) {
+			if (have_vht_mcs_24)
+				return 1;
+			parser_state = S_VHT_MCS;
+			mcs = vht_mcs_24;
+			n_mcs = &n_vht_mcs_24;
+			have_vht_mcs_24 = true;
+		} else if (strcmp(argv[i], "vht-mcs-5") == 0) {
+			if (have_vht_mcs_5)
+				return 1;
+			parser_state = S_VHT_MCS;
+			mcs = vht_mcs_5;
+			n_mcs = &n_vht_mcs_5;
+			have_vht_mcs_5 = true;
 		}
 		else switch (parser_state) {
 		case S_LEGACY:
@@ -72,7 +88,8 @@ static int handle_bitrates(struct nl80211_state *state,
 				return 1;
 			legacy[(*n_legacy)++] = tmpd * 2;
 			break;
-		case S_MCS:
+		case S_HT_MCS:
+		case S_VHT_MCS:
 			tmpl = strtol(argv[i], &end, 0);
 			if (*end != '\0')
 				return 1;
@@ -89,25 +106,29 @@ static int handle_bitrates(struct nl80211_state *state,
 	if (!nl_rates)
 		goto nla_put_failure;
 
-	if (have_legacy_24 || have_mcs_24) {
+	if (have_legacy_24 || have_ht_mcs_24 || have_vht_mcs_24) {
 		nl_band = nla_nest_start(msg, NL80211_BAND_2GHZ);
 		if (!nl_band)
 			goto nla_put_failure;
 		if (have_legacy_24)
 			nla_put(msg, NL80211_TXRATE_LEGACY, n_legacy_24, legacy_24);
-		if (have_mcs_24)
-			nla_put(msg, NL80211_TXRATE_MCS, n_mcs_24, mcs_24);
+		if (have_ht_mcs_24)
+			nla_put(msg, NL80211_TXRATE_HT_MCS, n_ht_mcs_24, ht_mcs_24);
+		if (have_vht_mcs_24)
+			nla_put(msg, NL80211_TXRATE_VHT_MCS, n_vht_mcs_24, vht_mcs_24);
 		nla_nest_end(msg, nl_band);
 	}
 
-	if (have_legacy_5 || have_mcs_5) {
+	if (have_legacy_5 || have_ht_mcs_5 || have_vht_mcs_5) {
 		nl_band = nla_nest_start(msg, NL80211_BAND_5GHZ);
 		if (!nl_band)
 			goto nla_put_failure;
 		if (have_legacy_5)
 			nla_put(msg, NL80211_TXRATE_LEGACY, n_legacy_5, legacy_5);
-		if (have_mcs_5)
-			nla_put(msg, NL80211_TXRATE_MCS, n_mcs_5, mcs_5);
+		if (have_ht_mcs_5)
+			nla_put(msg, NL80211_TXRATE_HT_MCS, n_ht_mcs_5, ht_mcs_5);
+		if (have_vht_mcs_5)
+			nla_put(msg, NL80211_TXRATE_VHT_MCS, n_vht_mcs_5, vht_mcs_5);
 		nla_nest_end(msg, nl_band);
 	}
 
@@ -119,9 +140,9 @@ static int handle_bitrates(struct nl80211_state *state,
 }
 
 #define DESCR_LEGACY "[legacy-<2.4|5> <legacy rate in Mbps>*]"
-#define DESCR DESCR_LEGACY " [mcs-<2.4|5> <MCS index>*]"
+#define DESCR DESCR_LEGACY " [ht-mcs-<2.4|5> <MCS index>*] [vht-mcs-<2.4|5> <MCS index>*]"
 
-COMMAND(set, bitrates, "[legacy-<2.4|5> <legacy rate in Mbps>*] [mcs-<2.4|5> <MCS index>*]",
+COMMAND(set, bitrates, "[legacy-<2.4|5> <legacy rate in Mbps>*] [ht-mcs-<2.4|5> <MCS index>*] [vht-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.9.5

Re: [PATCH 1/4] nl80211: don't clear bitrate_mask twice

[Johannes Berg]

On Tue, 2013-12-03 at 09:50 +0100, Janusz Dziedzic wrote:
> Don't clear cfg80211_bitrate_mask twice in
> nl80211_set_tx_bitrate_mask() function.

Applied.

johannes