Re: [PATCH] mac80211: Remove unused 'struct rate_control_ref' variable

2016-11-21 Thread Johannes Berg
On Mon, 2016-11-21 at 22:54 -0800, Kirtika Ruchandani wrote:
> Commit 3b17fbf87d5d introduced sta_get_expected_throughput()
> leaving variable 'struct rate_control_ref* ref' set but unused.
> Compiling with W=1 gives the following warning, fix it.
> 
> net/mac80211/sta_info.c: In function ‘sta_set_sinfo’:
> net/mac80211/sta_info.c:2052:27: warning: variable ‘ref’ set but not
> used [-Wunused-but-set-variable]

Applied all three of these, thanks Kirtika.

johannes


Re: [PATCH] RFC: Universal scan proposal

2016-11-21 Thread Luca Coelho
Hi Dmitry,
On Wed, 2016-11-16 at 22:47 +, dimitr...@google.com wrote:
>  From 68a9d37a4c7e9dc7a90a6e922cdea52737a98d66 Mon Sep 17 00:00:00 2001
> From: Dmitry Shmidt 
> Date: Wed, 16 Nov 2016 14:27:26 -0800
> Subject: [PATCH] RFC: Universal scan proposal
> 
>Currently we have sched scan with possibility of various
> intervals. We would like to extend it to support also
> different types of scan.
>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
> Report: none / batch / immediate
>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
> 
> Change-Id: I9f3e4c975784f1c1c5156887144d80fc5a26bffa
> Signed-off-by: Dmitry Shmidt 
> ---

I like the initiative and I think this is definitely something that can
improve concurrent scanning instances.  But IMHO the most important is
to discuss the semantics of this change, such as which scans can be
combined, who makes the decisions of combining them, how priorities are
sorted out etc.  I think the types of scan are not relevant in the
nl80211 API, but the characteristics of the scans are.  For instance,
"urgent scan" (for initial connection), best-effort scan for roaming...
and latency requirements, such as low-latency for location and initial
connection and high-latency for scheduled scan.  Then we decided, in
the kernel, how to combine and prioritize them according to their
characteristics, instead of having to map scan types to these
characteristics.

What do you think?

--
Cheers,
Luca.


[PATCH] mac80211: Remove unused 'struct rate_control_ref' variable

2016-11-21 Thread Kirtika Ruchandani
Commit 3b17fbf87d5d introduced sta_get_expected_throughput()
leaving variable 'struct rate_control_ref* ref' set but unused.
Compiling with W=1 gives the following warning, fix it.

net/mac80211/sta_info.c: In function ‘sta_set_sinfo’:
net/mac80211/sta_info.c:2052:27: warning: variable ‘ref’ set but not used 
[-Wunused-but-set-variable]

Fixes: 3b17fbf87d5d ("mac80211: mesh: Add support for HW RC implementation")
Cc: Johannes Berg 
Cc: Maxim Altshul 
Signed-off-by: Kirtika Ruchandani 
---
 net/mac80211/sta_info.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 1711bae..4ab75a9 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -2049,16 +2049,12 @@ void sta_set_sinfo(struct sta_info *sta, struct 
station_info *sinfo)
 {
struct ieee80211_sub_if_data *sdata = sta->sdata;
struct ieee80211_local *local = sdata->local;
-   struct rate_control_ref *ref = NULL;
u32 thr = 0;
int i, ac, cpu;
struct ieee80211_sta_rx_stats *last_rxstats;

last_rxstats = sta_get_last_rx_stats(sta);

-   if (test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
-   ref = local->rate_ctrl;
-
sinfo->generation = sdata->local->sta_generation;

/* do before driver, so beacon filtering drivers have a
--
2.8.0.rc3.226.g39d4020



[PATCH] mac80211: Remove unused 'rates_idx' variable

2016-11-21 Thread Kirtika Ruchandani
Commit f027c2aca0cf introduced 'rates_idx' in
ieee80211_tx_status_noskb but did not use it. Compiling with W=1
gives the following warning, fix it.

mac80211/status.c: In function ‘ieee80211_tx_status_noskb’:
mac80211/status.c:636:6: warning: variable ‘rates_idx’ set but not used 
[-Wunused-but-set-variable]

This is a harmless warning, and is only being fixed to reduce the
noise generated with W=1.

Fixes: f027c2aca0cf ("mac80211: add ieee80211_tx_status_noskb")
Cc: Johannes Berg 
Cc: Felix Fietkau 
Signed-off-by: Kirtika Ruchandani 
---
 net/mac80211/status.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index ddf71c6..f7c5ae5 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -633,10 +633,9 @@ void ieee80211_tx_status_noskb(struct ieee80211_hw *hw,
struct ieee80211_local *local = hw_to_local(hw);
struct ieee80211_supported_band *sband;
int retry_count;
-   int rates_idx;
bool acked, noack_success;

-   rates_idx = ieee80211_tx_get_rates(hw, info, _count);
+   ieee80211_tx_get_rates(hw, info, _count);

sband = hw->wiphy->bands[info->band];

--
2.8.0.rc3.226.g39d4020



Re: scheduled scan interval

2016-11-21 Thread Luca Coelho
Hi Arend,
On Mon, 2016-11-21 at 20:34 +0100, Arend Van Spriel wrote:
> On 21-11-2016 16:08, Luca Coelho wrote:
> > Hi Arend,
> > 
> > On Mon, 2016-11-21 at 13:03 +0100, Arend Van Spriel wrote:
> > > On 21-11-2016 12:30, Arend Van Spriel wrote:
> > > > On 21-11-2016 12:19, Arend Van Spriel wrote:
> > > > > Hi Johannes, Luca,
> > > > > 
> > > > > The gscan work made me look at scheduled scan and the implementation 
> > > > > of
> > > > > it in brcmfmac. The driver ignored the interval parameter from
> > > > > user-space. Now I am fixing that. One thing is that our firmware has a
> > > > > minimum interval which can not be indicated in struct wiphy. The other
> > > > > issue is how the maximum interval is used in the nl80211.c.
> > > > > 
> > > > > In nl80211_parse_sched_scan_plans() it is used against value passed in
> > > > > NL80211_ATTR_SCHED_SCAN_INTERVAL and NL80211_SCHED_SCAN_PLAN_INTERVAL.
> > > > > For the first one it caps the value to the maximum, but for the second
> > > > > one it returns -EINVAL. I suspect this is done because maximum 
> > > > > interval
> > > > > was introduced with schedule scan plans, but it feels inconsistent.
> > > > 
> > > > It also maybe simply wrong to cap. At least brcmfmac does not set the
> > > > maximum so it will always get interval being zero. Maybe better to do:
> > > > 
> > > > if (wiphy->max_sched_scan_plan_interval &&
> > > > request->scan_plans[0].interval >
> > > > wiphy->max_sched_scan_plan_interval)
> > > > return -EINVAL;
> > > > 
> > > > > Thoughts?
> > > 
> > > Digging deeper. Looking at v4.3 before introduction of sched_scan_plans.
> > > struct sched_scan_request::interval was specified in milliseconds! Below
> > > the drivers that I see having scheduled scan support:
> > > 
> > > iwlmvm: cap interval, convert to seconds.
> > > ath6kl: cap to 1sec minimum, no max check, convert to seconds.
> > > wl12xx: no checking in driver, fw need milliseconds.
> > > wl18xx: no checking in driver, fw need milliseconds.
> > > 
> > > The milliseconds conversion seems to be taken care of by multiplying
> > > with MSEC_PER_SEC in wl{12,18}xx drivers.
> > > 
> > > It seems in 4.8 only iwlmvm set wiphy->max_sched_scan_plan_interval so
> > > other drivers will get interval of zero which only ath6kl handles.
> > 
> > With the introduction of scheduled scan plans, we sort of deprecated
> > the "generic" scheduled scan interval.  It doesn't make sense to have
> > both passed at the same time, so nl80211 forbids
> > NL80211_ATTR_SCHED_SCAN_INTERVAL if we pass
> > NL80211_ATTR_SCHED_SCAN_PLANS.
> 
> Indeed, but if no plans are passed it is still allowed.

That's right.  But the driver will get it as a single plan.


> > The original NL80211_ATTR_SCHED_SCAN_INTERVAL was specified in msecs,
> > which is silly because we can never get millisecond accuracy in this. 
> > Thus, in the plans API, we decided to use seconds instead (because it
> > makes much more sense).  Additionally, the interval is considered
> > "advisory", because the FW may not be able guarantee the exact
> > intervals (for instance, the iwlwifi driver actually starts the
> > interval timer after scan completion, so if you specify 10 seconds
> > intervals, in practice they'll be 13-14 seconds).
> 
> Agree. Our firmware wants to have it in seconds as well.

It was actually my mistake when I implemented it in my TI days (can't
hide from git blame ;)). TI's firmware used msecs and I just blindly
followed it.


> > I'm not sure I'm answering your question, because I'm also not sure I
> > understood the question. :)
> 
> The question is this: Why is the interval capped at
> max_sched_scan_plan_interval if it exceeds it and no plans are provided
> (so continue to setup the scheduled scan request) whereas when plans are
> provided and an interval exceeds max_sched_scan_plan_interval it aborts
> with -EINVAL.

Oh, I see.  The problem is that the "max_sched_scan_plan_interval" was
introduced later.  If the userspace passes
NL80211__ATTR_SCHED_SCAN_INTERVAL, it probably means that it doesn't
know about NL80211_ATTR_MAX_SCAN_PLAN_INTERVAL (i.e. it's only using an
old API).  If it is also, for some reason, passing a very large number,
we shouldn't suddenly make it fail with -EINVALID, because that would
be a break of UABI.  And since we know the driver cannot support such a
large number, we cap it because it's the best we can do.

Now, if the userspace uses NL80211_ATTR_MAX_SCAN_PLAN_INTERVAL, it
means that it knows the new API (and was written after the new API was
introduced), so we can be stricter and assume it must have checked
NL80211_ATTR_MAX_SCAN_PLAN_INTERVAL.

Makes sense?


> And a follow-up question for this snippet of code in
> nl80211_parse_sched_scan_plans():
> 
>   if (!attrs[NL80211_ATTR_SCHED_SCAN_PLANS]) {
>   u32 interval;
> 
>   /*
>* If scan plans are not specified,
>

[PATCH] mac80211: Remove unused 'struct ieee80211_rx_status' ptr

2016-11-21 Thread Kirtika Ruchandani
Commit 554891e63a29 introduced 'struct ieee80211_rx_status' in
ieee80211_rx_h_defragment but did not use it. Compiling with W=1
gives the following warning, fix it.

net/mac80211/rx.c: In function ‘ieee80211_rx_h_defragment’:
net/mac80211/rx.c:1911:30: warning: variable ‘status’ set but not used 
[-Wunused-but-set-variable]

Fixes: 554891e63a29 ("mac80211: move packet flags into packet")
Cc: Johannes Berg 
Cc: John W. Linville 
Signed-off-by: Kirtika Ruchandani 
---
 net/mac80211/rx.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index eeab725..d2a00f2 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1908,7 +1908,6 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
unsigned int frag, seq;
struct ieee80211_fragment_entry *entry;
struct sk_buff *skb;
-   struct ieee80211_rx_status *status;

hdr = (struct ieee80211_hdr *)rx->skb->data;
fc = hdr->frame_control;
@@ -2034,9 +2033,6 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
dev_kfree_skb(skb);
}

-   /* Complete frame has been reassembled - process it now */
-   status = IEEE80211_SKB_RXCB(rx->skb);
-
  out:
ieee80211_led_rx(rx->local);
  out_no_led:
--
2.8.0.rc3.226.g39d4020



Re: scheduled scan interval

2016-11-21 Thread Arend Van Spriel
On 21-11-2016 16:08, Luca Coelho wrote:
> Hi Arend,
> 
> On Mon, 2016-11-21 at 13:03 +0100, Arend Van Spriel wrote:
>> On 21-11-2016 12:30, Arend Van Spriel wrote:
>>> On 21-11-2016 12:19, Arend Van Spriel wrote:
 Hi Johannes, Luca,

 The gscan work made me look at scheduled scan and the implementation of
 it in brcmfmac. The driver ignored the interval parameter from
 user-space. Now I am fixing that. One thing is that our firmware has a
 minimum interval which can not be indicated in struct wiphy. The other
 issue is how the maximum interval is used in the nl80211.c.

 In nl80211_parse_sched_scan_plans() it is used against value passed in
 NL80211_ATTR_SCHED_SCAN_INTERVAL and NL80211_SCHED_SCAN_PLAN_INTERVAL.
 For the first one it caps the value to the maximum, but for the second
 one it returns -EINVAL. I suspect this is done because maximum interval
 was introduced with schedule scan plans, but it feels inconsistent.
>>>
>>> It also maybe simply wrong to cap. At least brcmfmac does not set the
>>> maximum so it will always get interval being zero. Maybe better to do:
>>>
>>> if (wiphy->max_sched_scan_plan_interval &&
>>> request->scan_plans[0].interval >
>>> wiphy->max_sched_scan_plan_interval)
>>> return -EINVAL;
>>>
 Thoughts?
>>
>> Digging deeper. Looking at v4.3 before introduction of sched_scan_plans.
>> struct sched_scan_request::interval was specified in milliseconds! Below
>> the drivers that I see having scheduled scan support:
>>
>> iwlmvm: cap interval, convert to seconds.
>> ath6kl: cap to 1sec minimum, no max check, convert to seconds.
>> wl12xx: no checking in driver, fw need milliseconds.
>> wl18xx: no checking in driver, fw need milliseconds.
>>
>> The milliseconds conversion seems to be taken care of by multiplying
>> with MSEC_PER_SEC in wl{12,18}xx drivers.
>>
>> It seems in 4.8 only iwlmvm set wiphy->max_sched_scan_plan_interval so
>> other drivers will get interval of zero which only ath6kl handles.
> 
> With the introduction of scheduled scan plans, we sort of deprecated
> the "generic" scheduled scan interval.  It doesn't make sense to have
> both passed at the same time, so nl80211 forbids
> NL80211_ATTR_SCHED_SCAN_INTERVAL if we pass
> NL80211_ATTR_SCHED_SCAN_PLANS.

Indeed, but if no plans are passed it is still allowed.

> The original NL80211_ATTR_SCHED_SCAN_INTERVAL was specified in msecs,
> which is silly because we can never get millisecond accuracy in this. 
> Thus, in the plans API, we decided to use seconds instead (because it
> makes much more sense).  Additionally, the interval is considered
> "advisory", because the FW may not be able guarantee the exact
> intervals (for instance, the iwlwifi driver actually starts the
> interval timer after scan completion, so if you specify 10 seconds
> intervals, in practice they'll be 13-14 seconds).

Agree. Our firmware wants to have it in seconds as well.

> I'm not sure I'm answering your question, because I'm also not sure I
> understood the question. :)

The question is this: Why is the interval capped at
max_sched_scan_plan_interval if it exceeds it and no plans are provided
(so continue to setup the scheduled scan request) whereas when plans are
provided and an interval exceeds max_sched_scan_plan_interval it aborts
with -EINVAL.

And a follow-up question for this snippet of code in
nl80211_parse_sched_scan_plans():

if (!attrs[NL80211_ATTR_SCHED_SCAN_PLANS]) {
u32 interval;

/*
 * If scan plans are not specified,
 * %NL80211_ATTR_SCHED_SCAN_INTERVAL must be specified. In this
 * case one scan plan will be set with the specified scan
 * interval and infinite number of iterations.
 */
if (!attrs[NL80211_ATTR_SCHED_SCAN_INTERVAL])
return -EINVAL;

interval = nla_get_u32(attrs[NL80211_ATTR_SCHED_SCAN_INTERVAL]);
if (!interval)
return -EINVAL;

request->scan_plans[0].interval =
DIV_ROUND_UP(interval, MSEC_PER_SEC);
if (!request->scan_plans[0].interval)
return -EINVAL;

if (request->scan_plans[0].interval >
wiphy->max_sched_scan_plan_interval)
request->scan_plans[0].interval =
wiphy->max_sched_scan_plan_interval;

return 0;
}

So in v4.3 the interval was only validated to be non-zero. Now the
interval is validated and capped to wiphy->max_sched_scan_plan_interval
but apart from iwlmvm there are no driver specifying that so
max_sched_scan_plan_interval = 0 for those and interval is unsigned int
not equal to zero. So the last if statement above is true setting the
interval to zero. I think the if statement 

Re: scheduled scan interval

2016-11-21 Thread Arend Van Spriel


On 21-11-2016 17:59, Dave Taht wrote:
> On Mon, Nov 21, 2016 at 7:08 AM, Luca Coelho  wrote:
>> Hi Arend,
>>
>> On Mon, 2016-11-21 at 13:03 +0100, Arend Van Spriel wrote:
>>> On 21-11-2016 12:30, Arend Van Spriel wrote:
 On 21-11-2016 12:19, Arend Van Spriel wrote:
> Hi Johannes, Luca,
>
> The gscan work made me look at scheduled scan and the implementation of
> it in brcmfmac. The driver ignored the interval parameter from
> user-space. Now I am fixing that. One thing is that our firmware has a
> minimum interval which can not be indicated in struct wiphy. The other
> issue is how the maximum interval is used in the nl80211.c.
>
> In nl80211_parse_sched_scan_plans() it is used against value passed in
> NL80211_ATTR_SCHED_SCAN_INTERVAL and NL80211_SCHED_SCAN_PLAN_INTERVAL.
> For the first one it caps the value to the maximum, but for the second
> one it returns -EINVAL. I suspect this is done because maximum interval
> was introduced with schedule scan plans, but it feels inconsistent.

 It also maybe simply wrong to cap. At least brcmfmac does not set the
 maximum so it will always get interval being zero. Maybe better to do:

 if (wiphy->max_sched_scan_plan_interval &&
 request->scan_plans[0].interval >
 wiphy->max_sched_scan_plan_interval)
 return -EINVAL;

> Thoughts?
>>>
>>> Digging deeper. Looking at v4.3 before introduction of sched_scan_plans.
>>> struct sched_scan_request::interval was specified in milliseconds! Below
>>> the drivers that I see having scheduled scan support:
>>>
>>> iwlmvm: cap interval, convert to seconds.
>>> ath6kl: cap to 1sec minimum, no max check, convert to seconds.
>>> wl12xx: no checking in driver, fw need milliseconds.
>>> wl18xx: no checking in driver, fw need milliseconds.
>>>
>>> The milliseconds conversion seems to be taken care of by multiplying
>>> with MSEC_PER_SEC in wl{12,18}xx drivers.
>>>
>>> It seems in 4.8 only iwlmvm set wiphy->max_sched_scan_plan_interval so
>>> other drivers will get interval of zero which only ath6kl handles.
>>
>> With the introduction of scheduled scan plans, we sort of deprecated
>> the "generic" scheduled scan interval.  It doesn't make sense to have
>> both passed at the same time, so nl80211 forbids
>> NL80211_ATTR_SCHED_SCAN_INTERVAL if we pass
>> NL80211_ATTR_SCHED_SCAN_PLANS.
>>
>> The original NL80211_ATTR_SCHED_SCAN_INTERVAL was specified in msecs,
>> which is silly because we can never get millisecond accuracy in this.
>> Thus, in the plans API, we decided to use seconds instead (because it
>> makes much more sense).  Additionally, the interval is considered
>> "advisory", because the FW may not be able guarantee the exact
>> intervals (for instance, the iwlwifi driver actually starts the
>> interval timer after scan completion, so if you specify 10 seconds
>> intervals, in practice they'll be 13-14 seconds).
>>
>> I'm not sure I'm answering your question, because I'm also not sure I
>> understood the question. :)
> 
> I'm not sure if I understand the discussion and hooks myself, but
> recently fixes for doing channel scans saner from userspace ended up
> here, after some discussion.

scheduled scan is a scan-offload feature in which user-space requests
the firmware on the device to perform a period scan at a requested
interval. As such it is not different from NM or wpa_supplicant
performing a scan, but the host will only get informed about found SSIDs
which user-space has configured, eg. the networks that are configured in
wpa_supplicant. How much time the scan takes is determined by the
channels in the scheduled scan request. Worst case that means all
supported 2G and 5G channels. So it is just about offloading. There is
not necessarily reduced impact.

Regards,
Arend

> https://bugzilla.gnome.org/show_bug.cgi?id=766482
> 
> Anything that can reduce the impact of this behavior, I'm for!
> 
> http://www.taht.net/~d/channel_scans_destroying_latency_under_load_for_10s.png
> 
> 
> 
>>
>> --
>> Cheers,
>> Luca.
> 
> 
> 


Re: [PATCH v3 4/5] mwifiex: wait firmware dump complete during card remove process

2016-11-21 Thread Brian Norris
Hi,

On Wed, Nov 16, 2016 at 06:39:08PM +0530, Amitkumar Karwar wrote:
> From: Xinming Hu 
> 
> Wait for firmware dump complete in card remove function.
> For sdio interface, there are two diffenrent cases,
> card reset trigger sdio_work and firmware dump trigger sdio_work.
> Do code rearrangement for distinguish between these two cases.

On second review of the SDIO card reset code (which I'll repeat is quite
ugly), you seem to be making a bad distinction here. What if there is a
firmware dump happening concurrently with your card-reset handling? You
*do* want to synchronize with the firmware dump before completing the
card reset, or else you might be freeing up internal card resources that
are still in use. See below.

> 
> Signed-off-by: Xinming Hu 
> Signed-off-by: Amitkumar Karwar 
> ---
> v2: 1. Get rid of reset_triggered flag. Instead split the code and use
> __mwifiex_sdio_remove() (Brian Norris/Dmitry Torokhov)
> 2. "v1 4/5 mwifiex: firmware dump code rearrangement.." is dropped. So
> rebased accordingly.
> v3: same as [v2,5/5]. The improvement of 'moving pcie_work to card struct'
> suggested by Brian is taken care in next patch.
> ---
>  drivers/net/wireless/marvell/mwifiex/pcie.c |  6 +-
>  drivers/net/wireless/marvell/mwifiex/sdio.c | 15 ---
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c 
> b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index dd8f7aa..c8e69a4 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -51,6 +51,9 @@ static int mwifiex_pcie_probe_of(struct device *dev)
>   return 0;
>  }
>  
> +static void mwifiex_pcie_work(struct work_struct *work);
> +static DECLARE_WORK(pcie_work, mwifiex_pcie_work);
> +
>  static int
>  mwifiex_map_pci_memory(struct mwifiex_adapter *adapter, struct sk_buff *skb,
>  size_t size, int flags)
> @@ -254,6 +257,8 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
>   if (!adapter || !adapter->priv_num)
>   return;
>  
> + cancel_work_sync(_work);
> +
>   if (user_rmmod && !adapter->mfg_mode) {
>   mwifiex_deauthenticate_all(adapter);
>  
> @@ -2722,7 +2727,6 @@ static void mwifiex_pcie_work(struct work_struct *work)
>   mwifiex_pcie_device_dump_work(save_adapter);
>  }
>  
> -static DECLARE_WORK(pcie_work, mwifiex_pcie_work);
>  /* This function dumps FW information */
>  static void mwifiex_pcie_device_dump(struct mwifiex_adapter *adapter)
>  {
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
> b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index 16d1d30..78f2cc9 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -46,6 +46,9 @@
>   */
>  static u8 user_rmmod;
>  
> +static void mwifiex_sdio_work(struct work_struct *work);
> +static DECLARE_WORK(sdio_work, mwifiex_sdio_work);
> +
>  static struct mwifiex_if_ops sdio_ops;
>  static unsigned long iface_work_flags;
>  
> @@ -220,7 +223,7 @@ static int mwifiex_sdio_resume(struct device *dev)
>   * This function removes the interface and frees up the card structure.
>   */
>  static void
> -mwifiex_sdio_remove(struct sdio_func *func)
> +__mwifiex_sdio_remove(struct sdio_func *func)
>  {
>   struct sdio_mmc_card *card;
>   struct mwifiex_adapter *adapter;
> @@ -249,6 +252,13 @@ static int mwifiex_sdio_resume(struct device *dev)
>   mwifiex_remove_card(adapter);
>  }
>  
> +static void
> +mwifiex_sdio_remove(struct sdio_func *func)
> +{
> + cancel_work_sync(_work);
> + __mwifiex_sdio_remove(func);
> +}
> +
>  /*
>   * SDIO suspend.
>   *
> @@ -2227,7 +2237,7 @@ static void mwifiex_recreate_adapter(struct 
> sdio_mmc_card *card)
>* discovered and initializes them from scratch.
>*/
>  
> - mwifiex_sdio_remove(func);
> + __mwifiex_sdio_remove(func);

^^ So here, you're trying to avoid syncing with the card-reset work
event, except that function will free up all your resources (including
the static save_adapter). Thus, you're explicitly allowing a
use-after-free error here. That seems unwise.

Instead, you should actually retain the invariant that you're doing a
full remove/reinitialize here, which includes doing the *same*
cancel_work_sync() here in mwifiex_recreate_adapter() as you would in
any other remove().

IOW, kill the __mwifiex_sdio_remove() and just call
mwifiex_sdio_remove() as you were.

That also means that you can do the same per-adapter cleanup in the
following patch as you do for PCIe.

Brian

>  
>   /*
>* Normally, we would let the driver core take care of releasing these.
> @@ -2568,7 +2578,6 @@ static void mwifiex_sdio_work(struct work_struct *work)
>   mwifiex_sdio_card_reset_work(save_adapter);
>  }
>  
> -static DECLARE_WORK(sdio_work, mwifiex_sdio_work);
>  

Re: [PATCH v3 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm

2016-11-21 Thread Brian Norris
On Thu, Nov 17, 2016 at 02:41:11PM +0200, Kalle Valo wrote:
> Patchwork link for the same:
> 
> https://patchwork.kernel.org/project/linux-wireless/list/?state=2=mwifiex
> 
> Does that look ok?

FWIW, your merges (since you made the above comments) look sane to me.
Thanks!

Brian


Re: Break-it testing for wifi

2016-11-21 Thread Ben Greear

On 11/21/2016 08:55 AM, Steve deRosier wrote:



On Mon, Nov 21, 2016 at 8:10 AM, Ben Greear > wrote:

Hello!

I am thinking about adding some sort of framework to wpa_supplicant and/or 
the
mac80211 stack to allow purposefully creating bad station behaviour in 
order to
test robustness of APs.

Some ideas so far:

1)  Allow supplicant to do bad state-machine transitions (start 4-way 
before associating, for instance).

2)  Randomly corrupt mgt frames in driver and/or mac80211 stack and/or 
supplicant.

3)  Possibly allow user to make specific corruptions.  This would probably 
be in supplicant
only, and I am not sure how this would be configured.  Maybe allow user 
to over-ride
existing IEs and add bogus ones of their own choosing.

4)  Maybe some specific tests like putting in over-flow sized lengths of 
IEs.

Has anyone done anything similar they would like to share?

Johannes:  Any interest in having such a framework in upstream kernels?

Any other ideas for how to improve this feature set?

Thanks,
Ben


Hi Ben,

I did something related to this many years ago using the radiotap interface and 
based on Andy Green's work.  It's very limited but might be interesting.

https://github.com/derosier/packetvector

I would absolutely welcome such a tool. I'd like it to be useful for testing 
clients and mesh nodes in addition to APs, but I don't think that's a big 
stretch.

I guess my only comment is if it's possible for this to be in userspace, that's 
where'd like to see it. If not, some sort of framework we all could use would be
nice. All in one place would also be nice, but as you've got multiple 
independent components that cooperate that might be difficult to achieve.


[re-added linux-wireless to CC]

Some things, like probe requests, are created as far down as the firmware 
(ath10k, for instance).
Some IEs are made in mac80211 as well.

And, ath10k (and likely other thick-firmware implementations) is crap for 
packet injection.

I think the closer I can be to 'real' supplicant, stack & driver behaviour, the 
better chance I have of finding
more interesting bugs.

So, I don't think a pure user-space app is that useful for my desired testing 
scenario.

My specific goal is testing APs, but I think it should work fine for testing 
other
connection types.

If I made some generic code to mangle management frames, including parsing IEs 
and re-writing
them and such (as well as randomly bit-flipping as requested), maybe it could 
be called from
tx logic in mac80211.  Corrupting things in the stack might help harden the 
drivers (and firmware)
a bit on the sending path, and might work for generic network devices.  
Possibly I would need to add hooks in the
driver as well if frames were actually generated there.  For probe requests, I 
might have to
really hack all the way down into the firmware to have full control for 
corrupting that.

Since probe requests don't require much state, possibly that is a place where a 
relatively
dumb user-space packet injection would be best.

Thanks,
Ben

--
Ben Greear 
Candela Technologies Inc  http://www.candelatech.com



Re: [PATCH 0/7] rtl8xxxu: Pending patches

2016-11-21 Thread Jes Sorensen
Barry Day  writes:
> On Sat, Nov 19, 2016 at 06:53:42PM -0500, Jes Sorensen wrote:
>> Barry Day  writes:
>> > On Fri, Nov 18, 2016 at 09:00:10PM -0500, Jes Sorensen wrote:
>> >> Barry Day  writes:
>> >> > On Fri, Nov 18, 2016 at 04:44:21PM -0500, jes.soren...@redhat.com wrote:
>> >> >> From: Jes Sorensen 
>> >> >> 
>> >> >> Kalle,
>> >> >> 
>> >> >> Please find attached a number of patches for the rtl8xxxu
>> >> >> driver.
>> >> >> 
>> >> >> The issues reported with wpa_supplicant on 8723bu still needs further
>> >> >> investigation.
>> >> >> 
>> >> >
>> >> > The patch I posted that you want tested more will also fix the
>> >> > wpa_supplicant issue.  Currently I'm looking at why the tx rate is not
>> >> > what it should be. I feel fixing that first will be beneficial for
>> >> > fixing any other issues.
>> >> 
>> >> Interesting, I was thinking that might be the case. I do want to dig
>> >> into this further to understand it better. If we use your solution I
>> >> will want to make sure we cover both gen1 and gen2 parts.
>> >> 
>> >> > The recent merge has made my local branch of rtl8xxxu-devel 14
>> >> > commits ahead.
>> >> > Do I need to do a reset and submit a new patch for the DWA-131 dongle?
>> >> 
>> >> In general you need to use 'git pull --rebase' on my tree. I rebase it
>> >> to stay in sync with Kalle's tree.
>> >> 
>> >> The DWA-131 is the 8192eu? Sorry a bit behind and my mind is losing
>> >> bits. If it's the patch you posted earlier I can dig it out and play
>> >> with it - I am still catching up though, so please be patient.
>> >
>> > yes it's an 8192eu.
>> 
>> Gotcha - how do you do your testing to reproduce the problem btw? Most
>> of my testing is using the 8723au as a primary device and the next
>> device as a follow-on, so I may not see as long a run with the device
>> active as you see.
>> 
>
> Testing is simple. Connect to an AP in the usual
> manner..disconnect..reconnect.  The 8192eu will fail to reconnect and
> John Heenan reported the 8723bu also fails to reconnect. Even though
> he was directly stopping and restarting wpa_supplicant, it's the same
> thing to the driver - connect..disconnect..reconnect.

Thanks for the details - I'll have a look shortly.

> With using a usb_device pointer, each message starts with the usb bus
> address.  Plug it into a different port and that address could
> change. By using a pointer to the device associated with the wiphy
> each message will begin with the driver name. Just makes it easier for
> the average user to report what's in the log because he can just grep
> for "rtl8xxxu".

I see - that would be problematic for me as I quite often have 3-4 of
these things plugged in at the same time. Not knowing which port spits
out the message would make it a lot harder to track. In fact my primary
devel box for this (Lenovo Yoga 13) has an rtl8723au soldered on the
motherboard, so the moment I plug in any other dongle I'll have two.

The alternative would be to add a prefer to the individual messages.

Cheers,
Jes


Re: scheduled scan interval

2016-11-21 Thread Dave Taht
On Mon, Nov 21, 2016 at 7:08 AM, Luca Coelho  wrote:
> Hi Arend,
>
> On Mon, 2016-11-21 at 13:03 +0100, Arend Van Spriel wrote:
>> On 21-11-2016 12:30, Arend Van Spriel wrote:
>> > On 21-11-2016 12:19, Arend Van Spriel wrote:
>> > > Hi Johannes, Luca,
>> > >
>> > > The gscan work made me look at scheduled scan and the implementation of
>> > > it in brcmfmac. The driver ignored the interval parameter from
>> > > user-space. Now I am fixing that. One thing is that our firmware has a
>> > > minimum interval which can not be indicated in struct wiphy. The other
>> > > issue is how the maximum interval is used in the nl80211.c.
>> > >
>> > > In nl80211_parse_sched_scan_plans() it is used against value passed in
>> > > NL80211_ATTR_SCHED_SCAN_INTERVAL and NL80211_SCHED_SCAN_PLAN_INTERVAL.
>> > > For the first one it caps the value to the maximum, but for the second
>> > > one it returns -EINVAL. I suspect this is done because maximum interval
>> > > was introduced with schedule scan plans, but it feels inconsistent.
>> >
>> > It also maybe simply wrong to cap. At least brcmfmac does not set the
>> > maximum so it will always get interval being zero. Maybe better to do:
>> >
>> > if (wiphy->max_sched_scan_plan_interval &&
>> > request->scan_plans[0].interval >
>> > wiphy->max_sched_scan_plan_interval)
>> > return -EINVAL;
>> >
>> > > Thoughts?
>>
>> Digging deeper. Looking at v4.3 before introduction of sched_scan_plans.
>> struct sched_scan_request::interval was specified in milliseconds! Below
>> the drivers that I see having scheduled scan support:
>>
>> iwlmvm: cap interval, convert to seconds.
>> ath6kl: cap to 1sec minimum, no max check, convert to seconds.
>> wl12xx: no checking in driver, fw need milliseconds.
>> wl18xx: no checking in driver, fw need milliseconds.
>>
>> The milliseconds conversion seems to be taken care of by multiplying
>> with MSEC_PER_SEC in wl{12,18}xx drivers.
>>
>> It seems in 4.8 only iwlmvm set wiphy->max_sched_scan_plan_interval so
>> other drivers will get interval of zero which only ath6kl handles.
>
> With the introduction of scheduled scan plans, we sort of deprecated
> the "generic" scheduled scan interval.  It doesn't make sense to have
> both passed at the same time, so nl80211 forbids
> NL80211_ATTR_SCHED_SCAN_INTERVAL if we pass
> NL80211_ATTR_SCHED_SCAN_PLANS.
>
> The original NL80211_ATTR_SCHED_SCAN_INTERVAL was specified in msecs,
> which is silly because we can never get millisecond accuracy in this.
> Thus, in the plans API, we decided to use seconds instead (because it
> makes much more sense).  Additionally, the interval is considered
> "advisory", because the FW may not be able guarantee the exact
> intervals (for instance, the iwlwifi driver actually starts the
> interval timer after scan completion, so if you specify 10 seconds
> intervals, in practice they'll be 13-14 seconds).
>
> I'm not sure I'm answering your question, because I'm also not sure I
> understood the question. :)

I'm not sure if I understand the discussion and hooks myself, but
recently fixes for doing channel scans saner from userspace ended up
here, after some discussion.

https://bugzilla.gnome.org/show_bug.cgi?id=766482

Anything that can reduce the impact of this behavior, I'm for!

http://www.taht.net/~d/channel_scans_destroying_latency_under_load_for_10s.png



>
> --
> Cheers,
> Luca.



-- 
Dave Täht
Let's go make home routers and wifi faster! With better software!
http://blog.cerowrt.org


Re: [PATCH] rtl8xxxu: Fix failure to reconnect to AP

2016-11-21 Thread Jes Sorensen
Jes Sorensen  writes:
> Barry Day  writes:
>> The rtl8192e and rtl8723 fail to reconnect to an AP after being
>> disconnected. Ths patch fixes that without affecting the rtl8192cu.
>> I don't have a rtl8723 to test but it has been tested on a rtl8192eu.
>> After going through the orginal realtek code for the rtl8723, I am
>> confident the patch is applicable to both.
>>
>> Signed-off-by: Barry Day 
>> ---
>>  rtl8xxxu_core.c | 18 ++
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> Hi Barry,
>
> Thank you for the patch. There are a couple of items which I am not
> 100% sure about the order of.
>
>> diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c
>> index 04141e5..6ac10d2 100644
>> --- a/rtl8xxxu_core.c
>> +++ b/rtl8xxxu_core.c
>> @@ -4372,17 +4372,25 @@ void rtl8xxxu_gen1_report_connect(struct 
>> rtl8xxxu_priv *priv,
>>  void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
>>u8 macid, bool connect)
>>  {
>> +u8 val8;
>>  struct h2c_cmd h2c;
>>  
>>  memset(, 0, sizeof(struct h2c_cmd));
>>  
>>  h2c.media_status_rpt.cmd = H2C_8723B_MEDIA_STATUS_RPT;
>> -if (connect)
>> +if (connect) {
>>  h2c.media_status_rpt.parm |= BIT(0);
>> -else
>> -h2c.media_status_rpt.parm &= ~BIT(0);
>> +rtl8xxxu_gen2_h2c_cmd(priv, ,
>> +sizeof(h2c.media_status_rpt));
>> +} else {
>> +val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
>> +val8 &= ~BEACON_FUNCTION_ENABLE;
>> +
>> +rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
>> +rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x00);
>> +rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
>> +}
>>  
>> -rtl8xxxu_gen2_h2c_cmd(priv, , sizeof(h2c.media_status_rpt));
>>  }

Barry,

So looking at this again, I am pretty sure you will break monitor mode
with this. By setting REG_RXFLTMAP2 to 0x you stop reception of all
data frames.

The other thing here is that you change removes the part notifying the
firmware that we disconnected since you now only send the
media_start_rpt command on connect, but not on disconnect.

I am curious if the problem goes away if you simply add the BEACON_CTRL
and REG_DUAL_TSF_RST parts?

>
> This only affects 8192eu and not 8192cu - we left RXFLTMAP2 out of here
> on purpose for monitor mode, but you now disable it for 8192eu/8723bu.
>
>>  void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
>> @@ -4515,6 +4523,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, 
>> struct ieee80211_vif *vif,
>>  sgi = 1;
>>  rcu_read_unlock();
>>  
>> +rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x);
>> +
>>  priv->fops->update_rate_mask(priv, ramask, sgi);
>>  
>>  rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);

I believe this change only matters because you disable RXFLTMAP2
above. If we really have to write to RXFLTMAP2 to make this work, I
suspect we need to keep some sort of state information.

I would also be curious if RXFLTMAP2 gets reset somehow by the firmware,
and we do not account for that.

Cheers,
Jes



Re: Break-it testing for wifi

2016-11-21 Thread Mohammed Shafi Shajakhan
Hi Ben,

just googled out 'wifi fuzzy testing' and found something relevant
as below
https://www.blackhat.com/presentations/bh-europe-07/Butti/Presentation/bh-eu-07-Butti.pdf

regards,
shafi

On Mon, Nov 21, 2016 at 08:10:37AM -0800, Ben Greear wrote:
> Hello!
> 
> I am thinking about adding some sort of framework to wpa_supplicant and/or the
> mac80211 stack to allow purposefully creating bad station behaviour in order 
> to
> test robustness of APs.
> 
> Some ideas so far:
> 
> 1)  Allow supplicant to do bad state-machine transitions (start 4-way before 
> associating, for instance).
> 
> 2)  Randomly corrupt mgt frames in driver and/or mac80211 stack and/or 
> supplicant.
> 
> 3)  Possibly allow user to make specific corruptions.  This would probably be 
> in supplicant
> only, and I am not sure how this would be configured.  Maybe allow user 
> to over-ride
> existing IEs and add bogus ones of their own choosing.
> 
> 4)  Maybe some specific tests like putting in over-flow sized lengths of IEs.
> 
> Has anyone done anything similar they would like to share?
> 
> Johannes:  Any interest in having such a framework in upstream kernels?
> 
> Any other ideas for how to improve this feature set?
> 
> Thanks,
> Ben
> 
> -- 
> Ben Greear 
> Candela Technologies Inc  http://www.candelatech.com
> 


Re: [ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

2016-11-21 Thread Zefir Kurtisi
On 11/21/2016 04:10 PM, Michal Kazior wrote:
> On 21 November 2016 at 15:41, Zefir Kurtisi  wrote:
>> On 11/21/2016 03:04 PM, Benjamin Berg wrote:
>>> In the case that a spectral scan is enabled the PHY errors sent by the
>>> hardware as part of the scanning might trigger the radar detection and
>>> channels might be marked as 'unusable' incorrectly. This patch fixes
>>> the issue by preventing the spectral scan to be enabled if DFS is used
>>> and only analysing the PHY errors for DFS if radar detection is enabled.
>>>
>>> [...]
>>
>> From the relevant source code portion in channel.c:ath_set_channel()
>>
>> 80 /* Enable radar pulse detection if on a DFS channel. Spectral
>> 81  * scanning and radar detection can not be used concurrently.
>> 82  */
>> 83 if (hw->conf.radar_enabled) {
>> 84 u32 rxfilter;
>> 85
>> 86 rxfilter = ath9k_hw_getrxfilter(ah);
>> 87 rxfilter |= ATH9K_RX_FILTER_PHYRADAR |
>> 88 ATH9K_RX_FILTER_PHYERR;
>> 89 ath9k_hw_setrxfilter(ah, rxfilter);
>> 90 ath_dbg(common, DFS, "DFS enabled at freq %d\n",
>> 91 chan->center_freq);
>> 92 } else {
>> 93 /* perform spectral scan if requested. */
>> 94 if (test_bit(ATH_OP_SCANNING, >op_flags) &&
>> 95 sc->spec_priv.spectral_mode == SPECTRAL_CHANSCAN)
>> 96 ath9k_cmn_spectral_scan_trigger(common, 
>> >spec_priv);
>> 97 }
>>
>> it seems that spectral can't ever be activated while operating on a DFS 
>> channel.
>>
>> If you need to catch the opposite case, i.e. prevent feeding pseudo-radar 
>> pulses
>> into the pattern detector, you just need to ensure that they depend on
>> hw->conf.radar_enabled. A patch like the attached one should be enough.
> 
> Good point. I guess set_channel could be oversimplified as well. I
> mean, it makes sense to consider radar and spectral mutually exclusive
> if they use the same phyerr code. However some chips actually seem (as
> per the comment I mentioned) to distinguish the two so I don't know if
> the "mutually exclusive" is true for all chips per se. Just thinking
> out loud.
> 
You're right, blindly feeding PHYERR frames to spectral when spectral is not
enabled is as wrong as feeding them to the dfs-detector when not on a radar
channel. Therefore, the patch I proposed should be extended to make calling
ath_cmn_process_fft() depending on
a) not operating on radar channel, and
b) spectral scan is enabled

Updated patch attached as proposal.

> I also wonder if calling ieee80211_radar_detect() should have any
> effect if there are no radar operated interfaces in the first place?
> 
True, calling ieee80211_radar_detect() for a non-DFS channel is (or should be)
ignored by mac. But feeding the dfs-detector with invalid pulses causes quite 
some
computational overhead - dropping them if no radar detection is required is a 
good
thing to do.


Cheers,
Zefir




>From aaa28dcaf0879c6bfc7be96077c79894bb230dfb Mon Sep 17 00:00:00 2001
From: Zefir Kurtisi 
Date: Mon, 21 Nov 2016 15:33:45 +0100
Subject: [PATCH] ath9k: feed DFS detector only if operating on radar channel

---
 drivers/net/wireless/ath/ath9k/recv.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 6697342..48f8af1 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -867,10 +867,21 @@ static int ath9k_rx_skb_preprocess(struct ath_softc *sc,
 	 * can be dropped.
 	 */
 	if (rx_stats->rs_status & ATH9K_RXERR_PHY) {
-		ath9k_dfs_process_phyerr(sc, hdr, rx_stats, rx_status->mactime);
-		if (ath_cmn_process_fft(>spec_priv, hdr, rx_stats, rx_status->mactime))
+		/*
+		 * DFS and spectral are mutually exclusive
+		 *
+		 * Since some chips use RXERR_PHY as indication for both, we
+		 * need to double check which feature is enabled to prevent
+		 * feeding spectral or dfs-detector with wrong frames.
+		 */
+		if (hw->conf.radar_enabled) {
+			ath9k_dfs_process_phyerr(sc, hdr, rx_stats,
+		 rx_status->mactime);
+		} else if (sc->spec_priv.spectral_mode != SPECTRAL_DISABLED &&
+			   ath_cmn_process_fft(>spec_priv, hdr, rx_stats,
+	   rx_status->mactime)) {
 			RX_STAT_INC(rx_spectral);
-
+		}
 		return -EINVAL;
 	}
 
-- 
2.7.4



Break-it testing for wifi

2016-11-21 Thread Ben Greear

Hello!

I am thinking about adding some sort of framework to wpa_supplicant and/or the
mac80211 stack to allow purposefully creating bad station behaviour in order to
test robustness of APs.

Some ideas so far:

1)  Allow supplicant to do bad state-machine transitions (start 4-way before 
associating, for instance).

2)  Randomly corrupt mgt frames in driver and/or mac80211 stack and/or 
supplicant.

3)  Possibly allow user to make specific corruptions.  This would probably be 
in supplicant
only, and I am not sure how this would be configured.  Maybe allow user to 
over-ride
existing IEs and add bogus ones of their own choosing.

4)  Maybe some specific tests like putting in over-flow sized lengths of IEs.

Has anyone done anything similar they would like to share?

Johannes:  Any interest in having such a framework in upstream kernels?

Any other ideas for how to improve this feature set?

Thanks,
Ben

--
Ben Greear 
Candela Technologies Inc  http://www.candelatech.com



Re: wl1251 & mac address & calibration data

2016-11-21 Thread Pali Rohár
On Friday 11 November 2016 18:20:50 Pali Rohár wrote:
> Hi! I will open discussion about mac address and calibration data for 
> wl1251 wireless chip again...
> 
> Problem: Mac address & calibration data for wl1251 chip on Nokia N900 
> are stored on second nand partition (mtd1) in special proprietary format 
> which is used only for Nokia N900 (probably on N8x0 and N9 too). 
> Wireless driver wl1251.ko cannot work without mac address and 
> calibration data.
> 
> Absence of mac address cause that driver generates random mac address at 
> every kernel boot which has couple of problems (unstable identifier of 
> wireless device due to udev permanent storage rules; unpredictable 
> behaviour for dhcp mac address assignment, mac address filtering, ...).
> 
> Currently there is no way to set (permanent) mac address for network 
> interface from userspace. And it does not make sense to implement in 
> linux kernel large parser for proprietary format of second nand 
> partition where is mac address stored only for one device -- Nokia N900.
> 
> Driver wl1251.ko loads calibration data via request_firmware() for file 
> wl1251-nvs.bin. There are some "example" calibration file in linux-
> firmware repository, but it is not suitable for normal usage as real 
> calibration data are per-device specific.
> 
> So questions are:
> 
> 1) How to set mac address from userspace for that wl1251 interface? In 
> userspace I can write parser for that proprietary format of nand 
> partition and extract mac address from it

Proposed solutions for 1)

* Introduce new IOCL for setting that permanent mac address from
  userspace. Currently we have IOCL for get request

* Use request_firmware() (with flag from 2)) to ask for mac address from
  userspace. This is already used by wl12xx driver (as mac address is
  part of calibration data firmware file)

* Allow to set mac address via sysfs file, e.g.
  /sys/class/ieee80211/phy0/macaddress

> 2) How to send calibration data to wl1251 driver? Those are again stored 
> in proprietary format and I can write userspace parser for it.

Proposed solution for 2)

Introduce new flag for request_firmware(), so it first try to use
userspace helper for loading firmware file with possibility to fallback
to direct VFS access.


So... what do you think about it?

-- 
Pali Rohár
pali.ro...@gmail.com


Re: [ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

2016-11-21 Thread Michal Kazior
On 21 November 2016 at 15:41, Zefir Kurtisi  wrote:
> On 11/21/2016 03:04 PM, Benjamin Berg wrote:
>> In the case that a spectral scan is enabled the PHY errors sent by the
>> hardware as part of the scanning might trigger the radar detection and
>> channels might be marked as 'unusable' incorrectly. This patch fixes
>> the issue by preventing the spectral scan to be enabled if DFS is used
>> and only analysing the PHY errors for DFS if radar detection is enabled.
>>
>> [...]
>
> From the relevant source code portion in channel.c:ath_set_channel()
>
> 80 /* Enable radar pulse detection if on a DFS channel. Spectral
> 81  * scanning and radar detection can not be used concurrently.
> 82  */
> 83 if (hw->conf.radar_enabled) {
> 84 u32 rxfilter;
> 85
> 86 rxfilter = ath9k_hw_getrxfilter(ah);
> 87 rxfilter |= ATH9K_RX_FILTER_PHYRADAR |
> 88 ATH9K_RX_FILTER_PHYERR;
> 89 ath9k_hw_setrxfilter(ah, rxfilter);
> 90 ath_dbg(common, DFS, "DFS enabled at freq %d\n",
> 91 chan->center_freq);
> 92 } else {
> 93 /* perform spectral scan if requested. */
> 94 if (test_bit(ATH_OP_SCANNING, >op_flags) &&
> 95 sc->spec_priv.spectral_mode == SPECTRAL_CHANSCAN)
> 96 ath9k_cmn_spectral_scan_trigger(common, 
> >spec_priv);
> 97 }
>
> it seems that spectral can't ever be activated while operating on a DFS 
> channel.
>
> If you need to catch the opposite case, i.e. prevent feeding pseudo-radar 
> pulses
> into the pattern detector, you just need to ensure that they depend on
> hw->conf.radar_enabled. A patch like the attached one should be enough.

Good point. I guess set_channel could be oversimplified as well. I
mean, it makes sense to consider radar and spectral mutually exclusive
if they use the same phyerr code. However some chips actually seem (as
per the comment I mentioned) to distinguish the two so I don't know if
the "mutually exclusive" is true for all chips per se. Just thinking
out loud.

I also wonder if calling ieee80211_radar_detect() should have any
effect if there are no radar operated interfaces in the first place?


Michał


Re: scheduled scan interval

2016-11-21 Thread Luca Coelho
Hi Arend,

On Mon, 2016-11-21 at 13:03 +0100, Arend Van Spriel wrote:
> On 21-11-2016 12:30, Arend Van Spriel wrote:
> > On 21-11-2016 12:19, Arend Van Spriel wrote:
> > > Hi Johannes, Luca,
> > > 
> > > The gscan work made me look at scheduled scan and the implementation of
> > > it in brcmfmac. The driver ignored the interval parameter from
> > > user-space. Now I am fixing that. One thing is that our firmware has a
> > > minimum interval which can not be indicated in struct wiphy. The other
> > > issue is how the maximum interval is used in the nl80211.c.
> > > 
> > > In nl80211_parse_sched_scan_plans() it is used against value passed in
> > > NL80211_ATTR_SCHED_SCAN_INTERVAL and NL80211_SCHED_SCAN_PLAN_INTERVAL.
> > > For the first one it caps the value to the maximum, but for the second
> > > one it returns -EINVAL. I suspect this is done because maximum interval
> > > was introduced with schedule scan plans, but it feels inconsistent.
> > 
> > It also maybe simply wrong to cap. At least brcmfmac does not set the
> > maximum so it will always get interval being zero. Maybe better to do:
> > 
> > if (wiphy->max_sched_scan_plan_interval &&
> > request->scan_plans[0].interval >
> > wiphy->max_sched_scan_plan_interval)
> > return -EINVAL;
> > 
> > > Thoughts?
> 
> Digging deeper. Looking at v4.3 before introduction of sched_scan_plans.
> struct sched_scan_request::interval was specified in milliseconds! Below
> the drivers that I see having scheduled scan support:
> 
> iwlmvm: cap interval, convert to seconds.
> ath6kl: cap to 1sec minimum, no max check, convert to seconds.
> wl12xx: no checking in driver, fw need milliseconds.
> wl18xx: no checking in driver, fw need milliseconds.
> 
> The milliseconds conversion seems to be taken care of by multiplying
> with MSEC_PER_SEC in wl{12,18}xx drivers.
> 
> It seems in 4.8 only iwlmvm set wiphy->max_sched_scan_plan_interval so
> other drivers will get interval of zero which only ath6kl handles.

With the introduction of scheduled scan plans, we sort of deprecated
the "generic" scheduled scan interval.  It doesn't make sense to have
both passed at the same time, so nl80211 forbids
NL80211_ATTR_SCHED_SCAN_INTERVAL if we pass
NL80211_ATTR_SCHED_SCAN_PLANS.

The original NL80211_ATTR_SCHED_SCAN_INTERVAL was specified in msecs,
which is silly because we can never get millisecond accuracy in this. 
Thus, in the plans API, we decided to use seconds instead (because it
makes much more sense).  Additionally, the interval is considered
"advisory", because the FW may not be able guarantee the exact
intervals (for instance, the iwlwifi driver actually starts the
interval timer after scan completion, so if you specify 10 seconds
intervals, in practice they'll be 13-14 seconds).

I'm not sure I'm answering your question, because I'm also not sure I
understood the question. :)

--
Cheers,
Luca.


Re: [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

2016-11-21 Thread Zefir Kurtisi
On 11/21/2016 03:04 PM, Benjamin Berg wrote:
> In the case that a spectral scan is enabled the PHY errors sent by the
> hardware as part of the scanning might trigger the radar detection and
> channels might be marked as 'unusable' incorrectly. This patch fixes
> the issue by preventing the spectral scan to be enabled if DFS is used
> and only analysing the PHY errors for DFS if radar detection is enabled.
> 
> [...]

>From the relevant source code portion in channel.c:ath_set_channel()

80 /* Enable radar pulse detection if on a DFS channel. Spectral
81  * scanning and radar detection can not be used concurrently.
82  */
83 if (hw->conf.radar_enabled) {
84 u32 rxfilter;
85
86 rxfilter = ath9k_hw_getrxfilter(ah);
87 rxfilter |= ATH9K_RX_FILTER_PHYRADAR |
88 ATH9K_RX_FILTER_PHYERR;
89 ath9k_hw_setrxfilter(ah, rxfilter);
90 ath_dbg(common, DFS, "DFS enabled at freq %d\n",
91 chan->center_freq);
92 } else {
93 /* perform spectral scan if requested. */
94 if (test_bit(ATH_OP_SCANNING, >op_flags) &&
95 sc->spec_priv.spectral_mode == SPECTRAL_CHANSCAN)
96 ath9k_cmn_spectral_scan_trigger(common, 
>spec_priv);
97 }

it seems that spectral can't ever be activated while operating on a DFS channel.

If you need to catch the opposite case, i.e. prevent feeding pseudo-radar pulses
into the pattern detector, you just need to ensure that they depend on
hw->conf.radar_enabled. A patch like the attached one should be enough.


Cheers,
Zefir
>From c24edf82e1f509490ba9dd3e34eec3ac3b309321 Mon Sep 17 00:00:00 2001
From: Zefir Kurtisi 
Date: Mon, 21 Nov 2016 15:33:45 +0100
Subject: [PATCH] ath9k: feed DFS detector only if operating on radar channel

---
 drivers/net/wireless/ath/ath9k/recv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 6697342..e4701a7 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -867,7 +867,8 @@ static int ath9k_rx_skb_preprocess(struct ath_softc *sc,
 	 * can be dropped.
 	 */
 	if (rx_stats->rs_status & ATH9K_RXERR_PHY) {
-		ath9k_dfs_process_phyerr(sc, hdr, rx_stats, rx_status->mactime);
+		if (hw->conf.radar_enabled)
+			ath9k_dfs_process_phyerr(sc, hdr, rx_stats, rx_status->mactime);
 		if (ath_cmn_process_fft(>spec_priv, hdr, rx_stats, rx_status->mactime))
 			RX_STAT_INC(rx_spectral);
 
-- 
2.7.4



Re: [PATCH] nl80211: fix logic inversion in start_nan()

2016-11-21 Thread Arend Van Spriel
On 21-11-2016 13:57, Arend Van Spriel wrote:
> On 21-11-2016 13:55, Johannes Berg wrote:
>> From: Johannes Berg 
>>
>> Arend inadvertedly inverted the logic while converting to
>> wdev_running(), fix that.
> 
> It was indeed inadvertedly.

Actually spelling checker complains. Should be 'inadvertently'.

Regards,
Arend

> Thanks,
> Arend
> 
>> Fixes: 73c7da3dae1e ("cfg80211: add generic helper to check interface is 
>> running")
>> Signed-off-by: Johannes Berg 
>> ---
>>  net/wireless/nl80211.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>> index 4e9bda49e957..e4f718e956d7 100644
>> --- a/net/wireless/nl80211.c
>> +++ b/net/wireless/nl80211.c
>> @@ -10625,7 +10625,7 @@ static int nl80211_start_nan(struct sk_buff *skb, 
>> struct genl_info *info)
>>  if (wdev->iftype != NL80211_IFTYPE_NAN)
>>  return -EOPNOTSUPP;
>>  
>> -if (!wdev_running(wdev))
>> +if (wdev_running(wdev))
>>  return -EEXIST;
>>  
>>  if (rfkill_blocked(rdev->rfkill))
>>


Re: [PATCH] nl80211: fix logic inversion in start_nan()

2016-11-21 Thread Arend Van Spriel
On 21-11-2016 13:55, Johannes Berg wrote:
> From: Johannes Berg 
> 
> Arend inadvertedly inverted the logic while converting to
> wdev_running(), fix that.

It was indeed inadvertedly.

Thanks,
Arend

> Fixes: 73c7da3dae1e ("cfg80211: add generic helper to check interface is 
> running")
> Signed-off-by: Johannes Berg 
> ---
>  net/wireless/nl80211.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 4e9bda49e957..e4f718e956d7 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -10625,7 +10625,7 @@ static int nl80211_start_nan(struct sk_buff *skb, 
> struct genl_info *info)
>   if (wdev->iftype != NL80211_IFTYPE_NAN)
>   return -EOPNOTSUPP;
>  
> - if (!wdev_running(wdev))
> + if (wdev_running(wdev))
>   return -EEXIST;
>  
>   if (rfkill_blocked(rdev->rfkill))
> 


[PATCH] nl80211: fix logic inversion in start_nan()

2016-11-21 Thread Johannes Berg
From: Johannes Berg 

Arend inadvertedly inverted the logic while converting to
wdev_running(), fix that.

Fixes: 73c7da3dae1e ("cfg80211: add generic helper to check interface is 
running")
Signed-off-by: Johannes Berg 
---
 net/wireless/nl80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 4e9bda49e957..e4f718e956d7 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -10625,7 +10625,7 @@ static int nl80211_start_nan(struct sk_buff *skb, 
struct genl_info *info)
if (wdev->iftype != NL80211_IFTYPE_NAN)
return -EOPNOTSUPP;
 
-   if (!wdev_running(wdev))
+   if (wdev_running(wdev))
return -EEXIST;
 
if (rfkill_blocked(rdev->rfkill))
-- 
2.9.3



Re: ath10k stuck in mesh mode

2016-11-21 Thread Matteo Grandi
Yes, I noticed it, in fact it seems to be the reg. domain of the ath9k
(there is also an ath9k card plugged on the same board).
But it is in contrast with what I found in the syslog:

Nov 21 07:53:23 MrProper kernel: [9.591563] ath: Regpair used: 0x3a
Nov 21 07:53:23 MrProper kernel: [9.591573] cfg80211: Updating
information on frequency 5180 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.591581] cfg80211: (514 KHz
- 536 KHz @ 8 KHz), (N/A, 3000 mBm)
[...]
- 536 KHz @ 8 KHz), (N/A, 3000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.591654] cfg80211: Updating
information on frequency 5300 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.591661] cfg80211: (514 KHz
- 536 KHz @ 8 KHz), (N/A, 3000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.591668] cfg80211: Updating
information on frequency 5320 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.591675] cfg80211: (514 KHz
- 536 KHz @ 8 KHz), (N/A, 3000 mBm

where there are @80MHz frequency slots.

Is it possible that the system, showing all these different reg.
domains from different cards, chose the most constrained one?

2016-11-21 11:53 GMT+01:00 Michal Kazior :
> On 21 November 2016 at 10:46, Matteo Grandi  wrote:
>> Dear Bob, Michal, all,
>>
>> I've just tried your advices (actually I already tried it following
>> the wireless.wiki.kernel web pages) and I had a look at the syslog
>> while I was typing the commands
>> At the beginning I have this
> [...]
>> Other (hopefully) useful info:
>> root@MrProper:~# iw reg get
>> global
>> country US: DFS-UNSET
>> (2402 - 2472 @ 40), (3, 27), (N/A)
>> (5170 - 5250 @ 40), (3, 17), (N/A)
>> (5250 - 5330 @ 40), (3, 20), (0 ms), DFS
>> (5490 - 5600 @ 40), (3, 20), (0 ms), DFS
>> (5650 - 5710 @ 40), (3, 20), (0 ms), DFS
>> (5735 - 5835 @ 40), (3, 30), (N/A)
>> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>>
>> phy#2
>> country US: DFS-UNSET
>> (2402 - 2472 @ 40), (3, 27), (N/A)
>> (5170 - 5250 @ 40), (3, 17), (N/A)
>> (5250 - 5330 @ 40), (3, 20), (0 ms), DFS
>> (5490 - 5600 @ 40), (3, 20), (0 ms), DFS
>> (5650 - 5710 @ 40), (3, 20), (0 ms), DFS
>> (5735 - 5835 @ 40), (3, 30), (N/A)
>> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>>
>> phy#0
>> country US: DFS-UNSET
>> (2402 - 2472 @ 40), (3, 27), (N/A)
>> (5170 - 5250 @ 40), (3, 17), (N/A)
>> (5250 - 5330 @ 40), (3, 20), (0 ms), DFS
>> (5490 - 5600 @ 40), (3, 20), (0 ms), DFS
>> (5650 - 5710 @ 40), (3, 20), (0 ms), DFS
>> (5735 - 5835 @ 40), (3, 30), (N/A)
>> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>>
>> phy#1
>> country US: DFS-UNSET
>> (2402 - 2472 @ 40), (3, 27), (N/A)
>> (5170 - 5250 @ 40), (3, 17), (N/A)
>> (5250 - 5330 @ 40), (3, 20), (0 ms), DFS
>> (5490 - 5600 @ 40), (3, 20), (0 ms), DFS
>> (5650 - 5710 @ 40), (3, 20), (0 ms), DFS
>> (5735 - 5835 @ 40), (3, 30), (N/A)
>> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>>[...]
>>
>> Checking on Internet I didn't find a working solution, and the data
>> rate stucks to 120Mbps MCS7.
>> For me it still a mistery, I hope in your help.
>
> Note the "@ 40" for all frequency ranges (except 60GHz band). Your
> regulatory database seems to be limiting you to 40 MHz (it's probably
> very old/ out of date). You'll need to update it to be able to use 80
> MHz.
>
>
> Michal


Re: [PATCH v7 1/2] nl80211: multicast_to_unicast can be changed while IFF_UP

2016-11-21 Thread Johannes Berg
On Mon, 2016-10-31 at 14:40 +0100, Michael Braun wrote:
> There is no need to prevent toggling multicast_to_unicast while
> interface is already up. This change simplifies reconfiguration
> from hostapd.

Applied. This check never should've been there anyway, if desired,
NEED_NETDEV_UP should've been used.

johannes


Re: [PATCH 2/2] nl80211: check NL80211_ATTR_SCHED_SCAN_INTERVAL only once

2016-11-21 Thread Johannes Berg
On Thu, 2016-11-17 at 09:02 +, Arend van Spriel wrote:
> The presence of the NL80211_ATTR_SCHED_SCAN_INTERVAL attribute was
> checked in nl80211_parse_sched_scan() and
> nl80211_parse_sched_scan_plans() which might be a bit redundant
> so removing one.
> 
makes sense, applied.

johannes


Re: [PATCH V2] cfg80211: get rid of name indirection trick for ieee80211_get_channel()

2016-11-21 Thread Johannes Berg
On Thu, 2016-11-17 at 12:48 +, Arend van Spriel wrote:
> The comment on the name indirection suggested an issue but turned out
> to be untrue. Digging in older kernel version showed issue with
> ipw2x00
> but that is no longer true so get rid on the name indirection.
> 
Applied, thanks.

johannes


Re: scheduled scan interval

2016-11-21 Thread Arend Van Spriel
On 21-11-2016 12:19, Arend Van Spriel wrote:
> Hi Johannes, Luca,
> 
> The gscan work made me look at scheduled scan and the implementation of
> it in brcmfmac. The driver ignored the interval parameter from
> user-space. Now I am fixing that. One thing is that our firmware has a
> minimum interval which can not be indicated in struct wiphy. The other
> issue is how the maximum interval is used in the nl80211.c.
> 
> In nl80211_parse_sched_scan_plans() it is used against value passed in
> NL80211_ATTR_SCHED_SCAN_INTERVAL and NL80211_SCHED_SCAN_PLAN_INTERVAL.
> For the first one it caps the value to the maximum, but for the second
> one it returns -EINVAL. I suspect this is done because maximum interval
> was introduced with schedule scan plans, but it feels inconsistent.

It also maybe simply wrong to cap. At least brcmfmac does not set the
maximum so it will always get interval being zero. Maybe better to do:

if (wiphy->max_sched_scan_plan_interval &&
request->scan_plans[0].interval >
wiphy->max_sched_scan_plan_interval)
return -EINVAL;

> Thoughts?
> 
> Regards,
> Arend
> 


scheduled scan interval

2016-11-21 Thread Arend Van Spriel
Hi Johannes, Luca,

The gscan work made me look at scheduled scan and the implementation of
it in brcmfmac. The driver ignored the interval parameter from
user-space. Now I am fixing that. One thing is that our firmware has a
minimum interval which can not be indicated in struct wiphy. The other
issue is how the maximum interval is used in the nl80211.c.

In nl80211_parse_sched_scan_plans() it is used against value passed in
NL80211_ATTR_SCHED_SCAN_INTERVAL and NL80211_SCHED_SCAN_PLAN_INTERVAL.
For the first one it caps the value to the maximum, but for the second
one it returns -EINVAL. I suspect this is done because maximum interval
was introduced with schedule scan plans, but it feels inconsistent.

Thoughts?

Regards,
Arend


Re: ath10k stuck in mesh mode

2016-11-21 Thread Michal Kazior
On 21 November 2016 at 10:46, Matteo Grandi  wrote:
> Dear Bob, Michal, all,
>
> I've just tried your advices (actually I already tried it following
> the wireless.wiki.kernel web pages) and I had a look at the syslog
> while I was typing the commands
> At the beginning I have this
[...]
> Other (hopefully) useful info:
> root@MrProper:~# iw reg get
> global
> country US: DFS-UNSET
> (2402 - 2472 @ 40), (3, 27), (N/A)
> (5170 - 5250 @ 40), (3, 17), (N/A)
> (5250 - 5330 @ 40), (3, 20), (0 ms), DFS
> (5490 - 5600 @ 40), (3, 20), (0 ms), DFS
> (5650 - 5710 @ 40), (3, 20), (0 ms), DFS
> (5735 - 5835 @ 40), (3, 30), (N/A)
> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>
> phy#2
> country US: DFS-UNSET
> (2402 - 2472 @ 40), (3, 27), (N/A)
> (5170 - 5250 @ 40), (3, 17), (N/A)
> (5250 - 5330 @ 40), (3, 20), (0 ms), DFS
> (5490 - 5600 @ 40), (3, 20), (0 ms), DFS
> (5650 - 5710 @ 40), (3, 20), (0 ms), DFS
> (5735 - 5835 @ 40), (3, 30), (N/A)
> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>
> phy#0
> country US: DFS-UNSET
> (2402 - 2472 @ 40), (3, 27), (N/A)
> (5170 - 5250 @ 40), (3, 17), (N/A)
> (5250 - 5330 @ 40), (3, 20), (0 ms), DFS
> (5490 - 5600 @ 40), (3, 20), (0 ms), DFS
> (5650 - 5710 @ 40), (3, 20), (0 ms), DFS
> (5735 - 5835 @ 40), (3, 30), (N/A)
> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>
> phy#1
> country US: DFS-UNSET
> (2402 - 2472 @ 40), (3, 27), (N/A)
> (5170 - 5250 @ 40), (3, 17), (N/A)
> (5250 - 5330 @ 40), (3, 20), (0 ms), DFS
> (5490 - 5600 @ 40), (3, 20), (0 ms), DFS
> (5650 - 5710 @ 40), (3, 20), (0 ms), DFS
> (5735 - 5835 @ 40), (3, 30), (N/A)
> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>[...]
>
> Checking on Internet I didn't find a working solution, and the data
> rate stucks to 120Mbps MCS7.
> For me it still a mistery, I hope in your help.

Note the "@ 40" for all frequency ranges (except 60GHz band). Your
regulatory database seems to be limiting you to 40 MHz (it's probably
very old/ out of date). You'll need to update it to be able to use 80
MHz.


Michal


Re: ath10k stuck in mesh mode

2016-11-21 Thread Matteo Grandi
Dear Bob, Michal, all,

I've just tried your advices (actually I already tried it following
the wireless.wiki.kernel web pages) and I had a look at the syslog
while I was typing the commands
At the beginning I have this
root@MrProper:~# tail -f -n 200 /var/log/syslog
Nov 21 07:53:23 MrProper kernel: [9.121882] cfg80211: Updating
information on frequency 5260 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.121891] cfg80211: (525 KHz
- 533 KHz @ 4 KHz), (300 mBi, 2000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.121900] cfg80211: Updating
information on frequency 5280 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.121906] cfg80211: (525 KHz
- 533 KHz @ 4 KHz), (300 mBi, 2000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.121913] cfg80211: Updating
information on frequency 5300 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.121923] cfg80211: (525 KHz
- 533 KHz @ 4 KHz), (300 mBi, 2000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.121931] cfg80211: Updating
information on frequency 5320 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.121937] cfg80211: (525 KHz
- 533 KHz @ 4 KHz), (300 mBi, 2000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.121946] cfg80211: Updating
information on frequency 5500 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.121954] cfg80211: (549 KHz
- 560 KHz @ 4 KHz), (300 mBi, 2000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.121963] cfg80211: Updating
information on frequency 5520 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.121972] cfg80211: (549 KHz
- 560 KHz @ 4 KHz), (300 mBi, 2000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.121982] cfg80211: Updating
information on frequency 5540 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.121990] cfg80211: (549 KHz
- 560 KHz @ 4 KHz), (300 mBi, 2000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.121997] cfg80211: Updating
information on frequency 5560 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.122004] cfg80211: (549 KHz
- 560 KHz @ 4 KHz), (300 mBi, 2000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.122012] cfg80211: Updating
information on frequency 5580 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.122021] cfg80211: (549 KHz
- 560 KHz @ 4 KHz), (300 mBi, 2000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.122029] cfg80211: Disabling
freq 5600 MHz for good
Nov 21 07:53:23 MrProper kernel: [9.122038] cfg80211: Disabling
freq 5620 MHz for good
Nov 21 07:53:23 MrProper kernel: [9.122044] cfg80211: Disabling
freq 5640 MHz for good
Nov 21 07:53:23 MrProper kernel: [9.122051] cfg80211: Updating
information on frequency 5660 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.122058] cfg80211: (565 KHz
- 571 KHz @ 4 KHz), (300 mBi, 2000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.122068] cfg80211: Updating
information on frequency 5680 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.122077] cfg80211: (565 KHz
- 571 KHz @ 4 KHz), (300 mBi, 2000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.122085] cfg80211: Updating
information on frequency 5700 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.122094] cfg80211: (565 KHz
- 571 KHz @ 4 KHz), (300 mBi, 2000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.122099] cfg80211: Disabling
freq 5720 MHz for good
Nov 21 07:53:23 MrProper kernel: [9.122110] cfg80211: Updating
information on frequency 5745 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.122118] cfg80211: (5735000 KHz
- 5835000 KHz @ 4 KHz), (300 mBi, 3000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.122125] cfg80211: Updating
information on frequency 5765 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.122133] cfg80211: (5735000 KHz
- 5835000 KHz @ 4 KHz), (300 mBi, 3000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.122141] cfg80211: Updating
information on frequency 5785 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.122147] cfg80211: (5735000 KHz
- 5835000 KHz @ 4 KHz), (300 mBi, 3000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.122156] cfg80211: Updating
information on frequency 5805 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.122166] cfg80211: (5735000 KHz
- 5835000 KHz @ 4 KHz), (300 mBi, 3000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.122175] cfg80211: Updating
information on frequency 5825 MHz with regulatory rule:
Nov 21 07:53:23 MrProper kernel: [9.122184] cfg80211: (5735000 KHz
- 5835000 KHz @ 4 KHz), (300 mBi, 3000 mBm)
Nov 21 07:53:23 MrProper kernel: [9.487186] ath10k_pci
:07:00.0: qca988x hw2.0 (0x4100016c, 0x043222ff sub :) fw
10.2.4.70.58 fwapi 5 bdapi 1 htt-ver 2.1 wmi-op 5 htt-op 2 cal otp
max-sta 128 raw 1 hwcrypto 1 features no-p2p,raw-mode

Re: BCM4356 + Ubuntu 16.10

2016-11-21 Thread Arend Van Spriel
+ linux-wireless

On 20-11-2016 14:26, Familie Stepniak wrote:
> Hi,
> 
> I try run Ubuntu 16.10 on the Trekstor W2 Tablet but Wi-Fi donts work..
> that find WLAN-Network, I give to Passwort and Nothing.. the chip ist
>  BCM4356B.. please Help me to install the true Driver on the Linux-Ubuntu..

Not sure what "true Driver" means, but let me see if I can help. First
see if I understand your issue. So you were able to select to WLAN
network and got a pop-up to provide the password. After that you are not
getting a connection. Is this right?

So is this a first attempt to run Ubuntu on it or have you been able to
run older version.

Can you do the following:
- open a terminal.
- run following commands (assuming wlan0):
$ iw dev
$ iw list
$ lspci
$ sudo nmcli nm wifi off
$ sudo rfkill unblock wifi
$ sudo ifconfig wlan0 up
$ sudo iw wlan0 scan
$ dmesg
- send me output of all these commands.

Regards,
Arend


Re: [ath9k-devel] [RFC v2 2/2] ath9k: Reset chip on potential deaf state

2016-11-21 Thread Ferry Huberts



On 21/11/16 10:10, Sven Eckelmann wrote:

On Montag, 21. November 2016 10:07:43 CET Ferry Huberts wrote:
[...]

v2:
 - reduce amount of possible goto-raptor attacks by one (thanks Kalle Valo)

This problem was discovered in mesh setups. It was noticed that some nodes



What kind of setup?
Using 802.11s?


Unencrypted IBSS.



ok, thanks. that is different then.

I _can_ tell you that using the high priority queue (EF class traffic) 
seems to somehow 'unwedge' the chip during/after rekeying. Still have to 
verify this again, but that is what I saw last week.


Re: [ath9k-devel] [RFC v2 2/2] ath9k: Reset chip on potential deaf state

2016-11-21 Thread Ferry Huberts



On 17/11/16 09:36, Sven Eckelmann wrote:

From: Simon Wunderlich 

The chip is switching seemingly random into a state which can be described
as "deaf". No or nearly no interrupts are generated anymore for incoming
packets. Existing links either break down after a while and new links will
not be established.

The driver doesn't know if there is no other device available or if it
ended up in an "deaf" state. Resetting the chip proactively avoids
permanent problems in case the chip really was in its "deaf" state but
maybe causes unnecessary resets in case it wasn't "deaf".

Signed-off-by: Simon Wunderlich 
[sven.eckelm...@open-mesh.com: port to recent ath9k, add commit message]
Signed-off-by: Sven Eckelmann 
---
v2:
 - reduce amount of possible goto-raptor attacks by one (thanks Kalle Valo)

This problem was discovered in mesh setups. It was noticed that some nodes



What kind of setup?
Using 802.11s?

I ask this because I have almost completed a patch for authsae that 
checks rekey.


The problems there might show as the behaviour described here.



were not able to see their neighbors (mostly after running for a while) -
even when those neighbors received data from them via IBSS. A simple `iw
dev wlan0 scan` fixed the problem for them. But the problem seems to
reappear after while(tm) in a large enough(tm) mesh.

This patch is a little bit obscure because it requires CONFIG_ATH9K_DEBUGFS
to actually work. But there still seems to be potential interest in
Freifunk communities or Freifunk meta-projects (e.g. freifunk-gluon). It is
currently not known if it helps them but publishing this to allow them to
test and play around with it will not hurt :)
---
 drivers/net/wireless/ath/ath9k/ath9k.h |  3 +++
 drivers/net/wireless/ath/ath9k/debug.c |  1 +
 drivers/net/wireless/ath/ath9k/debug.h |  1 +
 drivers/net/wireless/ath/ath9k/link.c  | 48 +-
 4 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h 
b/drivers/net/wireless/ath/ath9k/ath9k.h
index 9c6fee7..3987ad5 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -996,6 +996,9 @@ struct ath_softc {
short nbcnvifs;
unsigned long ps_usecount;

+   unsigned long last_check_time;
+   u32 last_check_interrupts;
+
struct ath_rx rx;
struct ath_tx tx;
struct ath_beacon beacon;
diff --git a/drivers/net/wireless/ath/ath9k/debug.c 
b/drivers/net/wireless/ath/ath9k/debug.c
index 608b370..6d5c253 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -768,6 +768,7 @@ static int read_file_reset(struct seq_file *file, void 
*data)
[RESET_TX_DMA_ERROR] = "Tx DMA stop error",
[RESET_RX_DMA_ERROR] = "Rx DMA stop error",
[RESET_TYPE_DEADBEEF] = "deadbeef hang",
+   [RESET_TYPE_DEAF] = "deaf hang",
};
int i;

diff --git a/drivers/net/wireless/ath/ath9k/debug.h 
b/drivers/net/wireless/ath/ath9k/debug.h
index 0d77abbf6..6f186bd 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -53,6 +53,7 @@ enum ath_reset_type {
RESET_TX_DMA_ERROR,
RESET_RX_DMA_ERROR,
RESET_TYPE_DEADBEEF,
+   RESET_TYPE_DEAF,
__RESET_TYPE_MAX
 };

diff --git a/drivers/net/wireless/ath/ath9k/link.c 
b/drivers/net/wireless/ath/ath9k/link.c
index 04195d5..ae99c02 100644
--- a/drivers/net/wireless/ath/ath9k/link.c
+++ b/drivers/net/wireless/ath/ath9k/link.c
@@ -158,13 +158,59 @@ static bool ath_hw_hang_deadbeef(struct ath_softc *sc)
return true;
 }

+static bool ath_hw_hang_deaf(struct ath_softc *sc)
+{
+#ifndef CONFIG_ATH9K_DEBUGFS
+   return false;
+#else
+   struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+   u32 interrupts, interrupt_per_s;
+   unsigned int interval;
+
+   /* get historic data */
+   interval = jiffies_to_msecs(jiffies - sc->last_check_time);
+   if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_EDMA)
+   interrupts = sc->debug.stats.istats.rxlp;
+   else
+   interrupts = sc->debug.stats.istats.rxok;
+
+   interrupts -= sc->last_check_interrupts;
+
+   /* save current data */
+   sc->last_check_time = jiffies;
+   if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_EDMA)
+   sc->last_check_interrupts = sc->debug.stats.istats.rxlp;
+   else
+   sc->last_check_interrupts = sc->debug.stats.istats.rxok;
+
+   /* sanity check, should be 30 seconds */
+   if (interval > 4 || interval < 2)
+   return false;
+
+   /* should be at least one interrupt per second */
+   interrupt_per_s = interrupts / (interval / 1000);
+   if (interrupt_per_s >= 1)
+   return false;
+
+   

form factor subsystem (Re: [PATCH] cfg80211/nl80211: add wifi tx power mode switching support)

2016-11-21 Thread Johannes Berg
Hi,

I'm revisiting this since we're asked to do the same for iwlwifi.

On Thu, 2016-05-05 at 14:44 +0800, Wei-Ning Huang wrote:
> Recent new hardware has the ability to switch between tablet mode and
> clamshell mode. To optimize WiFi performance, we want to be able to
> use different power table between modes. This patch adds a new
> netlink message type and cfg80211_ops function to allow userspace to
> trigger a power mode switch for a given wireless interface.

I've thought about this a bit more, and also heard this in different
contexts now, and I'm actually not convinced that the wifi subsystem
exposing this *in any way* is the right thing to do.

Why don't we add a minimal "form factor" subsystem to the kernel?
Something that allows

 1) sensor/BIOS/... drivers to call a function to switch form factor
 2) consumers to register and get callbacks when switching happens

If the sensor driver is in the kernel (some kind of driver probably has
to be anyway), or form factor switch ends up being a BIOS notification,
then this gets rid of a lot of complexity - no more need to bump this
through userspace etc.

If, for some reason, the sensor driver really has to be in userspace,
then some kind of API *to this subsystem* can be implemented to set the
form factor mode from userspace, and all the other stuff can be left as
is.

This would also allow other clients to know about this information,
let's say, for the sake of an argument that doesn't seem *too* far-
fetched, that the compass driver needs to adjust the direction if you
switch modes.

It seems to me that this would be more robust, which can only be a good
thing if we start using it for things that might be regulatory
relevant.

Additionally, the subsystem could allow userspace to also get an event
if it wants to know about this, e.g. to switch to a more touch-friendly 
UI on switching to tablet mode, or something.

Thoughts?

johannes