[PATCH 0/5] Setting coverage class (and ACK timeout and slot time), take two

[PATCH 0/5] Setting coverage class (and ACK timeout and slot time), take two

[Lukáš Turek]

These patches implement a feature essential for long distance wireless links:
setting ACK timeout and slot time. The API is based on a parameter called
Coverage Class specified by the 802.11 standard.

The first two patches add support to the kernel 802.11 stack via a new nl80211
attribute and a mac80211 callback.

Following three patches implement the callback in ath5k driver, after some
requisite fixes. Implementation in ath9k driver is planned.

Please note the last patch outside the series is not a kernel patch, it adds
the necessary support to userspace program iw.

This is a second version of these patches, with these changes:

* The coverage class is calculated from roundtrip time instead of one-way
  delay asi specified by the standard

* Calculation of slot time and ACK timeouts is based on 802.11 standard and
  Madwifi 0.10.5.6, instead of athctrl program from Madwifi 0.9.4

* Full name "coverage_class" is used everywhere for consistency

Lukas Turek

[PATCH 1/5] nl80211: Add new WIPHY attribute COVERAGE_CLASS

[Lukáš Turek]

The new attribute NL80211_ATTR_WIPHY_COVERAGE_CLASS sets IEEE 802.11
Coverage Class, which depends on maximum distance of nodes in a
wireless network. It's required for long distance links (more than a few
hundred meters).

The attribute is now ignored by two non-mac80211 drivers, rndis and
iwmc3200wifi, together with WIPHY_PARAM_RETRY_SHORT and
WIPHY_PARAM_RETRY_LONG. If it turns out to be a problem, we could split
set_wiphy_params callback or add new capability bits.

Signed-off-by: Lukas Turek <8an@praha12.net>
---
 include/linux/nl80211.h |    4 ++++
 include/net/cfg80211.h  |    2 ++
 net/wireless/core.c     |    1 +
 net/wireless/nl80211.c  |   15 +++++++++++++++
 4 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index da8ea2e..e241ed1 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -402,6 +402,8 @@ enum nl80211_commands {
  * @NL80211_ATTR_WIPHY_RTS_THRESHOLD: RTS threshold (TX frames with length
  *	larger than or equal to this use RTS/CTS handshake); allowed range:
  *	0..65536, disable with (u32)-1; dot11RTSThreshold; u32
+ * @NL80211_ATTR_WIPHY_COVERAGE_CLASS: Coverage Class as defined by IEEE 802.11
+ *	section 7.3.2.9; dot11CoverageClass; u8
  *
  * @NL80211_ATTR_IFINDEX: network interface index of the device to operate on
  * @NL80211_ATTR_IFNAME: network interface name
@@ -743,6 +745,8 @@ enum nl80211_attrs {
 	NL80211_ATTR_PMKID,
 	NL80211_ATTR_MAX_NUM_PMKIDS,
 
+	NL80211_ATTR_WIPHY_COVERAGE_CLASS,
+
 	/* 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 0884b9a..fe50c71 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -837,6 +837,7 @@ enum wiphy_params_flags {
 	WIPHY_PARAM_RETRY_LONG		= 1 << 1,
 	WIPHY_PARAM_FRAG_THRESHOLD	= 1 << 2,
 	WIPHY_PARAM_RTS_THRESHOLD	= 1 << 3,
+	WIPHY_PARAM_COVERAGE_CLASS	= 1 << 4,
 };
 
 /**
@@ -1217,6 +1218,7 @@ struct wiphy {
 	u8 retry_long;
 	u32 frag_threshold;
 	u32 rts_threshold;
+	u8 coverage_class;
 
 	char fw_version[ETHTOOL_BUSINFO_LEN];
 	u32 hw_version;
diff --git a/net/wireless/core.c b/net/wireless/core.c
index c674567..14e14ce 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -402,6 +402,7 @@ struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
 	rdev->wiphy.retry_long = 4;
 	rdev->wiphy.frag_threshold = (u32) -1;
 	rdev->wiphy.rts_threshold = (u32) -1;
+	rdev->wiphy.coverage_class = 0;
 
 	return &rdev->wiphy;
 }
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index a602843..7835846 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -69,6 +69,7 @@ static struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] __read_mostly = {
 	[NL80211_ATTR_WIPHY_RETRY_LONG] = { .type = NLA_U8 },
 	[NL80211_ATTR_WIPHY_FRAG_THRESHOLD] = { .type = NLA_U32 },
 	[NL80211_ATTR_WIPHY_RTS_THRESHOLD] = { .type = NLA_U32 },
+	[NL80211_ATTR_WIPHY_COVERAGE_CLASS] = { .type = NLA_U8 },
 
 	[NL80211_ATTR_IFTYPE] = { .type = NLA_U32 },
 	[NL80211_ATTR_IFINDEX] = { .type = NLA_U32 },
@@ -442,6 +443,8 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
 		    dev->wiphy.frag_threshold);
 	NLA_PUT_U32(msg, NL80211_ATTR_WIPHY_RTS_THRESHOLD,
 		    dev->wiphy.rts_threshold);
+	NLA_PUT_U8(msg, NL80211_ATTR_WIPHY_COVERAGE_CLASS,
+		    dev->wiphy.coverage_class);
 
 	NLA_PUT_U8(msg, NL80211_ATTR_MAX_NUM_SCAN_SSIDS,
 		   dev->wiphy.max_scan_ssids);
@@ -681,6 +684,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 	u32 changed;
 	u8 retry_short = 0, retry_long = 0;
 	u32 frag_threshold = 0, rts_threshold = 0;
+	u8 coverage_class = 0;
 
 	rtnl_lock();
 
@@ -803,9 +807,16 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 		changed |= WIPHY_PARAM_RTS_THRESHOLD;
 	}
 
+	if (info->attrs[NL80211_ATTR_WIPHY_COVERAGE_CLASS]) {
+		coverage_class = nla_get_u8(
+			info->attrs[NL80211_ATTR_WIPHY_COVERAGE_CLASS]);
+		changed |= WIPHY_PARAM_COVERAGE_CLASS;
+	}
+
 	if (changed) {
 		u8 old_retry_short, old_retry_long;
 		u32 old_frag_threshold, old_rts_threshold;
+		u8 old_coverage_class;
 
 		if (!rdev->ops->set_wiphy_params) {
 			result = -EOPNOTSUPP;
@@ -816,6 +827,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 		old_retry_long = rdev->wiphy.retry_long;
 		old_frag_threshold = rdev->wiphy.frag_threshold;
 		old_rts_threshold = rdev->wiphy.rts_threshold;
+		old_coverage_class = rdev->wiphy.coverage_class;
 
 		if (changed & WIPHY_PARAM_RETRY_SHORT)
 			rdev->wiphy.retry_short = retry_short;
@@ -825,6 +837,8 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 			rdev->wiphy.frag_threshold = frag_threshold;
 		if (changed & WIPHY_PARAM_RTS_THRESHOLD)
 			rdev->wiphy.rts_threshold = rts_threshold;
+		if (changed & WIPHY_PARAM_COVERAGE_CLASS)
+			rdev->wiphy.coverage_class = coverage_class;
 
 		result = rdev->ops->set_wiphy_params(&rdev->wiphy, changed);
 		if (result) {
@@ -832,6 +846,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 			rdev->wiphy.retry_long = old_retry_long;
 			rdev->wiphy.frag_threshold = old_frag_threshold;
 			rdev->wiphy.rts_threshold = old_rts_threshold;
+			rdev->wiphy.coverage_class = old_coverage_class;
 		}
 	}
 
-- 
1.6.4.4

[PATCH 2/5] mac80211: Add new callback set_coverage_class

[Lukáš Turek]

Mac80211 callback to driver set_coverage_class() sets slot time and ACK
timeout for given IEEE 802.11 coverage class. The callback is optional,
but it's essential for long distance links.

Signed-off-by: Lukas Turek <8an@praha12.net>
---
 include/net/mac80211.h |    5 +++++
 net/mac80211/cfg.c     |    7 +++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 2aff490..786e1d6 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1483,6 +1483,10 @@ enum ieee80211_ampdu_mlme_action {
  *	need to set wiphy->rfkill_poll to %true before registration,
  *	and need to call wiphy_rfkill_set_hw_state() in the callback.
  *
+ * @set_coverage_class: Set slot time for given coverage class as specified
+ *	in IEEE 802.11-2007 section 17.3.8.6 and modify ACK timeout
+ *	accordingly. This callback is not required and may sleep.
+ *
  * @testmode_cmd: Implement a cfg80211 test mode command.
  */
 struct ieee80211_ops {
@@ -1537,6 +1541,7 @@ struct ieee80211_ops {
 			    struct ieee80211_sta *sta, u16 tid, u16 *ssn);
 
 	void (*rfkill_poll)(struct ieee80211_hw *hw);
+	void (*set_coverage_class)(struct ieee80211_hw *hw, u8 coverage_class);
 #ifdef CONFIG_NL80211_TESTMODE
 	int (*testmode_cmd)(struct ieee80211_hw *hw, void *data, int len);
 #endif
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 6dc3579..c5537ae 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1237,6 +1237,13 @@ static int ieee80211_set_wiphy_params(struct wiphy *wiphy, u32 changed)
 	struct ieee80211_local *local = wiphy_priv(wiphy);
 	int err;
 
+	if (changed & WIPHY_PARAM_COVERAGE_CLASS) {
+		if (!local->ops->set_coverage_class)
+			return -EOPNOTSUPP;
+		local->ops->set_coverage_class(&local->hw,
+					       wiphy->coverage_class);
+	}
+
 	if (changed & WIPHY_PARAM_RTS_THRESHOLD) {
 		err = drv_set_rts_threshold(local, wiphy->rts_threshold);
 
-- 
1.6.4.4

[PATCH 3/5] ath5k: Fix functions for getting/setting slot time

[Lukáš Turek]

Functions ath5k_hw_get_slot_time and ath5k_hw_set_slot_time were
converting microseconds to clocks only for AR5210, although it's needed
for all supported devices. The conversion was moved outside the
hardware-specific branches.

The original code also limited minimum slot time to 9, while turbo modes
use 6, this was fixed too.

Signed-off-by: Lukas Turek <8an@praha12.net>
---
 drivers/net/wireless/ath/ath5k/qcu.c |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/qcu.c b/drivers/net/wireless/ath/ath5k/qcu.c
index eeebb9a..ed9021a 100644
--- a/drivers/net/wireless/ath/ath5k/qcu.c
+++ b/drivers/net/wireless/ath/ath5k/qcu.c
@@ -520,12 +520,16 @@ int ath5k_hw_reset_tx_queue(struct ath5k_hw *ah, unsigned int queue)
  */
 unsigned int ath5k_hw_get_slot_time(struct ath5k_hw *ah)
 {
+	unsigned int slot_time_clock;
+
 	ATH5K_TRACE(ah->ah_sc);
+
 	if (ah->ah_version == AR5K_AR5210)
-		return ath5k_hw_clocktoh(ath5k_hw_reg_read(ah,
-				AR5K_SLOT_TIME) & 0xffff, ah->ah_turbo);
+		slot_time_clock = ath5k_hw_reg_read(ah, AR5K_SLOT_TIME);
 	else
-		return ath5k_hw_reg_read(ah, AR5K_DCU_GBL_IFS_SLOT) & 0xffff;
+		slot_time_clock = ath5k_hw_reg_read(ah, AR5K_DCU_GBL_IFS_SLOT);
+
+	return ath5k_hw_clocktoh(slot_time_clock & 0xffff, ah->ah_turbo);
 }
 
 /*
@@ -533,15 +537,17 @@ unsigned int ath5k_hw_get_slot_time(struct ath5k_hw *ah)
  */
 int ath5k_hw_set_slot_time(struct ath5k_hw *ah, unsigned int slot_time)
 {
+	u32 slot_time_clock = ath5k_hw_htoclock(slot_time, ah->ah_turbo);
+
 	ATH5K_TRACE(ah->ah_sc);
-	if (slot_time < AR5K_SLOT_TIME_9 || slot_time > AR5K_SLOT_TIME_MAX)
+
+	if (slot_time < 6 || slot_time_clock > AR5K_SLOT_TIME_MAX)
 		return -EINVAL;
 
 	if (ah->ah_version == AR5K_AR5210)
-		ath5k_hw_reg_write(ah, ath5k_hw_htoclock(slot_time,
-				ah->ah_turbo), AR5K_SLOT_TIME);
+		ath5k_hw_reg_write(ah, slot_time_clock, AR5K_SLOT_TIME);
 	else
-		ath5k_hw_reg_write(ah, slot_time, AR5K_DCU_GBL_IFS_SLOT);
+		ath5k_hw_reg_write(ah, slot_time_clock, AR5K_DCU_GBL_IFS_SLOT);
 
 	return 0;
 }
-- 
1.6.4.4

[PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Lukáš Turek]

The original code was correct in 802.11a mode only, 802.11b/g uses
different clock rates. The new code uses values taken from FreeBSD HAL
and should be correct for all modes including turbo modes.

The former rate calculation was used by slope coefficient calculation
function ath5k_hw_write_ofdm_timings. However, this function requires
the 802.11a values even in 802.11g mode. Thus the use of
ath5k_hw_htoclock was replaced by hardcoded values. Possibly the slope
coefficient calculation is not related to clock rate at all.

Signed-off-by: Lukas Turek <8an@praha12.net>
---
 drivers/net/wireless/ath/ath5k/ath5k.h |   22 ++---------
 drivers/net/wireless/ath/ath5k/pcu.c   |   64 +++++++++++++++++++++++++++-----
 drivers/net/wireless/ath/ath5k/qcu.c   |    4 +-
 drivers/net/wireless/ath/ath5k/reset.c |    3 +-
 4 files changed, 61 insertions(+), 32 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
index 6a2a967..ae311d2 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -1231,6 +1231,10 @@ extern int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout);
 extern unsigned int ath5k_hw_get_ack_timeout(struct ath5k_hw *ah);
 extern int ath5k_hw_set_cts_timeout(struct ath5k_hw *ah, unsigned int timeout);
 extern unsigned int ath5k_hw_get_cts_timeout(struct ath5k_hw *ah);
+/* Clock rate related functions */
+unsigned int ath5k_hw_htoclock(struct ath5k_hw *ah, unsigned int usec);
+unsigned int ath5k_hw_clocktoh(struct ath5k_hw *ah, unsigned int clock);
+unsigned int ath5k_hw_get_clockrate(struct ath5k_hw *ah);
 /* Key table (WEP) functions */
 extern int ath5k_hw_reset_key(struct ath5k_hw *ah, u16 entry);
 extern int ath5k_hw_is_key_valid(struct ath5k_hw *ah, u16 entry);
@@ -1310,24 +1314,6 @@ extern int ath5k_hw_set_txpower_limit(struct ath5k_hw *ah, u8 txpower);
  * Functions used internaly
  */
 
-/*
- * Translate usec to hw clock units
- * TODO: Half/quarter rate
- */
-static inline unsigned int ath5k_hw_htoclock(unsigned int usec, bool turbo)
-{
-	return turbo ? (usec * 80) : (usec * 40);
-}
-
-/*
- * Translate hw clock units to usec
- * TODO: Half/quarter rate
- */
-static inline unsigned int ath5k_hw_clocktoh(unsigned int clock, bool turbo)
-{
-	return turbo ? (clock / 80) : (clock / 40);
-}
-
 static inline struct ath_common *ath5k_hw_common(struct ath5k_hw *ah)
 {
         return &ah->common;
diff --git a/drivers/net/wireless/ath/ath5k/pcu.c b/drivers/net/wireless/ath/ath5k/pcu.c
index 64fc1eb..8c72845 100644
--- a/drivers/net/wireless/ath/ath5k/pcu.c
+++ b/drivers/net/wireless/ath/ath5k/pcu.c
@@ -187,8 +187,8 @@ unsigned int ath5k_hw_get_ack_timeout(struct ath5k_hw *ah)
 {
 	ATH5K_TRACE(ah->ah_sc);
 
-	return ath5k_hw_clocktoh(AR5K_REG_MS(ath5k_hw_reg_read(ah,
-			AR5K_TIME_OUT), AR5K_TIME_OUT_ACK), ah->ah_turbo);
+	return ath5k_hw_clocktoh(ah, AR5K_REG_MS(ath5k_hw_reg_read(ah,
+			AR5K_TIME_OUT), AR5K_TIME_OUT_ACK));
 }
 
 /**
@@ -200,12 +200,12 @@ unsigned int ath5k_hw_get_ack_timeout(struct ath5k_hw *ah)
 int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout)
 {
 	ATH5K_TRACE(ah->ah_sc);
-	if (ath5k_hw_clocktoh(AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_ACK),
-			ah->ah_turbo) <= timeout)
+	if (ath5k_hw_clocktoh(ah, AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_ACK))
+			<= timeout)
 		return -EINVAL;
 
 	AR5K_REG_WRITE_BITS(ah, AR5K_TIME_OUT, AR5K_TIME_OUT_ACK,
-		ath5k_hw_htoclock(timeout, ah->ah_turbo));
+		ath5k_hw_htoclock(ah, timeout));
 
 	return 0;
 }
@@ -218,8 +218,8 @@ int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout)
 unsigned int ath5k_hw_get_cts_timeout(struct ath5k_hw *ah)
 {
 	ATH5K_TRACE(ah->ah_sc);
-	return ath5k_hw_clocktoh(AR5K_REG_MS(ath5k_hw_reg_read(ah,
-			AR5K_TIME_OUT), AR5K_TIME_OUT_CTS), ah->ah_turbo);
+	return ath5k_hw_clocktoh(ah, AR5K_REG_MS(ath5k_hw_reg_read(ah,
+			AR5K_TIME_OUT), AR5K_TIME_OUT_CTS));
 }
 
 /**
@@ -231,17 +231,61 @@ unsigned int ath5k_hw_get_cts_timeout(struct ath5k_hw *ah)
 int ath5k_hw_set_cts_timeout(struct ath5k_hw *ah, unsigned int timeout)
 {
 	ATH5K_TRACE(ah->ah_sc);
-	if (ath5k_hw_clocktoh(AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_CTS),
-			ah->ah_turbo) <= timeout)
+	if (ath5k_hw_clocktoh(ah, AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_CTS))
+			<= timeout)
 		return -EINVAL;
 
 	AR5K_REG_WRITE_BITS(ah, AR5K_TIME_OUT, AR5K_TIME_OUT_CTS,
-			ath5k_hw_htoclock(timeout, ah->ah_turbo));
+			ath5k_hw_htoclock(ah, timeout));
 
 	return 0;
 }
 
 /**
+ * ath5k_hw_htoclock - Translate usec to hw clock units
+ *
+ * @ah: The &struct ath5k_hw
+ * @usec: value in microseconds
+ */
+unsigned int ath5k_hw_htoclock(struct ath5k_hw *ah, unsigned int usec)
+{
+	return usec * ath5k_hw_get_clockrate(ah);
+}
+
+/**
+ * ath5k_hw_clocktoh - Translate hw clock units to usec
+ * @clock: value in hw clock units
+ */
+unsigned int ath5k_hw_clocktoh(struct ath5k_hw *ah, unsigned int clock)
+{
+	return clock / ath5k_hw_get_clockrate(ah);
+}
+
+/**
+ * ath5k_hw_get_clockrate - Get the clock rate for current mode
+ *
+ * @ah: The &struct ath5k_hw
+ */
+unsigned int ath5k_hw_get_clockrate(struct ath5k_hw *ah)
+{
+	struct ieee80211_channel *channel = ah->ah_current_channel;
+	int clock;
+
+	if (channel->hw_value & CHANNEL_5GHZ)
+		clock = 40; /* 802.11a */
+	else if (channel->hw_value & CHANNEL_CCK)
+		clock = 22; /* 802.11b */
+	else
+		clock = 44; /* 802.11g */
+
+	/* Clock rate in turbo modes is twice the normal rate */
+	if (channel->hw_value & CHANNEL_TURBO)
+		clock *= 2;
+
+	return clock;
+}
+
+/**
  * ath5k_hw_set_lladdr - Set station id
  *
  * @ah: The &struct ath5k_hw
diff --git a/drivers/net/wireless/ath/ath5k/qcu.c b/drivers/net/wireless/ath/ath5k/qcu.c
index ed9021a..6af0ac8 100644
--- a/drivers/net/wireless/ath/ath5k/qcu.c
+++ b/drivers/net/wireless/ath/ath5k/qcu.c
@@ -529,7 +529,7 @@ unsigned int ath5k_hw_get_slot_time(struct ath5k_hw *ah)
 	else
 		slot_time_clock = ath5k_hw_reg_read(ah, AR5K_DCU_GBL_IFS_SLOT);
 	
-	return ath5k_hw_clocktoh(slot_time_clock & 0xffff, ah->ah_turbo);
+	return ath5k_hw_clocktoh(ah, slot_time_clock & 0xffff);
 }
 
 /*
@@ -537,7 +537,7 @@ unsigned int ath5k_hw_get_slot_time(struct ath5k_hw *ah)
  */
 int ath5k_hw_set_slot_time(struct ath5k_hw *ah, unsigned int slot_time)
 {
-	u32 slot_time_clock = ath5k_hw_htoclock(slot_time, ah->ah_turbo);
+	u32 slot_time_clock = ath5k_hw_htoclock(ah, slot_time);
 
 	ATH5K_TRACE(ah->ah_sc);
 
diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
index 62954fc..f1dc4c8 100644
--- a/drivers/net/wireless/ath/ath5k/reset.c
+++ b/drivers/net/wireless/ath/ath5k/reset.c
@@ -64,8 +64,7 @@ static inline int ath5k_hw_write_ofdm_timings(struct ath5k_hw *ah,
 	 * we scale coef by shifting clock value by 24 for
 	 * better precision since we use integers */
 	/* TODO: Half/quarter rate */
-	clock =  ath5k_hw_htoclock(1, channel->hw_value & CHANNEL_TURBO);
-
+	clock =  (channel->hw_value & CHANNEL_TURBO) ? 80 : 40;
 	coef_scaled = ((5 * (clock << 24)) / 2) / channel->center_freq;
 
 	/* Get exponent
-- 
1.6.4.4

[PATCH 5/5] ath5k: Implement mac80211 callback set_coverage_class

[Lukáš Turek]

The callback sets slot time as specified in IEEE 802.11-2007 section
17.3.8.6 (for 20MHz channels only for now) and raises ACK and CTS
timeouts accordingly. The values are persistent, they are restored after
device reset.

Signed-off-by: Lukas Turek <8an@praha12.net>
---
 drivers/net/wireless/ath/ath5k/ath5k.h |    2 +
 drivers/net/wireless/ath/ath5k/base.c  |   23 +++++++++++++
 drivers/net/wireless/ath/ath5k/pcu.c   |   55 ++++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath5k/reset.c |    4 ++
 4 files changed, 84 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
index ae311d2..66bcb50 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -1063,6 +1063,7 @@ struct ath5k_hw {
 	u32			ah_cw_min;
 	u32			ah_cw_max;
 	u32			ah_limit_tx_retries;
+	u8			ah_coverage_class;
 
 	/* Antenna Control */
 	u32			ah_ant_ctl[AR5K_EEPROM_N_MODES][AR5K_ANT_MAX];
@@ -1200,6 +1201,7 @@ extern bool ath5k_eeprom_is_hb63(struct ath5k_hw *ah);
 
 /* Protocol Control Unit Functions */
 extern int ath5k_hw_set_opmode(struct ath5k_hw *ah);
+extern void ath5k_hw_set_coverage_class(struct ath5k_hw *ah, u8 coverage_class);
 /* BSSID Functions */
 extern int ath5k_hw_set_lladdr(struct ath5k_hw *ah, const u8 *mac);
 extern void ath5k_hw_set_associd(struct ath5k_hw *ah);
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index a4c086f..203622e 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -254,6 +254,8 @@ static void ath5k_bss_info_changed(struct ieee80211_hw *hw,
 		u32 changes);
 static void ath5k_sw_scan_start(struct ieee80211_hw *hw);
 static void ath5k_sw_scan_complete(struct ieee80211_hw *hw);
+static void ath5k_set_coverage_class(struct ieee80211_hw *hw,
+		u8 coverage_class);
 
 static const struct ieee80211_ops ath5k_hw_ops = {
 	.tx 		= ath5k_tx,
@@ -274,6 +276,7 @@ static const struct ieee80211_ops ath5k_hw_ops = {
 	.bss_info_changed = ath5k_bss_info_changed,
 	.sw_scan_start	= ath5k_sw_scan_start,
 	.sw_scan_complete = ath5k_sw_scan_complete,
+	.set_coverage_class = ath5k_set_coverage_class,
 };
 
 /*
@@ -3274,3 +3277,23 @@ static void ath5k_sw_scan_complete(struct ieee80211_hw *hw)
 	ath5k_hw_set_ledstate(sc->ah, sc->assoc ?
 		AR5K_LED_ASSOC : AR5K_LED_INIT);
 }
+
+/**
+ * ath5k_set_coverage_class - Set IEEE 802.11 coverage class
+ *
+ * @hw: struct ieee80211_hw pointer
+ * @coverage_class: IEEE 802.11 coverage class number
+ *
+ * Mac80211 callback. Sets slot time, ACK timeout and CTS timeout for given
+ * coverage class. The values are persistent, they are restored after device
+ * reset.
+ */
+static void ath5k_set_coverage_class(struct ieee80211_hw *hw, u8 coverage_class)
+{
+	struct ath5k_softc *sc = hw->priv;
+
+	mutex_lock(&sc->lock);
+	ath5k_hw_set_coverage_class(sc->ah, coverage_class);
+	sc->ah->ah_coverage_class = coverage_class;
+	mutex_unlock(&sc->lock);
+}
diff --git a/drivers/net/wireless/ath/ath5k/pcu.c b/drivers/net/wireless/ath/ath5k/pcu.c
index 8c72845..88ad112 100644
--- a/drivers/net/wireless/ath/ath5k/pcu.c
+++ b/drivers/net/wireless/ath/ath5k/pcu.c
@@ -286,6 +286,42 @@ unsigned int ath5k_hw_get_clockrate(struct ath5k_hw *ah)
 }
 
 /**
+ * ath5k_hw_get_default_slottime - Get the default slot time for current mode
+ *
+ * @ah: The &struct ath5k_hw
+ */
+unsigned int ath5k_hw_get_default_slottime(struct ath5k_hw *ah)
+{
+	struct ieee80211_channel *channel = ah->ah_current_channel;
+
+	if (channel->hw_value & CHANNEL_TURBO)
+		return 6; /* both turbo modes */
+
+	if (channel->hw_value & CHANNEL_CCK)
+		return 20; /* 802.11b */
+
+	return 9; /* 802.11 a/g */
+}
+
+/**
+ * ath5k_hw_get_default_sifs - Get the default SIFS for current mode
+ *
+ * @ah: The &struct ath5k_hw
+ */
+unsigned int ath5k_hw_get_default_sifs(struct ath5k_hw *ah)
+{
+	struct ieee80211_channel *channel = ah->ah_current_channel;
+
+	if (channel->hw_value & CHANNEL_TURBO)
+		return 8; /* both turbo modes */
+
+	if (channel->hw_value & CHANNEL_5GHZ)
+		return 16; /* 802.11a */
+
+	return 10; /* 802.11 b/g */
+}
+
+/**
  * ath5k_hw_set_lladdr - Set station id
  *
  * @ah: The &struct ath5k_hw
@@ -1094,3 +1130,22 @@ int ath5k_hw_set_key_lladdr(struct ath5k_hw *ah, u16 entry, const u8 *mac)
 	return 0;
 }
 
+/**
+ * ath5k_hw_set_coverage_class - Set IEEE 802.11 coverage class
+ *
+ * @ah: The &struct ath5k_hw
+ * @coverage_class: IEEE 802.11 coverage class number
+ *
+ * Sets slot time, ACK timeout and CTS timeout for given coverage class.
+ */
+void ath5k_hw_set_coverage_class(struct ath5k_hw *ah, u8 coverage_class)
+{
+	/* As defined by IEEE 802.11-2007 17.3.8.6 */
+	int slot_time = ath5k_hw_get_default_slottime(ah) + 3 * coverage_class;
+	int ack_timeout = ath5k_hw_get_default_sifs(ah) + slot_time;
+	int cts_timeout = ack_timeout;
+
+	ath5k_hw_set_slot_time(ah, slot_time);
+	ath5k_hw_set_ack_timeout(ah, ack_timeout);
+	ath5k_hw_set_cts_timeout(ah, cts_timeout);
+}
diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
index f1dc4c8..ec3b357 100644
--- a/drivers/net/wireless/ath/ath5k/reset.c
+++ b/drivers/net/wireless/ath/ath5k/reset.c
@@ -1316,6 +1316,10 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
 	/* Restore antenna mode */
 	ath5k_hw_set_antenna_mode(ah, ah->ah_ant_mode);
 
+	/* Restore slot time and ACK timeouts */
+	if (ah->ah_coverage_class > 0)
+		ath5k_hw_set_coverage_class(ah, ah->ah_coverage_class);
+
 	/*
 	 * Configure QCUs/DCUs
 	 */
-- 
1.6.4.4

[PATCH] iw: Add support for NL80211_ATTR_WIPHY_COVERAGE_CLASS

[Lukáš Turek]

New nl80211 attribute NL80211_ATTR_WIPHY_COVERAGE_CLASS allows setting
IEEE 802.11 coverage class, which is then used by drivers to calculate
slot time and ACK timeout for long distance links.

Two iw parameters are added: 'coverage' sets the coverage class
directly, while 'distance' is more user-friendly, as it allows to set
just the link distance and let iw do the necessary calculation itself.

Signed-off-by: Lukas Turek <8an@praha12.net>
---
 info.c    |    8 ++++++++
 nl80211.h |   15 +++++++++++++++
 phy.c     |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 0 deletions(-)

diff --git a/info.c b/info.c
index 69af216..1a1b588 100644
--- a/info.c
+++ b/info.c
@@ -156,6 +156,14 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
 			printf("\tRTS threshold: %d\n", rts);
 	}
 
+	if (tb_msg[NL80211_ATTR_WIPHY_COVERAGE_CLASS]) {
+		unsigned char coverage;
+
+		coverage = nla_get_u8(tb_msg[NL80211_ATTR_WIPHY_COVERAGE_CLASS]);
+		/* See handle_distance() for an explanation where the '450' comes from */
+		printf("\tCoverage class: %d (up to %dm)\n", coverage, 450 * coverage);
+	}
+
 	if (!tb_msg[NL80211_ATTR_SUPPORTED_IFTYPES])
 		goto commands;
 
diff --git a/nl80211.h b/nl80211.h
index 45db17f..e241ed1 100644
--- a/nl80211.h
+++ b/nl80211.h
@@ -349,6 +349,10 @@ enum nl80211_commands {
 	NL80211_CMD_GET_SURVEY,
 	NL80211_CMD_NEW_SURVEY_RESULTS,
 
+	NL80211_CMD_SET_PMKSA,
+	NL80211_CMD_DEL_PMKSA,
+	NL80211_CMD_FLUSH_PMKSA,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
@@ -398,6 +402,8 @@ enum nl80211_commands {
  * @NL80211_ATTR_WIPHY_RTS_THRESHOLD: RTS threshold (TX frames with length
  *	larger than or equal to this use RTS/CTS handshake); allowed range:
  *	0..65536, disable with (u32)-1; dot11RTSThreshold; u32
+ * @NL80211_ATTR_WIPHY_COVERAGE_CLASS: Coverage Class as defined by IEEE 802.11
+ *	section 7.3.2.9; dot11CoverageClass; u8
  *
  * @NL80211_ATTR_IFINDEX: network interface index of the device to operate on
  * @NL80211_ATTR_IFNAME: network interface name
@@ -598,6 +604,10 @@ enum nl80211_commands {
  *      the survey response for %NL80211_CMD_GET_SURVEY, nested attribute
  *      containing info as possible, see &enum survey_info.
  *
+ * @NL80211_ATTR_PMKID: PMK material for PMKSA caching.
+ * @NL80211_ATTR_MAX_NUM_PMKIDS: maximum number of PMKIDs a firmware can
+ *	cache, a wiphy attribute.
+ *
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
  */
@@ -732,6 +742,11 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_SURVEY_INFO,
 
+	NL80211_ATTR_PMKID,
+	NL80211_ATTR_MAX_NUM_PMKIDS,
+
+	NL80211_ATTR_WIPHY_COVERAGE_CLASS,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/phy.c b/phy.c
index 8dd01aa..1de58a9 100644
--- a/phy.c
+++ b/phy.c
@@ -164,3 +164,61 @@ static int handle_netns(struct nl80211_state *state,
 COMMAND(set, netns, "<pid>",
 	NL80211_CMD_SET_WIPHY_NETNS, 0, CIB_PHY, handle_netns,
 	"Put this wireless device into a different network namespace");
+
+static int handle_coverage(struct nl80211_state *state,
+			struct nl_cb *cb,
+			struct nl_msg *msg,
+			int argc, char **argv)
+{
+	unsigned int coverage;
+
+	if (argc != 1)
+		return 1;
+
+	coverage = strtoul(argv[0], NULL, 10);
+	if (coverage > 255)
+		return 1;
+	
+	NLA_PUT_U8(msg, NL80211_ATTR_WIPHY_COVERAGE_CLASS, coverage);
+
+	return 0;
+ nla_put_failure:
+	return -ENOBUFS;
+}
+COMMAND(set, coverage, "<coverage class>",
+	NL80211_CMD_SET_WIPHY, 0, CIB_PHY, handle_coverage,
+	"Set coverage class (1 for every 3 usec of air propagation time).\n"
+	"Valid values: 0 - 255.");
+
+static int handle_distance(struct nl80211_state *state,
+			struct nl_cb *cb,
+			struct nl_msg *msg,
+			int argc, char **argv)
+{
+	unsigned int distance, coverage;
+
+	if (argc != 1)
+		return 1;
+
+	distance = strtoul(argv[0], NULL, 10);
+	
+	/*
+	 * Divide double the distance by the speed of light in m/usec (300) to
+	 * get round-trip time in microseconds and then divide the result by
+	 * three to get coverage class as specified in IEEE 802.11-2007 table
+	 * 7-27. Values are rounded upwards.
+	 */
+	coverage = (distance + 449) / 450;
+	if (coverage > 255)
+		return 1;
+	
+	NLA_PUT_U8(msg, NL80211_ATTR_WIPHY_COVERAGE_CLASS, coverage);
+
+	return 0;
+ nla_put_failure:
+	return -ENOBUFS;
+}
+COMMAND(set, distance, "<distance>",
+	NL80211_CMD_SET_WIPHY, 0, CIB_PHY, handle_distance,
+	"Set appropriate coverage class for given link distance in meters.\n"
+	"Valid values: 0 - 114750");
-- 
1.6.4.4

Re: [PATCH 2/5] mac80211: Add new callback set_coverage_class

[Johannes Berg]

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


> +	if (changed & WIPHY_PARAM_COVERAGE_CLASS) {
> +		if (!local->ops->set_coverage_class)
> +			return -EOPNOTSUPP;
> +		local->ops->set_coverage_class(&local->hw,
> +					       wiphy->coverage_class);
> +	}

You should have a drv_ inline wrapper, tracer and a might_sleep() in
that.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [ath5k-devel] [PATCH 2/5] mac80211: Add new callback set_coverage_class

[Luis R. Rodriguez]

On Tue, Dec 15, 2009 at 09:56:49AM -0800, Lukáš Turek wrote:
> Mac80211 callback to driver set_coverage_class() sets slot time and ACK
> timeout for given IEEE 802.11 coverage class. The callback is optional,
> but it's essential for long distance links.
> 
> Signed-off-by: Lukas Turek <8an@praha12.net>
> ---
>  include/net/mac80211.h |    5 +++++
>  net/mac80211/cfg.c     |    7 +++++++
>  2 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 2aff490..786e1d6 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1483,6 +1483,10 @@ enum ieee80211_ampdu_mlme_action {
>   *     need to set wiphy->rfkill_poll to %true before registration,
>   *     and need to call wiphy_rfkill_set_hw_state() in the callback.
>   *
> + * @set_coverage_class: Set slot time for given coverage class as specified
> + *     in IEEE 802.11-2007 section 17.3.8.6 and modify ACK timeout
> + *     accordingly. This callback is not required and may sleep.
> + *
>   * @testmode_cmd: Implement a cfg80211 test mode command.
>   */
>  struct ieee80211_ops {
> @@ -1537,6 +1541,7 @@ struct ieee80211_ops {
>                             struct ieee80211_sta *sta, u16 tid, u16 *ssn);
> 
>         void (*rfkill_poll)(struct ieee80211_hw *hw);
> +       void (*set_coverage_class)(struct ieee80211_hw *hw, u8 coverage_class);
>  #ifdef CONFIG_NL80211_TESTMODE
>         int (*testmode_cmd)(struct ieee80211_hw *hw, void *data, int len);
>  #endif
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 6dc3579..c5537ae 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -1237,6 +1237,13 @@ static int ieee80211_set_wiphy_params(struct wiphy *wiphy, u32 changed)
>         struct ieee80211_local *local = wiphy_priv(wiphy);
>         int err;
> 
> +       if (changed & WIPHY_PARAM_COVERAGE_CLASS) {
> +               if (!local->ops->set_coverage_class)
> +                       return -EOPNOTSUPP;

Hm, it seems best to just add the capability bit that way userspace
can stuff what it wishes and the kernel will only set what is supported.
As is now this would lead to -EOPNOTSUPP but we'd have no way of knowing
from userspace what failed.

This comment is not specific to just this parameter but if you see the
current implementation of ieee80211_set_wiphy_params() doesn't fail
with -EOPNOTSUPP because I suppose WIPHY_PARAM_RTS_THRESHOLD is
required.

> +               local->ops->set_coverage_class(&local->hw,
> +                                              wiphy->coverage_class);
> +       }
> +
>         if (changed & WIPHY_PARAM_RTS_THRESHOLD) {
>                 err = drv_set_rts_threshold(local, wiphy->rts_threshold);

  Luis

Re: [ath5k-devel] [PATCH 5/5] ath5k: Implement mac80211 callback set_coverage_class

[Luis R. Rodriguez]

On Tue, Dec 15, 2009 at 09:56:52AM -0800, Lukáš Turek wrote:
> The callback sets slot time as specified in IEEE 802.11-2007 section
> 17.3.8.6 (for 20MHz channels only for now) and raises ACK and CTS
> timeouts accordingly. The values are persistent, they are restored after
> device reset.
> 
> Signed-off-by: Lukas Turek <8an@praha12.net>
> ---
>  drivers/net/wireless/ath/ath5k/ath5k.h |    2 +
>  drivers/net/wireless/ath/ath5k/base.c  |   23 +++++++++++++
>  drivers/net/wireless/ath/ath5k/pcu.c   |   55 ++++++++++++++++++++++++++++++++
>  drivers/net/wireless/ath/ath5k/reset.c |    4 ++
>  4 files changed, 84 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
> index ae311d2..66bcb50 100644
> --- a/drivers/net/wireless/ath/ath5k/ath5k.h
> +++ b/drivers/net/wireless/ath/ath5k/ath5k.h
> @@ -1063,6 +1063,7 @@ struct ath5k_hw {
>         u32                     ah_cw_min;
>         u32                     ah_cw_max;
>         u32                     ah_limit_tx_retries;
> +       u8                      ah_coverage_class;
> 
>         /* Antenna Control */
>         u32                     ah_ant_ctl[AR5K_EEPROM_N_MODES][AR5K_ANT_MAX];
> @@ -1200,6 +1201,7 @@ extern bool ath5k_eeprom_is_hb63(struct ath5k_hw *ah);
> 
>  /* Protocol Control Unit Functions */
>  extern int ath5k_hw_set_opmode(struct ath5k_hw *ah);
> +extern void ath5k_hw_set_coverage_class(struct ath5k_hw *ah, u8 coverage_class);
>  /* BSSID Functions */
>  extern int ath5k_hw_set_lladdr(struct ath5k_hw *ah, const u8 *mac);
>  extern void ath5k_hw_set_associd(struct ath5k_hw *ah);
> diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
> index a4c086f..203622e 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -254,6 +254,8 @@ static void ath5k_bss_info_changed(struct ieee80211_hw *hw,
>                 u32 changes);
>  static void ath5k_sw_scan_start(struct ieee80211_hw *hw);
>  static void ath5k_sw_scan_complete(struct ieee80211_hw *hw);
> +static void ath5k_set_coverage_class(struct ieee80211_hw *hw,
> +               u8 coverage_class);
> 
>  static const struct ieee80211_ops ath5k_hw_ops = {
>         .tx             = ath5k_tx,
> @@ -274,6 +276,7 @@ static const struct ieee80211_ops ath5k_hw_ops = {
>         .bss_info_changed = ath5k_bss_info_changed,
>         .sw_scan_start  = ath5k_sw_scan_start,
>         .sw_scan_complete = ath5k_sw_scan_complete,
> +       .set_coverage_class = ath5k_set_coverage_class,
>  };
> 
>  /*
> @@ -3274,3 +3277,23 @@ static void ath5k_sw_scan_complete(struct ieee80211_hw *hw)
>         ath5k_hw_set_ledstate(sc->ah, sc->assoc ?
>                 AR5K_LED_ASSOC : AR5K_LED_INIT);
>  }
> +
> +/**
> + * ath5k_set_coverage_class - Set IEEE 802.11 coverage class
> + *
> + * @hw: struct ieee80211_hw pointer
> + * @coverage_class: IEEE 802.11 coverage class number
> + *
> + * Mac80211 callback. Sets slot time, ACK timeout and CTS timeout for given
> + * coverage class. The values are persistent, they are restored after device
> + * reset.
> + */
> +static void ath5k_set_coverage_class(struct ieee80211_hw *hw, u8 coverage_class)
> +{
> +       struct ath5k_softc *sc = hw->priv;
> +
> +       mutex_lock(&sc->lock);
> +       ath5k_hw_set_coverage_class(sc->ah, coverage_class);
> +       sc->ah->ah_coverage_class = coverage_class;

Can you move this last line setting the sc->ah->ah_coverage_class
to ath5k_hw_set_coverage_class() instead? Although ath5k will likely
require a real "hw module" split as we have in ath9k now ath9k_hw
it would still be good to see this sort of stuff being done
consistantly.

Also -- if an ath_hw_set_coverage_class(common, coverage_class) can be
defined on drivers/net/wireless/ath/hw.c along with:

ath_hw_set_slot_time(common, slot_time),
ath_hw_set_ack_timeout(common, ack_timeout);
ath_hw_set_cts_timeout(common, cts_timeout);

Whether or not this makes sense to merge eventually is saparate from
my main point though but just wanted to illustrate that if we do intend
on sharing more hw code leaving ah struct settings to hw code is best.

Wonder if ar9170 can support setting these too.

  Luis

Re: [ath5k-devel] [PATCH 1/5] nl80211: Add new WIPHY attribute COVERAGE_CLASS

[Luis R. Rodriguez]

On Tue, Dec 15, 2009 at 09:56:48AM -0800, Lukáš Turek wrote:
> The new attribute NL80211_ATTR_WIPHY_COVERAGE_CLASS sets IEEE 802.11
> Coverage Class, which depends on maximum distance of nodes in a
> wireless network. It's required for long distance links (more than a few
> hundred meters).
> 
> The attribute is now ignored by two non-mac80211 drivers, rndis and
> iwmc3200wifi, together with WIPHY_PARAM_RETRY_SHORT and
> WIPHY_PARAM_RETRY_LONG. If it turns out to be a problem, we could split
> set_wiphy_params callback or add new capability bits.
> 
> Signed-off-by: Lukas Turek <8an@praha12.net>
> @@ -803,9 +807,16 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
>                 changed |= WIPHY_PARAM_RTS_THRESHOLD;
>         }
> 
> +       if (info->attrs[NL80211_ATTR_WIPHY_COVERAGE_CLASS]) {
> +               coverage_class = nla_get_u8(
> +                       info->attrs[NL80211_ATTR_WIPHY_COVERAGE_CLASS]);
> +               changed |= WIPHY_PARAM_COVERAGE_CLASS;
> +       }
> +

Does setting the coverage class make sense for all modes of operation?

If not it'd be good to catch those early and avoid setting them and also
properly document them.

The AP seems to pass the coverage class on country IE, so I guess
this means we can support this for AP mode and IBSS and only through the
country IE for STA. Mind you that would mean hostapd would need to kick
the coverage class as well and some new code on cfg80211 reg.c
country_ie_2_rd() to parse it.

Doesn't seem to make sense to set this for monitor interfaces.

  Luis

Re: [ath5k-devel] [PATCH 5/5] ath5k: Implement mac80211 callback set_coverage_class

[Luis R. Rodriguez]

On Tue, Dec 15, 2009 at 10:50:16AM -0800, Luis Rodriguez wrote:
> On Tue, Dec 15, 2009 at 09:56:52AM -0800, Lukáš Turek wrote:
> > The callback sets slot time as specified in IEEE 802.11-2007 section
> > 17.3.8.6 (for 20MHz channels only for now) and raises ACK and CTS
> > timeouts accordingly. The values are persistent, they are restored after
> > device reset.
> >
> > Signed-off-by: Lukas Turek <8an@praha12.net>
> > ---
> >  drivers/net/wireless/ath/ath5k/ath5k.h |    2 +
> >  drivers/net/wireless/ath/ath5k/base.c  |   23 +++++++++++++
> >  drivers/net/wireless/ath/ath5k/pcu.c   |   55 ++++++++++++++++++++++++++++++++
> >  drivers/net/wireless/ath/ath5k/reset.c |    4 ++
> >  4 files changed, 84 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
> > index ae311d2..66bcb50 100644
> > --- a/drivers/net/wireless/ath/ath5k/ath5k.h
> > +++ b/drivers/net/wireless/ath/ath5k/ath5k.h
> > @@ -1063,6 +1063,7 @@ struct ath5k_hw {
> >         u32                     ah_cw_min;
> >         u32                     ah_cw_max;
> >         u32                     ah_limit_tx_retries;
> > +       u8                      ah_coverage_class;
> >
> >         /* Antenna Control */
> >         u32                     ah_ant_ctl[AR5K_EEPROM_N_MODES][AR5K_ANT_MAX];
> > @@ -1200,6 +1201,7 @@ extern bool ath5k_eeprom_is_hb63(struct ath5k_hw *ah);
> >
> >  /* Protocol Control Unit Functions */
> >  extern int ath5k_hw_set_opmode(struct ath5k_hw *ah);
> > +extern void ath5k_hw_set_coverage_class(struct ath5k_hw *ah, u8 coverage_class);
> >  /* BSSID Functions */
> >  extern int ath5k_hw_set_lladdr(struct ath5k_hw *ah, const u8 *mac);
> >  extern void ath5k_hw_set_associd(struct ath5k_hw *ah);
> > diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
> > index a4c086f..203622e 100644
> > --- a/drivers/net/wireless/ath/ath5k/base.c
> > +++ b/drivers/net/wireless/ath/ath5k/base.c
> > @@ -254,6 +254,8 @@ static void ath5k_bss_info_changed(struct ieee80211_hw *hw,
> >                 u32 changes);
> >  static void ath5k_sw_scan_start(struct ieee80211_hw *hw);
> >  static void ath5k_sw_scan_complete(struct ieee80211_hw *hw);
> > +static void ath5k_set_coverage_class(struct ieee80211_hw *hw,
> > +               u8 coverage_class);
> >
> >  static const struct ieee80211_ops ath5k_hw_ops = {
> >         .tx             = ath5k_tx,
> > @@ -274,6 +276,7 @@ static const struct ieee80211_ops ath5k_hw_ops = {
> >         .bss_info_changed = ath5k_bss_info_changed,
> >         .sw_scan_start  = ath5k_sw_scan_start,
> >         .sw_scan_complete = ath5k_sw_scan_complete,
> > +       .set_coverage_class = ath5k_set_coverage_class,
> >  };
> >
> >  /*
> > @@ -3274,3 +3277,23 @@ static void ath5k_sw_scan_complete(struct ieee80211_hw *hw)
> >         ath5k_hw_set_ledstate(sc->ah, sc->assoc ?
> >                 AR5K_LED_ASSOC : AR5K_LED_INIT);
> >  }
> > +
> > +/**
> > + * ath5k_set_coverage_class - Set IEEE 802.11 coverage class
> > + *
> > + * @hw: struct ieee80211_hw pointer
> > + * @coverage_class: IEEE 802.11 coverage class number
> > + *
> > + * Mac80211 callback. Sets slot time, ACK timeout and CTS timeout for given
> > + * coverage class. The values are persistent, they are restored after device
> > + * reset.
> > + */
> > +static void ath5k_set_coverage_class(struct ieee80211_hw *hw, u8 coverage_class)
> > +{
> > +       struct ath5k_softc *sc = hw->priv;
> > +
> > +       mutex_lock(&sc->lock);
> > +       ath5k_hw_set_coverage_class(sc->ah, coverage_class);
> > +       sc->ah->ah_coverage_class = coverage_class;
> 
> Can you move this last line setting the sc->ah->ah_coverage_class
> to ath5k_hw_set_coverage_class() instead? Although ath5k will likely

I meant will likely *not*

> require a real "hw module" split as we have in ath9k now ath9k_hw
> it would still be good to see this sort of stuff being done
> consistantly.

  Luis

Re: [ath5k-devel] [PATCH 1/5] nl80211: Add new WIPHY attribute COVERAGE_CLASS

[Luis R. Rodriguez]

On Tue, Dec 15, 2009 at 11:00:51AM -0800, Luis Rodriguez wrote:
> On Tue, Dec 15, 2009 at 09:56:48AM -0800, Lukáš Turek wrote:
> > The new attribute NL80211_ATTR_WIPHY_COVERAGE_CLASS sets IEEE 802.11
> > Coverage Class, which depends on maximum distance of nodes in a
> > wireless network. It's required for long distance links (more than a few
> > hundred meters).
> >
> > The attribute is now ignored by two non-mac80211 drivers, rndis and
> > iwmc3200wifi, together with WIPHY_PARAM_RETRY_SHORT and
> > WIPHY_PARAM_RETRY_LONG. If it turns out to be a problem, we could split
> > set_wiphy_params callback or add new capability bits.
> >
> > Signed-off-by: Lukas Turek <8an@praha12.net>
> > @@ -803,9 +807,16 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
> >                 changed |= WIPHY_PARAM_RTS_THRESHOLD;
> >         }
> >
> > +       if (info->attrs[NL80211_ATTR_WIPHY_COVERAGE_CLASS]) {
> > +               coverage_class = nla_get_u8(
> > +                       info->attrs[NL80211_ATTR_WIPHY_COVERAGE_CLASS]);
> > +               changed |= WIPHY_PARAM_COVERAGE_CLASS;
> > +       }
> > +
> 
> Does setting the coverage class make sense for all modes of operation?
> 
> If not it'd be good to catch those early and avoid setting them and also
> properly document them.
> 
> The AP seems to pass the coverage class on country IE, so I guess
> this means we can support this for AP mode and IBSS and only through the
> country IE for STA.

And if your IBSS and the AP already sent the coverage class through the
country IE I am not sure if we should allow overriding it.

> Mind you that would mean hostapd would need to kick
> the coverage class as well and some new code on cfg80211 reg.c
> country_ie_2_rd() to parse it.
> 
> Doesn't seem to make sense to set this for monitor interfaces.

  Luis

Re: [ath5k-devel] [PATCH 1/5] nl80211: Add new WIPHY attribute COVERAGE_CLASS

[Lukáš Turek]

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

On 15.12.2009 20:00 you wrote:
> Does setting the coverage class make sense for all modes of operation?
> If not it'd be good to catch those early and avoid setting them and also
> properly document them.
The coverage class is a physical device parameter, not interface parameter, so 
it's independent on whether the interface is AP or STA, there could even be 
multiple interfaces on one physical device. It's the same as RTS threshold, 
for example. And it has to be a physical device parameter, because in the end 
it just sets some hardware registers.

> The AP seems to pass the coverage class on country IE, so I guess
> this means we can support this for AP mode and IBSS and only through the
> country IE for STA. Mind you that would mean hostapd would need to kick
> the coverage class as well and some new code on cfg80211 reg.c
> country_ie_2_rd() to parse it.
This part is not implemented yet, I wanted to do the low level setting first, 
as the regulatory stuff is quite complex. It's still usable for long distance 
point-to-point links (that are now impossible with in-kernel drivers). 
Hostapd would then use the same API.

Lukas Turek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [ath5k-devel] [PATCH 1/5] nl80211: Add new WIPHY attribute COVERAGE_CLASS

[Lukáš Turek]

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

On 15.12.2009 20:02 you wrote:
> And if your IBSS and the AP already sent the coverage class through the
> country IE I am not sure if we should allow overriding it.
Overriding the coverage class is harmless, it won't allow you to break any 
laws like overriding regulatory domain does.

Lukas Turek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [ath5k-devel] [PATCH 2/5] mac80211: Add new callback set_coverage_class

[Lukáš Turek]

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

On 15.12.2009 19:11 Luis R. Rodriguez wrote:
> Hm, it seems best to just add the capability bit that way userspace
> can stuff what it wishes and the kernel will only set what is supported.
> As is now this would lead to -EOPNOTSUPP but we'd have no way of knowing
> from userspace what failed.
I could add the capability bit, but I don't understand you completely - do you 
mean the kernel should just ignore unsupported attributes and leave the 
checks to userspace?

Lukas Turek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [ath5k-devel] [PATCH 2/5] mac80211: Add new callback set_coverage_class

[Johannes Berg]

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

On Tue, 2009-12-15 at 10:11 -0800, Luis R. Rodriguez wrote:

> > +       if (changed & WIPHY_PARAM_COVERAGE_CLASS) {
> > +               if (!local->ops->set_coverage_class)
> > +                       return -EOPNOTSUPP;
> 
> Hm, it seems best to just add the capability bit that way userspace
> can stuff what it wishes and the kernel will only set what is supported.
> As is now this would lead to -EOPNOTSUPP but we'd have no way of knowing
> from userspace what failed.

I think if it cares, userspace should just be careful to only change one
thing at a time.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [ath5k-devel] [PATCH 5/5] ath5k: Implement mac80211 callback set_coverage_class

[Lukáš Turek]

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

On 15.12.2009 19:50 you wrote:
> Can you move this last line setting the sc->ah->ah_coverage_class
> to ath5k_hw_set_coverage_class() instead?
ath5k_hw_set_coverage_class() is also called after interface reset:
ath5k_hw_set_coverage_class(ah, ah->ah_coverage_class);

So it would be quite strange if it set the value again...
But I see your point, and don't see a better solution.

> Also -- if an ath_hw_set_coverage_class(common, coverage_class) can be
> defined on drivers/net/wireless/ath/hw.c along with:
>
> ath_hw_set_slot_time(common, slot_time),
> ath_hw_set_ack_timeout(common, ack_timeout);
> ath_hw_set_cts_timeout(common, cts_timeout);
Unfortunately the functions are really hardware specific, they write a 
register that even differs between chip versions, and also depend on chip 
clock rate.

It would be possible to move ath_hw_set_coverage_class() to ath to share the 
calculation, but that would require exporting six functions from the modules.

> Wonder if ar9170 can support setting these too.
It seems it supports setting slot time, but I don't see a register with ACK 
timeout, which is required too.

Lukas Turek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [ath5k-devel] [PATCH 1/5] nl80211: Add new WIPHY attribute COVERAGE_CLASS

[Luis R. Rodriguez]

On Tue, Dec 15, 2009 at 01:07:32PM -0800, Lukáš Turek wrote:
> On 15.12.2009 20:02 you wrote:
> > And if your IBSS and the AP already sent the coverage class through the
> > country IE I am not sure if we should allow overriding it.
> Overriding the coverage class is harmless, it won't allow you to break any 
> laws like overriding regulatory domain does.

I'm not concerned about how you override it, I am concerned about
creating consistancy though and avoiding tweaks through userspace
which can screw things. I'll address this in more details in the other
threads.

  Luis

Re: [ath5k-devel] [PATCH 1/5] nl80211: Add new WIPHY attribute COVERAGE_CLASS

[Luis R. Rodriguez]

On Tue, Dec 15, 2009 at 12:56:42PM -0800, Lukáš Turek wrote:
> On 15.12.2009 20:00 you wrote:
> > Does setting the coverage class make sense for all modes of operation?
> > If not it'd be good to catch those early and avoid setting them and also
> > properly document them.
>
> The coverage class is a physical device parameter, not interface parameter, so 
> it's independent on whether the interface is AP or STA, there could even be 
> multiple interfaces on one physical device.

I don't see how this makes sense. Are you saying if we have multiple interfaces
you must restrict the slot_time, ack_timeout and CTS timeout all to the same
values for all of them?

A hardware configuration can be just that -- hardware configuration but if
if such hardware knob can change depending on the BSS I don't see why this
wouldn't be a BSS parameter, specially if the AP/IBSS can send this out through
an information element.

> It's the same as RTS threshold, 
> for example.

I'd argue the same for it. We haven't yet really supported connecting to
multiple APs but this may change in the future. And say you wanted to support
an AP with one RTS threshold and connect to an AP at the same time with another
RTS threshold I think this should be possible.

> And it has to be a physical device parameter, because in the end 
> it just sets some hardware registers.

It can certainly be pegged onto the ath_hw or ath5k_hw but it doesn't
mean it can't be a per BSS setting.

> > The AP seems to pass the coverage class on country IE, so I guess
> > this means we can support this for AP mode and IBSS and only through the
> > country IE for STA. Mind you that would mean hostapd would need to kick
> > the coverage class as well and some new code on cfg80211 reg.c
> > country_ie_2_rd() to parse it.
>
> This part is not implemented yet, I wanted to do the low level setting first, 

I'd say expose it through debugfs first then instead of adding proper APIs
for userspace. If the country IE is the way to pass this information alog to
STAs there would be no need to tweak this on the user end. If you're an AP
though you are likely going to want to change this though so I see, for example
hostapd wanting to set this through nl80211. It also seems reasonable for IBSS
but IBSS won't send country IEs unless I guess we use wpa_supplicant and somehow
leverage the IE generation from hostapd.

> as the regulatory stuff is quite complex.

Hehe yeah, I skipped it :)

I haven't even read this part of the IEEE docs but I did read enough to know the
coverage_class is defined and set ont he country IE. This part at least is defined
on include/linux/ieee80211.h:

struct ieee80211_country_ie_triplet {
        union {
                struct {
                        u8 first_channel;
                        u8 num_channels;
                        s8 max_power;
                } __attribute__ ((packed)) chans;
                struct {
                        u8 reg_extension_id;
                        u8 reg_class;
                        u8 coverage_class;
                } __attribute__ ((packed)) ext;
        };
} __attribute__ ((packed));

Parsing the Country IE is also already done for you, all that would need to
be done is to allow parsing the triplets when class first_channel >
IEEE80211_COUNTRY_EXTENSION_ID, and then we can get the coverage class.

But -- like I said, I really didn't read this section, not sure how the reg_class
fits into this. Not sure if this helps:

http://wireless.kernel.org/en/developers/Documentation/ChannelList

> It's still usable for long distance 
> point-to-point links (that are now impossible with in-kernel drivers). 

Sure, I'm not saying I don't see value into adding this upstream, what
seems pointless is to start allowing mucking with settings when we haven't
been doing the same for other hw configurable parameters through a well
defined userspace API.

If we want to expose this through debugfs though that's different, go for it.

> Hostapd would then use the same API.

Well hostapd would use it for AP mode, sure and that makes sense. Can't say
it makes sense to let users configure through a well defined userspace API
if the AP instead can juse use the country IE and we can enable debugfs for
experimenting.

  Luis

Re: [ath5k-devel] [PATCH 5/5] ath5k: Implement mac80211 callback set_coverage_class

[Luis R. Rodriguez]

On Tue, Dec 15, 2009 at 01:35:01PM -0800, Lukáš Turek wrote:
> On 15.12.2009 19:50 you wrote:
> > Can you move this last line setting the sc->ah->ah_coverage_class
> > to ath5k_hw_set_coverage_class() instead?
> ath5k_hw_set_coverage_class() is also called after interface reset:
> ath5k_hw_set_coverage_class(ah, ah->ah_coverage_class);
> 
> So it would be quite strange if it set the value again...
> But I see your point, and don't see a better solution.

What I meant was not for you to pass the ah->ah_coverage_class to
ath5k_hw_set_coverage_class() but instead to pass the value obtained
from mac80211 and let ath5k_hw_set_coverage_class() itself set it on
ah.

> > Also -- if an ath_hw_set_coverage_class(common, coverage_class) can be
> > defined on drivers/net/wireless/ath/hw.c along with:
> >
> > ath_hw_set_slot_time(common, slot_time),
> > ath_hw_set_ack_timeout(common, ack_timeout);
> > ath_hw_set_cts_timeout(common, cts_timeout);
>
> Unfortunately the functions are really hardware specific, they write a 
> register that even differs between chip versions, and also depend on chip 
> clock rate.

Thanks for checking, I just wanted to elaborate as to how we can share code
and to because of that it seems like a good idea to remain consistant and
treat settings ah-> variables in hw code alone.

The less we muck with ah-> settings on base.c for ath5k the more well kept
and separated the actual hw code is. This really will only serve purpose
to later merge mow hw code between ath9k/ath5k. How much we end up sharing
remains to be seen, I don't mind the current split we have but if we do
see something obviously common it makes sense to just stuff it in when we
get a chance to ath.

> It would be possible to move ath_hw_set_coverage_class() to ath to share the 
> calculation, but that would require exporting six functions from the modules.
> 
> > Wonder if ar9170 can support setting these too.
>
> It seems it supports setting slot time, but I don't see a register with ACK 
> timeout, which is required too.

Thanks for checking, I was lazy, and just curious. Reason for asking was if
it could use at least caching the coverage class a hardware parameter we
can stuff it into ath_common just as we do with the bssid_mask and stuff.
Even if its just shared between ath5k and ath9k its still worth stashing it
there if both will use it.

Since tuning cts timeout, ack timouet and slot time may not be an operation
which we do that often it would still not be so bad to just stuff a generic
helper for ath5k/ath9k into ath which will handle the different hw revisions
itself.

  Luis

Re: [ath5k-devel] [PATCH 1/5] nl80211: Add new WIPHY attribute COVERAGE_CLASS

[Felix Fietkau]

On 2009-12-15 10:58 PM, Luis R. Rodriguez wrote:
> On Tue, Dec 15, 2009 at 12:56:42PM -0800, Lukáš Turek wrote:
>> On 15.12.2009 20:00 you wrote:
>> > Does setting the coverage class make sense for all modes of operation?
>> > If not it'd be good to catch those early and avoid setting them and also
>> > properly document them.
>>
>> The coverage class is a physical device parameter, not interface parameter, so 
>> it's independent on whether the interface is AP or STA, there could even be 
>> multiple interfaces on one physical device.
> 
> I don't see how this makes sense. Are you saying if we have multiple interfaces
> you must restrict the slot_time, ack_timeout and CTS timeout all to the same
> values for all of them?
We could advertise this as a per-BSS parameter and let mac80211 send the
maximum of the coverage class values of all BSS interfaces to the
driver. That would allow stations connected to AP interfaces with
(bss_coverage_class < driver_coverage_class) to use more aggressive
timings than the AP is using. I'm not sure there's much to be gained
from this, though - especially with 11n.

>> It's the same as RTS threshold, 
>> for example.
> I'd argue the same for it. We haven't yet really supported connecting to
> multiple APs but this may change in the future. And say you wanted to support
> an AP with one RTS threshold and connect to an AP at the same time with another
> RTS threshold I think this should be possible.
It's not the same as the RTS threshold, as the RTS threshold is
evaluated per-packet.

>> This part is not implemented yet, I wanted to do the low level setting first, 
> I'd say expose it through debugfs first then instead of adding proper APIs
> for userspace. If the country IE is the way to pass this information alog to
> STAs there would be no need to tweak this on the user end. If you're an AP
> though you are likely going to want to change this though so I see, for example
> hostapd wanting to set this through nl80211. It also seems reasonable for IBSS
> but IBSS won't send country IEs unless I guess we use wpa_supplicant and somehow
> leverage the IE generation from hostapd.
I'm not sure merging the debugfs variant first is particularly useful.
Let's just define the API for setting the coverage class values and
merge an implementation that allows the user to override the effective
coverage class values for every interface mode. We can then sort out the
remaining implementation issues later.
I don't think merging a debugfs based setting would be a good idea. I'd
prefer starting to implement this setting for more drivers including
non-ath ones sooner rather than later.

- Felix

Re: [ath5k-devel] [PATCH 1/5] nl80211: Add new WIPHY attribute COVERAGE_CLASS

[Lukáš Turek]

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

On 15.12.2009 22:58 you wrote:
> I don't see how this makes sense. Are you saying if we have multiple
> interfaces you must restrict the slot_time, ack_timeout and CTS timeout all
> to the same values for all of them?
If you have multiple interfaces, they must be all on the same channel and you 
can't do anything with it. It's the with slot time: it's a parameter of the 
CSMA/CA method for medium sharing. When stations use a different value of 
slot time, collisions will happen more likely. It doesn't matter they are 
connected to different BSS, there's just one shared medium.

There is no such problem with ACK timeout, but there is still another one: you 
can have packets coming from multiple interfaces in hardware queue. And you 
can't change the register value before sending each one, because the hardware 
sends them on their own terms and just reports it's done via an interrupt. So 
effectively the ACK timeout has to be the same as well.

> I'd say expose it through debugfs first then instead of adding proper APIs
> for userspace. If the country IE is the way to pass this information alog
> to STAs there would be no need to tweak this on the user end. If you're an
> AP though you are likely going to want to change this though so I see, for
> example hostapd wanting to set this through nl80211. It also seems
> reasonable for IBSS but IBSS won't send country IEs unless I guess we use
> wpa_supplicant and somehow leverage the IE generation from hostapd.
The problem is, I don't know any hardware that really sends the coverage 
class. There's a lot of hardware that allows setting only the ACK timeout, 
like all the Ubiquity devices, Ovislink 5000 and so on. So the client 
connecting to these devices will need to specify the coverage class manually.

When the coverage class is sent in beacons, it should take precedence over the 
user-specified value, I'm not against that. It's just not relevant now, until 
there's a hardware that sends it or until the hostap part is done. And I said 
I want to do that, just not in this series, it's already problematic 
enough...

And about the debugfs: originaly I thought i would just implement a simple 
debugfs API for setting ACK/CTS timeout and slot time. However, Johannes Berg 
recommended me to do this coverage class thing and I didn't envision how much 
trouble it's gonna be...

Lukas Turek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [ath5k-devel] [PATCH 1/5] nl80211: Add new WIPHY attribute COVERAGE_CLASS

[Holger Schurig]

> And if your IBSS and the AP already sent the coverage class
> through the country IE I am not sure if we should allow
> overriding it. 

Hmm, I think a local setting should override some broadcasted 
setting *IF* this cannot be used for some kind of 
denial-of-service-attack on links farther away.

But if the AP says "I'm 20 km away from you", but you *KNOW* that 
you're only 100 m fromt he AP, you should be able to override 
this --- again, if this wouldn't harm the operation of another 
device which is really 20 km away.

-- 
http://www.holgerschurig.de

Re: [ath5k-devel] [PATCH 1/5] nl80211: Add new WIPHY attribute COVERAGE_CLASS

[Luis R. Rodriguez]

On Tue, Dec 15, 2009 at 2:52 PM, Lukáš Turek <8an@praha12.net> wrote:

> The problem is, I don't know any hardware that really sends the coverage
> class. There's a lot of hardware that allows setting only the ACK timeout,
> like all the Ubiquity devices, Ovislink 5000 and so on. So the client
> connecting to these devices will need to specify the coverage class manually.

You have a good point here.

> When the coverage class is sent in beacons, it should take precedence over the
> user-specified value, I'm not against that.

OK.

> It's just not relevant now, until
> there's a hardware that sends it or until the hostap part is done. And I said
> I want to do that, just not in this series, it's already problematic
> enough...

OK, yeah you have a good point.

> And about the debugfs: originaly I thought i would just implement a simple
> debugfs API for setting ACK/CTS timeout and slot time. However, Johannes Berg
> recommended me to do this coverage class thing and I didn't envision how much
> trouble it's gonna be...

Thanks a lot for your work on this!

  Luis

Re: [ath5k-devel] [PATCH 1/5] nl80211: Add new WIPHY attribute COVERAGE_CLASS

[Lukáš Turek]

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

On 16.12.2009 09:30 you wrote:
> Thanks a lot for your work on this!
I thank you too for the comments, but, what do I have to do to get an ACK, 
besides the trivial fixes like the drv_ wrapper and moving the ah_coverage 
setting?

Lukas Turek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [ath5k-devel] [PATCH 1/5] nl80211: Add new WIPHY attribute COVERAGE_CLASS

[Luis R. Rodriguez]

On Fri, Dec 18, 2009 at 08:33:52AM -0800, Lukáš Turek wrote:
> On 16.12.2009 09:30 you wrote:
> > Thanks a lot for your work on this!
> I thank you too for the comments, but, what do I have to do to get an ACK, 
> besides the trivial fixes like the drv_ wrapper and moving the ah_coverage 
> setting?

Resend and add v2 to the patch prefix "--subject-prefix="PATCH v2" IIRC
on git format-patch.

  Luis

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[海藻敬之]

Hi Lukas

 Didn't we have to handle CHANNEL_2GHZ case in 
ath5k_hw_write_ofdm_timings() shown below ?
 I think we should do. then I made my own patch to hadle it and it 
seemed  to improve the throughput of 2.4GHz. (even still not as good as 
5Ghz case )

> diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
> index 62954fc..f1dc4c8 100644
> --- a/drivers/net/wireless/ath/ath5k/reset.c
> +++ b/drivers/net/wireless/ath/ath5k/reset.c
> @@ -64,8 +64,7 @@ static inline int ath5k_hw_write_ofdm_timings(struct ath5k_hw *ah,
>  	 * we scale coef by shifting clock value by 24 for
>  	 * better precision since we use integers */
>  	/* TODO: Half/quarter rate */
> -	clock =  ath5k_hw_htoclock(1, channel->hw_value & CHANNEL_TURBO);
> -
> +	clock =  (channel->hw_value & CHANNEL_TURBO) ? 80 : 40;
>  	coef_scaled = ((5 * (clock << 24)) / 2) / channel->center_freq;
thanks
Takayuki Kaiso
Kobe/Japan

> The original code was correct in 802.11a mode only, 802.11b/g uses
> different clock rates. The new code uses values taken from FreeBSD HAL
> and should be correct for all modes including turbo modes.
>
> The former rate calculation was used by slope coefficient calculation
> function ath5k_hw_write_ofdm_timings. However, this function requires
> the 802.11a values even in 802.11g mode. Thus the use of
> ath5k_hw_htoclock was replaced by hardcoded values. Possibly the slope
> coefficient calculation is not related to clock rate at all.
>
> Signed-off-by: Lukas Turek <8an@praha12.net>
> ---
>  drivers/net/wireless/ath/ath5k/ath5k.h |   22 ++---------
>  drivers/net/wireless/ath/ath5k/pcu.c   |   64 +++++++++++++++++++++++++++-----
>  drivers/net/wireless/ath/ath5k/qcu.c   |    4 +-
>  drivers/net/wireless/ath/ath5k/reset.c |    3 +-
>  4 files changed, 61 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
> index 6a2a967..ae311d2 100644
> --- a/drivers/net/wireless/ath/ath5k/ath5k.h
> +++ b/drivers/net/wireless/ath/ath5k/ath5k.h
> @@ -1231,6 +1231,10 @@ extern int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout);
>  extern unsigned int ath5k_hw_get_ack_timeout(struct ath5k_hw *ah);
>  extern int ath5k_hw_set_cts_timeout(struct ath5k_hw *ah, unsigned int timeout);
>  extern unsigned int ath5k_hw_get_cts_timeout(struct ath5k_hw *ah);
> +/* Clock rate related functions */
> +unsigned int ath5k_hw_htoclock(struct ath5k_hw *ah, unsigned int usec);
> +unsigned int ath5k_hw_clocktoh(struct ath5k_hw *ah, unsigned int clock);
> +unsigned int ath5k_hw_get_clockrate(struct ath5k_hw *ah);
>  /* Key table (WEP) functions */
>  extern int ath5k_hw_reset_key(struct ath5k_hw *ah, u16 entry);
>  extern int ath5k_hw_is_key_valid(struct ath5k_hw *ah, u16 entry);
> @@ -1310,24 +1314,6 @@ extern int ath5k_hw_set_txpower_limit(struct ath5k_hw *ah, u8 txpower);
>   * Functions used internaly
>   */
>  
> -/*
> - * Translate usec to hw clock units
> - * TODO: Half/quarter rate
> - */
> -static inline unsigned int ath5k_hw_htoclock(unsigned int usec, bool turbo)
> -{
> -	return turbo ? (usec * 80) : (usec * 40);
> -}
> -
> -/*
> - * Translate hw clock units to usec
> - * TODO: Half/quarter rate
> - */
> -static inline unsigned int ath5k_hw_clocktoh(unsigned int clock, bool turbo)
> -{
> -	return turbo ? (clock / 80) : (clock / 40);
> -}
> -
>  static inline struct ath_common *ath5k_hw_common(struct ath5k_hw *ah)
>  {
>          return &ah->common;
> diff --git a/drivers/net/wireless/ath/ath5k/pcu.c b/drivers/net/wireless/ath/ath5k/pcu.c
> index 64fc1eb..8c72845 100644
> --- a/drivers/net/wireless/ath/ath5k/pcu.c
> +++ b/drivers/net/wireless/ath/ath5k/pcu.c
> @@ -187,8 +187,8 @@ unsigned int ath5k_hw_get_ack_timeout(struct ath5k_hw *ah)
>  {
>  	ATH5K_TRACE(ah->ah_sc);
>  
> -	return ath5k_hw_clocktoh(AR5K_REG_MS(ath5k_hw_reg_read(ah,
> -			AR5K_TIME_OUT), AR5K_TIME_OUT_ACK), ah->ah_turbo);
> +	return ath5k_hw_clocktoh(ah, AR5K_REG_MS(ath5k_hw_reg_read(ah,
> +			AR5K_TIME_OUT), AR5K_TIME_OUT_ACK));
>  }
>  
>  /**
> @@ -200,12 +200,12 @@ unsigned int ath5k_hw_get_ack_timeout(struct ath5k_hw *ah)
>  int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout)
>  {
>  	ATH5K_TRACE(ah->ah_sc);
> -	if (ath5k_hw_clocktoh(AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_ACK),
> -			ah->ah_turbo) <= timeout)
> +	if (ath5k_hw_clocktoh(ah, AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_ACK))
> +			<= timeout)
>  		return -EINVAL;
>  
>  	AR5K_REG_WRITE_BITS(ah, AR5K_TIME_OUT, AR5K_TIME_OUT_ACK,
> -		ath5k_hw_htoclock(timeout, ah->ah_turbo));
> +		ath5k_hw_htoclock(ah, timeout));
>  
>  	return 0;
>  }
> @@ -218,8 +218,8 @@ int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout)
>  unsigned int ath5k_hw_get_cts_timeout(struct ath5k_hw *ah)
>  {
>  	ATH5K_TRACE(ah->ah_sc);
> -	return ath5k_hw_clocktoh(AR5K_REG_MS(ath5k_hw_reg_read(ah,
> -			AR5K_TIME_OUT), AR5K_TIME_OUT_CTS), ah->ah_turbo);
> +	return ath5k_hw_clocktoh(ah, AR5K_REG_MS(ath5k_hw_reg_read(ah,
> +			AR5K_TIME_OUT), AR5K_TIME_OUT_CTS));
>  }
>  
>  /**
> @@ -231,17 +231,61 @@ unsigned int ath5k_hw_get_cts_timeout(struct ath5k_hw *ah)
>  int ath5k_hw_set_cts_timeout(struct ath5k_hw *ah, unsigned int timeout)
>  {
>  	ATH5K_TRACE(ah->ah_sc);
> -	if (ath5k_hw_clocktoh(AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_CTS),
> -			ah->ah_turbo) <= timeout)
> +	if (ath5k_hw_clocktoh(ah, AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_CTS))
> +			<= timeout)
>  		return -EINVAL;
>  
>  	AR5K_REG_WRITE_BITS(ah, AR5K_TIME_OUT, AR5K_TIME_OUT_CTS,
> -			ath5k_hw_htoclock(timeout, ah->ah_turbo));
> +			ath5k_hw_htoclock(ah, timeout));
>  
>  	return 0;
>  }
>  
>  /**
> + * ath5k_hw_htoclock - Translate usec to hw clock units
> + *
> + * @ah: The &struct ath5k_hw
> + * @usec: value in microseconds
> + */
> +unsigned int ath5k_hw_htoclock(struct ath5k_hw *ah, unsigned int usec)
> +{
> +	return usec * ath5k_hw_get_clockrate(ah);
> +}
> +
> +/**
> + * ath5k_hw_clocktoh - Translate hw clock units to usec
> + * @clock: value in hw clock units
> + */
> +unsigned int ath5k_hw_clocktoh(struct ath5k_hw *ah, unsigned int clock)
> +{
> +	return clock / ath5k_hw_get_clockrate(ah);
> +}
> +
> +/**
> + * ath5k_hw_get_clockrate - Get the clock rate for current mode
> + *
> + * @ah: The &struct ath5k_hw
> + */
> +unsigned int ath5k_hw_get_clockrate(struct ath5k_hw *ah)
> +{
> +	struct ieee80211_channel *channel = ah->ah_current_channel;
> +	int clock;
> +
> +	if (channel->hw_value & CHANNEL_5GHZ)
> +		clock = 40; /* 802.11a */
> +	else if (channel->hw_value & CHANNEL_CCK)
> +		clock = 22; /* 802.11b */
> +	else
> +		clock = 44; /* 802.11g */
> +
> +	/* Clock rate in turbo modes is twice the normal rate */
> +	if (channel->hw_value & CHANNEL_TURBO)
> +		clock *= 2;
> +
> +	return clock;
> +}
> +
> +/**
>   * ath5k_hw_set_lladdr - Set station id
>   *
>   * @ah: The &struct ath5k_hw
> diff --git a/drivers/net/wireless/ath/ath5k/qcu.c b/drivers/net/wireless/ath/ath5k/qcu.c
> index ed9021a..6af0ac8 100644
> --- a/drivers/net/wireless/ath/ath5k/qcu.c
> +++ b/drivers/net/wireless/ath/ath5k/qcu.c
> @@ -529,7 +529,7 @@ unsigned int ath5k_hw_get_slot_time(struct ath5k_hw *ah)
>  	else
>  		slot_time_clock = ath5k_hw_reg_read(ah, AR5K_DCU_GBL_IFS_SLOT);
>  	
> -	return ath5k_hw_clocktoh(slot_time_clock & 0xffff, ah->ah_turbo);
> +	return ath5k_hw_clocktoh(ah, slot_time_clock & 0xffff);
>  }
>  
>  /*
> @@ -537,7 +537,7 @@ unsigned int ath5k_hw_get_slot_time(struct ath5k_hw *ah)
>   */
>  int ath5k_hw_set_slot_time(struct ath5k_hw *ah, unsigned int slot_time)
>  {
> -	u32 slot_time_clock = ath5k_hw_htoclock(slot_time, ah->ah_turbo);
> +	u32 slot_time_clock = ath5k_hw_htoclock(ah, slot_time);
>  
>  	ATH5K_TRACE(ah->ah_sc);
>  
> diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
> index 62954fc..f1dc4c8 100644
> --- a/drivers/net/wireless/ath/ath5k/reset.c
> +++ b/drivers/net/wireless/ath/ath5k/reset.c
> @@ -64,8 +64,7 @@ static inline int ath5k_hw_write_ofdm_timings(struct ath5k_hw *ah,
>  	 * we scale coef by shifting clock value by 24 for
>  	 * better precision since we use integers */
>  	/* TODO: Half/quarter rate */
> -	clock =  ath5k_hw_htoclock(1, channel->hw_value & CHANNEL_TURBO);
> -
> +	clock =  (channel->hw_value & CHANNEL_TURBO) ? 80 : 40;
>  	coef_scaled = ((5 * (clock << 24)) / 2) / channel->center_freq;
>  
>  	/* Get exponent
>   


-- 
*****************************************
株式会社　シンクチューブ
海藻　敬之 tkaiso@thinktube.com
〒658-0032 神戸市東灘区向洋町中６－９　KFMビル　4E-10
Phone: 078-857-8390
Fax: 078-857-8389
www.thinktube.com

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Lukáš Turek]

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

On 21.12.2009 11:26 海藻敬之 wrote:
>  Didn't we have to handle CHANNEL_2GHZ case in
> ath5k_hw_write_ofdm_timings() shown below ?
The ath5k gives exactly the same results as original Atheros HAL:

  #define INIT_CLOCKMHZSCALED     0x64000000
  unsigned long clockMhzScaled = INIT_CLOCKMHZSCALED;
  if (IS_CHAN_TURBO(chan)) clockMhzScaled *= 2;
  coef_scaled = clockMhzScaled / chan->channel;

I don't know how the calculations work, they might be explained in the 
referenced patent, but I didn't change their semantics at all.

>  I think we should do. then I made my own patch to hadle it and it
> seemed  to improve the throughput of 2.4GHz. (even still not as good as
> 5Ghz case )
Where's the patch?

Lukas Turek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Lukáš Turek]

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

On 21.12.2009 11:41 海藻敬之 wrote:
>  In ath5k_hw_write_ofdm_timings(),  comment says
>       "ALGO: coef = (5 * clock * carrier_freq) / 2) ",
>  but current code is
>       "coef_scaled = ((5 * (clock << 24)) / 2) / channel->center_freq;"
>
>  Did they match each other ?
>  I am wondering the the comment is wrong, but I am not sure that either
> is wrong.
Good point, it seems the comment is wrong. The calculation would overflow 
32-bit integer if there was a multiplication instead of a division.

Lukas Turek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Bob Copeland]

2009/12/21 Lukáš Turek <8an@praha12.net>:
> On 21.12.2009 11:41 海藻敬之 wrote:
>>  In ath5k_hw_write_ofdm_timings(),  comment says
>>       "ALGO: coef = (5 * clock * carrier_freq) / 2) ",
>>  but current code is
>>       "coef_scaled = ((5 * (clock << 24)) / 2) / channel->center_freq;"
>>
>>  Did they match each other ?
>>  I am wondering the the comment is wrong, but I am not sure that either
>> is wrong.
> Good point, it seems the comment is wrong. The calculation would overflow
> 32-bit integer if there was a multiplication instead of a division.
>
> Lukas Turek

The original comment was:

/*
 * ALGO -> coef = 1e8/fcarrier*fclock/40;
 * scaled coef to provide precision for this floating calculation
 */
coef_scaled = clockMhzScaled / chan->channel;

So dividing by the carrier frequency sounds like the right thing,
I guess the comment is wrong.

I reviewed the patch, looks fine to me.  The ATH hal uses a lookup
table to keep it inline but I don't think we have a convenient
index available to do the same.

I'll see what I can find about the pilot tracking to see if that
makes sense here.

-- 
Bob Copeland %% www.bobcopeland.com

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Lukáš Turek]

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

On 21.12.2009 16:08 Bob Copeland wrote:
> I reviewed the patch, looks fine to me.  The ATH hal uses a lookup
> table to keep it inline but I don't think we have a convenient
> index available to do the same.
Yes, HAL uses a lookup table for mode -> clocks coversion, but before that can 
be used it has to convert channel flags to mode, and that's a sequence of ifs 
too (see function ath_hal_chan2wmode).

If performance mattered, we could store the mode index somwhere. But it 
doesn't, the conversion is only needed when setting ACK timeout and slot 
time - which is, for most users, never done and defaults from initvals.c are 
used instead.

Lukas Turek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Bob Copeland]

On Mon, Dec 21, 2009 at 04:28:35PM +0100, Luk???? Turek wrote:
> On 21.12.2009 16:08 Bob Copeland wrote:
> If performance mattered, we could store the mode index somwhere. But it 
> doesn't, the conversion is only needed when setting ACK timeout and slot 
> time - which is, for most users, never done and defaults from initvals.c are 
> used instead.

Agreed.  Feel free to add my

Acked-by: Bob Copeland <me@bobcopeland.com>

to that patch.

-- 
Bob Copeland %% www.bobcopeland.com

Re: [ath5k-devel] [PATCH 4/5] ath5k: Reimplement clock rate to usec conversion

[Lukáš Turek]

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

On 22.12.2009 02:24 海藻敬之 wrote:
> Does original Atheros HAL calls the function just for 5GHz
> or calls for both 2GHz and 5 GHz.?
For both, too, the condition used is IS_CHAN_OFDM(chan).

> --- reset.c_org 2009-12-17 17:01:29.000000000 +0900
> +++ reset.c 2009-12-22 09:51:16.000000000 +0900
> @@ -64,7 +64,14 @@
> * we scale coef by shifting clock value by 24 for
> * better precision since we use integers */
> /* TODO: Half/quarter rate */
> - clock = (channel->hw_value & CHANNEL_TURBO) ? 80 : 40;
> + if (channel->hw_value & CHANNEL_2GHZ)
> + clock = 44; /* here, we do not have to worry about CCK */

If it really improved your throughput, there might be something on it, but I 
still don't think we should change it like that without really understanding 
the algorithm. According to my interpretation the calculation should depend 
on channel width (20MHz in normal mode, 40MHz in turbo mode, same for 802.11g 
and 802.11a), not on MAC chip clocks. At least some Atheros chipsets have 
separate radio chip with its own 40 MHz crystal.

I looked at ath9k source, and it uses the same calculation as FreeBSD HAL, 
including the 0x64000000 constant. Is there anyone with access to Atheros 
documentation who could explain the algorithm at last?

Lukas Turek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]