[PATCH] Support Cisco DTPC IE to reduce transmit power

[PATCH] Support Cisco DTPC IE to reduce transmit power

[Steinar H. Gunderson]

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

Hi,

I noticed the other day that Linux and Cisco don't always seem to speak the
same language when it comes to reducing the client transmit power. I've
written a quick patch to try to remedy it; please consider applying to
the wireless tree.

/* Steinar */
-- 
Homepage: http://www.sesse.net/

[-- Attachment #2: 0001-Support-DTPC-IE-from-Cisco-Client-eXtensions.patch --]
[-- Type: text/x-diff, Size: 5014 bytes --]

>From d8fc81539016ae8663b0f479ad95cc42f286ec0b Mon Sep 17 00:00:00 2001
From: "Steinar H. Gunderson" <sgunderson@bigfoot.com>
Date: Sat, 23 Aug 2014 23:22:08 +0200
Subject: [PATCH] Support DTPC IE (from Cisco Client eXtensions)

Linux already supports 802.11h, where the access point can tell the
client to reduce its transmission power. However, 802.11h is only
defined for 5 GHz, where the need for this is much smaller than on
2.4 GHz.

Cisco has their own solution, called DTPC (Dynamic Transmit Power
Control).  Cisco APs on a controller sometimes but not always send
802.11h; they always send DTPC, even on 2.4 GHz. This patch adds support
for parsing and honoring the DTPC IE if there is no 802.11h element;
the format is not documented, but very simple.

Tested (on top of wireless.git and on 3.16.1) against a Cisco Aironet
1142 joined to a Cisco 2504 WLC, by setting various transmit power
levels for the given access points and observing the results.
The Wireshark 802.11 dissector agrees with the interpretation of the
element, except for negative numbers, which we guard against to be
conservative.

Signed-Off-By: Steinar H. Gunderson <sgunderson@bigfoot.com>
---
 include/linux/ieee80211.h  |    3 ++-
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |   27 +++++++++++++++++++++++++++
 net/mac80211/util.c        |   19 +++++++++++++++++++
 4 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 63ab3873..d0ec287 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1806,7 +1806,8 @@ enum ieee80211_eid {
 	WLAN_EID_DMG_TSPEC = 146,
 	WLAN_EID_DMG_AT = 147,
 	WLAN_EID_DMG_CAP = 148,
-	/* 149-150 reserved for Cisco */
+	/* 149 reserved for Cisco */
+	WLAN_EID_CISCO_VENDOR_SPECIFIC = 150,
 	WLAN_EID_DMG_OPERATION = 151,
 	WLAN_EID_DMG_BSS_PARAM_CHANGE = 152,
 	WLAN_EID_DMG_BEAM_REFINEMENT = 153,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ef7a089..e995556 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1367,6 +1367,7 @@ struct ieee802_11_elems {
 	const struct ieee80211_wide_bw_chansw_ie *wide_bw_chansw_ie;
 	const u8 *country_elem;
 	const u8 *pwr_constr_elem;
+	const u8 *cisco_dtpc_elem;
 	const struct ieee80211_timeout_interval_ie *timeout_int;
 	const u8 *opmode_notif;
 	const struct ieee80211_sec_chan_offs_ie *sec_chan_offs;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 31a8afa..1c8c0a3 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1227,6 +1227,30 @@ static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 	return 0;
 }
 
+static u32 ieee80211_handle_cisco_dtpc(struct ieee80211_sub_if_data *sdata,
+				       struct ieee80211_channel *channel,
+				       const u8 *cisco_dtpc_elem)
+{
+	/* From practical testing, the two data bytes of the DTPC element
+	 * seem to contain the requested dBm level in little-endian.
+	 * It never seems to go negative even if -1 dBm is requested
+	 * (capped at zero), but just to be sure, we do it ourselves.
+	 */
+	__s16 new_ap_level = (__s16) le16_to_cpup((__le16 *)(cisco_dtpc_elem + 4));
+	new_ap_level = max_t(__s16, 0, new_ap_level);
+
+	if (sdata->ap_power_level == new_ap_level)
+		return 0;
+
+	sdata_info(sdata,
+		   "Limiting TX power to %d dBm as advertised by %pM\n",
+		   new_ap_level, sdata->u.mgd.bssid);
+	sdata->ap_power_level = new_ap_level;
+	if (__ieee80211_recalc_txpower(sdata))
+		return BSS_CHANGED_TXPOWER;
+	return 0;
+}
+
 /* powersave */
 static void ieee80211_enable_ps(struct ieee80211_local *local,
 				struct ieee80211_sub_if_data *sdata)
@@ -3197,6 +3221,9 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 						       elems.country_elem,
 						       elems.country_elem_len,
 						       elems.pwr_constr_elem);
+	else if (elems.cisco_dtpc_elem) 
+		changed |= ieee80211_handle_cisco_dtpc(sdata, chan,
+						       elems.cisco_dtpc_elem);
 
 	ieee80211_bss_info_change_notify(sdata, changed);
 }
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 725af7a..a253487 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1014,6 +1014,25 @@ u32 ieee802_11_parse_elems_crc(const u8 *start, size_t len, bool action,
 			}
 			elems->pwr_constr_elem = pos;
 			break;
+		case WLAN_EID_CISCO_VENDOR_SPECIFIC:
+			/* Lots of different options exist, but we only care about the
+			 * Dynamic Transmit Power Control element. First check
+			 * for the Cisco OUI, then for the DTPC tag (0x00).
+			 */
+			if (elen < 4) {
+				elem_parse_failed = true;
+				break;
+			}
+			if (pos[0] != 0x00 || pos[1] != 0x40 ||
+			    pos[2] != 0x96 || pos[3] != 0x00) {
+				break;
+			}
+			if (elen != 6) {
+				elem_parse_failed = true;
+				break;
+			}
+			elems->cisco_dtpc_elem = pos;
+			break;
 		case WLAN_EID_TIMEOUT_INTERVAL:
 			if (elen >= sizeof(struct ieee80211_timeout_interval_ie))
 				elems->timeout_int = (void *)pos;
-- 
1.7.10.4

Re: [PATCH] Support Cisco DTPC IE to reduce transmit power

[Steinar H. Gunderson]

On Sat, Aug 23, 2014 at 11:28:27PM +0200, Steinar H. Gunderson wrote:
> I noticed the other day that Linux and Cisco don't always seem to speak the
> same language when it comes to reducing the client transmit power. I've
> written a quick patch to try to remedy it; please consider applying to
> the wireless tree.

I looked in the CLI as well, and it pretty clearly indicates that the range
here is -127 to 127 dBm, which means I've changed my mind; this is not a
two-byte value. Updated patch attached.

/* Steinar */
-- 
Homepage: http://www.sesse.net/

[PATCH] Support DTPC IE (from Cisco Client eXtensions)

[Steinar H. Gunderson]

Linux already supports 802.11h, where the access point can tell the
client to reduce its transmission power. However, 802.11h is only
defined for 5 GHz, where the need for this is much smaller than on
2.4 GHz.

Cisco has their own solution, called DTPC (Dynamic Transmit Power
Control).  Cisco APs on a controller sometimes but not always send
802.11h; they always send DTPC, even on 2.4 GHz. This patch adds support
for parsing and honoring the DTPC IE if there is no 802.11h element;
the format is not documented, but very simple.

Tested (on top of wireless.git and on 3.16.1) against a Cisco Aironet
1142 joined to a Cisco 2504 WLC, by setting various transmit power
levels for the given access points and observing the results.
The Wireshark 802.11 dissector agrees with the interpretation of the
element, except for negative numbers, which seem to never happen
anyway.

Signed-off-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
---
 include/linux/ieee80211.h  |    3 ++-
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |   27 +++++++++++++++++++++++++++
 net/mac80211/util.c        |   19 +++++++++++++++++++
 4 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 63ab3873..d0ec287 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1806,7 +1806,8 @@ enum ieee80211_eid {
 	WLAN_EID_DMG_TSPEC = 146,
 	WLAN_EID_DMG_AT = 147,
 	WLAN_EID_DMG_CAP = 148,
-	/* 149-150 reserved for Cisco */
+	/* 149 reserved for Cisco */
+	WLAN_EID_CISCO_VENDOR_SPECIFIC = 150,
 	WLAN_EID_DMG_OPERATION = 151,
 	WLAN_EID_DMG_BSS_PARAM_CHANGE = 152,
 	WLAN_EID_DMG_BEAM_REFINEMENT = 153,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ef7a089..e995556 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1367,6 +1367,7 @@ struct ieee802_11_elems {
 	const struct ieee80211_wide_bw_chansw_ie *wide_bw_chansw_ie;
 	const u8 *country_elem;
 	const u8 *pwr_constr_elem;
+	const u8 *cisco_dtpc_elem;
 	const struct ieee80211_timeout_interval_ie *timeout_int;
 	const u8 *opmode_notif;
 	const struct ieee80211_sec_chan_offs_ie *sec_chan_offs;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 31a8afa..39167c1 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1227,6 +1227,30 @@ static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 	return 0;
 }
 
+static u32 ieee80211_handle_cisco_dtpc(struct ieee80211_sub_if_data *sdata,
+				       struct ieee80211_channel *channel,
+				       const u8 *cisco_dtpc_elem)
+{
+	/* From practical testing, the first data byte of the DTPC element
+	 * seems to contain the requested dBm level, and the CLI on Cisco
+	 * APs clearly state the range is -127 to 127 dBm, which indicates
+	 * a signed byte, although it seemingly never actually goes negative.
+	 * The other byte seems to always be zero.
+	 */
+	__s8 new_ap_level = (__s8)cisco_dtpc_elem[4];
+
+	if (sdata->ap_power_level == new_ap_level)
+		return 0;
+
+	sdata_info(sdata,
+		   "Limiting TX power to %d dBm as advertised by %pM\n",
+		   new_ap_level, sdata->u.mgd.bssid);
+	sdata->ap_power_level = new_ap_level;
+	if (__ieee80211_recalc_txpower(sdata))
+		return BSS_CHANGED_TXPOWER;
+	return 0;
+}
+
 /* powersave */
 static void ieee80211_enable_ps(struct ieee80211_local *local,
 				struct ieee80211_sub_if_data *sdata)
@@ -3197,6 +3221,9 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 						       elems.country_elem,
 						       elems.country_elem_len,
 						       elems.pwr_constr_elem);
+	else if (elems.cisco_dtpc_elem)
+		changed |= ieee80211_handle_cisco_dtpc(sdata, chan,
+						       elems.cisco_dtpc_elem);
 
 	ieee80211_bss_info_change_notify(sdata, changed);
 }
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 725af7a..a253487 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1014,6 +1014,25 @@ u32 ieee802_11_parse_elems_crc(const u8 *start, size_t len, bool action,
 			}
 			elems->pwr_constr_elem = pos;
 			break;
+		case WLAN_EID_CISCO_VENDOR_SPECIFIC:
+			/* Lots of different options exist, but we only care about the
+			 * Dynamic Transmit Power Control element. First check
+			 * for the Cisco OUI, then for the DTPC tag (0x00).
+			 */
+			if (elen < 4) {
+				elem_parse_failed = true;
+				break;
+			}
+			if (pos[0] != 0x00 || pos[1] != 0x40 ||
+			    pos[2] != 0x96 || pos[3] != 0x00) {
+				break;
+			}
+			if (elen != 6) {
+				elem_parse_failed = true;
+				break;
+			}
+			elems->cisco_dtpc_elem = pos;
+			break;
 		case WLAN_EID_TIMEOUT_INTERVAL:
 			if (elen >= sizeof(struct ieee80211_timeout_interval_ie))
 				elems->timeout_int = (void *)pos;
-- 
1.7.10.4

Re: [PATCH] Support DTPC IE (from Cisco Client eXtensions)

[Johannes Berg]

On Sun, 2014-08-24 at 12:08 +0200, Steinar H. Gunderson wrote:
> Linux already supports 802.11h, where the access point can tell the
> client to reduce its transmission power. However, 802.11h is only
> defined for 5 GHz, where the need for this is much smaller than on
> 2.4 GHz.
> 
> Cisco has their own solution, called DTPC (Dynamic Transmit Power
> Control).  Cisco APs on a controller sometimes but not always send
> 802.11h; they always send DTPC, even on 2.4 GHz. This patch adds support
> for parsing and honoring the DTPC IE if there is no 802.11h element;
> the format is not documented, but very simple.
> 
> Tested (on top of wireless.git and on 3.16.1) against a Cisco Aironet
> 1142 joined to a Cisco 2504 WLC, by setting various transmit power
> levels for the given access points and observing the results.
> The Wireshark 802.11 dissector agrees with the interpretation of the
> element, except for negative numbers, which seem to never happen
> anyway.

Can you say *why* we want this? Does it yield better behaviour on a
Cisco deployment?

johannes

Re: [PATCH] Support DTPC IE (from Cisco Client eXtensions)

[Steinar H. Gunderson]

On Thu, Aug 28, 2014 at 09:43:38AM +0200, Johannes Berg wrote:
> Can you say *why* we want this? Does it yield better behaviour on a
> Cisco deployment?

Yes. The basic idea is: When your density goes up (many clients, many access
points), the 2.4 GHz band becomes very crowded and you get interference
across cells. The controller observes this (by seeing that the APs see each
other too strongly), and reduces the transmit power to reduce the cell size.
(This might be counterintuitive, because people's initial idea for better
Wi-Fi usually involves turning power _up_, but that only makes the problem
here worse, since the problem is interference, not range.)

It then also asks the clients to do the same (through DTPC), because the
clients' traffic of course also contributes to the interference. We should
honor its request, just as we honor it when it comes through 802.11h or
802.11d. If the controller detects that this creates coverage holes, it will
turn the power back up and just live with the interference.

Like the subject says, this is part of CCX (Cisco's set of wireless
extensions), so as far as I know, any CCX-certified driver, e.g. Intel
drivers on Windows, already support this.

/* Steinar */
-- 
Homepage: http://www.sesse.net/

Re: [PATCH] Support DTPC IE (from Cisco Client eXtensions)

[Steinar H. Gunderson]

On Thu, Aug 28, 2014 at 10:37:38AM +0200, Steinar H. Gunderson wrote:
> We should honor its request, just as we honor it when it comes through
> 802.11h or 802.11d.

I've been running with this a few days now, and I've noticed that while the
patch works fine, it is not entirely complete.

The problem is that sometimes the AP can send _both_ 802.11h and the DTPC IE;
the 802.11h IE seemingly because there's some kind of regulatory pressure
(ie., “something detected on your channel, you have to go down”), and the
DTPC IE due to the configured lower transmit power. In this case, 802.11h
says 21 (24 - 3) dBm and DTPC 7 dBm, and we pick the former.

In this situation, my current patch lets 802.11h override (on the grounds that
it's an official standard, and that it's required for regulatory compliance),
but it should probably do a min() of the two advertised power levels instead.
Does this sound reasonable? I'll be sending a patch later today.

/* Steinar */
-- 
Homepage: http://www.sesse.net/

[PATCH 1/2] mac80211: split 802.11h parsing from transmit power policy

[Steinar H. Gunderson]

Decouple the logic of parsing the 802.11d and 802.11h IEs from the
part of deciding what to do about the data (messaging, clamping to
0 dBm, doing the actual setting). This paves the way for the next
patch, which introduces more data sources for transmit power limitation.

Signed-off-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
---
 net/mac80211/mlme.c |   62 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 20 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index b82a12a..5c90e49 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1157,19 +1157,22 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 			  TU_TO_EXP_TIME(csa_ie.count * cbss->beacon_interval));
 }
 
-static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
-				       struct ieee80211_channel *channel,
-				       const u8 *country_ie, u8 country_ie_len,
-				       const u8 *pwr_constr_elem)
+static bool ieee80211_find_80211h_pwr_constr(
+	struct ieee80211_sub_if_data *sdata,
+	struct ieee80211_channel *channel,
+	const u8 *country_ie, u8 country_ie_len,
+	const u8 *pwr_constr_elem,
+	int *chan_pwr,
+	int *pwr_reduction)
 {
 	struct ieee80211_country_ie_triplet *triplet;
 	int chan = ieee80211_frequency_to_channel(channel->center_freq);
-	int i, chan_pwr, chan_increment, new_ap_level;
+	int i, chan_increment;
 	bool have_chan_pwr = false;
 
 	/* Invalid IE */
 	if (country_ie_len % 2 || country_ie_len < IEEE80211_COUNTRY_IE_MIN_LEN)
-		return 0;
+		return false;
 
 	triplet = (void *)(country_ie + 3);
 	country_ie_len -= 3;
@@ -1197,7 +1200,7 @@ static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 		for (i = 0; i < triplet->chans.num_channels; i++) {
 			if (first_channel + i * chan_increment == chan) {
 				have_chan_pwr = true;
-				chan_pwr = triplet->chans.max_power;
+				*chan_pwr = triplet->chans.max_power;
 				break;
 			}
 		}
@@ -1209,19 +1212,41 @@ static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 		country_ie_len -= 3;
 	}
 
-	if (!have_chan_pwr)
-		return 0;
+	if (have_chan_pwr)
+		*pwr_reduction = *pwr_constr_elem;
+	return have_chan_pwr;
+}
 
-	new_ap_level = max_t(int, 0, chan_pwr - *pwr_constr_elem);
+static bool ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
+					struct ieee80211_channel *channel,
+					struct ieee80211_mgmt *mgmt,
+					const u8 *country_ie, u8 country_ie_len,
+					const u8 *pwr_constr_ie)
+{
+	bool has_80211h_pwr = false;
+	int chan_pwr, pwr_reduction_80211h;
+	int new_ap_level;
 
-	if (sdata->ap_power_level == new_ap_level)
+	if (country_ie && pwr_constr_ie &&
+	    mgmt->u.probe_resp.capab_info &
+		cpu_to_le16(WLAN_CAPABILITY_SPECTRUM_MGMT)) {
+		has_80211h_pwr = ieee80211_find_80211h_pwr_constr(
+			sdata, channel, country_ie, country_ie_len,
+			pwr_constr_ie, &chan_pwr, &pwr_reduction_80211h);
+		new_ap_level = max_t(int, 0, chan_pwr - pwr_reduction_80211h);
+	}
+
+	if (!has_80211h_pwr)
 		return 0;
 
 	sdata_info(sdata,
 		   "Limiting TX power to %d (%d - %d) dBm as advertised by %pM\n",
-		   new_ap_level, chan_pwr, *pwr_constr_elem,
+		   new_ap_level, chan_pwr, pwr_reduction_80211h,
 		   sdata->u.mgd.bssid);
-	sdata->ap_power_level = new_ap_level;
+
+	if (sdata->ap_power_level == new_ap_level)
+		return 0;
+
 	if (__ieee80211_recalc_txpower(sdata))
 		return BSS_CHANGED_TXPOWER;
 	return 0;
@@ -3190,13 +3215,10 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 					    rx_status->band, true);
 	mutex_unlock(&local->sta_mtx);
 
-	if (elems.country_elem && elems.pwr_constr_elem &&
-	    mgmt->u.probe_resp.capab_info &
-				cpu_to_le16(WLAN_CAPABILITY_SPECTRUM_MGMT))
-		changed |= ieee80211_handle_pwr_constr(sdata, chan,
-						       elems.country_elem,
-						       elems.country_elem_len,
-						       elems.pwr_constr_elem);
+	changed |= ieee80211_handle_pwr_constr(sdata, chan, mgmt,
+					       elems.country_elem,
+					       elems.country_elem_len,
+					       elems.pwr_constr_elem);
 
 	ieee80211_bss_info_change_notify(sdata, changed);
 }
-- 
1.7.10.4

[PATCH 2/2] mac80211: support DTPC IE (from Cisco Client eXtensions)

[Steinar H. Gunderson]

Linux already supports 802.11h, where the access point can tell the
client to reduce its transmission power. However, 802.11h is only
defined for 5 GHz, where the need for this is much smaller than on
2.4 GHz.

Cisco has their own solution, called DTPC (Dynamic Transmit Power
Control). Cisco APs on a controller sometimes but not always send
802.11h; they always send DTPC, even on 2.4 GHz. This patch adds support
for parsing and honoring the DTPC IE in addition to the 802.11h
element (they do not always contain the same limits, so both must
be honored); the format is not documented, but very simple.

Tested (on top of wireless.git and on 3.16.1) against a Cisco Aironet
1142 joined to a Cisco 2504 WLC, by setting various transmit power
levels for the given access points and observing the results.
The Wireshark 802.11 dissector agrees with the interpretation of the
element, except for negative numbers, which seem to never happen
anyway.

Signed-off-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
---
 include/linux/ieee80211.h  |    3 ++-
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |   55 ++++++++++++++++++++++++++++++++++++--------
 net/mac80211/util.c        |   20 ++++++++++++++++
 4 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 63ab3873..d0ec287 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1806,7 +1806,8 @@ enum ieee80211_eid {
 	WLAN_EID_DMG_TSPEC = 146,
 	WLAN_EID_DMG_AT = 147,
 	WLAN_EID_DMG_CAP = 148,
-	/* 149-150 reserved for Cisco */
+	/* 149 reserved for Cisco */
+	WLAN_EID_CISCO_VENDOR_SPECIFIC = 150,
 	WLAN_EID_DMG_OPERATION = 151,
 	WLAN_EID_DMG_BSS_PARAM_CHANGE = 152,
 	WLAN_EID_DMG_BEAM_REFINEMENT = 153,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ef7a089..e995556 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1367,6 +1367,7 @@ struct ieee802_11_elems {
 	const struct ieee80211_wide_bw_chansw_ie *wide_bw_chansw_ie;
 	const u8 *country_elem;
 	const u8 *pwr_constr_elem;
+	const u8 *cisco_dtpc_elem;
 	const struct ieee80211_timeout_interval_ie *timeout_int;
 	const u8 *opmode_notif;
 	const struct ieee80211_sec_chan_offs_ie *sec_chan_offs;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 5c90e49..49cc669 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1217,14 +1217,31 @@ static bool ieee80211_find_80211h_pwr_constr(
 	return have_chan_pwr;
 }
 
+static bool ieee80211_find_cisco_dtpc(struct ieee80211_sub_if_data *sdata,
+				      struct ieee80211_channel *channel,
+				      const u8 *cisco_dtpc_ie,
+				      int *pwr_level)
+{
+	/* From practical testing, the first data byte of the DTPC element
+	 * seems to contain the requested dBm level, and the CLI on Cisco
+	 * APs clearly state the range is -127 to 127 dBm, which indicates
+	 * a signed byte, although it seemingly never actually goes negative.
+	 * The other byte seems to always be zero.
+	 */
+	*pwr_level = (__s8)cisco_dtpc_ie[4];
+	return true;
+}
+
 static bool ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 					struct ieee80211_channel *channel,
 					struct ieee80211_mgmt *mgmt,
 					const u8 *country_ie, u8 country_ie_len,
-					const u8 *pwr_constr_ie)
+					const u8 *pwr_constr_ie,
+					const u8 *cisco_dtpc_ie)
 {
-	bool has_80211h_pwr = false;
-	int chan_pwr, pwr_reduction_80211h;
+	bool has_80211h_pwr = false, has_cisco_pwr = false;
+	int chan_pwr = 0, pwr_reduction_80211h = 0;
+	int pwr_level_cisco, pwr_level_80211h;
 	int new_ap_level;
 
 	if (country_ie && pwr_constr_ie &&
@@ -1233,16 +1250,33 @@ static bool ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 		has_80211h_pwr = ieee80211_find_80211h_pwr_constr(
 			sdata, channel, country_ie, country_ie_len,
 			pwr_constr_ie, &chan_pwr, &pwr_reduction_80211h);
-		new_ap_level = max_t(int, 0, chan_pwr - pwr_reduction_80211h);
+		pwr_level_80211h =
+			max_t(int, 0, chan_pwr - pwr_reduction_80211h);
 	}
 
-	if (!has_80211h_pwr)
+	if (cisco_dtpc_ie)
+		has_cisco_pwr = ieee80211_find_cisco_dtpc(
+			sdata, channel, cisco_dtpc_ie, &pwr_level_cisco);
+
+	if (!has_80211h_pwr && !has_cisco_pwr)
 		return 0;
 
-	sdata_info(sdata,
-		   "Limiting TX power to %d (%d - %d) dBm as advertised by %pM\n",
-		   new_ap_level, chan_pwr, pwr_reduction_80211h,
-		   sdata->u.mgd.bssid);
+	/* If we have both 802.11h and Cisco DTPC, apply both limits
+	 * by picking the smallest of the two power levels advertised.
+	 */
+	if (has_80211h_pwr &&
+	    (!has_cisco_pwr || pwr_level_80211h <= pwr_level_cisco)) {
+		sdata_info(sdata,
+			   "Limiting TX power to %d (%d - %d) dBm as advertised by %pM\n",
+			   pwr_level_80211h, chan_pwr, pwr_reduction_80211h,
+			   sdata->u.mgd.bssid);
+		new_ap_level = pwr_level_80211h;
+	} else {  /* has_cisco_pwr is always true here. */
+		sdata_info(sdata,
+			   "Limiting TX power to %d dBm as advertised by %pM\n",
+			   pwr_level_cisco, sdata->u.mgd.bssid);
+		new_ap_level = pwr_level_cisco;
+	}
 
 	if (sdata->ap_power_level == new_ap_level)
 		return 0;
@@ -3218,7 +3252,8 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 	changed |= ieee80211_handle_pwr_constr(sdata, chan, mgmt,
 					       elems.country_elem,
 					       elems.country_elem_len,
-					       elems.pwr_constr_elem);
+					       elems.pwr_constr_elem,
+					       elems.cisco_dtpc_elem);
 
 	ieee80211_bss_info_change_notify(sdata, changed);
 }
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 725af7a..56105f4 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1014,6 +1014,26 @@ u32 ieee802_11_parse_elems_crc(const u8 *start, size_t len, bool action,
 			}
 			elems->pwr_constr_elem = pos;
 			break;
+		case WLAN_EID_CISCO_VENDOR_SPECIFIC:
+			/* Lots of different options exist, but we only care
+			 * about the Dynamic Transmit Power Control element.
+			 * First check for the Cisco OUI, then for the DTPC
+			 * tag (0x00).
+			 */
+			if (elen < 4) {
+				elem_parse_failed = true;
+				break;
+			}
+			if (pos[0] != 0x00 || pos[1] != 0x40 ||
+			    pos[2] != 0x96 || pos[3] != 0x00) {
+				break;
+			}
+			if (elen != 6) {
+				elem_parse_failed = true;
+				break;
+			}
+			elems->cisco_dtpc_elem = pos;
+			break;
 		case WLAN_EID_TIMEOUT_INTERVAL:
 			if (elen >= sizeof(struct ieee80211_timeout_interval_ie))
 				elems->timeout_int = (void *)pos;
-- 
1.7.10.4

Re: [PATCH] Support DTPC IE (from Cisco Client eXtensions)

[Steinar H. Gunderson]

On Fri, Aug 29, 2014 at 10:19:02AM +0200, Steinar H. Gunderson wrote:
> I'll be sending a patch later today.

...and of course, in the refactoring, I removed the part that actually sets
sdata->ap_power_level, so my new patch series is borked, too. I'll fix,
do some more testing, and then send a new patch series.

Feedback on the general structure is very welcome, though!

/* Steinar */
-- 
Homepage: http://www.sesse.net/

Re: [PATCH] Support DTPC IE (from Cisco Client eXtensions)

[Steinar H. Gunderson]

On Sat, Aug 30, 2014 at 02:24:18AM +0200, Steinar H. Gunderson wrote:
> ...and of course, in the refactoring, I removed the part that actually sets
> sdata->ap_power_level, so my new patch series is borked, too. I'll fix,
> do some more testing, and then send a new patch series.

The fix was trivial (just add back the line). I'll be traveling a bit the
next two weeks, so I guess I'll be seeing a few different installations and
see how this pans out. But in my home network, it now looks to be working
very well.

Sending v4 of the patch series as response to this email.

/* Steinar */
-- 
Homepage: http://www.sesse.net/

[PATCH v5 1/2] mac80211: split 802.11h parsing from transmit power policy

[Steinar H. Gunderson]

Decouple the logic of parsing the 802.11d and 802.11h IEs from the
part of deciding what to do about the data (messaging, clamping to
0 dBm, doing the actual setting). This paves the way for the next
patch, which introduces more data sources for transmit power limitation.

Signed-off-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
---
 net/mac80211/mlme.c |   61 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 19 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index b82a12a..524c7cc 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1157,19 +1157,22 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 			  TU_TO_EXP_TIME(csa_ie.count * cbss->beacon_interval));
 }
 
-static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
-				       struct ieee80211_channel *channel,
-				       const u8 *country_ie, u8 country_ie_len,
-				       const u8 *pwr_constr_elem)
+static bool ieee80211_find_80211h_pwr_constr(
+	struct ieee80211_sub_if_data *sdata,
+	struct ieee80211_channel *channel,
+	const u8 *country_ie, u8 country_ie_len,
+	const u8 *pwr_constr_elem,
+	int *chan_pwr,
+	int *pwr_reduction)
 {
 	struct ieee80211_country_ie_triplet *triplet;
 	int chan = ieee80211_frequency_to_channel(channel->center_freq);
-	int i, chan_pwr, chan_increment, new_ap_level;
+	int i, chan_increment;
 	bool have_chan_pwr = false;
 
 	/* Invalid IE */
 	if (country_ie_len % 2 || country_ie_len < IEEE80211_COUNTRY_IE_MIN_LEN)
-		return 0;
+		return false;
 
 	triplet = (void *)(country_ie + 3);
 	country_ie_len -= 3;
@@ -1197,7 +1200,7 @@ static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 		for (i = 0; i < triplet->chans.num_channels; i++) {
 			if (first_channel + i * chan_increment == chan) {
 				have_chan_pwr = true;
-				chan_pwr = triplet->chans.max_power;
+				*chan_pwr = triplet->chans.max_power;
 				break;
 			}
 		}
@@ -1209,18 +1212,41 @@ static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 		country_ie_len -= 3;
 	}
 
-	if (!have_chan_pwr)
-		return 0;
+	if (have_chan_pwr)
+		*pwr_reduction = *pwr_constr_elem;
+	return have_chan_pwr;
+}
 
-	new_ap_level = max_t(int, 0, chan_pwr - *pwr_constr_elem);
+static bool ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
+					struct ieee80211_channel *channel,
+					struct ieee80211_mgmt *mgmt,
+					const u8 *country_ie, u8 country_ie_len,
+					const u8 *pwr_constr_ie)
+{
+	bool has_80211h_pwr = false;
+	int chan_pwr, pwr_reduction_80211h;
+	int new_ap_level;
 
-	if (sdata->ap_power_level == new_ap_level)
+	if (country_ie && pwr_constr_ie &&
+	    mgmt->u.probe_resp.capab_info &
+		cpu_to_le16(WLAN_CAPABILITY_SPECTRUM_MGMT)) {
+		has_80211h_pwr = ieee80211_find_80211h_pwr_constr(
+			sdata, channel, country_ie, country_ie_len,
+			pwr_constr_ie, &chan_pwr, &pwr_reduction_80211h);
+		new_ap_level = max_t(int, 0, chan_pwr - pwr_reduction_80211h);
+	}
+
+	if (!has_80211h_pwr)
 		return 0;
 
 	sdata_info(sdata,
 		   "Limiting TX power to %d (%d - %d) dBm as advertised by %pM\n",
-		   new_ap_level, chan_pwr, *pwr_constr_elem,
+		   new_ap_level, chan_pwr, pwr_reduction_80211h,
 		   sdata->u.mgd.bssid);
+
+	if (sdata->ap_power_level == new_ap_level)
+		return 0;
+
 	sdata->ap_power_level = new_ap_level;
 	if (__ieee80211_recalc_txpower(sdata))
 		return BSS_CHANGED_TXPOWER;
@@ -3190,13 +3216,10 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 					    rx_status->band, true);
 	mutex_unlock(&local->sta_mtx);
 
-	if (elems.country_elem && elems.pwr_constr_elem &&
-	    mgmt->u.probe_resp.capab_info &
-				cpu_to_le16(WLAN_CAPABILITY_SPECTRUM_MGMT))
-		changed |= ieee80211_handle_pwr_constr(sdata, chan,
-						       elems.country_elem,
-						       elems.country_elem_len,
-						       elems.pwr_constr_elem);
+	changed |= ieee80211_handle_pwr_constr(sdata, chan, mgmt,
+					       elems.country_elem,
+					       elems.country_elem_len,
+					       elems.pwr_constr_elem);
 
 	ieee80211_bss_info_change_notify(sdata, changed);
 }
-- 
1.7.10.4

Re: [PATCH v5 1/2] mac80211: split 802.11h parsing from transmit power policy

[Johannes Berg]

On Fri, 2014-08-29 at 19:27 +0200, Steinar H. Gunderson wrote:

> ieee80211_find_80211h_pwr_constr

Please format the code like this:

static bool
ieee80211_find_80211h_pwr_constr(struct ieee80211_sub_if_data *sdata, 
                                 struct ieee80211_channel *channel,
                                 const u8 *country_ie, u8
country_ie_len,
                                 const u8 *pwr_constr_elem,
                                 int *chan_pwr, int *pwr_reduction)

Do we need the pwr_reduction? Couldn't we just return it? We don't
handle 0/negative anyway, do we?

> -static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,

> +static bool ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,

I think you meant to keep u32 here? Otherwise return values from the
function simply get converted to 0/1 which doesn't seem right.

johannes

Re: [PATCH v5 1/2] mac80211: split 802.11h parsing from transmit power policy

[Steinar H. Gunderson]

Thanks for reviewing. I'll send out a new patch series as soon as I'm
through all of your comments.

On Wed, Sep 03, 2014 at 01:56:40PM +0200, Johannes Berg wrote:
> Please format the code like this:
> 
> static bool
> ieee80211_find_80211h_pwr_constr(struct ieee80211_sub_if_data *sdata, 
>                                  struct ieee80211_channel *channel,
>                                  const u8 *country_ie, u8
> country_ie_len,
>                                  const u8 *pwr_constr_elem,
>                                  int *chan_pwr, int *pwr_reduction)

Done.

> Do we need the pwr_reduction? Couldn't we just return it? We don't
> handle 0/negative anyway, do we?

We could do the calculation directly in ieee80211_find_80211h_pwr_constr;
however, then we couldn't message the calculation in the printk (it currently
says e.g. 0 (17 - 23) dBm). I don't know if that is important or not, but I
opted to let it stay as-is.

>> -static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
> 
>> +static bool ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
> 
> I think you meant to keep u32 here? Otherwise return values from the
> function simply get converted to 0/1 which doesn't seem right.

You're right; this function is supposed to return a bitmask. Fixed.

/* Steinar */
-- 
Homepage: http://www.sesse.net/

[PATCH 1/2] mac80211: split 802.11h parsing from transmit power policy

[Steinar H. Gunderson]

Decouple the logic of parsing the 802.11d and 802.11h IEs from the
part of deciding what to do about the data (messaging, clamping to
0 dBm, doing the actual setting). This paves the way for the next
patch, which introduces more data sources for transmit power limitation.

Signed-off-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
---
 net/mac80211/mlme.c |   61 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 19 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index b82a12a..524c7cc 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1157,19 +1157,22 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 			  TU_TO_EXP_TIME(csa_ie.count * cbss->beacon_interval));
 }
 
-static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
-				       struct ieee80211_channel *channel,
-				       const u8 *country_ie, u8 country_ie_len,
-				       const u8 *pwr_constr_elem)
+static bool ieee80211_find_80211h_pwr_constr(
+	struct ieee80211_sub_if_data *sdata,
+	struct ieee80211_channel *channel,
+	const u8 *country_ie, u8 country_ie_len,
+	const u8 *pwr_constr_elem,
+	int *chan_pwr,
+	int *pwr_reduction)
 {
 	struct ieee80211_country_ie_triplet *triplet;
 	int chan = ieee80211_frequency_to_channel(channel->center_freq);
-	int i, chan_pwr, chan_increment, new_ap_level;
+	int i, chan_increment;
 	bool have_chan_pwr = false;
 
 	/* Invalid IE */
 	if (country_ie_len % 2 || country_ie_len < IEEE80211_COUNTRY_IE_MIN_LEN)
-		return 0;
+		return false;
 
 	triplet = (void *)(country_ie + 3);
 	country_ie_len -= 3;
@@ -1197,7 +1200,7 @@ static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 		for (i = 0; i < triplet->chans.num_channels; i++) {
 			if (first_channel + i * chan_increment == chan) {
 				have_chan_pwr = true;
-				chan_pwr = triplet->chans.max_power;
+				*chan_pwr = triplet->chans.max_power;
 				break;
 			}
 		}
@@ -1209,18 +1212,41 @@ static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 		country_ie_len -= 3;
 	}
 
-	if (!have_chan_pwr)
-		return 0;
+	if (have_chan_pwr)
+		*pwr_reduction = *pwr_constr_elem;
+	return have_chan_pwr;
+}
 
-	new_ap_level = max_t(int, 0, chan_pwr - *pwr_constr_elem);
+static bool ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
+					struct ieee80211_channel *channel,
+					struct ieee80211_mgmt *mgmt,
+					const u8 *country_ie, u8 country_ie_len,
+					const u8 *pwr_constr_ie)
+{
+	bool has_80211h_pwr = false;
+	int chan_pwr, pwr_reduction_80211h;
+	int new_ap_level;
 
-	if (sdata->ap_power_level == new_ap_level)
+	if (country_ie && pwr_constr_ie &&
+	    mgmt->u.probe_resp.capab_info &
+		cpu_to_le16(WLAN_CAPABILITY_SPECTRUM_MGMT)) {
+		has_80211h_pwr = ieee80211_find_80211h_pwr_constr(
+			sdata, channel, country_ie, country_ie_len,
+			pwr_constr_ie, &chan_pwr, &pwr_reduction_80211h);
+		new_ap_level = max_t(int, 0, chan_pwr - pwr_reduction_80211h);
+	}
+
+	if (!has_80211h_pwr)
 		return 0;
 
 	sdata_info(sdata,
 		   "Limiting TX power to %d (%d - %d) dBm as advertised by %pM\n",
-		   new_ap_level, chan_pwr, *pwr_constr_elem,
+		   new_ap_level, chan_pwr, pwr_reduction_80211h,
 		   sdata->u.mgd.bssid);
+
+	if (sdata->ap_power_level == new_ap_level)
+		return 0;
+
 	sdata->ap_power_level = new_ap_level;
 	if (__ieee80211_recalc_txpower(sdata))
 		return BSS_CHANGED_TXPOWER;
@@ -3190,13 +3216,10 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 					    rx_status->band, true);
 	mutex_unlock(&local->sta_mtx);
 
-	if (elems.country_elem && elems.pwr_constr_elem &&
-	    mgmt->u.probe_resp.capab_info &
-				cpu_to_le16(WLAN_CAPABILITY_SPECTRUM_MGMT))
-		changed |= ieee80211_handle_pwr_constr(sdata, chan,
-						       elems.country_elem,
-						       elems.country_elem_len,
-						       elems.pwr_constr_elem);
+	changed |= ieee80211_handle_pwr_constr(sdata, chan, mgmt,
+					       elems.country_elem,
+					       elems.country_elem_len,
+					       elems.pwr_constr_elem);
 
 	ieee80211_bss_info_change_notify(sdata, changed);
 }
-- 
1.7.10.4

[PATCH v5 2/2] mac80211: support DTPC IE (from Cisco Client eXtensions)

[Steinar H. Gunderson]

Linux already supports 802.11h, where the access point can tell the
client to reduce its transmission power. However, 802.11h is only
defined for 5 GHz, where the need for this is much smaller than on
2.4 GHz.

Cisco has their own solution, called DTPC (Dynamic Transmit Power
Control). Cisco APs on a controller sometimes but not always send
802.11h; they always send DTPC, even on 2.4 GHz. This patch adds support
for parsing and honoring the DTPC IE in addition to the 802.11h
element (they do not always contain the same limits, so both must
be honored); the format is not documented, but very simple.

Tested (on top of wireless.git and on 3.16.1) against a Cisco Aironet
1142 joined to a Cisco 2504 WLC, by setting various transmit power
levels for the given access points and observing the results.
The Wireshark 802.11 dissector agrees with the interpretation of the
element, except for negative numbers, which seem to never happen
anyway.

Signed-off-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
---
 include/linux/ieee80211.h  |    3 ++-
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |   59 +++++++++++++++++++++++++++++++++++---------
 net/mac80211/util.c        |   22 +++++++++++++++++
 4 files changed, 73 insertions(+), 12 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 63ab3873..d0ec287 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1806,7 +1806,8 @@ enum ieee80211_eid {
 	WLAN_EID_DMG_TSPEC = 146,
 	WLAN_EID_DMG_AT = 147,
 	WLAN_EID_DMG_CAP = 148,
-	/* 149-150 reserved for Cisco */
+	/* 149 reserved for Cisco */
+	WLAN_EID_CISCO_VENDOR_SPECIFIC = 150,
 	WLAN_EID_DMG_OPERATION = 151,
 	WLAN_EID_DMG_BSS_PARAM_CHANGE = 152,
 	WLAN_EID_DMG_BEAM_REFINEMENT = 153,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ef7a089..e995556 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1367,6 +1367,7 @@ struct ieee802_11_elems {
 	const struct ieee80211_wide_bw_chansw_ie *wide_bw_chansw_ie;
 	const u8 *country_elem;
 	const u8 *pwr_constr_elem;
+	const u8 *cisco_dtpc_elem;
 	const struct ieee80211_timeout_interval_ie *timeout_int;
 	const u8 *opmode_notif;
 	const struct ieee80211_sec_chan_offs_ie *sec_chan_offs;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 524c7cc..49d743b 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1217,14 +1217,31 @@ static bool ieee80211_find_80211h_pwr_constr(
 	return have_chan_pwr;
 }
 
+static bool ieee80211_find_cisco_dtpc(struct ieee80211_sub_if_data *sdata,
+				      struct ieee80211_channel *channel,
+				      const u8 *cisco_dtpc_ie,
+				      int *pwr_level)
+{
+	/* From practical testing, the first data byte of the DTPC element
+	 * seems to contain the requested dBm level, and the CLI on Cisco
+	 * APs clearly state the range is -127 to 127 dBm, which indicates
+	 * a signed byte, although it seemingly never actually goes negative.
+	 * The other byte seems to always be zero.
+	 */
+	*pwr_level = (__s8)cisco_dtpc_ie[4];
+	return true;
+}
+
 static bool ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 					struct ieee80211_channel *channel,
 					struct ieee80211_mgmt *mgmt,
 					const u8 *country_ie, u8 country_ie_len,
-					const u8 *pwr_constr_ie)
+					const u8 *pwr_constr_ie,
+					const u8 *cisco_dtpc_ie)
 {
-	bool has_80211h_pwr = false;
-	int chan_pwr, pwr_reduction_80211h;
+	bool has_80211h_pwr = false, has_cisco_pwr = false;
+	int chan_pwr = 0, pwr_reduction_80211h = 0;
+	int pwr_level_cisco, pwr_level_80211h;
 	int new_ap_level;
 
 	if (country_ie && pwr_constr_ie &&
@@ -1233,16 +1250,33 @@ static bool ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 		has_80211h_pwr = ieee80211_find_80211h_pwr_constr(
 			sdata, channel, country_ie, country_ie_len,
 			pwr_constr_ie, &chan_pwr, &pwr_reduction_80211h);
-		new_ap_level = max_t(int, 0, chan_pwr - pwr_reduction_80211h);
+		pwr_level_80211h =
+			max_t(int, 0, chan_pwr - pwr_reduction_80211h);
 	}
 
-	if (!has_80211h_pwr)
+	if (cisco_dtpc_ie)
+		has_cisco_pwr = ieee80211_find_cisco_dtpc(
+			sdata, channel, cisco_dtpc_ie, &pwr_level_cisco);
+
+	if (!has_80211h_pwr && !has_cisco_pwr)
 		return 0;
 
-	sdata_info(sdata,
-		   "Limiting TX power to %d (%d - %d) dBm as advertised by %pM\n",
-		   new_ap_level, chan_pwr, pwr_reduction_80211h,
-		   sdata->u.mgd.bssid);
+	/* If we have both 802.11h and Cisco DTPC, apply both limits
+	 * by picking the smallest of the two power levels advertised.
+	 */
+	if (has_80211h_pwr &&
+	    (!has_cisco_pwr || pwr_level_80211h <= pwr_level_cisco)) {
+		sdata_info(sdata,
+			   "Limiting TX power to %d (%d - %d) dBm as advertised by %pM\n",
+			   pwr_level_80211h, chan_pwr, pwr_reduction_80211h,
+			   sdata->u.mgd.bssid);
+		new_ap_level = pwr_level_80211h;
+	} else {  /* has_cisco_pwr is always true here. */
+		sdata_info(sdata,
+			   "Limiting TX power to %d dBm as advertised by %pM\n",
+			   pwr_level_cisco, sdata->u.mgd.bssid);
+		new_ap_level = pwr_level_cisco;
+	}
 
 	if (sdata->ap_power_level == new_ap_level)
 		return 0;
@@ -2911,7 +2945,9 @@ static void ieee80211_rx_mgmt_probe_resp(struct ieee80211_sub_if_data *sdata,
 /*
  * This is the canonical list of information elements we care about,
  * the filter code also gives us all changes to the Microsoft OUI
- * (00:50:F2) vendor IE which is used for WMM which we need to track.
+ * (00:50:F2) vendor IE which is used for WMM which we need to track,
+ * as well as the DTPC IE (part of the Cisco OUI) used for signaling
+ * changes to requested client power.
  *
  * We implement beacon filtering in software since that means we can
  * avoid processing the frame here and in cfg80211, and userspace
@@ -3219,7 +3255,8 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 	changed |= ieee80211_handle_pwr_constr(sdata, chan, mgmt,
 					       elems.country_elem,
 					       elems.country_elem_len,
-					       elems.pwr_constr_elem);
+					       elems.pwr_constr_elem,
+					       elems.cisco_dtpc_elem);
 
 	ieee80211_bss_info_change_notify(sdata, changed);
 }
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 725af7a..e393dfa 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1014,6 +1014,28 @@ u32 ieee802_11_parse_elems_crc(const u8 *start, size_t len, bool action,
 			}
 			elems->pwr_constr_elem = pos;
 			break;
+		case WLAN_EID_CISCO_VENDOR_SPECIFIC:
+			/* Lots of different options exist, but we only care
+			 * about the Dynamic Transmit Power Control element.
+			 * First check for the Cisco OUI, then for the DTPC
+			 * tag (0x00).
+			 */
+			if (elen < 4) {
+				elem_parse_failed = true;
+				break;
+			}
+			if (pos[0] != 0x00 || pos[1] != 0x40 ||
+			    pos[2] != 0x96 || pos[3] != 0x00) {
+				break;
+			}
+			if (elen != 6) {
+				elem_parse_failed = true;
+				break;
+			}
+			if (calc_crc)
+				crc = crc32_be(crc, pos - 2, elen + 2);
+			elems->cisco_dtpc_elem = pos;
+			break;
 		case WLAN_EID_TIMEOUT_INTERVAL:
 			if (elen >= sizeof(struct ieee80211_timeout_interval_ie))
 				elems->timeout_int = (void *)pos;
-- 
1.7.10.4

Re: [PATCH v5 2/2] mac80211: support DTPC IE (from Cisco Client eXtensions)

[Johannes Berg]

On Fri, 2014-08-29 at 19:38 +0200, Steinar H. Gunderson wrote:
> Linux already supports 802.11h, where the access point can tell the
> client to reduce its transmission power. However, 802.11h is only
> defined for 5 GHz, where the need for this is much smaller than on
> 2.4 GHz.
> 
> Cisco has their own solution, called DTPC (Dynamic Transmit Power
> Control). Cisco APs on a controller sometimes but not always send
> 802.11h; they always send DTPC, even on 2.4 GHz. This patch adds support
> for parsing and honoring the DTPC IE in addition to the 802.11h
> element (they do not always contain the same limits, so both must
> be honored); the format is not documented, but very simple.

The format is documented for CCX partners, I suppose, but we don't have
access to that :)

I think for some vendors shipping our stack this might become
problematic. I think it would make sense to have a Kconfig option for
this, probably hidden away under "if EXPERT" and defaulting to yes, to
enable this code, it might be something that interferes with more CCX
implementations maybe?

> +static bool ieee80211_find_cisco_dtpc(struct ieee80211_sub_if_data
> *sdata,

The return value is useless here, so it could be void?

> +                       if (pos[0] != 0x00 || pos[1] != 0x40 ||
> +                           pos[2] != 0x96 || pos[3] != 0x00) {
> +                               break;
> +                       }

Please remove those useless braces - maybe run ./scripts/checkpatch.pl?

johannes

Re: [PATCH v5 2/2] mac80211: support DTPC IE (from Cisco Client eXtensions)

[Steinar H. Gunderson]

On Wed, Sep 03, 2014 at 02:00:36PM +0200, Johannes Berg wrote:
> I think for some vendors shipping our stack this might become
> problematic. I think it would make sense to have a Kconfig option for
> this, probably hidden away under "if EXPERT" and defaulting to yes, to
> enable this code, it might be something that interferes with more CCX
> implementations maybe?

Are there any CCX implementations for Linux at this time? Could you even do
that from just userspace? (I sort of doubt it, but it's never easy to know
what illustrious userspace programmers can do :-) )

I can make a config option, but it seems a bit odd, maybe. Are you thinking
there would be problems since this is an undocumented protocol, or because of
the possible conflict?

>> +static bool ieee80211_find_cisco_dtpc(struct ieee80211_sub_if_data
>> *sdata,
> The return value is useless here, so it could be void?

It is useless indeed; I kept it this way for symmetry with the other _find_
function and because it makes for slightly easier code around (you can set
has_cisco_pwr = directly without needing an additional statement to make it
true). But I've changed it so it's void.

I could also change it to use a return instead of an output parameter for
pwr_level_cisco if you want, but I think it might become a bit confusing to
have two so similar functions with different calling style.

>> +                       if (pos[0] != 0x00 || pos[1] != 0x40 ||
>> +                           pos[2] != 0x96 || pos[3] != 0x00) {
>> +                               break;
>> +                       }
> Please remove those useless braces - maybe run ./scripts/checkpatch.pl?

Removed. But I've already run checkpatch and it didn't complain about this
(the patch set is checkpatch-clean).

I'm in a hotel room in Seattle right now whose Wi-Fi is not Cisco-based,
so I can't test the new version as thoroughly as I'd like, but I'll at least
give it a boot test and then send an updated patch series as reply to this
message.

/* Steinar */
-- 
Homepage: http://www.sesse.net/

[PATCH v6 1/2] mac80211: split 802.11h parsing from transmit power policy

[Steinar H. Gunderson]

Decouple the logic of parsing the 802.11d and 802.11h IEs from the
part of deciding what to do about the data (messaging, clamping to
0 dBm, doing the actual setting). This paves the way for the next
patch, which introduces more data sources for transmit power limitation.

Signed-off-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
---
 net/mac80211/mlme.c | 60 ++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 19 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index b82a12a..918011a 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1157,19 +1157,21 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
 			  TU_TO_EXP_TIME(csa_ie.count * cbss->beacon_interval));
 }
 
-static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
-				       struct ieee80211_channel *channel,
-				       const u8 *country_ie, u8 country_ie_len,
-				       const u8 *pwr_constr_elem)
+static bool
+ieee80211_find_80211h_pwr_constr(struct ieee80211_sub_if_data *sdata,
+				 struct ieee80211_channel *channel,
+				 const u8 *country_ie, u8 country_ie_len,
+				 const u8 *pwr_constr_elem,
+				 int *chan_pwr, int *pwr_reduction)
 {
 	struct ieee80211_country_ie_triplet *triplet;
 	int chan = ieee80211_frequency_to_channel(channel->center_freq);
-	int i, chan_pwr, chan_increment, new_ap_level;
+	int i, chan_increment;
 	bool have_chan_pwr = false;
 
 	/* Invalid IE */
 	if (country_ie_len % 2 || country_ie_len < IEEE80211_COUNTRY_IE_MIN_LEN)
-		return 0;
+		return false;
 
 	triplet = (void *)(country_ie + 3);
 	country_ie_len -= 3;
@@ -1197,7 +1199,7 @@ static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 		for (i = 0; i < triplet->chans.num_channels; i++) {
 			if (first_channel + i * chan_increment == chan) {
 				have_chan_pwr = true;
-				chan_pwr = triplet->chans.max_power;
+				*chan_pwr = triplet->chans.max_power;
 				break;
 			}
 		}
@@ -1209,18 +1211,41 @@ static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 		country_ie_len -= 3;
 	}
 
-	if (!have_chan_pwr)
-		return 0;
+	if (have_chan_pwr)
+		*pwr_reduction = *pwr_constr_elem;
+	return have_chan_pwr;
+}
+
+static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
+				       struct ieee80211_channel *channel,
+				       struct ieee80211_mgmt *mgmt,
+				       const u8 *country_ie, u8 country_ie_len,
+				       const u8 *pwr_constr_ie)
+{
+	bool has_80211h_pwr = false;
+	int chan_pwr, pwr_reduction_80211h;
+	int new_ap_level;
 
-	new_ap_level = max_t(int, 0, chan_pwr - *pwr_constr_elem);
+	if (country_ie && pwr_constr_ie &&
+	    mgmt->u.probe_resp.capab_info &
+		cpu_to_le16(WLAN_CAPABILITY_SPECTRUM_MGMT)) {
+		has_80211h_pwr = ieee80211_find_80211h_pwr_constr(
+			sdata, channel, country_ie, country_ie_len,
+			pwr_constr_ie, &chan_pwr, &pwr_reduction_80211h);
+		new_ap_level = max_t(int, 0, chan_pwr - pwr_reduction_80211h);
+	}
 
-	if (sdata->ap_power_level == new_ap_level)
+	if (!has_80211h_pwr)
 		return 0;
 
 	sdata_info(sdata,
 		   "Limiting TX power to %d (%d - %d) dBm as advertised by %pM\n",
-		   new_ap_level, chan_pwr, *pwr_constr_elem,
+		   new_ap_level, chan_pwr, pwr_reduction_80211h,
 		   sdata->u.mgd.bssid);
+
+	if (sdata->ap_power_level == new_ap_level)
+		return 0;
+
 	sdata->ap_power_level = new_ap_level;
 	if (__ieee80211_recalc_txpower(sdata))
 		return BSS_CHANGED_TXPOWER;
@@ -3190,13 +3215,10 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 					    rx_status->band, true);
 	mutex_unlock(&local->sta_mtx);
 
-	if (elems.country_elem && elems.pwr_constr_elem &&
-	    mgmt->u.probe_resp.capab_info &
-				cpu_to_le16(WLAN_CAPABILITY_SPECTRUM_MGMT))
-		changed |= ieee80211_handle_pwr_constr(sdata, chan,
-						       elems.country_elem,
-						       elems.country_elem_len,
-						       elems.pwr_constr_elem);
+	changed |= ieee80211_handle_pwr_constr(sdata, chan, mgmt,
+					       elems.country_elem,
+					       elems.country_elem_len,
+					       elems.pwr_constr_elem);
 
 	ieee80211_bss_info_change_notify(sdata, changed);
 }
-- 
2.1.0

[PATCH v6 2/2] mac80211: support DTPC IE (from Cisco Client eXtensions)

[Steinar H. Gunderson]

Linux already supports 802.11h, where the access point can tell the
client to reduce its transmission power. However, 802.11h is only
defined for 5 GHz, where the need for this is much smaller than on
2.4 GHz.

Cisco has their own solution, called DTPC (Dynamic Transmit Power
Control). Cisco APs on a controller sometimes but not always send
802.11h; they always send DTPC, even on 2.4 GHz. This patch adds support
for parsing and honoring the DTPC IE in addition to the 802.11h
element (they do not always contain the same limits, so both must
be honored); the format is not documented, but very simple.

Tested (on top of wireless.git and on 3.16.1) against a Cisco Aironet
1142 joined to a Cisco 2504 WLC, by setting various transmit power
levels for the given access points and observing the results.
The Wireshark 802.11 dissector agrees with the interpretation of the
element, except for negative numbers, which seem to never happen
anyway.

Signed-off-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
---
 include/linux/ieee80211.h  |  3 ++-
 net/mac80211/ieee80211_i.h |  1 +
 net/mac80211/mlme.c        | 60 +++++++++++++++++++++++++++++++++++++---------
 net/mac80211/util.c        | 21 ++++++++++++++++
 4 files changed, 73 insertions(+), 12 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 63ab3873..d0ec287 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1806,7 +1806,8 @@ enum ieee80211_eid {
 	WLAN_EID_DMG_TSPEC = 146,
 	WLAN_EID_DMG_AT = 147,
 	WLAN_EID_DMG_CAP = 148,
-	/* 149-150 reserved for Cisco */
+	/* 149 reserved for Cisco */
+	WLAN_EID_CISCO_VENDOR_SPECIFIC = 150,
 	WLAN_EID_DMG_OPERATION = 151,
 	WLAN_EID_DMG_BSS_PARAM_CHANGE = 152,
 	WLAN_EID_DMG_BEAM_REFINEMENT = 153,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ef7a089..e995556 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1367,6 +1367,7 @@ struct ieee802_11_elems {
 	const struct ieee80211_wide_bw_chansw_ie *wide_bw_chansw_ie;
 	const u8 *country_elem;
 	const u8 *pwr_constr_elem;
+	const u8 *cisco_dtpc_elem;
 	const struct ieee80211_timeout_interval_ie *timeout_int;
 	const u8 *opmode_notif;
 	const struct ieee80211_sec_chan_offs_ie *sec_chan_offs;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 918011a..352f50c 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1216,14 +1216,30 @@ ieee80211_find_80211h_pwr_constr(struct ieee80211_sub_if_data *sdata,
 	return have_chan_pwr;
 }
 
+static void ieee80211_find_cisco_dtpc(struct ieee80211_sub_if_data *sdata,
+				      struct ieee80211_channel *channel,
+				      const u8 *cisco_dtpc_ie,
+				      int *pwr_level)
+{
+	/* From practical testing, the first data byte of the DTPC element
+	 * seems to contain the requested dBm level, and the CLI on Cisco
+	 * APs clearly state the range is -127 to 127 dBm, which indicates
+	 * a signed byte, although it seemingly never actually goes negative.
+	 * The other byte seems to always be zero.
+	 */
+	*pwr_level = (__s8)cisco_dtpc_ie[4];
+}
+
 static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 				       struct ieee80211_channel *channel,
 				       struct ieee80211_mgmt *mgmt,
 				       const u8 *country_ie, u8 country_ie_len,
-				       const u8 *pwr_constr_ie)
+				       const u8 *pwr_constr_ie,
+				       const u8 *cisco_dtpc_ie)
 {
-	bool has_80211h_pwr = false;
-	int chan_pwr, pwr_reduction_80211h;
+	bool has_80211h_pwr = false, has_cisco_pwr = false;
+	int chan_pwr = 0, pwr_reduction_80211h = 0;
+	int pwr_level_cisco, pwr_level_80211h;
 	int new_ap_level;
 
 	if (country_ie && pwr_constr_ie &&
@@ -1232,16 +1248,35 @@ static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 		has_80211h_pwr = ieee80211_find_80211h_pwr_constr(
 			sdata, channel, country_ie, country_ie_len,
 			pwr_constr_ie, &chan_pwr, &pwr_reduction_80211h);
-		new_ap_level = max_t(int, 0, chan_pwr - pwr_reduction_80211h);
+		pwr_level_80211h =
+			max_t(int, 0, chan_pwr - pwr_reduction_80211h);
+	}
+
+	if (cisco_dtpc_ie) {
+		ieee80211_find_cisco_dtpc(
+			sdata, channel, cisco_dtpc_ie, &pwr_level_cisco);
+		has_cisco_pwr = true;
 	}
 
-	if (!has_80211h_pwr)
+	if (!has_80211h_pwr && !has_cisco_pwr)
 		return 0;
 
-	sdata_info(sdata,
-		   "Limiting TX power to %d (%d - %d) dBm as advertised by %pM\n",
-		   new_ap_level, chan_pwr, pwr_reduction_80211h,
-		   sdata->u.mgd.bssid);
+	/* If we have both 802.11h and Cisco DTPC, apply both limits
+	 * by picking the smallest of the two power levels advertised.
+	 */
+	if (has_80211h_pwr &&
+	    (!has_cisco_pwr || pwr_level_80211h <= pwr_level_cisco)) {
+		sdata_info(sdata,
+			   "Limiting TX power to %d (%d - %d) dBm as advertised by %pM\n",
+			   pwr_level_80211h, chan_pwr, pwr_reduction_80211h,
+			   sdata->u.mgd.bssid);
+		new_ap_level = pwr_level_80211h;
+	} else {  /* has_cisco_pwr is always true here. */
+		sdata_info(sdata,
+			   "Limiting TX power to %d dBm as advertised by %pM\n",
+			   pwr_level_cisco, sdata->u.mgd.bssid);
+		new_ap_level = pwr_level_cisco;
+	}
 
 	if (sdata->ap_power_level == new_ap_level)
 		return 0;
@@ -2910,7 +2945,9 @@ static void ieee80211_rx_mgmt_probe_resp(struct ieee80211_sub_if_data *sdata,
 /*
  * This is the canonical list of information elements we care about,
  * the filter code also gives us all changes to the Microsoft OUI
- * (00:50:F2) vendor IE which is used for WMM which we need to track.
+ * (00:50:F2) vendor IE which is used for WMM which we need to track,
+ * as well as the DTPC IE (part of the Cisco OUI) used for signaling
+ * changes to requested client power.
  *
  * We implement beacon filtering in software since that means we can
  * avoid processing the frame here and in cfg80211, and userspace
@@ -3218,7 +3255,8 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 	changed |= ieee80211_handle_pwr_constr(sdata, chan, mgmt,
 					       elems.country_elem,
 					       elems.country_elem_len,
-					       elems.pwr_constr_elem);
+					       elems.pwr_constr_elem,
+					       elems.cisco_dtpc_elem);
 
 	ieee80211_bss_info_change_notify(sdata, changed);
 }
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 725af7a..95a9159 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1014,6 +1014,27 @@ u32 ieee802_11_parse_elems_crc(const u8 *start, size_t len, bool action,
 			}
 			elems->pwr_constr_elem = pos;
 			break;
+		case WLAN_EID_CISCO_VENDOR_SPECIFIC:
+			/* Lots of different options exist, but we only care
+			 * about the Dynamic Transmit Power Control element.
+			 * First check for the Cisco OUI, then for the DTPC
+			 * tag (0x00).
+			 */
+			if (elen < 4) {
+				elem_parse_failed = true;
+				break;
+			}
+			if (pos[0] != 0x00 || pos[1] != 0x40 ||
+			    pos[2] != 0x96 || pos[3] != 0x00)
+				break;
+			if (elen != 6) {
+				elem_parse_failed = true;
+				break;
+			}
+			if (calc_crc)
+				crc = crc32_be(crc, pos - 2, elen + 2);
+			elems->cisco_dtpc_elem = pos;
+			break;
 		case WLAN_EID_TIMEOUT_INTERVAL:
 			if (elen >= sizeof(struct ieee80211_timeout_interval_ie))
 				elems->timeout_int = (void *)pos;
-- 
2.1.0

Re: [PATCH v6 2/2] mac80211: support DTPC IE (from Cisco Client eXtensions)

[Johannes Berg]

On Wed, 2014-09-03 at 06:48 -0700, Steinar H. Gunderson wrote:
> Linux already supports 802.11h, where the access point can tell the
> client to reduce its transmission power. However, 802.11h is only
> defined for 5 GHz, where the need for this is much smaller than on
> 2.4 GHz.
> 
> Cisco has their own solution, called DTPC (Dynamic Transmit Power
> Control). Cisco APs on a controller sometimes but not always send
> 802.11h; they always send DTPC, even on 2.4 GHz. This patch adds support
> for parsing and honoring the DTPC IE in addition to the 802.11h
> element (they do not always contain the same limits, so both must
> be honored); the format is not documented, but very simple.
> 
> Tested (on top of wireless.git and on 3.16.1) against a Cisco Aironet
> 1142 joined to a Cisco 2504 WLC, by setting various transmit power
> levels for the given access points and observing the results.
> The Wireshark 802.11 dissector agrees with the interpretation of the
> element, except for negative numbers, which seem to never happen
> anyway.

Applied both.

johannes

Re: [PATCH v6 2/2] mac80211: support DTPC IE (from Cisco Client eXtensions)

[Steinar H. Gunderson]

On Mon, Sep 08, 2014 at 10:53:20AM +0200, Johannes Berg wrote:
>> Linux already supports 802.11h, where the access point can tell the
>> client to reduce its transmission power. However, 802.11h is only
>> defined for 5 GHz, where the need for this is much smaller than on
>> 2.4 GHz.
>> 
>> Cisco has their own solution, called DTPC (Dynamic Transmit Power
>> Control). Cisco APs on a controller sometimes but not always send
>> 802.11h; they always send DTPC, even on 2.4 GHz. This patch adds support
>> for parsing and honoring the DTPC IE in addition to the 802.11h
>> element (they do not always contain the same limits, so both must
>> be honored); the format is not documented, but very simple.
>> 
>> Tested (on top of wireless.git and on 3.16.1) against a Cisco Aironet
>> 1142 joined to a Cisco 2504 WLC, by setting various transmit power
>> levels for the given access points and observing the results.
>> The Wireshark 802.11 dissector agrees with the interpretation of the
>> element, except for negative numbers, which seem to never happen
>> anyway.
> Applied both.

I found what I believe is an edge case: What happens if the element suddenly
goes away from one beacon to another? Shouldn't there be a reset of the power
level and then a new call to __ieee80211_recalc_txpower()?

(If so, I believe this bug was already present in the code before my first
patch, as I understand the existing logic.)

/* Steinar */
-- 
Homepage: http://www.sesse.net/

Re: [PATCH v6 2/2] mac80211: support DTPC IE (from Cisco Client eXtensions)

[Johannes Berg]

On Wed, 2014-09-17 at 01:47 +0200, Steinar H. Gunderson wrote:
> On Mon, Sep 08, 2014 at 10:53:20AM +0200, Johannes Berg wrote:
> >> Linux already supports 802.11h, where the access point can tell the
> >> client to reduce its transmission power. However, 802.11h is only
> >> defined for 5 GHz, where the need for this is much smaller than on
> >> 2.4 GHz.
> >> 
> >> Cisco has their own solution, called DTPC (Dynamic Transmit Power
> >> Control). Cisco APs on a controller sometimes but not always send
> >> 802.11h; they always send DTPC, even on 2.4 GHz. This patch adds support
> >> for parsing and honoring the DTPC IE in addition to the 802.11h
> >> element (they do not always contain the same limits, so both must
> >> be honored); the format is not documented, but very simple.
> >> 
> >> Tested (on top of wireless.git and on 3.16.1) against a Cisco Aironet
> >> 1142 joined to a Cisco 2504 WLC, by setting various transmit power
> >> levels for the given access points and observing the results.
> >> The Wireshark 802.11 dissector agrees with the interpretation of the
> >> element, except for negative numbers, which seem to never happen
> >> anyway.
> > Applied both.
> 
> I found what I believe is an edge case: What happens if the element suddenly
> goes away from one beacon to another? Shouldn't there be a reset of the power
> level and then a new call to __ieee80211_recalc_txpower()?
> 
> (If so, I believe this bug was already present in the code before my first
> patch, as I understand the existing logic.)

I'm not sure it's worth worrying about, but if you want to write a patch
I'd take it :)

johannes

Re: [PATCH v6 2/2] mac80211: support DTPC IE (from Cisco Client eXtensions)

[Steinar H. Gunderson]

On Mon, Oct 06, 2014 at 04:52:14PM +0200, Johannes Berg wrote:
>> I found what I believe is an edge case: What happens if the element suddenly
>> goes away from one beacon to another? Shouldn't there be a reset of the power
>> level and then a new call to __ieee80211_recalc_txpower()?
>> 
>> (If so, I believe this bug was already present in the code before my first
>> patch, as I understand the existing logic.)
> I'm not sure it's worth worrying about, but if you want to write a patch
> I'd take it :)

It actually happened to me in a real situation, which is why I noticed.
It seems neither power element is typically sent at all if the value is 0.
(Well, Aruba seems to do. I haven't found anyone else who does.)

/* Steinar */
-- 
Homepage: http://www.sesse.net/

Re: [PATCH v5 2/2] mac80211: support DTPC IE (from Cisco Client eXtensions)

[Johannes Berg]

On Wed, 2014-09-03 at 15:33 +0200, Steinar H. Gunderson wrote:
> On Wed, Sep 03, 2014 at 02:00:36PM +0200, Johannes Berg wrote:
> > I think for some vendors shipping our stack this might become
> > problematic. I think it would make sense to have a Kconfig option for
> > this, probably hidden away under "if EXPERT" and defaulting to yes, to
> > enable this code, it might be something that interferes with more CCX
> > implementations maybe?
> 
> Are there any CCX implementations for Linux at this time? Could you even do
> that from just userspace? (I sort of doubt it, but it's never easy to know
> what illustrious userspace programmers can do :-) )

I have no idea :-)

> I can make a config option, but it seems a bit odd, maybe. Are you thinking
> there would be problems since this is an undocumented protocol, or because of
> the possible conflict?

I'm just thinking it might be a problem for a vendor to ship an
(obviously incomplete) CCX implementation, even without advertising
such. Most vendors have some relationship with Cisco at least for other
drivers, so I'd prefer to avoid any possible conflict. You could argue
that we shouldn't care upstream, and I'm not really sure where I fall on
that question - it seems that vendors might then decide to patch it out
though (which would be easy enough to do as well.)

> >> +static bool ieee80211_find_cisco_dtpc(struct ieee80211_sub_if_data
> >> *sdata,
> > The return value is useless here, so it could be void?
> 
> It is useless indeed; I kept it this way for symmetry with the other _find_
> function and because it makes for slightly easier code around (you can set
> has_cisco_pwr = directly without needing an additional statement to make it
> true). But I've changed it so it's void.
> 
> I could also change it to use a return instead of an output parameter for
> pwr_level_cisco if you want, but I think it might become a bit confusing to
> have two so similar functions with different calling style.

Right, that makes sense.

> >> +                       if (pos[0] != 0x00 || pos[1] != 0x40 ||
> >> +                           pos[2] != 0x96 || pos[3] != 0x00) {
> >> +                               break;
> >> +                       }
> > Please remove those useless braces - maybe run ./scripts/checkpatch.pl?
> 
> Removed. But I've already run checkpatch and it didn't complain about this
> (the patch set is checkpatch-clean).

Hmmm, interesting. I thought it warned about that, oh well. Thanks :)

> I'm in a hotel room in Seattle right now whose Wi-Fi is not Cisco-based,
> so I can't test the new version as thoroughly as I'd like, but I'll at least
> give it a boot test and then send an updated patch series as reply to this
> message.

Great, thanks. Safe travels then :-)

It's getting late here, but I'll take a look tomorrow. Regarding the
Kconfig question, I'll make up my mind and just do whichever option I
decide for myself, if that's OK with you?

johannes

Re: [PATCH v5 2/2] mac80211: support DTPC IE (from Cisco Client eXtensions)

[Steinar H. Gunderson]

On Wed, Sep 03, 2014 at 03:33:28PM +0200, Steinar H. Gunderson wrote:
> I'm in a hotel room in Seattle right now whose Wi-Fi is not Cisco-based,
> so I can't test the new version as thoroughly as I'd like, but I'll at least
> give it a boot test and then send an updated patch series as reply to this
> message.

It seems the airport in Seattle has Cisco-based Wi-Fi, so while waiting on
the plane to take me on, I got:

  wlan0: Limiting TX power to 23 dBm as advertised by 50:06:04:bb:d2:de

In other words, v6 still appears to work fine in practical testing. :-)

/* Steinar */
-- 
Homepage: http://www.sesse.net/

[PATCH 2/2] mac80211: support DTPC IE (from Cisco Client eXtensions)

[Steinar H. Gunderson]

Linux already supports 802.11h, where the access point can tell the
client to reduce its transmission power. However, 802.11h is only
defined for 5 GHz, where the need for this is much smaller than on
2.4 GHz.

Cisco has their own solution, called DTPC (Dynamic Transmit Power
Control). Cisco APs on a controller sometimes but not always send
802.11h; they always send DTPC, even on 2.4 GHz. This patch adds support
for parsing and honoring the DTPC IE in addition to the 802.11h
element (they do not always contain the same limits, so both must
be honored); the format is not documented, but very simple.

Tested (on top of wireless.git and on 3.16.1) against a Cisco Aironet
1142 joined to a Cisco 2504 WLC, by setting various transmit power
levels for the given access points and observing the results.
The Wireshark 802.11 dissector agrees with the interpretation of the
element, except for negative numbers, which seem to never happen
anyway.

Signed-off-by: Steinar H. Gunderson <sgunderson@bigfoot.com>
---
 include/linux/ieee80211.h  |    3 ++-
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |   55 ++++++++++++++++++++++++++++++++++++--------
 net/mac80211/util.c        |   20 ++++++++++++++++
 4 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 63ab3873..d0ec287 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1806,7 +1806,8 @@ enum ieee80211_eid {
 	WLAN_EID_DMG_TSPEC = 146,
 	WLAN_EID_DMG_AT = 147,
 	WLAN_EID_DMG_CAP = 148,
-	/* 149-150 reserved for Cisco */
+	/* 149 reserved for Cisco */
+	WLAN_EID_CISCO_VENDOR_SPECIFIC = 150,
 	WLAN_EID_DMG_OPERATION = 151,
 	WLAN_EID_DMG_BSS_PARAM_CHANGE = 152,
 	WLAN_EID_DMG_BEAM_REFINEMENT = 153,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ef7a089..e995556 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1367,6 +1367,7 @@ struct ieee802_11_elems {
 	const struct ieee80211_wide_bw_chansw_ie *wide_bw_chansw_ie;
 	const u8 *country_elem;
 	const u8 *pwr_constr_elem;
+	const u8 *cisco_dtpc_elem;
 	const struct ieee80211_timeout_interval_ie *timeout_int;
 	const u8 *opmode_notif;
 	const struct ieee80211_sec_chan_offs_ie *sec_chan_offs;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 524c7cc..a83b67e 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1217,14 +1217,31 @@ static bool ieee80211_find_80211h_pwr_constr(
 	return have_chan_pwr;
 }
 
+static bool ieee80211_find_cisco_dtpc(struct ieee80211_sub_if_data *sdata,
+				      struct ieee80211_channel *channel,
+				      const u8 *cisco_dtpc_ie,
+				      int *pwr_level)
+{
+	/* From practical testing, the first data byte of the DTPC element
+	 * seems to contain the requested dBm level, and the CLI on Cisco
+	 * APs clearly state the range is -127 to 127 dBm, which indicates
+	 * a signed byte, although it seemingly never actually goes negative.
+	 * The other byte seems to always be zero.
+	 */
+	*pwr_level = (__s8)cisco_dtpc_ie[4];
+	return true;
+}
+
 static bool ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 					struct ieee80211_channel *channel,
 					struct ieee80211_mgmt *mgmt,
 					const u8 *country_ie, u8 country_ie_len,
-					const u8 *pwr_constr_ie)
+					const u8 *pwr_constr_ie,
+					const u8 *cisco_dtpc_ie)
 {
-	bool has_80211h_pwr = false;
-	int chan_pwr, pwr_reduction_80211h;
+	bool has_80211h_pwr = false, has_cisco_pwr = false;
+	int chan_pwr = 0, pwr_reduction_80211h = 0;
+	int pwr_level_cisco, pwr_level_80211h;
 	int new_ap_level;
 
 	if (country_ie && pwr_constr_ie &&
@@ -1233,16 +1250,33 @@ static bool ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 		has_80211h_pwr = ieee80211_find_80211h_pwr_constr(
 			sdata, channel, country_ie, country_ie_len,
 			pwr_constr_ie, &chan_pwr, &pwr_reduction_80211h);
-		new_ap_level = max_t(int, 0, chan_pwr - pwr_reduction_80211h);
+		pwr_level_80211h =
+			max_t(int, 0, chan_pwr - pwr_reduction_80211h);
 	}
 
-	if (!has_80211h_pwr)
+	if (cisco_dtpc_ie)
+		has_cisco_pwr = ieee80211_find_cisco_dtpc(
+			sdata, channel, cisco_dtpc_ie, &pwr_level_cisco);
+
+	if (!has_80211h_pwr && !has_cisco_pwr)
 		return 0;
 
-	sdata_info(sdata,
-		   "Limiting TX power to %d (%d - %d) dBm as advertised by %pM\n",
-		   new_ap_level, chan_pwr, pwr_reduction_80211h,
-		   sdata->u.mgd.bssid);
+	/* If we have both 802.11h and Cisco DTPC, apply both limits
+	 * by picking the smallest of the two power levels advertised.
+	 */
+	if (has_80211h_pwr &&
+	    (!has_cisco_pwr || pwr_level_80211h <= pwr_level_cisco)) {
+		sdata_info(sdata,
+			   "Limiting TX power to %d (%d - %d) dBm as advertised by %pM\n",
+			   pwr_level_80211h, chan_pwr, pwr_reduction_80211h,
+			   sdata->u.mgd.bssid);
+		new_ap_level = pwr_level_80211h;
+	} else {  /* has_cisco_pwr is always true here. */
+		sdata_info(sdata,
+			   "Limiting TX power to %d dBm as advertised by %pM\n",
+			   pwr_level_cisco, sdata->u.mgd.bssid);
+		new_ap_level = pwr_level_cisco;
+	}
 
 	if (sdata->ap_power_level == new_ap_level)
 		return 0;
@@ -3219,7 +3253,8 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 	changed |= ieee80211_handle_pwr_constr(sdata, chan, mgmt,
 					       elems.country_elem,
 					       elems.country_elem_len,
-					       elems.pwr_constr_elem);
+					       elems.pwr_constr_elem,
+					       elems.cisco_dtpc_elem);
 
 	ieee80211_bss_info_change_notify(sdata, changed);
 }
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 725af7a..56105f4 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1014,6 +1014,26 @@ u32 ieee802_11_parse_elems_crc(const u8 *start, size_t len, bool action,
 			}
 			elems->pwr_constr_elem = pos;
 			break;
+		case WLAN_EID_CISCO_VENDOR_SPECIFIC:
+			/* Lots of different options exist, but we only care
+			 * about the Dynamic Transmit Power Control element.
+			 * First check for the Cisco OUI, then for the DTPC
+			 * tag (0x00).
+			 */
+			if (elen < 4) {
+				elem_parse_failed = true;
+				break;
+			}
+			if (pos[0] != 0x00 || pos[1] != 0x40 ||
+			    pos[2] != 0x96 || pos[3] != 0x00) {
+				break;
+			}
+			if (elen != 6) {
+				elem_parse_failed = true;
+				break;
+			}
+			elems->cisco_dtpc_elem = pos;
+			break;
 		case WLAN_EID_TIMEOUT_INTERVAL:
 			if (elen >= sizeof(struct ieee80211_timeout_interval_ie))
 				elems->timeout_int = (void *)pos;
-- 
1.7.10.4