Re: [PATCH 00/58] networking: Convert timers to use timer_setup()

[Kalle Valo]

Kees Cook  writes:

> On Tue, Oct 17, 2017 at 7:18 AM, Kalle Valo  wrote:
>> + linux-wireless
>>
>> Hi Kees,
>>
>> Kees Cook  writes:
>>
>>> This is the current set of outstanding networking patches to perform
>>> conversions to the new timer interface (rebased to -next). This is not
>>> all expected conversions, but it contains everything needed in networking
>>> to eliminate init_timer(), and all the non-standard setup_*_timer() uses.
>>
>> So this also includes patches which I had queued for
>> wireless-drivers-next:
>>
>> https://patchwork.kernel.org/patch/9986253/
>> https://patchwork.kernel.org/patch/9986245/
>>
>> And looking at patchwork[1] I have even more timer_setup() related
>> patches from you. It would be really helpful if you could clearly
>> document to which tree you want the patches to be applied. I don't care
>
> Hi! Sorry about that. It's been a bit tricky to juggle everything.

Yeah, I understand.

>> if it's net-next or wireless-drivers-next as long as it's not the both
>> (meaning that both Dave and me apply the same patch, which would be
>> bad). The thing is that I really do not have time to figure out for
>> every patch via which tree it's supposed to go.
>
> Which split is preferred? I had been trying to separate wireless from
> the rest of net (but missed some cases).

So what we try to follow is that I apply all patches for
drivers/net/wireless to my wireless-drivers trees, with exception of
Johannes taking mac80211_hwsim.c patches to his mac80211 trees. And
Johannes of course takes all patches for net/wireless and net/mac80211.

So in general I prefer that I take all drivers/net/wireless patches and
make it obvious for Dave that he can ignore those patches (not mix
wireless-drivers and net patches into same set etc). But like I said,
it's ok to push API changes like these via Dave's net trees as well if
you want (and if Dave is ok with that). The chances of conflicts is low,
and if there are be any those would be easy to fix either by me or Dave.

>> For now I'll just drop all your timer_setup() related patches from my
>> queue and I'll assume Dave will take those. Ok?
>>
>> [1] https://patchwork.kernel.org/project/linux-wireless/list/
>
> I guess I'll wait to see what Dave says.

Ok, I don't drop the patches from my queue quite yet then.

-- 
Kalle Valo

Re: Two rtlwifi drivers?

[Kalle Valo]

Pkshih  writes:

>> My recommendation is to avoid accumulating patches at all cost and start
>> submitting them as soon as you can. This way you get patches committed
>> much more smoother. So do not wait until _all_ patches are ready,
>> instead start submitting patches as soon as you have _some_ patches
>> ready. In other words, keep the delta between mainline and your
>> not-yet-submitted patches as small as possible.
>> 
>> And the patches don't need to be bug free as you can always fix bugs
>> later. Just mention in the commit logs that this is preparation for some
>> new feature and not fully tested yet. We do that all the time, for
>> example Intel's iwlwifi has support for hardware which have not reached
>> customers yet.
>> 
>
> Thanks for your answer. I'll submit patches when the drivers are ready and
> stable. I have another question about the rules of new files. If I want to
> add some new files, could I send a big patch with all new files? Is there
> any limit? 

The only limit I know is the byte size limit in the mailing list. When
submitting a new driver some people split the driver to one file per
patch for easier review but the final commit needs to be one big patch
with all files included. If you are adding big files to rtlwifi I think
you could try to follow the same model.

-- 
Kalle Valo

Re: After upgrading to 4.11.1, wifi driver refuses to load after being unloaded once.

[Kalle Valo]

Luca Coelho  writes:

> On Tue, 2017-10-17 at 14:23 -0700, Marc MERLIN wrote:
>> On Tue, Oct 17, 2017 at 05:05:57PM +0300, Luca Coelho wrote:
>> > Hi,
>> > 
>> > On Tue, 2017-10-17 at 02:44 -0700, Marc MERLIN wrote:
>> > > Was broken in 4.11, still broken in 4.12. This is crippling, I'm
>> > > not
>> > > running linux so that I have to reboot it to reload an intel
>> > > wireless
>> > > driver :-/
>> > 
>> > Can you report a bug in https://bugzilla.kernel.org so it's easier
>> > to
>> > track this?
>> > 
>> > The problem seems to be that the rootport is not leaving D3 for
>> > some
>> > reason when the driver is loaded again.  Do you have
>> > CONFIG_IWLWIFI_PCIE_RTPM enabled in your .config? (you shouldn't)
>> 
>>  
>> I don't know how or why, but I seem to:
>> saruman:~# grep IWLWIFI /boot/config-4.12.10-amd64-preempt-sysrq-
>> 20170406 
>> CONFIG_IWLWIFI=m
>> CONFIG_IWLWIFI_LEDS=y
>> CONFIG_IWLWIFI_OPMODE_MODULAR=y
>> # CONFIG_IWLWIFI_BCAST_FILTERING is not set
>> CONFIG_IWLWIFI_PCIE_RTPM=y
>> CONFIG_IWLWIFI_DEBUG=y
>> CONFIG_IWLWIFI_DEVICE_TRACING=y
>> 
>> I'll remove that, thanks.
>
> Cool, I think that might help.  If it doesn't, please report a bug in
> buzilla. ;)

But a Kconfig option should never break functionality, so IMHO this
still sounds like a bug in iwlwifi.

-- 
Kalle Valo

Re: [PATCH v6 1/3] dt-bindings: net: add mt76 wireless device binding

[Rob Herring]

On Sat, Oct 14, 2017 at 10:43 AM, Christian Lamparter
 wrote:
> On Saturday, October 14, 2017 9:20:46 AM CEST Felix Fietkau wrote:
>> On 2017-10-13 21:07, Rob Herring wrote:
>> > On Fri, Oct 06, 2017 at 01:02:47PM +0200, Felix Fietkau wrote:
>> >> Add documentation describing how device tree can be used to configure
>> >> wireless chips supported by the mt76 driver.
>> >>
>> >> Signed-off-by: Felix Fietkau 
>> >> ---
>> >>  .../bindings/net/wireless/mediatek,mt76.txt| 24 
>> >> ++
>> >>  1 file changed, 24 insertions(+)
>> >>  create mode 100644 
>> >> Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt
>> >>
>> >> diff --git 
>> >> a/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt 
>> >> b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt
>> >> new file mode 100644
>> >> index ..19522ab97d62
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.txt
>> >> @@ -0,0 +1,24 @@
>> >> +* MediaTek mt76xx devices
>> >> +
>> >> +This node provides properties for configuring the MediaTek mt76xx 
>> >> wireless
>> >> +device. The node is expected to be specified as a child node of the PCI
>> >> +controller to which the wireless chip is connected.
>> >> +
>> >> +Optional properties:
>> >> +
>> >> +- mac-address: See ethernet.txt in the parent directory
>> >> +- local-mac-address: See ethernet.txt in the parent directory
>> >> +- ieee80211-freq-limit: See ieee80211.txt
>> >> +- mediatek,mtd-eeprom: Specify a MTD partition + offset containing 
>> >> EEPROM data
>> >
>> > MTD is a Linuxism. And is an EEPROM the only supported device? I'd
>> > suggest naming if after what the data contains.
>> PCI cards with this kind of wireless chip usually come with some form of
>> EEPROM or use the on-chip OTP ROM.
>> This property is for the case where the chip is built into an embedded
>> device and the data that would otherwise be on an EEPROM is stored on a
>> MTD partition instead.
>> The EEPROM data itself contains multiple things: calibration data,
>> hardware capabilities, MAC address, etc.
>> I couldn't think of a better name for it, do you have any suggestions?

Naming is hard.

> This sort of reminds me of the failed ath9k nvmem patches:
> https://patchwork.kernel.org/patch/9622127/
>
> Which uses the nvmem system.
>
> https://github.com/torvalds/linux/blob/master/Documentation/nvmem/nvmem.txt
>
> Rob, would this be acceptable?

Yeah, alignment with other drivers is a good thing.

Rob

[PATCH] ath10k: Fix offchannel tx failure when no ath10k_mac_tx_frm_has_freq

[greearb]

From: Ben Greear 

This bug appears to have been added between 4.0 (which works for us),
and 4.4, which does not work.

I think this is because the tx-offchannel logic gets in a loop when
ath10k_mac_tx_frm_has_freq(ar) is false, so pkt is never actually
sent to the firmware for transmit.

This patch fixes the problem on 4.9 for me, and now HS20 clients
can work again with my firmware.

Signed-off-by: Ben Greear 
---
 drivers/net/wireless/ath/ath10k/mac.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
b/drivers/net/wireless/ath/ath10k/mac.c
index c45ca04..f0a7864 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4101,7 +4101,7 @@ static int ath10k_mac_tx(struct ath10k *ar,
 struct ieee80211_sta *sta,
 enum ath10k_hw_txrx_mode txmode,
 enum ath10k_mac_tx_path txpath,
-struct sk_buff *skb)
+struct sk_buff *skb, bool noque_offchan)
 {
struct ieee80211_hw *hw = ar->hw;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
@@ -4134,10 +4134,10 @@ static int ath10k_mac_tx(struct ath10k *ar,
}
}
 
-   if (info->flags & IEEE80211_TX_CTL_TX_OFFCHAN) {
+   if ((!noque_offchan) && info->flags & IEEE80211_TX_CTL_TX_OFFCHAN) {
if (!ath10k_mac_tx_frm_has_freq(ar)) {
-   ath10k_dbg(ar, ATH10K_DBG_MAC, "queued offchannel skb 
%pK\n",
-  skb);
+   ath10k_dbg(ar, ATH10K_DBG_MAC, "no-tx-frm-has-freq: 
queued offchannel skb %pK, len: %d\n",
+  skb, skb->len);
 
skb_queue_tail(>offchan_tx_queue, skb);
ieee80211_queue_work(hw, >offchan_tx_work);
@@ -4198,8 +4198,8 @@ void ath10k_offchan_tx_work(struct work_struct *work)
 
mutex_lock(>conf_mutex);
 
-   ath10k_dbg(ar, ATH10K_DBG_MAC, "mac offchannel skb %pK\n",
-  skb);
+   ath10k_dbg(ar, ATH10K_DBG_MAC, "offchan-tx-work: mac offchannel 
skb %pK len: %d\n",
+  skb, skb->len);
 
hdr = (struct ieee80211_hdr *)skb->data;
peer_addr = ieee80211_get_DA(hdr);
@@ -4245,7 +4245,7 @@ void ath10k_offchan_tx_work(struct work_struct *work)
txmode = ath10k_mac_tx_h_get_txmode(ar, vif, sta, skb);
txpath = ath10k_mac_tx_h_get_txpath(ar, skb, txmode);
 
-   ret = ath10k_mac_tx(ar, vif, sta, txmode, txpath, skb);
+   ret = ath10k_mac_tx(ar, vif, sta, txmode, txpath, skb, true);
if (ret) {
ath10k_warn(ar, "failed to transmit offchannel frame: 
%d\n",
ret);
@@ -4255,8 +4255,8 @@ void ath10k_offchan_tx_work(struct work_struct *work)
time_left =
wait_for_completion_timeout(>offchan_tx_completed, 3 * HZ);
if (time_left == 0)
-   ath10k_warn(ar, "timed out waiting for offchannel skb 
%pK\n",
-   skb);
+   ath10k_warn(ar, "timed out waiting for offchannel skb 
%pK, len: %d\n",
+   skb, skb->len);
 
if (!peer && tmp_peer_created) {
ret = ath10k_peer_delete(ar, vdev_id, peer_addr);
@@ -4513,7 +4513,7 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
spin_unlock_bh(>htt.tx_lock);
}
 
-   ret = ath10k_mac_tx(ar, vif, sta, txmode, txpath, skb);
+   ret = ath10k_mac_tx(ar, vif, sta, txmode, txpath, skb, false);
if (unlikely(ret)) {
ath10k_warn(ar, "failed to push frame: %d\n", ret);
 
@@ -4802,7 +4802,7 @@ static void ath10k_mac_op_tx(struct ieee80211_hw *hw,
spin_unlock_bh(>htt.tx_lock);
}
 
-   ret = ath10k_mac_tx(ar, vif, sta, txmode, txpath, skb);
+   ret = ath10k_mac_tx(ar, vif, sta, txmode, txpath, skb, false);
if (ret) {
ath10k_warn(ar, "failed to transmit frame: %d\n", ret);
if (is_htt) {
-- 
2.4.11

Re: iwlwifi crash with hostapd

[James Cameron]

On Tue, Oct 17, 2017 at 09:35:39PM +0200, Mario Theodoridis wrote:
> On 16.10.2017 05:37, James Cameron wrote:
> >On Sun, Oct 15, 2017 at 06:21:36PM +0200, Mario Theodoridis wrote:
> >>Thanks for the pointers, James.
> >>
> >>On 12.10.2017 23:24, James Cameron wrote:
> >>>There's a good chance this problem has been fixed already.  You
> >>>are using a v4.4 kernel with many patches applied by Ubuntu.  Here, we
> >>>are more concerned with the latest kernels, and v4.4 is quite old.
> >>>
> >>>Please test some of the later kernels, see
> >>>https://wiki.ubuntu.com/Kernel/MainlineBuilds
> >>>
> >>>In particular, test v4.13 or v4.14-rc4.
> >>
> >>I'm having a hard time with that, because the virtualbox-dkms build fails
> >>with the 4.13 kernel, and virtualbox unfortunately is essential.
> >
> >Is virtualbox essential for reproducing the problem, or essential for
> >your general use?
> 
> It is essential for general use, like Internet connectivity.

Okay, good, that means we can ignore virtualbox, and leave that to
you.

Please test v4.13 or v4.14-rc5, ignoring virtualbox for the time being.

> >If the former, then that's interesting.
> >
> >If the latter, then you might instead test the v4.13 or v14-rc4
> >kernels for only the problem, and then revert to an older kernel after
> >testing.
> >
> >Either way, to use virtualbox-dkms with a later kernel you may be able
> >to upgrade just the virtualbox packages from a later Ubuntu release.
> >
> >See https://packages.ubuntu.com/virtualbox-dkms and
> >https://packages.ubuntu.com/virtualbox for the later versions available.
> >
> >Purpose of the test can be to help isolate the cause, not only to
> >solve your problem.
> 
> Thanks for the info.
> 
> >
> >[...]
> >You might also try with later firmware package.
> >See https://packages.ubuntu.com/linux-firmware
> >
> >You might also test with booting installation media in live-mode,
> >ignoring the internal disk.
> 
> Ok, that was completely off the radar.

Updating linux-firmware may run different firmware on the wireless
card, and the change in behaviour may fix the problem.  A gamble.

A test with later installation media is useful, because you can verify
problems with different kernels and wireless firmware without change
to configuration.  You might try Ubuntu 17.10 Artful ISO.

> I ended up going the other way. I still had a 4.4.0-79-generic kernel and
> booted that. It does not have this problem.
> After checking out
> git://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/xenial
> i tried to find the culprit but was not able to trace the back trace to a
> potential null pointer or some such. I got stuck at
> iwl_mvm_send_cmd_pdu_status not finding a reference to iwl_mvm_disable_txq
> from there.
> 
> I did got the following diff though
> 
> git diff Ubuntu-4.4.0-79.100 Ubuntu-4.4.0-93.116 --
> drivers/net/wireless/iwlwifi/ drivers/net/wireless/mac80211_hwsim.c >
> wifi.patch
> 
> I don't know whether this came from upstream or was ubuntu sourced.

Upstream.

You found your problem was introduced in an Ubuntu kernel, in the
update from -79 to -93.  This contained Ubuntu backports of two
stable kernel patches, which are also upstream patches;

8fbcfeb8a9cc ("mac80211_hwsim: Replace bogus hrtimer clockid")
from v4.4.69

50ea05efaf3b ("mac80211: pass block ack session timeout to to driver")
from v4.4.77

git log Ubuntu-4.4.0-79.100..Ubuntu-4.4.0-93.116 -- \
drivers/net/wireless/iwlwifi/ drivers/net/wireless/mac80211_hwsim.c

git remote add stable \
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
git fetch stable
git log v4.4.68..v4.4.92 -- \
drivers/net/wireless/iwlwifi/ drivers/net/wireless/mac80211_hwsim.c

> This fixed the issue for now, but now i'm stuck on that kernel :(

Yes.

Here in upstream, we would run the latest kernel v4.13 and work to
fix that.  Trouble you had with virtualbox packages would be
eventually solvable, but aren't really a problem with the kernel
itself.

So your next step may be to report an Ubuntu bug, and say that -79
worked fine, and -93 did not.

> While i'm perfectly comfortable with user land C, i have no kernel
> experience (clue stick links definitely welcome).

You might verify the above patches caused the problem by doing a
bisection between -79 and -93.

https://wiki.ubuntu.com/Kernel/KernelBisection

Or by reverting only those patches.

Then report to Ubuntu which patch caused the problem.

> [...]

Hope that helps.

-- 
James Cameron
http://quozl.netrek.org/

[PATCH] net: mac80211: mark expected switch fall-throughs

[Gustavo A. R. Silva]

In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Notice that in some cases I replaced "fall through on else" and
"otherwise fall through" comments with just a "fall through" comment,
which is what GCC is expecting to find.

Signed-off-by: Gustavo A. R. Silva 
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify that the actual intention of the code is to fall through.

 net/mac80211/cfg.c| 3 +++
 net/mac80211/ht.c | 1 +
 net/mac80211/iface.c  | 2 +-
 net/mac80211/mesh.c   | 2 ++
 net/mac80211/mesh_hwmp.c  | 1 +
 net/mac80211/mesh_plink.c | 2 +-
 net/mac80211/mlme.c   | 1 +
 net/mac80211/offchannel.c | 4 ++--
 net/mac80211/tdls.c   | 1 +
 net/mac80211/wme.c| 1 +
 10 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a354f19..9bd8bef 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -573,10 +573,12 @@ static int ieee80211_get_key(struct wiphy *wiphy, struct 
net_device *dev,
case WLAN_CIPHER_SUITE_BIP_CMAC_256:
BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) !=
 offsetof(typeof(kseq), aes_cmac));
+   /* fall through */
case WLAN_CIPHER_SUITE_BIP_GMAC_128:
case WLAN_CIPHER_SUITE_BIP_GMAC_256:
BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) !=
 offsetof(typeof(kseq), aes_gmac));
+   /* fall through */
case WLAN_CIPHER_SUITE_GCMP:
case WLAN_CIPHER_SUITE_GCMP_256:
BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) !=
@@ -2205,6 +2207,7 @@ static int ieee80211_scan(struct wiphy *wiphy,
 * for now fall through to allow scanning only when
 * beaconing hasn't been configured yet
 */
+   /* fall through */
case NL80211_IFTYPE_AP:
/*
 * If the scan has been forced (and the driver supports
diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
index 41f5e48..e55dabf 100644
--- a/net/mac80211/ht.c
+++ b/net/mac80211/ht.c
@@ -491,6 +491,7 @@ int ieee80211_send_smps_action(struct ieee80211_sub_if_data 
*sdata,
case IEEE80211_SMPS_AUTOMATIC:
case IEEE80211_SMPS_NUM_MODES:
WARN_ON(1);
+   /* fall through */
case IEEE80211_SMPS_OFF:
action_frame->u.action.u.ht_smps.smps_control =
WLAN_HT_SMPS_CONTROL_DISABLED;
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 13b16f9..435e735 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1633,7 +1633,7 @@ static void ieee80211_assign_perm_addr(struct 
ieee80211_local *local,
goto out_unlock;
}
}
-   /* otherwise fall through */
+   /* fall through */
default:
/* assign a new address if possible -- try n_addresses first */
for (i = 0; i < local->hw.wiphy->n_addresses; i++) {
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 7a76c4a..d29a545 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -988,8 +988,10 @@ ieee80211_mesh_process_chnswitch(struct 
ieee80211_sub_if_data *sdata,
switch (sdata->vif.bss_conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
sta_flags |= IEEE80211_STA_DISABLE_HT;
+   /* fall through */
case NL80211_CHAN_WIDTH_20:
sta_flags |= IEEE80211_STA_DISABLE_40MHZ;
+   /* fall through */
case NL80211_CHAN_WIDTH_40:
sta_flags |= IEEE80211_STA_DISABLE_VHT;
break;
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 146ec6c..0e75abf 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -1247,6 +1247,7 @@ void mesh_path_tx_root_frame(struct ieee80211_sub_if_data 
*sdata)
break;
case IEEE80211_PROACTIVE_PREQ_WITH_PREP:
flags |= IEEE80211_PREQ_PROACTIVE_PREP_FLAG;
+   /* fall through */
case IEEE80211_PROACTIVE_PREQ_NO_PREP:
interval = ifmsh->mshcfg.dot11MeshHWMPactivePathToRootTimeout;
target_flags |= IEEE80211_PREQ_TO_FLAG |
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index e2d00cc..0f6c9ca 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -672,7 +672,7 @@ void mesh_plink_timer(struct timer_list *t)
break;
}
reason = WLAN_REASON_MESH_MAX_RETRIES;
-   /* fall through on else */
+   /* fall through */
case NL80211_PLINK_CNF_RCVD:
/* confirm timer */
if (!reason)
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 

Re: using verifier to ensure a BPF program uses certain metadata?

[Alexei Starovoitov]

On Mon, Oct 16, 2017 at 09:38:44AM +0200, Johannes Berg wrote:
> Hi,
> 
> As we discussed in April already (it's really been that long...), I'd
> wanted to allow using BPF to filter wireless monitor frames, to enable
> new use cases and higher performance in monitoring. I have some code,
> at
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git/log/?h=bpf

bpf bits looks pretty straightforward.
attach looks fine too. I'm assuming there is some rtnl or other lock,
so multiple assigns cannot race?
It's missing query interface though.
Please add support to return prog_id.

> which implements parts of this. It's still missing the TX status path
> and perhaps associated metadata, but that part is easy.
> 
> The bigger "problem" is that we're going to be adding support for
> devices that have 802.11->Ethernet conversion already in hardware, and
> in that case the notion that the filter program will get an 802.11
> header to look at is no longer right.
> 
> Now, most likely for the actual in-service monitoring we'll actually
> have to reconstitute the 802.11 header on the fly (in pure monitoring
> where nothing else is active, we can just disable the conversion), but
> the filtering shouldn't really be reliant on that, since that's not the
> cheapest thing to do.
> 
> The obvious idea around this is to add a metadata field (just a bit
> really), something like "is_data_ethernet", saying that it was both a
> data frame and is already converted to have an Ethernet header.
> However, since these devices don't really exist yet for the vast
> majority of people, I'm a bit afraid that we'll find later a lot of
> code simply ignoring this field and looking at the "802.11" header,
> which is then broken if it encounters an Ethernet header instead.
> 
> Are there lies my question: If we added a new callback to
> bpf_verifier_ops (e.g. "post_verifier_check"), to be called after the
> normal verification, and also added a context argument to
> "is_valid_access" (*), we could easily track that this new metadata
> field is accessed, and reject programs that don't access it at all.
> 
> Now, I realize that people could trivially just work around this in
> their program if they wanted, but I think most will take the reminder
> and just implement
> 
> if (ctx->is_data_ethernet)
> return DROP_FRAME;
> 
> instead, since mostly data frames will not be very relevant to them.
> 
> What do you think?

sounds fine and considering new verifier ops after Jakub refactoring
a check that is_data_ethernet was accessed would fit nicely.
Without void** hack.

Re: After upgrading to 4.11.1, wifi driver refuses to load after being unloaded once.

[Luca Coelho]

On Tue, 2017-10-17 at 14:23 -0700, Marc MERLIN wrote:
> On Tue, Oct 17, 2017 at 05:05:57PM +0300, Luca Coelho wrote:
> > Hi,
> > 
> > On Tue, 2017-10-17 at 02:44 -0700, Marc MERLIN wrote:
> > > Was broken in 4.11, still broken in 4.12. This is crippling, I'm
> > > not
> > > running linux so that I have to reboot it to reload an intel
> > > wireless
> > > driver :-/
> > 
> > Can you report a bug in https://bugzilla.kernel.org so it's easier
> > to
> > track this?
> > 
> > The problem seems to be that the rootport is not leaving D3 for
> > some
> > reason when the driver is loaded again.  Do you have
> > CONFIG_IWLWIFI_PCIE_RTPM enabled in your .config? (you shouldn't)
> 
>  
> I don't know how or why, but I seem to:
> saruman:~# grep IWLWIFI /boot/config-4.12.10-amd64-preempt-sysrq-
> 20170406 
> CONFIG_IWLWIFI=m
> CONFIG_IWLWIFI_LEDS=y
> CONFIG_IWLWIFI_OPMODE_MODULAR=y
> # CONFIG_IWLWIFI_BCAST_FILTERING is not set
> CONFIG_IWLWIFI_PCIE_RTPM=y
> CONFIG_IWLWIFI_DEBUG=y
> CONFIG_IWLWIFI_DEVICE_TRACING=y
> 
> I'll remove that, thanks.

Cool, I think that might help.  If it doesn't, please report a bug in
buzilla. ;)

--
Cheers,
Luca.

Re: After upgrading to 4.11.1, wifi driver refuses to load after being unloaded once.

[Marc MERLIN]

On Tue, Oct 17, 2017 at 05:05:57PM +0300, Luca Coelho wrote:
> Hi,
> 
> On Tue, 2017-10-17 at 02:44 -0700, Marc MERLIN wrote:
> > Was broken in 4.11, still broken in 4.12. This is crippling, I'm not
> > running linux so that I have to reboot it to reload an intel wireless
> > driver :-/
> 
> Can you report a bug in https://bugzilla.kernel.org so it's easier to
> track this?
> 
> The problem seems to be that the rootport is not leaving D3 for some
> reason when the driver is loaded again.  Do you have
> CONFIG_IWLWIFI_PCIE_RTPM enabled in your .config? (you shouldn't)
 
I don't know how or why, but I seem to:
saruman:~# grep IWLWIFI /boot/config-4.12.10-amd64-preempt-sysrq-20170406 
CONFIG_IWLWIFI=m
CONFIG_IWLWIFI_LEDS=y
CONFIG_IWLWIFI_OPMODE_MODULAR=y
# CONFIG_IWLWIFI_BCAST_FILTERING is not set
CONFIG_IWLWIFI_PCIE_RTPM=y
CONFIG_IWLWIFI_DEBUG=y
CONFIG_IWLWIFI_DEVICE_TRACING=y

I'll remove that, thanks.

Marc
-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems 
   what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/ | PGP 1024R/763BE901

[PATCH] mac80211: aggregation: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

This removes the tid mapping array and expands the tid structures to
add a pointer back to the station, along with the tid index itself.

Cc: Johannes Berg 
Cc: "David S. Miller" 
Cc: linux-wireless@vger.kernel.org
Cc: net...@vger.kernel.org
Signed-off-by: Kees Cook 
---
Resend, with linux-wireless in Cc (no idea how it got missed before)
---
 net/mac80211/agg-rx.c   | 41 +
 net/mac80211/agg-tx.c   | 42 --
 net/mac80211/sta_info.c |  8 
 net/mac80211/sta_info.h | 12 ++--
 4 files changed, 43 insertions(+), 60 deletions(-)

diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
index 88cc1ae935ea..63aba6dbc92a 100644
--- a/net/mac80211/agg-rx.c
+++ b/net/mac80211/agg-rx.c
@@ -151,21 +151,17 @@ EXPORT_SYMBOL(ieee80211_stop_rx_ba_session);
  * After accepting the AddBA Request we activated a timer,
  * resetting it after each frame that arrives from the originator.
  */
-static void sta_rx_agg_session_timer_expired(unsigned long data)
+static void sta_rx_agg_session_timer_expired(struct timer_list *t)
 {
-   /* not an elegant detour, but there is no choice as the timer passes
-* only one argument, and various sta_info are needed here, so init
-* flow in sta_info_create gives the TID as data, while the timer_to_id
-* array gives the sta through container_of */
-   u8 *ptid = (u8 *)data;
-   u8 *timer_to_id = ptid - *ptid;
-   struct sta_info *sta = container_of(timer_to_id, struct sta_info,
-timer_to_tid[0]);
+   struct tid_ampdu_rx *tid_rx_timer =
+   from_timer(tid_rx_timer, t, session_timer);
+   struct sta_info *sta = tid_rx_timer->sta;
+   u16 tid = tid_rx_timer->tid;
struct tid_ampdu_rx *tid_rx;
unsigned long timeout;
 
rcu_read_lock();
-   tid_rx = rcu_dereference(sta->ampdu_mlme.tid_rx[*ptid]);
+   tid_rx = rcu_dereference(sta->ampdu_mlme.tid_rx[tid]);
if (!tid_rx) {
rcu_read_unlock();
return;
@@ -180,21 +176,18 @@ static void sta_rx_agg_session_timer_expired(unsigned 
long data)
rcu_read_unlock();
 
ht_dbg(sta->sdata, "RX session timer expired on %pM tid %d\n",
-  sta->sta.addr, (u16)*ptid);
+  sta->sta.addr, tid);
 
-   set_bit(*ptid, sta->ampdu_mlme.tid_rx_timer_expired);
+   set_bit(tid, sta->ampdu_mlme.tid_rx_timer_expired);
ieee80211_queue_work(>local->hw, >ampdu_mlme.work);
 }
 
-static void sta_rx_agg_reorder_timer_expired(unsigned long data)
+static void sta_rx_agg_reorder_timer_expired(struct timer_list *t)
 {
-   u8 *ptid = (u8 *)data;
-   u8 *timer_to_id = ptid - *ptid;
-   struct sta_info *sta = container_of(timer_to_id, struct sta_info,
-   timer_to_tid[0]);
+   struct tid_ampdu_rx *tid_rx = from_timer(tid_rx, t, reorder_timer);
 
rcu_read_lock();
-   ieee80211_release_reorder_timeout(sta, *ptid);
+   ieee80211_release_reorder_timeout(tid_rx->sta, tid_rx->tid);
rcu_read_unlock();
 }
 
@@ -356,14 +349,12 @@ void ___ieee80211_start_rx_ba_session(struct sta_info 
*sta,
spin_lock_init(_agg_rx->reorder_lock);
 
/* rx timer */
-   setup_deferrable_timer(_agg_rx->session_timer,
-  sta_rx_agg_session_timer_expired,
-  (unsigned long)>timer_to_tid[tid]);
+   timer_setup(_agg_rx->session_timer,
+   sta_rx_agg_session_timer_expired, TIMER_DEFERRABLE);
 
/* rx reorder timer */
-   setup_timer(_agg_rx->reorder_timer,
-   sta_rx_agg_reorder_timer_expired,
-   (unsigned long)>timer_to_tid[tid]);
+   timer_setup(_agg_rx->reorder_timer,
+   sta_rx_agg_reorder_timer_expired, 0);
 
/* prepare reordering buffer */
tid_agg_rx->reorder_buf =
@@ -399,6 +390,8 @@ void ___ieee80211_start_rx_ba_session(struct sta_info *sta,
tid_agg_rx->auto_seq = auto_seq;
tid_agg_rx->started = false;
tid_agg_rx->reorder_buf_filtered = 0;
+   tid_agg_rx->tid = tid;
+   tid_agg_rx->sta = sta;
status = WLAN_STATUS_SUCCESS;
 
/* activate it for RX */
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index bef516ec47f9..dedbb1fb10e7 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -422,15 +422,12 @@ int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, 
u16 tid,
  * add Block Ack response will arrive from the recipient.
  * If this timer expires sta_addba_resp_timer_expired will be executed.
  */
-static void 

[PATCH] cfg80211: fix connect/disconnect edge cases

[Johannes Berg]

From: Johannes Berg 

If we try to connect while already connected/connecting, but
this fails, we set ssid_len=0 but leave current_bss hanging,
leading to errors.

Check all of this better, first of all ensuring that we can't
try to connect to a different SSID while connected/ing; ensure
that prev_bssid is set for re-association attempts even in the
case of the driver supporting the connect() method, and don't
reset ssid_len in the failure cases.

While at it, also reset ssid_len while disconnecting unless we
were connected and expect a disconnected event, and warn on a
successful connection without ssid_len being set.

Cc: sta...@vger.kernel.org
Signed-off-by: Johannes Berg 
---
 net/wireless/sme.c | 50 +-
 1 file changed, 41 insertions(+), 9 deletions(-)

diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index f38ed490e42b..29bf453a0d5f 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -522,11 +522,6 @@ static int cfg80211_sme_connect(struct wireless_dev *wdev,
return -EOPNOTSUPP;
 
if (wdev->current_bss) {
-   if (!prev_bssid)
-   return -EALREADY;
-   if (prev_bssid &&
-   !ether_addr_equal(prev_bssid, wdev->current_bss->pub.bssid))
-   return -ENOTCONN;
cfg80211_unhold_bss(wdev->current_bss);
cfg80211_put_bss(wdev->wiphy, >current_bss->pub);
wdev->current_bss = NULL;
@@ -1106,11 +1101,35 @@ int cfg80211_connect(struct cfg80211_registered_device 
*rdev,
 
ASSERT_WDEV_LOCK(wdev);
 
-   if (WARN_ON(wdev->connect_keys)) {
-   kzfree(wdev->connect_keys);
-   wdev->connect_keys = NULL;
+   /*
+* If we have an ssid_len, we're trying to connect or are
+* already connected, so reject a new SSID unless it's the
+* same (which is the case for re-association.)
+*/
+   if (wdev->ssid_len &&
+   (wdev->ssid_len != connect->ssid_len ||
+memcmp(wdev->ssid, connect->ssid, wdev->ssid_len)))
+   return -EALREADY;
+
+   /*
+* If connected, reject (re-)association unless prev_bssid
+* matches the current BSSID.
+*/
+   if (wdev->current_bss) {
+   if (!prev_bssid)
+   return -EALREADY;
+   if (!ether_addr_equal(prev_bssid, wdev->current_bss->pub.bssid))
+   return -ENOTCONN;
}
 
+   /*
+* Reject if we're in the process of connecting with WEP,
+* this case isn't very interesting and trying to handle
+* it would make the code much more complex.
+*/
+   if (wdev->connect_keys)
+   return -EINPROGRESS;
+
cfg80211_oper_and_ht_capa(>ht_capa_mask,
  rdev->wiphy.ht_capa_mod_mask);
 
@@ -1161,7 +1180,12 @@ int cfg80211_connect(struct cfg80211_registered_device 
*rdev,
 
if (err) {
wdev->connect_keys = NULL;
-   wdev->ssid_len = 0;
+   /*
+* This could be reassoc getting refused, don't clear
+* ssid_len in that case.
+*/
+   if (!wdev->current_bss)
+   wdev->ssid_len = 0;
return err;
}
 
@@ -1188,6 +1212,14 @@ int cfg80211_disconnect(struct 
cfg80211_registered_device *rdev,
else if (wdev->ssid_len)
err = rdev_disconnect(rdev, dev, reason);
 
+   /*
+* Clear ssid_len unless we actually were fully connected,
+* in which case cfg80211_disconnected() will take care of
+* this later.
+*/
+   if (!wdev->current_bss)
+   wdev->ssid_len = 0;
+
return err;
 }
 
-- 
2.14.2

[PATCH] nl80211: don't expose wdev->ssid for most interfaces

[Johannes Berg]

From: Johannes Berg 

For mesh, this is simply wrong - there's no SSID, only the
mesh ID, so don't expose it at all.
For (P2P) client, it's wrong, because it exposes an internal
value that's only used when certain APIs are used.
For AP, it's actually the only correct case, so leave that.
All other interface types shouldn't be setting this anyway,
so there it won't change anything.

Fixes: b84e7a05f619 ("nl80211: send the NL80211_ATTR_SSID in 
nl80211_send_iface()")
Signed-off-by: Johannes Berg 
---
 net/wireless/nl80211.c | 26 --
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index fce2cbe6a193..d23eb5700788 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2605,10 +2605,32 @@ static int nl80211_send_iface(struct sk_buff *msg, u32 
portid, u32 seq, int flag
goto nla_put_failure;
}
 
-   if (wdev->ssid_len) {
-   if (nla_put(msg, NL80211_ATTR_SSID, wdev->ssid_len, wdev->ssid))
+   wdev_lock(wdev);
+   switch (wdev->iftype) {
+   case NL80211_IFTYPE_AP:
+   if (wdev->ssid_len &&
+   nla_put(msg, NL80211_ATTR_SSID, wdev->ssid_len, wdev->ssid))
goto nla_put_failure;
+   break;
+   case NL80211_IFTYPE_STATION:
+   case NL80211_IFTYPE_P2P_CLIENT:
+   case NL80211_IFTYPE_ADHOC: {
+   const u8 *ssid_ie;
+   if (!wdev->current_bss)
+   break;
+   ssid_ie = ieee80211_bss_get_ie(>current_bss->pub,
+  WLAN_EID_SSID);
+   if (!ssid_ie)
+   break;
+   if (nla_put(msg, NL80211_ATTR_SSID, ssid_ie[1], ssid_ie + 2))
+   goto nla_put_failure;
+   break;
+   }
+   default:
+   /* nothing */
+   break;
}
+   wdev_unlock(wdev);
 
genlmsg_end(msg, hdr);
return 0;
-- 
2.14.2

Re: [PATCH 00/58] networking: Convert timers to use timer_setup()

[Kees Cook]

On Tue, Oct 17, 2017 at 7:18 AM, Kalle Valo  wrote:
> + linux-wireless
>
> Hi Kees,
>
> Kees Cook  writes:
>
>> This is the current set of outstanding networking patches to perform
>> conversions to the new timer interface (rebased to -next). This is not
>> all expected conversions, but it contains everything needed in networking
>> to eliminate init_timer(), and all the non-standard setup_*_timer() uses.
>
> So this also includes patches which I had queued for
> wireless-drivers-next:
>
> https://patchwork.kernel.org/patch/9986253/
> https://patchwork.kernel.org/patch/9986245/
>
> And looking at patchwork[1] I have even more timer_setup() related
> patches from you. It would be really helpful if you could clearly
> document to which tree you want the patches to be applied. I don't care

Hi! Sorry about that. It's been a bit tricky to juggle everything.

> if it's net-next or wireless-drivers-next as long as it's not the both
> (meaning that both Dave and me apply the same patch, which would be
> bad). The thing is that I really do not have time to figure out for
> every patch via which tree it's supposed to go.

Which split is preferred? I had been trying to separate wireless from
the rest of net (but missed some cases).

> For now I'll just drop all your timer_setup() related patches from my
> queue and I'll assume Dave will take those. Ok?
>
> [1] https://patchwork.kernel.org/project/linux-wireless/list/

I guess I'll wait to see what Dave says.

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

Re: iwlwifi crash with hostapd

[Mario Theodoridis]

On 16.10.2017 05:37, James Cameron wrote:

On Sun, Oct 15, 2017 at 06:21:36PM +0200, Mario Theodoridis wrote:

Thanks for the pointers, James.

On 12.10.2017 23:24, James Cameron wrote:

There's a good chance this problem has been fixed already.  You
are using a v4.4 kernel with many patches applied by Ubuntu.  Here, we
are more concerned with the latest kernels, and v4.4 is quite old.

Please test some of the later kernels, see
https://wiki.ubuntu.com/Kernel/MainlineBuilds

In particular, test v4.13 or v4.14-rc4.


I'm having a hard time with that, because the virtualbox-dkms build fails
with the 4.13 kernel, and virtualbox unfortunately is essential.


Is virtualbox essential for reproducing the problem, or essential for
your general use?


It is essential for general use, like Internet connectivity.


If the former, then that's interesting.

If the latter, then you might instead test the v4.13 or v14-rc4
kernels for only the problem, and then revert to an older kernel after
testing.

Either way, to use virtualbox-dkms with a later kernel you may be able
to upgrade just the virtualbox packages from a later Ubuntu release.

See https://packages.ubuntu.com/virtualbox-dkms and
https://packages.ubuntu.com/virtualbox for the later versions available.

Purpose of the test can be to help isolate the cause, not only to
solve your problem.


Thanks for the info.



[...]
You might also try with later firmware package.
See https://packages.ubuntu.com/linux-firmware

You might also test with booting installation media in live-mode,
ignoring the internal disk.


Ok, that was completely off the radar.


I ended up going the other way. I still had a 4.4.0-79-generic kernel 
and booted that. It does not have this problem.

After checking out
git://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/xenial
i tried to find the culprit but was not able to trace the back trace to 
a potential null pointer or some such. I got stuck at 
iwl_mvm_send_cmd_pdu_status not finding a reference to 
iwl_mvm_disable_txq from there.


I did got the following diff though

git diff Ubuntu-4.4.0-79.100 Ubuntu-4.4.0-93.116 -- 
drivers/net/wireless/iwlwifi/ drivers/net/wireless/mac80211_hwsim.c > 
wifi.patch


I don't know whether this came from upstream or was ubuntu sourced.

This fixed the issue for now, but now i'm stuck on that kernel :(

While i'm perfectly comfortable with user land C, i have no kernel 
experience (clue stick links definitely welcome).




--
Mit freundlichen Grüßen/Best regards

Mario Theodoridis
diff --git a/drivers/net/wireless/iwlwifi/dvm/mac80211.c b/drivers/net/wireless/iwlwifi/dvm/mac80211.c
index b3ad34e..1eb1a82 100644
--- a/drivers/net/wireless/iwlwifi/dvm/mac80211.c
+++ b/drivers/net/wireless/iwlwifi/dvm/mac80211.c
@@ -729,12 +729,15 @@ static inline bool iwl_enable_tx_ampdu(const struct iwl_cfg *cfg)
 
 static int iwlagn_mac_ampdu_action(struct ieee80211_hw *hw,
    struct ieee80211_vif *vif,
-   enum ieee80211_ampdu_mlme_action action,
-   struct ieee80211_sta *sta, u16 tid, u16 *ssn,
-   u8 buf_size, bool amsdu)
+   struct ieee80211_ampdu_params *params)
 {
 	struct iwl_priv *priv = IWL_MAC80211_GET_DVM(hw);
 	int ret = -EINVAL;
+	struct ieee80211_sta *sta = params->sta;
+	enum ieee80211_ampdu_mlme_action action = params->action;
+	u16 tid = params->tid;
+	u16 *ssn = >ssn;
+	u8 buf_size = params->buf_size;
 	struct iwl_station_priv *sta_priv = (void *) sta->drv_priv;
 
 	IWL_DEBUG_HT(priv, "A-MPDU action on addr %pM tid %d\n",
diff --git a/drivers/net/wireless/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/iwlwifi/mvm/mac80211.c
index ce12717..1a8ea77 100644
--- a/drivers/net/wireless/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/iwlwifi/mvm/mac80211.c
@@ -826,13 +826,16 @@ iwl_mvm_ampdu_check_trigger(struct iwl_mvm *mvm, struct ieee80211_vif *vif,
 
 static int iwl_mvm_mac_ampdu_action(struct ieee80211_hw *hw,
 struct ieee80211_vif *vif,
-enum ieee80211_ampdu_mlme_action action,
-struct ieee80211_sta *sta, u16 tid,
-u16 *ssn, u8 buf_size, bool amsdu)
+struct ieee80211_ampdu_params *params)
 {
 	struct iwl_mvm *mvm = IWL_MAC80211_GET_MVM(hw);
 	int ret;
 	bool tx_agg_ref = false;
+	struct ieee80211_sta *sta = params->sta;
+	enum ieee80211_ampdu_mlme_action action = params->action;
+	u16 tid = params->tid;
+	u16 *ssn = >ssn;
+	u8 buf_size = params->buf_size;
 
 	IWL_DEBUG_HT(mvm, "A-MPDU action on addr %pM tid %d: action %d\n",
 		 sta->addr, tid, action);
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 0cd9512..019d716 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -1817,10 +1817,12 @@ static int mac80211_hwsim_testmode_cmd(struct ieee80211_hw *hw,
 
 static int mac80211_hwsim_ampdu_action(struct ieee80211_hw *hw,
    struct ieee80211_vif *vif,
-   enum ieee80211_ampdu_mlme_action action,
-   struct 

[PATCH] libertas: don't write wdev->ssid/_len

[Johannes Berg]

From: Johannes Berg 

When joining an IBSS network, wdev->ssid/_len will already be
set, so there's no need to write them. In any case, they are
internal cfg80211 values, and have very little user-visible
impact.

Signed-off-by: Johannes Berg 
---
 drivers/net/wireless/marvell/libertas/cfg.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/wireless/marvell/libertas/cfg.c 
b/drivers/net/wireless/marvell/libertas/cfg.c
index 71ba2c8d09b5..bb76707881cd 100644
--- a/drivers/net/wireless/marvell/libertas/cfg.c
+++ b/drivers/net/wireless/marvell/libertas/cfg.c
@@ -1698,9 +1698,6 @@ static void lbs_join_post(struct lbs_private *priv,
  0, GFP_KERNEL);
cfg80211_put_bss(priv->wdev->wiphy, bss);
 
-   memcpy(priv->wdev->ssid, params->ssid, params->ssid_len);
-   priv->wdev->ssid_len = params->ssid_len;
-
cfg80211_ibss_joined(priv->dev, bssid, params->chandef.chan,
 GFP_KERNEL);
 
-- 
2.14.2

[PATCH] wil6210: remove wil6210_uapi.h from MAINTAINERS

[Johannes Berg]

From: Johannes Berg 

This file doesn't exist.

Signed-off-by: Johannes Berg 
---
 MAINTAINERS | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3944f1626911..7051e2ae9971 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14565,7 +14565,6 @@ L:  wil6...@qca.qualcomm.com
 S: Supported
 W: http://wireless.kernel.org/en/users/Drivers/wil6210
 F: drivers/net/wireless/ath/wil6210/
-F: include/uapi/linux/wil6210_uapi.h
 
 WIMAX STACK
 M: Inaky Perez-Gonzalez 
-- 
2.14.2

[PATCH] wil6210: remove SSID debugfs

[Johannes Berg]

From: Johannes Berg 

This driver shouldn't be using wdev->ssid to start with, as
it's more or less an internal field in cfg80211 used for
various purposes. Reading it is possible through nl80211,
even if that's not really what we should be doing there
for anything but AP type interfaces.

It *really* shouldn't allow modifying it!

Remove the whole debugfs entry.

Signed-off-by: Johannes Berg 
---
 drivers/net/wireless/ath/wil6210/debugfs.c | 45 --
 1 file changed, 45 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c 
b/drivers/net/wireless/ath/wil6210/debugfs.c
index 6db00c167d2e..e58dc6dc1f9c 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -1048,50 +1048,6 @@ static const struct file_operations fops_bf = {
.llseek = seq_lseek,
 };
 
-/*-SSID*/
-static ssize_t wil_read_file_ssid(struct file *file, char __user *user_buf,
- size_t count, loff_t *ppos)
-{
-   struct wil6210_priv *wil = file->private_data;
-   struct wireless_dev *wdev = wil_to_wdev(wil);
-
-   return simple_read_from_buffer(user_buf, count, ppos,
-  wdev->ssid, wdev->ssid_len);
-}
-
-static ssize_t wil_write_file_ssid(struct file *file, const char __user *buf,
-  size_t count, loff_t *ppos)
-{
-   struct wil6210_priv *wil = file->private_data;
-   struct wireless_dev *wdev = wil_to_wdev(wil);
-   struct net_device *ndev = wil_to_ndev(wil);
-
-   if (*ppos != 0) {
-   wil_err(wil, "Unable to set SSID substring from [%d]\n",
-   (int)*ppos);
-   return -EINVAL;
-   }
-
-   if (count > sizeof(wdev->ssid)) {
-   wil_err(wil, "SSID too long, len = %d\n", (int)count);
-   return -EINVAL;
-   }
-   if (netif_running(ndev)) {
-   wil_err(wil, "Unable to change SSID on running interface\n");
-   return -EINVAL;
-   }
-
-   wdev->ssid_len = count;
-   return simple_write_to_buffer(wdev->ssid, wdev->ssid_len, ppos,
- buf, count);
-}
-
-static const struct file_operations fops_ssid = {
-   .read = wil_read_file_ssid,
-   .write = wil_write_file_ssid,
-   .open  = simple_open,
-};
-
 /*-temp*/
 static void print_temp(struct seq_file *s, const char *prefix, u32 t)
 {
@@ -1695,7 +1651,6 @@ static const struct {
{"stations", 0444,  _sta},
{"desc",0444,   _txdesc},
{"bf",  0444,   _bf},
-   {"ssid",0644,   _ssid},
{"mem_val", 0644,   _memread},
{"reset",   0244,   _reset},
{"rxon",0244,   _rxon},
-- 
2.14.2

[PATCH] mac80211: use constant time comparison with keys

[Jason A. Donenfeld]

Otherwise we risk leaking information via timing side channel.

Fixes: fdf7cb4185b6 ("mac80211: accept key reinstall without changing anything")
Signed-off-by: Jason A. Donenfeld 
---
 net/mac80211/key.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index ae995c8480db..035d16fe926e 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "ieee80211_i.h"
 #include "driver-ops.h"
@@ -635,7 +636,7 @@ int ieee80211_key_link(struct ieee80211_key *key,
 * new version of the key to avoid nonce reuse or replay issues.
 */
if (old_key && key->conf.keylen == old_key->conf.keylen &&
-   !memcmp(key->conf.key, old_key->conf.key, key->conf.keylen)) {
+   !crypto_memneq(key->conf.key, old_key->conf.key, key->conf.keylen)) 
{
ieee80211_key_free_unused(key);
ret = 0;
goto out;
-- 
2.14.2

Re: pull-request: mac80211 2017-10-16

[Jason A. Donenfeld]

On Tue, Oct 17, 2017 at 7:46 AM, Johannes Berg
 wrote:
> If it's not equal, you execute so much code
> beneath, going to the driver etc., that I'd think this particular time
> is in the noise.

Usually presumptions like this get you in trouble when some crafty
academic has a smart idea about that noise. I'll send a patch.

Re: [PATCH v2] ath10k: Retry pci probe on failure.

[Ben Greear]

On 10/17/2017 01:45 AM, Kalle Valo wrote:

Ben Greear  writes:


On 10/13/2017 08:50 AM, Adrian Chadd wrote:

On 13 October 2017 at 05:41, Kalle Valo  wrote:

gree...@candelatech.com writes:


From: Ben Greear 

This works around a problem we see when sometimes the wifi NIC does
not respond the first time.  This seems to happen especially often on
some of the 9984 NICs in mid-range platforms.

Signed-off-by: Ben Greear 


[...]


-static int ath10k_pci_probe(struct pci_dev *pdev,
- const struct pci_device_id *pci_dev)
+static int __ath10k_pci_probe(struct pci_dev *pdev,
+   const struct pci_device_id *pci_dev)
  {
   int ret = 0;
   struct ath10k *ar;
@@ -3672,6 +3672,22 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
   return ret;
  }

+static int ath10k_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *pci_dev)
+{
+ int cnt = 0;
+ int rv;
+ do {
+ rv = __ath10k_pci_probe(pdev, pci_dev);
+ if (rv == 0)
+ return rv;
+ pr_err("ath10k: failed to probe PCI : %d, retry-count: %d\n", rv, 
cnt);
+ mdelay(10); /* let the ath10k firmware gerbil take a small break 
*/
+ } while (cnt++ < 10);
+ return rv;
+}


This is a sledgehammer approach and it causes reload for all error
cases, like when hardware is broken or memory allocation is failing.

When the problem happens does it always fail at the the same place? Is
it hw reset or something else? It's better to retry the invidiual action
than to do this hack. Or is it just some more delay needed somewhere?


I am seeing WMI timeouts during initial firmware load and wait on
QCA9984 + BCM7444S SoC.
My guess is the WMI wakeup time is not "right" enough and needs to be
extended a little bit.

But then, I have played a lot of whackamole with WMI timeouts during
my lng porting effort..


The failure I saw was a failure to wake pci, and from comments, it seems that
the current wait is longer than what should be required, and it warns on slow
wakes, and I never saw that warning.  So I assume that waiting longer would not 
help.

I saw it fail twice in a row to wake pci and then succeed on the third
try, for instance,
when testing my patch.

As for a big hammer, I guess we could check for certain return codes if you 
think
that is better than just retrying all failures?


ath10k_pci_probe() has a lots of stuff which should not affect your
problem, like allocating memory, setting up timers and interrupts etc.
It's quite ugly to redo that in every cycle. A more fine grained
solution, like looping specific action (reset, wake whatever) is much
more preferred.

Do you have debug logs of failing cases?


I'll gather the logs next time I see this problem.

The patch I wrote likely does more than the minimal required to fix
this problem, but it does not complicate the code much, so I think that
is a benefit.  If we try to make it more specific, it will first likely
require a lot of testing effort to see if it is as effective, and second, it
will likely complicate the probe method quite a bit.

Its not like this is a performance issue...the extra loops will only be run
if the probe fails, and only on driver load.

If the driver fails to load due to issues that my hack cannot work around,
then the user has bigger problems than an extra second of time during the
boot.

Thanks,
Ben

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

Re: Commit 0711d638 breaks mwifiex

[Jesse Sung]

On Tue, Oct 17, 2017 at 11:10 PM, Johannes Berg
 wrote:
> Hi,
>
>> > > https://p.sipsolutions.net/8f9d5917a5cbe4b3.txt
>> > >
>> > > Can you also give that a spin?
>
> Thanks for testing!
>
>> > Issued four reassociate, the network interface is still alive. Also
>> > with this patch it stays with BSSID1 and I don't see any
>> > "Association request to the driver failed" in the process.
>>
>> Correction: I can still see "Association request to the driver
>> failed" and switching between BSSIDs with more reassociates but I
>> think that shouldn't be an issue?
>
> Not sure what you mean by "with more reassociates"?

I mean do more "reassociate" in the wpa_cli. :)

Thanks,
Jesse

Re: Commit 0711d638 breaks mwifiex

[Johannes Berg]

Hi,

> > > https://p.sipsolutions.net/8f9d5917a5cbe4b3.txt
> > > 
> > > Can you also give that a spin?

Thanks for testing!

> > Issued four reassociate, the network interface is still alive. Also
> > with this patch it stays with BSSID1 and I don't see any
> > "Association request to the driver failed" in the process.
> 
> Correction: I can still see "Association request to the driver
> failed" and switching between BSSIDs with more reassociates but I
> think that shouldn't be an issue?

Not sure what you mean by "with more reassociates"?

In any case, we suspect that there's another underlying issue in
mwifiex, which happens to tickle this problem.

Could you record a full trace with -e cfg80211 for me (and send it
privately, please compress trace.dat e.g. with xz, it compresses really
well)

Thanks,
johannes

Re: bcma: use bcma_debug and pr_cont in MIPS driver

[Kalle Valo]

Rafał Miłecki wrote:

> From: Rafał Miłecki 
> 
> Using bcma_debug gives a device-specific prefix for messages and pr_cont
> is a common helper for continuing a line.
> 
> Signed-off-by: Rafał Miłecki 
> Acked-By: Hauke Mehrtens 

Patch applied to wireless-drivers-next.git, thanks.

66cc04424960 bcma: use bcma_debug and pr_cont in MIPS driver

-- 
https://patchwork.kernel.org/patch/10008245/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: rtlwifi: Fix typo in if ... else if ... else construct

[Kalle Valo]

Larry Finger  wrote:

> The kbuild test robot reports two conditions with no effect (if == else).
> These are the result of copy and paste typographical errors.
> 
> Signed-off-by: Larry Finger 
> Cc: Ping-Ke Shih 
> Cc: Yan-Hsuan Chuang 
> Cc: Birming Chiu 
> Cc: Shaofu 
> Cc: Steven Ting 
> Cc: kbuild-...@01.org
> Cc: Julia Lawall 

Patch applied to wireless-drivers-next.git, thanks.

a7986ce1cb01 rtlwifi: Fix typo in if ... else if ... else construct

-- 
https://patchwork.kernel.org/patch/10006693/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 00/58] networking: Convert timers to use timer_setup()

[Kalle Valo]

+ linux-wireless

Hi Kees,

Kees Cook  writes:

> This is the current set of outstanding networking patches to perform
> conversions to the new timer interface (rebased to -next). This is not
> all expected conversions, but it contains everything needed in networking
> to eliminate init_timer(), and all the non-standard setup_*_timer() uses.

So this also includes patches which I had queued for
wireless-drivers-next:

https://patchwork.kernel.org/patch/9986253/
https://patchwork.kernel.org/patch/9986245/

And looking at patchwork[1] I have even more timer_setup() related
patches from you. It would be really helpful if you could clearly
document to which tree you want the patches to be applied. I don't care
if it's net-next or wireless-drivers-next as long as it's not the both
(meaning that both Dave and me apply the same patch, which would be
bad). The thing is that I really do not have time to figure out for
every patch via which tree it's supposed to go.

For now I'll just drop all your timer_setup() related patches from my
queue and I'll assume Dave will take those. Ok?

[1] https://patchwork.kernel.org/project/linux-wireless/list/

-- 
Kalle Valo

Re: Help with bug "Black screen freeze on suspend to ram (rtl8188ee)"

[Larry Finger]

On 10/17/2017 09:03 AM, Lauro Costa wrote:

Dear maintainer,

I have opened a bug report on bugzilla [1] regarding this issue I find when I 
attempt to suspend to ram, unfortunately no one has replied to me yet.


I assumed this issue is related to the driver rtl8188ee since the black screen 
doesn't happen when I rmmod the wifi driver, but I'm not sure what I have is 
enough to assume this.


Do you know if this driver rtl8188ee is supposed to support suspend to ram?

This system log message:

|(NULL device *): firmware: direct-loading firmware rtlwifi/rtl8188efw.bin|

is something I haven't seen before, I don't understand why it says (NULL device 
*). Could it mean there is something else wrong and the rtl driver crashes as a 
result of that?


I'm not sure how to further investigate this issue, so any advice you could give 
here or in the bug report would be much appreciated.


Sorry that I missed that bug report. Unfortunately, without the traceback from 
the occurrence of the bug, it will be very difficult to find the problem.


If you add a script to unload the module when the machine hibernates or 
suspends, and reloads it on startup, you should be able to avoid the lockups.


I will try to duplicate the problem here.

Larry

Re: Commit 0711d638 breaks mwifiex

[Jesse Sung]

On Tue, Oct 17, 2017 at 10:08 PM, Jesse Sung  wrote:
> On Tue, Oct 17, 2017 at 9:13 PM, Johannes Berg
>  wrote:
>>
>> On Tue, 2017-10-17 at 21:07 +0800, Jesse Sung wrote:
>> >
>> > > Not really quite sure about it yet, but that should address the
>> > > issue?
>> >
>> > A very rough test by issuing reassociate in wpa_cli:
>> > mwifiex works after reassociate - looks good
>>
>> Ok. Discussing this with Ilan, we realized that this was buggy, and
>> came up with this patch instead:
>>
>> https://p.sipsolutions.net/8f9d5917a5cbe4b3.txt
>>
>> Can you also give that a spin?
>
> Issued four reassociate, the network interface is still alive. Also with this
> patch it stays with BSSID1 and I don't see any "Association request to
> the driver failed" in the process.

Correction: I can still see "Association request to the driver failed" and
switching between BSSIDs with more reassociates but I think that shouldn't
be an issue?

Re: Commit 0711d638 breaks mwifiex

[Jesse Sung]

On Tue, Oct 17, 2017 at 9:13 PM, Johannes Berg
 wrote:
>
> On Tue, 2017-10-17 at 21:07 +0800, Jesse Sung wrote:
> >
> > > Not really quite sure about it yet, but that should address the
> > > issue?
> >
> > A very rough test by issuing reassociate in wpa_cli:
> > mwifiex works after reassociate - looks good
>
> Ok. Discussing this with Ilan, we realized that this was buggy, and
> came up with this patch instead:
>
> https://p.sipsolutions.net/8f9d5917a5cbe4b3.txt
>
> Can you also give that a spin?

Issued four reassociate, the network interface is still alive. Also with this
patch it stays with BSSID1 and I don't see any "Association request to
the driver failed" in the process.

Thanks,
Jesse

Re: After upgrading to 4.11.1, wifi driver refuses to load after being unloaded once.

[Luca Coelho]

Hi,

On Tue, 2017-10-17 at 02:44 -0700, Marc MERLIN wrote:
> Was broken in 4.11, still broken in 4.12. This is crippling, I'm not
> running linux so that I have to reboot it to reload an intel wireless
> driver :-/

Can you report a bug in https://bugzilla.kernel.org so it's easier to
track this?

The problem seems to be that the rootport is not leaving D3 for some
reason when the driver is loaded again.  Do you have
CONFIG_IWLWIFI_PCIE_RTPM enabled in your .config? (you shouldn't)

In any case, better continue tracking this in bugzilla, so please
report it there and include the full dmesg output so we can better
understand what is going on.

--
Cheers,
Luca.

Re: Commit 0711d638 breaks mwifiex

[Johannes Berg]

On Tue, 2017-10-17 at 21:07 +0800, Jesse Sung wrote:
> 
> > Not really quite sure about it yet, but that should address the
> > issue?
> 
> A very rough test by issuing reassociate in wpa_cli:
> mwifiex works after reassociate - looks good

Ok. Discussing this with Ilan, we realized that this was buggy, and
came up with this patch instead:

https://p.sipsolutions.net/8f9d5917a5cbe4b3.txt

Can you also give that a spin?

johannes

Re: Commit 0711d638 breaks mwifiex

[Jesse Sung]

On Tue, Oct 17, 2017 at 6:48 PM, Johannes Berg
 wrote:
> On Tue, 2017-10-17 at 18:18 +0800, Jesse Sung wrote:
>
>> > Does mwifiex treat this -EALREADY as *keeping* an old connection,
>> > or tearing it down entirely?
>>
>> From the call trace:
>
> Well, the call trace can't really answer that :-)
> Does mwifiex firmware stay connected?

Sorry I don't know how to tell firmware's state.  @@

>> 139.451318: nl80211_get_valid_chan <-nl80211_connect
>> 139.451321: cfg80211_connect <-nl80211_connect
>> 139.451322: cfg80211_oper_and_ht_capa <-cfg80211_connect
>> 139.451323: mwifiex_cfg80211_connect <-cfg80211_connect
>> 139.451337: nl80211_post_doit <-genl_family_rcv_msg
>> 139.451423: nl80211_pre_doit <-genl_family_rcv_msg
>> 139.451425: nl80211_disconnect <-genl_family_rcv_msg
>> 139.451426: cfg80211_disconnect <-nl80211_disconnect
>> 139.451430: mwifiex_cfg80211_disconnect <-cfg80211_disconnect
>>
>> mwifiex_cfg80211_disconnect() would be called after
>> mwifiex_cfg80211_connect(), though I'm not sure if it's triggered by
>> the error returned.
>
> I think so - it's probably wpa_supplicant trying to get back to a well-
> known state (of being disconnected).
>
>> > I think your fix is invalid because we then reset ssid_len and
>> > still
>> > keep an old connection (current_bss) which will lead to strange
>> > nl80211
>> > behaviour when getting interface data etc.
>>
>> Since this is how it works before commit 0711d638 (use current_bss
>> instead of ssid_len), so I'm guessing this would still work. But I
>> agree that this may not be a proper fix...
>
> It would probably work, but we get data inconsistencies, and at the
> very least you get no SSID data back when you query the current state.
>
> I don't see anything in nl80211 or so that would say we should accept a
> connect() while already connected, so how about this?
>
> diff --git a/net/wireless/sme.c b/net/wireless/sme.c
> index b347e63d7aaa..fe0037ad1f5e 100644
> --- a/net/wireless/sme.c
> +++ b/net/wireless/sme.c
> @@ -1042,6 +1042,9 @@ int cfg80211_connect(struct cfg80211_registered_device 
> *rdev,
>
> ASSERT_WDEV_LOCK(wdev);
>
> +   if (wdev->current_bss)
> +   return -EALREADY;
> +
> if (WARN_ON(wdev->connect_keys)) {
> kzfree(wdev->connect_keys);
> wdev->connect_keys = NULL;
>
> Not really quite sure about it yet, but that should address the issue?

A very rough test by issuing reassociate in wpa_cli:
mwifiex works after reassociate - looks good

> reassociate
OK
<3>CTRL-EVENT-SCAN-STARTED
<3>CTRL-EVENT-SCAN-RESULTS
<3>Trying to associate with  (SSID= freq=)
<3>Association request to the driver failed
<3>CTRL-EVENT-REGDOM-CHANGE init=CORE type=WORLD
<3>CTRL-EVENT-SCAN-STARTED
<3>CTRL-EVENT-SCAN-RESULTS
<3>WPS-AP-AVAILABLE
<3>Trying to associate with  (SSID= freq=)
<3>Associated with 
<3>WPA: Key negotiation completed with  [PTK=CCMP GTK=CCMP]
<3>CTRL-EVENT-CONNECTED - Connection to  completed [id=0 id_str=]
<3>CTRL-EVENT-SCAN-STARTED
> reassociate
OK
<3>CTRL-EVENT-SCAN-RESULTS
<3>Trying to associate with  (SSID= freq=)
<3>Associated with 
<3>CTRL-EVENT-REGDOM-CHANGE init=CORE type=WORLD
<3>WPA: Key negotiation completed with  [PTK=CCMP GTK=CCMP]
<3>CTRL-EVENT-CONNECTED - Connection to  completed [id=0 id_str=]
> reassociate
OK
<3>CTRL-EVENT-SCAN-STARTED
>
<3>CTRL-EVENT-SCAN-RESULTS
<3>Trying to associate with  (SSID= freq=)
<3>Association request to the driver failed
<3>CTRL-EVENT-REGDOM-CHANGE init=CORE type=WORLD
<3>CTRL-EVENT-SCAN-STARTED
<3>CTRL-EVENT-SCAN-RESULTS
<3>WPS-AP-AVAILABLE
<3>Trying to associate with  (SSID= freq=)
<3>Associated with 
<3>WPA: Key negotiation completed with  [PTK=CCMP GTK=CCMP]
<3>CTRL-EVENT-CONNECTED - Connection to  completed [id=0 id_str=]
> reassociate
OK
<3>CTRL-EVENT-SCAN-STARTED
<3>CTRL-EVENT-SCAN-RESULTS
<3>Trying to associate with  (SSID= freq=)
<3>Association request to the driver failed
<3>CTRL-EVENT-REGDOM-CHANGE init=CORE type=WORLD
<3>CTRL-EVENT-SCAN-STARTED
<3>CTRL-EVENT-SCAN-RESULTS
<3>WPS-AP-AVAILABLE
<3>Trying to associate with  (SSID= freq=)
<3>Associated with 
<3>WPA: Key negotiation completed with  [PTK=CCMP GTK=CCMP]
<3>CTRL-EVENT-CONNECTED - Connection to  completed [id=0 id_str=]
<3>CTRL-EVENT-SCAN-STARTED
<3>CTRL-EVENT-SCAN-RESULTS
<3>Trying to associate with  (SSID= freq=)
<3>Association request to the driver failed
<3>CTRL-EVENT-REGDOM-CHANGE init=CORE type=WORLD
<3>CTRL-EVENT-SCAN-STARTED
<3>CTRL-EVENT-SCAN-RESULTS
<3>WPS-AP-AVAILABLE
<3>Trying to associate with  (SSID= freq=)
<3>Associated with 
<3>WPA: Key negotiation completed with  [PTK=CCMP GTK=CCMP]
<3>CTRL-EVENT-CONNECTED - Connection to  completed [id=0 id_str=]


Thanks,
Jesse

Re: AP6335 with mainline kernel

[Vanessa Maegima]

Hi Arend,

On Qui, 2017-09-21 at 12:30 -0300, Vanessa Ayumi Maegima wrote:
> Hi Arend,
> 
> On Thu, Sep 21, 2017 at 4:26 AM, Arend van Spriel
>  wrote:
> > 
> > On 20-09-17 21:33, Vanessa Ayumi Maegima wrote:
> > > 
> > > 
> > > Hi,
> > > 
> > > I am trying to enable Wifi on imx7d-pico using mainline kernel.
> > > imx7d-pico
> > > has an AP6335 chip.
> > > 
> > > I am facing some issues related to the nvram file. I am using the
> > > firmware
> > > provided by Buildroot (brcmfmac4339-sdio.bin). I get the
> > > following error:
> > > 
> > > [8.630380] brcmfmac: brcmf_sdio_htclk: HT Avail timeout
> > > (100):
> > > clkctl 0x50
> > > 
> > > I have tried to use the firmware and nvram provided by TechNexion
> > > but I
> > > get
> > > the same error.
> > > 
> > > Is there anyone that could enable Wifi on AP6335 using kernel
> > > mainline?
> > > What nvram file was used?
> > > 
> > > I am able to use Wifi on the board if I use the firmware, nvram
> > > file and
> > > kernel
> > > provided by TechNexion. They use a 4.1 kernel from NXP with the
> > > bcmdhd
> > > driver.
> > > 
> > > So I know that the hardware is functional.
> > > 
> > > Any suggestions as how to get it working with a 4.13 and brcmfmac
> > > driver
> > > is
> > > appreciated.
> > 
> > So the nvram file is specific to the wifi chipset on your platform
> > so best
> > to stick with the provided one. The "HT Avail timeout" most often
> > is an
> > indication that the firmware crashed. So getting more debug output
> > would
> > help us understand how it ended up like that. Can you build the
> > brcmfmac
> > with CONFIG_BRCMDBG and load the driver using:
> > 
> > $ insmod brcmfmac.ko debug=0x1416
> Thanks for the reply!
> 
> Here is the log (using 4.14-rc1):
> 
> # dmesg | grep brcmfmac
> [   19.297206] brcmfmac: brcmfmac_module_init No platform data
> available.
> [   19.307075] brcmfmac: brcmf_sdio_probe Enter
> [   19.308384] brcmfmac: F1 signature read @0x1800=0x16224335
> [   19.309026] brcmfmac: brcmf_chip_recognition found AXI chip:
> BCM4339, rev=2
> [   19.317115] brcmfmac: brcmf_chip_cores_check  [1 ] core 0x800:46
> base 0x1800 wrap 0x1810
> [   19.317141] brcmfmac: brcmf_chip_cores_check  [2 ] core 0x812:46
> base 0x18001000 wrap 0x18101000
> [   19.317165] brcmfmac: brcmf_chip_cores_check  [3 ] core 0x83e:4
> base 0x18002000 wrap 0x18102000
> [   19.317188] brcmfmac: brcmf_chip_cores_check  [4 ] core 0x83c:4
> base 0x18003000 wrap 0x18103000
> [   19.317210] brcmfmac: brcmf_chip_cores_check  [5 ] core 0x81a:20
> base 0x18004000 wrap 0x18104000
> [   19.317233] brcmfmac: brcmf_chip_cores_check  [6 ] core 0x829:21
> base 0x18005000 wrap 0x18105000
> [   19.317256] brcmfmac: brcmf_chip_cores_check  [7 ] core 0x135:0
> base 0x wrap 0x18109000
> [   19.317279] brcmfmac: brcmf_chip_cores_check  [8 ] core 0x240:0
> base 0x wrap 0x
> [   19.317298] brcmfmac: brcmf_chip_set_passive Enter
> [   19.322232] brcmfmac: brcmf_chip_get_raminfo RAM: base=0x18
> size=786432 (0xc) sr=0 (0x0)
> [   19.322457] brcmfmac: brcmf_chip_setup ccrev=46, pmurev=23,
> pmucaps=0x39cc5f17
> [   19.322481] brcmfmac: brcmf_get_module_param Enter, bus=0,
> chip=17209, rev=2
> [   19.322504] brcmfmac: brcmf_sdiod_sgtable_alloc nents=35
> [   19.322531] brcmfmac: brcmf_sdio_kso_init Enter
> [   19.322618] brcmfmac: brcmf_sdio_drivestrengthinit No SDIO driver
> strength init needed for chip 43
> 39 rev 2 pmurev 23
> [   19.323235] brcmfmac: brcmf_attach Enter
> [   19.323725] brcmfmac: brcmf_proto_attach Enter
> [   19.323769] brcmfmac: brcmf_fweh_register event handler registered
> for PSM_WATCHDOG
> [   19.324306] brcmfmac: brcmf_sdio_probe completed!!
> [   19.324337] brcmfmac: brcmf_fw_map_chip_to_name: using
> brcm/brcmfmac4339-sdio.bin for chip 0x00433
> 9(17209) rev 0x02
> [   19.335353] brcmfmac: brcmf_fw_get_firmwares_pcie enter:
> dev=mmc0:0001:1
> [   19.351787] brcmfmac: brcmf_fw_request_code_done enter:
> dev=mmc0:0001:1
> [   19.353202] brcmfmac: brcmf_fw_request_nvram_done enter:
> dev=mmc0:0001:1
> [   19.353424] brcmfmac: brcmf_sdio_firmware_callback Enter:
> dev=mmc0:0001:1, err=0
> [   19.353814] brcmfmac: brcmf_sdio_download_code_file Enter
> [   19.388586] brcmfmac: brcmf_sdio_verifymemory Compare RAM dl & ul
> at 0x0018; size=493599
> [   19.546675] brcmfmac: brcmf_sdio_download_nvram Enter
> [   19.547432] brcmfmac: brcmf_sdio_verifymemory Compare RAM dl & ul
> at 0x0023f730; size=2256
> [   19.548665] brcmfmac: brcmf_chip_set_active Enter
> [   20.562974] brcmfmac: brcmf_sdio_htclk: HT Avail timeout
> (100):
> clkctl 0x50
> [   20.570490] brcmfmac: brcmf_sdio_firmware_callback failed:
> dev=mmc0:0001:1, err=0
> [   20.570739] brcmfmac: brcmf_sdio_remove Enter
> [   20.570775] brcmfmac: brcmf_detach Enter
> [   20.610414] brcmfmac: brcmf_bus_change_state 0 -> 0
> [   20.610441] brcmfmac: brcmf_sdio_bus_stop Enter
> [   21.622477] brcmfmac: brcmf_sdio_htclk: 

Re: Commit 0711d638 breaks mwifiex

[Johannes Berg]

On Tue, 2017-10-17 at 18:18 +0800, Jesse Sung wrote:

> > Does mwifiex treat this -EALREADY as *keeping* an old connection,
> > or tearing it down entirely?
> 
> From the call trace:

Well, the call trace can't really answer that :-)
Does mwifiex firmware stay connected?


> 139.451318: nl80211_get_valid_chan <-nl80211_connect
> 139.451321: cfg80211_connect <-nl80211_connect
> 139.451322: cfg80211_oper_and_ht_capa <-cfg80211_connect
> 139.451323: mwifiex_cfg80211_connect <-cfg80211_connect
> 139.451337: nl80211_post_doit <-genl_family_rcv_msg
> 139.451423: nl80211_pre_doit <-genl_family_rcv_msg
> 139.451425: nl80211_disconnect <-genl_family_rcv_msg
> 139.451426: cfg80211_disconnect <-nl80211_disconnect
> 139.451430: mwifiex_cfg80211_disconnect <-cfg80211_disconnect
> 
> mwifiex_cfg80211_disconnect() would be called after
> mwifiex_cfg80211_connect(), though I'm not sure if it's triggered by
> the error returned.

I think so - it's probably wpa_supplicant trying to get back to a well-
known state (of being disconnected).

> > I think your fix is invalid because we then reset ssid_len and
> > still
> > keep an old connection (current_bss) which will lead to strange
> > nl80211
> > behaviour when getting interface data etc.
> 
> Since this is how it works before commit 0711d638 (use current_bss
> instead of ssid_len), so I'm guessing this would still work. But I
> agree that this may not be a proper fix...

It would probably work, but we get data inconsistencies, and at the
very least you get no SSID data back when you query the current state.

I don't see anything in nl80211 or so that would say we should accept a
connect() while already connected, so how about this?

diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index b347e63d7aaa..fe0037ad1f5e 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -1042,6 +1042,9 @@ int cfg80211_connect(struct cfg80211_registered_device 
*rdev,
 
ASSERT_WDEV_LOCK(wdev);
 
+   if (wdev->current_bss)
+   return -EALREADY;
+
if (WARN_ON(wdev->connect_keys)) {
kzfree(wdev->connect_keys);
wdev->connect_keys = NULL;

Not really quite sure about it yet, but that should address the issue?

johannes

Re: Commit 0711d638 breaks mwifiex

[Jesse Sung]

Hi Johannes,

On Tue, Oct 17, 2017 at 5:51 PM, Johannes Berg
 wrote:
> Hi,
>
>> While working on an issue that marvell module stops connecting to AP,
>> bisect reveals that the issue starts to happen from commit 0711d638,
>> which uses wdev->ssid_len instead of wdev->current_bss to determine
>> if driver's .disconnect() should be called.
>>
>> It happens because mwifiex_cfg80211_connect() returns -EALREADY
>> when it finds wdev->current_bss is valid:
>>
>> if (priv->wdev.current_bss) {
>> [PRINT LOG]
>> return -EALREADY;
>> }
>>
>> This would make cfg80211_connect() set wdev->ssid_len to 0, and thus
>> mwifiex_cfg80211_disconnect() won't be called by
>> cfg80211_disconnect().
>
> Hmm, none of this makes much sense to me right now.
>
> Does mwifiex treat this -EALREADY as *keeping* an old connection, or
> tearing it down entirely?

>From the call trace:

139.451318: nl80211_get_valid_chan <-nl80211_connect
139.451321: cfg80211_connect <-nl80211_connect
139.451322: cfg80211_oper_and_ht_capa <-cfg80211_connect
139.451323: mwifiex_cfg80211_connect <-cfg80211_connect
139.451337: nl80211_post_doit <-genl_family_rcv_msg
139.451423: nl80211_pre_doit <-genl_family_rcv_msg
139.451425: nl80211_disconnect <-genl_family_rcv_msg
139.451426: cfg80211_disconnect <-nl80211_disconnect
139.451430: mwifiex_cfg80211_disconnect <-cfg80211_disconnect

mwifiex_cfg80211_disconnect() would be called after
mwifiex_cfg80211_connect(), though I'm not sure if it's triggered by the
error returned.

> Because right now clearly cfg80211 assumes, on the one hand, that no
> connection is kept (resetting ssid_len), but on the other hand it got
> here with current_bss set - so perhaps we should reject that before in
> cfg80211, rather than in mwifiex?

Yes the inconsistency may be a problem.

> I think your fix is invalid because we then reset ssid_len and still
> keep an old connection (current_bss) which will lead to strange nl80211
> behaviour when getting interface data etc.

Since this is how it works before commit 0711d638 (use current_bss instead
of ssid_len), so I'm guessing this would still work. But I agree that this
may not be a proper fix...

Thanks,
Jesse

Re: [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs

[Toke Høiland-Jørgensen]

Johannes Berg  writes:

> On Tue, 2017-10-17 at 09:34 +0200, Toke Høiland-Jørgensen wrote:
>
>> Yeah, I did that initially. The reason I ended up squashing them is
>> that
>> this patch moved the per-station 'airtime' debugfs-entry that was
>> previously created by ath9k into mac80211. I assumed it would create
>> problems if both the driver and mac80211 tried to create the same
>> file;
>> not sure how to handle that if it's split into two patches?
>
> That might indeed be problematic, so perhaps just use a different
> name?

Well I would like to keep compatibility with my tooling that reads the
values :)

> Then again, if adding the entry fails in ath9k, nothing will happen
> since it doesn't even check the return value - so I think it doesn't
> actually matter.

Right, I'll try that, and otherwise create a separate patch to ath9k
that removes the debugfs entry before applying the mac80211 patch.

>> > I'd prefer this were called "airtime" or such,
>> 
>> Right. I picked rx_time for symmetry with the tx side; should I
>> rename that as well, then, or is asymmetry fine?
>
> Let's not change the tx side in this patch, but we can clean it up
> later I guess.

ACK.

-Toke

Re: [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs

[Johannes Berg]

On Tue, 2017-10-17 at 09:34 +0200, Toke Høiland-Jørgensen wrote:

> Yeah, I did that initially. The reason I ended up squashing them is
> that
> this patch moved the per-station 'airtime' debugfs-entry that was
> previously created by ath9k into mac80211. I assumed it would create
> problems if both the driver and mac80211 tried to create the same
> file;
> not sure how to handle that if it's split into two patches?

That might indeed be problematic, so perhaps just use a different name?
Then again, if adding the entry fails in ath9k, nothing will happen
since it doesn't even check the return value - so I think it doesn't
actually matter.

> > I'd prefer this were called "airtime" or such,
> 
> Right. I picked rx_time for symmetry with the tx side; should I
> rename that as well, then, or is asymmetry fine?

Let's not change the tx side in this patch, but we can clean it up
later I guess.

johannes

Re: Commit 0711d638 breaks mwifiex

[Johannes Berg]

Hi,

> While working on an issue that marvell module stops connecting to AP,
> bisect reveals that the issue starts to happen from commit 0711d638,
> which uses wdev->ssid_len instead of wdev->current_bss to determine
> if driver's .disconnect() should be called.
> 
> It happens because mwifiex_cfg80211_connect() returns -EALREADY
> when it finds wdev->current_bss is valid:
> 
> if (priv->wdev.current_bss) {
> [PRINT LOG]
> return -EALREADY;
> }
> 
> This would make cfg80211_connect() set wdev->ssid_len to 0, and thus
> mwifiex_cfg80211_disconnect() won't be called by
> cfg80211_disconnect().

Hmm, none of this makes much sense to me right now.

Does mwifiex treat this -EALREADY as *keeping* an old connection, or
tearing it down entirely?

Because right now clearly cfg80211 assumes, on the one hand, that no
connection is kept (resetting ssid_len), but on the other hand it got
here with current_bss set - so perhaps we should reject that before in
cfg80211, rather than in mwifiex?

I think your fix is invalid because we then reset ssid_len and still
keep an old connection (current_bss) which will lead to strange nl80211
behaviour when getting interface data etc.

johannes

Re: After upgrading to 4.11.1, wifi driver refuses to load after being unloaded once.

[Marc MERLIN]

Was broken in 4.11, still broken in 4.12. This is crippling, I'm not
running linux so that I have to reboot it to reload an intel wireless
driver :-/

I currently have:
-rw-rw-r-- 1 merlin merlin 1745176 Jun 18  2015 
/lib/firmware/iwlwifi-8000C-13.ucode
-rw-r--r-- 1 root   root   2345768 Dec  6  2015 
/lib/firmware/iwlwifi-8000C-14.ucode
-rw-r--r-- 1 root   root   2351636 Dec  6  2015 
/lib/firmware/iwlwifi-8000C-16.ucode
-rw-r--r-- 1 root   root   2390004 Feb 12  2016 
/lib/firmware/iwlwifi-8000C-17.ucode
-rw-r--r-- 1 merlin merlin 2382972 Feb 25  2016 
/lib/firmware/iwlwifi-8000C-19.ucode
-rw-r--r-- 1 merlin merlin 2227284 Jun  3 09:46 
/lib/firmware/iwlwifi-8000C-27.ucode

https://www.intel.com/content/www/us/en/support/articles/05511/network-and-i-o/wireless-networking.html
still says to install
iwlwifi-8000-ucode-25.30.13.0.tgz
which in turn installs
iwlwifi-8000C-13.ucode
while the driver looks for iwlwifi-8000C-30.ucode iwlwifi-8000C-29.ucode 
iwlwifi-8000C-28.ucode
before loading iwlwifi-8000C-27.ucode

so, what do I do?

Intel(R) Wireless WiFi driver for Linux
Copyright(c) 2003- 2015 Intel Corporation
iwlwifi :04:00.0: Refused to change power state, currently in D3
Timeout waiting for hardware access (CSR_GP_CNTRL 0x)
[ cut here ]
WARNING: CPU: 1 PID: 22824 at 
drivers/net/wireless/intel/iwlwifi/pcie/trans.c:1855 
iwl_trans_pcie_grab_nic_access+0xc0/0xd7 [iwlwifi] Modules linked in: 
iwlwifi(+) cfg80211 msr cmac tun ccm rfcomm ipt_MASQUERADE 
nf_nat_masquerade_ipv4 ipt_REJECT nf_reject_ipv4 xt_tcpudp xt_conntrack nf_ 
ntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle 
ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter ip_tables x_t 
boxnetflt(OE) vboxdrv(OE) autofs4 binfmt_misc uinput nfsd auth_rpcgss nfs_acl 
nfs lockd grace fscache sunrpc nls_utf8 nls_cp437 vfat fat configs input_po 
cuse ecryptfs ppdev parport_pc lp parport uvcvideo btusb videobuf2_vmalloc 
btrtl hid_generic btbcm videobuf2_memops btintel videobuf2_v4l2 videobuf2_cor 
ic joydev arc4 rtsx_pci_ms iTCO_wdt memstick iTCO_vendor_support rtsx_pci_sdmmc 
snd_hda_codec_realtek snd_hda_codec_generic mei_wdt coretemp x86_pkg_temp_t kvm 
snd_hda_codec snd_hda_core irqbypass crct10dif_pclmul crc32_pclmul snd_hwdep 
xhci_pci efi_pstore snd_seq ghash_clmulni_intel xhci_hcd intel_cstate nv pi 
psmouse vgastate intel_rapl_perf efivars i2c_i801 rtsx_pci sg nvram snd_timer 
fb_ddc usbcore snd mei_me intel_pch_thermal soundcore rfkill tpm_crb hwm ipath 
mmc_block mmc_core dm_snapshot dm_bufio dm_mirror dm_region_hash dm_log 
dm_crypt dm_mod async_raid6_recov async_pq async_xor async_memcpy async_tx 
blowfish_x86_64 blowfish_common crc32c_intel bcache aesni_intel aes_x86_64 
input_leds ptp crypto_simd i915 cryptd glue_helper serio_raw pps_core shpchp 
thermal evdev [last unloaded: cfg80211]
CPU: 1 PID: 22824 Comm: modprobe Tainted: GW  OE   
4.12.10-amd64-preempt-sysrq-20170406 #1
Hardware name: LENOVO 20ERCTO1WW/20ERCTO1WW, BIOS N1DET41W (1.15 ) 12/31/2015
task: 8ba2de876000 task.stack: a0f213c34000
RIP: 0010:iwl_trans_pcie_grab_nic_access+0xc0/0xd7 [iwlwifi]
RSP: 0018:a0f213c37a30 EFLAGS: 00010086
RAX: 003d RBX: 8ba2bd560018 RCX: 0007
RDX:  RSI: 0002 RDI: 8ba52144dd60
RBP: a0f213c37a48 R08: 0002 R09: 
R10:  R11: a1f34e68 R12: 8ba2bd568f20
R13: a0f213c37a68 R14: 8ba4fc62e0a0 R15: 
FS:  7ff1c9816700() GS:8ba52144() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 56441582aed8 CR3: 0001b59e8000 CR4: 003406e0
DR0:  DR1:  DR2: 
DR3:  DR6: fffe0ff0 DR7: 0400
Call Trace:
 iwl_trans_pcie_alloc+0x2f2/0x7ac [iwlwifi]
 ? mutex_unlock+0x22/0x34
 iwl_pci_probe+0x21/0x2ce [iwlwifi]
 ? _raw_spin_unlock_irqrestore+0x14/0x24
 ? __pm_runtime_resume+0x4d/0x58
 local_pci_probe+0x3d/0x80
 pci_device_probe+0x10c/0x13b
 driver_probe_device+0x19b/0x3f6
 __driver_attach+0x80/0xdb
 ? driver_probe_device+0x3f6/0x3f6
 bus_for_each_dev+0x5d/0x85
 driver_attach+0x1e/0x20
 bus_add_driver+0xfd/0x239
 driver_register+0x88/0xbf
 ? 0xc094
 __pci_register_driver+0x4c/0x4e
 iwl_pci_register_driver+0x24/0x3b [iwlwifi]
 ? 0xc094
 iwl_drv_init+0x65/0x67 [iwlwifi]
 do_one_initcall+0x9f/0x156
 ? slab_pre_alloc_hook+0x1a/0x44
 ? slab_post_alloc_hook.isra.47+0xe/0x1d
 ? kmem_cache_alloc_trace+0xec/0xfc
 do_init_module+0x5f/0x1f7
 load_module+0x1e53/0x257a
 ? strstarts+0x28/0x28
 SYSC_finit_module+0x8c/0xb9
 ? SYSC_finit_module+0x8c/0xb9
 SyS_finit_module+0xe/0x10
 do_syscall_64+0x6b/0x7d
 entry_SYSCALL64_slow_path+0x25/0x25
RIP: 0033:0x7ff1c9384119
RSP: 002b:7fff00565518 EFLAGS: 0246 ORIG_RAX: 0139
RAX: ffda RBX: 5619e9a47370 RCX: 

RE: [2/2] ath10k: handle tdls peer events

[Manikanta Pubbisetty]

>This patch introduces new warnings:
>
>$ ath10k-check
>drivers/net/wireless/ath/ath10k/wmi.c:4470:39: warning: incorrect type in
>argument 2 (different base types)
>drivers/net/wireless/ath/ath10k/wmi.c:4470:39:expected int [signed]
>vdev_id
>drivers/net/wireless/ath/ath10k/wmi.c:4470:39:got restricted __le32
>[usertype] vdev_id
>drivers/net/wireless/ath/ath10k/wmi.c:4481:27: warning: restricted __le32
>degrades to integer
>drivers/net/wireless/ath/ath10k/wmi.c:4481:27: warning: restricted __le32
>degrades to integer
>drivers/net/wireless/ath/ath10k/wmi.c:4481:27: warning: restricted __le32
>degrades to integer
>drivers/net/wireless/ath/ath10k/wmi.c:4492:48: warning: incorrect type in
>argument 2 (different base types)
>drivers/net/wireless/ath/ath10k/wmi.c:4492:48:expected unsigned int
>[unsigned] [usertype] vdev_id
>drivers/net/wireless/ath/ath10k/wmi.c:4492:48:got restricted __le32
>[usertype] vdev_id
>drivers/net/wireless/ath/ath10k/wmi.c:4479:19: warning: restricted __le32
>degrades to integer
>drivers/net/wireless/ath/ath10k/wmi.c:4460:6: warning: symbol
>'ath10k_wmi_handle_tdls_peer_event' was not declared. Should it be static?
>
>--
>https://patchwork.kernel.org/patch/9995131/
>
>https://wireless.wiki.kernel.org/en/developers/documentation/submittingpa
>tches

My bad!!
Thanks kalle, I will fix this and send a follow up version.

Manikanta Pubbisetty

Commit 0711d638 breaks mwifiex

[Jesse Sung]

Hi,

While working on an issue that marvell module stops connecting to AP,
bisect reveals that the issue starts to happen from commit 0711d638,
which uses wdev->ssid_len instead of wdev->current_bss to determine
if driver's .disconnect() should be called.

It happens because mwifiex_cfg80211_connect() returns -EALREADY
when it finds wdev->current_bss is valid:

if (priv->wdev.current_bss) {
[PRINT LOG]
return -EALREADY;
}

This would make cfg80211_connect() set wdev->ssid_len to 0, and thus
mwifiex_cfg80211_disconnect() won't be called by cfg80211_disconnect().

The easiest way to overcome this is

diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 0a49b88..104edb4 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -1142,7 +1142,7 @@ int cfg80211_disconnect(struct
cfg80211_registered_device *rdev,
err = cfg80211_sme_disconnect(wdev, reason);
else if (!rdev->ops->disconnect)
cfg80211_mlme_down(rdev, dev);
-   else if (wdev->ssid_len)
+   else if (wdev->ssid_len || wdev->current_bss)
err = rdev_disconnect(rdev, dev, reason);

return err;

but I'm not sure if this is a proper fix for this issue.

Thanks,
Jesse

Re: [PATCH v2] ath10k: Retry pci probe on failure.

[Kalle Valo]

Ben Greear  writes:

> On 10/13/2017 08:50 AM, Adrian Chadd wrote:
>> On 13 October 2017 at 05:41, Kalle Valo  wrote:
>>> gree...@candelatech.com writes:
>>>
 From: Ben Greear 

 This works around a problem we see when sometimes the wifi NIC does
 not respond the first time.  This seems to happen especially often on
 some of the 9984 NICs in mid-range platforms.

 Signed-off-by: Ben Greear 
>>>
>>> [...]
>>>
 -static int ath10k_pci_probe(struct pci_dev *pdev,
 - const struct pci_device_id *pci_dev)
 +static int __ath10k_pci_probe(struct pci_dev *pdev,
 +   const struct pci_device_id *pci_dev)
   {
int ret = 0;
struct ath10k *ar;
 @@ -3672,6 +3672,22 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
return ret;
   }

 +static int ath10k_pci_probe(struct pci_dev *pdev,
 + const struct pci_device_id *pci_dev)
 +{
 + int cnt = 0;
 + int rv;
 + do {
 + rv = __ath10k_pci_probe(pdev, pci_dev);
 + if (rv == 0)
 + return rv;
 + pr_err("ath10k: failed to probe PCI : %d, retry-count: 
 %d\n", rv, cnt);
 + mdelay(10); /* let the ath10k firmware gerbil take a small 
 break */
 + } while (cnt++ < 10);
 + return rv;
 +}
>>>
>>> This is a sledgehammer approach and it causes reload for all error
>>> cases, like when hardware is broken or memory allocation is failing.
>>>
>>> When the problem happens does it always fail at the the same place? Is
>>> it hw reset or something else? It's better to retry the invidiual action
>>> than to do this hack. Or is it just some more delay needed somewhere?
>>
>> I am seeing WMI timeouts during initial firmware load and wait on
>> QCA9984 + BCM7444S SoC.
>> My guess is the WMI wakeup time is not "right" enough and needs to be
>> extended a little bit.
>>
>> But then, I have played a lot of whackamole with WMI timeouts during
>> my lng porting effort..
>
> The failure I saw was a failure to wake pci, and from comments, it seems that
> the current wait is longer than what should be required, and it warns on slow
> wakes, and I never saw that warning.  So I assume that waiting longer would 
> not help.
>
> I saw it fail twice in a row to wake pci and then succeed on the third
> try, for instance,
> when testing my patch.
>
> As for a big hammer, I guess we could check for certain return codes if you 
> think
> that is better than just retrying all failures?

ath10k_pci_probe() has a lots of stuff which should not affect your
problem, like allocating memory, setting up timers and interrupts etc.
It's quite ugly to redo that in every cycle. A more fine grained
solution, like looping specific action (reset, wake whatever) is much
more preferred.

Do you have debug logs of failing cases?

-- 
Kalle Valo

Re: [PATCH] wireless: qtnfmac: Convert timers to use timer_setup()

[Sergey Matyukevich]

> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: Igor Mitsyanko 
> Cc: Avinash Patil 
> Cc: Sergey Matyukevich 
> Cc: Kalle Valo 
> Cc: Kamlesh Rath 
> Cc: linux-wireless@vger.kernel.org
> Cc: net...@vger.kernel.org
> Signed-off-by: Kees Cook 
> ---
>  drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 7 +++
>  drivers/net/wireless/quantenna/qtnfmac/core.c | 2 +-
>  2 files changed, 4 insertions(+), 5 deletions(-)

Reviewed-by: Sergey Matyukevich 

Thanks!
Sergey

Re: [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs

[Toke Høiland-Jørgensen]

Johannes Berg  writes:

>> Only ath9k currently sets the AIRTIME_ACCOUNTING flag.
>
> I think you should actually make that a separate patch.
>
> In the previous patch that's not possible or it breaks things, but
> this patch just adds a new feature that drivers _may_ use, they don't
> have to since it doesn't change the API, so you should split the
> patches.

Yeah, I did that initially. The reason I ended up squashing them is that
this patch moved the per-station 'airtime' debugfs-entry that was
previously created by ath9k into mac80211. I assumed it would create
problems if both the driver and mac80211 tried to create the same file;
not sure how to handle that if it's split into two patches?

> [snip ath9k, I don't really know anything about it]
>
>> --- a/include/net/mac80211.h
>> +++ b/include/net/mac80211.h
>> @@ -1202,6 +1202,7 @@ struct ieee80211_rx_status {
>>  u32 device_timestamp;
>>  u32 ampdu_reference;
>>  u32 flag;
>> +u16 rx_time;
>
> I'd prefer this were called "airtime" or such,

Right. I picked rx_time for symmetry with the tx side; should I rename
that as well, then, or is asymmetry fine?

> and you clearly need to add documentation regarding this field. In
> particular, you should point to the IEEE80211_HW_AIRTIME_ACCOUNTING hw
> flag, and document what should be taken into account here (preamble
> length, IFS, rts/cts/ack, etc.?) and additionally how this should be
> handled wrt. A-MPDU (and A- MSDU where decapsulated by the device.)

Ah yes, of course; will add some documentation.

> For aggregation, either form, I expect you just want to see 0 for all
> but the very first frame? but that may be very difficult for some
> drivers to implement.

Well yeah, either report the airtime of the whole aggregate on the first
frame and zero on the rest, or report the aggregate overhead on the
first frame and the data+padding time on each frame. Depends on the
information the driver has handy / can calculate, I guess.

>> +++ b/net/mac80211/rx.c
>> @@ -1637,6 +1637,15 @@ ieee80211_rx_h_sta_process(struct
>> ieee80211_rx_data *rx)
>>  if (ieee80211_vif_is_mesh(>sdata->vif))
>>  ieee80211_mps_rx_h_sta_process(sta, hdr);
>>  
>> +/* airtime accounting */
>> +if (ieee80211_hw_check(>local->hw, AIRTIME_ACCOUNTING)
>> &&
>
> Is there much point in this check? I think we can assume drivers are
> well-behaved and just leave the field at 0 if they don't support it?

I guess it could be omitted for the RX side, yeah. I added the flag
because the tx_time field overlaps with other fields in the tx_info
struct and so can be non-zero even if the driver doesn't add airtime
information (but seems I botched the TX side check as you noted below).

>> +status->rx_time) {
>> +spin_lock_bh(>lock);
>> +sta->airtime_stats.rx_airtime += status->rx_time;
>> +sta->airtime_deficit -= status->rx_time;
>> +spin_unlock_bh(>lock);
>> +}
>
> I can't say I'm a big fan of the locking here, we have multi-queue RX
> where we spread the load across multiple CPUs, and this will lead to
> massive cache-line bouncing. Maybe we can make it per-CPU or
> something?

A per-CPU counter on RX and a separate task that sums those into the
global counter, perhaps?

> This gets tricky though, so perhaps we can defer that to a separate
> patch to make it multi-queue aware.

ACK. I also don't have any hardware to test this, so would probably be
good to leave that as a separate entry once we're convinced that the
rest is correct.

> Perhaps for such drivers it'd be better anyway to report the used RX
> airtime per station *separately*, as part of some kind of statistics
> API, rather than inline through RX - there's no need for the inline
> reporting after all as long as the values are updated frequently
> enough.

Ah yes, that would be an option as well.

>> +++ b/net/mac80211/status.c
>> @@ -823,6 +823,13 @@ static void __ieee80211_tx_status(struct
>> ieee80211_hw *hw,
>>  ieee80211_lost_packet(sta, info);
>>  }
>>  }
>> +
>> +if (info->status.tx_time) {
>> +spin_lock_bh(>lock);
>> +sta->airtime_stats.tx_airtime += info->status.tx_time;
>> +sta->airtime_deficit -= info->status.tx_time;
>> +spin_unlock_bh(>lock);
>> +}
>>  }
>
> Those lines also seem pretty long, and the concerns from above apply.
>
> Here you also don't have the hw_check,

Yeah, oops. :)

[snip]

Will fix the rest of your comments and resend. Thanks!

-Toke

Re: [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs

[Johannes Berg]

> Only ath9k currently sets the AIRTIME_ACCOUNTING flag.

I think you should actually make that a separate patch.

In the previous patch that's not possible or it breaks things, but this
patch just adds a new feature that drivers _may_ use, they don't have
to since it doesn't change the API, so you should split the patches.

[snip ath9k, I don't really know anything about it]

> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1202,6 +1202,7 @@ struct ieee80211_rx_status {
>   u32 device_timestamp;
>   u32 ampdu_reference;
>   u32 flag;
> + u16 rx_time;

I'd prefer this were called "airtime" or such, and you clearly need to
add documentation regarding this field. In particular, you should point
to the IEEE80211_HW_AIRTIME_ACCOUNTING hw flag, and document what
should be taken into account here (preamble length, IFS, rts/cts/ack,
etc.?) and additionally how this should be handled wrt. A-MPDU (and A-
MSDU where decapsulated by the device.)

For aggregation, either form, I expect you just want to see 0 for all
but the very first frame? but that may be very difficult for some
drivers to implement.

> +++ b/net/mac80211/rx.c
> @@ -1637,6 +1637,15 @@ ieee80211_rx_h_sta_process(struct
> ieee80211_rx_data *rx)
>   if (ieee80211_vif_is_mesh(>sdata->vif))
>   ieee80211_mps_rx_h_sta_process(sta, hdr);
>  
> + /* airtime accounting */
> + if (ieee80211_hw_check(>local->hw, AIRTIME_ACCOUNTING)
> &&

Is there much point in this check? I think we can assume drivers are
well-behaved and just leave the field at 0 if they don't support it?

> + status->rx_time) {
> + spin_lock_bh(>lock);
> + sta->airtime_stats.rx_airtime += status->rx_time;
> + sta->airtime_deficit -= status->rx_time;
> + spin_unlock_bh(>lock);
> + }

I can't say I'm a big fan of the locking here, we have multi-queue RX
where we spread the load across multiple CPUs, and this will lead to
massive cache-line bouncing. Maybe we can make it per-CPU or something?

This gets tricky though, so perhaps we can defer that to a separate
patch to make it multi-queue aware.

Perhaps for such drivers it'd be better anyway to report the used RX
airtime per station *separately*, as part of some kind of statistics
API, rather than inline through RX - there's no need for the inline
reporting after all as long as the values are updated frequently
enough.

> +++ b/net/mac80211/status.c
> @@ -823,6 +823,13 @@ static void __ieee80211_tx_status(struct
> ieee80211_hw *hw,
>   ieee80211_lost_packet(sta, info);
>   }
>   }
> +
> + if (info->status.tx_time) {
> + spin_lock_bh(>lock);
> + sta->airtime_stats.tx_airtime += info->status.tx_time;
> + sta->airtime_deficit -= info->status.tx_time;
> + spin_unlock_bh(>lock);
> + }
>   }

Those lines also seem pretty long, and the concerns from above apply.

Here you also don't have the hw_check,

>   /* SNMP counters
> @@ -947,6 +954,14 @@ void ieee80211_tx_status_ext(struct ieee80211_hw
> *hw,
>   sta->status_stats.retry_failed++;
>   sta->status_stats.retry_count += retry_count;
>  
> + if (ieee80211_hw_check(>hw, AIRTIME_ACCOUNTING) &&

but here you do?

tx_time already existed anyway though, for other purposes, so you
probably need the check (or do useless work if the driver uses tx_time
for WMM-AC but not for airtime accounting).

>   if (list_empty(>schedule_order)) {
> - list_add_tail(>schedule_order, >active_txqs);
> + list_add_tail(>schedule_order, >active_txqs_new);
>   ret = 1;
>   }

lines seem long?

> + txqi = list_first_entry(head, struct txq_info,
> schedule_order);
> +
> + if (txqi->txq.sta) {
> + struct sta_info *sta = container_of(txqi->txq.sta,
> struct sta_info, sta);
> +
> + spin_lock_bh(>lock);
> + if (sta->airtime_deficit < 0) {
> + sta->airtime_deficit +=
> IEEE80211_AIRTIME_QUANTUM;
> + list_move_tail(>schedule_order,
> >active_txqs_old);
> + spin_unlock_bh(>lock);
> + goto begin;
> + }
> + spin_unlock_bh(>lock);
> + }

ditto here.

johannes

Re: [PATCH 1/2] mac80211: Add TXQ scheduling API

[Johannes Berg]

>   /* protects shared structure data */
>   spinlock_t data_lock;
> - /* protects: ar->txqs, artxq->list */
> - spinlock_t txqs_lock;
>  
> - struct list_head txqs;

I don't see you removing the artxq->list member, but surely it can't be
used any more now, without a list?

[snip] that's about all I can comment on ath*k :)

> +/**
> + * ieee80211_schedule_txq - add txq to scheduling loop
> + *
> + * @hw: pointer as obtained from ieee80211_alloc_hw()
> + * @txq: pointer obtained from station or virtual interface

or more likely next_txq()? :)

> + * Returns true if the txq was actually added to the scheduling,
> + * false otherwise.

Perhaps use %true/%false.

>   if (sta->sta.txq[0]) {
> + bool wake = false;

please add a blank line after this new line

>   for (i = 0; i < ARRAY_SIZE(sta->sta.txq); i++) {
>   if (!txq_has_queue(sta->sta.txq[i]))
>   continue;
>  
> - drv_wake_tx_queue(local, to_txq_info(sta->sta.txq[i]));
> + wake = wake || ieee80211_schedule_txq(>hw, 
> sta->sta.txq[i]);
>   }
> + if (wake)
> + drv_wake_tx_queue(local);
>   }

Are you sure you want to skip the call to ieee80211_schedule_txq() if
wake is true? I think you don't, but boolean short-circuit evaluation
would lead to that, afaict?

So it better be written as

  if (ieee80211_schedule_txq(...))
wake = true;

No need to even check wake anyway, since it can only ever go from false
to true.

(Also, that line is getting a bit long)

>   skb_queue_head_init();
> diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
> index 3d9ac17af407..531c0e7f2358 100644
> --- a/net/mac80211/trace.h
> +++ b/net/mac80211/trace.h
> @@ -2550,33 +2550,20 @@ TRACE_EVENT(drv_tdls_recv_channel_switch,
>  );
>  
>  TRACE_EVENT(drv_wake_tx_queue,
> - TP_PROTO(struct ieee80211_local *local,
> -  struct ieee80211_sub_if_data *sdata,
> -  struct txq_info *txq),
> + TP_PROTO(struct ieee80211_local *local),

You should just replace all of this with

DEFINE_EVENT(local_only_evt, drv_wake_tx_queue,
TP_PROTO(struct ieee80211_local *local),
TP_ARGS(local)
);

now.

> +bool ieee80211_schedule_txq(struct ieee80211_hw *hw,
> +  struct ieee80211_txq *txq)
> +{
> + struct ieee80211_local *local = hw_to_local(hw);
> + struct txq_info *txqi = to_txq_info(txq);
> + int ret = 0;

bool/false/true please.

> +struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw)
> +{
> + struct ieee80211_local *local = hw_to_local(hw);
> + struct txq_info *txqi = NULL;
> +
> + spin_lock_bh(>active_txq_lock);
> +
> + if (list_empty(>active_txqs))
> + goto out;
> +
> + txqi = list_first_entry(>active_txqs, struct txq_info, 
> schedule_order);

that line seems pretty long, but I haven't counted :)

johannes

Re: [PATCH] iw: add command to register and capture mgmt frames

[Johannes Berg]

> Maybe we could add an additional nl attribute to 
> NL80211_CMD_REGISTER_FRAME command to allow applications to
> advertise 
> what is their intention, something like
> NL80211_ATTR_MGMT_LISTENER_TYPE. 
> Only allow to register more then one listener if it explicitly
> specifies 
> that it will not try to answer (TYPE_LISTEN_ONLY or smth like that).
> This will preserve behavior for existing userspace.

We could, in theory, but I don't like using this API for monitoring
purposes. We really should just come up with more generic and flexible
ways of doing that, and I think the eBPF thing is pretty good there.

johannes