All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 1/2] wireless:  Support ht-capabilities over-rides.
@ 2011-11-08 19:36 greearb
  2011-11-08 19:36 ` [PATCH v8 2/2] mac80211: Support ht-cap over-rides greearb
  2011-11-08 20:07 ` [PATCH v8 1/2] wireless: Support ht-capabilities over-rides Johannes Berg
  0 siblings, 2 replies; 20+ messages in thread
From: greearb @ 2011-11-08 19:36 UTC (permalink / raw)
  To: linux-wireless; +Cc: Ben Greear

From: Ben Greear <greearb@candelatech.com>

This allows users to disable features such as HT, HT40,
and to modify the MCS, AMPDU, and AMSDU settings for
drivers that support it.

The MCS, AMPDU, and AMSDU features that may be disabled are
are reported in the phy-info netlink message as a mask.

Attemping to disable features that are not supported will
take no affect, but will not return errors.  This is to aid
backwards compatibility in user-space apps that may not be
clever enough to deal with parsing the the capabilities mask.

This patch only enables the infrastructure.  An additional
patch will enable the feature in mac80211.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

v8:  Fix compile bugs from v7.  Must have compiled the
     wrong tree while testing that.

:100644 100644 8049bf7... 34c3973... M	include/linux/nl80211.h
:100644 100644 92cf1c2... 5598b91... M	include/net/cfg80211.h
:100644 100644 b9ec306... 8357ed6... M	net/wireless/core.h
:100644 100644 21fc970... bf538e1... M	net/wireless/mlme.c
:100644 100644 b3a476f... 31f17c8... M	net/wireless/nl80211.c
:100644 100644 6e86d5a... ed9d0e6... M	net/wireless/sme.c
 include/linux/nl80211.h |   15 +++++++++++++++
 include/net/cfg80211.h  |   28 ++++++++++++++++++++++++++++
 net/wireless/core.h     |   10 ++++++++--
 net/wireless/mlme.c     |   37 ++++++++++++++++++++++++++++++++++---
 net/wireless/nl80211.c  |   44 +++++++++++++++++++++++++++++++++++++++++++-
 net/wireless/sme.c      |    7 ++++++-
 6 files changed, 134 insertions(+), 7 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 8049bf7..34c3973 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1109,6 +1109,18 @@ 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_HT:  Force HT capable interfaces to disable
+ *      this feature.  Currently, only supported in mac80211 drivers.
+ * @NL80211_ATTR_HT_CAPABILITY_MASK: Specify which bits of the
+ *      ATTR_HT_CAPABILITY to which attention should be paid.
+ *      Currently, only mac80211 NICs support this feature.
+ *      The values that may be configured are:
+ *       MCS rates, MAX-AMSDU, HT-20-40 and HT_CAP_SGI_40
+ *       AMPDU density and AMPDU factor.
+ *      All values are treated as suggestions and may be ignored
+ *      by the driver as required.  The actual values may be seen in
+ *      the station debugfs ht_caps file.
+ *
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
  */
@@ -1337,6 +1349,9 @@ enum nl80211_attrs {
 	NL80211_ATTR_TDLS_SUPPORT,
 	NL80211_ATTR_TDLS_EXTERNAL_SETUP,
 
+	NL80211_ATTR_DISABLE_HT,
+	NL80211_ATTR_HT_CAPABILITY_MASK,
+
 	/* 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..5598b91 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1036,6 +1036,15 @@ struct cfg80211_auth_request {
 };
 
 /**
+ * enum cfg80211_assoc_req_flags - Over-ride default behaviour in association.
+ *
+ * @ASSOC_REQ_DISABLE_HT:  Disable HT (802.11n)
+ */
+enum cfg80211_assoc_req_flags {
+	ASSOC_REQ_DISABLE_HT		= BIT(0),
+};
+
+/**
  * struct cfg80211_assoc_request - (Re)Association request data
  *
  * This structure provides information needed to complete IEEE 802.11
@@ -1046,6 +1055,10 @@ struct cfg80211_auth_request {
  * @use_mfp: Use management frame protection (IEEE 802.11w) in this association
  * @crypto: crypto settings
  * @prev_bssid: previous BSSID, if not %NULL use reassociate frame
+ * @flags:  See &enum cfg80211_assoc_req_flags
+ * @ht_capa:  HT Capabilities over-rides.  Values set in ht_capa_mask
+ *   will be used in ht_capa.  Un-supported values will be ignored.
+ * @ht_capa_mask:  The bits of ht_capa which are to be used.
  */
 struct cfg80211_assoc_request {
 	struct cfg80211_bss *bss;
@@ -1053,6 +1066,9 @@ struct cfg80211_assoc_request {
 	size_t ie_len;
 	struct cfg80211_crypto_settings crypto;
 	bool use_mfp;
+	u32 flags;
+	struct ieee80211_ht_cap ht_capa;
+	struct ieee80211_ht_cap ht_capa_mask;
 };
 
 /**
@@ -1151,6 +1167,10 @@ struct cfg80211_ibss_params {
  * @key_len: length of WEP key for shared key authentication
  * @key_idx: index of WEP key for shared key authentication
  * @key: WEP key for shared key authentication
+ * @flags:  See &enum cfg80211_assoc_req_flags
+ * @ht_capa:  HT Capabilities over-rides.  Values set in ht_capa_mask
+ *   will be used in ht_capa.  Un-supported values will be ignored.
+ * @ht_capa_mask:  The bits of ht_capa which are to be used.
  */
 struct cfg80211_connect_params {
 	struct ieee80211_channel *channel;
@@ -1164,6 +1184,9 @@ struct cfg80211_connect_params {
 	struct cfg80211_crypto_settings crypto;
 	const u8 *key;
 	u8 key_len, key_idx;
+	u32 flags;
+	struct ieee80211_ht_cap ht_capa;
+	struct ieee80211_ht_cap ht_capa_mask;
 };
 
 /**
@@ -1903,6 +1926,9 @@ struct wiphy_wowlan_support {
  *	may request, if implemented.
  *
  * @wowlan: WoWLAN support information
+ *
+ * @ht_capa_mod_mask:  Specify what ht_cap values can be over-ridden.
+ *	If null, then none can be over-ridden.
  */
 struct wiphy {
 	/* assign these fields before you register the wiphy */
@@ -1983,6 +2009,8 @@ struct wiphy {
 	/* dir in debugfs: ieee80211/<wiphyname> */
 	struct dentry *debugfsdir;
 
+	const struct ieee80211_ht_cap *ht_capa_mod_mask;
+
 #ifdef CONFIG_NET_NS
 	/* the network namespace this phy lives in currently */
 	struct net *_net;
diff --git a/net/wireless/core.h b/net/wireless/core.h
index b9ec306..8357ed6 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -339,13 +339,17 @@ int __cfg80211_mlme_assoc(struct cfg80211_registered_device *rdev,
 			  const u8 *bssid, const u8 *prev_bssid,
 			  const u8 *ssid, int ssid_len,
 			  const u8 *ie, int ie_len, bool use_mfp,
-			  struct cfg80211_crypto_settings *crypt);
+			  struct cfg80211_crypto_settings *crypt,
+			  u32 assoc_flags, struct ieee80211_ht_cap *ht_capa,
+			  struct ieee80211_ht_cap *ht_capa_mask);
 int cfg80211_mlme_assoc(struct cfg80211_registered_device *rdev,
 			struct net_device *dev, struct ieee80211_channel *chan,
 			const u8 *bssid, const u8 *prev_bssid,
 			const u8 *ssid, int ssid_len,
 			const u8 *ie, int ie_len, bool use_mfp,
-			struct cfg80211_crypto_settings *crypt);
+			struct cfg80211_crypto_settings *crypt,
+			u32 assoc_flags, struct ieee80211_ht_cap *ht_capa,
+			struct ieee80211_ht_cap *ht_capa_mask);
 int __cfg80211_mlme_deauth(struct cfg80211_registered_device *rdev,
 			   struct net_device *dev, const u8 *bssid,
 			   const u8 *ie, int ie_len, u16 reason,
@@ -377,6 +381,8 @@ int cfg80211_mlme_mgmt_tx(struct cfg80211_registered_device *rdev,
 			  bool channel_type_valid, unsigned int wait,
 			  const u8 *buf, size_t len, bool no_cck,
 			  u64 *cookie);
+void cfg80211_oper_and_ht_capa(struct ieee80211_ht_cap *ht_capa,
+			       const struct ieee80211_ht_cap *ht_capa_mask);
 
 /* SME */
 int __cfg80211_connect(struct cfg80211_registered_device *rdev,
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 21fc970..bf538e1 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -501,13 +501,32 @@ int cfg80211_mlme_auth(struct cfg80211_registered_device *rdev,
 	return err;
 }
 
+/*  Do a logical ht_capa &= ht_capa_mask.  */
+void cfg80211_oper_and_ht_capa(struct ieee80211_ht_cap *ht_capa,
+			       const struct ieee80211_ht_cap *ht_capa_mask)
+{
+	int i;
+	u8 *p1, *p2;
+	if (!ht_capa_mask) {
+		memset(ht_capa, 0, sizeof(*ht_capa));
+		return;
+	}
+
+	p1 = (u8*)(ht_capa);
+	p2 = (u8*)(ht_capa_mask);
+	for (i = 0; i<sizeof(*ht_capa); i++)
+		p1[i] &= p2[i];
+}
+
 int __cfg80211_mlme_assoc(struct cfg80211_registered_device *rdev,
 			  struct net_device *dev,
 			  struct ieee80211_channel *chan,
 			  const u8 *bssid, const u8 *prev_bssid,
 			  const u8 *ssid, int ssid_len,
 			  const u8 *ie, int ie_len, bool use_mfp,
-			  struct cfg80211_crypto_settings *crypt)
+			  struct cfg80211_crypto_settings *crypt,
+			  u32 assoc_flags, struct ieee80211_ht_cap *ht_capa,
+			  struct ieee80211_ht_cap *ht_capa_mask)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	struct cfg80211_assoc_request req;
@@ -537,6 +556,15 @@ int __cfg80211_mlme_assoc(struct cfg80211_registered_device *rdev,
 	memcpy(&req.crypto, crypt, sizeof(req.crypto));
 	req.use_mfp = use_mfp;
 	req.prev_bssid = prev_bssid;
+	req.flags = assoc_flags;
+	if (ht_capa)
+		memcpy(&req.ht_capa, ht_capa, sizeof(req.ht_capa));
+	if (ht_capa_mask)
+		memcpy(&req.ht_capa_mask, ht_capa_mask,
+		       sizeof(req.ht_capa_mask));
+	cfg80211_oper_and_ht_capa(&req.ht_capa_mask,
+				  rdev->wiphy.ht_capa_mod_mask);
+
 	req.bss = cfg80211_get_bss(&rdev->wiphy, chan, bssid, ssid, ssid_len,
 				   WLAN_CAPABILITY_ESS, WLAN_CAPABILITY_ESS);
 	if (!req.bss) {
@@ -574,14 +602,17 @@ int cfg80211_mlme_assoc(struct cfg80211_registered_device *rdev,
 			const u8 *bssid, const u8 *prev_bssid,
 			const u8 *ssid, int ssid_len,
 			const u8 *ie, int ie_len, bool use_mfp,
-			struct cfg80211_crypto_settings *crypt)
+			struct cfg80211_crypto_settings *crypt,
+			u32 assoc_flags, struct ieee80211_ht_cap *ht_capa,
+			struct ieee80211_ht_cap *ht_capa_mask)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	int err;
 
 	wdev_lock(wdev);
 	err = __cfg80211_mlme_assoc(rdev, dev, chan, bssid, prev_bssid,
-				    ssid, ssid_len, ie, ie_len, use_mfp, crypt);
+				    ssid, ssid_len, ie, ie_len, use_mfp, crypt,
+				    assoc_flags, ht_capa, ht_capa_mask);
 	wdev_unlock(wdev);
 
 	return err;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b3a476f..31f17c8 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -196,6 +196,10 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
 	[NL80211_ATTR_TDLS_OPERATION] = { .type = NLA_U8 },
 	[NL80211_ATTR_TDLS_SUPPORT] = { .type = NLA_FLAG },
 	[NL80211_ATTR_TDLS_EXTERNAL_SETUP] = { .type = NLA_FLAG },
+	[NL80211_ATTR_DISABLE_HT] = { .type = NLA_FLAG },
+	[NL80211_ATTR_HT_CAPABILITY_MASK] = {
+		.len = NL80211_HT_CAPABILITY_LEN
+	},
 };
 
 /* policy for the key attributes */
@@ -1007,6 +1011,11 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
 	if (nl80211_put_iface_combinations(&dev->wiphy, msg))
 		goto nla_put_failure;
 
+	if (dev->wiphy.ht_capa_mod_mask)
+		NLA_PUT(msg, NL80211_ATTR_HT_CAPABILITY_MASK,
+			sizeof(*dev->wiphy.ht_capa_mod_mask),
+			dev->wiphy.ht_capa_mod_mask);
+
 	return genlmsg_end(msg, hdr);
 
  nla_put_failure:
@@ -4359,6 +4368,9 @@ static int nl80211_associate(struct sk_buff *skb, struct genl_info *info)
 	const u8 *bssid, *ssid, *ie = NULL, *prev_bssid = NULL;
 	int err, ssid_len, ie_len = 0;
 	bool use_mfp = false;
+	u32 flags = 0;
+	struct ieee80211_ht_cap *ht_capa = NULL;
+	struct ieee80211_ht_cap *ht_capa_mask = NULL;
 
 	if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE]))
 		return -EINVAL;
@@ -4402,11 +4414,25 @@ static int nl80211_associate(struct sk_buff *skb, struct genl_info *info)
 	if (info->attrs[NL80211_ATTR_PREV_BSSID])
 		prev_bssid = nla_data(info->attrs[NL80211_ATTR_PREV_BSSID]);
 
+	if (nla_get_flag(info->attrs[NL80211_ATTR_DISABLE_HT]))
+		flags |= ASSOC_REQ_DISABLE_HT;
+
+	if (info->attrs[NL80211_ATTR_HT_CAPABILITY_MASK])
+		ht_capa_mask =
+			nla_data(info->attrs[NL80211_ATTR_HT_CAPABILITY_MASK]);
+
+	if (info->attrs[NL80211_ATTR_HT_CAPABILITY]) {
+		if (!ht_capa_mask)
+			return -EINVAL;
+		ht_capa = nla_data(info->attrs[NL80211_ATTR_HT_CAPABILITY]);
+	}
+
 	err = nl80211_crypto_settings(rdev, info, &crypto, 1);
 	if (!err)
 		err = cfg80211_mlme_assoc(rdev, dev, chan, bssid, prev_bssid,
 					  ssid, ssid_len, ie, ie_len, use_mfp,
-					  &crypto);
+					  &crypto, flags, ht_capa,
+					  ht_capa_mask);
 
 	return err;
 }
@@ -4896,6 +4922,22 @@ static int nl80211_connect(struct sk_buff *skb, struct genl_info *info)
 			return PTR_ERR(connkeys);
 	}
 
+	if (nla_get_flag(info->attrs[NL80211_ATTR_DISABLE_HT]))
+		connect.flags |= ASSOC_REQ_DISABLE_HT;
+
+	if (info->attrs[NL80211_ATTR_HT_CAPABILITY_MASK])
+		memcpy(&connect.ht_capa_mask,
+		       nla_data(info->attrs[NL80211_ATTR_HT_CAPABILITY_MASK]),
+		       sizeof(connect.ht_capa_mask));
+
+	if (info->attrs[NL80211_ATTR_HT_CAPABILITY]) {
+		if (!info->attrs[NL80211_ATTR_HT_CAPABILITY_MASK])
+			return -EINVAL;
+		memcpy(&connect.ht_capa,
+		       nla_data(info->attrs[NL80211_ATTR_HT_CAPABILITY]),
+		       sizeof(connect.ht_capa));
+	}
+
 	err = cfg80211_connect(rdev, dev, &connect, connkeys);
 	if (err)
 		kfree(connkeys);
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 6e86d5a..ed9d0e6 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -189,7 +189,9 @@ static int cfg80211_conn_do_work(struct wireless_dev *wdev)
 					    prev_bssid,
 					    params->ssid, params->ssid_len,
 					    params->ie, params->ie_len,
-					    false, &params->crypto);
+					    false, &params->crypto,
+					    params->flags, &params->ht_capa,
+					    &params->ht_capa_mask);
 		if (err)
 			__cfg80211_mlme_deauth(rdev, wdev->netdev, params->bssid,
 					       NULL, 0,
@@ -773,6 +775,9 @@ int __cfg80211_connect(struct cfg80211_registered_device *rdev,
 		wdev->connect_keys = NULL;
 	}
 
+	cfg80211_oper_and_ht_capa(&connect->ht_capa_mask,
+				  rdev->wiphy.ht_capa_mod_mask);
+
 	if (connkeys && connkeys->def >= 0) {
 		int idx;
 		u32 cipher;
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 19:36 [PATCH v8 1/2] wireless: Support ht-capabilities over-rides greearb
@ 2011-11-08 19:36 ` greearb
  2011-11-08 20:09   ` Johannes Berg
  2011-11-08 20:12   ` Johannes Berg
  2011-11-08 20:07 ` [PATCH v8 1/2] wireless: Support ht-capabilities over-rides Johannes Berg
  1 sibling, 2 replies; 20+ messages in thread
From: greearb @ 2011-11-08 19:36 UTC (permalink / raw)
  To: linux-wireless; +Cc: Ben Greear

From: Ben Greear <greearb@candelatech.com>

This implements ht-cap over-rides for mac80211 drivers.
HT may be disabled, making an /a/b/g/n station act like an
a/b/g station.  HT40 may be disabled forcing the station to
be HT20 even if the AP and local hardware support HT40.

MAX-AMSDU may be disabled.
AMPDU-Density may be increased.
AMPDU-Factor may be decreased.

This has been successfully tested with ath9k using patched
wpa_supplicant and iw.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

v8:  No changes from previous.

:100644 100644 48363c3... 25ea406... M	include/linux/ieee80211.h
:100644 100644 a9ded52... 7f4389e... M	net/mac80211/cfg.c
:100644 100644 f80a35c... 47b89bb... M	net/mac80211/ht.c
:100644 100644 ea10a51... 6afa409... M	net/mac80211/ieee80211_i.h
:100644 100644 d4ee6d2... 7be5ad8... M	net/mac80211/main.c
:100644 100644 2fbd652... 84d6943... M	net/mac80211/mlme.c
:100644 100644 6c53b6d... 3f318df... M	net/mac80211/work.c
 include/linux/ieee80211.h  |    6 +++
 net/mac80211/cfg.c         |    2 +-
 net/mac80211/ht.c          |   90 +++++++++++++++++++++++++++++++++++++++++++-
 net/mac80211/ieee80211_i.h |   10 ++++-
 net/mac80211/main.c        |   14 +++++++
 net/mac80211/mlme.c        |   12 +++++-
 net/mac80211/work.c        |   35 ++++++++++++-----
 7 files changed, 154 insertions(+), 15 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 48363c3..25ea406 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -906,6 +906,12 @@ struct ieee80211_mcs_info {
 #define IEEE80211_HT_MCS_TX_MAX_STREAMS_SHIFT	2
 #define		IEEE80211_HT_MCS_TX_MAX_STREAMS	4
 #define IEEE80211_HT_MCS_TX_UNEQUAL_MODULATION	0x10
+/*
+ * Stations supporting 802.11n are required to support
+ * at least the first 8 MCS rates.  See section 7.3.2.56.4
+ * and 20.1.1 of the 802.11n spec.
+ */
+#define IEEE80211_HT_MCS_REQ_RATES_STA		8
 
 /*
  * 802.11n D5.0 20.3.5 / 20.6 says:
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a9ded52..7f4389e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -778,7 +778,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..47b89bb 100644
--- a/net/mac80211/ht.c
+++ b/net/mac80211/ht.c
@@ -18,7 +18,89 @@
 #include "ieee80211_i.h"
 #include "rate.h"
 
-void ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_supported_band *sband,
+bool ieee80111_cfg_override_disables_ht40(struct ieee80211_sub_if_data *sdata)
+{
+	if ((sdata->u.mgd.ht_capa_mask.cap_info &
+	     IEEE80211_HT_CAP_SUP_WIDTH_20_40) &&
+	    !(sdata->u.mgd.ht_capa.cap_info &
+	      IEEE80211_HT_CAP_SUP_WIDTH_20_40))
+		return true;
+	return false;
+}
+
+void __check_htcap_disable(struct ieee80211_sub_if_data *sdata,
+			   struct ieee80211_sta_ht_cap *ht_cap,
+			   u16 flag)
+{
+	if (sdata->u.mgd.ht_capa_mask.cap_info & flag) {
+		if (!(sdata->u.mgd.ht_capa.cap_info & flag))
+			ht_cap->cap &= ~flag;
+	}
+}
+
+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->u.mgd.ht_capa.mcs.rx_mask);
+	u8 *smask = (u8 *)(&sdata->u.mgd.ht_capa_mask.mcs.rx_mask);
+	int i;
+
+	/* NOTE:  If you add more over-rides here, update register_hw
+	 * ht_capa_mod_msk logic in main.c as well.
+	 */
+
+	/* check for HT over-rides, MCS rates first. */
+	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? */
+	__check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SUP_WIDTH_20_40);
+	__check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SGI_40);
+
+	/* Allow user to disable the max-AMSDU bit. */
+	__check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_MAX_AMSDU);
+
+	/* Allow user to decrease AMPDU factor */
+	if (sdata->u.mgd.ht_capa_mask.ampdu_params_info &
+	    IEEE80211_HT_AMPDU_PARM_FACTOR) {
+		u16 n = sdata->u.mgd.ht_capa.ampdu_params_info
+			& IEEE80211_HT_AMPDU_PARM_FACTOR;
+		if (n < ht_cap->ampdu_factor)
+			ht_cap->ampdu_factor = n;
+	}
+
+	/* Allow the user to increase AMPDU density. */
+	if (sdata->u.mgd.ht_capa_mask.ampdu_params_info &
+	    IEEE80211_HT_AMPDU_PARM_DENSITY) {
+		u16 n = (sdata->u.mgd.ht_capa.ampdu_params_info &
+			 IEEE80211_HT_AMPDU_PARM_DENSITY)
+			>> IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT;
+		if (n > ht_cap->ampdu_density)
+			ht_cap->ampdu_density = n;
+	}
+}
+
+
+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 +184,12 @@ 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 ea10a51..6afa409 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -443,6 +443,9 @@ struct ieee80211_if_managed {
 	 */
 	int rssi_min_thold, rssi_max_thold;
 	int last_ave_beacon_signal;
+	struct ieee80211_ht_cap ht_capa; /* configured ht-cap over-rides */
+	struct ieee80211_ht_cap ht_capa_mask; /* Valid parts of ht_capa */
+
 };
 
 struct ieee80211_if_ibss {
@@ -1179,7 +1182,12 @@ 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,
+bool ieee80111_cfg_override_disables_ht40(struct ieee80211_sub_if_data *sdata);
+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/main.c b/net/mac80211/main.c
index d4ee6d2..7be5ad8 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -560,6 +560,19 @@ ieee80211_default_mgmt_stypes[NUM_NL80211_IFTYPES] = {
 	},
 };
 
+static const struct ieee80211_ht_cap mac80211_ht_capa_mod_mask = {
+	.ampdu_params_info = IEEE80211_HT_AMPDU_PARM_FACTOR |
+			     IEEE80211_HT_AMPDU_PARM_DENSITY,
+
+	.cap_info = IEEE80211_HT_CAP_SUP_WIDTH_20_40 |
+		    IEEE80211_HT_CAP_MAX_AMSDU |
+		    IEEE80211_HT_CAP_SGI_40,
+	.mcs = {
+		.rx_mask = { 0xff, 0xff, 0xff, 0xff, 0xff,
+			     0xff, 0xff, 0xff, 0xff, 0xff, },
+	},
+};
+
 struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 					const struct ieee80211_ops *ops)
 {
@@ -628,6 +641,7 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 	local->user_power_level = -1;
 	local->uapsd_queues = IEEE80211_DEFAULT_UAPSD_QUEUES;
 	local->uapsd_max_sp_len = IEEE80211_DEFAULT_MAX_SP_LEN;
+	wiphy->ht_capa_mod_mask = &mac80211_ht_capa_mod_mask;
 
 	INIT_LIST_HEAD(&local->interfaces);
 
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 2fbd652..84d6943 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) &&
+		    !ieee80111_cfg_override_disables_ht40(sdata) &&
 		    (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) {
@@ -1603,7 +1604,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;
@@ -1972,7 +1973,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;
@@ -2630,6 +2631,13 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
 			ifmgd->flags |= IEEE80211_STA_DISABLE_11N;
 
 
+	if (req->flags & ASSOC_REQ_DISABLE_HT)
+		ifmgd->flags |= IEEE80211_STA_DISABLE_11N;
+
+	memcpy(&ifmgd->ht_capa, &req->ht_capa, sizeof(ifmgd->ht_capa));
+	memcpy(&ifmgd->ht_capa_mask, &req->ht_capa_mask,
+	       sizeof(ifmgd->ht_capa_mask));
+
 	if (req->ie && req->ie_len) {
 		memcpy(wk->ie, req->ie, req->ie_len);
 		wk->ie_len = req->ie_len;
diff --git a/net/mac80211/work.c b/net/mac80211/work.c
index 6c53b6d..3f318df 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,20 @@ 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 stations must
+	 * support at least the first 8 MCS rates.  See section 20.1.1
+	 * of the 802.11n spec for details.
+	 */
+	ieee80211_apply_htcap_overrides(sdata, &ht_cap,
+					IEEE80211_HT_MCS_REQ_RATES_STA);
+
+	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 +181,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 +371,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 */
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 1/2] wireless:  Support ht-capabilities over-rides.
  2011-11-08 19:36 [PATCH v8 1/2] wireless: Support ht-capabilities over-rides greearb
  2011-11-08 19:36 ` [PATCH v8 2/2] mac80211: Support ht-cap over-rides greearb
@ 2011-11-08 20:07 ` Johannes Berg
  1 sibling, 0 replies; 20+ messages in thread
From: Johannes Berg @ 2011-11-08 20:07 UTC (permalink / raw)
  To: greearb; +Cc: linux-wireless

On Tue, 2011-11-08 at 11:36 -0800, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> This allows users to disable features such as HT, HT40,
> and to modify the MCS, AMPDU, and AMSDU settings for
> drivers that support it.
> 
> The MCS, AMPDU, and AMSDU features that may be disabled are
> are reported in the phy-info netlink message as a mask.
> 
> Attemping to disable features that are not supported will
> take no affect, but will not return errors.  This is to aid
> backwards compatibility in user-space apps that may not be
> clever enough to deal with parsing the the capabilities mask.
> 
> This patch only enables the infrastructure.  An additional
> patch will enable the feature in mac80211.

Looks ok to me now.

> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
> 
> v8:  Fix compile bugs from v7.  Must have compiled the
>      wrong tree while testing that.
> 
> :100644 100644 8049bf7... 34c3973... M	include/linux/nl80211.h
> :100644 100644 92cf1c2... 5598b91... M	include/net/cfg80211.h
> :100644 100644 b9ec306... 8357ed6... M	net/wireless/core.h
> :100644 100644 21fc970... bf538e1... M	net/wireless/mlme.c
> :100644 100644 b3a476f... 31f17c8... M	net/wireless/nl80211.c
> :100644 100644 6e86d5a... ed9d0e6... M	net/wireless/sme.c
>  include/linux/nl80211.h |   15 +++++++++++++++
>  include/net/cfg80211.h  |   28 ++++++++++++++++++++++++++++
>  net/wireless/core.h     |   10 ++++++++--
>  net/wireless/mlme.c     |   37 ++++++++++++++++++++++++++++++++++---
>  net/wireless/nl80211.c  |   44 +++++++++++++++++++++++++++++++++++++++++++-
>  net/wireless/sme.c      |    7 ++++++-
>  6 files changed, 134 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> index 8049bf7..34c3973 100644
> --- a/include/linux/nl80211.h
> +++ b/include/linux/nl80211.h
> @@ -1109,6 +1109,18 @@ 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_HT:  Force HT capable interfaces to disable
> + *      this feature.  Currently, only supported in mac80211 drivers.
> + * @NL80211_ATTR_HT_CAPABILITY_MASK: Specify which bits of the
> + *      ATTR_HT_CAPABILITY to which attention should be paid.
> + *      Currently, only mac80211 NICs support this feature.
> + *      The values that may be configured are:
> + *       MCS rates, MAX-AMSDU, HT-20-40 and HT_CAP_SGI_40
> + *       AMPDU density and AMPDU factor.
> + *      All values are treated as suggestions and may be ignored
> + *      by the driver as required.  The actual values may be seen in
> + *      the station debugfs ht_caps file.
> + *
>   * @NL80211_ATTR_MAX: highest attribute number currently defined
>   * @__NL80211_ATTR_AFTER_LAST: internal use
>   */
> @@ -1337,6 +1349,9 @@ enum nl80211_attrs {
>  	NL80211_ATTR_TDLS_SUPPORT,
>  	NL80211_ATTR_TDLS_EXTERNAL_SETUP,
>  
> +	NL80211_ATTR_DISABLE_HT,
> +	NL80211_ATTR_HT_CAPABILITY_MASK,
> +
>  	/* 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..5598b91 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1036,6 +1036,15 @@ struct cfg80211_auth_request {
>  };
>  
>  /**
> + * enum cfg80211_assoc_req_flags - Over-ride default behaviour in association.
> + *
> + * @ASSOC_REQ_DISABLE_HT:  Disable HT (802.11n)
> + */
> +enum cfg80211_assoc_req_flags {
> +	ASSOC_REQ_DISABLE_HT		= BIT(0),
> +};
> +
> +/**
>   * struct cfg80211_assoc_request - (Re)Association request data
>   *
>   * This structure provides information needed to complete IEEE 802.11
> @@ -1046,6 +1055,10 @@ struct cfg80211_auth_request {
>   * @use_mfp: Use management frame protection (IEEE 802.11w) in this association
>   * @crypto: crypto settings
>   * @prev_bssid: previous BSSID, if not %NULL use reassociate frame
> + * @flags:  See &enum cfg80211_assoc_req_flags
> + * @ht_capa:  HT Capabilities over-rides.  Values set in ht_capa_mask
> + *   will be used in ht_capa.  Un-supported values will be ignored.
> + * @ht_capa_mask:  The bits of ht_capa which are to be used.
>   */
>  struct cfg80211_assoc_request {
>  	struct cfg80211_bss *bss;
> @@ -1053,6 +1066,9 @@ struct cfg80211_assoc_request {
>  	size_t ie_len;
>  	struct cfg80211_crypto_settings crypto;
>  	bool use_mfp;
> +	u32 flags;
> +	struct ieee80211_ht_cap ht_capa;
> +	struct ieee80211_ht_cap ht_capa_mask;
>  };
>  
>  /**
> @@ -1151,6 +1167,10 @@ struct cfg80211_ibss_params {
>   * @key_len: length of WEP key for shared key authentication
>   * @key_idx: index of WEP key for shared key authentication
>   * @key: WEP key for shared key authentication
> + * @flags:  See &enum cfg80211_assoc_req_flags
> + * @ht_capa:  HT Capabilities over-rides.  Values set in ht_capa_mask
> + *   will be used in ht_capa.  Un-supported values will be ignored.
> + * @ht_capa_mask:  The bits of ht_capa which are to be used.
>   */
>  struct cfg80211_connect_params {
>  	struct ieee80211_channel *channel;
> @@ -1164,6 +1184,9 @@ struct cfg80211_connect_params {
>  	struct cfg80211_crypto_settings crypto;
>  	const u8 *key;
>  	u8 key_len, key_idx;
> +	u32 flags;
> +	struct ieee80211_ht_cap ht_capa;
> +	struct ieee80211_ht_cap ht_capa_mask;
>  };
>  
>  /**
> @@ -1903,6 +1926,9 @@ struct wiphy_wowlan_support {
>   *	may request, if implemented.
>   *
>   * @wowlan: WoWLAN support information
> + *
> + * @ht_capa_mod_mask:  Specify what ht_cap values can be over-ridden.
> + *	If null, then none can be over-ridden.
>   */
>  struct wiphy {
>  	/* assign these fields before you register the wiphy */
> @@ -1983,6 +2009,8 @@ struct wiphy {
>  	/* dir in debugfs: ieee80211/<wiphyname> */
>  	struct dentry *debugfsdir;
>  
> +	const struct ieee80211_ht_cap *ht_capa_mod_mask;
> +
>  #ifdef CONFIG_NET_NS
>  	/* the network namespace this phy lives in currently */
>  	struct net *_net;
> diff --git a/net/wireless/core.h b/net/wireless/core.h
> index b9ec306..8357ed6 100644
> --- a/net/wireless/core.h
> +++ b/net/wireless/core.h
> @@ -339,13 +339,17 @@ int __cfg80211_mlme_assoc(struct cfg80211_registered_device *rdev,
>  			  const u8 *bssid, const u8 *prev_bssid,
>  			  const u8 *ssid, int ssid_len,
>  			  const u8 *ie, int ie_len, bool use_mfp,
> -			  struct cfg80211_crypto_settings *crypt);
> +			  struct cfg80211_crypto_settings *crypt,
> +			  u32 assoc_flags, struct ieee80211_ht_cap *ht_capa,
> +			  struct ieee80211_ht_cap *ht_capa_mask);
>  int cfg80211_mlme_assoc(struct cfg80211_registered_device *rdev,
>  			struct net_device *dev, struct ieee80211_channel *chan,
>  			const u8 *bssid, const u8 *prev_bssid,
>  			const u8 *ssid, int ssid_len,
>  			const u8 *ie, int ie_len, bool use_mfp,
> -			struct cfg80211_crypto_settings *crypt);
> +			struct cfg80211_crypto_settings *crypt,
> +			u32 assoc_flags, struct ieee80211_ht_cap *ht_capa,
> +			struct ieee80211_ht_cap *ht_capa_mask);
>  int __cfg80211_mlme_deauth(struct cfg80211_registered_device *rdev,
>  			   struct net_device *dev, const u8 *bssid,
>  			   const u8 *ie, int ie_len, u16 reason,
> @@ -377,6 +381,8 @@ int cfg80211_mlme_mgmt_tx(struct cfg80211_registered_device *rdev,
>  			  bool channel_type_valid, unsigned int wait,
>  			  const u8 *buf, size_t len, bool no_cck,
>  			  u64 *cookie);
> +void cfg80211_oper_and_ht_capa(struct ieee80211_ht_cap *ht_capa,
> +			       const struct ieee80211_ht_cap *ht_capa_mask);
>  
>  /* SME */
>  int __cfg80211_connect(struct cfg80211_registered_device *rdev,
> diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
> index 21fc970..bf538e1 100644
> --- a/net/wireless/mlme.c
> +++ b/net/wireless/mlme.c
> @@ -501,13 +501,32 @@ int cfg80211_mlme_auth(struct cfg80211_registered_device *rdev,
>  	return err;
>  }
>  
> +/*  Do a logical ht_capa &= ht_capa_mask.  */
> +void cfg80211_oper_and_ht_capa(struct ieee80211_ht_cap *ht_capa,
> +			       const struct ieee80211_ht_cap *ht_capa_mask)
> +{
> +	int i;
> +	u8 *p1, *p2;
> +	if (!ht_capa_mask) {
> +		memset(ht_capa, 0, sizeof(*ht_capa));
> +		return;
> +	}
> +
> +	p1 = (u8*)(ht_capa);
> +	p2 = (u8*)(ht_capa_mask);
> +	for (i = 0; i<sizeof(*ht_capa); i++)
> +		p1[i] &= p2[i];
> +}
> +
>  int __cfg80211_mlme_assoc(struct cfg80211_registered_device *rdev,
>  			  struct net_device *dev,
>  			  struct ieee80211_channel *chan,
>  			  const u8 *bssid, const u8 *prev_bssid,
>  			  const u8 *ssid, int ssid_len,
>  			  const u8 *ie, int ie_len, bool use_mfp,
> -			  struct cfg80211_crypto_settings *crypt)
> +			  struct cfg80211_crypto_settings *crypt,
> +			  u32 assoc_flags, struct ieee80211_ht_cap *ht_capa,
> +			  struct ieee80211_ht_cap *ht_capa_mask)
>  {
>  	struct wireless_dev *wdev = dev->ieee80211_ptr;
>  	struct cfg80211_assoc_request req;
> @@ -537,6 +556,15 @@ int __cfg80211_mlme_assoc(struct cfg80211_registered_device *rdev,
>  	memcpy(&req.crypto, crypt, sizeof(req.crypto));
>  	req.use_mfp = use_mfp;
>  	req.prev_bssid = prev_bssid;
> +	req.flags = assoc_flags;
> +	if (ht_capa)
> +		memcpy(&req.ht_capa, ht_capa, sizeof(req.ht_capa));
> +	if (ht_capa_mask)
> +		memcpy(&req.ht_capa_mask, ht_capa_mask,
> +		       sizeof(req.ht_capa_mask));
> +	cfg80211_oper_and_ht_capa(&req.ht_capa_mask,
> +				  rdev->wiphy.ht_capa_mod_mask);
> +
>  	req.bss = cfg80211_get_bss(&rdev->wiphy, chan, bssid, ssid, ssid_len,
>  				   WLAN_CAPABILITY_ESS, WLAN_CAPABILITY_ESS);
>  	if (!req.bss) {
> @@ -574,14 +602,17 @@ int cfg80211_mlme_assoc(struct cfg80211_registered_device *rdev,
>  			const u8 *bssid, const u8 *prev_bssid,
>  			const u8 *ssid, int ssid_len,
>  			const u8 *ie, int ie_len, bool use_mfp,
> -			struct cfg80211_crypto_settings *crypt)
> +			struct cfg80211_crypto_settings *crypt,
> +			u32 assoc_flags, struct ieee80211_ht_cap *ht_capa,
> +			struct ieee80211_ht_cap *ht_capa_mask)
>  {
>  	struct wireless_dev *wdev = dev->ieee80211_ptr;
>  	int err;
>  
>  	wdev_lock(wdev);
>  	err = __cfg80211_mlme_assoc(rdev, dev, chan, bssid, prev_bssid,
> -				    ssid, ssid_len, ie, ie_len, use_mfp, crypt);
> +				    ssid, ssid_len, ie, ie_len, use_mfp, crypt,
> +				    assoc_flags, ht_capa, ht_capa_mask);
>  	wdev_unlock(wdev);
>  
>  	return err;
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index b3a476f..31f17c8 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -196,6 +196,10 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
>  	[NL80211_ATTR_TDLS_OPERATION] = { .type = NLA_U8 },
>  	[NL80211_ATTR_TDLS_SUPPORT] = { .type = NLA_FLAG },
>  	[NL80211_ATTR_TDLS_EXTERNAL_SETUP] = { .type = NLA_FLAG },
> +	[NL80211_ATTR_DISABLE_HT] = { .type = NLA_FLAG },
> +	[NL80211_ATTR_HT_CAPABILITY_MASK] = {
> +		.len = NL80211_HT_CAPABILITY_LEN
> +	},
>  };
>  
>  /* policy for the key attributes */
> @@ -1007,6 +1011,11 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
>  	if (nl80211_put_iface_combinations(&dev->wiphy, msg))
>  		goto nla_put_failure;
>  
> +	if (dev->wiphy.ht_capa_mod_mask)
> +		NLA_PUT(msg, NL80211_ATTR_HT_CAPABILITY_MASK,
> +			sizeof(*dev->wiphy.ht_capa_mod_mask),
> +			dev->wiphy.ht_capa_mod_mask);
> +
>  	return genlmsg_end(msg, hdr);
>  
>   nla_put_failure:
> @@ -4359,6 +4368,9 @@ static int nl80211_associate(struct sk_buff *skb, struct genl_info *info)
>  	const u8 *bssid, *ssid, *ie = NULL, *prev_bssid = NULL;
>  	int err, ssid_len, ie_len = 0;
>  	bool use_mfp = false;
> +	u32 flags = 0;
> +	struct ieee80211_ht_cap *ht_capa = NULL;
> +	struct ieee80211_ht_cap *ht_capa_mask = NULL;
>  
>  	if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE]))
>  		return -EINVAL;
> @@ -4402,11 +4414,25 @@ static int nl80211_associate(struct sk_buff *skb, struct genl_info *info)
>  	if (info->attrs[NL80211_ATTR_PREV_BSSID])
>  		prev_bssid = nla_data(info->attrs[NL80211_ATTR_PREV_BSSID]);
>  
> +	if (nla_get_flag(info->attrs[NL80211_ATTR_DISABLE_HT]))
> +		flags |= ASSOC_REQ_DISABLE_HT;
> +
> +	if (info->attrs[NL80211_ATTR_HT_CAPABILITY_MASK])
> +		ht_capa_mask =
> +			nla_data(info->attrs[NL80211_ATTR_HT_CAPABILITY_MASK]);
> +
> +	if (info->attrs[NL80211_ATTR_HT_CAPABILITY]) {
> +		if (!ht_capa_mask)
> +			return -EINVAL;
> +		ht_capa = nla_data(info->attrs[NL80211_ATTR_HT_CAPABILITY]);
> +	}
> +
>  	err = nl80211_crypto_settings(rdev, info, &crypto, 1);
>  	if (!err)
>  		err = cfg80211_mlme_assoc(rdev, dev, chan, bssid, prev_bssid,
>  					  ssid, ssid_len, ie, ie_len, use_mfp,
> -					  &crypto);
> +					  &crypto, flags, ht_capa,
> +					  ht_capa_mask);
>  
>  	return err;
>  }
> @@ -4896,6 +4922,22 @@ static int nl80211_connect(struct sk_buff *skb, struct genl_info *info)
>  			return PTR_ERR(connkeys);
>  	}
>  
> +	if (nla_get_flag(info->attrs[NL80211_ATTR_DISABLE_HT]))
> +		connect.flags |= ASSOC_REQ_DISABLE_HT;
> +
> +	if (info->attrs[NL80211_ATTR_HT_CAPABILITY_MASK])
> +		memcpy(&connect.ht_capa_mask,
> +		       nla_data(info->attrs[NL80211_ATTR_HT_CAPABILITY_MASK]),
> +		       sizeof(connect.ht_capa_mask));
> +
> +	if (info->attrs[NL80211_ATTR_HT_CAPABILITY]) {
> +		if (!info->attrs[NL80211_ATTR_HT_CAPABILITY_MASK])
> +			return -EINVAL;
> +		memcpy(&connect.ht_capa,
> +		       nla_data(info->attrs[NL80211_ATTR_HT_CAPABILITY]),
> +		       sizeof(connect.ht_capa));
> +	}
> +
>  	err = cfg80211_connect(rdev, dev, &connect, connkeys);
>  	if (err)
>  		kfree(connkeys);
> diff --git a/net/wireless/sme.c b/net/wireless/sme.c
> index 6e86d5a..ed9d0e6 100644
> --- a/net/wireless/sme.c
> +++ b/net/wireless/sme.c
> @@ -189,7 +189,9 @@ static int cfg80211_conn_do_work(struct wireless_dev *wdev)
>  					    prev_bssid,
>  					    params->ssid, params->ssid_len,
>  					    params->ie, params->ie_len,
> -					    false, &params->crypto);
> +					    false, &params->crypto,
> +					    params->flags, &params->ht_capa,
> +					    &params->ht_capa_mask);
>  		if (err)
>  			__cfg80211_mlme_deauth(rdev, wdev->netdev, params->bssid,
>  					       NULL, 0,
> @@ -773,6 +775,9 @@ int __cfg80211_connect(struct cfg80211_registered_device *rdev,
>  		wdev->connect_keys = NULL;
>  	}
>  
> +	cfg80211_oper_and_ht_capa(&connect->ht_capa_mask,
> +				  rdev->wiphy.ht_capa_mod_mask);
> +
>  	if (connkeys && connkeys->def >= 0) {
>  		int idx;
>  		u32 cipher;



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 19:36 ` [PATCH v8 2/2] mac80211: Support ht-cap over-rides greearb
@ 2011-11-08 20:09   ` Johannes Berg
  2011-11-08 20:44     ` Ben Greear
  2011-11-08 20:12   ` Johannes Berg
  1 sibling, 1 reply; 20+ messages in thread
From: Johannes Berg @ 2011-11-08 20:09 UTC (permalink / raw)
  To: greearb; +Cc: linux-wireless

On Tue, 2011-11-08 at 11:36 -0800, greearb@candelatech.com wrote:

> +			/*
> +			 * We always need to advert at least MCS0-7, to
> +			 * be a compliant HT station, for instance
> +			 */
> +			if (((i * 8 + q) >= min_rates) &&

This is a little misleading -- why min_rates when the comment says
MCS0-7?

johannes


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 19:36 ` [PATCH v8 2/2] mac80211: Support ht-cap over-rides greearb
  2011-11-08 20:09   ` Johannes Berg
@ 2011-11-08 20:12   ` Johannes Berg
  2011-11-08 20:17     ` Johannes Berg
  2011-11-08 20:58     ` Ben Greear
  1 sibling, 2 replies; 20+ messages in thread
From: Johannes Berg @ 2011-11-08 20:12 UTC (permalink / raw)
  To: greearb; +Cc: linux-wireless

On Tue, 2011-11-08 at 11:36 -0800, greearb@candelatech.com wrote:

> +bool ieee80111_cfg_override_disables_ht40(struct ieee80211_sub_if_data *sdata)
> +{
> +	if ((sdata->u.mgd.ht_capa_mask.cap_info &
> +	     IEEE80211_HT_CAP_SUP_WIDTH_20_40) &&
> +	    !(sdata->u.mgd.ht_capa.cap_info &
> +	      IEEE80211_HT_CAP_SUP_WIDTH_20_40))
> +		return true;
> +	return false;

Would it really go above 80 cols if you didn't line-wrap it? Maybe
remove the extra sets of parentheses? And even if it goes to a little
bit above 80 it's still be more readable without the wrapping ...

One thing I don't quite understand: Why don't you calculate the HT caps
to use upon assoc request, and then store *those* instead, then you
wouldn't have to check the overrides every time.

For example here:

>  		if (!(ap_ht_cap_flags & IEEE80211_HT_CAP_40MHZ_INTOLERANT) &&
> +		    !ieee80111_cfg_override_disables_ht40(sdata) &&
>  		    (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) {

This just adds complexity. If you calculate sdata->used_ht_caps first
then you can replace the sband->ht_cap.cap check with an
sdata->used_ht_caps.cap check and be done with it, instead of having to
check both.

johannes


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 20:12   ` Johannes Berg
@ 2011-11-08 20:17     ` Johannes Berg
  2011-11-08 20:58     ` Ben Greear
  1 sibling, 0 replies; 20+ messages in thread
From: Johannes Berg @ 2011-11-08 20:17 UTC (permalink / raw)
  To: greearb; +Cc: linux-wireless

On Tue, 2011-11-08 at 21:12 +0100, Johannes Berg wrote:

> For example here:
> 
> >  		if (!(ap_ht_cap_flags & IEEE80211_HT_CAP_40MHZ_INTOLERANT) &&
> > +		    !ieee80111_cfg_override_disables_ht40(sdata) &&
> >  		    (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) {
> 
> This just adds complexity. If you calculate sdata->used_ht_caps first
> then you can replace the sband->ht_cap.cap check with an
> sdata->used_ht_caps.cap check and be done with it, instead of having to
> check both.

Similarly in ieee80211_add_ht_ie(), you wouldn't have to re-calculate
the override, etc. I think it would be a lot simpler that way.

johannes


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 20:09   ` Johannes Berg
@ 2011-11-08 20:44     ` Ben Greear
  2011-11-08 20:58       ` Johannes Berg
  0 siblings, 1 reply; 20+ messages in thread
From: Ben Greear @ 2011-11-08 20:44 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On 11/08/2011 12:09 PM, Johannes Berg wrote:
> On Tue, 2011-11-08 at 11:36 -0800, greearb@candelatech.com wrote:
>
>> +			/*
>> +			 * We always need to advert at least MCS0-7, to
>> +			 * be a compliant HT station, for instance
>> +			 */
>> +			if (((i * 8 + q)>= min_rates)&&
>
> This is a little misleading -- why min_rates when the comment says
> MCS0-7?

I let caller determine the min, but comment was to tell why
the min might be set.  In APs, the min supported rates are 16, evidently...not
that this code supports APs at the moment...

When this is about local use instead of advertising, then any minimum
is OK.

Want me to just remove the comment entirely?

Thanks,
Ben

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 20:44     ` Ben Greear
@ 2011-11-08 20:58       ` Johannes Berg
  2011-11-08 21:00         ` Johannes Berg
  2011-11-08 21:06         ` Ben Greear
  0 siblings, 2 replies; 20+ messages in thread
From: Johannes Berg @ 2011-11-08 20:58 UTC (permalink / raw)
  To: Ben Greear; +Cc: linux-wireless

On Tue, 2011-11-08 at 12:44 -0800, Ben Greear wrote:
> On 11/08/2011 12:09 PM, Johannes Berg wrote:
> > On Tue, 2011-11-08 at 11:36 -0800, greearb@candelatech.com wrote:
> >
> >> +			/*
> >> +			 * We always need to advert at least MCS0-7, to
> >> +			 * be a compliant HT station, for instance
> >> +			 */
> >> +			if (((i * 8 + q)>= min_rates)&&
> >
> > This is a little misleading -- why min_rates when the comment says
> > MCS0-7?
> 
> I let caller determine the min, but comment was to tell why
> the min might be set.  In APs, the min supported rates are 16, evidently...not
> that this code supports APs at the moment...

About APs: that can't be right, there certainly will be 1x1 APs.

> When this is about local use instead of advertising, then any minimum
> is OK.
> 
> Want me to just remove the comment entirely?

Well, so, I think the logic there is a little odd anyway -- why aren't
you doing it byte-wise, if the only thing that can possibly happen is
that the first byte is masked or not? Maybe change the parameter to
"bool allow_single_stream_mask" or something like that and adjust the
algorithm like:

start = allow_single_stream_mask ? 0 : 1;

for (i = start; i < IEEE80211_HT_MCS_MASK_LEN; i++) {
	u8 val = smask[i] & scaps[i];
	val |= ht_cap->mcs.rx_mask[i] & ~smask[i];
	ht_cap->mcs.rx_mask[i] val;
}

or so. right? Much simpler?

johannes


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 20:12   ` Johannes Berg
  2011-11-08 20:17     ` Johannes Berg
@ 2011-11-08 20:58     ` Ben Greear
  2011-11-08 21:02       ` Johannes Berg
  1 sibling, 1 reply; 20+ messages in thread
From: Ben Greear @ 2011-11-08 20:58 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On 11/08/2011 12:12 PM, Johannes Berg wrote:
> On Tue, 2011-11-08 at 11:36 -0800, greearb@candelatech.com wrote:
>
>> +bool ieee80111_cfg_override_disables_ht40(struct ieee80211_sub_if_data *sdata)
>> +{
>> +	if ((sdata->u.mgd.ht_capa_mask.cap_info&
>> +	     IEEE80211_HT_CAP_SUP_WIDTH_20_40)&&
>> +	    !(sdata->u.mgd.ht_capa.cap_info&
>> +	      IEEE80211_HT_CAP_SUP_WIDTH_20_40))
>> +		return true;
>> +	return false;
>
> Would it really go above 80 cols if you didn't line-wrap it? Maybe
> remove the extra sets of parentheses? And even if it goes to a little
> bit above 80 it's still be more readable without the wrapping ...

It is more readable w/out the wrapping, but hard to know when
patches get rejected about that or not, so I tried to keep
checkpatch happy.  If you'll take slightly longer lines I'll
happily un-wrap it.

>
> One thing I don't quite understand: Why don't you calculate the HT caps
> to use upon assoc request, and then store *those* instead, then you
> wouldn't have to check the overrides every time.

Adding more state just gives more places to mess up that
state or forget to update it somehow.  Think of the channel
pointers in the scan & work code :)
Not to mention the extra bloat in RAM.
Since this is not hot-path code, I think having less state
is well worth the effort.

> For example here:
>
>>   		if (!(ap_ht_cap_flags&  IEEE80211_HT_CAP_40MHZ_INTOLERANT)&&
>> +		    !ieee80111_cfg_override_disables_ht40(sdata)&&
>>   		(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) {
>
> This just adds complexity. If you calculate sdata->used_ht_caps first
> then you can replace the sband->ht_cap.cap check with an
> sdata->used_ht_caps.cap check and be done with it, instead of having to
> check both.

I think that's a bad idea, but will change it if you insist.

Thanks,
Ben

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 20:58       ` Johannes Berg
@ 2011-11-08 21:00         ` Johannes Berg
  2011-11-08 21:06         ` Ben Greear
  1 sibling, 0 replies; 20+ messages in thread
From: Johannes Berg @ 2011-11-08 21:00 UTC (permalink / raw)
  To: Ben Greear; +Cc: linux-wireless

On Tue, 2011-11-08 at 21:58 +0100, Johannes Berg wrote:
> On Tue, 2011-11-08 at 12:44 -0800, Ben Greear wrote:
> > On 11/08/2011 12:09 PM, Johannes Berg wrote:
> > > On Tue, 2011-11-08 at 11:36 -0800, greearb@candelatech.com wrote:
> > >
> > >> +			/*
> > >> +			 * We always need to advert at least MCS0-7, to
> > >> +			 * be a compliant HT station, for instance
> > >> +			 */
> > >> +			if (((i * 8 + q)>= min_rates)&&
> > >
> > > This is a little misleading -- why min_rates when the comment says
> > > MCS0-7?
> > 
> > I let caller determine the min, but comment was to tell why
> > the min might be set.  In APs, the min supported rates are 16, evidently...not
> > that this code supports APs at the moment...
> 
> About APs: that can't be right, there certainly will be 1x1 APs.
> 
> > When this is about local use instead of advertising, then any minimum
> > is OK.
> > 
> > Want me to just remove the comment entirely?
> 
> Well, so, I think the logic there is a little odd anyway -- why aren't
> you doing it byte-wise, if the only thing that can possibly happen is
> that the first byte is masked or not? Maybe change the parameter to
> "bool allow_single_stream_mask" or something like that and adjust the
> algorithm like:
> 
> start = allow_single_stream_mask ? 0 : 1;
> 
> for (i = start; i < IEEE80211_HT_MCS_MASK_LEN; i++) {
> 	u8 val = smask[i] & scaps[i];
> 	val |= ht_cap->mcs.rx_mask[i] & ~smask[i];
> 	ht_cap->mcs.rx_mask[i] val;
> }

Ok, no, I totally misunderstood the variables here, but anyway, it seems
there's no need for bit-wise stuff.

johannes


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 20:58     ` Ben Greear
@ 2011-11-08 21:02       ` Johannes Berg
  2011-11-08 23:11         ` Ben Greear
  2011-11-10 19:25         ` Ben Greear
  0 siblings, 2 replies; 20+ messages in thread
From: Johannes Berg @ 2011-11-08 21:02 UTC (permalink / raw)
  To: Ben Greear; +Cc: linux-wireless

On Tue, 2011-11-08 at 12:58 -0800, Ben Greear wrote:

> >> +bool ieee80111_cfg_override_disables_ht40(struct ieee80211_sub_if_data *sdata)
> >> +{
> >> +	if ((sdata->u.mgd.ht_capa_mask.cap_info&
> >> +	     IEEE80211_HT_CAP_SUP_WIDTH_20_40)&&
> >> +	    !(sdata->u.mgd.ht_capa.cap_info&
> >> +	      IEEE80211_HT_CAP_SUP_WIDTH_20_40))
> >> +		return true;
> >> +	return false;
> >
> > Would it really go above 80 cols if you didn't line-wrap it? Maybe
> > remove the extra sets of parentheses? And even if it goes to a little
> > bit above 80 it's still be more readable without the wrapping ...
> 
> It is more readable w/out the wrapping, but hard to know when
> patches get rejected about that or not, so I tried to keep
> checkpatch happy.  If you'll take slightly longer lines I'll
> happily un-wrap it.

Please. I'd rather go over a bit than read the above :-)

> > One thing I don't quite understand: Why don't you calculate the HT caps
> > to use upon assoc request, and then store *those* instead, then you
> > wouldn't have to check the overrides every time.
> 
> Adding more state just gives more places to mess up that
> state or forget to update it somehow.  Think of the channel
> pointers in the scan & work code :)
> Not to mention the extra bloat in RAM.
> Since this is not hot-path code, I think having less state
> is well worth the effort.

You don't have more state though, you actually should end up with less
since you can throw away the configured ht_caps & ht_mask? And you
really just shift the access from sband->... to calculated_... stuff.


> > For example here:
> >
> >>   		if (!(ap_ht_cap_flags&  IEEE80211_HT_CAP_40MHZ_INTOLERANT)&&
> >> +		    !ieee80111_cfg_override_disables_ht40(sdata)&&
> >>   		(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) {
> >
> > This just adds complexity. If you calculate sdata->used_ht_caps first
> > then you can replace the sband->ht_cap.cap check with an
> > sdata->used_ht_caps.cap check and be done with it, instead of having to
> > check both.
> 
> I think that's a bad idea, but will change it if you insist.

I really think that'd be much nicer. As it is now we have to add all
these checks everywhere, if we just calculate it once and then change
places to use it we just have to remember to use the right thing.

johannes


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 20:58       ` Johannes Berg
  2011-11-08 21:00         ` Johannes Berg
@ 2011-11-08 21:06         ` Ben Greear
  2011-11-08 21:08           ` Johannes Berg
  1 sibling, 1 reply; 20+ messages in thread
From: Ben Greear @ 2011-11-08 21:06 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On 11/08/2011 12:58 PM, Johannes Berg wrote:
> On Tue, 2011-11-08 at 12:44 -0800, Ben Greear wrote:
>> On 11/08/2011 12:09 PM, Johannes Berg wrote:
>>> On Tue, 2011-11-08 at 11:36 -0800, greearb@candelatech.com wrote:
>>>
>>>> +			/*
>>>> +			 * We always need to advert at least MCS0-7, to
>>>> +			 * be a compliant HT station, for instance
>>>> +			 */
>>>> +			if (((i * 8 + q)>= min_rates)&&
>>>
>>> This is a little misleading -- why min_rates when the comment says
>>> MCS0-7?
>>
>> I let caller determine the min, but comment was to tell why
>> the min might be set.  In APs, the min supported rates are 16, evidently...not
>> that this code supports APs at the moment...
>
> About APs: that can't be right, there certainly will be 1x1 APs.

Section 20.1.1 again:

"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."

Again, I think the requirement is lame, and maybe everyone will just ignore it,
but it is there...


>> When this is about local use instead of advertising, then any minimum
>> is OK.
>>
>> Want me to just remove the comment entirely?
>
> Well, so, I think the logic there is a little odd anyway -- why aren't
> you doing it byte-wise, if the only thing that can possibly happen is
> that the first byte is masked or not? Maybe change the parameter to
> "bool allow_single_stream_mask" or something like that and adjust the
> algorithm like:
>
> start = allow_single_stream_mask ? 0 : 1;
>
> for (i = start; i<  IEEE80211_HT_MCS_MASK_LEN; i++) {
> 	u8 val = smask[i]&  scaps[i];
> 	val |= ht_cap->mcs.rx_mask[i]&  ~smask[i];
> 	ht_cap->mcs.rx_mask[i] val;
> }
>
> or so. right? Much simpler?

Depends on whether we want to honour the AP part of 20.1.1.
Since we don't support AP mode anyway right now,
I'm fine with your suggestion.  Let me know if you want
me to proceed with your suggested changes.

Thanks,
Ben

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 21:06         ` Ben Greear
@ 2011-11-08 21:08           ` Johannes Berg
  0 siblings, 0 replies; 20+ messages in thread
From: Johannes Berg @ 2011-11-08 21:08 UTC (permalink / raw)
  To: Ben Greear; +Cc: linux-wireless

On Tue, 2011-11-08 at 13:06 -0800, Ben Greear wrote:

> > About APs: that can't be right, there certainly will be 1x1 APs.
> 
> Section 20.1.1 again:
> 
> "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."
> 
> Again, I think the requirement is lame, and maybe everyone will just ignore it,
> but it is there...

Funny. Yeah I'm pretty sure everybody ignores that. Of course maybe you
can't get a 1x1 AP certified as an AP, but surely that'll happen with a
P2P GO all the time.

> > start = allow_single_stream_mask ? 0 : 1;
> >
> > for (i = start; i<  IEEE80211_HT_MCS_MASK_LEN; i++) {
> > 	u8 val = smask[i]&  scaps[i];
> > 	val |= ht_cap->mcs.rx_mask[i]&  ~smask[i];
> > 	ht_cap->mcs.rx_mask[i] val;
> > }
> >
> > or so. right? Much simpler?
> 
> Depends on whether we want to honour the AP part of 20.1.1.
> Since we don't support AP mode anyway right now,
> I'm fine with your suggestion.  Let me know if you want
> me to proceed with your suggested changes.

I don't want to honour it. In fact, the more I think about it the less I
want to honour the 0-7 part here, since that just made the code more
complex and almost nobody will be using this override anyway.

johannes


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 21:02       ` Johannes Berg
@ 2011-11-08 23:11         ` Ben Greear
  2011-11-09  8:37           ` Johannes Berg
  2011-11-10 19:25         ` Ben Greear
  1 sibling, 1 reply; 20+ messages in thread
From: Ben Greear @ 2011-11-08 23:11 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On 11/08/2011 01:02 PM, Johannes Berg wrote:
> On Tue, 2011-11-08 at 12:58 -0800, Ben Greear wrote:
>
>>>> +bool ieee80111_cfg_override_disables_ht40(struct ieee80211_sub_if_data *sdata)
>>>> +{
>>>> +	if ((sdata->u.mgd.ht_capa_mask.cap_info&
>>>> +	     IEEE80211_HT_CAP_SUP_WIDTH_20_40)&&
>>>> +	    !(sdata->u.mgd.ht_capa.cap_info&
>>>> +	      IEEE80211_HT_CAP_SUP_WIDTH_20_40))
>>>> +		return true;
>>>> +	return false;
>>>
>>> Would it really go above 80 cols if you didn't line-wrap it? Maybe
>>> remove the extra sets of parentheses? And even if it goes to a little
>>> bit above 80 it's still be more readable without the wrapping ...
>>
>> It is more readable w/out the wrapping, but hard to know when
>> patches get rejected about that or not, so I tried to keep
>> checkpatch happy.  If you'll take slightly longer lines I'll
>> happily un-wrap it.
>
> Please. I'd rather go over a bit than read the above :-)

I just fixed this by using a tmp variable.  Hopefully the
compiler is smart enough to make it disappear.

>>> For example here:
>>>
>>>>    		if (!(ap_ht_cap_flags&   IEEE80211_HT_CAP_40MHZ_INTOLERANT)&&
>>>> +		    !ieee80111_cfg_override_disables_ht40(sdata)&&
>>>>    		(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) {
>>>
>>> This just adds complexity. If you calculate sdata->used_ht_caps first
>>> then you can replace the sband->ht_cap.cap check with an
>>> sdata->used_ht_caps.cap check and be done with it, instead of having to
>>> check both.
>>
>> I think that's a bad idea, but will change it if you insist.
>
> I really think that'd be much nicer. As it is now we have to add all
> these checks everywhere, if we just calculate it once and then change
> places to use it we just have to remember to use the right thing.

I'm quite nervous about attempting this change.  I'm pretty
confident my current patch works as designed, and over all,
it's pretty non-intrusive.  I am afraid that if I start trying
to substitute something for sband->ht_cap that I'm going
to end up changing a lot of code and possibly adding all sorts
of strange bugs.

Out of curiousity, if one is doing off-channel work, wouldn't
sband be different from when we associated and possibly different
in it's ht-capabilities?  If so, the used-ht-caps would not
be valid for that work?

I'm posting a v9 with just the minimum MCS rates stuff removed
and some formatting cleanup.

If this used-ht-caps stuff is required, I'll see
what I can do.

Thanks,
Ben

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 23:11         ` Ben Greear
@ 2011-11-09  8:37           ` Johannes Berg
  0 siblings, 0 replies; 20+ messages in thread
From: Johannes Berg @ 2011-11-09  8:37 UTC (permalink / raw)
  To: Ben Greear; +Cc: linux-wireless

On Tue, 2011-11-08 at 15:11 -0800, Ben Greear wrote:

> > Please. I'd rather go over a bit than read the above :-)
> 
> I just fixed this by using a tmp variable.  Hopefully the
> compiler is smart enough to make it disappear.

That works, yeah.

> > I really think that'd be much nicer. As it is now we have to add all
> > these checks everywhere, if we just calculate it once and then change
> > places to use it we just have to remember to use the right thing.
> 
> I'm quite nervous about attempting this change.  I'm pretty
> confident my current patch works as designed, and over all,
> it's pretty non-intrusive.  I am afraid that if I start trying
> to substitute something for sband->ht_cap that I'm going
> to end up changing a lot of code and possibly adding all sorts
> of strange bugs.

You should only have to change the places where you now added the
overrides, no?

> Out of curiousity, if one is doing off-channel work, wouldn't
> sband be different from when we associated and possibly different
> in it's ht-capabilities?  If so, the used-ht-caps would not
> be valid for that work?

Well the off-channel work will be on the channel that you're going to be
using after a successful connection, so that should be OK.

I'd at least like to try. If it ends up a disaster maybe we shouldn't,
but the way you go back to the mask & set all the time makes me a bit
nervous.

johannes


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-08 21:02       ` Johannes Berg
  2011-11-08 23:11         ` Ben Greear
@ 2011-11-10 19:25         ` Ben Greear
  2011-11-17 11:28           ` Johannes Berg
  1 sibling, 1 reply; 20+ messages in thread
From: Ben Greear @ 2011-11-10 19:25 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On 11/08/2011 01:02 PM, Johannes Berg wrote:

>>> For example here:
>>>
>>>>    		if (!(ap_ht_cap_flags&   IEEE80211_HT_CAP_40MHZ_INTOLERANT)&&
>>>> +		    !ieee80111_cfg_override_disables_ht40(sdata)&&
>>>>    		(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) {
>>>
>>> This just adds complexity. If you calculate sdata->used_ht_caps first
>>> then you can replace the sband->ht_cap.cap check with an
>>> sdata->used_ht_caps.cap check and be done with it, instead of having to
>>> check both.
>>
>> I think that's a bad idea, but will change it if you insist.
>
> I really think that'd be much nicer. As it is now we have to add all
> these checks everywhere, if we just calculate it once and then change
> places to use it we just have to remember to use the right thing.

I rebased against today's wireless-testing tree and started work on this.

But, I don't think it is going to work..or at least if it can, I
don't see a good way to do it.

I'm stuck in the ieee80211_ht_cap_ie_to_sta_ht_cap method.  With
my original patch, I apply overrides here, at the bottom of the
method.  If we're associated (or started associating)
and user asked for over-rides, we'll apply requested overrides, else
nothing will change because the over-rides mask is not set.

But, if I have to use pre-computed values here then I need to
be certain they are set properly.  If association has been
requested, then that is fine.  But, what about the
sta_apply_parameters() method?  Can we guarantee that association has
been requested when this method is called?  I think we cannot,
and if not, then I cannot use pre-computed sdata->used_ht_caps.
I could attempt to set a flag when used_ht_caps
has been calculated, and add a check for that, but that is yet another piece
of computed state that could be stale if we make a mistake somewhere.

There is another caller in mesh_plink.c: mesh_plink_alloc()
I don't know much of anything about mesh, but I think we will always be
associated here.  The callers in mlme.c should always be associated as
far as I can tell.

The ieee80211_add_ht_ie method appears to be called when association is
already started, so it should be ok to use a derived value (it is
called only from send_assoc()).

If you prefer the additional-flag-if-already-calculated approach,
let me know and I'll continue on this path.  Or, if I'm confused
about the sta_apply_parameters path and it *is* always set?

Thanks,
Ben

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-10 19:25         ` Ben Greear
@ 2011-11-17 11:28           ` Johannes Berg
  2011-11-17 17:22             ` Ben Greear
  0 siblings, 1 reply; 20+ messages in thread
From: Johannes Berg @ 2011-11-17 11:28 UTC (permalink / raw)
  To: Ben Greear; +Cc: linux-wireless

Ben, sorry for the delay, I was too busy thinking about other stuff.

On Thu, 2011-11-10 at 11:25 -0800, Ben Greear wrote:

> But, I don't think it is going to work..or at least if it can, I
> don't see a good way to do it.
> 
> I'm stuck in the ieee80211_ht_cap_ie_to_sta_ht_cap method.  With
> my original patch, I apply overrides here, at the bottom of the
> method.  If we're associated (or started associating)
> and user asked for over-rides, we'll apply requested overrides, else
> nothing will change because the over-rides mask is not set.
> 
> But, if I have to use pre-computed values here then I need to
> be certain they are set properly.  If association has been
> requested, then that is fine.  But, what about the
> sta_apply_parameters() method?  Can we guarantee that association has
> been requested when this method is called?  I think we cannot,
> and if not, then I cannot use pre-computed sdata->used_ht_caps.
> I could attempt to set a flag when used_ht_caps
> has been calculated, and add a check for that, but that is yet another piece
> of computed state that could be stale if we make a mistake somewhere.

We can't, but can't we like assign sdata->ht_caps = sband->ht_caps? Or
maybe even calculate restricted HT caps for both 2.4 and 5 GHz?
Basically what I was thinking is this:

struct sub_if_data {
	...
	struct ieee80211_sta_ht_cap ht_cap[num_bands];
	...
};

We'd use *that* everywhere, and when associating we calculate
sdata->ht_cap[band] = apply_overrides(sbands[band]->ht_cap);

and when disassociating we simply
	memcpy(sdata->ht_cap[band], sbands[band]->ht_cap, ...);

or so? Then we can always use sdata->ht_cap[bands] instead of
sband->ht_caps everywhere, and overrides are implicit.

Was this what you attempted?

johannes


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-17 11:28           ` Johannes Berg
@ 2011-11-17 17:22             ` Ben Greear
  2011-11-17 17:25               ` Johannes Berg
  0 siblings, 1 reply; 20+ messages in thread
From: Ben Greear @ 2011-11-17 17:22 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On 11/17/2011 03:28 AM, Johannes Berg wrote:
> Ben, sorry for the delay, I was too busy thinking about other stuff.
>
> On Thu, 2011-11-10 at 11:25 -0800, Ben Greear wrote:
>
>> But, I don't think it is going to work..or at least if it can, I
>> don't see a good way to do it.
>>
>> I'm stuck in the ieee80211_ht_cap_ie_to_sta_ht_cap method.  With
>> my original patch, I apply overrides here, at the bottom of the
>> method.  If we're associated (or started associating)
>> and user asked for over-rides, we'll apply requested overrides, else
>> nothing will change because the over-rides mask is not set.
>>
>> But, if I have to use pre-computed values here then I need to
>> be certain they are set properly.  If association has been
>> requested, then that is fine.  But, what about the
>> sta_apply_parameters() method?  Can we guarantee that association has
>> been requested when this method is called?  I think we cannot,
>> and if not, then I cannot use pre-computed sdata->used_ht_caps.
>> I could attempt to set a flag when used_ht_caps
>> has been calculated, and add a check for that, but that is yet another piece
>> of computed state that could be stale if we make a mistake somewhere.
>
> We can't, but can't we like assign sdata->ht_caps = sband->ht_caps? Or
> maybe even calculate restricted HT caps for both 2.4 and 5 GHz?
> Basically what I was thinking is this:

> struct sub_if_data {
> 	...
> 	struct ieee80211_sta_ht_cap ht_cap[num_bands];
> 	...
> };

So we'd have to copy this data from sbands[] upon creation
of the interface to make sure it is always initialized?
This would allow us to easily support the overrides for
non-station interfaces, so I do like that benefit.

Can the sbands[] data ever change for any reason once
an interface is created?  If not, then probably this is
doable.  If it can change, then we are screwed.

> We'd use *that* everywhere, and when associating we calculate
> sdata->ht_cap[band] = apply_overrides(sbands[band]->ht_cap);
>
> and when disassociating we simply
> 	memcpy(sdata->ht_cap[band], sbands[band]->ht_cap, ...);
>
> or so? Then we can always use sdata->ht_cap[bands] instead of
> sband->ht_caps everywhere, and overrides are implicit.
>
> Was this what you attempted?

I was trying to use a single calculated-ht-cap struct instead
of one for each band, and I was trying to put it into sdata->u.sta,
which made initialization issues much more scary for me.

It seems you are basically wanting to copy the sband[] data
local to each interface (sdata), and then we would remove sband from most
(or all?) of the method calls that deal with sband->ht_caps?

Looks like quite a bit of code churn, and probably requiring at least two
patches:
  * Get rid of sband usage by copying the sband data into sdata
  * Add the over-ride logic


Thanks,
Ben

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-17 17:22             ` Ben Greear
@ 2011-11-17 17:25               ` Johannes Berg
  2011-11-17 17:42                 ` Ben Greear
  0 siblings, 1 reply; 20+ messages in thread
From: Johannes Berg @ 2011-11-17 17:25 UTC (permalink / raw)
  To: Ben Greear; +Cc: linux-wireless

On Thu, 2011-11-17 at 09:22 -0800, Ben Greear wrote:

> > We can't, but can't we like assign sdata->ht_caps = sband->ht_caps? Or
> > maybe even calculate restricted HT caps for both 2.4 and 5 GHz?
> > Basically what I was thinking is this:
> 
> > struct sub_if_data {
> > 	...
> > 	struct ieee80211_sta_ht_cap ht_cap[num_bands];
> > 	...
> > };
> 
> So we'd have to copy this data from sbands[] upon creation
> of the interface to make sure it is always initialized?
> This would allow us to easily support the overrides for
> non-station interfaces, so I do like that benefit.
> 
> Can the sbands[] data ever change for any reason once
> an interface is created?  If not, then probably this is
> doable.  If it can change, then we are screwed.

I don't think it can change but it's not const (I think?) so drivers
might do stupid things -- but that would be buggy anyway.

> It seems you are basically wanting to copy the sband[] data
> local to each interface (sdata), and then we would remove sband from most
> (or all?) of the method calls that deal with sband->ht_caps?

I guess. Though we still need the band to know which one to use (I'm
reluctant to say we just need one due to channel switching etc.)

> Looks like quite a bit of code churn, and probably requiring at least two
> patches:
>   * Get rid of sband usage by copying the sband data into sdata
>   * Add the over-ride logic

Yeah. It might be *too much* churn, but that's what I was thinking. If
you think it's likely too much churn let's go with what you have now,
there are only a few minor things about it I don't like much.

johannes


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8 2/2] mac80211:  Support ht-cap over-rides.
  2011-11-17 17:25               ` Johannes Berg
@ 2011-11-17 17:42                 ` Ben Greear
  0 siblings, 0 replies; 20+ messages in thread
From: Ben Greear @ 2011-11-17 17:42 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On 11/17/2011 09:25 AM, Johannes Berg wrote:
> On Thu, 2011-11-17 at 09:22 -0800, Ben Greear wrote:
>
>>> We can't, but can't we like assign sdata->ht_caps = sband->ht_caps? Or
>>> maybe even calculate restricted HT caps for both 2.4 and 5 GHz?
>>> Basically what I was thinking is this:
>>
>>> struct sub_if_data {
>>> 	...
>>> 	struct ieee80211_sta_ht_cap ht_cap[num_bands];
>>> 	...
>>> };
>>
>> So we'd have to copy this data from sbands[] upon creation
>> of the interface to make sure it is always initialized?
>> This would allow us to easily support the overrides for
>> non-station interfaces, so I do like that benefit.
>>
>> Can the sbands[] data ever change for any reason once
>> an interface is created?  If not, then probably this is
>> doable.  If it can change, then we are screwed.
>
> I don't think it can change but it's not const (I think?) so drivers
> might do stupid things -- but that would be buggy anyway.

I can imagine someone wanting to poke at a driver to enable/disable
features, but no idea if it is done yet.

>> It seems you are basically wanting to copy the sband[] data
>> local to each interface (sdata), and then we would remove sband from most
>> (or all?) of the method calls that deal with sband->ht_caps?
>
> I guess. Though we still need the band to know which one to use (I'm
> reluctant to say we just need one due to channel switching etc.)
>
>> Looks like quite a bit of code churn, and probably requiring at least two
>> patches:
>>    * Get rid of sband usage by copying the sband data into sdata
>>    * Add the over-ride logic
>
> Yeah. It might be *too much* churn, but that's what I was thinking. If
> you think it's likely too much churn let's go with what you have now,
> there are only a few minor things about it I don't like much.

I would much rather use something similar to my last-posted patch.  I
think it is a lot less risky than rewriting all of the sband usage.

I'll rebase against the latest wireless-testing and post a patch
for further review, hopefully later today.

Thanks,
Ben

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2011-11-17 17:42 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-08 19:36 [PATCH v8 1/2] wireless: Support ht-capabilities over-rides greearb
2011-11-08 19:36 ` [PATCH v8 2/2] mac80211: Support ht-cap over-rides greearb
2011-11-08 20:09   ` Johannes Berg
2011-11-08 20:44     ` Ben Greear
2011-11-08 20:58       ` Johannes Berg
2011-11-08 21:00         ` Johannes Berg
2011-11-08 21:06         ` Ben Greear
2011-11-08 21:08           ` Johannes Berg
2011-11-08 20:12   ` Johannes Berg
2011-11-08 20:17     ` Johannes Berg
2011-11-08 20:58     ` Ben Greear
2011-11-08 21:02       ` Johannes Berg
2011-11-08 23:11         ` Ben Greear
2011-11-09  8:37           ` Johannes Berg
2011-11-10 19:25         ` Ben Greear
2011-11-17 11:28           ` Johannes Berg
2011-11-17 17:22             ` Ben Greear
2011-11-17 17:25               ` Johannes Berg
2011-11-17 17:42                 ` Ben Greear
2011-11-08 20:07 ` [PATCH v8 1/2] wireless: Support ht-capabilities over-rides Johannes Berg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.