Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [PATCH 10/14] rtlwifi: Add BTC_TRACE_STRING to new btcoex
From: Larry Finger @ 2016-12-05 22:34 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: kvalo, devel, Ping-Ke Shih, linux-wireless
In-Reply-To: <20161205213447.GJ8176@mwanda>

On 12/05/2016 03:34 PM, Dan Carpenter wrote:
> On Thu, Dec 01, 2016 at 07:48:29PM -0600, Larry Finger wrote:
>> --- wireless-drivers-next.orig/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
>> +++ wireless-drivers-next/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
>> @@ -27,6 +27,29 @@
>>
>>  #include	"../wifi.h"
>>
>> +#ifdef CONFIG_RTLWIFI_DEBUG
>> +
>> +#define BTC_SPRINTF(ptr, ...)	snprintf(ptr, ##__VA_ARGS__)
>> +#define BTC_TRACE(fmt)						\
>> +do {								\
>> +	struct rtl_priv *rtlpriv = gl_bt_coexist.adapter;	\
>> +	if (!rtlpriv)					\
>> +		break;						\
>> +	RT_TRACE_STRING(rtlpriv, COMP_COEX, DBG_LOUD, fmt);	\
>> +} while (0)
>> +
>
> This sort of macro is exactly when the rtl drivers spent so long in
> staging...  Subsystems should not invent their own tracing but in
> particular these macros are so very very ugly.
>
> It's just super frustrating to even look at this...
>
> There are a lot of staging drivers I feel good about when they leave.
> The HyperV drivers.  The IIO stuff.  A lot of the media stuff and
> generally the media tree is getting better and better.  I like comedi
> and unisys, those are in staging, but they are great and could move out
> any time as far as I'm concerned.
>
> But this patch just makes me super discouraged.  What are we doing???

Dan,

It would not matter to me if these drivers got moved to staging, but there are a 
lot of users whose distros do not build staging drivers that would be very unhappy.

Can you point me to a driver with a better way to conditionally dump a debugging 
string to the logs?

Larry

^ permalink raw reply

* [PATCH] mac80211:  fix rx-rate report when rate is invalid.
From: greearb @ 2016-12-05 22:12 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes, Ben Greear

From: Ben Greear <greearb@candelatech.com>

This fixes obtaining the rate info via sta_set_sinfo
when the rx rate is invalid (for instance, on IBSS
interface that has received no frames from one of its
peers).

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 net/mac80211/sta_info.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 01868f9..868dc88 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -2076,10 +2076,12 @@ static void sta_set_rate_info_rx(struct sta_info *sta, struct rate_info *rinfo)
 {
 	u16 rate = ACCESS_ONCE(sta_get_last_rx_stats(sta)->last_rate);
 
-	if (rate == STA_STATS_RATE_INVALID)
-		rinfo->flags = 0;
-	else
+	if (rate == STA_STATS_RATE_INVALID) {
+		rinfo->flags = 0; /* means use legacy rates */
+		rinfo->legacy = 0; /* Initialize legacy rates to known value */
+	} else {
 		sta_stats_decode_rate(sta->local, rate, rinfo);
+	}
 }
 
 static void sta_set_tidstats(struct sta_info *sta,
-- 
2.4.11

^ permalink raw reply related

* [PATCH 2/2] ath10k: fix potential memory leak in ath10k_wmi_tlv_op_pull_fw_stats()
From: Christian Lamparter @ 2016-12-05 21:52 UTC (permalink / raw)
  To: linux-wireless, ath10k; +Cc: Kalle Valo
In-Reply-To: <992a4e2676037a06f482cdbe2d3d39e287530be5.1480974623.git.chunkeey@googlemail.com>

ath10k_wmi_tlv_op_pull_fw_stats() uses tb = ath10k_wmi_tlv_parse_alloc(...)
function, which allocates memory. If any of the three error-paths are
taken, this tb needs to be freed.

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index f304f6632c4f..1f6bb9e8bb01 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -1105,8 +1105,10 @@ static int ath10k_wmi_tlv_op_pull_fw_stats(struct ath10k *ar,
 		struct ath10k_fw_stats_pdev *dst;
 
 		src = data;
-		if (data_len < sizeof(*src))
+		if (data_len < sizeof(*src)) {
+			kfree(tb);
 			return -EPROTO;
+		}
 
 		data += sizeof(*src);
 		data_len -= sizeof(*src);
@@ -1126,8 +1128,10 @@ static int ath10k_wmi_tlv_op_pull_fw_stats(struct ath10k *ar,
 		struct ath10k_fw_stats_vdev *dst;
 
 		src = data;
-		if (data_len < sizeof(*src))
+		if (data_len < sizeof(*src)) {
+			kfree(tb);
 			return -EPROTO;
+		}
 
 		data += sizeof(*src);
 		data_len -= sizeof(*src);
@@ -1145,8 +1149,10 @@ static int ath10k_wmi_tlv_op_pull_fw_stats(struct ath10k *ar,
 		struct ath10k_fw_stats_peer *dst;
 
 		src = data;
-		if (data_len < sizeof(*src))
+		if (data_len < sizeof(*src)) {
+			kfree(tb);
 			return -EPROTO;
+		}
 
 		data += sizeof(*src);
 		data_len -= sizeof(*src);
-- 
2.11.0

^ permalink raw reply related

* [PATCH 1/2] ath10k: add accounting for the extended peer statistics
From: Christian Lamparter @ 2016-12-05 21:52 UTC (permalink / raw)
  To: linux-wireless, ath10k; +Cc: Kalle Valo

The 10.4 firmware adds extended peer information to the
firmware's statistics payload. This additional info is
stored as a separate data field and the elements are
stored in their own "peers_extd" list.

These elements can pile up in the same way as the peer
information elements. This is because the
ath10k_wmi_10_4_op_pull_fw_stats() function tries to
pull the same amount (num_peer_stats) for every statistic
data unit.

Fixes: 4a49ae94a448faa ("ath10k: fix 10.4 extended peer stats update")
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/net/wireless/ath/ath10k/debug.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 82a4c67f3672..4acd9eb65910 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -399,6 +399,7 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
 			 * prevent firmware from DoS-ing the host.
 			 */
 			ath10k_fw_stats_peers_free(&ar->debug.fw_stats.peers);
+			ath10k_fw_extd_stats_peers_free(&ar->debug.fw_stats.peers_extd);
 			ath10k_warn(ar, "dropping fw peer stats\n");
 			goto free;
 		}
@@ -409,10 +410,12 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
 			goto free;
 		}
 
+		if (!list_empty(&stats.peers))
+			list_splice_tail_init(&stats.peers_extd,
+					      &ar->debug.fw_stats.peers_extd);
+
 		list_splice_tail_init(&stats.peers, &ar->debug.fw_stats.peers);
 		list_splice_tail_init(&stats.vdevs, &ar->debug.fw_stats.vdevs);
-		list_splice_tail_init(&stats.peers_extd,
-				      &ar->debug.fw_stats.peers_extd);
 	}
 
 	complete(&ar->debug.fw_stats_complete);
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH 1/7] rtlwifi: btcoexist: Update routines for RTL8192EE
From: Dan Carpenter @ 2016-12-05 21:48 UTC (permalink / raw)
  To: Larry Finger; +Cc: devel, Ping-Ke Shih, linux-wireless, kvalo
In-Reply-To: <e4d07a23-d4d6-7ebd-7acd-69de3218a685@lwfinger.net>

On Mon, Dec 05, 2016 at 11:17:28AM -0600, Larry Finger wrote:
> On 12/05/2016 05:38 AM, Dan Carpenter wrote:
> >On Sat, Dec 03, 2016 at 11:32:01AM -0600, Larry Finger wrote:
> >>From: Ping-Ke Shih <pkshih@realtek.com>
> >>
> >>The btcoexist routines for the RTL8192EE have been extensively rewritten.
> >>
> >
> >This patch does a million things and is totally unreviewable.  The
> >patch description doesn't say what patch does or why.  It's 5k lines
> >of diff.  What the heck???
> 
> I am caught in a bind. The BT coexistence routines are written by a
> Realtek contractor. As I cannot get the individual patches from
> Realtek, there is no chance of getting them from a 3rd party.

These patches are absolutely impenetrable.  A few years ago in SCSI we
gave a vendor a one time pass to just merge a huge patch and sync with
mainline.  We could view this in the same light, except that I think
when we moved this driver out of staging that was the one time pass.  By
now they should be proper kernel devs who can wipe their own bottoms...

We maybe should merge this and move the driver back to staging until
they can figure out how to maintain stuff?  That's a serious suggestion.
This patch is no way close to minimum quality requirements for upstream.

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH 10/14] rtlwifi: Add BTC_TRACE_STRING to new btcoex
From: Dan Carpenter @ 2016-12-05 21:34 UTC (permalink / raw)
  To: Larry Finger; +Cc: kvalo, devel, Ping-Ke Shih, linux-wireless
In-Reply-To: <20161202014833.6856-11-Larry.Finger@lwfinger.net>

On Thu, Dec 01, 2016 at 07:48:29PM -0600, Larry Finger wrote:
> --- wireless-drivers-next.orig/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
> +++ wireless-drivers-next/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
> @@ -27,6 +27,29 @@
>  
>  #include	"../wifi.h"
>  
> +#ifdef CONFIG_RTLWIFI_DEBUG
> +
> +#define BTC_SPRINTF(ptr, ...)	snprintf(ptr, ##__VA_ARGS__)
> +#define BTC_TRACE(fmt)						\
> +do {								\
> +	struct rtl_priv *rtlpriv = gl_bt_coexist.adapter;	\
> +	if (!rtlpriv)					\
> +		break;						\
> +	RT_TRACE_STRING(rtlpriv, COMP_COEX, DBG_LOUD, fmt);	\
> +} while (0)
> +

This sort of macro is exactly when the rtl drivers spent so long in
staging...  Subsystems should not invent their own tracing but in
particular these macros are so very very ugly.

It's just super frustrating to even look at this...

There are a lot of staging drivers I feel good about when they leave.
The HyperV drivers.  The IIO stuff.  A lot of the media stuff and
generally the media tree is getting better and better.  I like comedi
and unisys, those are in staging, but they are great and could move out
any time as far as I'm concerned.

But this patch just makes me super discouraged.  What are we doing???

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH 05/14] rtlwifi: Add TX report and disable key will wait until report acked.
From: Dan Carpenter @ 2016-12-05 21:10 UTC (permalink / raw)
  To: Larry Finger; +Cc: kvalo, devel, Ping-Ke Shih, linux-wireless
In-Reply-To: <20161202014833.6856-6-Larry.Finger@lwfinger.net>

On Thu, Dec 01, 2016 at 07:48:24PM -0600, Larry Finger wrote:
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c
> index 2d48ccd..0f9d9f0 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c
> @@ -731,6 +731,14 @@ void rtl92ee_tx_fill_desc(struct ieee80211_hw *hw,
>  			SET_TX_DESC_OFFSET(pdesc, USB_HWDESC_HEADER_LEN);
>  		}
>  
> +		/* tx report */
> +		if (ptcb_desc->use_spe_rpt) {
> +			u16 sn = rtl_get_tx_report_sn(hw);
> +
> +			SET_TX_DESC_SPE_RPT(pdesc, 1);
> +			SET_TX_DESC_SW_DEFINE(pdesc, sn);
> +		}
> +

All the callers of rtl_get_tx_report_sn() use this same 5 line block.
Let's move it to a separate function.

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH 02/14] rtlwifi: Fix programing CAM content sequence.
From: Dan Carpenter @ 2016-12-05 20:49 UTC (permalink / raw)
  To: Larry Finger; +Cc: kvalo, devel, shaofu, Ping-Ke Shih, linux-wireless
In-Reply-To: <20161202014833.6856-3-Larry.Finger@lwfinger.net>

On Thu, Dec 01, 2016 at 07:48:21PM -0600, Larry Finger wrote:
> From: Ping-Ke Shih <pkshih@realtek.com>
> 
> There is a potential race condition when the control byte of a CAM
> entry is written first. Write in reverse order to correct the condition.
> 
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> Signed-off-by: shaofu <shaofu@realtek.com>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
>  drivers/net/wireless/realtek/rtlwifi/cam.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/cam.c b/drivers/net/wireless/realtek/rtlwifi/cam.c
> index 8fe8b4c..5d58ec0 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/cam.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/cam.c
> @@ -45,12 +45,13 @@ static void rtl_cam_program_entry(struct ieee80211_hw *hw, u32 entry_no,
>  
>  	u32 target_command;
>  	u32 target_content = 0;
> -	u8 entry_i;
> +	s8 entry_i;

Just make this an int.  s8 is for very specific things like hardware
registers.

regards,
dan carpenter

^ permalink raw reply

* wil6210: Low level RF sector API + Wrong Rx Sector Information
From: Hany Assasa @ 2016-12-05 20:26 UTC (permalink / raw)
  To: linux-wireless

Dear Maya and Lior,

1. Regarding the patch for adding new APIs for low level RF sector in
wil6210 driver, is it only intended for special firmware or it can
support the available firmware which comes with the ACER Laptop and
TP-Link AD7200 Router?

2. In the reported debug information in the beamforming file (bf), the
reported Rx sector is always constant (96) and above the allowed range
in the standard. is it because the sector sweep is limited to transmit
sector training?

Thanks.

Best Regards,
Hany Assasa

^ permalink raw reply

* RE: ath10k firmware crashes in mesh mode on QCA9880
From: Nagarajan, Ashok Raj @ 2016-12-05 19:24 UTC (permalink / raw)
  To: Benjamin Morgan, Mohammed Shafi Shajakhan
  Cc: agreen@cococorp.com, lede-dev@lists.infradead.org,
	linux-wireless@vger.kernel.org, ath10k@lists.infradead.org
In-Reply-To: <5845B325.6060002@cococorp.com>

Pj4gQXBwbGllZCB0aGUgcGF0Y2ggYW5kIHRyaWVkIHdpdGggMTAuMi40LjcwLjU0IGZpcm13YXJl
IGFuZCBpdCBzdGlsbCBjcmFzaGVzOg0KDQo+PiBbICAxNDIuNDM4Mzc3XSBhdGgxMGtfcGNpIDAw
MDA6MDE6MDAuMDogZmlybXdhcmUgY3Jhc2hlZCEgKHV1aWQgDQo+PiBhNTQ5OTU4Mi1lMjIwLTQ2
ZDItOTM1OS0wYjQ0MjE5ZjY5ZWEpDQo+PiBbICAxNDIuNDQ3NTEyXSBhdGgxMGtfcGNpIDAwMDA6
MDE6MDAuMDogcWNhOTg4eCBodzIuMCB0YXJnZXQgMHg0MTAwMDE2YyANCj4+IGNoaXBfaWQgMHgw
NDMyMDJmZiBzdWIgMDAwMDowMDAwDQo+PiBbICAxNDIuNDU2ODc5XSBhdGgxMGtfcGNpIDAwMDA6
MDE6MDAuMDoga2NvbmZpZyBkZWJ1ZyAwIGRlYnVnZnMgMSANCj4+IHRyYWNpbmcgMCBkZnMgMSB0
ZXN0bW9kZSAxDQo+PiBbICAxNDIuNDY5OTE2XSBhdGgxMGtfcGNpIDAwMDA6MDE6MDAuMDogZmly
bXdhcmUgdmVyIDEwLjIuNC43MC41NCBhcGkgNSANCmZlYXR1cmVzIG5vLXAycCxyYXctbW9kZSxt
ZnAgY3JjMzIgOWQzNDBkZDkNCj4+IFsgIDE0Mi40ODAyOTVdIGF0aDEwa19wY2kgMDAwMDowMTow
MC4wOiBib2FyZF9maWxlIGFwaSAxIGJtaV9pZCBOL0EgDQo+PiBjcmMzMiBiZWJjN2MwOA0KPj4g
WyAgMTQyLjQ4NzcxN10gYXRoMTBrX3BjaSAwMDAwOjAxOjAwLjA6IGh0dC12ZXIgMi4xIHdtaS1v
cCA1IGh0dC1vcCAyIA0KPj4gY2FsIGZpbGUgbWF4LXN0YSAxMjggcmF3IDAgaHdjcnlwdG8gMQ0K
Pj4gWyAgMTQyLjQ5OTM2MV0gYXRoMTBrX3BjaSAwMDAwOjAxOjAwLjA6IGZpcm13YXJlIHJlZ2lz
dGVyIGR1bXA6DQo+PiBbICAxNDIuNTA1MTI0XSBhdGgxMGtfcGNpIDAwMDA6MDE6MDAuMDogWzAw
XTogMHg0MTAwMDE2QyAweDAwMDAxNUIzIA0KMHgwMDlBNDU3NyAweDAwOTU1QjMxDQoNCkJlbmph
bWluLCBUaGFua3MgZm9yIHRoZSBsb2dzLg0KUXVpY2sgcXVlc3Rpb25zIHRvIGZ1cnRoZXIgZGVi
dWcgdGhlIGlzc3VlIGhlcmUsDQoNCjEuIElzIHRoaXMgaXNzdWUgc2VlbiBldmVyeSB0aW1lIHlv
dSBzdGFydCBzZW5kaW5nIGRhdGEgdHJhZmZpYz8NCjIuIElzc3VlIHNlZW4gd2l0aCBvbGRlciBm
aXJtd2FyZXM/IChGWVIsIGh0dHA6Ly9saW51eHdpcmVsZXNzLm9yZy9lbi91c2Vycy9Ecml2ZXJz
L2F0aDEway9maXJtd2FyZS8gKQ0KMy4gQ291bGQgeW91IHBsZWFzZSBzaGFyZSB0aGUgZG1lc2cg
ZnJvbSB5b3VyIGRldmljZSBhZnRlciBlbmFibGluZyBNQUMgYW5kIFdNSSBsb2dzIGluIGF0aDEw
ayBkcml2ZXINCglUbyBlbmFibGUgZGVidWcgbG9ncyBwbGVhc2Ugc2VlIGh0dHA6Ly9saW51eHdp
cmVsZXNzLm9yZy9lbi91c2Vycy9Ecml2ZXJzL2F0aDEway9kZWJ1Zy8JDQo0LiBEbyB5b3Uga25v
dyB3aGF0IGlzIHRoZSBOdW1iZXIgb2YgU3BhdGlhbCBTdHJlYW1zIHNlZW4gaW4gbWVzaCBiZWFj
b25zIGFuZCBpbiBtZXNoIGRhdGEgcGFja2V0Pw0KDQpUaGFua3MsDQpBc2hvaw0K

^ permalink raw reply

* [PATCH v2 2/2] mac80211:  Show pending txqlen in debugfs.
From: greearb @ 2016-12-05 18:58 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes, Ben Greear
In-Reply-To: <1480964310-16698-1-git-send-email-greearb@candelatech.com>

From: Ben Greear <greearb@candelatech.com>

Could be useful for debugging memory consumption issues,
and perhaps power-save as well.

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

v2:  Use more appropriate length, add comment to explain
  magic numbers.

 net/mac80211/debugfs.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index b251b2f7..b7a0493 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -162,6 +162,31 @@ static ssize_t hwflags_read(struct file *file, char __user *user_buf,
 	return rv;
 }
 
+static ssize_t misc_read(struct file *file, char __user *user_buf,
+			 size_t count, loff_t *ppos)
+{
+	struct ieee80211_local *local = file->private_data;
+	/* Max len of each line is 16 characters, plus 9 for 'pending:\n' */
+	size_t bufsz = IEEE80211_MAX_QUEUES * 16 + 9;
+	char *buf = kzalloc(bufsz, GFP_KERNEL);
+	char *pos = buf, *end = buf + bufsz - 1;
+	ssize_t rv;
+	int i;
+	int ln;
+
+	pos += scnprintf(pos, end - pos, "pending:\n");
+
+	for (i = 0; i < IEEE80211_MAX_QUEUES; i++) {
+		ln = skb_queue_len(&local->pending[i]);
+		pos += scnprintf(pos, end - pos, "[%i] %d\n",
+				 i, ln);
+	}
+
+	rv = simple_read_from_buffer(user_buf, count, ppos, buf, strlen(buf));
+	kfree(buf);
+	return rv;
+}
+
 static ssize_t queues_read(struct file *file, char __user *user_buf,
 			   size_t count, loff_t *ppos)
 {
@@ -182,6 +207,7 @@ static ssize_t queues_read(struct file *file, char __user *user_buf,
 
 DEBUGFS_READONLY_FILE_OPS(hwflags);
 DEBUGFS_READONLY_FILE_OPS(queues);
+DEBUGFS_READONLY_FILE_OPS(misc);
 
 /* statistics stuff */
 
@@ -250,6 +276,7 @@ void debugfs_hw_add(struct ieee80211_local *local)
 	DEBUGFS_ADD(total_ps_buffered);
 	DEBUGFS_ADD(wep_iv);
 	DEBUGFS_ADD(queues);
+	DEBUGFS_ADD(misc);
 #ifdef CONFIG_PM
 	DEBUGFS_ADD_MODE(reset, 0200);
 #endif
-- 
2.4.11

^ permalink raw reply related

* [PATCH v2 1/2] mac80211:  Return avg sig, rx, tx values in ethtool stats.
From: greearb @ 2016-12-05 18:58 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes, Ben Greear

From: Ben Greear <greearb@candelatech.com>

For non-station devices.  This gives at least some useful
summary in some cases (especially when we know AP has only one
station attached, for instance).

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

v2:  Remove commented out pr_err code, remove trailing i++ that had no effect.

 net/mac80211/ethtool.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/net/mac80211/ethtool.c b/net/mac80211/ethtool.c
index 4e937c1..e7df3d3 100644
--- a/net/mac80211/ethtool.c
+++ b/net/mac80211/ethtool.c
@@ -12,6 +12,25 @@
 #include "ieee80211_i.h"
 #include "sta_info.h"
 #include "driver-ops.h"
+#include <asm/div64.h>
+
+static inline __s64 mac_div(__s64 n, __u32 base)
+{
+	if (n < 0) {
+		__u64 tmp = -n;
+		do_div(tmp, base);
+		/* printk("pktgen: pg_div, n: %llu  base: %d  rv: %llu\n",
+		   n, base, tmp); */
+		return -tmp;
+	}
+	else {
+		__u64 tmp = n;
+		do_div(tmp, base);
+		/* printk("pktgen: pg_div, n: %llu  base: %d  rv: %llu\n",
+		   n, base, tmp); */
+		return tmp;
+	}
+}
 
 static int ieee80211_set_ringparam(struct net_device *dev,
 				   struct ethtool_ringparam *rp)
@@ -128,6 +147,12 @@ static void ieee80211_get_stats(struct net_device *dev,
 			data[i] = (u8)sinfo.signal_avg;
 		i++;
 	} else {
+		int amt_tx = 0;
+		int amt_rx = 0;
+		int amt_sig = 0;
+		s64 tx_accum = 0;
+		s64 rx_accum = 0;
+		s64 sig_accum = 0;
 		list_for_each_entry(sta, &local->sta_list, list) {
 			/* Make sure this station belongs to the proper dev */
 			if (sta->sdata->dev != dev)
@@ -137,6 +162,30 @@ static void ieee80211_get_stats(struct net_device *dev,
 			sta_set_sinfo(sta, &sinfo);
 			i = 0;
 			ADD_STA_STATS(sta);
+
+			i++; /* skip sta state */
+
+			if (sinfo.filled & BIT(NL80211_STA_INFO_TX_BITRATE)) {
+				tx_accum += 100000 *
+					cfg80211_calculate_bitrate(&sinfo.txrate);
+				amt_tx++;
+				data[i] = mac_div(tx_accum, amt_tx);
+			}
+			i++;
+
+			if (sinfo.filled & BIT(NL80211_STA_INFO_RX_BITRATE)) {
+				rx_accum += 100000 *
+					cfg80211_calculate_bitrate(&sinfo.rxrate);
+				amt_rx++;
+				data[i] = mac_div(rx_accum, amt_rx);
+			}
+			i++;
+
+			if (sinfo.filled & BIT(NL80211_STA_INFO_SIGNAL_AVG)) {
+				sig_accum += sinfo.signal_avg;
+				amt_sig++;
+				data[i] = (mac_div(sig_accum, amt_sig) & 0xFF);
+			}
 		}
 	}
 
-- 
2.4.11

^ permalink raw reply related

* Re: ath10k firmware crashes in mesh mode on QCA9880
From: Benjamin Morgan @ 2016-12-05 18:34 UTC (permalink / raw)
  To: Mohammed Shafi Shajakhan; +Cc: linux-wireless, ath10k, lede-dev, agreen
In-Reply-To: <20161203124600.GC31861@atheros-ThinkPad-T61>

Applied the patch and tried with 10.2.4.70.54 firmware and it still crashes:

[  142.438377] ath10k_pci 0000:01:00.0: firmware crashed! (uuid 
a5499582-e220-46d2-9359-0b44219f69ea)
[  142.447512] ath10k_pci 0000:01:00.0: qca988x hw2.0 target 0x4100016c 
chip_id 0x043202ff sub 0000:0000
[  142.456879] ath10k_pci 0000:01:00.0: kconfig debug 0 debugfs 1 
tracing 0 dfs 1 testmode 1
[  142.469916] ath10k_pci 0000:01:00.0: firmware ver 10.2.4.70.54 api 5 
features no-p2p,raw-mode,mfp crc32 9d340dd9
[  142.480295] ath10k_pci 0000:01:00.0: board_file api 1 bmi_id N/A 
crc32 bebc7c08
[  142.487717] ath10k_pci 0000:01:00.0: htt-ver 2.1 wmi-op 5 htt-op 2 
cal file max-sta 128 raw 0 hwcrypto 1
[  142.499361] ath10k_pci 0000:01:00.0: firmware register dump:
[  142.505124] ath10k_pci 0000:01:00.0: [00]: 0x4100016C 0x000015B3 
0x009A4577 0x00955B31
[  142.513157] ath10k_pci 0000:01:00.0: [04]: 0x009A4577 0x00060130 
0x00000002 0x00439E98
[  142.521203] ath10k_pci 0000:01:00.0: [08]: 0x0044110C 0x00442074 
0x00407120 0x004436CC
[  142.529246] ath10k_pci 0000:01:00.0: [12]: 0x00000009 0x00000000 
0x009A3518 0x009A3526
[  142.537285] ath10k_pci 0000:01:00.0: [16]: 0x00958080 0x009A3EA6 
0x00000000 0x00000000
[  142.545324] ath10k_pci 0000:01:00.0: [20]: 0x409A4577 0x0040AAC4 
0x0040AC60 0x0040AC09
[  142.553356] ath10k_pci 0000:01:00.0: [24]: 0x809A44BA 0x0040AB24 
0x00400000 0xC09A4577
[  142.561400] ath10k_pci 0000:01:00.0: [28]: 0x809A39DE 0x0040AB84 
0x0044110C 0x00442074
[  142.569444] ath10k_pci 0000:01:00.0: [32]: 0x809A5FE2 0x0040ABB4 
0x0044110C 0x00407120
[  142.577483] ath10k_pci 0000:01:00.0: [36]: 0x809A2E6C 0x0040ABF4 
0x0040AC14 0x00001580
[  142.585522] ath10k_pci 0000:01:00.0: [40]: 0x80990F6F 0x0040AD04 
0x009C643C 0x004436CC
[  142.593554] ath10k_pci 0000:01:00.0: [44]: 0x80998510 0x0040AD64 
0x004208FC 0x00439E4C
[  142.601600] ath10k_pci 0000:01:00.0: [48]: 0x8099AE95 0x0040AD84 
0x004208FC 0x0042638C
[  142.609642] ath10k_pci 0000:01:00.0: [52]: 0x809BFC55 0x0040AEE4 
0x00424FE8 0x00000002
[  142.617681] ath10k_pci 0000:01:00.0: [56]: 0x80940F18 0x0040AF14 
0x00000004 0x004039D0
[  142.727220] ieee80211 phy0: Hardware restart was requested
[  142.732850] ath10k_pci 0000:01:00.0: failed to synchronize monitor 
vdev 1 stop: -143
[  142.740739] ath10k_pci 0000:01:00.0: failed to stop monitor vdev: -143


~Benjamin

On 12/03/2016 04:46 AM, Mohammed Shafi Shajakhan wrote:
> https://patchwork.kernel.org/patch/9437519/
> (sorry missed this in the previous thread)
>
> On Sat, Dec 03, 2016 at 06:13:58PM +0530, Mohammed Shafi Shajakhan wrote:
>> Hi Benjamin,
>>
>> On Fri, Dec 02, 2016 at 05:28:02PM -0800, Benjamin Morgan wrote:
>>> Just tried 10.2.4.70.58 firmware that you linked to and it still crashes:
>>>
>>> [  131.568989] ath10k_pci 0000:01:00.0: firmware crashed! (uuid
>>> 1838347e-9380-4a26-ac9d-2963ee95968b)
>>> [  131.578124] ath10k_pci 0000:01:00.0: qca988x hw2.0 target
>>> 0x4100016c chip_id 0x043202ff sub 0000:0000
>>> [  131.587491] ath10k_pci 0000:01:00.0: kconfig debug 0 debugfs 1
>>> tracing 0 dfs 1 testmode 1
>>> [  131.600521] ath10k_pci 0000:01:00.0: firmware ver 10.2.4.70.58
>>> api 5 features no-p2p,raw-mode,mfp crc32 e1af076f
>>> [  131.610899] ath10k_pci 0000:01:00.0: board_file api 1 bmi_id N/A
>>> crc32 bebc7c08
>>> [  131.618325] ath10k_pci 0000:01:00.0: htt-ver 2.1 wmi-op 5 htt-op
>>> 2 cal file max-sta 128 raw 0 hwcrypto 1
>>> [  131.629965] ath10k_pci 0000:01:00.0: firmware register dump:
>>> [  131.635728] ath10k_pci 0000:01:00.0: [00]: 0x4100016C 0x000015B3
>>> 0x009A45AF 0x00955B31
>>> [  131.643761] ath10k_pci 0000:01:00.0: [04]: 0x009A45AF 0x00060130
>>> 0x00000002 0x00439E98
>>> [  131.651806] ath10k_pci 0000:01:00.0: [08]: 0x0044110C 0x00442074
>>> 0x00407120 0x004436CC
>>> [  131.659852] ath10k_pci 0000:01:00.0: [12]: 0x00000009 0x00000000
>>> 0x009A3550 0x009A355E
>>> [  131.667892] ath10k_pci 0000:01:00.0: [16]: 0x00958080 0x009A31D6
>>> 0x00000000 0x00000000
>>> [  131.675936] ath10k_pci 0000:01:00.0: [20]: 0x409A45AF 0x0040AAC4
>>> 0x0040AC60 0x0040AC09
>>> [  131.683968] ath10k_pci 0000:01:00.0: [24]: 0x809A44F2 0x0040AB24
>>> 0x00400000 0xC09A45AF
>>> [  131.692013] ath10k_pci 0000:01:00.0: [28]: 0x809A3A16 0x0040AB84
>>> 0x0044110C 0x00442074
>>> [  131.700056] ath10k_pci 0000:01:00.0: [32]: 0x809A601A 0x0040ABB4
>>> 0x0044110C 0x00407120
>>> [  131.708100] ath10k_pci 0000:01:00.0: [36]: 0x809A2EA4 0x0040ABF4
>>> 0x0040AC14 0x00001580
>>> [  131.716143] ath10k_pci 0000:01:00.0: [40]: 0x80990F63 0x0040AD04
>>> 0x009C6458 0x004436CC
>>> [  131.724175] ath10k_pci 0000:01:00.0: [44]: 0x80998520 0x0040AD64
>>> 0x004208FC 0x00439E4C
>>> [  131.732220] ath10k_pci 0000:01:00.0: [48]: 0x8099AEA5 0x0040AD84
>>> 0x004208FC 0x00425874
>>> [  131.740263] ath10k_pci 0000:01:00.0: [52]: 0x809BFC39 0x0040AEE4
>>> 0x00424FE8 0x00000002
>>> [  131.748306] ath10k_pci 0000:01:00.0: [56]: 0x80940F18 0x0040AF14
>>> 0x00000004 0x004039D0
>>> [  131.857076] ieee80211 phy0: Hardware restart was requested
>>> [  131.862705] ath10k_pci 0000:01:00.0: failed to synchronize
>>> monitor vdev 1 stop: -143
>>> [  131.870594] ath10k_pci 0000:01:00.0: failed to stop monitor vdev: -143
>> [shafi] request to try with the latest backports and ath.git
>> Also worth trying this fix in ath10k (once the next firmware version)
>> is available. I will keep you posted. thank you !
>>
>>> ~Benjamin
>>>
>>> On 11/30/2016 11:09 PM, Mohammed Shafi Shajakhan wrote:
>>>> Hi,
>>>>
>>>> On Tue, Nov 29, 2016 at 11:22:12AM -0800, Benjamin Morgan wrote:
>>>>> When we try to transmit traffic (ping) between two meshed ath10k
>>>>> devices running latest lede we keep experiencing ath10k firmware
>>>>> crashes. This seems to only happen when running in 802.11n/ac mode
>>>>> but not in 802.11a/g mode. Also, from the station dumps it appears
>>>>> that management traffic is flowing between the devices, however when
>>>>> we try to send unicast data traffic the firmware crashes
>>>>> immediately.
>>>> [shafi] Did you get a chance to try with the below firmware as well
>>>> https://github.com/kvalo/ath10k-firmware/blob/master/QCA988X/hw2.0/10.2.4.70/firmware-5.bin_10.2.4.70.58
>>>>
>>>>> Platform: Archer C7 AC1750 v2
>>>>> Software Image: LEDE (HEAD, r2299) Commit: https://github.com/lede-project/source/commit/d596c21ebd5a3e6ce933eff3e51989031e4b1d58
>>>>>
>>>>> Crypto: wpa_supplicant
>>>>> wpa_supplicant-wlan0.conf
>>>>> network={
>>>>> ssid="bmorgan_lede_mesh"
>>>>> key_mgmt=SAE
>>>>> mode=5
>>>>> frequency=5180
>>>>> psk="meshpassword"
>>>>> }
>>>>>
>>>>> Backports Verstion:
>>>>> [    9.818007] Loading modules backported from Linux version
>>>>> wt-2016-10-03-1-g6fcb1a6
>>>>> [    9.825736] Backport generated by backports.git
>>>>> backports-20160324-9-g0e38f5c
>>>>>
>>>>> ​​Ath10k Initialization on Station A (dmesg)
>>>>> [    9.896715] PCI: Enabling device 0000:01:00.0 (0000 -> 0002)
>>>>> [    9.902622] ath10k_pci 0000:01:00.0: pci irq legacy oper_irq_mode
>>>>> 1 irq_mode 0 reset_mode 0
>>>>> [   10.123734] ath10k_pci 0000:01:00.0: Direct firmware load for
>>>>> ath10k/pre-cal-pci-0000:01:00.0.bin failed with error -2
>>>>> [   10.134620] ath10k_pci 0000:01:00.0: Falling back to user helper
>>>>> [   10.287680] firmware ath10k!pre-cal-pci-0000:01:00.0.bin:
>>>>> firmware_loading_store: map pages failed
>>>>> [   10.622789] ath10k_pci 0000:01:00.0: qca988x hw2.0 target
>>>>> 0x4100016c chip_id 0x043202ff sub 0000:0000
>>>>> [   10.632184] ath10k_pci 0000:01:00.0: kconfig debug 0 debugfs 1
>>>>> tracing 0 dfs 1 testmode 1
>>>>> [   10.645231] ath10k_pci 0000:01:00.0: firmware ver 10.2.4.70.54
>>>>> api 5 features no-p2p,raw-mode,mfp crc32 9d340dd9
>>>>> [   10.655660] ath10k_pci 0000:01:00.0: Direct firmware load for
>>>>> ath10k/QCA988X/hw2.0/board-2.bin failed with error -2
>>>>> [   10.666264] ath10k_pci 0000:01:00.0: Falling back to user helper
>>>>> [   10.747925] firmware ath10k!QCA988X!hw2.0!board-2.bin:
>>>>> firmware_loading_store: map pages failed
>>>>> [   11.011123] ath10k_pci 0000:01:00.0: board_file api 1 bmi_id N/A
>>>>> crc32 bebc7c08
>>>>> [   12.155224] ath10k_pci 0000:01:00.0: htt-ver 2.1 wmi-op 5 htt-op
>>>>> 2 cal file max-sta 128 raw 0 hwcrypto 1
>>>>>
>>>>> Station A (wlan0):
>>>>> 18:A6:F7:23:6E:66
>>>>> 10.230.5.41
>>>>>
>>>>> Station B (wlan0):
>>>>> 18:a6:f7:26:0f:21
>>>>> 10.230.5.42
>>>>>
>>>>> Station Dump on Station A before ping:
>>>>> Station 18:a6:f7:26:0f:21 (on wlan0)
>>>>> inactive time:340 ms
>>>>> rx bytes:2472
>>>>> rx packets:28
>>>>> tx bytes:1204
>>>>> tx packets:9
>>>>> tx retries:0
>>>>> tx failed:0
>>>>> rx drop misc:1
>>>>> signal: -14 dBm
>>>>> signal avg:-14 dBm
>>>>> Toffset:18142530 us
>>>>> tx bitrate:6.0 MBit/s
>>>>> rx bitrate:6.0 MBit/s
>>>>> rx duration:1524 us
>>>>> mesh llid:0
>>>>> mesh plid:0
>>>>> mesh plink:ESTAB
>>>>> mesh local PS mode:ACTIVE
>>>>> mesh peer PS mode:UNKNOWN
>>>>> mesh non-peer PS mode:ACTIVE
>>>>> authorized:yes
>>>>> authenticated:yes
>>>>> associated:yes
>>>>> preamble:long
>>>>> WMM/WME:yes
>>>>> MFP:yes
>>>>> TDLS peer:no
>>>>> DTIM period:2
>>>>> beacon interval:1000
>>>>> connected time:10 seconds
>>>>>
>>>>> ​Crash Log on Station B (10.230.5.42)
>>>>> [245.483888] ath10k_pci 0000:01:00.0: firmware crashed! (uuid
>>>>> 2bab5ee9-08ff-4a17-95b1-636d212acebc)
>>>>> [245.493020] ath10k_pci 0000:01:00.0: qca988x hw2.0 target
>>>>> 0x4100016c chip_id 0x043202ff sub 0000:0000
>>>>> [245.502384] ath10k_pci 0000:01:00.0: kconfig debug 0 debugfs 1
>>>>> tracing 0 dfs 1 testmode 1
>>>>> [245.515436] ath10k_pci 0000:01:00.0: firmware ver 10.2.4.70.54 api
>>>>> 5 features no-p2p,raw-mode,mfp crc32 9d340dd9
>>>>> [245.525812] ath10k_pci 0000:01:00.0: board_file api 1 bmi_id N/A
>>>>> crc32 bebc7c08
>>>>> [245.533232] ath10k_pci 0000:01:00.0: htt-ver 2.1 wmi-op 5 htt-op 2
>>>>> cal file max-sta 128 raw 0 hwcrypto 1
>>>>> [245.544876] ath10k_pci 0000:01:00.0: firmware register dump:
>>>>> [245.550633] ath10k_pci 0000:01:00.0: [00]: 0x4100016C 0x000015B3
>>>>> 0x009A4577 0x00955B31
>>>>> [245.558676] ath10k_pci 0000:01:00.0: [04]: 0x009A4577 0x00060130
>>>>> 0x00000002 0x00439E98
>>>>> [245.566715] ath10k_pci 0000:01:00.0: [08]: 0x0044110C 0x00442074
>>>>> 0x00407120 0x004436CC
>>>>> [245.574749] ath10k_pci 0000:01:00.0: [12]: 0x00000009 0x00000000
>>>>> 0x009A3518 0x009A3526
>>>>> [245.582793] ath10k_pci 0000:01:00.0: [16]: 0x00958080 0x0094085D
>>>>> 0x00000000 0x00000000
>>>>> [245.590836] ath10k_pci 0000:01:00.0: [20]: 0x409A4577 0x0040AAC4
>>>>> 0x0040AC60 0x0040AC09
>>>>> [245.598882] ath10k_pci 0000:01:00.0: [24]: 0x809A44BA 0x0040AB24
>>>>> 0x00400000 0xC09A4577
>>>>> [245.606923] ath10k_pci 0000:01:00.0: [28]: 0x809A39DE 0x0040AB84
>>>>> 0x0044110C 0x00442074
>>>>> [245.614955] ath10k_pci 0000:01:00.0: [32]: 0x809A5FE2 0x0040ABB4
>>>>> 0x0044110C 0x00407120
>>>>> [245.623000] ath10k_pci 0000:01:00.0: [36]: 0x809A2E6C 0x0040ABF4
>>>>> 0x0040AC14 0x00001580
>>>>> [245.631043] ath10k_pci 0000:01:00.0: [40]: 0x80990F6F 0x0040AD04
>>>>> 0x009C643C 0x004436CC
>>>>> [245.639086] ath10k_pci 0000:01:00.0: [44]: 0x80998510 0x0040AD64
>>>>> 0x004208FC 0x00439E4C
>>>>> [245.647129] ath10k_pci 0000:01:00.0: [48]: 0x8099AE95 0x0040AD84
>>>>> 0x004208FC 0x00425404
>>>>> [245.655170] ath10k_pci 0000:01:00.0: [52]: 0x809BFC55 0x0040AEE4
>>>>> 0x00424FE8 0x00000002
>>>>> [245.663198] ath10k_pci 0000:01:00.0: [56]: 0x80940F18 0x0040AF14
>>>>> 0x00000004 0x004039D0
>>>>> [245.767023] ieee80211 phy0: Hardware restart was requested
>>>>> [245.772655] ath10k_pci 0000:01:00.0: failed to synchronize monitor
>>>>> vdev 1 stop: -143
>>>>> [245.780542] ath10k_pci 0000:01:00.0: failed to stop monitor vdev: -143
>>>>> ​
>>>>> With wpa_supplicant turned off:
>>>>>
>>>>> Station Dump on Station A before ping:
>>>>> Station 18:a6:f7:26:0f:21 (on wlan0)
>>>>> inactive time:60 ms
>>>>> rx bytes:3874
>>>>> rx packets:67
>>>>> tx bytes:707
>>>>> tx packets:7
>>>>> tx retries:0
>>>>> tx failed:1
>>>>> rx drop misc:3
>>>>> signal: -14 dBm
>>>>> signal avg:-14 dBm
>>>>> Toffset:18446744073684250999 us
>>>>> tx bitrate:6.0 MBit/s
>>>>> rx bitrate:6.0 MBit/s
>>>>> rx duration:1312 us
>>>>> mesh llid:24953
>>>>> mesh plid:59412
>>>>> mesh plink:ESTAB
>>>>> mesh local PS mode:ACTIVE
>>>>> mesh peer PS mode:UNKNOWN
>>>>> mesh non-peer PS mode:ACTIVE
>>>>> authorized:yes
>>>>> authenticated:yes
>>>>> associated:yes
>>>>> preamble:long
>>>>> WMM/WME:yes
>>>>> MFP:no
>>>>> TDLS peer:no
>>>>> DTIM period:2
>>>>> beacon interval:1000
>>>>> short slot time:yes
>>>>> connected time:33 seconds
>>>>>
>>>>> Firmware crash on Station A (10.230.5.41)
>>>>> [ 1040.984599] ath10k_pci 0000:01:00.0: firmware crashed! (uuid
>>>>> 42d4f6dc-0e89-4505-aefb-58cbb70d0dce)
>>>>> [ 1040.993732] ath10k_pci 0000:01:00.0: qca988x hw2.0 target
>>>>> 0x4100016c chip_id 0x043202ff sub 0000:0000
>>>>> [ 1041.003100] ath10k_pci 0000:01:00.0: kconfig debug 0 debugfs 1
>>>>> tracing 0 dfs 1 testmode 1
>>>>> [ 1041.016144] ath10k_pci 0000:01:00.0: firmware ver 10.2.4.70.54
>>>>> api 5 features no-p2p,raw-mode,mfp crc32 9d340dd9
>>>>> [ 1041.026523] ath10k_pci 0000:01:00.0: board_file api 1 bmi_id N/A
>>>>> crc32 bebc7c08
>>>>> [ 1041.033940] ath10k_pci 0000:01:00.0: htt-ver 2.1 wmi-op 5 htt-op
>>>>> 2 cal file max-sta 128 raw 0 hwcrypto 1
>>>>> [ 1041.045593] ath10k_pci 0000:01:00.0: firmware register dump:
>>>>> [ 1041.051336] ath10k_pci 0000:01:00.0: [00]: 0x4100016C 0x000015B3
>>>>> 0x009A4577 0x00955B31
>>>>> [ 1041.059383] ath10k_pci 0000:01:00.0: [04]: 0x009A4577 0x00060130
>>>>> 0x00000001 0x00435354
>>>>> [ 1041.067424] ath10k_pci 0000:01:00.0: [08]: 0x0044110C 0x00442074
>>>>> 0x00407120 0x004436CC
>>>>> [ 1041.075463] ath10k_pci 0000:01:00.0: [12]: 0x00000009 0x00000000
>>>>> 0x009A43A0 0x009A43DE
>>>>> [ 1041.083490] ath10k_pci 0000:01:00.0: [16]: 0x00958080 0x0094085D
>>>>> 0x00000000 0x00000000
>>>>> [ 1041.091535] ath10k_pci 0000:01:00.0: [20]: 0x409A4577 0x0040AAC4
>>>>> 0x0040AC60 0x0040AC09
>>>>> [ 1041.099579] ath10k_pci 0000:01:00.0: [24]: 0x809A44BA 0x0040AB24
>>>>> 0x00955A00 0xC09A4577
>>>>> [ 1041.107622] ath10k_pci 0000:01:00.0: [28]: 0x809A39DE 0x0040AB84
>>>>> 0x0044110C 0x00442074
>>>>> [ 1041.115661] ath10k_pci 0000:01:00.0: [32]: 0x809A5FE2 0x0040ABB4
>>>>> 0x0044110C 0x00407120
>>>>> [ 1041.123689] ath10k_pci 0000:01:00.0: [36]: 0x809A2E6C 0x0040ABF4
>>>>> 0x0040AC10 0x00001580
>>>>> [ 1041.131733] ath10k_pci 0000:01:00.0: [40]: 0x80990F6F 0x0040AD04
>>>>> 0x009C643C 0x004436CC
>>>>> [ 1041.139777] ath10k_pci 0000:01:00.0: [44]: 0x80998510 0x0040AD64
>>>>> 0x004208FC 0x00439E4C
>>>>> [ 1041.147820] ath10k_pci 0000:01:00.0: [48]: 0x8099AE95 0x0040AD84
>>>>> 0x004208FC 0x00425758
>>>>> [ 1041.155860] ath10k_pci 0000:01:00.0: [52]: 0x809BFC55 0x0040AEE4
>>>>> 0x00424FE8 0x00000002
>>>>> [ 1041.163888] ath10k_pci 0000:01:00.0: [56]: 0x80940F18 0x0040AF14
>>>>> 0x00000004 0x004039D0
>>>>> [ 1041.267025] ieee80211 phy0: Hardware restart was requested
>>>>> [ 1041.272656] ath10k_pci 0000:01:00.0: failed to synchronize
>>>>> monitor vdev 1 stop: -143
>>>>> [ 1041.280545] ath10k_pci 0000:01:00.0: failed to stop monitor vdev: -143
>>>>>
>>>>> ​
>>>>> Thanks in advance for your time and help,
>>>>>
>>>>> ~Benjamin
>>>>>

^ permalink raw reply

* Re: [PATCH] RFC: Universal scan proposal
From: Dmitry Shmidt @ 2016-12-05 18:32 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1480948100.31788.15.camel@sipsolutions.net>

Hi Johannes,

On Mon, Dec 5, 2016 at 6:28 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> Hi Dmitry,
>
> Sorry I didn't respond earlier.
>
>>    Currently we have sched scan with possibility of various
>> intervals. We would like to extend it to support also
>> different types of scan.
>
> "Different types of scan" is a bit misleading though, isn't it? I mean,
> mostly they differ in the reporting modes - the scanning itself still
> happens at "various intervals"?
>
>>    In case of powerful wlan CPU, all this functionality
>> can be offloaded.
>>    In general case FW processes additional scan requests
>> and puts them into queue based on start time and interval.
>> Once current request is fulfilled, FW adds it (if interval != 0)
>> again to the queue with proper interval. If requests are
>> overlapping, new request can be combined with either one before,
>> or one after, assuming that requests are not mutually exclusive.
>>    Combining requests is done by combining scan channels, ssids,
>> bssids and types of scan result. Once combined request was fulfilled
>> it will be reinserted as two (or three) different requests based on
>> their type and interval.
>>    Each request has attribute:
>> Type: connectivity / location
>
> I'm not super happy with this - after all, in theory nothing precludes
> using the results for one thing or the other, it's just about when and
> how they are gathered, no?

Indeed, results are results. I just want to take care of two things:
1) Memory consumption - we can clear stale scan results for connection,
but not for location if we are using history scan.
2) Use of insufficient results for connection - in case we had history
or hotlist scan only for very limited amount of channels, then we may not
have enough APs in our result for "sterling" connection decision.

> You do have a point wrt. an incomplete scan triggering something wrt.
> roaming or so in the connection manager, but I think that if it finds a
> better result there than the current connection it would make sense to
> pick it - even without full information.
>
> So ultimately, I think this might boil down to reporting the scan
> finished more accurately/precisely, rather than saying the type of
> scan?

This is intertwined with Luca's and yours point below - to use
scan by name or by description of the actions. Indeed maybe
this way is more generic and more useful.

>> Report: none / batch / immediate
>
> Not sure I see much point in "none"??
>
> Can you define these more clearly? Do you think "batch" reporting
> should be like the gscan buckets? Or actually have full information?

None - means that there is not need to report. It can be useful
in case of roaming scan, scheduling or hotlist scan - you didn't find
anything suitable - don't report that there is no scan results.
Batch - means to report only when buffer is full (or close to full) -
mostly for history scan or for example for case to report all scan
results together.

Immediate - is kind of self-explaining.

>>    Request may have priority and can be inserted into
>> the head of the queue.
>>    Types of scans:
>> - Normal scan
>> - Scheduled scan
>> - Hotlist (BSSID scan)
>> - Roaming
>> - AutoJoin
>
> I think somebody else said this but I didn't find it now - I think this
> would make more sense to define in terms of expected behaviour than use
> cases for each type of scan.

I think Luca made this statement. It is totally ok from SW point of
view - especially due to the fact that scan is scan. However,
I suspect it will be harder to handle from user experience. I mean
at the end wireless framework / driver / FW will convert special
scan type to usual scan with special params and response, but why
to put this burden on user?
Anyway, I am ok with this approach as well. If we see that it is
confusing we can use scan wrappers.

> johannes
>

^ permalink raw reply

* [PATCH] ath10k:  free host-mem with DMA_BIRECTIONAL flag.
From: greearb @ 2016-12-05 18:28 UTC (permalink / raw)
  To: linux-wireless; +Cc: ath10k, Ben Greear

From: Ben Greear <greearb@candelatech.com>

Hopefully this fixes the problem reported by Kalle:

Noticed this in my log, but I don't have time to investigate this in
detail right now:

[  413.795346] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
[  414.158755] IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
[  477.439659] ath10k_pci 0000:02:00.0: could not get mac80211 beacon
[  481.666630] ------------[ cut here ]------------
[  481.666669] WARNING: CPU: 0 PID: 1978 at lib/dma-debug.c:1155 check_unmap+0x320/0x8e0
[  481.666688] ath10k_pci 0000:02:00.0: DMA-API: device driver frees DMA memory with different direction [device address=0x000000002d130000] [size=63800 bytes] [mapped with DMA_BIDIRECTIONAL] [unmapped with DMA_TO_DEVICE]
[  481.666703] Modules linked in: ctr ccm ath10k_pci(E-) ath10k_core(E) ath(E) mac80211(E) cfg80211(E) snd_hda_codec_hdmi snd_hda_codec_idt snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_pcm snd_seq_midi arc4 snd_rawmidi snd_seq_midi_event snd_seq btusb btintel snd_seq_device joydev coret
[  481.671468] CPU: 0 PID: 1978 Comm: rmmod Tainted: G            E   4.9.0-rc7-wt+ #54
[  481.671478] Hardware name: Hewlett-Packard HP ProBook 6540b/1722, BIOS 68CDD Ver. F.04 01/27/2010
[  481.671489]  ef49dcec c842ee92 c8b5830e ef49dd34 ef49dd20 c80850f5 c8b5a13c ef49dd50
[  481.671560]  000007ba c8b5830e 00000483 c8461830 c8461830 00000483 ef49ddcc f34e64b8
[  481.671641]  c8b58360 ef49dd3c c80851bb 00000009 00000000 ef49dd34 c8b5a13c ef49dd50
[  481.671716] Call Trace:
[  481.671731]  [<c842ee92>] dump_stack+0x76/0xb4
[  481.671745]  [<c80850f5>] __warn+0xe5/0x100
[  481.671757]  [<c8461830>] ? check_unmap+0x320/0x8e0
[  481.671769]  [<c8461830>] ? check_unmap+0x320/0x8e0
[  481.671780]  [<c80851bb>] warn_slowpath_fmt+0x3b/0x40
[  481.671791]  [<c8461830>] check_unmap+0x320/0x8e0
[  481.671804]  [<c8462054>] debug_dma_unmap_page+0x84/0xa0
[  481.671835]  [<f937cd7a>] ath10k_wmi_free_host_mem+0x9a/0xe0 [ath10k_core]
[  481.671861]  [<f9363400>] ath10k_core_destroy+0x50/0x60 [ath10k_core]
[  481.671875]  [<f8e13969>] ath10k_pci_remove+0x79/0xa0 [ath10k_pci]
[  481.671889]  [<c848d8d8>] pci_device_remove+0x38/0xb0
[  481.671901]  [<c859fe4b>] __device_release_driver+0x7b/0x110
[  481.671913]  [<c85a00e7>] driver_detach+0x97/0xa0
[  481.671923]  [<c859ef8b>] bus_remove_driver+0x4b/0xb0
[  481.671934]  [<c85a0cda>] driver_unregister+0x2a/0x60
[  481.671949]  [<c848c888>] pci_unregister_driver+0x18/0x70
[  481.671965]  [<f8e14dae>] ath10k_pci_exit+0xd/0x25f [ath10k_pci]
[  481.671979]  [<c812bb84>] SyS_delete_module+0xf4/0x180
[  481.671995]  [<c81f801b>] ? __might_fault+0x8b/0xa0
[  481.672009]  [<c80037d0>] do_fast_syscall_32+0xa0/0x1e0
[  481.672025]  [<c88d4c88>] sysenter_past_esp+0x45/0x74
[  481.672037] ---[ end trace 3fd23759e17e1622 ]---
[  481.672049] Mapped at:
[  481.672060]  [  481.672072] [<c846062c>] debug_dma_map_page.part.25+0x1c/0xf0
[  481.672083]  [  481.672095] [<c8460799>] debug_dma_map_page+0x99/0xc0
[  481.672106]  [  481.672132] [<f93745ec>] ath10k_wmi_alloc_chunk+0x12c/0x1f0 [ath10k_core]
[  481.672142]  [  481.672168] [<f937d0c4>] ath10k_wmi_event_service_ready_work+0x304/0x540 [ath10k_core]
[  481.672178]  [  481.672190] [<c80a3643>] process_one_work+0x1c3/0x670
[  482.137134] ath10k_pci 0000:02:00.0: pci irq msi oper_irq_mode 2 irq_mode 0 reset_mode 0
[  482.313144] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/pre-cal-pci-0000:02:00.0.bin failed with error -2
[  482.313274] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/cal-pci-0000:02:00.0.bin failed with error -2
[  482.313768] ath10k_pci 0000:02:00.0: qca988x hw2.0 target 0x4100016c chip_id 0x043202ff sub 0000:0000
[  482.313777] ath10k_pci 0000:02:00.0: kconfig debug 1 debugfs 1 tracing 1 dfs 0 testmode 1
[  482.313974] ath10k_pci 0000:02:00.0: firmware ver 10.2.4.70.59-2 api 5 features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 4159f498
[  482.369858] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/QCA988X/hw2.0/board-2.bin failed with error -2
[  482.370011] ath10k_pci 0000:02:00.0: board_file api 1 bmi_id N/A crc32 bebc7c08
[  483.596770] ath10k_pci 0000:02:00.0: htt-ver 2.1 wmi-op 5 htt-op 2 cal otp max-sta 128 raw 0 hwcrypto 1
[  483.701686] ath: EEPROM regdomain: 0x0
[  483.701706] ath: EEPROM indicates default country code should be used
[  483.701713] ath: doing EEPROM country->regdmn map search
[  483.701721] ath: country maps to regdmn code: 0x3a
[  483.701730] ath: Country alpha2 being used: US
[  483.701737] ath: Regpair used: 0x3a

Reported-by: Kalle Valo <kvalo@qca.qualcomm.com>
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/ath/ath10k/wmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index c748923..0be76e4 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -8473,7 +8473,7 @@ void ath10k_wmi_free_host_mem(struct ath10k *ar)
 		dma_unmap_single(ar->dev,
 				 ar->wmi.mem_chunks[i].paddr,
 				 ar->wmi.mem_chunks[i].len,
-				 DMA_TO_DEVICE);
+				 DMA_BIDIRECTIONAL);
 		kfree(ar->wmi.mem_chunks[i].vaddr);
 	}
 
-- 
2.4.11

^ permalink raw reply related

* Re: [PATCH 1/3] ath10k: Ensure there are no stale ar->txqs entries.
From: Ben Greear @ 2016-12-05 18:19 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k@lists.infradead.org
In-Reply-To: <CA+BoTQnuLbVzuEVvf9+wOGhj7i3DV2LaMqGBfGhQHEgOSXei4g@mail.gmail.com>

On 12/05/2016 12:50 AM, Michal Kazior wrote:
> On 2 December 2016 at 01:24, Ben Greear <greearb@candelatech.com> wrote:
>> On 12/01/2016 02:52 PM, Ben Greear wrote:
>>>
>>> On 08/19/2016 06:34 AM, Ben Greear wrote:
>>>>
>>>>
>>>>
>>>> On 08/18/2016 11:59 PM, Michal Kazior wrote:
>>>>>
>>>>> On 19 August 2016 at 03:26,  <greearb@candelatech.com> wrote:
>>>>>>
>>>>>> From: Ben Greear <greearb@candelatech.com>
>>>>>>
>>>>>> I was seeing kernel crashes due to accessing freed memory
>>>>>> while debugging a 9984 firmware that was crashing often.
>>>>>>
>>>>>> This patch fixes the crashes.  I am not certain if there
>>>>>> is a better way or not.
>>
>>
>> Michal, thanks for the help on IRC.  I added this logic:
>>
>> static void ieee80211_drv_tx(struct ieee80211_local *local,
>>                              struct ieee80211_vif *vif,
>>                              struct ieee80211_sta *pubsta,
>>                              struct sk_buff *skb)
>> {
>>         struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
>>         struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
>>         struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
>>         struct ieee80211_tx_control control = {
>>                 .sta = pubsta,
>>         };
>>         struct ieee80211_txq *txq = NULL;
>>         struct txq_info *txqi;
>>         u8 ac;
>>
>>         if ((info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) ||
>>             (info->control.flags & IEEE80211_TX_CTRL_PS_RESPONSE))
>>                 goto tx_normal;
>>
>>         if (!ieee80211_is_data(hdr->frame_control))
>>                 goto tx_normal;
>>
>>         if (unlikely(!ieee80211_sdata_running(sdata))) {
>>                 WARN_ON_ONCE(1);
>>                 goto delete_and_return;
>>         }
>>
>> ...
>>
>>         if (atomic_read(&sdata->txqs_len[ac]) >=
>>             (local->hw.txq_ac_max_pending * 2)) {
>>                 /* Must be that something is not paying attention to
>>                  * max-pending, like pktgen, so just drop this frame.
>>                  */
>> delete_and_return:
>>                 ieee80211_free_txskb(&local->hw, skb);
>>                 return;
>>         }
>>
>>
>> But, I still see the txq entries on the ar->txqs list in the
>> ath10k_mac_txq_init
>> after firmware crash in my test case.  Is this the test you were suggesting?
>
> Yes.
>
> Now that I think about it mac80211 doesn't call anything in driver
> during hw_restart that would unref txqs. This means you'll have them
> still linked when add_interface/sta_state is called, no?
>
> This means that either:
>  (a) txq (re-)init should be smarter in ath10k
>  (b) txqs should be purged during hw_restart in ath10k

I posted a patch that does (a) last Friday:

"ath10k:  work-around for stale txq in ar->txqs"

I realized it will not apply upstream because it is also patching the previous
work-around I had in the patch that originated this email thread.

With these patches and the iterate hack to mac80211, then I no longer
see crashes in my test case that previously crashed very readily.

Thanks,
Ben


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

^ permalink raw reply

* Re: [PATCH] mac80211: Return avg sig, rx, tx values in ethtool stats.
From: Ben Greear @ 2016-12-05 18:05 UTC (permalink / raw)
  To: Felix Fietkau, Johannes Berg, linux-wireless
In-Reply-To: <ca0c6cee-6a7c-509c-3d88-801dbca43ddc@nbd.name>

On 12/05/2016 09:28 AM, Felix Fietkau wrote:
> On 2016-12-05 16:01, Ben Greear wrote:
>>
>>
>> On 12/05/2016 06:53 AM, Johannes Berg wrote:
>>>
>>>> Unless I screwed up, this patch also returns an average.
>>>
>>> Oops, sorry. I missed the whole mac_div() indirection thing.
>>>
>>> I'm not super convinced anyway though - all of this data already is
>>> available in a much more reliable fashion, even trackable when stations
>>> are removed (all data gets sent in the DEL_STATION notification), so
>>> adding a crippled way to get the same data seems a bit strange?
>>
>> Ethtool stats are easy to program against, and I am already making the
>> get-stats call to get other things, so it was a quick tweak to return
>> these additional values.  I agree the stats are of somewhat limited worth,
>> but the cost to get them is also pretty small code wise :)
> You're still adding bloat for everybody for the sake of a little
> convenience on your side. I think that's a bad trade-off.
> nl80211 is not *that* hard to program against...
>
> I don't really see much point in duplicating an arbitrary subset of
> nl80211 information in ethtool.

I don't care that much either way.  I already have local features
that cannot go upstream (per radio station hashes, ability to
set an advertised rateset, VHT on 2.4Ghz, 4.9Ghz support,
forked ath10k driver, etc), so I need to run my own kernel regardless.

I'll repost with the comment removed since I said I would.  Johannes
can apply it if he wishes.

I'm more interested in getting things like the 'iterate' logic fixed
and the ath10k crash bugs resolved.

Thanks,
Ben

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

^ permalink raw reply

* Re: [PATCH] mac80211: Remove invalid flag operations in mesh TSF synchronization
From: Thomas Pedersen @ 2016-12-05 17:59 UTC (permalink / raw)
  To: Masashi Honma; +Cc: Bob Copeland, Johannes Berg, linux-wireless
In-Reply-To: <59d30c75-4eb5-cdc8-c762-b62b22664b51@gmail.com>

On Fri, Dec 2, 2016 at 10:59 PM, Masashi Honma <masashi.honma@gmail.com> wrote:
> On 2016/12/03 06:13, Bob Copeland wrote:
>>
>> On Fri, Dec 02, 2016 at 12:07:18PM -0800, Thomas Pedersen wrote:
>
>
> # Rejected by linux wireless ML. This is resubmission.
>
> thomas and Bob, Thanks for comments.
>
>> 802.11-2012 13.13.2.2.3:
>> The mesh STA checks if the transmitter of the Beacon frame or Probe
>> Response frame is in the
>> process of the TBTT adjustment (see 13.13.4.4.3).
>
> There are two functionalities.
>
> 1) 13.13.2.2 Neighbor offset synchronization method
> 2) 13.13.4.4 TBTT adjustment
>
> The ifmsh->adjusting_tbtt flag implements "TBTT Adjusting field" in the
> Mesh Configuration field.
>
> The flag is updated by 2).
> 13.13.4.4.3 TBTT scanning and adjustment procedures:
> The mesh STA shall set the TBTT Adjusting field in the Mesh
> Configuration element to 1 in order to announce that the TBTT
> adjustment procedure is ongoing.
>
> And the flag is refered by 1) as you said.
>
>
> The purpose of the flag is to prevent 1) while 2) is ongoing.
>
> In other words, 1) has only read access authority to the flag. However,
> previous code updated the flag in 1). In addition, there is no code for
> 2). So I just remove the invalid accessing codes.

I don't think 1) has read only access to that flag. A TSF adjust will
by definition move the TBTT as well.

-- 
thomas

^ permalink raw reply

* [PATCH 130/130] Fixed to checkpatch errors.
From: Ozgur Karatas @ 2016-12-05 17:42 UTC (permalink / raw)
  To: johannes, David Miller; +Cc: linux-wireless, netdev, linux-kernel

Hello,

Fixed to checkpatch errors.

ERROR: net/wireless/util.c:1787: ERROR: that open brace { should be on the previous line
ERROR: net/wireless/util.c:1792: ERROR: that open brace { should be on the previous line

Signed-off-by: Ozgur Karatas <okaratas@member.fsf.org>
---
 net/wireless/util.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/wireless/util.c b/net/wireless/util.c
index 659b507..f4ac755 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1783,11 +1783,13 @@ EXPORT_SYMBOL(cfg80211_free_nan_func);
 
 /* See IEEE 802.1H for LLC/SNAP encapsulation/decapsulation */
 /* Ethernet-II snap header (RFC1042 for most EtherTypes) */
-const unsigned char rfc1042_header[] __aligned(2) =
-	{ 0xaa, 0xaa, 0x03, 0x00, 0x00, 0x00 };
+const unsigned char rfc1042_header[] __aligned(2) = {
+	0xaa, 0xaa, 0x03, 0x00, 0x00, 0x00
+};
 EXPORT_SYMBOL(rfc1042_header);
 
 /* Bridge-Tunnel header (for EtherTypes ETH_P_AARP and ETH_P_IPX) */
-const unsigned char bridge_tunnel_header[] __aligned(2) =
-	{ 0xaa, 0xaa, 0x03, 0x00, 0x00, 0xf8 };
+const unsigned char bridge_tunnel_header[] __aligned(2) = {
+	0xaa, 0xaa, 0x03, 0x00, 0x00, 0xf8
+};
 EXPORT_SYMBOL(bridge_tunnel_header);
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH] mac80211: Return avg sig, rx, tx values in ethtool stats.
From: Felix Fietkau @ 2016-12-05 17:28 UTC (permalink / raw)
  To: Ben Greear, Johannes Berg, linux-wireless
In-Reply-To: <5845813C.8070904@candelatech.com>

On 2016-12-05 16:01, Ben Greear wrote:
> 
> 
> On 12/05/2016 06:53 AM, Johannes Berg wrote:
>>
>>> Unless I screwed up, this patch also returns an average.
>>
>> Oops, sorry. I missed the whole mac_div() indirection thing.
>>
>> I'm not super convinced anyway though - all of this data already is
>> available in a much more reliable fashion, even trackable when stations
>> are removed (all data gets sent in the DEL_STATION notification), so
>> adding a crippled way to get the same data seems a bit strange?
> 
> Ethtool stats are easy to program against, and I am already making the
> get-stats call to get other things, so it was a quick tweak to return
> these additional values.  I agree the stats are of somewhat limited worth,
> but the cost to get them is also pretty small code wise :)
You're still adding bloat for everybody for the sake of a little
convenience on your side. I think that's a bad trade-off.
nl80211 is not *that* hard to program against...

I don't really see much point in duplicating an arbitrary subset of
nl80211 information in ethtool.

- Felix

^ permalink raw reply

* Re: [PATCH 1/2] mac80211: do not iterate active interfaces when in re-configure
From: Adrian Chadd @ 2016-12-05 17:23 UTC (permalink / raw)
  To: Ben Greear
  Cc: Johannes Berg, Michal Kazior, linux-wireless,
	ath10k@lists.infradead.org
In-Reply-To: <5845827B.2000000@candelatech.com>

fwiw, I'm facing the same kinds of cleanup problems with my port of
(oct 2015) ath10k to freebsd.

The oct 2015 ath10k tree doesn't have the firmware per-txq/tid/peer
feedback stuff in it, so this hasn't yet bitten me, but there rest of
the races have - mostly surrounding handling pending TX frames when a
VAP is deleted (vdev/interface in ath10k/mac80211 language) and if any
TX frames were stuck. Stuck TX frames happens more often than I'd like
because of how earlier firmware required peer entries to first appear
in the hardware.

Maybe we need some kind of lifecycle checkpoint for things like peer
addition/removal (for the txq issues ben had before) and the ability
to ask the firmware to stop/flush HTT TX and re-start it. That way we
can cleanly add/remove interfaces at any point without worrying about
any dangling frames in the transmit queue waiting for completion.



-adrian

^ permalink raw reply

* Re: [PATCH 1/7] rtlwifi: btcoexist: Update routines for RTL8192EE
From: Larry Finger @ 2016-12-05 17:17 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: kvalo, devel, Ping-Ke Shih, linux-wireless
In-Reply-To: <20161205113832.GD8176@mwanda>

On 12/05/2016 05:38 AM, Dan Carpenter wrote:
> On Sat, Dec 03, 2016 at 11:32:01AM -0600, Larry Finger wrote:
>> From: Ping-Ke Shih <pkshih@realtek.com>
>>
>> The btcoexist routines for the RTL8192EE have been extensively rewritten.
>>
>
> This patch does a million things and is totally unreviewable.  The
> patch description doesn't say what patch does or why.  It's 5k lines
> of diff.  What the heck???

I am caught in a bind. The BT coexistence routines are written by a Realtek 
contractor. As I cannot get the individual patches from Realtek, there is no 
chance of getting them from a 3rd party.

Larry

^ permalink raw reply

* Re: [PATCH] mac80211: fix Tx BA session stuck issue during sw scanning
From: Chris Chiu @ 2016-12-05 16:56 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, linux
In-Reply-To: <1480949847.31788.33.camel@sipsolutions.net>

On Mon, Dec 5, 2016 at 10:57 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>> > The premise seems fairly reasonable, although I'm a little worried
>> > that if so much new traffic is coming in we never finish the scan
>> > suspend? Actually, the queues are still stopped, so it's only
>> > management frames that can come in, so that should be ok?
>> >
>>
>> Actually it will finish scan eventually and back to SCAN_DECISION
>> state but almost 20~30 seconds elapsed. The local->scanning should be
>> cleared after all channels been scanned. However, from the debug
>> messages I added in ieee80211_iface_work(), it still returns when
>> check local->scanning and the DELBA still has no chance to be
>> transferred then stuck again at the next scan state machine. Supposed
>> to be another scan request issued but I don't know who's the killer.
>> Except to find who issue the next scan request, BA session have no
>> chance to reset in this long scan period (>20s) still need to be
>> taken
>> care.
>
> No no, you misunderstood. My question is more like:
>
> Where is this traffic coming from, since netdev queues should be
> stopped?
>
> And then, if there's so much traffic coming in that we can take 20-30
> seconds to send it out, could we - with the change - get stuck forever?
>
My test scenario is just simply ping (not greedy ping). When it
stalls, you just see 1 ping packet has been retried on the air for 3
times w/o ack from AP. Then no more tx QoS data packet observed on the
air. From the log, it shows the tx ba session been expired and a DELBA
is queued but never sent out. So the STA wants to terminate current BA
session but can't make it. The air capture pcap file shows It can't
deal with the coming "Action" frame for "ADDBA" also. The Block Ack
state machine just stops until the whole sw scan done. In my case,
it's >300 seconds.

So this patch just tries to seek a chance to send out the DELBA and
process the BA related packets in ieee80211_iface_work(). It tries to
avoid the stuck forever thing. I tried 30000+ pings and verified no
stuck observed.

>> You're right. I just want to clear_bit and set_bit in this case,
>> sorry for that confusing. Or any better suggestion?
>
> We seem to be using set_bit/clear_bit so that seems reasonable, unless
> you can prove that all of those are under the lock and we can remove
> the atomics entirely ... Not that it matters hugely, we don't scan all
> the time after all!
>
I agree. Will modify this patch and then send again for approval.
> johannes

^ permalink raw reply

* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Marcel Holtmann @ 2016-12-05 15:40 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Denis Kenzior, Andrew Zaborowski, linux-wireless
In-Reply-To: <1480950886.31788.44.camel@sipsolutions.net>

Hi Johannes,

>>> Well, no, that'd only work with an open connection :)
> 
> Actually, it also works fairly easily for when firmware has 4-way-
> handshake offload, which will be coming to a kernel near you soon.
> 
>> And even that is questionable in my mind for some of the more
>> advanced cases.
> 
> Well, at least in that case you can have things running (for a while)
> if the manager crashes?
> 
>> But I'm not sure what your point is, do you still have objections to 
>> this approach?
> 
> Well, first of all, you can keep things running, at least until you've
> figured out how to restart wpa_supplicant/whatever.
> 
> There also aren't really any important resources to clear when
> userspace dies, at least nothing that userspace can't trivially clear
> later by disconnecting (even unconditionally and ignoring the
> result)...
> 
> So basically I just don't see the advantage. It feels like trading a
> single line of userspace code to disconnect with some (not super
> complex, but still somewhat involved) kernel-side tracking. That
> doesn't really seem like a worthwhile tradeoff to me.

I do not get this argument. That is what WEXT forced you to do since the APIs were awful and clueless. I thought we have moved passed this messy userspace and racy guess work.

If I am a wireless management daemon, I want to decide what happens when I unexpectedly die or get killed for some reason where it is no longer in my control to do this. And there is no guarantee that the daemon even gets restarted. Now you leave dangling connections around. If userspace asks the kernel to clean up after it, then the kernel should be able to do so. That is the only sane option since everything else is racy. The daemon is not in control and has no idea how much time has passed or what happened while it was away. So the only clean thing to do, ask the kernel to disconnect in case something unexpected happens.

And frankly if the wireless daemon can not ask the kernel to clean up, how would a connection manager then even begin to handle this properly. It causes more races and more issues. It becomes a state tracking nightmare. And this is not some theoretical issue, we have seen all the issues with ConnMan. Remember that ConnMan runs in production consumer devices. So these things are real. These systems have to kill daemons for resource limits and other reasons that are out of the control of the daemon itself. And there is no recovery, and if there is, there is no knowing when that happens.

So userspace can clear later argument is not valid for me. If your userspace wants to do that, then fine. Our userspace does not want unsupervised or unmanaged connections. If it goes away, then it wants that these are cleared. And that option is clearly what this patch provides. I find this a clean and useful addition to nl80211. Why are you saying this is not worthwhile?

Regards

Marcel

^ permalink raw reply

* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Denis Kenzior @ 2016-12-05 15:32 UTC (permalink / raw)
  To: Johannes Berg, Andrew Zaborowski, linux-wireless
In-Reply-To: <1480950886.31788.44.camel@sipsolutions.net>

Hi Johannes,

On 12/05/2016 09:14 AM, Johannes Berg wrote:
>>> Well, no, that'd only work with an open connection :)
>
> Actually, it also works fairly easily for when firmware has 4-way-
> handshake offload, which will be coming to a kernel near you soon.
>

Great, but still not applicable for all devices, and still questionable 
how useful an unmanaged connection actually is.  But okay.

>> And even that is questionable in my mind for some of the more
>> advanced cases.
>
> Well, at least in that case you can have things running (for a while)
> if the manager crashes?
>

To what purpose?  That's like saying that maybe a socket should be kept 
open in case an application crashes.

>> But I'm not sure what your point is, do you still have objections to
>> this approach?
>
> Well, first of all, you can keep things running, at least until you've
> figured out how to restart wpa_supplicant/whatever.
>
> There also aren't really any important resources to clear when
> userspace dies, at least nothing that userspace can't trivially clear
> later by disconnecting (even unconditionally and ignoring the
> result)...

Sure, but in the meantime you have a completely unmanaged connection, 
with no connection manager to tell you what the state of it is.  Maybe 
okay for a command line system with a brain in front of it, but not much 
use anywhere else.

But for that use case the SOCKET_OWNER attribute can be omitted...

>
> So basically I just don't see the advantage. It feels like trading a
> single line of userspace code to disconnect with some (not super
> complex, but still somewhat involved) kernel-side tracking. That
> doesn't really seem like a worthwhile tradeoff to me.
>

Well, we'd be trading quite a bit of race-prone user space code that 
performs the clean up on start up for one line of attributes.  I call 
that a good trade.

Also, I thought it was good form / the UNIX way to clean up after 
processes exit ungracefully.  You seem to be arguing against that?  I 
would argue the kernel should be doing the opposite.  It should always 
clean up the connection unless user space has indicated it can be 'reused'.

Regards,
-Denis

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox