linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] ath6kl: Debugging and roaming
@ 2011-10-11 14:31 Jouni Malinen
  2011-10-11 14:31 ` [PATCH v2 1/5] ath6kl: Add endpoint_stats debugfs file Jouni Malinen
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Jouni Malinen @ 2011-10-11 14:31 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Jouni Malinen

This set of patches adds some more ath6kl debugging information and
control to debugfs and enables additional roaming functionality.

v2 addresses the comments from Kalle to move debug functionality in
patches 2 and 4 to debug.c.

Jouni Malinen (5):
  ath6kl: Add endpoint_stats debugfs file
  ath6kl: Add debugfs file for target roam table
  ath6kl: Add debugfs files for roaming control
  ath6kl: Add debugfs control for keepalive and disconnection timeout
  ath6kl: Allow CCKM AKM and KRK to be configured

 drivers/net/wireless/ath/ath6kl/cfg80211.c |   14 +
 drivers/net/wireless/ath/ath6kl/core.h     |    7 +
 drivers/net/wireless/ath/ath6kl/debug.c    |  390 ++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath6kl/debug.h    |   19 ++
 drivers/net/wireless/ath/ath6kl/wmi.c      |   55 ++++
 drivers/net/wireless/ath/ath6kl/wmi.h      |   28 ++-
 6 files changed, 507 insertions(+), 6 deletions(-)

-- 
1.7.4.1


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

* [PATCH v2 1/5] ath6kl: Add endpoint_stats debugfs file
  2011-10-11 14:31 [PATCH v2 0/5] ath6kl: Debugging and roaming Jouni Malinen
@ 2011-10-11 14:31 ` Jouni Malinen
  2011-10-11 16:39   ` Joe Perches
  2011-10-11 14:31 ` [PATCH v2 2/5] ath6kl: Add debugfs file for target roam table Jouni Malinen
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Jouni Malinen @ 2011-10-11 14:31 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Jouni Malinen

This file can be used to fetch endpoint statistics counters and
to clear them by writing 0 to it.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/debug.c |  102 +++++++++++++++++++++++++++++++
 1 files changed, 102 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
index ba3f23d..b9bf28d 100644
--- a/drivers/net/wireless/ath/ath6kl/debug.c
+++ b/drivers/net/wireless/ath/ath6kl/debug.c
@@ -595,6 +595,105 @@ static const struct file_operations fops_credit_dist_stats = {
 	.llseek = default_llseek,
 };
 
+static unsigned int print_endpoint_stat(struct htc_target *target, char *buf,
+					unsigned int buf_len, unsigned int len,
+					int offset, const char *name)
+{
+	int i;
+	struct htc_endpoint_stats *ep_st;
+	u32 *counter;
+
+	len += scnprintf(buf + len, buf_len - len, "%s:", name);
+	for (i = 0; i < ENDPOINT_MAX; i++) {
+		ep_st = &target->endpoint[i].ep_st;
+		counter = ((u32 *) ep_st) + (offset / 4);
+		len += scnprintf(buf + len, buf_len - len, " %u", *counter);
+	}
+	len += scnprintf(buf + len, buf_len - len, "\n");
+
+	return len;
+}
+
+static ssize_t ath6kl_endpoint_stats_read(struct file *file,
+					  char __user *user_buf,
+					  size_t count, loff_t *ppos)
+{
+	struct ath6kl *ar = file->private_data;
+	struct htc_target *target = ar->htc_target;
+	char *buf;
+	unsigned int buf_len, len = 0;
+	ssize_t ret_cnt;
+
+	buf_len = 1000 + ENDPOINT_MAX * 100;
+	buf = kzalloc(buf_len, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+#define EPSTAT(name)							\
+	len = print_endpoint_stat(target, buf, buf_len, len,		\
+				  offsetof(struct htc_endpoint_stats, name), \
+				  #name)
+	EPSTAT(cred_low_indicate);
+	EPSTAT(tx_issued);
+	EPSTAT(tx_pkt_bundled);
+	EPSTAT(tx_bundles);
+	EPSTAT(tx_dropped);
+	EPSTAT(tx_cred_rpt);
+	EPSTAT(cred_rpt_from_rx);
+	EPSTAT(cred_rpt_ep0);
+	EPSTAT(cred_from_rx);
+	EPSTAT(cred_from_other);
+	EPSTAT(cred_from_ep0);
+	EPSTAT(cred_cosumd);
+	EPSTAT(cred_retnd);
+	EPSTAT(rx_pkts);
+	EPSTAT(rx_lkahds);
+	EPSTAT(rx_bundl);
+	EPSTAT(rx_bundle_lkahd);
+	EPSTAT(rx_bundle_from_hdr);
+	EPSTAT(rx_alloc_thresh_hit);
+	EPSTAT(rxalloc_thresh_byte);
+#undef EPSTAT
+
+	if (len > buf_len)
+		len = buf_len;
+
+	ret_cnt = simple_read_from_buffer(user_buf, count, ppos, buf, len);
+	kfree(buf);
+	return ret_cnt;
+}
+
+static ssize_t ath6kl_endpoint_stats_write(struct file *file,
+					   const char __user *user_buf,
+					   size_t count, loff_t *ppos)
+{
+	struct ath6kl *ar = file->private_data;
+	struct htc_target *target = ar->htc_target;
+	int ret, i;
+	u32 val;
+	struct htc_endpoint_stats *ep_st;
+
+	ret = kstrtou32_from_user(user_buf, count, 0, &val);
+	if (ret)
+		return ret;
+	if (val == 0) {
+		for (i = 0; i < ENDPOINT_MAX; i++) {
+			ep_st = &target->endpoint[i].ep_st;
+			memset(ep_st, 0, sizeof(*ep_st));
+		}
+	}
+
+	return count;
+}
+
+static const struct file_operations fops_endpoint_stats = {
+	.open = ath6kl_debugfs_open,
+	.read = ath6kl_endpoint_stats_read,
+	.write = ath6kl_endpoint_stats_write,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 static unsigned long ath6kl_get_num_reg(void)
 {
 	int i;
@@ -901,6 +1000,9 @@ int ath6kl_debug_init(struct ath6kl *ar)
 	debugfs_create_file("credit_dist_stats", S_IRUSR, ar->debugfs_phy, ar,
 			    &fops_credit_dist_stats);
 
+	debugfs_create_file("endpoint_stats", S_IRUSR | S_IWUSR,
+			    ar->debugfs_phy, ar, &fops_endpoint_stats);
+
 	debugfs_create_file("fwlog", S_IRUSR, ar->debugfs_phy, ar,
 			    &fops_fwlog);
 
-- 
1.7.4.1


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

* [PATCH v2 2/5] ath6kl: Add debugfs file for target roam table
  2011-10-11 14:31 [PATCH v2 0/5] ath6kl: Debugging and roaming Jouni Malinen
  2011-10-11 14:31 ` [PATCH v2 1/5] ath6kl: Add endpoint_stats debugfs file Jouni Malinen
@ 2011-10-11 14:31 ` Jouni Malinen
  2011-10-11 14:31 ` [PATCH v2 3/5] ath6kl: Add debugfs files for roaming control Jouni Malinen
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Jouni Malinen @ 2011-10-11 14:31 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Jouni Malinen

The new roam_table debugfs file can be used to display the current
roam table from the target.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/core.h  |    4 +
 drivers/net/wireless/ath/ath6kl/debug.c |  109 +++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath6kl/debug.h |    8 ++
 drivers/net/wireless/ath/ath6kl/wmi.c   |   11 +++
 drivers/net/wireless/ath/ath6kl/wmi.h   |    7 ++
 5 files changed, 139 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/core.h b/drivers/net/wireless/ath/ath6kl/core.h
index 6d8a484..c58cfad 100644
--- a/drivers/net/wireless/ath/ath6kl/core.h
+++ b/drivers/net/wireless/ath/ath6kl/core.h
@@ -397,6 +397,7 @@ struct ath6kl_req_key {
 #define TESTMODE	     13
 #define CLEAR_BSSFILTER_ON_BEACON 14
 #define DTIM_PERIOD_AVAIL    15
+#define ROAM_TBL_PEND        16
 
 struct ath6kl {
 	struct device *dev;
@@ -529,6 +530,9 @@ struct ath6kl {
 		struct {
 			unsigned int invalid_rate;
 		} war_stats;
+
+		u8 *roam_tbl;
+		unsigned int roam_tbl_len;
 	} debug;
 #endif /* CONFIG_ATH6KL_DEBUG */
 };
diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
index b9bf28d..cec958a 100644
--- a/drivers/net/wireless/ath/ath6kl/debug.c
+++ b/drivers/net/wireless/ath/ath6kl/debug.c
@@ -966,6 +966,111 @@ static const struct file_operations fops_diag_reg_write = {
 	.llseek = default_llseek,
 };
 
+int ath6kl_debug_roam_tbl_event(struct ath6kl *ar, const void *buf,
+				size_t len)
+{
+	const struct wmi_target_roam_tbl *tbl;
+	u16 num_entries;
+
+	if (len < sizeof(*tbl))
+		return -EINVAL;
+
+	tbl = (const struct wmi_target_roam_tbl *) buf;
+	num_entries = le16_to_cpu(tbl->num_entries);
+	if (sizeof(*tbl) + num_entries * sizeof(struct wmi_bss_roam_info) >
+	    len)
+		return -EINVAL;
+
+	if (ar->debug.roam_tbl == NULL ||
+	    ar->debug.roam_tbl_len < (unsigned int) len) {
+		kfree(ar->debug.roam_tbl);
+		ar->debug.roam_tbl = kmalloc(len, GFP_ATOMIC);
+		if (ar->debug.roam_tbl == NULL)
+			return -ENOMEM;
+	}
+
+	memcpy(ar->debug.roam_tbl, buf, len);
+	ar->debug.roam_tbl_len = len;
+
+	if (test_bit(ROAM_TBL_PEND, &ar->flag)) {
+		clear_bit(ROAM_TBL_PEND, &ar->flag);
+		wake_up(&ar->event_wq);
+	}
+
+	return 0;
+}
+
+static ssize_t ath6kl_roam_table_read(struct file *file, char __user *user_buf,
+				      size_t count, loff_t *ppos)
+{
+	struct ath6kl *ar = file->private_data;
+	int ret;
+	long left;
+	struct wmi_target_roam_tbl *tbl;
+	u16 num_entries, i;
+	char *buf;
+	unsigned int len, buf_len;
+	ssize_t ret_cnt;
+
+	if (down_interruptible(&ar->sem))
+		return -EBUSY;
+
+	set_bit(ROAM_TBL_PEND, &ar->flag);
+
+	ret = ath6kl_wmi_get_roam_tbl_cmd(ar->wmi);
+	if (ret) {
+		up(&ar->sem);
+		return ret;
+	}
+
+	left = wait_event_interruptible_timeout(
+		ar->event_wq, !test_bit(ROAM_TBL_PEND, &ar->flag), WMI_TIMEOUT);
+	up(&ar->sem);
+
+	if (left <= 0)
+		return -ETIMEDOUT;
+
+	if (ar->debug.roam_tbl == NULL)
+		return -ENOMEM;
+
+	tbl = (struct wmi_target_roam_tbl *) ar->debug.roam_tbl;
+	num_entries = le16_to_cpu(tbl->num_entries);
+
+	buf_len = 100 + num_entries * 100;
+	buf = kzalloc(buf_len, GFP_KERNEL);
+	if (buf == NULL)
+		return -ENOMEM;
+	len = 0;
+	len += scnprintf(buf + len, buf_len - len,
+			 "roam_mode=%u\n\n"
+			 "# roam_util bssid rssi rssidt last_rssi util bias\n",
+			 le16_to_cpu(tbl->roam_mode));
+
+	for (i = 0; i < num_entries; i++) {
+		struct wmi_bss_roam_info *info = &tbl->info[i];
+		len += scnprintf(buf + len, buf_len - len,
+				 "%d %pM %d %d %d %d %d\n",
+				 a_sle32_to_cpu(info->roam_util), info->bssid,
+				 info->rssi, info->rssidt, info->last_rssi,
+				 info->util, info->bias);
+	}
+
+	if (len > buf_len)
+		len = buf_len;
+
+	ret_cnt = simple_read_from_buffer(user_buf, count, ppos, buf, len);
+
+	kfree(buf);
+	return ret_cnt;
+}
+
+static const struct file_operations fops_roam_table = {
+	.read = ath6kl_roam_table_read,
+	.open = ath6kl_debugfs_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 int ath6kl_debug_init(struct ath6kl *ar)
 {
 	ar->debug.fwlog_buf.buf = vmalloc(ATH6KL_FWLOG_SIZE);
@@ -1024,6 +1129,9 @@ int ath6kl_debug_init(struct ath6kl *ar)
 	debugfs_create_file("war_stats", S_IRUSR, ar->debugfs_phy, ar,
 			    &fops_war_stats);
 
+	debugfs_create_file("roam_table", S_IRUSR, ar->debugfs_phy, ar,
+			    &fops_roam_table);
+
 	return 0;
 }
 
@@ -1031,6 +1139,7 @@ void ath6kl_debug_cleanup(struct ath6kl *ar)
 {
 	vfree(ar->debug.fwlog_buf.buf);
 	kfree(ar->debug.fwlog_tmp);
+	kfree(ar->debug.roam_tbl);
 }
 
 #endif
diff --git a/drivers/net/wireless/ath/ath6kl/debug.h b/drivers/net/wireless/ath/ath6kl/debug.h
index e3740b0..f73bf15 100644
--- a/drivers/net/wireless/ath/ath6kl/debug.h
+++ b/drivers/net/wireless/ath/ath6kl/debug.h
@@ -90,6 +90,8 @@ void ath6kl_dump_registers(struct ath6kl_device *dev,
 void dump_cred_dist_stats(struct htc_target *target);
 void ath6kl_debug_fwlog_event(struct ath6kl *ar, const void *buf, size_t len);
 void ath6kl_debug_war(struct ath6kl *ar, enum ath6kl_war war);
+int ath6kl_debug_roam_tbl_event(struct ath6kl *ar, const void *buf,
+				size_t len);
 int ath6kl_debug_init(struct ath6kl *ar);
 void ath6kl_debug_cleanup(struct ath6kl *ar);
 
@@ -125,6 +127,12 @@ static inline void ath6kl_debug_war(struct ath6kl *ar, enum ath6kl_war war)
 {
 }
 
+static inline int ath6kl_debug_roam_tbl_event(struct ath6kl *ar,
+					      const void *buf, size_t len)
+{
+	return 0;
+}
+
 static inline int ath6kl_debug_init(struct ath6kl *ar)
 {
 	return 0;
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index ab782d7..4021527 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -2407,6 +2407,11 @@ int ath6kl_wmi_get_tx_pwr_cmd(struct wmi *wmi)
 	return ath6kl_wmi_simple_cmd(wmi, WMI_GET_TX_PWR_CMDID);
 }
 
+int ath6kl_wmi_get_roam_tbl_cmd(struct wmi *wmi)
+{
+	return ath6kl_wmi_simple_cmd(wmi, WMI_GET_ROAM_TBL_CMDID);
+}
+
 int ath6kl_wmi_set_lpreamble_cmd(struct wmi *wmi, u8 status, u8 preamble_policy)
 {
 	struct sk_buff *skb;
@@ -2844,6 +2849,11 @@ static int ath6kl_wmi_control_rx_xtnd(struct wmi *wmi, struct sk_buff *skb)
 	return ret;
 }
 
+static int ath6kl_wmi_roam_tbl_event_rx(struct wmi *wmi, u8 *datap, int len)
+{
+	return ath6kl_debug_roam_tbl_event(wmi->parent_dev, datap, len);
+}
+
 /* Control Path */
 int ath6kl_wmi_control_rx(struct wmi *wmi, struct sk_buff *skb)
 {
@@ -2948,6 +2958,7 @@ int ath6kl_wmi_control_rx(struct wmi *wmi, struct sk_buff *skb)
 		break;
 	case WMI_REPORT_ROAM_TBL_EVENTID:
 		ath6kl_dbg(ATH6KL_DBG_WMI, "WMI_REPORT_ROAM_TBL_EVENTID\n");
+		ret = ath6kl_wmi_roam_tbl_event_rx(wmi, datap, len);
 		break;
 	case WMI_EXTENSION_EVENTID:
 		ath6kl_dbg(ATH6KL_DBG_WMI, "WMI_EXTENSION_EVENTID\n");
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.h b/drivers/net/wireless/ath/ath6kl/wmi.h
index 96102c6..f986da1 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.h
+++ b/drivers/net/wireless/ath/ath6kl/wmi.h
@@ -1624,6 +1624,12 @@ struct wmi_bss_roam_info {
 	u8 reserved;
 } __packed;
 
+struct wmi_target_roam_tbl {
+	__le16 roam_mode;
+	__le16 num_entries;
+	struct wmi_bss_roam_info info[];
+} __packed;
+
 /* WMI_CAC_EVENTID */
 enum cac_indication {
 	CAC_INDICATION_ADMISSION = 0x00,
@@ -2221,6 +2227,7 @@ int ath6kl_wmi_setpmkid_cmd(struct wmi *wmi, const u8 *bssid,
 			    const u8 *pmkid, bool set);
 int ath6kl_wmi_set_tx_pwr_cmd(struct wmi *wmi, u8 dbM);
 int ath6kl_wmi_get_tx_pwr_cmd(struct wmi *wmi);
+int ath6kl_wmi_get_roam_tbl_cmd(struct wmi *wmi);
 
 int ath6kl_wmi_set_wmm_txop(struct wmi *wmi, enum wmi_txop_cfg cfg);
 int ath6kl_wmi_set_keepalive_cmd(struct wmi *wmi, u8 keep_alive_intvl);
-- 
1.7.4.1


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

* [PATCH v2 3/5] ath6kl: Add debugfs files for roaming control
  2011-10-11 14:31 [PATCH v2 0/5] ath6kl: Debugging and roaming Jouni Malinen
  2011-10-11 14:31 ` [PATCH v2 1/5] ath6kl: Add endpoint_stats debugfs file Jouni Malinen
  2011-10-11 14:31 ` [PATCH v2 2/5] ath6kl: Add debugfs file for target roam table Jouni Malinen
@ 2011-10-11 14:31 ` Jouni Malinen
  2011-10-11 14:31 ` [PATCH v2 4/5] ath6kl: Add debugfs control for keepalive and disconnection timeout Jouni Malinen
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Jouni Malinen @ 2011-10-11 14:31 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Jouni Malinen

Roaming mode can be changed by writing roam mode (default, bssbias, or
lock) to roam_mode. Forced roam can be requested by writing the BSSID
into force_roam.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/debug.c |   84 +++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath6kl/wmi.c   |   40 +++++++++++++++
 drivers/net/wireless/ath/ath6kl/wmi.h   |   21 ++++++--
 3 files changed, 139 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
index cec958a..41161ca 100644
--- a/drivers/net/wireless/ath/ath6kl/debug.c
+++ b/drivers/net/wireless/ath/ath6kl/debug.c
@@ -1071,6 +1071,84 @@ static const struct file_operations fops_roam_table = {
 	.llseek = default_llseek,
 };
 
+static ssize_t ath6kl_force_roam_write(struct file *file,
+				       const char __user *user_buf,
+				       size_t count, loff_t *ppos)
+{
+	struct ath6kl *ar = file->private_data;
+	int ret;
+	char buf[20];
+	size_t len;
+	u8 bssid[ETH_ALEN];
+	int i;
+	int addr[ETH_ALEN];
+
+	len = min(count, sizeof(buf) - 1);
+	if (copy_from_user(buf, user_buf, len))
+		return -EFAULT;
+	buf[len] = '\0';
+
+	if (sscanf(buf, "%02x:%02x:%02x:%02x:%02x:%02x",
+		   &addr[0], &addr[1], &addr[2], &addr[3], &addr[4], &addr[5])
+	    != ETH_ALEN)
+		return -EINVAL;
+	for (i = 0; i < ETH_ALEN; i++)
+		bssid[i] = addr[i];
+
+	ret = ath6kl_wmi_force_roam_cmd(ar->wmi, bssid);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static const struct file_operations fops_force_roam = {
+	.write = ath6kl_force_roam_write,
+	.open = ath6kl_debugfs_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
+static ssize_t ath6kl_roam_mode_write(struct file *file,
+				      const char __user *user_buf,
+				      size_t count, loff_t *ppos)
+{
+	struct ath6kl *ar = file->private_data;
+	int ret;
+	char buf[20];
+	size_t len;
+	enum wmi_roam_mode mode;
+
+	len = min(count, sizeof(buf) - 1);
+	if (copy_from_user(buf, user_buf, len))
+		return -EFAULT;
+	buf[len] = '\0';
+	if (len > 0 && buf[len - 1] == '\n')
+		buf[len - 1] = '\0';
+
+	if (strcasecmp(buf, "default") == 0)
+		mode = WMI_DEFAULT_ROAM_MODE;
+	else if (strcasecmp(buf, "bssbias") == 0)
+		mode = WMI_HOST_BIAS_ROAM_MODE;
+	else if (strcasecmp(buf, "lock") == 0)
+		mode = WMI_LOCK_BSS_MODE;
+	else
+		return -EINVAL;
+
+	ret = ath6kl_wmi_set_roam_mode_cmd(ar->wmi, mode);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static const struct file_operations fops_roam_mode = {
+	.write = ath6kl_roam_mode_write,
+	.open = ath6kl_debugfs_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 int ath6kl_debug_init(struct ath6kl *ar)
 {
 	ar->debug.fwlog_buf.buf = vmalloc(ATH6KL_FWLOG_SIZE);
@@ -1132,6 +1210,12 @@ int ath6kl_debug_init(struct ath6kl *ar)
 	debugfs_create_file("roam_table", S_IRUSR, ar->debugfs_phy, ar,
 			    &fops_roam_table);
 
+	debugfs_create_file("force_roam", S_IWUSR, ar->debugfs_phy, ar,
+			    &fops_force_roam);
+
+	debugfs_create_file("roam_mode", S_IWUSR, ar->debugfs_phy, ar,
+			    &fops_roam_mode);
+
 	return 0;
 }
 
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index 4021527..3fb2702 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -682,6 +682,46 @@ int ath6kl_wmi_set_roam_lrssi_cmd(struct wmi *wmi, u8 lrssi)
 	return 0;
 }
 
+int ath6kl_wmi_force_roam_cmd(struct wmi *wmi, const u8 *bssid)
+{
+	struct sk_buff *skb;
+	struct roam_ctrl_cmd *cmd;
+
+	skb = ath6kl_wmi_get_new_buf(sizeof(*cmd));
+	if (!skb)
+		return -ENOMEM;
+
+	cmd = (struct roam_ctrl_cmd *) skb->data;
+	memset(cmd, 0, sizeof(*cmd));
+
+	memcpy(cmd->info.bssid, bssid, ETH_ALEN);
+	cmd->roam_ctrl = WMI_FORCE_ROAM;
+
+	ath6kl_dbg(ATH6KL_DBG_WMI, "force roam to %pM\n", bssid);
+	return ath6kl_wmi_cmd_send(wmi, skb, WMI_SET_ROAM_CTRL_CMDID,
+				   NO_SYNC_WMIFLAG);
+}
+
+int ath6kl_wmi_set_roam_mode_cmd(struct wmi *wmi, enum wmi_roam_mode mode)
+{
+	struct sk_buff *skb;
+	struct roam_ctrl_cmd *cmd;
+
+	skb = ath6kl_wmi_get_new_buf(sizeof(*cmd));
+	if (!skb)
+		return -ENOMEM;
+
+	cmd = (struct roam_ctrl_cmd *) skb->data;
+	memset(cmd, 0, sizeof(*cmd));
+
+	cmd->info.roam_mode = mode;
+	cmd->roam_ctrl = WMI_SET_ROAM_MODE;
+
+	ath6kl_dbg(ATH6KL_DBG_WMI, "set roam mode %d\n", mode);
+	return ath6kl_wmi_cmd_send(wmi, skb, WMI_SET_ROAM_CTRL_CMDID,
+				   NO_SYNC_WMIFLAG);
+}
+
 static int ath6kl_wmi_connect_event_rx(struct wmi *wmi, u8 *datap, int len)
 {
 	struct wmi_connect_event *ev;
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.h b/drivers/net/wireless/ath/ath6kl/wmi.h
index f986da1..f0ca899 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.h
+++ b/drivers/net/wireless/ath/ath6kl/wmi.h
@@ -1354,14 +1354,20 @@ enum wmi_roam_ctrl {
 	WMI_SET_LRSSI_SCAN_PARAMS,
 };
 
+enum wmi_roam_mode {
+	WMI_DEFAULT_ROAM_MODE = 1, /* RSSI based roam */
+	WMI_HOST_BIAS_ROAM_MODE = 2, /* Host bias based roam */
+	WMI_LOCK_BSS_MODE = 3, /* Lock to the current BSS */
+};
+
 struct bss_bias {
 	u8 bssid[ETH_ALEN];
-	u8  bias;
+	s8 bias;
 } __packed;
 
 struct bss_bias_info {
 	u8 num_bss;
-	struct bss_bias bss_bias[1];
+	struct bss_bias bss_bias[0];
 } __packed;
 
 struct low_rssi_scan_params {
@@ -1374,10 +1380,11 @@ struct low_rssi_scan_params {
 
 struct roam_ctrl_cmd {
 	union {
-		u8 bssid[ETH_ALEN];
-		u8 roam_mode;
-		struct bss_bias_info bss;
-		struct low_rssi_scan_params params;
+		u8 bssid[ETH_ALEN]; /* WMI_FORCE_ROAM */
+		u8 roam_mode; /* WMI_SET_ROAM_MODE */
+		struct bss_bias_info bss; /* WMI_SET_HOST_BIAS */
+		struct low_rssi_scan_params params; /* WMI_SET_LRSSI_SCAN_PARAMS
+						     */
 	} __packed info;
 	u8 roam_ctrl;
 } __packed;
@@ -2237,6 +2244,8 @@ s32 ath6kl_wmi_get_rate(s8 rate_index);
 
 int ath6kl_wmi_set_ip_cmd(struct wmi *wmi, struct wmi_set_ip_cmd *ip_cmd);
 int ath6kl_wmi_set_roam_lrssi_cmd(struct wmi *wmi, u8 lrssi);
+int ath6kl_wmi_force_roam_cmd(struct wmi *wmi, const u8 *bssid);
+int ath6kl_wmi_set_roam_mode_cmd(struct wmi *wmi, enum wmi_roam_mode mode);
 
 /* AP mode */
 int ath6kl_wmi_ap_profile_commit(struct wmi *wmip, struct wmi_connect_cmd *p);
-- 
1.7.4.1


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

* [PATCH v2 4/5] ath6kl: Add debugfs control for keepalive and disconnection timeout
  2011-10-11 14:31 [PATCH v2 0/5] ath6kl: Debugging and roaming Jouni Malinen
                   ` (2 preceding siblings ...)
  2011-10-11 14:31 ` [PATCH v2 3/5] ath6kl: Add debugfs files for roaming control Jouni Malinen
@ 2011-10-11 14:31 ` Jouni Malinen
  2011-10-11 14:31 ` [PATCH v2 5/5] ath6kl: Allow CCKM AKM and KRK to be configured Jouni Malinen
  2011-10-11 16:13 ` [PATCH v2 0/5] ath6kl: Debugging and roaming Kalle Valo
  5 siblings, 0 replies; 10+ messages in thread
From: Jouni Malinen @ 2011-10-11 14:31 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Jouni Malinen

The new debugfs files keepalive and disconnect_timeout can be used to
fetch the current values and to change the values for keepalive and
disconnect event timeout (both in seconds).

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/core.h  |    3 +
 drivers/net/wireless/ath/ath6kl/debug.c |   95 +++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath6kl/debug.h |   11 ++++
 drivers/net/wireless/ath/ath6kl/wmi.c   |    4 +
 4 files changed, 113 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/core.h b/drivers/net/wireless/ath/ath6kl/core.h
index c58cfad..31e5c7e 100644
--- a/drivers/net/wireless/ath/ath6kl/core.h
+++ b/drivers/net/wireless/ath/ath6kl/core.h
@@ -533,6 +533,9 @@ struct ath6kl {
 
 		u8 *roam_tbl;
 		unsigned int roam_tbl_len;
+
+		u8 keepalive;
+		u8 disc_timeout;
 	} debug;
 #endif /* CONFIG_ATH6KL_DEBUG */
 };
diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
index 41161ca..7b1c9ae 100644
--- a/drivers/net/wireless/ath/ath6kl/debug.c
+++ b/drivers/net/wireless/ath/ath6kl/debug.c
@@ -1149,6 +1149,95 @@ static const struct file_operations fops_roam_mode = {
 	.llseek = default_llseek,
 };
 
+void ath6kl_debug_set_keepalive(struct ath6kl *ar, u8 keepalive)
+{
+	ar->debug.keepalive = keepalive;
+}
+
+static ssize_t ath6kl_keepalive_read(struct file *file, char __user *user_buf,
+				     size_t count, loff_t *ppos)
+{
+	struct ath6kl *ar = file->private_data;
+	char buf[16];
+	int len;
+
+	len = snprintf(buf, sizeof(buf), "%u\n", ar->debug.keepalive);
+
+	return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t ath6kl_keepalive_write(struct file *file,
+				      const char __user *user_buf,
+				      size_t count, loff_t *ppos)
+{
+	struct ath6kl *ar = file->private_data;
+	int ret;
+	u8 val;
+
+	ret = kstrtou8_from_user(user_buf, count, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = ath6kl_wmi_set_keepalive_cmd(ar->wmi, val);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static const struct file_operations fops_keepalive = {
+	.open = ath6kl_debugfs_open,
+	.read = ath6kl_keepalive_read,
+	.write = ath6kl_keepalive_write,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
+void ath6kl_debug_set_disconnect_timeout(struct ath6kl *ar, u8 timeout)
+{
+	ar->debug.disc_timeout = timeout;
+}
+
+static ssize_t ath6kl_disconnect_timeout_read(struct file *file,
+					      char __user *user_buf,
+					      size_t count, loff_t *ppos)
+{
+	struct ath6kl *ar = file->private_data;
+	char buf[16];
+	int len;
+
+	len = snprintf(buf, sizeof(buf), "%u\n", ar->debug.disc_timeout);
+
+	return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t ath6kl_disconnect_timeout_write(struct file *file,
+					       const char __user *user_buf,
+					       size_t count, loff_t *ppos)
+{
+	struct ath6kl *ar = file->private_data;
+	int ret;
+	u8 val;
+
+	ret = kstrtou8_from_user(user_buf, count, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = ath6kl_wmi_disctimeout_cmd(ar->wmi, val);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static const struct file_operations fops_disconnect_timeout = {
+	.open = ath6kl_debugfs_open,
+	.read = ath6kl_disconnect_timeout_read,
+	.write = ath6kl_disconnect_timeout_write,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 int ath6kl_debug_init(struct ath6kl *ar)
 {
 	ar->debug.fwlog_buf.buf = vmalloc(ATH6KL_FWLOG_SIZE);
@@ -1216,6 +1305,12 @@ int ath6kl_debug_init(struct ath6kl *ar)
 	debugfs_create_file("roam_mode", S_IWUSR, ar->debugfs_phy, ar,
 			    &fops_roam_mode);
 
+	debugfs_create_file("keepalive", S_IRUSR | S_IWUSR, ar->debugfs_phy, ar,
+			    &fops_keepalive);
+
+	debugfs_create_file("disconnect_timeout", S_IRUSR | S_IWUSR,
+			    ar->debugfs_phy, ar, &fops_disconnect_timeout);
+
 	return 0;
 }
 
diff --git a/drivers/net/wireless/ath/ath6kl/debug.h b/drivers/net/wireless/ath/ath6kl/debug.h
index f73bf15..7d5323d 100644
--- a/drivers/net/wireless/ath/ath6kl/debug.h
+++ b/drivers/net/wireless/ath/ath6kl/debug.h
@@ -92,6 +92,8 @@ void ath6kl_debug_fwlog_event(struct ath6kl *ar, const void *buf, size_t len);
 void ath6kl_debug_war(struct ath6kl *ar, enum ath6kl_war war);
 int ath6kl_debug_roam_tbl_event(struct ath6kl *ar, const void *buf,
 				size_t len);
+void ath6kl_debug_set_keepalive(struct ath6kl *ar, u8 keepalive);
+void ath6kl_debug_set_disconnect_timeout(struct ath6kl *ar, u8 timeout);
 int ath6kl_debug_init(struct ath6kl *ar);
 void ath6kl_debug_cleanup(struct ath6kl *ar);
 
@@ -133,6 +135,15 @@ static inline int ath6kl_debug_roam_tbl_event(struct ath6kl *ar,
 	return 0;
 }
 
+static inline void ath6kl_debug_set_keepalive(struct ath6kl *ar, u8 keepalive)
+{
+}
+
+static inline void ath6kl_debug_set_disconnect_timeout(struct ath6kl *ar,
+						       u8 timeout)
+{
+}
+
 static inline int ath6kl_debug_init(struct ath6kl *ar)
 {
 	return 0;
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index 3fb2702..7b6bfdd 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -1940,6 +1940,8 @@ int ath6kl_wmi_disctimeout_cmd(struct wmi *wmi, u8 timeout)
 
 	ret = ath6kl_wmi_cmd_send(wmi, skb, WMI_SET_DISC_TIMEOUT_CMDID,
 				  NO_SYNC_WMIFLAG);
+	if (ret == 0)
+		ath6kl_debug_set_disconnect_timeout(wmi->parent_dev, timeout);
 	return ret;
 }
 
@@ -2524,6 +2526,8 @@ int ath6kl_wmi_set_keepalive_cmd(struct wmi *wmi, u8 keep_alive_intvl)
 
 	ret = ath6kl_wmi_cmd_send(wmi, skb, WMI_SET_KEEPALIVE_CMDID,
 				  NO_SYNC_WMIFLAG);
+	if (ret == 0)
+		ath6kl_debug_set_keepalive(wmi->parent_dev, keep_alive_intvl);
 	return ret;
 }
 
-- 
1.7.4.1


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

* [PATCH v2 5/5] ath6kl: Allow CCKM AKM and KRK to be configured
  2011-10-11 14:31 [PATCH v2 0/5] ath6kl: Debugging and roaming Jouni Malinen
                   ` (3 preceding siblings ...)
  2011-10-11 14:31 ` [PATCH v2 4/5] ath6kl: Add debugfs control for keepalive and disconnection timeout Jouni Malinen
@ 2011-10-11 14:31 ` Jouni Malinen
  2011-10-11 16:13 ` [PATCH v2 0/5] ath6kl: Debugging and roaming Kalle Valo
  5 siblings, 0 replies; 10+ messages in thread
From: Jouni Malinen @ 2011-10-11 14:31 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Jouni Malinen

Use vendor-specific suite selectors to allow CCKM to be configured.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/cfg80211.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index b7b2c57..6b30b75 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -121,6 +121,8 @@ static struct ieee80211_supported_band ath6kl_band_5ghz = {
 	.bitrates = ath6kl_a_rates,
 };
 
+#define CCKM_KRK_CIPHER_SUITE 0x004096ff /* use for KRK */
+
 static int ath6kl_set_wpa_version(struct ath6kl *ar,
 				  enum nl80211_wpa_versions wpa_version)
 {
@@ -217,6 +219,11 @@ static void ath6kl_set_key_mgmt(struct ath6kl *ar, u32 key_mgmt)
 			ar->auth_mode = WPA_PSK_AUTH;
 		else if (ar->auth_mode == WPA2_AUTH)
 			ar->auth_mode = WPA2_PSK_AUTH;
+	} else if (key_mgmt == 0x00409600) {
+		if (ar->auth_mode == WPA_AUTH)
+			ar->auth_mode = WPA_AUTH_CCKM;
+		else if (ar->auth_mode == WPA2_AUTH)
+			ar->auth_mode = WPA2_AUTH_CCKM;
 	} else if (key_mgmt != WLAN_AKM_SUITE_8021X) {
 		ar->auth_mode = NONE_AUTH;
 	}
@@ -758,6 +765,12 @@ static int ath6kl_cfg80211_add_key(struct wiphy *wiphy, struct net_device *ndev,
 	if (!ath6kl_cfg80211_ready(ar))
 		return -EIO;
 
+	if (params->cipher == CCKM_KRK_CIPHER_SUITE) {
+		if (params->key_len != WMI_KRK_LEN)
+			return -EINVAL;
+		return ath6kl_wmi_add_krk_cmd(ar->wmi, params->key);
+	}
+
 	if (key_index < WMI_MIN_KEY_INDEX || key_index > WMI_MAX_KEY_INDEX) {
 		ath6kl_dbg(ATH6KL_DBG_WLAN_CFG,
 			   "%s: key index %d out of bounds\n", __func__,
@@ -1228,6 +1241,7 @@ static const u32 cipher_suites[] = {
 	WLAN_CIPHER_SUITE_WEP104,
 	WLAN_CIPHER_SUITE_TKIP,
 	WLAN_CIPHER_SUITE_CCMP,
+	CCKM_KRK_CIPHER_SUITE,
 };
 
 static bool is_rate_legacy(s32 rate)
-- 
1.7.4.1


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

* Re: [PATCH v2 0/5] ath6kl: Debugging and roaming
  2011-10-11 14:31 [PATCH v2 0/5] ath6kl: Debugging and roaming Jouni Malinen
                   ` (4 preceding siblings ...)
  2011-10-11 14:31 ` [PATCH v2 5/5] ath6kl: Allow CCKM AKM and KRK to be configured Jouni Malinen
@ 2011-10-11 16:13 ` Kalle Valo
  5 siblings, 0 replies; 10+ messages in thread
From: Kalle Valo @ 2011-10-11 16:13 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: linux-wireless

On 10/11/2011 05:31 PM, Jouni Malinen wrote:
> This set of patches adds some more ath6kl debugging information and
> control to debugfs and enables additional roaming functionality.
> 
> v2 addresses the comments from Kalle to move debug functionality in
> patches 2 and 4 to debug.c.

Thanks, all five applied.

Kalle

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

* Re: [PATCH v2 1/5] ath6kl: Add endpoint_stats debugfs file
  2011-10-11 14:31 ` [PATCH v2 1/5] ath6kl: Add endpoint_stats debugfs file Jouni Malinen
@ 2011-10-11 16:39   ` Joe Perches
  2011-10-11 18:52     ` Jouni Malinen
  0 siblings, 1 reply; 10+ messages in thread
From: Joe Perches @ 2011-10-11 16:39 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: kvalo, linux-wireless

On Tue, 2011-10-11 at 17:31 +0300, Jouni Malinen wrote:
> This file can be used to fetch endpoint statistics counters and
> to clear them by writing 0 to it.

Hi Jouni.

[petty carping follows]

> Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath6kl/debug.c |  102 +++++++++++++++++++++++++++++++
>  1 files changed, 102 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
> index ba3f23d..b9bf28d 100644
> --- a/drivers/net/wireless/ath/ath6kl/debug.c
> +++ b/drivers/net/wireless/ath/ath6kl/debug.c
> @@ -595,6 +595,105 @@ static const struct file_operations fops_credit_dist_stats = {
>  	.llseek = default_llseek,
>  };
>  
> +static unsigned int print_endpoint_stat(struct htc_target *target, char *buf,
> +					unsigned int buf_len, unsigned int len,
> +					int offset, const char *name)

Perhaps the function name is wrong.
This doesn't print, it fills a buffer.
len and offset are to me oddly used variable names.

> +{
> +	int i;
> +	struct htc_endpoint_stats *ep_st;
> +	u32 *counter;
> +
> +	len += scnprintf(buf + len, buf_len - len, "%s:", name);
> +	for (i = 0; i < ENDPOINT_MAX; i++) {
> +		ep_st = &target->endpoint[i].ep_st;
> +		counter = ((u32 *) ep_st) + (offset / 4);
> +		len += scnprintf(buf + len, buf_len - len, " %u", *counter);
> +	}
> +	len += scnprintf(buf + len, buf_len - len, "\n");
> +
> +	return len;
> +

perhaps:

static int endpoint_stats(struct htc_target *target,
			  char *buf, size_t len,
			  size_t pos, int index, const char *name)
{
	int i;

	pos += scnprintf(buf + pos, len - pos, "%s:", name);
	for (i = 0; i < ENDPOINT_MAX; i++) {
		u32 *stats = (u32 *)&target->endpoint[i].ep_st;
		offset += scnprintf(buf + pos, len - pos, " %u", stats[index]);
	}
	pos += scnprintf(buf + pos, len - pos, "\n");

	return pos;
}

> }
> +
> +static ssize_t ath6kl_endpoint_stats_read(struct file *file,
> +					  char __user *user_buf,
> +					  size_t count, loff_t *ppos)
> +{
> +	struct ath6kl *ar = file->private_data;
> +	struct htc_target *target = ar->htc_target;
> +	char *buf;
> +	unsigned int buf_len, len = 0;

size_t len = 0;
(or maybe pos instead)

> +	ssize_t ret_cnt;
> +
> +	buf_len = 1000 + ENDPOINT_MAX * 100;

It seems odd to start with size using 1000

There's ENDPOINT_MAX rows (9)
Each row contains:
	up to 20 byte row name
	20 counters * up to 10 bytes per counter
up to 220 bytes per row

Perhaps this?
	const size_t buf_len = ENDPOINT_MAX * (20 + sizeof(struct htc_endpoint_stats) / sizeof(u32) * 10);

> +	buf = kzalloc(buf_len, GFP_KERNEL);

kzalloc doesn't seem necessary,
everything is overwritten.  just kmalloc?

As is, it's (barely) possible to overflow
the buffer length and end up non-null terminated.
Perhaps that doesn't matter.

> +	if (!buf)
> +		return -ENOMEM;
> +
> +#define EPSTAT(name)							\
> +	len = print_endpoint_stat(target, buf, buf_len, len,		\
> +				  offsetof(struct htc_endpoint_stats, name), \
> +				  #name)

perhaps

#define EPSTAT(name)							\
	len = endpoint_stat(target, buf, buf_len, len,			\
			    offsetof(struct htc_endpoint_stats, name) / sizeof(u32), \
			    #name)

 
> +	EPSTAT(cred_low_indicate);
> +	EPSTAT(tx_issued);
> +	EPSTAT(tx_pkt_bundled);
> +	EPSTAT(tx_bundles);
> +	EPSTAT(tx_dropped);
> +	EPSTAT(tx_cred_rpt);
> +	EPSTAT(cred_rpt_from_rx);
> +	EPSTAT(cred_rpt_ep0);
> +	EPSTAT(cred_from_rx);
> +	EPSTAT(cred_from_other);
> +	EPSTAT(cred_from_ep0);
> +	EPSTAT(cred_cosumd);
> +	EPSTAT(cred_retnd);
> +	EPSTAT(rx_pkts);
> +	EPSTAT(rx_lkahds);
> +	EPSTAT(rx_bundl);
> +	EPSTAT(rx_bundle_lkahd);
> +	EPSTAT(rx_bundle_from_hdr);
> +	EPSTAT(rx_alloc_thresh_hit);
> +	EPSTAT(rxalloc_thresh_byte);
> +#undef EPSTAT
> +
> +	if (len > buf_len)
> +		len = buf_len;

Maybe it's better to ensure 0 termination?
	if (len > buf_len) {
		len = buf_len;
		buf[buf_len - 1] = 0;
	}

> +	ret_cnt = simple_read_from_buffer(user_buf, count, ppos, buf, len);
> +	kfree(buf);
> +	return ret_cnt;
> +}



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

* Re: [PATCH v2 1/5] ath6kl: Add endpoint_stats debugfs file
  2011-10-11 16:39   ` Joe Perches
@ 2011-10-11 18:52     ` Jouni Malinen
  2011-10-11 20:33       ` Joe Perches
  0 siblings, 1 reply; 10+ messages in thread
From: Jouni Malinen @ 2011-10-11 18:52 UTC (permalink / raw)
  To: Joe Perches; +Cc: kvalo, linux-wireless

On Tue, Oct 11, 2011 at 09:39:31AM -0700, Joe Perches wrote:
> > +static unsigned int print_endpoint_stat(struct htc_target *target, char *buf,
> > +					unsigned int buf_len, unsigned int len,
> > +					int offset, const char *name)
> 
> Perhaps the function name is wrong.
> This doesn't print, it fills a buffer.

Well, it prints the stuff into a buffer just like scnprintf prints
things.. Feel free to send a patch if you want to try to rename
*print* functions ;-).

> len and offset are to me oddly used variable names.

I guess len could be renamed to pos, but the use of len here matches
with the function used in the main function, i.e., this
print_endpoint_stat is more of a replacement for a macro. offset sounds
correct to me, i.e., it is the offset within the statistics structure.

> > +		counter = ((u32 *) ep_st) + (offset / 4);

> perhaps:
> 
> static int endpoint_stats(struct htc_target *target,
> 			  char *buf, size_t len,
> 			  size_t pos, int index, const char *name)

I would have nothing against pos, but index would be a bit confusing
here since that parameter is really the byte offset and not the index.

> 	pos += scnprintf(buf + pos, len - pos, "%s:", name);
> 	for (i = 0; i < ENDPOINT_MAX; i++) {
> 		u32 *stats = (u32 *)&target->endpoint[i].ep_st;
> 		offset += scnprintf(buf + pos, len - pos, " %u", stats[index]);

Well, okay, index to a u32 array.. If you feel strongly enough, please
send a separate cleanup patch since Kalle applied this already.


> > +static ssize_t ath6kl_endpoint_stats_read(struct file *file,
> > +					  char __user *user_buf,
> > +					  size_t count, loff_t *ppos)
> > +{
> > +	struct ath6kl *ar = file->private_data;
> > +	struct htc_target *target = ar->htc_target;
> > +	char *buf;
> > +	unsigned int buf_len, len = 0;
> 
> size_t len = 0;
> (or maybe pos instead)

This follows the style used throughout this file. Feel free to propose
cleanup to all the functions.

> 
> > +	ssize_t ret_cnt;
> > +
> > +	buf_len = 1000 + ENDPOINT_MAX * 100;
> 
> It seems odd to start with size using 1000

Yeah.. I think I had something as a prefix first and then just forgot to
remove that extra addition.

> There's ENDPOINT_MAX rows (9)
> Each row contains:
> 	up to 20 byte row name
> 	20 counters * up to 10 bytes per counter
> up to 220 bytes per row

It's actually the other way around, i.e., 20 rows with 9 counters.

> Perhaps this?
> 	const size_t buf_len = ENDPOINT_MAX * (20 + sizeof(struct htc_endpoint_stats) / sizeof(u32) * 10);

I was too lazy to count the exact maximum. If there is any chance of
overflowing the buf_len, this would obviously need to be fixed. The
theoretical maximum would likely be something like 20 * (22 + 9 * 11),
so yes, while I don't think this happens in practice, this buffer length
should be incremented to cover the theoretical case (and to make some
more sense).

> > +	buf = kzalloc(buf_len, GFP_KERNEL);
> 
> kzalloc doesn't seem necessary,
> everything is overwritten.  just kmalloc?

Yeah, I probably copied that from an existing function, but I would
agree that that is not really any need for clearing the buffer.

> As is, it's (barely) possible to overflow
> the buffer length and end up non-null terminated.
> Perhaps that doesn't matter.

I don't think there is any need for null-terminating the buffer since
this is really binary data with length returned. As I noted, this is
unlikely to end up having large enough counter values to overflow in
practice, but changing the buf_len calculation to make more sense is
useful cleanup anyway.

> > +	len = print_endpoint_stat(target, buf, buf_len, len,		\
> > +				  offsetof(struct htc_endpoint_stats, name), \
> > +				  #name)
> 
> perhaps
> 
> #define EPSTAT(name)							\
> 	len = endpoint_stat(target, buf, buf_len, len,			\
> 			    offsetof(struct htc_endpoint_stats, name) / sizeof(u32), \
> 			    #name)

I'm fine with this, too, as a separate cleanup patch.

> > +	if (len > buf_len)
> > +		len = buf_len;
> 
> Maybe it's better to ensure 0 termination?
> 	if (len > buf_len) {
> 		len = buf_len;
> 		buf[buf_len - 1] = 0;
> 	}

Why? This is a read file op.. It is not like the caller is expecting a
null-terminated string to come back from here.

-- 
Jouni Malinen                                            PGP id EFC895FA

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

* Re: [PATCH v2 1/5] ath6kl: Add endpoint_stats debugfs file
  2011-10-11 18:52     ` Jouni Malinen
@ 2011-10-11 20:33       ` Joe Perches
  0 siblings, 0 replies; 10+ messages in thread
From: Joe Perches @ 2011-10-11 20:33 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: kvalo, linux-wireless

On Tue, 2011-10-11 at 21:52 +0300, Jouni Malinen wrote:
> On Tue, Oct 11, 2011 at 09:39:31AM -0700, Joe Perches wrote:
> > > +static unsigned int print_endpoint_stat(struct htc_target *target, char *buf,
> > > +					unsigned int buf_len, unsigned int len,
> > > +					int offset, const char *name)
> > Perhaps the function name is wrong.
> > This doesn't print, it fills a buffer.
> Well, it prints the stuff into a buffer just like scnprintf prints
> things.. Feel free to send a patch if you want to try to rename
> *print* functions ;-).

OK, s/print/scnprint/



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

end of thread, other threads:[~2011-10-11 20:33 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-11 14:31 [PATCH v2 0/5] ath6kl: Debugging and roaming Jouni Malinen
2011-10-11 14:31 ` [PATCH v2 1/5] ath6kl: Add endpoint_stats debugfs file Jouni Malinen
2011-10-11 16:39   ` Joe Perches
2011-10-11 18:52     ` Jouni Malinen
2011-10-11 20:33       ` Joe Perches
2011-10-11 14:31 ` [PATCH v2 2/5] ath6kl: Add debugfs file for target roam table Jouni Malinen
2011-10-11 14:31 ` [PATCH v2 3/5] ath6kl: Add debugfs files for roaming control Jouni Malinen
2011-10-11 14:31 ` [PATCH v2 4/5] ath6kl: Add debugfs control for keepalive and disconnection timeout Jouni Malinen
2011-10-11 14:31 ` [PATCH v2 5/5] ath6kl: Allow CCKM AKM and KRK to be configured Jouni Malinen
2011-10-11 16:13 ` [PATCH v2 0/5] ath6kl: Debugging and roaming Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).