* Re: [PATCH v3 1/2] mt76: mt7615: enable support for mesh
From: Ryder Lee @ 2019-06-06 15:18 UTC (permalink / raw)
To: Sebastian Gottschall
Cc: Felix Fietkau, Lorenzo Bianconi, Roy Luo, YF Luo, Yiwei Chung,
Sean Wang, Chih-Min Chen, linux-wireless, linux-mediatek,
linux-kernel
In-Reply-To: <a0a6f631-2eb1-87cc-5653-338c6126690c@newmedia-net.de>
On Thu, 2019-06-06 at 12:14 +0200, Sebastian Gottschall wrote:
> in addition you should take care about this problem which is raised up
> if SAE is used. since AES-CMAC required tid to be non zero
>
> WARNING: CPU: 2 PID: 15324 at
> /home/seg/DEV/mt7621/src/router/private/compat-wireless-2017-09-03/net/mac80211/key.c:1096
> mt76_wcid_key_setup+0x58/0x9c [mt76]
> Modules linked in: shortcut_fe gcm ghash_generic ctr gf128mul mt7615e
> mt76 mac80211 compat
> CPU: 2 PID: 15324 Comm: wpa_supplicant Tainted: G W 4.14.123 #106
> Stack : 00000000 87c2d000 00000000 8007d8b4 80480000 80482b9c 80610000
> 805a4390
> 8057e2b4 854fb99c 87ed045c 805e4767 80578288 00000001 854fb940
> 805e9f78
> 00000000 00000000 80640000 00000000 81147bb8 00000584 00000007
> 00000000
> 00000000 80650000 80650000 20202020 80000000 00000000 80610000
> 872b9fe0
> 872a2b14 00000448 00000000 87c2d000 00000010 8022d660 00000008
> 80640008
> ...
> Call Trace:
> [<800153e0>] show_stack+0x58/0x100
> [<8042e83c>] dump_stack+0x9c/0xe0
> [<800349f0>] __warn+0xe4/0x144
> [<8003468c>] warn_slowpath_null+0x1c/0x30
> [<872b9fe0>] mt76_wcid_key_setup+0x58/0x9c [mt76]
> [<87611690>] mt7615_eeprom_init+0x7b4/0xe9c [mt7615e]
> ---[ end trace e24aeb4b542e0dea ]---
This is fixed by Lorenzo's patch -
https://patchwork.kernel.org/patch/10976191/
Thanks.
Ryder
^ permalink raw reply
* [PATCH] mt7601u: fix possible memory leak when the device is disconnected
From: Lorenzo Bianconi @ 2019-06-06 15:34 UTC (permalink / raw)
To: kubakici; +Cc: linux-wireless, lorenzo.bianconi
When the device is disconnected while passing traffic it is possible
to receive out of order urbs causing a memory leak since the skb liked
to the current tx urb is not removed. Fix the issue deallocating the skb
cleaning up the tx ring. Moreover this patch fixes the following kernel
warning
[ 57.480771] usb 1-1: USB disconnect, device number 2
[ 57.483451] ------------[ cut here ]------------
[ 57.483462] TX urb mismatch
[ 57.483481] WARNING: CPU: 1 PID: 32 at drivers/net/wireless/mediatek/mt7601u/dma.c:245 mt7601u_complete_tx+0x165/00
[ 57.483483] Modules linked in:
[ 57.483496] CPU: 1 PID: 32 Comm: kworker/1:1 Not tainted 5.2.0-rc1+ #72
[ 57.483498] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-2.fc30 04/01/2014
[ 57.483502] Workqueue: usb_hub_wq hub_event
[ 57.483507] RIP: 0010:mt7601u_complete_tx+0x165/0x1e0
[ 57.483510] Code: 8b b5 10 04 00 00 8b 8d 14 04 00 00 eb 8b 80 3d b1 cb e1 00 00 75 9e 48 c7 c7 a4 ea 05 82 c6 05 f
[ 57.483513] RSP: 0000:ffffc900000a0d28 EFLAGS: 00010092
[ 57.483516] RAX: 000000000000000f RBX: ffff88802c0a62c0 RCX: ffffc900000a0c2c
[ 57.483518] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff810a8371
[ 57.483520] RBP: ffff88803ced6858 R08: 0000000000000000 R09: 0000000000000001
[ 57.483540] R10: 0000000000000002 R11: 0000000000000000 R12: 0000000000000046
[ 57.483542] R13: ffff88802c0a6c88 R14: ffff88803baab540 R15: ffff88803a0cc078
[ 57.483548] FS: 0000000000000000(0000) GS:ffff88803eb00000(0000) knlGS:0000000000000000
[ 57.483550] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 57.483552] CR2: 000055e7f6780100 CR3: 0000000028c86000 CR4: 00000000000006a0
[ 57.483554] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 57.483556] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 57.483559] Call Trace:
[ 57.483561] <IRQ>
[ 57.483565] __usb_hcd_giveback_urb+0x77/0xe0
[ 57.483570] xhci_giveback_urb_in_irq.isra.0+0x8b/0x140
[ 57.483574] handle_cmd_completion+0xf5b/0x12c0
[ 57.483577] xhci_irq+0x1f6/0x1810
[ 57.483581] ? lockdep_hardirqs_on+0x9e/0x180
[ 57.483584] ? _raw_spin_unlock_irq+0x24/0x30
[ 57.483588] __handle_irq_event_percpu+0x3a/0x260
[ 57.483592] handle_irq_event_percpu+0x1c/0x60
[ 57.483595] handle_irq_event+0x2f/0x4c
[ 57.483599] handle_edge_irq+0x7e/0x1a0
[ 57.483603] handle_irq+0x17/0x20
[ 57.483607] do_IRQ+0x54/0x110
[ 57.483610] common_interrupt+0xf/0xf
[ 57.483612] </IRQ>
Fixes: c869f77d6abb ("add mt7601u driver")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
drivers/net/wireless/mediatek/mt7601u/dma.c | 24 +++++++++++++++------
drivers/net/wireless/mediatek/mt7601u/tx.c | 4 ++--
2 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c
index e7703990b291..bbf1deed7f3b 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -238,14 +238,25 @@ static void mt7601u_complete_tx(struct urb *urb)
struct sk_buff *skb;
unsigned long flags;
- spin_lock_irqsave(&dev->tx_lock, flags);
+ switch (urb->status) {
+ case -ECONNRESET:
+ case -ESHUTDOWN:
+ case -ENOENT:
+ return;
+ default:
+ dev_err_ratelimited(dev->dev, "tx urb failed: %d\n",
+ urb->status);
+ /* fall through */
+ case 0:
+ break;
+ }
- if (mt7601u_urb_has_error(urb))
- dev_err(dev->dev, "Error: TX urb failed:%d\n", urb->status);
+ spin_lock_irqsave(&dev->tx_lock, flags);
if (WARN_ONCE(q->e[q->start].urb != urb, "TX urb mismatch"))
goto out;
skb = q->e[q->start].skb;
+ q->e[q->start].skb = NULL;
trace_mt_tx_dma_done(dev, skb);
__skb_queue_tail(&dev->tx_skb_done, skb);
@@ -446,10 +457,10 @@ static void mt7601u_free_tx_queue(struct mt7601u_tx_queue *q)
{
int i;
- WARN_ON(q->used);
-
for (i = 0; i < q->entries; i++) {
usb_poison_urb(q->e[i].urb);
+ if (q->e[i].skb)
+ mt7601u_tx_status(q->dev, q->e[i].skb);
usb_free_urb(q->e[i].urb);
}
}
@@ -463,6 +474,7 @@ static void mt7601u_free_tx(struct mt7601u_dev *dev)
for (i = 0; i < __MT_EP_OUT_MAX; i++)
mt7601u_free_tx_queue(&dev->tx_q[i]);
+ tasklet_kill(&dev->tx_tasklet);
}
static int mt7601u_alloc_tx_queue(struct mt7601u_dev *dev,
@@ -528,6 +540,4 @@ void mt7601u_dma_cleanup(struct mt7601u_dev *dev)
mt7601u_free_rx(dev);
mt7601u_free_tx(dev);
-
- tasklet_kill(&dev->tx_tasklet);
}
diff --git a/drivers/net/wireless/mediatek/mt7601u/tx.c b/drivers/net/wireless/mediatek/mt7601u/tx.c
index 3600e911a63e..4d81c45722fb 100644
--- a/drivers/net/wireless/mediatek/mt7601u/tx.c
+++ b/drivers/net/wireless/mediatek/mt7601u/tx.c
@@ -117,9 +117,9 @@ void mt7601u_tx_status(struct mt7601u_dev *dev, struct sk_buff *skb)
info->status.rates[0].idx = -1;
info->flags |= IEEE80211_TX_STAT_ACK;
- spin_lock(&dev->mac_lock);
+ spin_lock_bh(&dev->mac_lock);
ieee80211_tx_status(dev->hw, skb);
- spin_unlock(&dev->mac_lock);
+ spin_unlock_bh(&dev->mac_lock);
}
static int mt7601u_skb_rooms(struct mt7601u_dev *dev, struct sk_buff *skb)
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v3 1/2] mt76: mt7615: enable support for mesh
From: Lorenzo Bianconi @ 2019-06-06 16:19 UTC (permalink / raw)
To: Sebastian Gottschall
Cc: Ryder Lee, Felix Fietkau, Roy Luo, YF Luo, Yiwei Chung, Sean Wang,
Chih-Min Chen, linux-wireless, linux-mediatek,
Linux Kernel Mailing List
In-Reply-To: <ade7ef01-8b06-ec7d-4caf-e581f4033819@newmedia-net.de>
>
> i tested your patch against a qca 9984 chipset using SAE and without
> encryption. both did not work. the devices are connecting, but no data
> connection is possible
Hi Sebastian,
I tested Ryder's patch using mt76x2 as mesh peer and it works fine for me.
Could you please provide some more info?
Regards,
Lorenzo
>
>
> Sebastian
>
> Am 03.06.2019 um 08:08 schrieb Ryder Lee:
> > Enable NL80211_IFTYPE_MESH_POINT and update its path.
> >
> > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> > ---
> > Changes since v3 - fix a wrong expression
> > Changes since v2 - remove unused definitions
> > ---
> > drivers/net/wireless/mediatek/mt76/mt7615/init.c | 6 ++++++
> > drivers/net/wireless/mediatek/mt76/mt7615/main.c | 1 +
> > drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 4 +++-
> > drivers/net/wireless/mediatek/mt76/mt7615/mcu.h | 6 ------
> > 4 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/init.c b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
> > index 59f604f3161f..f860af6a42da 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7615/init.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
> > @@ -133,6 +133,9 @@ static const struct ieee80211_iface_limit if_limits[] = {
> > {
> > .max = MT7615_MAX_INTERFACES,
> > .types = BIT(NL80211_IFTYPE_AP) |
> > +#ifdef CONFIG_MAC80211_MESH
> > + BIT(NL80211_IFTYPE_MESH_POINT) |
> > +#endif
> > BIT(NL80211_IFTYPE_STATION)
> > }
> > };
> > @@ -195,6 +198,9 @@ int mt7615_register_device(struct mt7615_dev *dev)
> > dev->mt76.antenna_mask = 0xf;
> >
> > wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
> > +#ifdef CONFIG_MAC80211_MESH
> > + BIT(NL80211_IFTYPE_MESH_POINT) |
> > +#endif
> > BIT(NL80211_IFTYPE_AP);
> >
> > ret = mt76_register_device(&dev->mt76, true, mt7615_rates,
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/main.c b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> > index b0bb7cc12385..585e67fa2728 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
> > @@ -37,6 +37,7 @@ static int get_omac_idx(enum nl80211_iftype type, u32 mask)
> >
> > switch (type) {
> > case NL80211_IFTYPE_AP:
> > + case NL80211_IFTYPE_MESH_POINT:
> > /* ap use hw bssid 0 and ext bssid */
> > if (~mask & BIT(HW_BSSID_0))
> > return HW_BSSID_0;
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > index 43f70195244c..e82297048449 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> > @@ -754,6 +754,7 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
> >
> > switch (vif->type) {
> > case NL80211_IFTYPE_AP:
> > + case NL80211_IFTYPE_MESH_POINT:
> > tx_wlan_idx = mvif->sta.wcid.idx;
> > conn_type = CONNECTION_INFRA_AP;
> > break;
> > @@ -968,7 +969,7 @@ int mt7615_mcu_add_wtbl(struct mt7615_dev *dev, struct ieee80211_vif *vif,
> > .rx_wtbl = {
> > .tag = cpu_to_le16(WTBL_RX),
> > .len = cpu_to_le16(sizeof(struct wtbl_rx)),
> > - .rca1 = vif->type != NL80211_IFTYPE_AP,
> > + .rca1 = vif->type == NL80211_IFTYPE_STATION,
> > .rca2 = 1,
> > .rv = 1,
> > },
> > @@ -1042,6 +1043,7 @@ static void sta_rec_convert_vif_type(enum nl80211_iftype type, u32 *conn_type)
> > {
> > switch (type) {
> > case NL80211_IFTYPE_AP:
> > + case NL80211_IFTYPE_MESH_POINT:
> > if (conn_type)
> > *conn_type = CONNECTION_INFRA_STA;
> > break;
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
> > index e96efb13fa4d..0915cb735699 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
> > @@ -105,25 +105,19 @@ enum {
> > #define STA_TYPE_STA BIT(0)
> > #define STA_TYPE_AP BIT(1)
> > #define STA_TYPE_ADHOC BIT(2)
> > -#define STA_TYPE_TDLS BIT(3)
> > #define STA_TYPE_WDS BIT(4)
> > #define STA_TYPE_BC BIT(5)
> >
> > #define NETWORK_INFRA BIT(16)
> > #define NETWORK_P2P BIT(17)
> > #define NETWORK_IBSS BIT(18)
> > -#define NETWORK_MESH BIT(19)
> > -#define NETWORK_BOW BIT(20)
> > #define NETWORK_WDS BIT(21)
> >
> > #define CONNECTION_INFRA_STA (STA_TYPE_STA | NETWORK_INFRA)
> > #define CONNECTION_INFRA_AP (STA_TYPE_AP | NETWORK_INFRA)
> > #define CONNECTION_P2P_GC (STA_TYPE_STA | NETWORK_P2P)
> > #define CONNECTION_P2P_GO (STA_TYPE_AP | NETWORK_P2P)
> > -#define CONNECTION_MESH_STA (STA_TYPE_STA | NETWORK_MESH)
> > -#define CONNECTION_MESH_AP (STA_TYPE_AP | NETWORK_MESH)
> > #define CONNECTION_IBSS_ADHOC (STA_TYPE_ADHOC | NETWORK_IBSS)
> > -#define CONNECTION_TDLS (STA_TYPE_STA | NETWORK_INFRA | STA_TYPE_TDLS)
> > #define CONNECTION_WDS (STA_TYPE_WDS | NETWORK_WDS)
> > #define CONNECTION_INFRA_BC (STA_TYPE_BC | NETWORK_INFRA)
> >
^ permalink raw reply
* Help with encrypting PMF management frames
From: Ben Greear @ 2019-06-06 16:54 UTC (permalink / raw)
To: linux-wireless@vger.kernel.org
Hello,
My variant of ath10k uses the normal 'native-wifi' tx path for management frames.
Internally in the firmware, it seems that the management TID is flagged to expect
raw frames, and I think that is why I see Action frames on-air that are not actually
encrypted but which have some space added to their packet that should be filled in by
the hw-crypt engine.
Is there a way to get mac80211 to software-crypt just management-tid PMF frames?
So far, I have not been able to find the correct place in the tx logic of
mac80211...
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH v3] {nl,mac}80211: allow 4addr AP operation on crypto controlled devices
From: Tom Psyborg @ 2019-06-06 18:41 UTC (permalink / raw)
To: Manikanta Pubbisetty; +Cc: Johannes Berg, linux-wireless
In-Reply-To: <c8484254-f4f7-9955-e3f8-8a423cc6c325@codeaurora.org>
On 31/05/2019, Manikanta Pubbisetty <mpubbise@codeaurora.org> wrote:
>
> On 5/14/2019 2:08 PM, Johannes Berg wrote:
>> On Wed, 2019-05-08 at 14:55 +0530, Manikanta Pubbisetty wrote:
>>> +++ b/net/mac80211/util.c
>>> @@ -3795,7 +3795,9 @@ int ieee80211_check_combinations(struct
>>> ieee80211_sub_if_data *sdata,
>>> }
>>>
>>> /* Always allow software iftypes */
>>> - if (local->hw.wiphy->software_iftypes & BIT(iftype)) {
>>> + if (local->hw.wiphy->software_iftypes & BIT(iftype) ||
>>> + (iftype == NL80211_IFTYPE_AP_VLAN &&
>>> + local->hw.wiphy->flags & WIPHY_FLAG_4ADDR_AP)) {
>>> if (radar_detect)
>>> return -EINVAL;
>> Shouldn't this check if 4addr is actually enabled too, like here:
>
>
> Sure Johannes, I'll look into it.
>
>
>>> case NETDEV_PRE_UP:
>>> - if (!(wdev->wiphy->interface_modes & BIT(wdev->iftype)))
>>> + if (!(wdev->wiphy->interface_modes & BIT(wdev->iftype)) &&
>>> + !(wdev->iftype == NL80211_IFTYPE_AP_VLAN &&
>>> + rdev->wiphy.flags & WIPHY_FLAG_4ADDR_AP &&
>>> + wdev->use_4addr))
>>> return notifier_from_errno(-EOPNOTSUPP);
>> ?
>> Or is there some reason it doesn't matter?
>>
>>> @@ -3439,6 +3438,11 @@ static int nl80211_new_interface(struct sk_buff
>>> *skb, struct genl_info *info)
>>> return err;
>>> }
>>>
>>> + if (!(rdev->wiphy.interface_modes & (1 << type)) &&
>>> + !(type == NL80211_IFTYPE_AP_VLAN && params.use_4addr &&
>>> + rdev->wiphy.flags & WIPHY_FLAG_4ADDR_AP))
>>> + return -EOPNOTSUPP;
>>> +
>> I also wonder if we shouldn't go "all in" and actually make the check
>> something like
>>
>> check_interface_allowed(iftype, 4addr):
>> if (iftype == AP_VLAN && 4addr)
>> return wiphy.flags & WIPHY_FLAG_4ADDR_AP;
>>
>> else return wiphy.interface_modes & BIT(iftype);
>>
>> i.e. make it "you must have WIPHY_FLAG_4ADDR_AP to use 4-addr AP_VLAN
>> interfaces", rather than "also allow it in this case".
>>
>> That would seem like the clearer semantics to me?
>
>
> Yeah, it can be better; I'll check if this is feasible.
>
>
> Thanks,
>
> Manikanta
>
>
Hi
Applying this patch instead of v1 broke WDS operation between two
Litebeam AC Gen2 devices:
Thu Jun 6 19:38:43 2019 kern.info kernel: [ 625.840896] device
wlan0.sta1 left promiscuous mode
Thu Jun 6 19:38:43 2019 kern.info kernel: [ 625.846146] br-lan: port
3(wlan0.sta1) entered disabled state
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.854349]
------------[ cut here ]------------
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.859253] WARNING:
CPU: 0 PID: 1417 at backports-4.19.32-1/net/mac80211/key.c:907
ieee80211_free_keys+0x170/0x228 [mac80211]
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.870871] Modules
linked in: ath9k ath9k_common pppoe ppp_async ath9k_hw ath10k_pci
ath10k_core ath pppox ppp_generic mac80211 iptable_nat iptable_mangle
iptable_filter ipt_REJECT ipt_MASQUERADE ip_tables cfg80211 xt_time
xt_tcpudp xt_state xt_nat xt_multiport xt_mark xt_mac xt_limit
xt_conntrack xt_comment xt_TCPMSS xt_REDIRECT xt_LOG xt_FLOWOFFLOAD
x_tables thermal_sys slhc nf_reject_ipv4 nf_nat_redirect
nf_nat_masquerade_ipv4 nf_conntrack_ipv4 nf_nat_ipv4 nf_nat
nf_log_ipv4 nf_log_common nf_flow_table_hw nf_flow_table
nf_defrag_ipv4 nf_conntrack_rtcache nf_conntrack hwmon crc_ccitt
compat ehci_platform ehci_hcd gpio_button_hotplug usbcore nls_base
usb_common
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.930674] CPU: 0 PID:
1417 Comm: hostapd Not tainted 4.14.118 #0
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.936958] Stack :
804d0000 80489150 00000000 00000000 80460fc0 82e79a9c 82e9d35c
804b1307
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.945494]
8045d190 00000589 80603670 0000038b 804838a0 00000001 82e79a50
688195c4
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.954197]
00000000 00000000 80600000 00003dd0 00000000 00000000 00000008
00000000
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.962745]
000000c8 d9e4e916 000000c7 00000000 80000000 00000000 83266130
83234cfc
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.971247]
00000009 0000038b 804838a0 00000100 00000001 8026bd44 00000000
80600000
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.979754] ...
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.982249] Call Trace:
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.984764] [<8006a9ec>]
show_stack+0x58/0x100
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.989305] [<80085080>]
__warn+0xe4/0x118
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.993492] [<80085144>]
warn_slowpath_null+0x1c/0x28
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 625.998793] [<83234cfc>]
ieee80211_free_keys+0x170/0x228 [mac80211]
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 626.005308] [<8321611c>]
ieee80211_ibss_leave+0xa70/0x1940 [mac80211]
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 626.011970] [<802f4998>]
rollback_registered_many+0x2dc/0x414
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 626.017813] [<802f60b0>]
unregister_netdevice_queue+0x94/0xec
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 626.023762] [<8321fd8c>]
ieee80211_nan_func_match+0x2894/0x29a0 [mac80211]
Thu Jun 6 19:38:43 2019 kern.warn kernel: [ 626.030795] ---[ end
trace 5309fee2cf0ee39d ]---
Thu Jun 6 19:38:43 2019 daemon.notice hostapd: wlan0:
WDS-STA-INTERFACE-REMOVED ifname=wlan0.sta1 sta_addr=18:e8:29:30:9b:2c
Thu Jun 6 19:38:48 2019 daemon.info hostapd: wlan0: STA
18:e8:29:30:9b:2c IEEE 802.11: authenticated
Thu Jun 6 19:38:48 2019 daemon.info hostapd: wlan0: STA
18:e8:29:30:9b:2c IEEE 802.11: associated (aid 1)
Thu Jun 6 19:38:48 2019 kern.info kernel: [ 631.114522] br-lan: port
3(wlan0.sta1) entered blocking state
Thu Jun 6 19:38:48 2019 kern.info kernel: [ 631.120364] br-lan: port
3(wlan0.sta1) entered disabled state
Thu Jun 6 19:38:48 2019 kern.info kernel: [ 631.126603] device
wlan0.sta1 entered promiscuous mode
Thu Jun 6 19:38:48 2019 daemon.notice hostapd: wlan0:
WDS-STA-INTERFACE-ADDED ifname=wlan0.sta1 sta_addr=18:e8:29:30:9b:2c
Thu Jun 6 19:38:48 2019 daemon.err hostapd: Could not set interface
wlan0.sta1 flags (UP): Invalid argument
Thu Jun 6 19:38:48 2019 daemon.err hostapd: nl80211: Failed to set
WDS STA interface wlan0.sta1 up
Thu Jun 6 19:38:48 2019 daemon.err hostapd: nl80211:
NL80211_ATTR_STA_VLAN (addr=18:e8:29:30:9b:2c ifname=wlan0.sta1
vlan_id=0) failed: -127 (Network is down)
Thu Jun 6 19:38:48 2019 daemon.notice hostapd: wlan0:
AP-STA-CONNECTED 18:e8:29:30:9b:2c
Thu Jun 6 19:38:48 2019 daemon.info hostapd: wlan0: STA
18:e8:29:30:9b:2c WPA: pairwise key handshake completed (RSN)
Reverting v1 of the patch restored connection.
Regards, Tom
^ permalink raw reply
* Re: [PATCH] mt7601u: fix possible memory leak when the device is disconnected
From: Jakub Kicinski @ 2019-06-06 18:48 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-wireless, lorenzo.bianconi
In-Reply-To: <27c6d00cfb5936978cfb8304c6e1f03973905848.1559835089.git.lorenzo@kernel.org>
On Thu, 6 Jun 2019 17:34:16 +0200, Lorenzo Bianconi wrote:
> When the device is disconnected while passing traffic it is possible
> to receive out of order urbs causing a memory leak since the skb liked
> to the current tx urb is not removed. Fix the issue deallocating the skb
> cleaning up the tx ring. Moreover this patch fixes the following kernel
> warning
Ugh if we don't have ordering guarantees then the entire "ring" scheme
no longer works :( Should we move to URB queues, I don't remember now,
but there seem to had been a better way to manage URBs :S
> [ 57.480771] usb 1-1: USB disconnect, device number 2
> [ 57.483451] ------------[ cut here ]------------
> [ 57.483462] TX urb mismatch
> [ 57.483481] WARNING: CPU: 1 PID: 32 at drivers/net/wireless/mediatek/mt7601u/dma.c:245 mt7601u_complete_tx+0x165/00
> [ 57.483483] Modules linked in:
> [ 57.483496] CPU: 1 PID: 32 Comm: kworker/1:1 Not tainted 5.2.0-rc1+ #72
> [ 57.483498] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-2.fc30 04/01/2014
> [ 57.483502] Workqueue: usb_hub_wq hub_event
> [ 57.483507] RIP: 0010:mt7601u_complete_tx+0x165/0x1e0
> [ 57.483510] Code: 8b b5 10 04 00 00 8b 8d 14 04 00 00 eb 8b 80 3d b1 cb e1 00 00 75 9e 48 c7 c7 a4 ea 05 82 c6 05 f
> [ 57.483513] RSP: 0000:ffffc900000a0d28 EFLAGS: 00010092
> [ 57.483516] RAX: 000000000000000f RBX: ffff88802c0a62c0 RCX: ffffc900000a0c2c
> [ 57.483518] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff810a8371
> [ 57.483520] RBP: ffff88803ced6858 R08: 0000000000000000 R09: 0000000000000001
> [ 57.483540] R10: 0000000000000002 R11: 0000000000000000 R12: 0000000000000046
> [ 57.483542] R13: ffff88802c0a6c88 R14: ffff88803baab540 R15: ffff88803a0cc078
> [ 57.483548] FS: 0000000000000000(0000) GS:ffff88803eb00000(0000) knlGS:0000000000000000
> [ 57.483550] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 57.483552] CR2: 000055e7f6780100 CR3: 0000000028c86000 CR4: 00000000000006a0
> [ 57.483554] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 57.483556] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 57.483559] Call Trace:
> [ 57.483561] <IRQ>
> [ 57.483565] __usb_hcd_giveback_urb+0x77/0xe0
> [ 57.483570] xhci_giveback_urb_in_irq.isra.0+0x8b/0x140
> [ 57.483574] handle_cmd_completion+0xf5b/0x12c0
> [ 57.483577] xhci_irq+0x1f6/0x1810
> [ 57.483581] ? lockdep_hardirqs_on+0x9e/0x180
> [ 57.483584] ? _raw_spin_unlock_irq+0x24/0x30
> [ 57.483588] __handle_irq_event_percpu+0x3a/0x260
> [ 57.483592] handle_irq_event_percpu+0x1c/0x60
> [ 57.483595] handle_irq_event+0x2f/0x4c
> [ 57.483599] handle_edge_irq+0x7e/0x1a0
> [ 57.483603] handle_irq+0x17/0x20
> [ 57.483607] do_IRQ+0x54/0x110
> [ 57.483610] common_interrupt+0xf/0xf
> [ 57.483612] </IRQ>
>
> Fixes: c869f77d6abb ("add mt7601u driver")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> drivers/net/wireless/mediatek/mt7601u/dma.c | 24 +++++++++++++++------
> drivers/net/wireless/mediatek/mt7601u/tx.c | 4 ++--
> 2 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c
> index e7703990b291..bbf1deed7f3b 100644
> --- a/drivers/net/wireless/mediatek/mt7601u/dma.c
> +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
> @@ -238,14 +238,25 @@ static void mt7601u_complete_tx(struct urb *urb)
> struct sk_buff *skb;
> unsigned long flags;
>
> - spin_lock_irqsave(&dev->tx_lock, flags);
> + switch (urb->status) {
> + case -ECONNRESET:
> + case -ESHUTDOWN:
> + case -ENOENT:
> + return;
> + default:
> + dev_err_ratelimited(dev->dev, "tx urb failed: %d\n",
> + urb->status);
> + /* fall through */
> + case 0:
> + break;
> + }
>
> - if (mt7601u_urb_has_error(urb))
> - dev_err(dev->dev, "Error: TX urb failed:%d\n", urb->status);
> + spin_lock_irqsave(&dev->tx_lock, flags);
> if (WARN_ONCE(q->e[q->start].urb != urb, "TX urb mismatch"))
> goto out;
>
> skb = q->e[q->start].skb;
> + q->e[q->start].skb = NULL;
> trace_mt_tx_dma_done(dev, skb);
>
> __skb_queue_tail(&dev->tx_skb_done, skb);
> @@ -446,10 +457,10 @@ static void mt7601u_free_tx_queue(struct mt7601u_tx_queue *q)
> {
> int i;
>
> - WARN_ON(q->used);
> -
> for (i = 0; i < q->entries; i++) {
> usb_poison_urb(q->e[i].urb);
> + if (q->e[i].skb)
> + mt7601u_tx_status(q->dev, q->e[i].skb);
Perhaps a separate patch?
> usb_free_urb(q->e[i].urb);
> }
> }
> @@ -463,6 +474,7 @@ static void mt7601u_free_tx(struct mt7601u_dev *dev)
>
> for (i = 0; i < __MT_EP_OUT_MAX; i++)
> mt7601u_free_tx_queue(&dev->tx_q[i]);
> + tasklet_kill(&dev->tx_tasklet);
> }
>
> static int mt7601u_alloc_tx_queue(struct mt7601u_dev *dev,
> @@ -528,6 +540,4 @@ void mt7601u_dma_cleanup(struct mt7601u_dev *dev)
>
> mt7601u_free_rx(dev);
> mt7601u_free_tx(dev);
> -
> - tasklet_kill(&dev->tx_tasklet);
> }
> diff --git a/drivers/net/wireless/mediatek/mt7601u/tx.c b/drivers/net/wireless/mediatek/mt7601u/tx.c
> index 3600e911a63e..4d81c45722fb 100644
> --- a/drivers/net/wireless/mediatek/mt7601u/tx.c
> +++ b/drivers/net/wireless/mediatek/mt7601u/tx.c
> @@ -117,9 +117,9 @@ void mt7601u_tx_status(struct mt7601u_dev *dev, struct sk_buff *skb)
> info->status.rates[0].idx = -1;
> info->flags |= IEEE80211_TX_STAT_ACK;
>
> - spin_lock(&dev->mac_lock);
> + spin_lock_bh(&dev->mac_lock);
> ieee80211_tx_status(dev->hw, skb);
> - spin_unlock(&dev->mac_lock);
> + spin_unlock_bh(&dev->mac_lock);
> }
>
> static int mt7601u_skb_rooms(struct mt7601u_dev *dev, struct sk_buff *skb)
^ permalink raw reply
* Re: [PATCH] mt7601u: do not schedule rx_tasklet when the device has been disconnected
From: Jakub Kicinski @ 2019-06-06 18:42 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-wireless, lorenzo.bianconi
In-Reply-To: <a3c72b07e229e1d16e93438d741d69abba6a18cf.1559822991.git.lorenzo@kernel.org>
On Thu, 6 Jun 2019 14:26:12 +0200, Lorenzo Bianconi wrote:
> Do not schedule rx_tasklet when the usb dongle is disconnected. This
> patch fixes the common kernel warning reported when the device is
> removed.
>
> [ 24.921354] usb 3-14: USB disconnect, device number 7
> [ 24.921593] ------------[ cut here ]------------
> [ 24.921594] RX urb mismatch
> [ 24.921675] WARNING: CPU: 4 PID: 163 at drivers/net/wireless/mediatek/mt7601u/dma.c:200 mt7601u_complete_rx+0xcb/0xd0 [mt7601u]
> [ 24.921769] CPU: 4 PID: 163 Comm: kworker/4:2 Tainted: G OE 4.19.31-041931-generic #201903231635
> [ 24.921770] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z97 Extreme4, BIOS P1.30 05/23/2014
> [ 24.921782] Workqueue: usb_hub_wq hub_event
> [ 24.921797] RIP: 0010:mt7601u_complete_rx+0xcb/0xd0 [mt7601u]
> [ 24.921800] RSP: 0018:ffff9bd9cfd03d08 EFLAGS: 00010086
> [ 24.921802] RAX: 0000000000000000 RBX: ffff9bd9bf043540 RCX: 0000000000000006
> [ 24.921803] RDX: 0000000000000007 RSI: 0000000000000096 RDI: ffff9bd9cfd16420
> [ 24.921804] RBP: ffff9bd9cfd03d28 R08: 0000000000000002 R09: 00000000000003a8
> [ 24.921805] R10: 0000002f485fca34 R11: 0000000000000000 R12: ffff9bd9bf043c1c
> [ 24.921806] R13: ffff9bd9c62fa3c0 R14: 0000000000000082 R15: 0000000000000000
> [ 24.921807] FS: 0000000000000000(0000) GS:ffff9bd9cfd00000(0000) knlGS:0000000000000000
> [ 24.921808] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 24.921808] CR2: 00007fb2648b0000 CR3: 0000000142c0a004 CR4: 00000000001606e0
> [ 24.921809] Call Trace:
> [ 24.921812] <IRQ>
> [ 24.921819] __usb_hcd_giveback_urb+0x8b/0x140
> [ 24.921821] usb_hcd_giveback_urb+0xca/0xe0
> [ 24.921828] xhci_giveback_urb_in_irq.isra.42+0x82/0xf0
> [ 24.921834] handle_cmd_completion+0xe02/0x10d0
> [ 24.921837] xhci_irq+0x274/0x4a0
> [ 24.921838] xhci_msi_irq+0x11/0x20
> [ 24.921851] __handle_irq_event_percpu+0x44/0x190
> [ 24.921856] handle_irq_event_percpu+0x32/0x80
> [ 24.921861] handle_irq_event+0x3b/0x5a
> [ 24.921867] handle_edge_irq+0x80/0x190
> [ 24.921874] handle_irq+0x20/0x30
> [ 24.921889] do_IRQ+0x4e/0xe0
> [ 24.921891] common_interrupt+0xf/0xf
> [ 24.921892] </IRQ>
> [ 24.921900] RIP: 0010:usb_hcd_flush_endpoint+0x78/0x180
> [ 24.921354] usb 3-14: USB disconnect, device number 7
Is this a new thing? I def tested unplugging the dongle under
traffic, but that must had been in 3.19 days :S
> Fixes: c869f77d6abb ("add mt7601u driver")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> I will post a patch to fix tx side as well
> ---
> drivers/net/wireless/mediatek/mt7601u/dma.c | 33 ++++++++++-----------
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c
> index f7edeffb2b19..e7703990b291 100644
> --- a/drivers/net/wireless/mediatek/mt7601u/dma.c
> +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
> @@ -193,10 +193,20 @@ static void mt7601u_complete_rx(struct urb *urb)
> struct mt7601u_rx_queue *q = &dev->rx_q;
> unsigned long flags;
>
> - spin_lock_irqsave(&dev->rx_lock, flags);
> + switch (urb->status) {
> + case -ECONNRESET:
> + case -ESHUTDOWN:
> + case -ENOENT:
> + return;
So we assume this is non-recoverable? Everything will fail after?
Because pending is incremented linearly :S That's why there is a
warning here.
> + default:
> + dev_err_ratelimited(dev->dev, "rx urb failed: %d\n",
> + urb->status);
> + /* fall through */
> + case 0:
> + break;
> + }
>
> - if (mt7601u_urb_has_error(urb))
> - dev_err(dev->dev, "Error: RX urb failed:%d\n", urb->status);
> + spin_lock_irqsave(&dev->rx_lock, flags);
> if (WARN_ONCE(q->e[q->end].urb != urb, "RX urb mismatch"))
> goto out;
>
> @@ -363,19 +373,10 @@ int mt7601u_dma_enqueue_tx(struct mt7601u_dev *dev, struct sk_buff *skb,
> static void mt7601u_kill_rx(struct mt7601u_dev *dev)
> {
> int i;
> - unsigned long flags;
> -
> - spin_lock_irqsave(&dev->rx_lock, flags);
>
> - for (i = 0; i < dev->rx_q.entries; i++) {
> - int next = dev->rx_q.end;
> -
> - spin_unlock_irqrestore(&dev->rx_lock, flags);
> - usb_poison_urb(dev->rx_q.e[next].urb);
> - spin_lock_irqsave(&dev->rx_lock, flags);
> - }
Why is there no need to take the lock? Admittedly it's not clear what
this lock is protecting here :P Perhaps a separate patch to remove the
unnecessary locking with an explanation?
> - spin_unlock_irqrestore(&dev->rx_lock, flags);
> + for (i = 0; i < dev->rx_q.entries; i++)
> + usb_poison_urb(dev->rx_q.e[i].urb);
> + tasklet_kill(&dev->rx_tasklet);
> }
>
> static int mt7601u_submit_rx_buf(struct mt7601u_dev *dev,
> @@ -525,8 +526,6 @@ void mt7601u_dma_cleanup(struct mt7601u_dev *dev)
> {
> mt7601u_kill_rx(dev);
>
> - tasklet_kill(&dev->rx_tasklet);
Why the move? Looks a bit unnecessary..
> mt7601u_free_rx(dev);
> mt7601u_free_tx(dev);
>
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Larry Finger @ 2019-06-06 19:26 UTC (permalink / raw)
To: Christoph Hellwig, Benjamin Herrenschmidt
Cc: Aaro Koskinen, Christian Zigotzky, Michael Ellerman, linuxppc-dev,
linux-wireless, linux-kernel
In-Reply-To: <20190606114325.GA7497@lst.de>
On 6/6/19 6:43 AM, Christoph Hellwig wrote:
> On Thu, Jun 06, 2019 at 08:57:49PM +1000, Benjamin Herrenschmidt wrote:
>>> Wow... that's an odd amount. One thing we could possibly do is add code
>>> to limit the amount of RAM when we detect that device....
>>
>> Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based
>> on detecting the presence of that device in the device-tree.
>
> swiotlb doesn't really help you, as these days swiotlb on matters for
> the dma_map* case. What would help is a ZONE_DMA that covers these
> devices. No need to do the 24-bit x86 does, but 30-bit would do it.
>
> WIP patch for testing below:
>
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index b8286a2013b4..7a367ce87c41 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -319,6 +319,10 @@ struct vm_area_struct;
> #endif /* __ASSEMBLY__ */
> #include <asm/slice.h>
>
> +#if 1 /* XXX: pmac? dynamic discovery? */
> +#define ARCH_ZONE_DMA_BITS 30
> +#else
> #define ARCH_ZONE_DMA_BITS 31
> +#endif
>
> #endif /* _ASM_POWERPC_PAGE_H */
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index cba29131bccc..2540d3b2588c 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -248,7 +248,8 @@ void __init paging_init(void)
> (long int)((top_of_ram - total_ram) >> 20));
>
> #ifdef CONFIG_ZONE_DMA
> - max_zone_pfns[ZONE_DMA] = min(max_low_pfn, 0x7fffffffUL >> PAGE_SHIFT);
> + max_zone_pfns[ZONE_DMA] = min(max_low_pfn,
> + ((1UL << ARCH_ZONE_DMA_BITS) - 1) >> PAGE_SHIFT);
> #endif
> max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
> #ifdef CONFIG_HIGHMEM
>
I am generating a test kernel with this patch.
FYI, the "free" command on my machine shows 1.5+ G of memory. That likely means
I have 2G installed.
I have tested a patched kernel in which b43legacy falls back to a 31-bit DMA
mask when the 32-bit one failed. That worked, but would likely kill the x86
version. Let me know if think a fix in the driver rather than the kernel would
be better. I still need to understand why the same setup works in b43 and fails
in b43legacy. :(
Larry
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Larry Finger @ 2019-06-06 20:11 UTC (permalink / raw)
To: Christoph Hellwig, Benjamin Herrenschmidt
Cc: Aaro Koskinen, Christian Zigotzky, Michael Ellerman, linuxppc-dev,
linux-wireless, linux-kernel
In-Reply-To: <20190606114325.GA7497@lst.de>
On 6/6/19 6:43 AM, Christoph Hellwig wrote:
> On Thu, Jun 06, 2019 at 08:57:49PM +1000, Benjamin Herrenschmidt wrote:
>>> Wow... that's an odd amount. One thing we could possibly do is add code
>>> to limit the amount of RAM when we detect that device....
>>
>> Sent too quickly... I mean that *or* force swiotlb at 30-bits on those systems based
>> on detecting the presence of that device in the device-tree.
>
> swiotlb doesn't really help you, as these days swiotlb on matters for
> the dma_map* case. What would help is a ZONE_DMA that covers these
> devices. No need to do the 24-bit x86 does, but 30-bit would do it.
>
> WIP patch for testing below:
>
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index b8286a2013b4..7a367ce87c41 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -319,6 +319,10 @@ struct vm_area_struct;
> #endif /* __ASSEMBLY__ */
> #include <asm/slice.h>
>
> +#if 1 /* XXX: pmac? dynamic discovery? */
> +#define ARCH_ZONE_DMA_BITS 30
> +#else
> #define ARCH_ZONE_DMA_BITS 31
> +#endif
>
> #endif /* _ASM_POWERPC_PAGE_H */
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index cba29131bccc..2540d3b2588c 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -248,7 +248,8 @@ void __init paging_init(void)
> (long int)((top_of_ram - total_ram) >> 20));
>
> #ifdef CONFIG_ZONE_DMA
> - max_zone_pfns[ZONE_DMA] = min(max_low_pfn, 0x7fffffffUL >> PAGE_SHIFT);
> + max_zone_pfns[ZONE_DMA] = min(max_low_pfn,
> + ((1UL << ARCH_ZONE_DMA_BITS) - 1) >> PAGE_SHIFT);
> #endif
> max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
> #ifdef CONFIG_HIGHMEM
>
This trial patch failed.
Larry
^ permalink raw reply
* Hi Jaganath
From: TG Kiran @ 2019-06-06 20:39 UTC (permalink / raw)
To: Jaganath Vemala, linux wireless, maheshkumar shiv, maheshrpc2002,
tg kiran, tgkiran, vijaya raghavan
Hi Jaganath
http://namasterestoran.ee/locate.php?rqyn=EOW9901
Thanks so much
TG Kiran
^ permalink raw reply
* Re: [PATCH] mt7601u: do not schedule rx_tasklet when the device has been disconnected
From: Lorenzo Bianconi @ 2019-06-06 21:02 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Lorenzo Bianconi, linux-wireless
In-Reply-To: <20190606114228.7a0ba115@cakuba.netronome.com>
>
> On Thu, 6 Jun 2019 14:26:12 +0200, Lorenzo Bianconi wrote:
> > Do not schedule rx_tasklet when the usb dongle is disconnected. This
> > patch fixes the common kernel warning reported when the device is
> > removed.
> >
> > [ 24.921354] usb 3-14: USB disconnect, device number 7
> > [ 24.921593] ------------[ cut here ]------------
> > [ 24.921594] RX urb mismatch
> > [ 24.921675] WARNING: CPU: 4 PID: 163 at drivers/net/wireless/mediatek/mt7601u/dma.c:200 mt7601u_complete_rx+0xcb/0xd0 [mt7601u]
> > [ 24.921769] CPU: 4 PID: 163 Comm: kworker/4:2 Tainted: G OE 4.19.31-041931-generic #201903231635
> > [ 24.921770] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z97 Extreme4, BIOS P1.30 05/23/2014
> > [ 24.921782] Workqueue: usb_hub_wq hub_event
> > [ 24.921797] RIP: 0010:mt7601u_complete_rx+0xcb/0xd0 [mt7601u]
> > [ 24.921800] RSP: 0018:ffff9bd9cfd03d08 EFLAGS: 00010086
> > [ 24.921802] RAX: 0000000000000000 RBX: ffff9bd9bf043540 RCX: 0000000000000006
> > [ 24.921803] RDX: 0000000000000007 RSI: 0000000000000096 RDI: ffff9bd9cfd16420
> > [ 24.921804] RBP: ffff9bd9cfd03d28 R08: 0000000000000002 R09: 00000000000003a8
> > [ 24.921805] R10: 0000002f485fca34 R11: 0000000000000000 R12: ffff9bd9bf043c1c
> > [ 24.921806] R13: ffff9bd9c62fa3c0 R14: 0000000000000082 R15: 0000000000000000
> > [ 24.921807] FS: 0000000000000000(0000) GS:ffff9bd9cfd00000(0000) knlGS:0000000000000000
> > [ 24.921808] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 24.921808] CR2: 00007fb2648b0000 CR3: 0000000142c0a004 CR4: 00000000001606e0
> > [ 24.921809] Call Trace:
> > [ 24.921812] <IRQ>
> > [ 24.921819] __usb_hcd_giveback_urb+0x8b/0x140
> > [ 24.921821] usb_hcd_giveback_urb+0xca/0xe0
> > [ 24.921828] xhci_giveback_urb_in_irq.isra.42+0x82/0xf0
> > [ 24.921834] handle_cmd_completion+0xe02/0x10d0
> > [ 24.921837] xhci_irq+0x274/0x4a0
> > [ 24.921838] xhci_msi_irq+0x11/0x20
> > [ 24.921851] __handle_irq_event_percpu+0x44/0x190
> > [ 24.921856] handle_irq_event_percpu+0x32/0x80
> > [ 24.921861] handle_irq_event+0x3b/0x5a
> > [ 24.921867] handle_edge_irq+0x80/0x190
> > [ 24.921874] handle_irq+0x20/0x30
> > [ 24.921889] do_IRQ+0x4e/0xe0
> > [ 24.921891] common_interrupt+0xf/0xf
> > [ 24.921892] </IRQ>
> > [ 24.921900] RIP: 0010:usb_hcd_flush_endpoint+0x78/0x180
> > [ 24.921354] usb 3-14: USB disconnect, device number 7
>
Hi Jakub,
thx for the fast review :)
> Is this a new thing? I def tested unplugging the dongle under
> traffic, but that must had been in 3.19 days :S
>
I do not know if the issue has been introduced in recent kernel, I am
able to trigger it in a vm running
wireless-drivers-next and it has been reported here:
https://bugzilla.kernel.org/show_bug.cgi?id=203249
> > Fixes: c869f77d6abb ("add mt7601u driver")
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > I will post a patch to fix tx side as well
> > ---
> > drivers/net/wireless/mediatek/mt7601u/dma.c | 33 ++++++++++-----------
> > 1 file changed, 16 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > index f7edeffb2b19..e7703990b291 100644
> > --- a/drivers/net/wireless/mediatek/mt7601u/dma.c
> > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > @@ -193,10 +193,20 @@ static void mt7601u_complete_rx(struct urb *urb)
> > struct mt7601u_rx_queue *q = &dev->rx_q;
> > unsigned long flags;
> >
> > - spin_lock_irqsave(&dev->rx_lock, flags);
> > + switch (urb->status) {
> > + case -ECONNRESET:
> > + case -ESHUTDOWN:
> > + case -ENOENT:
> > + return;
>
> So we assume this is non-recoverable? Everything will fail after?
> Because pending is incremented linearly :S That's why there is a
> warning here.
AFAIK -ECONNRESET/-ENOENT are related to urb unlink/kill while
-ESHUTDOWN is related to device disconnection.
I guess we can assume the device has been removed if we get these errors
>
> > + default:
> > + dev_err_ratelimited(dev->dev, "rx urb failed: %d\n",
> > + urb->status);
> > + /* fall through */
> > + case 0:
> > + break;
> > + }
> >
> > - if (mt7601u_urb_has_error(urb))
> > - dev_err(dev->dev, "Error: RX urb failed:%d\n", urb->status);
> > + spin_lock_irqsave(&dev->rx_lock, flags);
> > if (WARN_ONCE(q->e[q->end].urb != urb, "RX urb mismatch"))
> > goto out;
> >
> > @@ -363,19 +373,10 @@ int mt7601u_dma_enqueue_tx(struct mt7601u_dev *dev, struct sk_buff *skb,
> > static void mt7601u_kill_rx(struct mt7601u_dev *dev)
> > {
> > int i;
> > - unsigned long flags;
> > -
> > - spin_lock_irqsave(&dev->rx_lock, flags);
> >
> > - for (i = 0; i < dev->rx_q.entries; i++) {
> > - int next = dev->rx_q.end;
> > -
> > - spin_unlock_irqrestore(&dev->rx_lock, flags);
> > - usb_poison_urb(dev->rx_q.e[next].urb);
> > - spin_lock_irqsave(&dev->rx_lock, flags);
> > - }
>
> Why is there no need to take the lock? Admittedly it's not clear what
> this lock is protecting here :P Perhaps a separate patch to remove the
> unnecessary locking with an explanation?
usb_poison_urb() can run concurrently with urb completion so I guess
we do not need locks here.
I guess we need to maintain this chunk in the same patch since now
when the device is disconnected
we do not increment dev->rx_q.end. What do you think?
>
> > - spin_unlock_irqrestore(&dev->rx_lock, flags);
> > + for (i = 0; i < dev->rx_q.entries; i++)
> > + usb_poison_urb(dev->rx_q.e[i].urb);
> > + tasklet_kill(&dev->rx_tasklet);
> > }
> >
> > static int mt7601u_submit_rx_buf(struct mt7601u_dev *dev,
> > @@ -525,8 +526,6 @@ void mt7601u_dma_cleanup(struct mt7601u_dev *dev)
> > {
> > mt7601u_kill_rx(dev);
> >
> > - tasklet_kill(&dev->rx_tasklet);
>
> Why the move? Looks a bit unnecessary..
>
ack, I will put it back in v2 :)
Regards,
Lorenzo
> > mt7601u_free_rx(dev);
> > mt7601u_free_tx(dev);
> >
^ permalink raw reply
* Re: [PATCH] mt7601u: fix possible memory leak when the device is disconnected
From: Lorenzo Bianconi @ 2019-06-06 21:10 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Lorenzo Bianconi, linux-wireless
In-Reply-To: <20190606114845.4026270e@cakuba.netronome.com>
On Thu, Jun 6, 2019 at 8:49 PM Jakub Kicinski <kubakici@wp.pl> wrote:
>
> On Thu, 6 Jun 2019 17:34:16 +0200, Lorenzo Bianconi wrote:
> > When the device is disconnected while passing traffic it is possible
> > to receive out of order urbs causing a memory leak since the skb liked
> > to the current tx urb is not removed. Fix the issue deallocating the skb
> > cleaning up the tx ring. Moreover this patch fixes the following kernel
> > warning
>
> Ugh if we don't have ordering guarantees then the entire "ring" scheme
> no longer works :( Should we move to URB queues, I don't remember now,
> but there seem to had been a better way to manage URBs :S
>
actually I have observed these issues on tx/rx side just during device
disconnection while passing traffic.
I guess we can assume a proper urb ordering during normal operation
(and so tx/rx ring works fine).
Btw what do you mean with 'URB queues'? :)
> > [ 57.480771] usb 1-1: USB disconnect, device number 2
> > [ 57.483451] ------------[ cut here ]------------
> > [ 57.483462] TX urb mismatch
> > [ 57.483481] WARNING: CPU: 1 PID: 32 at drivers/net/wireless/mediatek/mt7601u/dma.c:245 mt7601u_complete_tx+0x165/00
> > [ 57.483483] Modules linked in:
> > [ 57.483496] CPU: 1 PID: 32 Comm: kworker/1:1 Not tainted 5.2.0-rc1+ #72
> > [ 57.483498] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-2.fc30 04/01/2014
> > [ 57.483502] Workqueue: usb_hub_wq hub_event
> > [ 57.483507] RIP: 0010:mt7601u_complete_tx+0x165/0x1e0
> > [ 57.483510] Code: 8b b5 10 04 00 00 8b 8d 14 04 00 00 eb 8b 80 3d b1 cb e1 00 00 75 9e 48 c7 c7 a4 ea 05 82 c6 05 f
> > [ 57.483513] RSP: 0000:ffffc900000a0d28 EFLAGS: 00010092
> > [ 57.483516] RAX: 000000000000000f RBX: ffff88802c0a62c0 RCX: ffffc900000a0c2c
> > [ 57.483518] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff810a8371
> > [ 57.483520] RBP: ffff88803ced6858 R08: 0000000000000000 R09: 0000000000000001
> > [ 57.483540] R10: 0000000000000002 R11: 0000000000000000 R12: 0000000000000046
> > [ 57.483542] R13: ffff88802c0a6c88 R14: ffff88803baab540 R15: ffff88803a0cc078
> > [ 57.483548] FS: 0000000000000000(0000) GS:ffff88803eb00000(0000) knlGS:0000000000000000
> > [ 57.483550] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 57.483552] CR2: 000055e7f6780100 CR3: 0000000028c86000 CR4: 00000000000006a0
> > [ 57.483554] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [ 57.483556] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > [ 57.483559] Call Trace:
> > [ 57.483561] <IRQ>
> > [ 57.483565] __usb_hcd_giveback_urb+0x77/0xe0
> > [ 57.483570] xhci_giveback_urb_in_irq.isra.0+0x8b/0x140
> > [ 57.483574] handle_cmd_completion+0xf5b/0x12c0
> > [ 57.483577] xhci_irq+0x1f6/0x1810
> > [ 57.483581] ? lockdep_hardirqs_on+0x9e/0x180
> > [ 57.483584] ? _raw_spin_unlock_irq+0x24/0x30
> > [ 57.483588] __handle_irq_event_percpu+0x3a/0x260
> > [ 57.483592] handle_irq_event_percpu+0x1c/0x60
> > [ 57.483595] handle_irq_event+0x2f/0x4c
> > [ 57.483599] handle_edge_irq+0x7e/0x1a0
> > [ 57.483603] handle_irq+0x17/0x20
> > [ 57.483607] do_IRQ+0x54/0x110
> > [ 57.483610] common_interrupt+0xf/0xf
> > [ 57.483612] </IRQ>
> >
> > Fixes: c869f77d6abb ("add mt7601u driver")
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > drivers/net/wireless/mediatek/mt7601u/dma.c | 24 +++++++++++++++------
> > drivers/net/wireless/mediatek/mt7601u/tx.c | 4 ++--
> > 2 files changed, 19 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > index e7703990b291..bbf1deed7f3b 100644
> > --- a/drivers/net/wireless/mediatek/mt7601u/dma.c
> > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > @@ -238,14 +238,25 @@ static void mt7601u_complete_tx(struct urb *urb)
> > struct sk_buff *skb;
> > unsigned long flags;
> >
> > - spin_lock_irqsave(&dev->tx_lock, flags);
> > + switch (urb->status) {
> > + case -ECONNRESET:
> > + case -ESHUTDOWN:
> > + case -ENOENT:
> > + return;
> > + default:
> > + dev_err_ratelimited(dev->dev, "tx urb failed: %d\n",
> > + urb->status);
> > + /* fall through */
> > + case 0:
> > + break;
> > + }
> >
> > - if (mt7601u_urb_has_error(urb))
> > - dev_err(dev->dev, "Error: TX urb failed:%d\n", urb->status);
> > + spin_lock_irqsave(&dev->tx_lock, flags);
> > if (WARN_ONCE(q->e[q->start].urb != urb, "TX urb mismatch"))
> > goto out;
> >
> > skb = q->e[q->start].skb;
> > + q->e[q->start].skb = NULL;
> > trace_mt_tx_dma_done(dev, skb);
> >
> > __skb_queue_tail(&dev->tx_skb_done, skb);
> > @@ -446,10 +457,10 @@ static void mt7601u_free_tx_queue(struct mt7601u_tx_queue *q)
> > {
> > int i;
> >
> > - WARN_ON(q->used);
> > -
> > for (i = 0; i < q->entries; i++) {
> > usb_poison_urb(q->e[i].urb);
> > + if (q->e[i].skb)
> > + mt7601u_tx_status(q->dev, q->e[i].skb);
>
> Perhaps a separate patch?
>
As I did for rx side, if we do not schedule the tx tasklet when the
device has been disconnected I guess we need this chuck in the same
patch. What do you think?
Regards,
Lorenzo
> > usb_free_urb(q->e[i].urb);
> > }
> > }
> > @@ -463,6 +474,7 @@ static void mt7601u_free_tx(struct mt7601u_dev *dev)
> >
> > for (i = 0; i < __MT_EP_OUT_MAX; i++)
> > mt7601u_free_tx_queue(&dev->tx_q[i]);
> > + tasklet_kill(&dev->tx_tasklet);
> > }
> >
> > static int mt7601u_alloc_tx_queue(struct mt7601u_dev *dev,
> > @@ -528,6 +540,4 @@ void mt7601u_dma_cleanup(struct mt7601u_dev *dev)
> >
> > mt7601u_free_rx(dev);
> > mt7601u_free_tx(dev);
> > -
> > - tasklet_kill(&dev->tx_tasklet);
> > }
> > diff --git a/drivers/net/wireless/mediatek/mt7601u/tx.c b/drivers/net/wireless/mediatek/mt7601u/tx.c
> > index 3600e911a63e..4d81c45722fb 100644
> > --- a/drivers/net/wireless/mediatek/mt7601u/tx.c
> > +++ b/drivers/net/wireless/mediatek/mt7601u/tx.c
> > @@ -117,9 +117,9 @@ void mt7601u_tx_status(struct mt7601u_dev *dev, struct sk_buff *skb)
> > info->status.rates[0].idx = -1;
> > info->flags |= IEEE80211_TX_STAT_ACK;
> >
> > - spin_lock(&dev->mac_lock);
> > + spin_lock_bh(&dev->mac_lock);
> > ieee80211_tx_status(dev->hw, skb);
> > - spin_unlock(&dev->mac_lock);
> > + spin_unlock_bh(&dev->mac_lock);
> > }
> >
> > static int mt7601u_skb_rooms(struct mt7601u_dev *dev, struct sk_buff *skb)
>
^ permalink raw reply
* Re: [PATCH] mt7601u: do not schedule rx_tasklet when the device has been disconnected
From: Jakub Kicinski @ 2019-06-06 21:26 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: Lorenzo Bianconi, linux-wireless
In-Reply-To: <CAJ0CqmUySwtgzPCFR0QsZ-XA7Lm7xU8eMsELV8LEd2MEkhfnLA@mail.gmail.com>
On Thu, 6 Jun 2019 23:02:08 +0200, Lorenzo Bianconi wrote:
> > On Thu, 6 Jun 2019 14:26:12 +0200, Lorenzo Bianconi wrote:
> Hi Jakub,
>
> thx for the fast review :)
I guess I'm used to the 24h "review timeout" on netdev :)
> > Is this a new thing? I def tested unplugging the dongle under
> > traffic, but that must had been in 3.19 days :S
>
> I do not know if the issue has been introduced in recent kernel, I am
> able to trigger it in a vm running
> wireless-drivers-next and it has been reported here:
> https://bugzilla.kernel.org/show_bug.cgi?id=203249
I see. I'm just worried that we make a mistake here and it will get
backported all the way back to very old kernels because of the fixes
tag. If old kernels worked perhaps it's not worth disturbing them.
> > > Fixes: c869f77d6abb ("add mt7601u driver")
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> > > I will post a patch to fix tx side as well
> > > ---
> > > drivers/net/wireless/mediatek/mt7601u/dma.c | 33 ++++++++++-----------
> > > 1 file changed, 16 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > index f7edeffb2b19..e7703990b291 100644
> > > --- a/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > @@ -193,10 +193,20 @@ static void mt7601u_complete_rx(struct urb *urb)
> > > struct mt7601u_rx_queue *q = &dev->rx_q;
> > > unsigned long flags;
> > >
> > > - spin_lock_irqsave(&dev->rx_lock, flags);
> > > + switch (urb->status) {
> > > + case -ECONNRESET:
> > > + case -ESHUTDOWN:
> > > + case -ENOENT:
> > > + return;
> >
> > So we assume this is non-recoverable? Everything will fail after?
> > Because pending is incremented linearly :S That's why there is a
> > warning here.
>
> AFAIK -ECONNRESET/-ENOENT are related to urb unlink/kill while
> -ESHUTDOWN is related to device disconnection.
> I guess we can assume the device has been removed if we get these errors
Makes sense. A bit of an implicit assumption, USB subsystem may break
this for us. Let's at least put a comment here so we can go back and
know that at the time of committing we did double check?
> > > + default:
> > > + dev_err_ratelimited(dev->dev, "rx urb failed: %d\n",
> > > + urb->status);
> > > + /* fall through */
> > > + case 0:
> > > + break;
> > > + }
> > >
> > > - if (mt7601u_urb_has_error(urb))
> > > - dev_err(dev->dev, "Error: RX urb failed:%d\n", urb->status);
> > > + spin_lock_irqsave(&dev->rx_lock, flags);
> > > if (WARN_ONCE(q->e[q->end].urb != urb, "RX urb mismatch"))
> > > goto out;
> > >
> > > @@ -363,19 +373,10 @@ int mt7601u_dma_enqueue_tx(struct mt7601u_dev *dev, struct sk_buff *skb,
> > > static void mt7601u_kill_rx(struct mt7601u_dev *dev)
> > > {
> > > int i;
> > > - unsigned long flags;
> > > -
> > > - spin_lock_irqsave(&dev->rx_lock, flags);
> > >
> > > - for (i = 0; i < dev->rx_q.entries; i++) {
> > > - int next = dev->rx_q.end;
> > > -
> > > - spin_unlock_irqrestore(&dev->rx_lock, flags);
> > > - usb_poison_urb(dev->rx_q.e[next].urb);
> > > - spin_lock_irqsave(&dev->rx_lock, flags);
> > > - }
> >
> > Why is there no need to take the lock? Admittedly it's not clear what
> > this lock is protecting here :P Perhaps a separate patch to remove the
> > unnecessary locking with an explanation?
>
> usb_poison_urb() can run concurrently with urb completion so I guess
> we do not need locks here.
> I guess we need to maintain this chunk in the same patch since now
> when the device is disconnected
> we do not increment dev->rx_q.end. What do you think?
Ah, I see! The completion used to run in between the unlock/lock!
Now we just poison out of order, because completion doesn't care about
ordering for errored URBs. Perhaps worth putting in the commit message?
^ permalink raw reply
* Re: [PATCH] mt7601u: fix possible memory leak when the device is disconnected
From: Jakub Kicinski @ 2019-06-06 21:36 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: Lorenzo Bianconi, linux-wireless
In-Reply-To: <CAJ0CqmUCch1dU1J24XDOg_fg35BfWLWjvXc3nV7QN6JHgWhZJw@mail.gmail.com>
On Thu, 6 Jun 2019 23:10:15 +0200, Lorenzo Bianconi wrote:
> On Thu, Jun 6, 2019 at 8:49 PM Jakub Kicinski <kubakici@wp.pl> wrote:
> >
> > On Thu, 6 Jun 2019 17:34:16 +0200, Lorenzo Bianconi wrote:
> > > When the device is disconnected while passing traffic it is possible
> > > to receive out of order urbs causing a memory leak since the skb liked
> > > to the current tx urb is not removed. Fix the issue deallocating the skb
> > > cleaning up the tx ring. Moreover this patch fixes the following kernel
> > > warning
> >
> > Ugh if we don't have ordering guarantees then the entire "ring" scheme
> > no longer works :( Should we move to URB queues, I don't remember now,
> > but there seem to had been a better way to manage URBs :S
> >
>
> actually I have observed these issues on tx/rx side just during device
> disconnection while passing traffic.
> I guess we can assume a proper urb ordering during normal operation
> (and so tx/rx ring works fine).
Ah, phew, okay. That's fine then.
> Btw what do you mean with 'URB queues'? :)
I think it may have been URB anchors. I remember looking at carl9170
and liking how the DMAs operated there.
> > > [ 57.480771] usb 1-1: USB disconnect, device number 2
> > > [ 57.483451] ------------[ cut here ]------------
> > > [ 57.483462] TX urb mismatch
> > > [ 57.483481] WARNING: CPU: 1 PID: 32 at drivers/net/wireless/mediatek/mt7601u/dma.c:245 mt7601u_complete_tx+0x165/00
> > > [ 57.483483] Modules linked in:
> > > [ 57.483496] CPU: 1 PID: 32 Comm: kworker/1:1 Not tainted 5.2.0-rc1+ #72
> > > [ 57.483498] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-2.fc30 04/01/2014
> > > [ 57.483502] Workqueue: usb_hub_wq hub_event
> > > [ 57.483507] RIP: 0010:mt7601u_complete_tx+0x165/0x1e0
> > > [ 57.483510] Code: 8b b5 10 04 00 00 8b 8d 14 04 00 00 eb 8b 80 3d b1 cb e1 00 00 75 9e 48 c7 c7 a4 ea 05 82 c6 05 f
> > > [ 57.483513] RSP: 0000:ffffc900000a0d28 EFLAGS: 00010092
> > > [ 57.483516] RAX: 000000000000000f RBX: ffff88802c0a62c0 RCX: ffffc900000a0c2c
> > > [ 57.483518] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff810a8371
> > > [ 57.483520] RBP: ffff88803ced6858 R08: 0000000000000000 R09: 0000000000000001
> > > [ 57.483540] R10: 0000000000000002 R11: 0000000000000000 R12: 0000000000000046
> > > [ 57.483542] R13: ffff88802c0a6c88 R14: ffff88803baab540 R15: ffff88803a0cc078
> > > [ 57.483548] FS: 0000000000000000(0000) GS:ffff88803eb00000(0000) knlGS:0000000000000000
> > > [ 57.483550] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > [ 57.483552] CR2: 000055e7f6780100 CR3: 0000000028c86000 CR4: 00000000000006a0
> > > [ 57.483554] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > [ 57.483556] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > [ 57.483559] Call Trace:
> > > [ 57.483561] <IRQ>
> > > [ 57.483565] __usb_hcd_giveback_urb+0x77/0xe0
> > > [ 57.483570] xhci_giveback_urb_in_irq.isra.0+0x8b/0x140
> > > [ 57.483574] handle_cmd_completion+0xf5b/0x12c0
> > > [ 57.483577] xhci_irq+0x1f6/0x1810
> > > [ 57.483581] ? lockdep_hardirqs_on+0x9e/0x180
> > > [ 57.483584] ? _raw_spin_unlock_irq+0x24/0x30
> > > [ 57.483588] __handle_irq_event_percpu+0x3a/0x260
> > > [ 57.483592] handle_irq_event_percpu+0x1c/0x60
> > > [ 57.483595] handle_irq_event+0x2f/0x4c
> > > [ 57.483599] handle_edge_irq+0x7e/0x1a0
> > > [ 57.483603] handle_irq+0x17/0x20
> > > [ 57.483607] do_IRQ+0x54/0x110
> > > [ 57.483610] common_interrupt+0xf/0xf
> > > [ 57.483612] </IRQ>
> > >
> > > Fixes: c869f77d6abb ("add mt7601u driver")
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> > > drivers/net/wireless/mediatek/mt7601u/dma.c | 24 +++++++++++++++------
> > > drivers/net/wireless/mediatek/mt7601u/tx.c | 4 ++--
> > > 2 files changed, 19 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > index e7703990b291..bbf1deed7f3b 100644
> > > --- a/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > @@ -238,14 +238,25 @@ static void mt7601u_complete_tx(struct urb *urb)
> > > struct sk_buff *skb;
> > > unsigned long flags;
> > >
> > > - spin_lock_irqsave(&dev->tx_lock, flags);
> > > + switch (urb->status) {
> > > + case -ECONNRESET:
> > > + case -ESHUTDOWN:
> > > + case -ENOENT:
> > > + return;
> > > + default:
> > > + dev_err_ratelimited(dev->dev, "tx urb failed: %d\n",
> > > + urb->status);
> > > + /* fall through */
> > > + case 0:
> > > + break;
> > > + }
> > >
> > > - if (mt7601u_urb_has_error(urb))
> > > - dev_err(dev->dev, "Error: TX urb failed:%d\n", urb->status);
> > > + spin_lock_irqsave(&dev->tx_lock, flags);
> > > if (WARN_ONCE(q->e[q->start].urb != urb, "TX urb mismatch"))
> > > goto out;
> > >
> > > skb = q->e[q->start].skb;
> > > + q->e[q->start].skb = NULL;
> > > trace_mt_tx_dma_done(dev, skb);
> > >
> > > __skb_queue_tail(&dev->tx_skb_done, skb);
> > > @@ -446,10 +457,10 @@ static void mt7601u_free_tx_queue(struct mt7601u_tx_queue *q)
> > > {
> > > int i;
> > >
> > > - WARN_ON(q->used);
> > > -
> > > for (i = 0; i < q->entries; i++) {
> > > usb_poison_urb(q->e[i].urb);
> > > + if (q->e[i].skb)
> > > + mt7601u_tx_status(q->dev, q->e[i].skb);
> >
> > Perhaps a separate patch?
>
> As I did for rx side, if we do not schedule the tx tasklet when the
> device has been disconnected I guess we need this chuck in the same
> patch. What do you think?
Yeah, I see.
^ permalink raw reply
* Re: [PATCH v2 3/3] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
From: Doug Anderson @ 2019-06-06 21:37 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, Kalle Valo, Arend van Spriel, brcm80211-dev-list.pdl,
open list:ARM/Rockchip SoC..., Double Lo, Brian Norris,
linux-wireless, Naveen Gupta, Madhan Mohan R, Matthias Kaehlcke,
Wright Feng, Chi-Hsien Lin, netdev, brcm80211-dev-list,
David S. Miller, Franky Lin, LKML, Hante Meuleman, YueHaibing,
Michael Trimarchi
In-Reply-To: <42fc30b1-adab-7fa8-104c-cbb7855f2032@intel.com>
Hi,
On Thu, Jun 6, 2019 at 7:00 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 3/06/19 9:37 PM, Douglas Anderson wrote:
> > There are certain cases, notably when transitioning between sleep and
> > active state, when Broadcom SDIO WiFi cards will produce errors on the
> > SDIO bus. This is evident from the source code where you can see that
> > we try commands in a loop until we either get success or we've tried
> > too many times. The comment in the code reinforces this by saying
> > "just one write attempt may fail"
> >
> > Unfortunately these failures sometimes end up causing an "-EILSEQ"
> > back to the core which triggers a retuning of the SDIO card and that
> > blocks all traffic to the card until it's done.
> >
> > Let's disable retuning around the commands we expect might fail.
>
> It seems to me that re-tuning needs to be prevented before the
> first access otherwise it might be attempted there,
By this I think you mean I wasn't starting my section early enough to
catch the "1st KSO write". Oops. Thanks!
> and it needs
> to continue to be prevented during the transition when it might
> reasonably be expected to fail.
>
> What about something along these lines:
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index 4e15ea57d4f5..d932780ef56e 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -664,9 +664,18 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
> int err = 0;
> int err_cnt = 0;
> int try_cnt = 0;
> + int need_retune = 0;
> + bool retune_release = false;
>
> brcmf_dbg(TRACE, "Enter: on=%d\n", on);
>
> + /* Cannot re-tune if device is asleep */
> + if (on) {
> + need_retune = sdio_retune_get_needed(bus->sdiodev->func1); // TODO: host->can_retune ? host->need_retune : 0
> + sdio_retune_hold_now(bus->sdiodev->func1); // TODO: add sdio_retune_hold_now()
> + retune_release = true;
> + }
The code below still has retries even for the "!on" case. That
implies that you could still get CRC errors from the card in the "!on"
direction too. Any reason why we shouldn't just hold retuning even
for the !on case?
> +
> wr_val = (on << SBSDIO_FUNC1_SLEEPCSR_KSO_SHIFT);
> /* 1st KSO write goes to AOS wake up core if device is asleep */
> brcmf_sdiod_writeb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR, wr_val, &err);
> @@ -711,8 +720,16 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
> err_cnt = 0;
> }
> /* bail out upon subsequent access errors */
> - if (err && (err_cnt++ > BRCMF_SDIO_MAX_ACCESS_ERRORS))
> - break;
> + if (err && (err_cnt++ > BRCMF_SDIO_MAX_ACCESS_ERRORS)) {
> + if (!retune_release)
> + break;
> + /*
> + * Allow one more retry with re-tuning released in case
> + * it helps.
> + */
> + sdio_retune_release(bus->sdiodev->func1);
> + retune_release = false;
I would be tempted to wait before adding this logic until we actually
see that it's needed. Sure, doing one more transfer probably won't
really hurt, but until we know that it actually helps it seems like
we're just adding extra complexity?
> + }
>
> udelay(KSO_WAIT_US);
> brcmf_sdiod_writeb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR, wr_val,
> @@ -727,6 +744,18 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
> if (try_cnt > MAX_KSO_ATTEMPTS)
> brcmf_err("max tries: rd_val=0x%x err=%d\n", rd_val, err);
>
> + if (retune_release) {
> + /*
> + * CRC errors are not unexpected during the transition but they
> + * also trigger re-tuning. Clear that here to avoid an
> + * unnecessary re-tune if it wasn't already triggered to start
> + * with.
> + */
> + if (!need_retune)
> + sdio_retune_clear_needed(bus->sdiodev->func1); // TODO: host->need_retune = 0
> + sdio_retune_release(bus->sdiodev->func1); // TODO: add sdio_retune_release()
> + }
Every time I re-look at this I have to re-figure out all the subtle
differences between the variables and functions involved here. Let me
see if I got everything right:
* need_retune: set to 1 if we can retune and some event happened that
makes us truly believe that we need to be retuned, like we got a CRC
error or a timer expired or our host controller told us to retune.
* retune_now: set to 1 it's an OK time to be retuning. Specifically
if retune_now is false we won't send any retuning commands but we'll
still keep track of the need to retune.
* hold_retune: If this gets set to 1 by mmc_retune_hold_now() then a
future call to mmc_retune_hold() will _not_ schedule a retune by
setting retune_now (because mmc_retune_hold() will see that
hold_retune was already 1). ...and a future call to
mmc_retune_recheck() between mmc_hold() and mmc_release() will also
not schedule a retune because hold_retune will be 2 (or generally >
1).
---
So overall trying to summarize what I think are the differences
between your patch and my patch.
1. If we needed to re-tune _before_ calling brcmf_sdio_kso_control(),
with your patch we'll make sure that we don't actually attempt to
retune until brcmf_sdio_kso_control() finishes.
2. If we needed to retune during brcmf_sdio_kso_control() (because a
timer expired?) then we wouldn't trigger that retune while
brcmf_sdio_kso_control() is running.
In the case of dw_mmc, which I'm most familiar with, we don't have any
sort of automated or timed-based retuning. ...so we'll only re-tune
when we see the CRC error. If I'm understanding things correctly then
that for dw_mmc my solution and yours behave the same. That means the
difference is how we deal with other retuning requests, either ones
that come about because of an interrupt that the host controller
provided or because of a timer. Did I get that right?
...and I guess the reason we have to deal specially with these cases
is because any time that SDIO card is "sleeping" we don't want to
retune because it won't work. Right? NOTE: the solution that would
come to my mind first to solve this would be to hold the retuning for
the whole time that the card was sleeping and then release it once the
card was awake again. ...but I guess we don't truly need to do that
because tuning only happens as a side effect of sending a command to
the card and the only command we send to the card is the "wake up"
command. That's why your solution to hold tuning while sending the
"wake up" command works, right?
---
OK, so assuming all the above is correct, I feel like we're actually
solving two problems and in fact I believe we actually need both our
approaches to solve everything correctly. With just your patch in
place there's a problem because we will clobber any external retuning
requests that happened while we were waking up the card. AKA, imagine
this:
A) brcmf_sdio_kso_control(on=True) gets called; need_retune starts as 0
B) We call sdio_retune_hold_now()
C) A retuning timer goes off or the SD Host controller tells us to retune
D) We get to the end of brcmf_sdio_kso_control() and clear the "retune
needed" since need_retune was 0 at the start.
...so we dropped the retuning request from C), right?
What we truly need is:
1. CRC errors shouldn't trigger a retuning request when we're in
brcmf_sdio_kso_control()
2. A separate patch that holds any retuning requests while the SDIO
card is off. This patch _shouldn't_ do any clearing of retuning
requests, just defer them.
Does that make sense to you? If so, I can try to code it up...
-Doug
^ permalink raw reply
* Re: [PATCH] mt7601u: do not schedule rx_tasklet when the device has been disconnected
From: Lorenzo Bianconi @ 2019-06-06 21:59 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Lorenzo Bianconi, linux-wireless
In-Reply-To: <20190606142622.7e5f11de@cakuba.netronome.com>
>
> On Thu, 6 Jun 2019 23:02:08 +0200, Lorenzo Bianconi wrote:
> > > On Thu, 6 Jun 2019 14:26:12 +0200, Lorenzo Bianconi wrote:
> > Hi Jakub,
> >
> > thx for the fast review :)
>
> I guess I'm used to the 24h "review timeout" on netdev :)
>
:-)
> > > Is this a new thing? I def tested unplugging the dongle under
> > > traffic, but that must had been in 3.19 days :S
> >
> > I do not know if the issue has been introduced in recent kernel, I am
> > able to trigger it in a vm running
> > wireless-drivers-next and it has been reported here:
> > https://bugzilla.kernel.org/show_bug.cgi?id=203249
>
> I see. I'm just worried that we make a mistake here and it will get
> backported all the way back to very old kernels because of the fixes
> tag. If old kernels worked perhaps it's not worth disturbing them.
>
> > > > Fixes: c869f77d6abb ("add mt7601u driver")
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > ---
> > > > I will post a patch to fix tx side as well
> > > > ---
> > > > drivers/net/wireless/mediatek/mt7601u/dma.c | 33 ++++++++++-----------
> > > > 1 file changed, 16 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > > index f7edeffb2b19..e7703990b291 100644
> > > > --- a/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > > @@ -193,10 +193,20 @@ static void mt7601u_complete_rx(struct urb *urb)
> > > > struct mt7601u_rx_queue *q = &dev->rx_q;
> > > > unsigned long flags;
> > > >
> > > > - spin_lock_irqsave(&dev->rx_lock, flags);
> > > > + switch (urb->status) {
> > > > + case -ECONNRESET:
> > > > + case -ESHUTDOWN:
> > > > + case -ENOENT:
> > > > + return;
> > >
> > > So we assume this is non-recoverable? Everything will fail after?
> > > Because pending is incremented linearly :S That's why there is a
> > > warning here.
> >
> > AFAIK -ECONNRESET/-ENOENT are related to urb unlink/kill while
> > -ESHUTDOWN is related to device disconnection.
> > I guess we can assume the device has been removed if we get these errors
>
> Makes sense. A bit of an implicit assumption, USB subsystem may break
> this for us. Let's at least put a comment here so we can go back and
> know that at the time of committing we did double check?
>
ack, I will put a comment in v2
> > > > + default:
> > > > + dev_err_ratelimited(dev->dev, "rx urb failed: %d\n",
> > > > + urb->status);
> > > > + /* fall through */
> > > > + case 0:
> > > > + break;
> > > > + }
> > > >
> > > > - if (mt7601u_urb_has_error(urb))
> > > > - dev_err(dev->dev, "Error: RX urb failed:%d\n", urb->status);
> > > > + spin_lock_irqsave(&dev->rx_lock, flags);
> > > > if (WARN_ONCE(q->e[q->end].urb != urb, "RX urb mismatch"))
> > > > goto out;
> > > >
> > > > @@ -363,19 +373,10 @@ int mt7601u_dma_enqueue_tx(struct mt7601u_dev *dev, struct sk_buff *skb,
> > > > static void mt7601u_kill_rx(struct mt7601u_dev *dev)
> > > > {
> > > > int i;
> > > > - unsigned long flags;
> > > > -
> > > > - spin_lock_irqsave(&dev->rx_lock, flags);
> > > >
> > > > - for (i = 0; i < dev->rx_q.entries; i++) {
> > > > - int next = dev->rx_q.end;
> > > > -
> > > > - spin_unlock_irqrestore(&dev->rx_lock, flags);
> > > > - usb_poison_urb(dev->rx_q.e[next].urb);
> > > > - spin_lock_irqsave(&dev->rx_lock, flags);
> > > > - }
> > >
> > > Why is there no need to take the lock? Admittedly it's not clear what
> > > this lock is protecting here :P Perhaps a separate patch to remove the
> > > unnecessary locking with an explanation?
> >
> > usb_poison_urb() can run concurrently with urb completion so I guess
> > we do not need locks here.
> > I guess we need to maintain this chunk in the same patch since now
> > when the device is disconnected
> > we do not increment dev->rx_q.end. What do you think?
>
> Ah, I see! The completion used to run in between the unlock/lock!
> Now we just poison out of order, because completion doesn't care about
> ordering for errored URBs. Perhaps worth putting in the commit message?
ack, I will put a comment in v2
Regards,
Lorenzo
^ permalink raw reply
* Re: [PATCH] mt7601u: fix possible memory leak when the device is disconnected
From: Lorenzo Bianconi @ 2019-06-06 22:05 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Lorenzo Bianconi, linux-wireless
In-Reply-To: <20190606143627.575c2499@cakuba.netronome.com>
>
> On Thu, 6 Jun 2019 23:10:15 +0200, Lorenzo Bianconi wrote:
> > On Thu, Jun 6, 2019 at 8:49 PM Jakub Kicinski <kubakici@wp.pl> wrote:
> > >
> > > On Thu, 6 Jun 2019 17:34:16 +0200, Lorenzo Bianconi wrote:
> > > > When the device is disconnected while passing traffic it is possible
> > > > to receive out of order urbs causing a memory leak since the skb liked
> > > > to the current tx urb is not removed. Fix the issue deallocating the skb
> > > > cleaning up the tx ring. Moreover this patch fixes the following kernel
> > > > warning
> > >
> > > Ugh if we don't have ordering guarantees then the entire "ring" scheme
> > > no longer works :( Should we move to URB queues, I don't remember now,
> > > but there seem to had been a better way to manage URBs :S
> > >
> >
> > actually I have observed these issues on tx/rx side just during device
> > disconnection while passing traffic.
> > I guess we can assume a proper urb ordering during normal operation
> > (and so tx/rx ring works fine).
>
> Ah, phew, okay. That's fine then.
>
> > Btw what do you mean with 'URB queues'? :)
>
> I think it may have been URB anchors. I remember looking at carl9170
> and liking how the DMAs operated there.
Uhm, interesting. AFAIK urb anchors are used to cease I/O operations
but maybe they can be used even for other purposes (it will be
interesting even for mt76).
I guess this can be a future improvement, do you agree? Do I need to
resubmit this patch?
Regards,
Lorenzo
>
> > > > [ 57.480771] usb 1-1: USB disconnect, device number 2
> > > > [ 57.483451] ------------[ cut here ]------------
> > > > [ 57.483462] TX urb mismatch
> > > > [ 57.483481] WARNING: CPU: 1 PID: 32 at drivers/net/wireless/mediatek/mt7601u/dma.c:245 mt7601u_complete_tx+0x165/00
> > > > [ 57.483483] Modules linked in:
> > > > [ 57.483496] CPU: 1 PID: 32 Comm: kworker/1:1 Not tainted 5.2.0-rc1+ #72
> > > > [ 57.483498] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-2.fc30 04/01/2014
> > > > [ 57.483502] Workqueue: usb_hub_wq hub_event
> > > > [ 57.483507] RIP: 0010:mt7601u_complete_tx+0x165/0x1e0
> > > > [ 57.483510] Code: 8b b5 10 04 00 00 8b 8d 14 04 00 00 eb 8b 80 3d b1 cb e1 00 00 75 9e 48 c7 c7 a4 ea 05 82 c6 05 f
> > > > [ 57.483513] RSP: 0000:ffffc900000a0d28 EFLAGS: 00010092
> > > > [ 57.483516] RAX: 000000000000000f RBX: ffff88802c0a62c0 RCX: ffffc900000a0c2c
> > > > [ 57.483518] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff810a8371
> > > > [ 57.483520] RBP: ffff88803ced6858 R08: 0000000000000000 R09: 0000000000000001
> > > > [ 57.483540] R10: 0000000000000002 R11: 0000000000000000 R12: 0000000000000046
> > > > [ 57.483542] R13: ffff88802c0a6c88 R14: ffff88803baab540 R15: ffff88803a0cc078
> > > > [ 57.483548] FS: 0000000000000000(0000) GS:ffff88803eb00000(0000) knlGS:0000000000000000
> > > > [ 57.483550] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > [ 57.483552] CR2: 000055e7f6780100 CR3: 0000000028c86000 CR4: 00000000000006a0
> > > > [ 57.483554] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > [ 57.483556] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > [ 57.483559] Call Trace:
> > > > [ 57.483561] <IRQ>
> > > > [ 57.483565] __usb_hcd_giveback_urb+0x77/0xe0
> > > > [ 57.483570] xhci_giveback_urb_in_irq.isra.0+0x8b/0x140
> > > > [ 57.483574] handle_cmd_completion+0xf5b/0x12c0
> > > > [ 57.483577] xhci_irq+0x1f6/0x1810
> > > > [ 57.483581] ? lockdep_hardirqs_on+0x9e/0x180
> > > > [ 57.483584] ? _raw_spin_unlock_irq+0x24/0x30
> > > > [ 57.483588] __handle_irq_event_percpu+0x3a/0x260
> > > > [ 57.483592] handle_irq_event_percpu+0x1c/0x60
> > > > [ 57.483595] handle_irq_event+0x2f/0x4c
> > > > [ 57.483599] handle_edge_irq+0x7e/0x1a0
> > > > [ 57.483603] handle_irq+0x17/0x20
> > > > [ 57.483607] do_IRQ+0x54/0x110
> > > > [ 57.483610] common_interrupt+0xf/0xf
> > > > [ 57.483612] </IRQ>
> > > >
> > > > Fixes: c869f77d6abb ("add mt7601u driver")
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > ---
> > > > drivers/net/wireless/mediatek/mt7601u/dma.c | 24 +++++++++++++++------
> > > > drivers/net/wireless/mediatek/mt7601u/tx.c | 4 ++--
> > > > 2 files changed, 19 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > > index e7703990b291..bbf1deed7f3b 100644
> > > > --- a/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
> > > > @@ -238,14 +238,25 @@ static void mt7601u_complete_tx(struct urb *urb)
> > > > struct sk_buff *skb;
> > > > unsigned long flags;
> > > >
> > > > - spin_lock_irqsave(&dev->tx_lock, flags);
> > > > + switch (urb->status) {
> > > > + case -ECONNRESET:
> > > > + case -ESHUTDOWN:
> > > > + case -ENOENT:
> > > > + return;
> > > > + default:
> > > > + dev_err_ratelimited(dev->dev, "tx urb failed: %d\n",
> > > > + urb->status);
> > > > + /* fall through */
> > > > + case 0:
> > > > + break;
> > > > + }
> > > >
> > > > - if (mt7601u_urb_has_error(urb))
> > > > - dev_err(dev->dev, "Error: TX urb failed:%d\n", urb->status);
> > > > + spin_lock_irqsave(&dev->tx_lock, flags);
> > > > if (WARN_ONCE(q->e[q->start].urb != urb, "TX urb mismatch"))
> > > > goto out;
> > > >
> > > > skb = q->e[q->start].skb;
> > > > + q->e[q->start].skb = NULL;
> > > > trace_mt_tx_dma_done(dev, skb);
> > > >
> > > > __skb_queue_tail(&dev->tx_skb_done, skb);
> > > > @@ -446,10 +457,10 @@ static void mt7601u_free_tx_queue(struct mt7601u_tx_queue *q)
> > > > {
> > > > int i;
> > > >
> > > > - WARN_ON(q->used);
> > > > -
> > > > for (i = 0; i < q->entries; i++) {
> > > > usb_poison_urb(q->e[i].urb);
> > > > + if (q->e[i].skb)
> > > > + mt7601u_tx_status(q->dev, q->e[i].skb);
> > >
> > > Perhaps a separate patch?
> >
> > As I did for rx side, if we do not schedule the tx tasklet when the
> > device has been disconnected I guess we need this chuck in the same
> > patch. What do you think?
>
> Yeah, I see.
^ permalink raw reply
* Re: Help with encrypting PMF management frames
From: Ben Greear @ 2019-06-06 22:12 UTC (permalink / raw)
To: linux-wireless@vger.kernel.org
In-Reply-To: <fe571fe9-d5e5-38b8-5afa-2ba4fbb51f67@candelatech.com>
On 6/6/19 9:54 AM, Ben Greear wrote:
> Hello,
>
> My variant of ath10k uses the normal 'native-wifi' tx path for management frames.
> Internally in the firmware, it seems that the management TID is flagged to expect
> raw frames, and I think that is why I see Action frames on-air that are not actually
> encrypted but which have some space added to their packet that should be filled in by
> the hw-crypt engine.
>
> Is there a way to get mac80211 to software-crypt just management-tid PMF frames?
>
> So far, I have not been able to find the correct place in the tx logic of
> mac80211...
>
> Thanks,
> Ben
>
Ok, I found the issue. It seems the ath10k hardware refuses to encrypt
management frames unless they are sent in RAW mode (nwifi mgt frames are sent
w/out encryption for whatever reason).
So, the fix is in the ath10k driver (and just my driver I suppose, stock
driver uses a different API for mgt frames that eventually is raw-tx down in
the firmware).
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] mt7601u: fix possible memory leak when the device is disconnected
From: Jakub Kicinski @ 2019-06-06 22:19 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: Lorenzo Bianconi, linux-wireless
In-Reply-To: <CAJ0CqmW4SMCPxDAR360XO-kpev-5kHAgqaai9uSAiXth2WP36A@mail.gmail.com>
On Fri, 7 Jun 2019 00:05:56 +0200, Lorenzo Bianconi wrote:
> I guess this can be a future improvement, do you agree?
Def.
> Do I need to resubmit this patch?
My only concern is the Fixes tag, then. In the good old days we could
just wait a bit ourselves, now some bot will autoselect it, ugh.
But up to you, for the code:
Acked-by: Jakub Kicinski <kubakici@wp.pl>
^ permalink raw reply
* iwlwifi module crash
From: Balakrishnan Balasubramanian @ 2019-06-06 23:44 UTC (permalink / raw)
To: linux-wireless
[-- Attachment #1: Type: text/plain, Size: 209 bytes --]
I am using iwd demon for wifi. Once a while I loose connectivity. Restarting
the demon does not help. But once I restart the system, it starts working fine.
Attaching stack trace from journal.
Regards,
Bala
[-- Attachment #2: Denis Kenzior <denkenz@gmail.com>: Re: iwd crashes randomly --]
[-- Type: message/rfc822, Size: 3271 bytes --]
From: Denis Kenzior <denkenz@gmail.com>
To: Balakrishnan Balasubramanian <iwd-lists@balki.me>, iwd@lists.01.org
Subject: Re: iwd crashes randomly
Date: Thu, 06 Jun 2019 18:07:40 -0500
Message-ID: <9a29fc57-0547-203a-9e9f-417b16f9617e@gmail.com>
Hi Bala,
On 06/06/2019 06:00 PM, Balakrishnan Balasubramanian wrote:
> Sometimes after a week and sometimes after two days. Once crashed, restarting
> the service does not help. Had to restart the computer. Attaching stack trace
> from journal.
That implies that your kernel is crashing, not iwd. The attached log
shows a kernel stack trace somewhere inside iwlwifi module. I would
post this trace to linux-wireless@vger.kernel.org.
If you have an associated iwd backtrace, then certainly post this here,
but if the kernel module is crashing, there isn't much we can do.
Regards,
-Denis
[-- Attachment #3: iwd_crash.log --]
[-- Type: text/x-log, Size: 14052 bytes --]
Jun 03 19:53:47 zadesk dbus-daemon[475]: [system] Successfully activated service 'org.kde.powerdevil.backlighthelper'
Jun 03 21:32:49 zadesk dhcpcd[505]: wlan0: fe80::481d:70ff:feaf:2a13 is unreachable, expiring it
Jun 03 21:32:49 zadesk dhcpcd[505]: wlan0: carrier lost
Jun 03 21:32:49 zadesk dhcpcd[505]: wlan0: deleting address 2607:f2c0:e568:c9:3c6f:aab:250c:45d4/128
Jun 03 21:32:49 zadesk dhcpcd[505]: wlan0: deleting address 2607:f2c0:e568:c9:16d1:ac43:f2f7:9142/64
Jun 03 21:32:49 zadesk dhcpcd[505]: wlan0: deleting route to 2607:f2c0:e568:c9::/64
Jun 03 21:32:49 zadesk dhcpcd[505]: wlan0: deleting default route via fe80::481d:70ff:feaf:2a13
Jun 03 21:32:52 zadesk iwd[474]: Received Deauthentication event, reason: 4, from_ap: false
Jun 03 21:32:52 zadesk dhcpcd[505]: wlan0: deleting route to 192.168.0.0/24
Jun 03 21:32:52 zadesk kernel: wlan0: Connection to AP 00:00:00:00:00:00 lost
Jun 03 21:32:52 zadesk dhcpcd[505]: wlan0: deleting default route via 192.168.0.1
Jun 03 21:33:14 zadesk kernel: wlan0: authenticate with d4:5d:df:25:ee:90
Jun 03 21:33:14 zadesk kernel: wlan0: send auth to d4:5d:df:25:ee:90 (try 1/3)
Jun 03 21:33:14 zadesk kernel: wlan0: authenticated
Jun 03 21:33:14 zadesk kernel: wlan0: associate with d4:5d:df:25:ee:90 (try 1/3)
Jun 03 21:33:14 zadesk kernel: wlan0: associate with d4:5d:df:25:ee:90 (try 2/3)
Jun 03 21:33:14 zadesk kernel: wlan0: RX AssocResp from d4:5d:df:25:ee:90 (capab=0x411 status=0 aid=1)
Jun 03 21:33:14 zadesk kernel: wlan0: associated
Jun 03 21:33:14 zadesk dhcpcd[505]: wlan0: carrier acquired
Jun 03 21:33:14 zadesk dhcpcd[505]: wlan0: IAID ce:1e:6b:76
Jun 03 21:33:14 zadesk kernel: wlan0: Limiting TX power to 14 (17 - 3) dBm as advertised by d4:5d:df:25:ee:90
Jun 03 21:33:15 zadesk dhcpcd[505]: wlan0: soliciting an IPv6 router
Jun 03 21:33:15 zadesk dhcpcd[505]: wlan0: Router Advertisement from fe80::481d:70ff:feaf:2a13
Jun 03 21:33:15 zadesk dhcpcd[505]: wlan0: adding address 2607:f2c0:e568:c9:16d1:ac43:f2f7:9142/64
Jun 03 21:33:15 zadesk dhcpcd[505]: wlan0: adding route to 2607:f2c0:e568:c9::/64
Jun 03 21:33:15 zadesk dhcpcd[505]: wlan0: adding default route via fe80::481d:70ff:feaf:2a13
Jun 03 21:33:15 zadesk dhcpcd[505]: wlan0: confirming prior DHCPv6 lease
Jun 03 21:33:15 zadesk dhcpcd[505]: wlan0: rebinding lease of 192.168.0.16
Jun 03 21:33:16 zadesk dhcpcd[505]: wlan0: probing address 192.168.0.16/24
Jun 03 21:33:16 zadesk dhcpcd[505]: wlan0: REPLY6 received from fe80::481d:70ff:feaf:2a13
Jun 03 21:33:16 zadesk dhcpcd[505]: wlan0: adding address 2607:f2c0:e568:c9:3c6f:aab:250c:45d4/128
Jun 03 21:33:16 zadesk dhcpcd[505]: wlan0: renew in 302400, rebind in 483840, expire in 604800 seconds
Jun 03 21:33:21 zadesk dhcpcd[505]: wlan0: leased 192.168.0.16 for 604800 seconds
Jun 03 21:33:21 zadesk dhcpcd[505]: wlan0: adding route to 192.168.0.0/24
Jun 03 21:33:21 zadesk dhcpcd[505]: wlan0: adding default route via 192.168.0.1
Jun 03 21:34:03 zadesk kernel: ------------[ cut here ]------------
Jun 03 21:34:03 zadesk kernel: Timeout waiting for hardware access (CSR_GP_CNTRL 0xffffffff)
Jun 03 21:34:03 zadesk kernel: WARNING: CPU: 2 PID: 0 at drivers/net/wireless/intel/iwlwifi/pcie/trans.c:1988 iwl_trans_pcie_grab_nic_access+0x1e6/0x220 [iwlwifi]
Jun 03 21:34:03 zadesk kernel: Modules linked in: fuse snd_hda_codec_hdmi ccm algif_aead des_generic algif_skcipher 8021q garp mrp stp cmac llc md4 algif_hash af_alg arc4 intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel i915 kvm iwlmvm uvcvideo irqbypass i2c_algo_bit mac80211 videobuf2_vmalloc videobuf2_memops snd_hda_codec_realtek drm_kms_helper snd_hda_codec_generic videobuf2_v4l2 ledtrig_audio videobuf2_common snd_hda_intel crct10dif_pclmul videodev snd_hda_codec drm crc32_pclmul iwlwifi ghash_clmulni_intel btusb btrtl btbcm btintel bluetooth snd_hda_core ofpart cmdlinepart media intel_spi_platform snd_hwdep intel_spi spi_nor iTCO_wdt mei_wdt mei_hdcp mtd iTCO_vendor_support aesni_intel snd_pcm aes_x86_64 crypto_simd cryptd snd_timer intel_gtt glue_helper intel_cstate intel_uncore joydev mousedev agpgart input_leds syscopyarea r8169 intel_rapl_perf ecdh_generic cfg80211 snd sysfillrect mei_me pcspkr i2c_i801 realtek sysimgblt soundcore mei libphy fb_sys_fops rfkill lpc_ich fintek_cir
Jun 03 21:34:03 zadesk kernel: rc_core tpm_tis tpm_tis_core tpm evdev rng_core mac_hid pcc_cpufreq pkcs8_key_parser crypto_user ip_tables x_tables ext4 crc32c_generic crc16 mbcache jbd2 hid_logitech_hidpp hid_logitech_dj hid_generic usbhid hid sr_mod cdrom sd_mod ahci xhci_pci libahci xhci_hcd libata crc32c_intel scsi_mod ehci_pci ehci_hcd
Jun 03 21:34:03 zadesk kernel: CPU: 2 PID: 0 Comm: swapper/2 Not tainted 5.1.5-arch1-2-ARCH #1
Jun 03 21:34:03 zadesk kernel: Hardware name: AIO H87H3-TI/H87H3-TI, BIOS 4.6.5 07/01/2014
Jun 03 21:34:03 zadesk kernel: RIP: 0010:iwl_trans_pcie_grab_nic_access+0x1e6/0x220 [iwlwifi]
Jun 03 21:34:03 zadesk kernel: Code: b1 cf 49 8d 56 08 bf 40 01 00 00 e8 94 1b 88 ce e9 35 ff ff ff 89 c6 48 c7 c7 70 7b c4 c0 c6 05 b7 7d 03 00 01 e8 94 62 86 ce <0f> 0b e9 f0 fe ff ff 48 8b 7b 30 48 c7 c1 d8 7b c4 c0 31 d2 31 f6
Jun 03 21:34:03 zadesk kernel: RSP: 0018:ffff981696903de8 EFLAGS: 00010082
Jun 03 21:34:03 zadesk kernel: RAX: 0000000000000000 RBX: ffff981693d80018 RCX: 0000000000000000
Jun 03 21:34:03 zadesk kernel: RDX: 0000000000000007 RSI: 0000000000000092 RDI: 00000000ffffffff
Jun 03 21:34:03 zadesk kernel: RBP: 0000000000000000 R08: 00000000000003a1 R09: 0000000000000001
Jun 03 21:34:03 zadesk kernel: R10: 0000000000000000 R11: 0000000000000001 R12: ffff981693d8a3a4
Jun 03 21:34:03 zadesk kernel: R13: ffff981696903e18 R14: 00000000ffffffff R15: 0000000000000004
Jun 03 21:34:03 zadesk kernel: FS: 0000000000000000(0000) GS:ffff981696900000(0000) knlGS:0000000000000000
Jun 03 21:34:03 zadesk kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Jun 03 21:34:03 zadesk kernel: CR2: 00007ffa69d3c020 CR3: 00000001b820e002 CR4: 00000000001606e0
Jun 03 21:34:03 zadesk kernel: Call Trace:
Jun 03 21:34:03 zadesk kernel: <IRQ>
Jun 03 21:34:03 zadesk kernel: iwl_read_prph+0x32/0x80 [iwlwifi]
Jun 03 21:34:03 zadesk kernel: iwl_trans_pcie_log_scd_error+0x139/0x210 [iwlwifi]
Jun 03 21:34:03 zadesk kernel: iwl_pcie_txq_stuck_timer+0x44/0x60 [iwlwifi]
Jun 03 21:34:03 zadesk kernel: ? iwl_pcie_txq_inc_wr_ptr+0x100/0x100 [iwlwifi]
Jun 03 21:34:03 zadesk kernel: ? iwl_pcie_txq_inc_wr_ptr+0x100/0x100 [iwlwifi]
Jun 03 21:34:03 zadesk kernel: call_timer_fn+0x2b/0x160
Jun 03 21:34:03 zadesk kernel: ? iwl_pcie_txq_inc_wr_ptr+0x100/0x100 [iwlwifi]
Jun 03 21:34:03 zadesk kernel: expire_timers+0x99/0x110
Jun 03 21:34:03 zadesk kernel: run_timer_softirq+0x8a/0x160
Jun 03 21:34:03 zadesk kernel: ? sched_clock+0x5/0x10
Jun 03 21:34:03 zadesk kernel: ? sched_clock_cpu+0xe/0xd0
Jun 03 21:34:03 zadesk kernel: ? irqtime_account_irq+0x3d/0xc0
Jun 03 21:34:03 zadesk kernel: __do_softirq+0x112/0x356
Jun 03 21:34:03 zadesk kernel: irq_exit+0xd9/0xf0
Jun 03 21:34:03 zadesk kernel: smp_apic_timer_interrupt+0x87/0x180
Jun 03 21:34:03 zadesk kernel: apic_timer_interrupt+0xf/0x20
Jun 03 21:34:03 zadesk kernel: </IRQ>
Jun 03 21:34:03 zadesk kernel: RIP: 0010:cpuidle_enter_state+0xbc/0x480
Jun 03 21:34:03 zadesk kernel: Code: e8 a9 c0 9f ff 80 7c 24 13 00 74 17 9c 58 0f 1f 44 00 00 f6 c4 02 0f 85 99 03 00 00 31 ff e8 3b 03 a6 ff fb 66 0f 1f 44 00 00 <45> 85 e4 0f 88 c4 02 00 00 49 63 cc 4c 8b 3c 24 4c 2b 7c 24 08 48
Jun 03 21:34:03 zadesk kernel: RSP: 0018:ffff9fec80cf7e98 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
Jun 03 21:34:03 zadesk kernel: RAX: ffff981696900000 RBX: ffffffff906bd5c0 RCX: 000000000000001f
Jun 03 21:34:03 zadesk kernel: RDX: 0000083837ad6417 RSI: 000000002c3d7be3 RDI: 0000000000000000
Jun 03 21:34:03 zadesk kernel: RBP: ffff98169692be00 R08: 0000000000000002 R09: 0000000000021740
Jun 03 21:34:03 zadesk kernel: R10: 000017ceeb3a3497 R11: ffff981696920d64 R12: 0000000000000005
Jun 03 21:34:03 zadesk kernel: R13: ffffffff906bd7b8 R14: 0000000000000005 R15: 0000000000000000
Jun 03 21:34:03 zadesk kernel: ? cpuidle_enter_state+0x97/0x480
Jun 03 21:34:03 zadesk kernel: do_idle+0x217/0x250
Jun 03 21:34:03 zadesk kernel: cpu_startup_entry+0x19/0x20
Jun 03 21:34:03 zadesk kernel: start_secondary+0x180/0x1d0
Jun 03 21:34:03 zadesk kernel: secondary_startup_64+0xa4/0xb0
Jun 03 21:34:03 zadesk kernel: ---[ end trace d9201afd2c4b3e67 ]---
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:02:00.0: iwlwifi transaction failed, dumping registers
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:02:00.0: iwlwifi device config registers:
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:02:00.0: 00000000: 08b18086 00100000 028000bb 00000000 00000004 00000000 00000000 00000000
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:02:00.0: 00000020: 00000000 00000000 00000000 40708086 00000000 000000c8 00000000 00000100
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:02:00.0: iwlwifi device memory mapped registers:
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:02:00.0: 00000000: ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:02:00.0: 00000020: ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:02:00.0: iwlwifi device AER capability structure:
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:02:00.0: 00000000: 14010001 00100000 00000000 00462031 000031c1 00002000 00000014 40000001
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:02:00.0: 00000020: 0000000f f7d00460 00000000
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:02:00.0: iwlwifi parent port (0000:00:1c.2) config registers:
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:00:1c.2: 00000000: 8c148086 00100407 060400d5 00810010 00000000 00000000 00020200 200000f0
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:00:1c.2: 00000020: f7d0f7d0 0001fff1 00000000 00000000 00000000 00000040 00000000 00120304
Jun 03 21:34:03 zadesk kernel: iwlwifi 0000:02:00.0: Queue 4 is active on fifo 2 and stuck for 10000 ms. SW [87, 103] HW [90, 90] FH TRB=0x05a5a5a5a
Jun 03 21:34:23 zadesk kernel: iwlwifi 0000:02:00.0: Failed to wake NIC for hcmd
Jun 03 21:34:23 zadesk kernel: iwlwifi 0000:02:00.0: Error sending SCAN_OFFLOAD_REQUEST_CMD: enqueue_hcmd failed: -5
Jun 03 21:34:23 zadesk kernel: iwlwifi 0000:02:00.0: Scan failed! ret -5
Jun 03 21:34:23 zadesk iwd[474]: Received error during CMD_TRIGGER_SCAN: Input/output error (5)
Jun 03 21:35:19 zadesk dhcpcd[505]: wlan0: fe80::481d:70ff:feaf:2a13 is unreachable, expiring it
Jun 03 21:35:23 zadesk kernel: iwlwifi 0000:02:00.0: Failed to wake NIC for hcmd
Jun 03 21:35:23 zadesk kernel: iwlwifi 0000:02:00.0: Error sending SCAN_OFFLOAD_REQUEST_CMD: enqueue_hcmd failed: -5
Jun 03 21:35:23 zadesk kernel: iwlwifi 0000:02:00.0: Scan failed! ret -5
Jun 03 21:35:23 zadesk iwd[474]: Received error during CMD_TRIGGER_SCAN: Input/output error (5)
Jun 03 21:36:23 zadesk kernel: iwlwifi 0000:02:00.0: Failed to wake NIC for hcmd
Jun 03 21:36:23 zadesk kernel: iwlwifi 0000:02:00.0: Error sending SCAN_OFFLOAD_REQUEST_CMD: enqueue_hcmd failed: -5
Jun 03 21:36:23 zadesk kernel: iwlwifi 0000:02:00.0: Scan failed! ret -5
Jun 03 21:36:23 zadesk iwd[474]: Received error during CMD_TRIGGER_SCAN: Input/output error (5)
Jun 03 21:37:23 zadesk kernel: iwlwifi 0000:02:00.0: Failed to wake NIC for hcmd
Jun 03 21:37:23 zadesk kernel: iwlwifi 0000:02:00.0: Error sending SCAN_OFFLOAD_REQUEST_CMD: enqueue_hcmd failed: -5
Jun 03 21:37:23 zadesk kernel: iwlwifi 0000:02:00.0: Scan failed! ret -5
Jun 03 21:37:23 zadesk iwd[474]: Received error during CMD_TRIGGER_SCAN: Input/output error (5)
Jun 03 21:38:24 zadesk kernel: iwlwifi 0000:02:00.0: Failed to wake NIC for hcmd
Jun 03 21:38:24 zadesk kernel: iwlwifi 0000:02:00.0: Error sending SCAN_OFFLOAD_REQUEST_CMD: enqueue_hcmd failed: -5
Jun 03 21:38:24 zadesk kernel: iwlwifi 0000:02:00.0: Scan failed! ret -5
Jun 03 21:38:24 zadesk iwd[474]: Received error during CMD_TRIGGER_SCAN: Input/output error (5)
Jun 03 21:39:24 zadesk kernel: iwlwifi 0000:02:00.0: Failed to wake NIC for hcmd
Jun 03 21:39:24 zadesk kernel: iwlwifi 0000:02:00.0: Error sending SCAN_OFFLOAD_REQUEST_CMD: enqueue_hcmd failed: -5
Jun 03 21:39:24 zadesk kernel: iwlwifi 0000:02:00.0: Scan failed! ret -5
Jun 03 21:39:24 zadesk iwd[474]: Received error during CMD_TRIGGER_SCAN: Input/output error (5)
Jun 03 21:40:24 zadesk kernel: iwlwifi 0000:02:00.0: Failed to wake NIC for hcmd
Jun 03 21:40:24 zadesk kernel: iwlwifi 0000:02:00.0: Error sending SCAN_OFFLOAD_REQUEST_CMD: enqueue_hcmd failed: -5
Jun 03 21:40:24 zadesk kernel: iwlwifi 0000:02:00.0: Scan failed! ret -5
Jun 03 21:40:24 zadesk iwd[474]: Received error during CMD_TRIGGER_SCAN: Input/output error (5)
Jun 03 21:41:24 zadesk kernel: iwlwifi 0000:02:00.0: Failed to wake NIC for hcmd
Jun 03 21:41:24 zadesk kernel: iwlwifi 0000:02:00.0: Error sending SCAN_OFFLOAD_REQUEST_CMD: enqueue_hcmd failed: -5
Jun 03 21:41:24 zadesk kernel: iwlwifi 0000:02:00.0: Scan failed! ret -5
Jun 03 21:41:24 zadesk iwd[474]: Received error during CMD_TRIGGER_SCAN: Input/output error (5)
Jun 03 21:42:24 zadesk kernel: iwlwifi 0000:02:00.0: Failed to wake NIC for hcmd
Jun 03 21:42:24 zadesk kernel: iwlwifi 0000:02:00.0: Error sending SCAN_OFFLOAD_REQUEST_CMD: enqueue_hcmd failed: -5
Jun 03 21:42:24 zadesk kernel: iwlwifi 0000:02:00.0: Scan failed! ret -5
Jun 03 21:42:24 zadesk iwd[474]: Received error during CMD_TRIGGER_SCAN: Input/output error (5)
Jun 03 21:43:24 zadesk kernel: iwlwifi 0000:02:00.0: Failed to wake NIC for hcmd
Jun 03 21:43:24 zadesk kernel: iwlwifi 0000:02:00.0: Error sending SCAN_OFFLOAD_REQUEST_CMD: enqueue_hcmd failed: -5
Jun 03 21:43:24 zadesk kernel: iwlwifi 0000:02:00.0: Scan failed! ret -5
Jun 03 21:43:24 zadesk iwd[474]: Received error during CMD_TRIGGER_SCAN: Input/output error (5)
Jun 03 21:44:24 zadesk kernel: iwlwifi 0000:02:00.0: Failed to wake NIC for hcmd
^ permalink raw reply
* Re: [PATCH V2] iw: print HE capabilities
From: Joe Perches @ 2019-06-07 2:11 UTC (permalink / raw)
To: John Crispin, Johannes Berg; +Cc: linux-wireless, Shashidhar Lakkavalli
In-Reply-To: <20190528065828.25356-1-john@phrozen.org>
On Tue, 2019-05-28 at 08:58 +0200, John Crispin wrote:
> Print the HE MAC/PHY capabilities and MCS/NSS sets.
trivia:
> diff --git a/util.c b/util.c
[]
> +void print_he_info(struct nlattr *nl_iftype)
> +{
> + struct nlattr *tb[NL80211_BAND_IFTYPE_ATTR_MAX + 1];
> + struct nlattr *tb_flags[NL80211_IFTYPE_MAX + 1];
> + char *iftypes[NUM_NL80211_IFTYPES] = {
> + "Unspec", "Adhoc", "Station", "AP", "AP/VLAN", "WDS", "Monitor",
> + "Mesh", "P2P/Client", "P2P/Go", "P2P/Device", "OCB", "NAN",
> + };
Should probably be
const char * const iftypes[...]
^ permalink raw reply
* Re: [PATCH v2 3/3] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
From: Arend Van Spriel @ 2019-06-07 5:12 UTC (permalink / raw)
To: Doug Anderson, Adrian Hunter
Cc: Ulf Hansson, Kalle Valo, brcm80211-dev-list.pdl,
open list:ARM/Rockchip SoC..., Double Lo, Brian Norris,
linux-wireless, Naveen Gupta, Madhan Mohan R, Matthias Kaehlcke,
Wright Feng, Chi-Hsien Lin, netdev, brcm80211-dev-list,
David S. Miller, Franky Lin, LKML, Hante Meuleman, YueHaibing,
Michael Trimarchi
In-Reply-To: <CAD=FV=UPfCOr-syAbVZ-FjHQy7bgQf5BS5pdV-Bwd3hquRqEGg@mail.gmail.com>
On June 6, 2019 11:37:22 PM Doug Anderson <dianders@chromium.org> wrote:
>
> In the case of dw_mmc, which I'm most familiar with, we don't have any
> sort of automated or timed-based retuning. ...so we'll only re-tune
> when we see the CRC error. If I'm understanding things correctly then
> that for dw_mmc my solution and yours behave the same. That means the
> difference is how we deal with other retuning requests, either ones
> that come about because of an interrupt that the host controller
> provided or because of a timer. Did I get that right?
Right.
> ...and I guess the reason we have to deal specially with these cases
> is because any time that SDIO card is "sleeping" we don't want to
> retune because it won't work. Right? NOTE: the solution that would
> come to my mind first to solve this would be to hold the retuning for
> the whole time that the card was sleeping and then release it once the
> card was awake again. ...but I guess we don't truly need to do that
> because tuning only happens as a side effect of sending a command to
> the card and the only command we send to the card is the "wake up"
> command. That's why your solution to hold tuning while sending the
> "wake up" command works, right?
Yup.
> ---
>
> OK, so assuming all the above is correct, I feel like we're actually
> solving two problems and in fact I believe we actually need both our
> approaches to solve everything correctly. With just your patch in
> place there's a problem because we will clobber any external retuning
> requests that happened while we were waking up the card. AKA, imagine
> this:
>
> A) brcmf_sdio_kso_control(on=True) gets called; need_retune starts as 0
>
> B) We call sdio_retune_hold_now()
>
> C) A retuning timer goes off or the SD Host controller tells us to retune
>
> D) We get to the end of brcmf_sdio_kso_control() and clear the "retune
> needed" since need_retune was 0 at the start.
>
> ...so we dropped the retuning request from C), right?
>
>
> What we truly need is:
>
> 1. CRC errors shouldn't trigger a retuning request when we're in
> brcmf_sdio_kso_control()
>
> 2. A separate patch that holds any retuning requests while the SDIO
> card is off. This patch _shouldn't_ do any clearing of retuning
> requests, just defer them.
>
>
> Does that make sense to you? If so, I can try to code it up...
FWIW it does make sense to me. However, I am still not sure if our sdio
hardware supports retuning. Have to track down an asic designer who can
tell or dive into vhdl myself.
So I want to disable device sleep and trigger retuning through debugfs or
some other hack.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH v3 1/2] mt76: mt7615: enable support for mesh
From: Sebastian Gottschall @ 2019-06-07 6:03 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Ryder Lee, Felix Fietkau, Roy Luo, YF Luo, Yiwei Chung, Sean Wang,
Chih-Min Chen, linux-wireless, linux-mediatek,
Linux Kernel Mailing List
In-Reply-To: <CAJ0CqmVBogQrqf4Gckr5gQ6tCrdZG=p60ZiC+-WW-yxt93+40Q@mail.gmail.com>
Am 06.06.2019 um 18:19 schrieb Lorenzo Bianconi:
>> i tested your patch against a qca 9984 chipset using SAE and without
>> encryption. both did not work. the devices are connecting, but no data
>> connection is possible
> Hi Sebastian,
>
> I tested Ryder's patch using mt76x2 as mesh peer and it works fine for me.
> Could you please provide some more info?
the peer was a QCA9984 chipset running ath10k. the following wpa
supplicant config was used
and encryption clearly cannot work since mt76_wcid_key_setup does
trigger a bug since ieeee_get_key_rx_seq does not accept tid as
argument != 0 for aes_cmac (which is used for sae)
consider the same setup works with ath5k, 9k and 10k perfect with no
issues. in my test setup i also connected now 3 devices. 2 9984 and 1
mt7615 to ensure that all is working. the qca 9984 devices can ping each
other, only the mt7615 does not work. the setups are all identical
(except for ips)
fast_reauth=1
eapol_version=1
sae_groups=19 20 21
network={
disable_ht40=1
ssid="mt7615-24"
mode=5
frequency=2452
htmode=HT20
scan_ssid=1
key_mgmt=SAE
ieee80211w=2
noscan=1
pairwise=CCMP
group=CCMP
proto=RSN
sae_password="12345678"
}
and the bug again
WARNING: CPU: 2 PID: 22086 at
/home/seg/DEV/mt7621/src/router/private/compat-wireless-2017-09-03/net/mac80211/key.c:1096
mt76_wcid_key_setup+0x58/0x9c [mt76]
Modules linked in: shortcut_fe gcm ghash_generic ctr gf128mul mt7615e
mt76 mac80211 compat
CPU: 2 PID: 22086 Comm: wpa_supplicant Tainted: G W 4.14.123 #106
Stack : 00000000 87c2d000 00000000 8007d8b4 80480000 80482b9c 80610000
805a4390
8057e2b4 87c4b99c 870a59dc 805e4767 80578288 00000001 87c4b940
805e9f78
00000000 00000000 80640000 00000000 81147bb8 000006ce 00000007
00000000
00000000 80650000 80650000 20202020 80000000 00000000 80610000
872b9fe0
872a2b14 00000448 00000000 87c2d000 00000010 8022d660 00000008
80640008
...
Call Trace:
[<800153e0>] show_stack+0x58/0x100
[<8042e83c>] dump_stack+0x9c/0xe0
[<800349f0>] __warn+0xe4/0x144
[<8003468c>] warn_slowpath_null+0x1c/0x30
[<872b9fe0>] mt76_wcid_key_setup+0x58/0x9c [mt76]
[<87611690>] mt7615_eeprom_init+0x7b4/0xe9c [mt7615e]
---[ end trace e24aeb4b542e0df9 ]---
>
> Regards,
> Lorenzo
>
>>
>> Sebastian
>>
>> Am 03.06.2019 um 08:08 schrieb Ryder Lee:
>>> Enable NL80211_IFTYPE_MESH_POINT and update its path.
>>>
>>> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
>>> ---
>>> Changes since v3 - fix a wrong expression
>>> Changes since v2 - remove unused definitions
>>> ---
>>> drivers/net/wireless/mediatek/mt76/mt7615/init.c | 6 ++++++
>>> drivers/net/wireless/mediatek/mt76/mt7615/main.c | 1 +
>>> drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 4 +++-
>>> drivers/net/wireless/mediatek/mt76/mt7615/mcu.h | 6 ------
>>> 4 files changed, 10 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/init.c b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
>>> index 59f604f3161f..f860af6a42da 100644
>>> --- a/drivers/net/wireless/mediatek/mt76/mt7615/init.c
>>> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
>>> @@ -133,6 +133,9 @@ static const struct ieee80211_iface_limit if_limits[] = {
>>> {
>>> .max = MT7615_MAX_INTERFACES,
>>> .types = BIT(NL80211_IFTYPE_AP) |
>>> +#ifdef CONFIG_MAC80211_MESH
>>> + BIT(NL80211_IFTYPE_MESH_POINT) |
>>> +#endif
>>> BIT(NL80211_IFTYPE_STATION)
>>> }
>>> };
>>> @@ -195,6 +198,9 @@ int mt7615_register_device(struct mt7615_dev *dev)
>>> dev->mt76.antenna_mask = 0xf;
>>>
>>> wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
>>> +#ifdef CONFIG_MAC80211_MESH
>>> + BIT(NL80211_IFTYPE_MESH_POINT) |
>>> +#endif
>>> BIT(NL80211_IFTYPE_AP);
>>>
>>> ret = mt76_register_device(&dev->mt76, true, mt7615_rates,
>>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/main.c b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
>>> index b0bb7cc12385..585e67fa2728 100644
>>> --- a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
>>> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
>>> @@ -37,6 +37,7 @@ static int get_omac_idx(enum nl80211_iftype type, u32 mask)
>>>
>>> switch (type) {
>>> case NL80211_IFTYPE_AP:
>>> + case NL80211_IFTYPE_MESH_POINT:
>>> /* ap use hw bssid 0 and ext bssid */
>>> if (~mask & BIT(HW_BSSID_0))
>>> return HW_BSSID_0;
>>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
>>> index 43f70195244c..e82297048449 100644
>>> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
>>> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
>>> @@ -754,6 +754,7 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
>>>
>>> switch (vif->type) {
>>> case NL80211_IFTYPE_AP:
>>> + case NL80211_IFTYPE_MESH_POINT:
>>> tx_wlan_idx = mvif->sta.wcid.idx;
>>> conn_type = CONNECTION_INFRA_AP;
>>> break;
>>> @@ -968,7 +969,7 @@ int mt7615_mcu_add_wtbl(struct mt7615_dev *dev, struct ieee80211_vif *vif,
>>> .rx_wtbl = {
>>> .tag = cpu_to_le16(WTBL_RX),
>>> .len = cpu_to_le16(sizeof(struct wtbl_rx)),
>>> - .rca1 = vif->type != NL80211_IFTYPE_AP,
>>> + .rca1 = vif->type == NL80211_IFTYPE_STATION,
>>> .rca2 = 1,
>>> .rv = 1,
>>> },
>>> @@ -1042,6 +1043,7 @@ static void sta_rec_convert_vif_type(enum nl80211_iftype type, u32 *conn_type)
>>> {
>>> switch (type) {
>>> case NL80211_IFTYPE_AP:
>>> + case NL80211_IFTYPE_MESH_POINT:
>>> if (conn_type)
>>> *conn_type = CONNECTION_INFRA_STA;
>>> break;
>>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
>>> index e96efb13fa4d..0915cb735699 100644
>>> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
>>> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
>>> @@ -105,25 +105,19 @@ enum {
>>> #define STA_TYPE_STA BIT(0)
>>> #define STA_TYPE_AP BIT(1)
>>> #define STA_TYPE_ADHOC BIT(2)
>>> -#define STA_TYPE_TDLS BIT(3)
>>> #define STA_TYPE_WDS BIT(4)
>>> #define STA_TYPE_BC BIT(5)
>>>
>>> #define NETWORK_INFRA BIT(16)
>>> #define NETWORK_P2P BIT(17)
>>> #define NETWORK_IBSS BIT(18)
>>> -#define NETWORK_MESH BIT(19)
>>> -#define NETWORK_BOW BIT(20)
>>> #define NETWORK_WDS BIT(21)
>>>
>>> #define CONNECTION_INFRA_STA (STA_TYPE_STA | NETWORK_INFRA)
>>> #define CONNECTION_INFRA_AP (STA_TYPE_AP | NETWORK_INFRA)
>>> #define CONNECTION_P2P_GC (STA_TYPE_STA | NETWORK_P2P)
>>> #define CONNECTION_P2P_GO (STA_TYPE_AP | NETWORK_P2P)
>>> -#define CONNECTION_MESH_STA (STA_TYPE_STA | NETWORK_MESH)
>>> -#define CONNECTION_MESH_AP (STA_TYPE_AP | NETWORK_MESH)
>>> #define CONNECTION_IBSS_ADHOC (STA_TYPE_ADHOC | NETWORK_IBSS)
>>> -#define CONNECTION_TDLS (STA_TYPE_STA | NETWORK_INFRA | STA_TYPE_TDLS)
>>> #define CONNECTION_WDS (STA_TYPE_WDS | NETWORK_WDS)
>>> #define CONNECTION_INFRA_BC (STA_TYPE_BC | NETWORK_INFRA)
>>>
^ permalink raw reply
* Re: [PATCH V2] iw: print HE capabilities
From: Arend Van Spriel @ 2019-06-07 7:22 UTC (permalink / raw)
To: Joe Perches, John Crispin, Johannes Berg
Cc: linux-wireless, Shashidhar Lakkavalli
In-Reply-To: <c3f982260cdf0749201f5535bf0f3bf9f6bfa7ba.camel@perches.com>
On 6/7/2019 4:11 AM, Joe Perches wrote:
> On Tue, 2019-05-28 at 08:58 +0200, John Crispin wrote:
>> Print the HE MAC/PHY capabilities and MCS/NSS sets.
>
> trivia:
>
>> diff --git a/util.c b/util.c
> []
>> +void print_he_info(struct nlattr *nl_iftype)
>> +{
>> + struct nlattr *tb[NL80211_BAND_IFTYPE_ATTR_MAX + 1];
>> + struct nlattr *tb_flags[NL80211_IFTYPE_MAX + 1];
>> + char *iftypes[NUM_NL80211_IFTYPES] = {
>> + "Unspec", "Adhoc", "Station", "AP", "AP/VLAN", "WDS", "Monitor",
>> + "Mesh", "P2P/Client", "P2P/Go", "P2P/Device", "OCB", "NAN",
>> + };
>
> Should probably be
> const char * const iftypes[...]
We are doing this iftype string mapping in several places so a helper
function may be warranted.
Gr. AvS
^ permalink raw reply
* pull-request: wireless-drivers 2019-06-07
From: Kalle Valo @ 2019-06-07 9:19 UTC (permalink / raw)
To: David Miller; +Cc: linux-wireless, netdev, linux-kernel
Hi Dave,
here's a pull request to net tree for 5.2, more info below. Please let
me know if there are any problems.
Kalle
The following changes since commit 3e66b7cc50ef921121babc91487e1fb98af1ba6e:
net: tulip: de4x5: Drop redundant MODULE_DEVICE_TABLE() (2019-05-26 21:50:11 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git tags/wireless-drivers-for-davem-2019-06-07
for you to fetch changes up to 69ae4f6aac1578575126319d3f55550e7e440449:
mwifiex: Fix heap overflow in mwifiex_uap_parse_tail_ies() (2019-06-01 08:06:24 +0300)
----------------------------------------------------------------
wireless-drivers fixes for 5.2
First set of fixes for 5.2. Most important here are buffer overflow
fixes for mwifiex.
rtw88
* fix out of bounds compiler warning
* fix rssi handling to get 4x more throughput
* avoid circular locking
rsi
* fix unitilised data warning, these are hopefully the last ones so
that the warning can be enabled by default
mwifiex
* fix buffer overflows
iwlwifi
* remove not used debugfs file
* various fixes
----------------------------------------------------------------
Emmanuel Grumbach (1):
iwlwifi: fix load in rfkill flow for unified firmware
Jia-Ju Bai (1):
iwlwifi: Fix double-free problems in iwl_req_fw_callback()
Johannes Berg (1):
iwlwifi: mvm: remove d3_sram debugfs file
Lior Cohen (1):
iwlwifi: mvm: change TLC config cmd sent by rs to be async
Matt Chen (1):
iwlwifi: fix AX201 killer sku loading firmware issue
Nathan Chancellor (1):
rsi: Properly initialize data in rsi_sdio_ta_reset
Shahar S Matityahu (2):
iwlwifi: clear persistence bit according to device family
iwlwifi: print fseq info upon fw assert
Stanislaw Gruszka (2):
rtw88: fix subscript above array bounds compiler warning
rtw88: avoid circular locking between local->iflist_mtx and rtwdev->mutex
Takashi Iwai (3):
mwifiex: Fix possible buffer overflows at parsing bss descriptor
mwifiex: Abort at too short BSS descriptor element
mwifiex: Fix heap overflow in mwifiex_uap_parse_tail_ies()
Yan-Hsuan Chuang (1):
rtw88: fix unassigned rssi_level in rtw_sta_info
YueHaibing (1):
rtw88: Make some symbols static
drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 39 +++++++++++++++
drivers/net/wireless/intel/iwlwifi/fw/dbg.h | 2 +
drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 1 -
drivers/net/wireless/intel/iwlwifi/iwl-prph.h | 22 ++++++++-
drivers/net/wireless/intel/iwlwifi/mvm/d3.c | 22 ---------
drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 57 ----------------------
drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 23 ++++++---
drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c | 2 +-
drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 4 +-
drivers/net/wireless/intel/iwlwifi/mvm/ops.c | 20 +++++---
drivers/net/wireless/intel/iwlwifi/mvm/rs-fw.c | 3 +-
drivers/net/wireless/intel/iwlwifi/mvm/utils.c | 2 +
drivers/net/wireless/intel/iwlwifi/pcie/internal.h | 2 +-
drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 53 ++++++++++++++------
drivers/net/wireless/marvell/mwifiex/ie.c | 47 ++++++++++++------
drivers/net/wireless/marvell/mwifiex/scan.c | 19 ++++++++
drivers/net/wireless/realtek/rtw88/fw.c | 6 ++-
drivers/net/wireless/realtek/rtw88/main.c | 3 +-
drivers/net/wireless/realtek/rtw88/phy.c | 22 ++++++---
drivers/net/wireless/rsi/rsi_91x_sdio.c | 21 +++++---
20 files changed, 220 insertions(+), 150 deletions(-)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox