[wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[greearb]

From: Ben Greear <greearb@candelatech.com>

This allows a user to configure a wifi station interface
to disable 802.11n, even if the AP and NIC supports it.

The additional netlink bits is to allow this patch to work on 3.0
and should not be included in the final patch.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 8049bf7... d9c2cdb... M	include/linux/nl80211.h
:100644 100644 92cf1c2... 1331a5b... M	include/net/cfg80211.h
:100644 100644 e253afa... 8221a3a... M	net/mac80211/cfg.c
:100644 100644 4c3d1f5... 72c6726... M	net/mac80211/ieee80211_i.h
:100644 100644 0e5d8da... 393b480... M	net/mac80211/mlme.c
:100644 100644 48260c2... cb2eb67... M	net/wireless/nl80211.c
 include/linux/nl80211.h    |    4 ++++
 include/net/cfg80211.h     |    4 +++-
 net/mac80211/cfg.c         |    3 +++
 net/mac80211/ieee80211_i.h |    2 ++
 net/mac80211/mlme.c        |    3 +++
 net/wireless/nl80211.c     |    7 +++++++
 6 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 8049bf7..d9c2cdb 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1109,6 +1109,9 @@ enum nl80211_commands {
  *	%NL80211_CMD_TDLS_MGMT. Otherwise %NL80211_CMD_TDLS_OPER should be
  *	used for asking the driver to perform a TDLS operation.
  *
+ * @NL80211_ATTR_DISABLE_80211N:  Force /n capable stations to instead
+ *      function as /a/b/g stations.
+ *
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
  */
@@ -1337,6 +1340,7 @@ enum nl80211_attrs {
 	NL80211_ATTR_TDLS_SUPPORT,
 	NL80211_ATTR_TDLS_EXTERNAL_SETUP,
 
+	NL80211_ATTR_DISABLE_11N,
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 92cf1c2..1331a5b 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -254,9 +254,11 @@ struct ieee80211_supported_band {
 /**
  * struct vif_params - describes virtual interface parameters
  * @use_4addr: use 4-address frames
+ * @disable_11n:  Don't use 11n features (HT, etc)
  */
 struct vif_params {
-       int use_4addr;
+	int use_4addr;
+	int disable_11n;
 };
 
 /**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index e253afa..8221a3a 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -57,6 +57,9 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	int ret;
 
+	if (params->disable_11n != -1)
+		sdata->cfg_disable_11n = params->disable_11n;
+
 	ret = ieee80211_if_change_type(sdata, type);
 	if (ret)
 		return ret;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 4c3d1f5..72c6726 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -595,6 +595,8 @@ struct ieee80211_sub_if_data {
 	/* to detect idle changes */
 	bool old_idle;
 
+	bool cfg_disable_11n; /* configured to disable 11n? */
+
 	/* Fragment table for host-based reassembly */
 	struct ieee80211_fragment_entry	fragments[IEEE80211_FRAGMENT_MAX];
 	unsigned int fragment_next;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 0e5d8da..393b480 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2597,6 +2597,9 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
 			ifmgd->flags |= IEEE80211_STA_DISABLE_11N;
 
 
+	if (sdata->cfg_disable_11n)
+		ifmgd->flags |= IEEE80211_STA_DISABLE_11N;
+
 	if (req->ie && req->ie_len) {
 		memcpy(wk->ie, req->ie, req->ie_len);
 		wk->ie_len = req->ie_len;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 48260c2..cb2eb67 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1641,6 +1641,13 @@ static int nl80211_set_interface(struct sk_buff *skb, struct genl_info *info)
 		change = true;
 	}
 
+	if (info->attrs[NL80211_ATTR_DISABLE_11N]) {
+		params.disable_11n = !!nla_get_u8(info->attrs[NL80211_ATTR_DISABLE_11N]);
+		change = true;
+	} else {
+		params.disable_11n = -1;
+	}
+
 	if (change)
 		err = cfg80211_change_iface(rdev, dev, ntype, flags, &params);
 	else
-- 
1.7.3.4

[wireless-next PATCH 2/5] wifi: Support disabling ht40.

[greearb]

From: Ben Greear <greearb@candelatech.com>

Allows users to forceably disable HT40 support in station
interfaces.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 d9c2cdb... ae50ade... M	include/linux/nl80211.h
:100644 100644 1331a5b... 9d7a5e0... M	include/net/cfg80211.h
:100644 100644 8221a3a... c63d7f0... M	net/mac80211/cfg.c
:100644 100644 72c6726... f4a7618... M	net/mac80211/ieee80211_i.h
:100644 100644 393b480... 164cdb1... M	net/mac80211/mlme.c
:100644 100644 cb2eb67... 5696621... M	net/wireless/nl80211.c
 include/linux/nl80211.h    |    4 ++++
 include/net/cfg80211.h     |    2 ++
 net/mac80211/cfg.c         |    3 +++
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |    1 +
 net/wireless/nl80211.c     |    7 +++++++
 6 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index d9c2cdb..ae50ade 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1111,6 +1111,8 @@ enum nl80211_commands {
  *
  * @NL80211_ATTR_DISABLE_80211N:  Force /n capable stations to instead
  *      function as /a/b/g stations.
+ * @NL80211_ATTR_DISABLE_HT40:  Disable HT-40 even if AP and hardware
+ *      support it.
  *
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -1341,6 +1343,8 @@ enum nl80211_attrs {
 	NL80211_ATTR_TDLS_EXTERNAL_SETUP,
 
 	NL80211_ATTR_DISABLE_11N,
+	NL80211_ATTR_DISABLE_HT40,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 1331a5b..9d7a5e0 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -255,10 +255,12 @@ struct ieee80211_supported_band {
  * struct vif_params - describes virtual interface parameters
  * @use_4addr: use 4-address frames
  * @disable_11n:  Don't use 11n features (HT, etc)
+ * @disable_ht40:  Don't use HT40, even if hardware & AP support it.
  */
 struct vif_params {
 	int use_4addr;
 	int disable_11n;
+	int disable_ht40;
 };
 
 /**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 8221a3a..c63d7f0 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -60,6 +60,9 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
 	if (params->disable_11n != -1)
 		sdata->cfg_disable_11n = params->disable_11n;
 
+	if (params->disable_ht40 != -1)
+		sdata->cfg_disable_ht40 = params->disable_ht40;
+
 	ret = ieee80211_if_change_type(sdata, type);
 	if (ret)
 		return ret;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 72c6726..f4a7618 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -596,6 +596,7 @@ struct ieee80211_sub_if_data {
 	bool old_idle;
 
 	bool cfg_disable_11n; /* configured to disable 11n? */
+	bool cfg_disable_ht40; /* configured to not use HT-40 */
 
 	/* Fragment table for host-based reassembly */
 	struct ieee80211_fragment_entry	fragments[IEEE80211_FRAGMENT_MAX];
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 393b480..164cdb1 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -207,6 +207,7 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 		channel_type = NL80211_CHAN_HT20;
 
 		if (!(ap_ht_cap_flags & IEEE80211_HT_CAP_40MHZ_INTOLERANT) &&
+		    !sdata->cfg_disable_ht40 &&
 		    (sband->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) &&
 		    (hti->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)) {
 			switch(hti->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index cb2eb67..5696621 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1648,6 +1648,13 @@ static int nl80211_set_interface(struct sk_buff *skb, struct genl_info *info)
 		params.disable_11n = -1;
 	}
 
+	if (info->attrs[NL80211_ATTR_DISABLE_HT40]) {
+		params.disable_ht40 = !!nla_get_u8(info->attrs[NL80211_ATTR_DISABLE_HT40]);
+		change = true;
+	} else {
+		params.disable_ht40 = -1;
+	}
+
 	if (change)
 		err = cfg80211_change_iface(rdev, dev, ntype, flags, &params);
 	else
-- 
1.7.3.4

[wireless-next PATCH 3/5] wifi: Allow overriding some HT information.

[greearb]

From: Ben Greear <greearb@candelatech.com>

* Allow configuring the mcs (/n) rates available.
* Allow configuration of MAX-A-MSDU
* Allow configuration of A-MPDU factor & density.

Users can only remove existing rates.  The MSDU and MPDU
values can be set to any value allowed by the 802.11n
specification.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 ae50ade... 7f269cd... M	include/linux/nl80211.h
:100644 100644 9d7a5e0... 802ff5f... M	include/net/cfg80211.h
:100644 100644 c63d7f0... dde541f... M	net/mac80211/cfg.c
:100644 100644 f80a35c... 0753c96... M	net/mac80211/ht.c
:100644 100644 f4a7618... f279ee9... M	net/mac80211/ieee80211_i.h
:100644 100644 164cdb1... 681ba4e... M	net/mac80211/mlme.c
:100644 100644 94472eb... 2f546be... M	net/mac80211/work.c
:100644 100644 5696621... 2cbb7c6... M	net/wireless/nl80211.c
 include/linux/nl80211.h    |    3 ++
 include/net/cfg80211.h     |    4 ++
 net/mac80211/cfg.c         |   28 +++++++++++++++++-
 net/mac80211/ht.c          |   70 +++++++++++++++++++++++++++++++++++++++++++-
 net/mac80211/ieee80211_i.h |    9 +++++-
 net/mac80211/mlme.c        |    4 +-
 net/mac80211/work.c        |   34 +++++++++++++++------
 net/wireless/nl80211.c     |   14 +++++++++
 8 files changed, 151 insertions(+), 15 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index ae50ade..7f269cd 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1113,6 +1113,8 @@ enum nl80211_commands {
  *      function as /a/b/g stations.
  * @NL80211_ATTR_DISABLE_HT40:  Disable HT-40 even if AP and hardware
  *      support it.
+ * @NL80211_ATTR_HT_CAPABILITY_MASK: Specify which bits of the HT_CAPs
+ *      to pay attention to.
  *
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -1344,6 +1346,7 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_DISABLE_11N,
 	NL80211_ATTR_DISABLE_HT40,
+	NL80211_ATTR_HT_CAPABILITY_MASK,
 
 	/* add attributes here, update the policy in nl80211.c */
 
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9d7a5e0..802ff5f 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -256,11 +256,15 @@ struct ieee80211_supported_band {
  * @use_4addr: use 4-address frames
  * @disable_11n:  Don't use 11n features (HT, etc)
  * @disable_ht40:  Don't use HT40, even if hardware & AP support it.
+ * @ht_capa:  HT Capabilities for this interface.
+ * @ht_capa_mask:  Bits of ht_capa that are to be used.
  */
 struct vif_params {
 	int use_4addr;
 	int disable_11n;
 	int disable_ht40;
+	struct ieee80211_ht_cap *ht_capa;
+	struct ieee80211_ht_cap *ht_capa_mask;
 };
 
 /**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index c63d7f0..dde541f 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -105,6 +105,32 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
 		}
 	}
 
+	if (params->ht_capa) {
+		u8 *caps = (u8 *)(params->ht_capa);
+		u8 *mask = (u8 *)(params->ht_capa_mask);
+		u8 *scaps = (u8 *)(&sdata->ht_capa);
+		u8 *smask = (u8 *)(&sdata->ht_capa_mask);
+		int i;
+
+		for (i = 0; i < sizeof(sdata->ht_capa); i++) {
+			if (mask[i]) {
+				int q;
+				smask[i] |= mask[i];
+				for (q = 0; q < 8; q++) {
+					if (mask[i] & (1<<q)) {
+						if (caps[i] & (1<<q))
+							scaps[i] |= (1<<q);
+						else
+							scaps[i] &= ~(1<<q);
+					}
+				}
+			}
+		}
+	} else if (params->ht_capa_mask) {
+		memcpy(&sdata->ht_capa_mask, params->ht_capa_mask,
+		       sizeof(sdata->ht_capa_mask));
+	}
+
 	return 0;
 }
 
@@ -784,7 +810,7 @@ static void sta_apply_parameters(struct ieee80211_local *local,
 	}
 
 	if (params->ht_capa)
-		ieee80211_ht_cap_ie_to_sta_ht_cap(sband,
+		ieee80211_ht_cap_ie_to_sta_ht_cap(sdata, sband,
 						  params->ht_capa,
 						  &sta->sta.ht_cap);
 
diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
index f80a35c..0753c96 100644
--- a/net/mac80211/ht.c
+++ b/net/mac80211/ht.c
@@ -18,7 +18,70 @@
 #include "ieee80211_i.h"
 #include "rate.h"
 
-void ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_supported_band *sband,
+void ieee80211_apply_htcap_overrides(struct ieee80211_sub_if_data *sdata,
+				     struct ieee80211_sta_ht_cap *ht_cap,
+				     int min_rates)
+{
+	u8 *scaps = (u8 *)(&sdata->ht_capa.mcs.rx_mask);
+	u8 *smask = (u8 *)(&sdata->ht_capa_mask.mcs.rx_mask);
+	int i;
+
+	/* check for HT over-rides, mcs rates only at this time,
+	 * and can only disable them, not force new ones to be
+	 * made available.
+	 */
+	for (i = 0; i < IEEE80211_HT_MCS_MASK_LEN; i++) {
+		int q;
+		for (q = 0; q < 8; q++) {
+			/*
+			 * We always need to advert at least MCS0-7, to
+			 * be a compliant HT station, for instance
+			 */
+			if (((i * 8 + q) >= min_rates) &&
+			    (smask[i] & (1<<q))) {
+				if (!(scaps[i] & (1<<q))) {
+					/*
+					 * Can only disable rates, not force
+					 * new ones
+					 */
+					ht_cap->mcs.rx_mask[i] &= ~(1<<q);
+				}
+			}
+		}
+	}
+
+	/* Force removal of HT-40 capabilities? */
+	if (sdata->cfg_disable_ht40) {
+		ht_cap->cap &= ~(IEEE80211_HT_CAP_SUP_WIDTH_20_40
+				 | IEEE80211_HT_CAP_SGI_40);
+	}
+
+	/* Allow user to set max AMDSU bit. */
+	if (sdata->ht_capa_mask.cap_info & IEEE80211_HT_CAP_MAX_AMSDU) {
+		if (sdata->ht_capa.cap_info & IEEE80211_HT_CAP_MAX_AMSDU)
+			ht_cap->cap |= IEEE80211_HT_CAP_MAX_AMSDU;
+		else
+			ht_cap->cap &= ~IEEE80211_HT_CAP_MAX_AMSDU;
+	}
+
+	/* Set the AMPDU factor */
+	if (sdata->ht_capa_mask.ampdu_params_info &
+	    IEEE80211_HT_AMPDU_PARM_FACTOR)
+		ht_cap->ampdu_factor = sdata->ht_capa.ampdu_params_info &
+			IEEE80211_HT_AMPDU_PARM_FACTOR;
+
+	/* Set the AMPDU density */
+	if (sdata->ht_capa_mask.ampdu_params_info &
+	    IEEE80211_HT_AMPDU_PARM_DENSITY)
+		ht_cap->ampdu_density =
+			(sdata->ht_capa.ampdu_params_info &
+			 IEEE80211_HT_AMPDU_PARM_DENSITY)
+			>> IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT;
+}
+
+
+void ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_sub_if_data *sdata,
+				       struct ieee80211_supported_band *sband,
 				       struct ieee80211_ht_cap *ht_cap_ie,
 				       struct ieee80211_sta_ht_cap *ht_cap)
 {
@@ -102,6 +165,11 @@ void ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_supported_band *sband,
 	/* handle MCS rate 32 too */
 	if (sband->ht_cap.mcs.rx_mask[32/8] & ht_cap_ie->mcs.rx_mask[32/8] & 1)
 		ht_cap->mcs.rx_mask[32/8] |= 1;
+
+	/* If user has specified capability over-rides, take care
+	 * of that here.
+	 */
+	ieee80211_apply_htcap_overrides(sdata, ht_cap, 0);
 }
 
 void ieee80211_sta_tear_down_BA_sessions(struct sta_info *sta, bool tx)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index f4a7618..f279ee9 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -598,6 +598,9 @@ struct ieee80211_sub_if_data {
 	bool cfg_disable_11n; /* configured to disable 11n? */
 	bool cfg_disable_ht40; /* configured to not use HT-40 */
 
+	struct ieee80211_ht_cap ht_capa; /* configured ht-cap over-rides */
+	struct ieee80211_ht_cap ht_capa_mask; /* Valid parts of ht_capa */
+
 	/* Fragment table for host-based reassembly */
 	struct ieee80211_fragment_entry	fragments[IEEE80211_FRAGMENT_MAX];
 	unsigned int fragment_next;
@@ -1181,7 +1184,11 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 				       struct net_device *dev);
 
 /* HT */
-void ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_supported_band *sband,
+void ieee80211_apply_htcap_overrides(struct ieee80211_sub_if_data *sdata,
+				     struct ieee80211_sta_ht_cap *ht_cap,
+				     int min_rates);
+void ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_sub_if_data *sdata,
+				       struct ieee80211_supported_band *sband,
 				       struct ieee80211_ht_cap *ht_cap_ie,
 				       struct ieee80211_sta_ht_cap *ht_cap);
 void ieee80211_send_delba(struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 164cdb1..681ba4e 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1571,7 +1571,7 @@ static bool ieee80211_assoc_success(struct ieee80211_work *wk,
 		sdata->flags &= ~IEEE80211_SDATA_OPERATING_GMODE;
 
 	if (elems.ht_cap_elem && !(ifmgd->flags & IEEE80211_STA_DISABLE_11N))
-		ieee80211_ht_cap_ie_to_sta_ht_cap(sband,
+		ieee80211_ht_cap_ie_to_sta_ht_cap(sdata, sband,
 				elems.ht_cap_elem, &sta->sta.ht_cap);
 
 	ap_ht_cap_flags = sta->sta.ht_cap.cap;
@@ -1940,7 +1940,7 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 
 		sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
 
-		ieee80211_ht_cap_ie_to_sta_ht_cap(sband,
+		ieee80211_ht_cap_ie_to_sta_ht_cap(sdata, sband,
 				elems.ht_cap_elem, &sta->sta.ht_cap);
 
 		ap_ht_cap_flags = sta->sta.ht_cap.cap;
diff --git a/net/mac80211/work.c b/net/mac80211/work.c
index 94472eb..2f546be 100644
--- a/net/mac80211/work.c
+++ b/net/mac80211/work.c
@@ -94,7 +94,8 @@ static int ieee80211_compatible_rates(const u8 *supp_rates, int supp_rates_len,
 
 /* frame sending functions */
 
-static void ieee80211_add_ht_ie(struct sk_buff *skb, const u8 *ht_info_ie,
+static void ieee80211_add_ht_ie(struct ieee80211_sub_if_data *sdata,
+				struct sk_buff *skb, const u8 *ht_info_ie,
 				struct ieee80211_supported_band *sband,
 				struct ieee80211_channel *channel,
 				enum ieee80211_smps_mode smps)
@@ -102,11 +103,11 @@ static void ieee80211_add_ht_ie(struct sk_buff *skb, const u8 *ht_info_ie,
 	struct ieee80211_ht_info *ht_info;
 	u8 *pos;
 	u32 flags = channel->flags;
-	u16 cap = sband->ht_cap.cap;
+	u16 cap;
 	__le16 tmp;
+	struct ieee80211_sta_ht_cap ht_cap;
 
-	if (!sband->ht_cap.ht_supported)
-		return;
+	BUILD_BUG_ON(sizeof(ht_cap) != sizeof(sband->ht_cap));
 
 	if (!ht_info_ie)
 		return;
@@ -114,6 +115,19 @@ static void ieee80211_add_ht_ie(struct sk_buff *skb, const u8 *ht_info_ie,
 	if (ht_info_ie[1] < sizeof(struct ieee80211_ht_info))
 		return;
 
+	memcpy(&ht_cap, &sband->ht_cap, sizeof(ht_cap));
+	/*
+	 * This is for an association attempt, and we must
+	 * advert at least the first 8 rates, even if we
+	 * will later force the rate control to a lower rate.
+	 */
+	ieee80211_apply_htcap_overrides(sdata, &ht_cap, 8);
+
+	cap = ht_cap.cap;
+
+	if (!ht_cap.ht_supported)
+		return;
+
 	ht_info = (struct ieee80211_ht_info *)(ht_info_ie + 2);
 
 	/* determine capability flags */
@@ -166,13 +180,13 @@ static void ieee80211_add_ht_ie(struct sk_buff *skb, const u8 *ht_info_ie,
 	pos += sizeof(u16);
 
 	/* AMPDU parameters */
-	*pos++ = sband->ht_cap.ampdu_factor |
-		 (sband->ht_cap.ampdu_density <<
-			IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT);
+	*pos++ = ht_cap.ampdu_factor |
+		 (ht_cap.ampdu_density <<
+		  IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT);
 
 	/* MCS set */
-	memcpy(pos, &sband->ht_cap.mcs, sizeof(sband->ht_cap.mcs));
-	pos += sizeof(sband->ht_cap.mcs);
+	memcpy(pos, &ht_cap.mcs, sizeof(ht_cap.mcs));
+	pos += sizeof(ht_cap.mcs);
 
 	/* extended capabilities */
 	pos += sizeof(__le16);
@@ -356,7 +370,7 @@ static void ieee80211_send_assoc(struct ieee80211_sub_if_data *sdata,
 
 	if (wk->assoc.use_11n && wk->assoc.wmm_used &&
 	    local->hw.queues >= 4)
-		ieee80211_add_ht_ie(skb, wk->assoc.ht_information_ie,
+		ieee80211_add_ht_ie(sdata, skb, wk->assoc.ht_information_ie,
 				    sband, wk->chan, wk->assoc.smps);
 
 	/* if present, add any custom non-vendor IEs that go after HT */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 5696621..2cbb7c6 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1655,6 +1655,20 @@ static int nl80211_set_interface(struct sk_buff *skb, struct genl_info *info)
 		params.disable_ht40 = -1;
 	}
 
+	if (info->attrs[NL80211_ATTR_HT_CAPABILITY_MASK]) {
+		params.ht_capa_mask =
+			nla_data(info->attrs[NL80211_ATTR_HT_CAPABILITY_MASK]);
+		change = true;
+	}
+
+	if (info->attrs[NL80211_ATTR_HT_CAPABILITY]) {
+		if (!params.ht_capa_mask)
+			return -EINVAL;
+		params.ht_capa =
+			nla_data(info->attrs[NL80211_ATTR_HT_CAPABILITY]);
+		change = true;
+	}
+
 	if (change)
 		err = cfg80211_change_iface(rdev, dev, ntype, flags, &params);
 	else
-- 
1.7.3.4

[wireless-next PATCH 4/5] wifi: Warn if cannot add station debugfs entries.

[greearb]

From: Ben Greear <greearb@candelatech.com>

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 c5f3417... 1ceec86... M	net/mac80211/debugfs_sta.c
 net/mac80211/debugfs_sta.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index c5f3417..1ceec86 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -337,8 +337,11 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
 
 	sta->debugfs.add_has_run = true;
 
-	if (!stations_dir)
+	if (!stations_dir) {
+		printk(KERN_DEBUG "%s: sta_debugfs_add: stations_dir is NULL\n",
+			sta->sdata->name);
 		return;
+	}
 
 	snprintf(mac, sizeof(mac), "%pM", sta->sta.addr);
 
@@ -352,8 +355,11 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
 	 * dir might still be around.
 	 */
 	sta->debugfs.dir = debugfs_create_dir(mac, stations_dir);
-	if (!sta->debugfs.dir)
+	if (!sta->debugfs.dir) {
+		printk(KERN_DEBUG "%s: sta_debugfs_add: Failed to create sta->debugfs.dir\n",
+			sta->sdata->name);
 		return;
+	}
 
 	DEBUGFS_ADD(flags);
 	DEBUGFS_ADD(num_ps_buf_frames);
-- 
1.7.3.4

[wireless-next PATCH 5/5] wifi-debugfs: Fix AMSDU rate printout.

[greearb]

From: Ben Greear <greearb@candelatech.com>

It was flipped.  See section 7.3.2.56 of the 802.11n
spec for details.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 1ceec86... 4310431... M	net/mac80211/debugfs_sta.c
 net/mac80211/debugfs_sta.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index 1ceec86..4310431 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -274,9 +274,9 @@ static ssize_t sta_ht_capa_read(struct file *file, char __user *userbuf,
 
 		PRINT_HT_CAP((htc->cap & BIT(10)), "HT Delayed Block Ack");
 
-		PRINT_HT_CAP((htc->cap & BIT(11)), "Max AMSDU length: "
-			     "3839 bytes");
 		PRINT_HT_CAP(!(htc->cap & BIT(11)), "Max AMSDU length: "
+			     "3839 bytes");
+		PRINT_HT_CAP((htc->cap & BIT(11)), "Max AMSDU length: "
 			     "7935 bytes");
 
 		/*
-- 
1.7.3.4

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Ben Greear]

On 10/27/2011 10:11 PM, greearb@candelatech.com wrote:
> From: Ben Greear<greearb@candelatech.com>
>
> This allows a user to configure a wifi station interface
> to disable 802.11n, even if the AP and NIC supports it.
>
> The additional netlink bits is to allow this patch to work on 3.0
> and should not be included in the final patch.

I have the wpa_supplicant patches for this series tested as well,
but I figured it best to make sure these kernel patches are acceptable
before posting the hostap patches....

Tested this on ath9k.  According to the rate output seen by
iwconfig and the debug stuff in
cat /debug/ieee80211/wiphy0/netdev\:sta2/stations/00\:88\:99\:88\:99\:01/ht_capa
I think I have this working pretty well.

I have not yet tried verifying things with a sniffer...

Thanks,
Ben

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

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Johannes Berg]

On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:

> The additional netlink bits is to allow this patch to work on 3.0
> and should not be included in the final patch.

What additional bit?


> + * @NL80211_ATTR_DISABLE_80211N:  Force /n capable stations to instead
> + *      function as /a/b/g stations.

IMHO this should be called DISABLE_HT -- "11n" will not exist for much
longer.

> +++ b/net/mac80211/cfg.c
> @@ -57,6 +57,9 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
>  	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
>  	int ret;
>  
> +	if (params->disable_11n != -1)
> +		sdata->cfg_disable_11n = params->disable_11n;

This doesn't seem right -- why change the iface for it? It's a per
connection parameter.

> +++ b/net/mac80211/ieee80211_i.h
> @@ -595,6 +595,8 @@ struct ieee80211_sub_if_data {
>  	/* to detect idle changes */
>  	bool old_idle;
>  
> +	bool cfg_disable_11n; /* configured to disable 11n? */

That should be in the u.mgd part of the struct.

> +++ b/net/wireless/nl80211.c
> @@ -1641,6 +1641,13 @@ static int nl80211_set_interface(struct sk_buff *skb, struct genl_info *info)
>  		change = true;
>  	}
>  
> +	if (info->attrs[NL80211_ATTR_DISABLE_11N]) {
> +		params.disable_11n = !!nla_get_u8(info->attrs[NL80211_ATTR_DISABLE_11N]);
> +		change = true;
> +	} else {
> +		params.disable_11n = -1;
> +	}

This should be a parameter to connect() and assoc(), not a generic
netdev parameter, since it applies to the connection.

Also, it would be good to have a capability check for it etc. since a
lot of fullmac drivers will likely never implement this.

johannes

Re: [wireless-next PATCH 2/5] wifi: Support disabling ht40.

[Johannes Berg]

On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:

>  struct vif_params {
>  	int use_4addr;
>  	int disable_11n;
> +	int disable_ht40;
>  };

All the comments on patch 1 apply -- per connection parameter,
capability flag etc.

I do wonder if it would be worthwhile to make it a u32 "connection
flags" or so instead of defining them separately.

johannes

Re: [wireless-next PATCH 3/5] wifi: Allow overriding some HT information.

[Johannes Berg]

On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> * Allow configuring the mcs (/n) rates available.
> * Allow configuration of MAX-A-MSDU
> * Allow configuration of A-MPDU factor & density.
> 
> Users can only remove existing rates.  The MSDU and MPDU
> values can be set to any value allowed by the 802.11n
> specification.

That can't work -- the device might not support it.

I also don't really like the way you pass in some binary "mask" when
it's not really a binary masking operation.

>  struct vif_params {
>  	int use_4addr;
>  	int disable_11n;
>  	int disable_ht40;
> +	struct ieee80211_ht_cap *ht_capa;
> +	struct ieee80211_ht_cap *ht_capa_mask;

Same comments as before again -- this is per connection right?

> @@ -114,6 +115,19 @@ static void ieee80211_add_ht_ie(struct sk_buff *skb, const u8 *ht_info_ie,
>  	if (ht_info_ie[1] < sizeof(struct ieee80211_ht_info))
>  		return;
>  
> +	memcpy(&ht_cap, &sband->ht_cap, sizeof(ht_cap));
> +	/*
> +	 * This is for an association attempt, and we must
> +	 * advert at least the first 8 rates, even if we
> +	 * will later force the rate control to a lower rate.
> +	 */
> +	ieee80211_apply_htcap_overrides(sdata, &ht_cap, 8);

Yuck, why, why hard-code 8, etc.

johannes

Re: [wireless-next PATCH 4/5] wifi: Warn if cannot add station debugfs entries.

[Johannes Berg]

On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
> :100644 100644 c5f3417... 1ceec86... M	net/mac80211/debugfs_sta.c
>  net/mac80211/debugfs_sta.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
> index c5f3417..1ceec86 100644
> --- a/net/mac80211/debugfs_sta.c
> +++ b/net/mac80211/debugfs_sta.c
> @@ -337,8 +337,11 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
>  
>  	sta->debugfs.add_has_run = true;
>  
> -	if (!stations_dir)
> +	if (!stations_dir) {
> +		printk(KERN_DEBUG "%s: sta_debugfs_add: stations_dir is NULL\n",
> +			sta->sdata->name);
>  		return;
> +	}

I honestly don't see any point in this.

johannes

Re: [wireless-next PATCH 5/5] wifi-debugfs: Fix AMSDU rate printout.

[Johannes Berg]

On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> It was flipped.  See section 7.3.2.56 of the 802.11n
> spec for details.
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>

Acked-by: Johannes Berg <johannes@sipsolutions.net>

Thanks!

johannes

Re: [wireless-next PATCH 4/5] wifi: Warn if cannot add station debugfs entries.

[Ben Greear]

On 10/28/2011 01:13 AM, Johannes Berg wrote:
> On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:
>> From: Ben Greear<greearb@candelatech.com>
>>
>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>> ---
>> :100644 100644 c5f3417... 1ceec86... M	net/mac80211/debugfs_sta.c
>>   net/mac80211/debugfs_sta.c |   10 ++++++++--
>>   1 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
>> index c5f3417..1ceec86 100644
>> --- a/net/mac80211/debugfs_sta.c
>> +++ b/net/mac80211/debugfs_sta.c
>> @@ -337,8 +337,11 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
>>
>>   	sta->debugfs.add_has_run = true;
>>
>> -	if (!stations_dir)
>> +	if (!stations_dir) {
>> +		printk(KERN_DEBUG "%s: sta_debugfs_add: stations_dir is NULL\n",
>> +			sta->sdata->name);
>>   		return;
>> +	}
>
> I honestly don't see any point in this.

Ok, I wasn't sure if this was wanted, so please just
drop it.  I did see a case where the debugfs files didn't
exist, and was trying to figure out why..but never reproduced
the problem after this printk was in, so not sure if this
was the issue or not.

Thanks,
Ben

>
> johannes


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

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Ben Greear]

On 10/28/2011 01:08 AM, Johannes Berg wrote:
> On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:
>
>> The additional netlink bits is to allow this patch to work on 3.0
>> and should not be included in the final patch.
>
> What additional bit?

Sorry, that description is bad.  It was from when I wrote the patch
against the 3.0.6 kernel and I had to add a bunch of netlink
crap to make the header file sync up with top-of-tree hostapd.

>> + * @NL80211_ATTR_DISABLE_80211N:  Force /n capable stations to instead
>> + *      function as /a/b/g stations.
>
> IMHO this should be called DISABLE_HT -- "11n" will not exist for much
> longer.

Ok, I can change that.

>
>> +++ b/net/mac80211/cfg.c
>> @@ -57,6 +57,9 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
>>   	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
>>   	int ret;
>>
>> +	if (params->disable_11n != -1)
>> +		sdata->cfg_disable_11n = params->disable_11n;
>
> This doesn't seem right -- why change the iface for it? It's a per
> connection parameter.

I wanted it to be an interface parameter, or at least I think
that is what I want.

>> +++ b/net/mac80211/ieee80211_i.h
>> @@ -595,6 +595,8 @@ struct ieee80211_sub_if_data {
>>   	/* to detect idle changes */
>>   	bool old_idle;
>>
>> +	bool cfg_disable_11n; /* configured to disable 11n? */
>
> That should be in the u.mgd part of the struct.

I would like eventually to support this same feature for AP
interfaces, and probably other types.  Would it still be in
the u.mgd struct in that case?

>> +++ b/net/wireless/nl80211.c
>> @@ -1641,6 +1641,13 @@ static int nl80211_set_interface(struct sk_buff *skb, struct genl_info *info)
>>   		change = true;
>>   	}
>>
>> +	if (info->attrs[NL80211_ATTR_DISABLE_11N]) {
>> +		params.disable_11n = !!nla_get_u8(info->attrs[NL80211_ATTR_DISABLE_11N]);
>> +		change = true;
>> +	} else {
>> +		params.disable_11n = -1;
>> +	}
>
> This should be a parameter to connect() and assoc(), not a generic
> netdev parameter, since it applies to the connection.
>
> Also, it would be good to have a capability check for it etc. since a
> lot of fullmac drivers will likely never implement this.

I'll see if I can make it thus.

Thanks,
Ben

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

Re: [wireless-next PATCH 2/5] wifi: Support disabling ht40.

[Ben Greear]

On 10/28/2011 01:09 AM, Johannes Berg wrote:
> On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:
>
>>   struct vif_params {
>>   	int use_4addr;
>>   	int disable_11n;
>> +	int disable_ht40;
>>   };
>
> All the comments on patch 1 apply -- per connection parameter,
> capability flag etc.
>
> I do wonder if it would be worthwhile to make it a u32 "connection
> flags" or so instead of defining them separately.

I don't care either way.  If you want a single variable to hold
the flags that is easy enough.  We'd need to pass in a mask to
allow user-space to change only a subset of flags at a time
though.

Thanks,
Ben

>
> johannes
>
> --
> 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


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

Re: [wireless-next PATCH 3/5] wifi: Allow overriding some HT information.

[Ben Greear]

On 10/28/2011 01:12 AM, Johannes Berg wrote:
> On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:
>> From: Ben Greear<greearb@candelatech.com>
>>
>> * Allow configuring the mcs (/n) rates available.
>> * Allow configuration of MAX-A-MSDU
>> * Allow configuration of A-MPDU factor&  density.
>>
>> Users can only remove existing rates.  The MSDU and MPDU
>> values can be set to any value allowed by the 802.11n
>> specification.
>
> That can't work -- the device might not support it.

The device would always support removing rates, right?

As for the MSDU and MPDU stuff, I would need to add capabilities
flags and then enable each driver as they are tested?

> I also don't really like the way you pass in some binary "mask" when
> it's not really a binary masking operation.

I found the mask to work very well.  It's easy enough to
deal with in user-space, and easily allows us to add override features
(the additional bits to support the MPDU stuff once the
HT rates logic was in is a very small bit of code).

Otherwise, I'll need to add new netlink commands for each new
feature.  That is going to bloat hostapd as well as the
kernel.

>
>>   struct vif_params {
>>   	int use_4addr;
>>   	int disable_11n;
>>   	int disable_ht40;
>> +	struct ieee80211_ht_cap *ht_capa;
>> +	struct ieee80211_ht_cap *ht_capa_mask;
>
> Same comments as before again -- this is per connection right?

I meant it to be per interface.  I'm using lots of virtual stations,
and want some to act like /abg radios, and others to be ht-20 only,
and others to be capable of only up to mcs7, etc.

>> @@ -114,6 +115,19 @@ static void ieee80211_add_ht_ie(struct sk_buff *skb, const u8 *ht_info_ie,
>>   	if (ht_info_ie[1]<  sizeof(struct ieee80211_ht_info))
>>   		return;
>>
>> +	memcpy(&ht_cap,&sband->ht_cap, sizeof(ht_cap));
>> +	/*
>> +	 * This is for an association attempt, and we must
>> +	 * advert at least the first 8 rates, even if we
>> +	 * will later force the rate control to a lower rate.
>> +	 */
>> +	ieee80211_apply_htcap_overrides(sdata,&ht_cap, 8);
>
> Yuck, why, why hard-code 8, etc.

I can make a define that involves MCS7.

Thanks,
Ben

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

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Ben Greear]

On 10/28/2011 01:08 AM, Johannes Berg wrote:
> On Thu, 2011-10-27 at 22:11 -0700, greearb@candelatech.com wrote:

>> +++ b/net/wireless/nl80211.c
>> @@ -1641,6 +1641,13 @@ static int nl80211_set_interface(struct sk_buff *skb, struct genl_info *info)
>>   		change = true;
>>   	}
>>
>> +	if (info->attrs[NL80211_ATTR_DISABLE_11N]) {
>> +		params.disable_11n = !!nla_get_u8(info->attrs[NL80211_ATTR_DISABLE_11N]);
>> +		change = true;
>> +	} else {
>> +		params.disable_11n = -1;
>> +	}
>
> This should be a parameter to connect() and assoc(), not a generic
> netdev parameter, since it applies to the connection.
>
> Also, it would be good to have a capability check for it etc. since a
> lot of fullmac drivers will likely never implement this.

The existing code always sets the IEEE80211_STA_DISABLE_11N flag in u.mgd if
WEP or TKIP is configured, without any capability checks, and my patch
sets that flag in the same location.

So, maybe it is OK as is?

If not, I will add a new capability bit and just enable
it in ath9k (and let others enable it in their drivers as they wish).

Thanks,
Ben

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

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Johannes Berg]

On Fri, 2011-10-28 at 11:55 -0700, Ben Greear wrote:

> > This should be a parameter to connect() and assoc(), not a generic
> > netdev parameter, since it applies to the connection.
> >
> > Also, it would be good to have a capability check for it etc. since a
> > lot of fullmac drivers will likely never implement this.
> 
> The existing code always sets the IEEE80211_STA_DISABLE_11N flag in u.mgd if
> WEP or TKIP is configured, without any capability checks, and my patch
> sets that flag in the same location.
> 
> So, maybe it is OK as is?
> 
> If not, I will add a new capability bit and just enable
> it in ath9k (and let others enable it in their drivers as they wish).

You misunderstood -- I said fullmac drivers, not differences between
ath9k, iwlwifi etc. Other cfg80211 drivers, not other mac80211 drivers.

johannes

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Johannes Berg]

On Fri, 2011-10-28 at 09:24 -0700, Ben Greear wrote:

> >> +++ b/net/mac80211/cfg.c
> >> @@ -57,6 +57,9 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
> >>   	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> >>   	int ret;
> >>
> >> +	if (params->disable_11n != -1)
> >> +		sdata->cfg_disable_11n = params->disable_11n;
> >
> > This doesn't seem right -- why change the iface for it? It's a per
> > connection parameter.
> 
> I wanted it to be an interface parameter, or at least I think
> that is what I want.

Why? I'm thinking that it's better as a connection parameter as then
it's more temporal. I know we have interface parameters like RTS/CTS
settings etc, but I like connection parameters better as they go away
with each new connection, so the behaviour is less surprising to most
users. Imagine your wpa_supplicant crashes, and then the user who was
restricting it to no-HT starts the regular wpa_supplicant; now his
interface will be in no-HT until he reboots or figures out the right
magic to change it. I think that kind of situation is undesirable.


> I would like eventually to support this same feature for AP
> interfaces, and probably other types.  Would it still be in
> the u.mgd struct in that case?

No, but again there I'd argue that it should be a "connection" parameter
as well.

johannes

Re: [wireless-next PATCH 3/5] wifi: Allow overriding some HT information.

[Johannes Berg]

On Fri, 2011-10-28 at 09:33 -0700, Ben Greear wrote:

> >> * Allow configuring the mcs (/n) rates available.
> >> * Allow configuration of MAX-A-MSDU
> >> * Allow configuration of A-MPDU factor&  density.
> >>
> >> Users can only remove existing rates.  The MSDU and MPDU
> >> values can be set to any value allowed by the 802.11n
> >> specification.
> >
> > That can't work -- the device might not support it.
> 
> The device would always support removing rates, right?

No. As I pointed out in another email, you need to think beyond
mac80211.

> As for the MSDU and MPDU stuff, I would need to add capabilities
> flags and then enable each driver as they are tested?

No. Neither of these can ever be increased so it's not that simple. And
making it smaller is always possible since it's just advertising.
Presumably you do understand the reasons for this advertising/these
restrictions?

> > I also don't really like the way you pass in some binary "mask" when
> > it's not really a binary masking operation.
> 
> I found the mask to work very well.  It's easy enough to
> deal with in user-space, and easily allows us to add override features
> (the additional bits to support the MPDU stuff once the
> HT rates logic was in is a very small bit of code).
> 
> Otherwise, I'll need to add new netlink commands for each new
> feature.  That is going to bloat hostapd as well as the
> kernel.

I'm not sure it'll bloat it, but I can live with the binary struct. It
seems a bit ugly to me but I think it's acceptable. However, you should
document more clearly what struct it is and how it is defined to work.
You're also relying on userspace to not do stupid things (otherwise your
code reads memory out of bounds), this would help:

>         /* add attributes here, update the policy in nl80211.c */

Anyway, I'm out of breath. I think this is simply bad implementation &
execution of your ideas. These are somewhat interesting features and I'm
willing to accommodate even some of the more esoteric features that most
people will never use, but I'm quite annoyed by how much of your patches
I end up doing myself through review.

Please focus more on the quality of your patches in "details" like
security, API issues, non-mac80211 drivers and even simply logic like
the A-MSDU thing I just pointed out. I'm spending a disproportionate
amount of time reviewing your patches over and over again with every
patchset and am no longer willing to do that.

Obviously we'll still have to discuss things like whether it should be a
connection or interface parameter -- but I shouldn't have to point out
trivial security issues -- you should be focusing more on these things
while writing the code, not after the fact where it's much harder. E.g.
never ever use nla_data() without validating nla_len() in some way, it
seems to me that should be completely obvious.


> >> @@ -114,6 +115,19 @@ static void ieee80211_add_ht_ie(struct sk_buff *skb, const u8 *ht_info_ie,
> >>   	if (ht_info_ie[1]<  sizeof(struct ieee80211_ht_info))
> >>   		return;
> >>
> >> +	memcpy(&ht_cap,&sband->ht_cap, sizeof(ht_cap));
> >> +	/*
> >> +	 * This is for an association attempt, and we must
> >> +	 * advert at least the first 8 rates, even if we
> >> +	 * will later force the rate control to a lower rate.
> >> +	 */
> >> +	ieee80211_apply_htcap_overrides(sdata,&ht_cap, 8);
> >
> > Yuck, why, why hard-code 8, etc.
> 
> I can make a define that involves MCS7.

I don't think I understand then why MCSs 0-7 must be supported?

johannes

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Ben Greear]

On 11/02/2011 12:53 AM, Johannes Berg wrote:
> On Fri, 2011-10-28 at 11:55 -0700, Ben Greear wrote:
>
>>> This should be a parameter to connect() and assoc(), not a generic
>>> netdev parameter, since it applies to the connection.
>>>
>>> Also, it would be good to have a capability check for it etc. since a
>>> lot of fullmac drivers will likely never implement this.
>>
>> The existing code always sets the IEEE80211_STA_DISABLE_11N flag in u.mgd if
>> WEP or TKIP is configured, without any capability checks, and my patch
>> sets that flag in the same location.
>>
>> So, maybe it is OK as is?
>>
>> If not, I will add a new capability bit and just enable
>> it in ath9k (and let others enable it in their drivers as they wish).
>
> You misunderstood -- I said fullmac drivers, not differences between
> ath9k, iwlwifi etc. Other cfg80211 drivers, not other mac80211 drivers.

Can you at least point me to some existing code that does a similar
check?  I have no idea of what a fullmac driver even does.  I can test
and eventually somewhat understand the paths for ath9k, but I have no
ability to test fullmac (as far as I know).

If you don't have time to explain in more detail, then I will just
carry these patches in my own tree.  That is a total pain in
the arse when dealing with new netlink commands, but I just do not
have enough understanding of the entire wifi stack to make much
progress with a lot of your suggestions.

Thanks,
Ben

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

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Ben Greear]

On 11/02/2011 12:56 AM, Johannes Berg wrote:
> On Fri, 2011-10-28 at 09:24 -0700, Ben Greear wrote:
>
>>>> +++ b/net/mac80211/cfg.c
>>>> @@ -57,6 +57,9 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
>>>>    	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
>>>>    	int ret;
>>>>
>>>> +	if (params->disable_11n != -1)
>>>> +		sdata->cfg_disable_11n = params->disable_11n;
>>>
>>> This doesn't seem right -- why change the iface for it? It's a per
>>> connection parameter.
>>
>> I wanted it to be an interface parameter, or at least I think
>> that is what I want.
>
> Why? I'm thinking that it's better as a connection parameter as then
> it's more temporal. I know we have interface parameters like RTS/CTS
> settings etc, but I like connection parameters better as they go away
> with each new connection, so the behaviour is less surprising to most
> users. Imagine your wpa_supplicant crashes, and then the user who was
> restricting it to no-HT starts the regular wpa_supplicant; now his
> interface will be in no-HT until he reboots or figures out the right
> magic to change it. I think that kind of situation is undesirable.

Ok, I will try to figure out how to do this as part of connection.

Thanks,
Ben

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

Re: [wireless-next PATCH 3/5] wifi: Allow overriding some HT information.

[Ben Greear]

On 11/02/2011 01:13 AM, Johannes Berg wrote:
> On Fri, 2011-10-28 at 09:33 -0700, Ben Greear wrote:
>
>>>> * Allow configuring the mcs (/n) rates available.
>>>> * Allow configuration of MAX-A-MSDU
>>>> * Allow configuration of A-MPDU factor&   density.
>>>>
>>>> Users can only remove existing rates.  The MSDU and MPDU
>>>> values can be set to any value allowed by the 802.11n
>>>> specification.
>>>
>>> That can't work -- the device might not support it.
>>
>> The device would always support removing rates, right?
>
> No. As I pointed out in another email, you need to think beyond
> mac80211.
>
>> As for the MSDU and MPDU stuff, I would need to add capabilities
>> flags and then enable each driver as they are tested?
>
> No. Neither of these can ever be increased so it's not that simple. And
> making it smaller is always possible since it's just advertising.
> Presumably you do understand the reasons for this advertising/these
> restrictions?

It seems that a driver might default to a mid-range value for the MPDU values
because it is somehow 'better' for whatever reason, and yet it might still
support larger values if the user desires, perhaps because in specific
scenarios larger values are better.  Ath9k does set to the max value,
so if we do a per-driver capabilities flag as I did in v2 then we
are safe.


>>> I also don't really like the way you pass in some binary "mask" when
>>> it's not really a binary masking operation.
>>
>> I found the mask to work very well.  It's easy enough to
>> deal with in user-space, and easily allows us to add override features
>> (the additional bits to support the MPDU stuff once the
>> HT rates logic was in is a very small bit of code).
>>
>> Otherwise, I'll need to add new netlink commands for each new
>> feature.  That is going to bloat hostapd as well as the
>> kernel.
>
> I'm not sure it'll bloat it, but I can live with the binary struct. It
> seems a bit ugly to me but I think it's acceptable. However, you should
> document more clearly what struct it is and how it is defined to work.
> You're also relying on userspace to not do stupid things (otherwise your
> code reads memory out of bounds), this would help:
>
>>          /* add attributes here, update the policy in nl80211.c */

I copied some of that code from nl80211_set_station, which appears to
also forget to check the length for the NL80211_ATTR_HT_CAPABILITY
object.  Is there some reason why it doesn't need to check, or does
that code need fixing as well?

> Anyway, I'm out of breath. I think this is simply bad implementation&
> execution of your ideas. These are somewhat interesting features and I'm
> willing to accommodate even some of the more esoteric features that most
> people will never use, but I'm quite annoyed by how much of your patches
> I end up doing myself through review.
>
> Please focus more on the quality of your patches in "details" like
> security, API issues, non-mac80211 drivers and even simply logic like
> the A-MSDU thing I just pointed out. I'm spending a disproportionate
> amount of time reviewing your patches over and over again with every
> patchset and am no longer willing to do that.

I listed several thinks that could be changed in the patch description,
and your comment was:

That can't work -- the device might not support it.

Now, how am I supposed to know what you are talking about?

> Obviously we'll still have to discuss things like whether it should be a
> connection or interface parameter -- but I shouldn't have to point out
> trivial security issues -- you should be focusing more on these things
> while writing the code, not after the fact where it's much harder. E.g.
> never ever use nla_data() without validating nla_len() in some way, it
> seems to me that should be completely obvious.

Well, it's more obvious now.  Do you also need to check the length when
doing code like this code from nl80211_set_station:

	if (info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL])
		params.listen_interval =
		    nla_get_u16(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL]);

In other words, is it safe to assume you have 16 bits, or could a broken
user-space pass in a single byte and screw this up?

>>>> +	memcpy(&ht_cap,&sband->ht_cap, sizeof(ht_cap));
>>>> +	/*
>>>> +	 * This is for an association attempt, and we must
>>>> +	 * advert at least the first 8 rates, even if we
>>>> +	 * will later force the rate control to a lower rate.
>>>> +	 */
>>>> +	ieee80211_apply_htcap_overrides(sdata,&ht_cap, 8);
>>>
>>> Yuck, why, why hard-code 8, etc.
>>
>> I can make a define that involves MCS7.
>
> I don't think I understand then why MCSs 0-7 must be supported?

 From 20.1.1 of the 802.11n spec:

"An HT non-AP STA shall support all equal modulation (EQM) rates for one spatial stream (MCSs 0 through
7) using 20 MHz channel width. An HT AP shall support all EQM rates for one and two spatial streams
(MCSs 0 through 15) using 20 MHz channel width."

That is why I wrote that code as I did, but perhaps I misunderstand that section of
the docs.

Thanks,
Ben

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

Re: [wireless-next PATCH 3/5] wifi: Allow overriding some HT information.

[Johannes Berg]

On Wed, 2011-11-02 at 09:59 -0700, Ben Greear wrote:

> > No. Neither of these can ever be increased so it's not that simple. And
> > making it smaller is always possible since it's just advertising.
> > Presumably you do understand the reasons for this advertising/these
> > restrictions?
> 
> It seems that a driver might default to a mid-range value for the MPDU values
> because it is somehow 'better' for whatever reason, and yet it might still
> support larger values if the user desires, perhaps because in specific
> scenarios larger values are better.  Ath9k does set to the max value,
> so if we do a per-driver capabilities flag as I did in v2 then we
> are safe.

Then the proper way to do that would be to not have a flag, but a max it
can be set to. However, I see no reason it should default to a mid-range
value?! iwlwifi for example needs to allocate enough space but ... I
don't get it. What's wrong with simply not allowing to increase, only
decrease?

> >>          /* add attributes here, update the policy in nl80211.c */
> 
> I copied some of that code from nl80211_set_station, which appears to
> also forget to check the length for the NL80211_ATTR_HT_CAPABILITY
> object.  Is there some reason why it doesn't need to check, or does
> that code need fixing as well?

NL80211_ATTR_HT_CAPABILITY in particular *has* a policy entry.

> Well, it's more obvious now.  Do you also need to check the length when
> doing code like this code from nl80211_set_station:
> 
> 	if (info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL])
> 		params.listen_interval =
> 		    nla_get_u16(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL]);
> 
> In other words, is it safe to assume you have 16 bits, or could a broken
> user-space pass in a single byte and screw this up?

There's the policy that validates it.


>  From 20.1.1 of the 802.11n spec:
> 
> "An HT non-AP STA shall support all equal modulation (EQM) rates for one spatial stream (MCSs 0 through
> 7) using 20 MHz channel width. An HT AP shall support all EQM rates for one and two spatial streams
> (MCSs 0 through 15) using 20 MHz channel width."
> 
> That is why I wrote that code as I did, but perhaps I misunderstand that section of
> the docs.

No, that makes some sense, I wasn't aware of that restriction.

johannes

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Johannes Berg]

On Wed, 2011-11-02 at 09:34 -0700, Ben Greear wrote:

> > You misunderstood -- I said fullmac drivers, not differences between
> > ath9k, iwlwifi etc. Other cfg80211 drivers, not other mac80211 drivers.
> 
> Can you at least point me to some existing code that does a similar
> check?  I have no idea of what a fullmac driver even does.  I can test
> and eventually somewhat understand the paths for ath9k, but I have no
> ability to test fullmac (as far as I know).

Well ideally you have two patches -- one for cfg80211, and one for
mac80211. Then you can test the cfg80211 one w/o mac80211 patches, so it
behaves as though it didn't support it, and with the mac80211 patch it
sets the flag to support it. Just think about drivers like ath6kl.

johannes

Re: [wireless-next PATCH 3/5] wifi: Allow overriding some HT information.

[Ben Greear]

On 11/02/2011 10:49 AM, Johannes Berg wrote:
> On Wed, 2011-11-02 at 09:59 -0700, Ben Greear wrote:
>
>>> No. Neither of these can ever be increased so it's not that simple. And
>>> making it smaller is always possible since it's just advertising.
>>> Presumably you do understand the reasons for this advertising/these
>>> restrictions?
>>
>> It seems that a driver might default to a mid-range value for the MPDU values
>> because it is somehow 'better' for whatever reason, and yet it might still
>> support larger values if the user desires, perhaps because in specific
>> scenarios larger values are better.  Ath9k does set to the max value,
>> so if we do a per-driver capabilities flag as I did in v2 then we
>> are safe.
>
> Then the proper way to do that would be to not have a flag, but a max it
> can be set to. However, I see no reason it should default to a mid-range
> value?! iwlwifi for example needs to allocate enough space but ... I
> don't get it. What's wrong with simply not allowing to increase, only
> decrease?

Ok, I'll work on allowing the value to only be decreased.
I should be able to compare against whatever the hardware set in
the channel ht-info I think.

>>>>           /* add attributes here, update the policy in nl80211.c */
>>
>> I copied some of that code from nl80211_set_station, which appears to
>> also forget to check the length for the NL80211_ATTR_HT_CAPABILITY
>> object.  Is there some reason why it doesn't need to check, or does
>> that code need fixing as well?
>
> NL80211_ATTR_HT_CAPABILITY in particular *has* a policy entry.

Ahh, I didn't realize that's what was meant by policy.  Mind if
I change that comment to something like what is below?

/* add attributes here, update the nl80211_policy array in nl80211.c */


>>    From 20.1.1 of the 802.11n spec:
>>
>> "An HT non-AP STA shall support all equal modulation (EQM) rates for one spatial stream (MCSs 0 through
>> 7) using 20 MHz channel width. An HT AP shall support all EQM rates for one and two spatial streams
>> (MCSs 0 through 15) using 20 MHz channel width."
>>
>> That is why I wrote that code as I did, but perhaps I misunderstand that section of
>> the docs.
>
> No, that makes some sense, I wasn't aware of that restriction.

Well, personally it seems like a lame restriction, and at least hostapd and ath9k will
deal fine with a station advertising less than that, but probably best to
stick with the spec if possible.

Thanks,
Ben


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

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Ben Greear]

On 11/02/2011 10:51 AM, Johannes Berg wrote:
> On Wed, 2011-11-02 at 09:34 -0700, Ben Greear wrote:
>
>>> You misunderstood -- I said fullmac drivers, not differences between
>>> ath9k, iwlwifi etc. Other cfg80211 drivers, not other mac80211 drivers.
>>
>> Can you at least point me to some existing code that does a similar
>> check?  I have no idea of what a fullmac driver even does.  I can test
>> and eventually somewhat understand the paths for ath9k, but I have no
>> ability to test fullmac (as far as I know).
>
> Well ideally you have two patches -- one for cfg80211, and one for
> mac80211. Then you can test the cfg80211 one w/o mac80211 patches, so it
> behaves as though it didn't support it, and with the mac80211 patch it
> sets the flag to support it. Just think about drivers like ath6kl.

I think I made at least most of the other changes you were asking
for, but I'm still baffled about what to do about fullmac drivers.

Based on the comment above, if I simply left out the mac80211 stuff
then the new values passed in to the associate/connect logic will just
be ignored.

So, I suppose the fullmac drivers will just silently ignore the new
variables as well.  I looked, but didn't figure out where fullmac
connects into the cfg80211 logic.  If I can find it, then I could
add explicit checks for the new variables and return failure if
they are set..but I'm not sure that is any better than just silently
ignoring them anyway.

Anyway, hopefully the 3 v3 patches are closer to what you are
looking for.  They seem to test out fine for me, though I never
see AMSDU go to max value.  Maybe ath9k just doesn't support
that by default, as I didn't see it on the stock wireless-testing
code either...or maybe my AP or something else is weird.

I'll post the hostap patches when the kernel side is resolved.
They are a lot smaller now that everything goes through
the connect/associate logic.

Thanks,
Ben


>
> johannes


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

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Johannes Berg]

On Wed, 2011-11-02 at 23:04 -0700, Ben Greear wrote:

> I think I made at least most of the other changes you were asking
> for, but I'm still baffled about what to do about fullmac drivers.
> 
> Based on the comment above, if I simply left out the mac80211 stuff
> then the new values passed in to the associate/connect logic will just
> be ignored.
> 
> So, I suppose the fullmac drivers will just silently ignore the new
> variables as well.  I looked, but didn't figure out where fullmac
> connects into the cfg80211 logic.  If I can find it, then I could
> add explicit checks for the new variables and return failure if
> they are set..but I'm not sure that is any better than just silently
> ignoring them anyway.

So I think just ignoring it would be a bad idea, because you have no way
to know whether or not the values were used afterwards. That might be
sufficient for you right now, but typically becomes a support problem at
some point because people try it and can't figure out what part broke.
That's why I think silently ignoring something like that is not a good
idea.

Hence the typical handling of this with a wiphy flag that you set and if
it's unset but userspace is requesting this you reject the command.

johannes

Re: [wireless-next PATCH 3/5] wifi: Allow overriding some HT information.

[Johannes Berg]

On Wed, 2011-11-02 at 11:03 -0700, Ben Greear wrote:

> >>>>           /* add attributes here, update the policy in nl80211.c */
> >>
> >> I copied some of that code from nl80211_set_station, which appears to
> >> also forget to check the length for the NL80211_ATTR_HT_CAPABILITY
> >> object.  Is there some reason why it doesn't need to check, or does
> >> that code need fixing as well?
> >
> > NL80211_ATTR_HT_CAPABILITY in particular *has* a policy entry.
> 
> Ahh, I didn't realize that's what was meant by policy.  Mind if
> I change that comment to something like what is below?
> 
> /* add attributes here, update the nl80211_policy array in nl80211.c */

That seems reasonable, though there are multiple policies for
differently nested attributes, but I guess anyone who understands that
will get far enough anyway :-)

FWIW, I just found that the exact example you pointed out had a bug --
NLA_BINARY enforces a *max*, not *min* length. There's a patch in flight
to fix it.

johannes

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Ben Greear]

On 11/03/2011 01:30 AM, Johannes Berg wrote:
> On Wed, 2011-11-02 at 23:04 -0700, Ben Greear wrote:
>
>> I think I made at least most of the other changes you were asking
>> for, but I'm still baffled about what to do about fullmac drivers.
>>
>> Based on the comment above, if I simply left out the mac80211 stuff
>> then the new values passed in to the associate/connect logic will just
>> be ignored.
>>
>> So, I suppose the fullmac drivers will just silently ignore the new
>> variables as well.  I looked, but didn't figure out where fullmac
>> connects into the cfg80211 logic.  If I can find it, then I could
>> add explicit checks for the new variables and return failure if
>> they are set..but I'm not sure that is any better than just silently
>> ignoring them anyway.
>
> So I think just ignoring it would be a bad idea, because you have no way
> to know whether or not the values were used afterwards. That might be
> sufficient for you right now, but typically becomes a support problem at
> some point because people try it and can't figure out what part broke.
> That's why I think silently ignoring something like that is not a good
> idea.
>
> Hence the typical handling of this with a wiphy flag that you set and if
> it's unset but userspace is requesting this you reject the command.

So back to the capabilities flag like I added in the -v2 patch?
Do you want one flag for each thing (set-mcs, disable-ht,
disable-ht40, set-mpdu, set-msdu), or maybe one flag for all of
this:  set-ht-cap

My opinion remains that we should silently ignore un-supported
values..this way user-space works with the same config on old and new
kernels.  In future patches, we can report the actual settings via
netlink or similar.

But, I can add logic in user-space to detect kernel versions and
such and deal with this.

So, please tell me how you want it done.  If it's capabilities flags,
that is fine with me.

Thanks,
Ben

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

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Johannes Berg]

On Thu, 2011-11-03 at 11:17 -0700, Ben Greear wrote:

> So back to the capabilities flag like I added in the -v2 patch?
> Do you want one flag for each thing (set-mcs, disable-ht,
> disable-ht40, set-mpdu, set-msdu), or maybe one flag for all of
> this:  set-ht-cap

I think maybe a single one would be sufficient, but you'd still have no
way of knowing what is actually supported for changing. Maybe you could
advertise an ht_mask of things that can be changed?

> My opinion remains that we should silently ignore un-supported
> values..this way user-space works with the same config on old and new
> kernels.  In future patches, we can report the actual settings via
> netlink or similar.
> 
> But, I can add logic in user-space to detect kernel versions and
> such and deal with this.

Then you also wouldn't have to muck around with kernel version detection
etc. you could just query the mask of things that can be changed.

johannes

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Ben Greear]

On 11/04/2011 07:42 AM, Johannes Berg wrote:
> On Thu, 2011-11-03 at 11:17 -0700, Ben Greear wrote:
>
>> So back to the capabilities flag like I added in the -v2 patch?
>> Do you want one flag for each thing (set-mcs, disable-ht,
>> disable-ht40, set-mpdu, set-msdu), or maybe one flag for all of
>> this:  set-ht-cap
>
> I think maybe a single one would be sufficient, but you'd still have no
> way of knowing what is actually supported for changing. Maybe you could
> advertise an ht_mask of things that can be changed?

That seems feasible, though it still won't help with the valid ranges
for mpdu-density, for instance.

How about if I add some way to query this, but leave the code loose
in that it won't fail if someone tries to set a value that isn't
supported.  That way, user-space can be lazy if it wants, but
can also get the details if it cares.

To get this info, I'm going to have to add a new driver API, as far
as I can tell, and I only have the ability to deal with ath9k, so
that will be the only driver that reports the mask.  Of course,
others could modify their drivers as they wish.

Thanks,
Ben

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

Re: [wireless-next PATCH 1/5] mac80211: Support forcing station to disable 11n.

[Johannes Berg]

On Fri, 2011-11-04 at 09:11 -0700, Ben Greear wrote:

> > I think maybe a single one would be sufficient, but you'd still have no
> > way of knowing what is actually supported for changing. Maybe you could
> > advertise an ht_mask of things that can be changed?
> 
> That seems feasible, though it still won't help with the valid ranges
> for mpdu-density, for instance.

Well I still think that reducing the density is not feasible, so
userspace can query the HT info about the min and the mask to see if
what it can do.

> How about if I add some way to query this, but leave the code loose
> in that it won't fail if someone tries to set a value that isn't
> supported.  That way, user-space can be lazy if it wants, but
> can also get the details if it cares.

Yeah, that seems like it would be a good way of doing it, although I
think I'd reject it if there's no such configuration possible at all.

> To get this info, I'm going to have to add a new driver API, as far
> as I can tell, and I only have the ability to deal with ath9k, so
> that will be the only driver that reports the mask.  Of course,
> others could modify their drivers as they wish.

I think you just have to add the mask to the wiphy struct somewhere. And
the mask wouldn't be specific to ath9k since any mac80211 driver would
be able to deal with it, just no non-mac80211 driver.

You could just have a pointer to the mask in the wiphy struct and if
NULL then it's not allowed, if non-NULL the mask indicates what can be
changed.

johannes

Re: [wireless-next PATCH 5/5] wifi-debugfs: Fix AMSDU rate printout.

[Ben Greear]

On 10/27/2011 10:11 PM, greearb@candelatech.com wrote:
> From: Ben Greear<greearb@candelatech.com>
>
> It was flipped.  See section 7.3.2.56 of the 802.11n
> spec for details.

This patch is stand-alone, even though it was originally
part of a series I posted.  Do I need to re-post as 1/1
(it's not yet in wireless-testing tree).

Thanks,
Ben

>
> Signed-off-by: Ben Greear<greearb@candelatech.com>
> ---
> :100644 100644 1ceec86... 4310431... M	net/mac80211/debugfs_sta.c
>   net/mac80211/debugfs_sta.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
> index 1ceec86..4310431 100644
> --- a/net/mac80211/debugfs_sta.c
> +++ b/net/mac80211/debugfs_sta.c
> @@ -274,9 +274,9 @@ static ssize_t sta_ht_capa_read(struct file *file, char __user *userbuf,
>
>   		PRINT_HT_CAP((htc->cap&  BIT(10)), "HT Delayed Block Ack");
>
> -		PRINT_HT_CAP((htc->cap&  BIT(11)), "Max AMSDU length: "
> -			     "3839 bytes");
>   		PRINT_HT_CAP(!(htc->cap&  BIT(11)), "Max AMSDU length: "
> +			     "3839 bytes");
> +		PRINT_HT_CAP((htc->cap&  BIT(11)), "Max AMSDU length: "
>   			     "7935 bytes");
>
>   		/*


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

Re: [wireless-next PATCH 5/5] wifi-debugfs: Fix AMSDU rate printout.

[John W. Linville]

On Thu, Nov 17, 2011 at 09:49:38AM -0800, Ben Greear wrote:
> On 10/27/2011 10:11 PM, greearb@candelatech.com wrote:
> >From: Ben Greear<greearb@candelatech.com>
> >
> >It was flipped.  See section 7.3.2.56 of the 802.11n
> >spec for details.
> 
> This patch is stand-alone, even though it was originally
> part of a series I posted.  Do I need to re-post as 1/1
> (it's not yet in wireless-testing tree).
> 
> Thanks,
> Ben

Sorry, I dropped the entire series.  Please repost this one
individually.

Thanks,

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.