Re: [PATCH] mac80211: Remove unused 'struct rate_control_ref' variable
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
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
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 BergCc: 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
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 BergCc: 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
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
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 BergCc: 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
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
On 21-11-2016 17:59, Dave Taht wrote: > On Mon, Nov 21, 2016 at 7:08 AM, Luca Coelhowrote: >> 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
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
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
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
Barry Daywrites: > 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
On Mon, Nov 21, 2016 at 7:08 AM, Luca Coelhowrote: > 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
Jes Sorensenwrites: > 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
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
On 11/21/2016 04:10 PM, Michal Kazior wrote: > On 21 November 2016 at 15:41, Zefir Kurtisiwrote: >> 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
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 GreearCandela Technologies Inc http://www.candelatech.com
Re: wl1251 & mac address & calibration data
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
On 21 November 2016 at 15:41, Zefir Kurtisiwrote: > 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
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
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 KurtisiDate: 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()
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()
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()
From: Johannes BergArend 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
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
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
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()
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
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
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
On 21 November 2016 at 10:46, Matteo Grandiwrote: > 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
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
+ 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
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
On 17/11/16 09:36, Sven Eckelmann wrote: From: Simon WunderlichThe 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)
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