On 02/01/2016 09:46 AM, Hante Meuleman wrote:
Hi,
Didn’t know that that patch got reverted. Our internal repository still has that patch and we have had no problems whatsoever for the last 7 (june 26 this got submitted internally) months on any pcie target (also used it on r8000 openwrt). Rafal's patch is overdone. If you don’t up the hashsize space then there is really no use to switch to u16. You can simply limit the nr of flowrings to 255 in brcmf_proto_msgbuf_attach or apply the patch I originally submitted.
Ok. While no issues were seen I think we can not ignore the reported
problem.
Rafal,
Can you try Hante's patch on your current branch, ie. incorporate hash
size change and see if this shows any issues on 43602 target. If so full
driver logging would be great so we can look at that.
Regards,
Arend
Regards,
Hante
-----Original Message-----
From: Rafał Miłecki [mailto:zajec5@xxxxxxxxx]
Sent: Sunday, January 31, 2016 12:44 PM
To: Arend van Spriel
Cc: Kalle Valo; linux-wireless@xxxxxxxxxxxxxxx; Brett Rudley; Arend Van Spriel; Franky (Zhenhui) Lin; Hante Meuleman; brcm80211-dev-list
Subject: Re: [PATCH FIX?] brcmfmac: fix possible overflows in flowrings code by bumping u8 to u16
On 31 January 2016 at 10:56, Arend van Spriel <aspriel@xxxxxxxxx> wrote:
On 31-01-16 01:07, Rafał Miłecki wrote:
Some devices may use more than 255 flowings, below is log from BCM4366:
[ 194.606245] brcmfmac: brcmf_pcie_init_ringbuffers Nr of flowrings is 264
At various places we were using u8 which could lead to storing wrong
number or infinite loops when indexing incorrectly. Initially this
issue was spotted as infinite loop in brcmf_flowring_detach.
There has already been a patch submitted for this [1]. However, because
you reported issues with it on your device (not sure which one). Did you
test this patch on that particular device.
I wasn't aware Hante's patch contained changes from this patch. Anyway
the main difference is that my patch doesn't touch
BRCMF_FLOWRING_HASHSIZE.
So my patch:
1) Fixes possible overflows in flowrings
Hante's patch:
1) Fixes possible overflows in flowrings
2) Bumps BRCMF_FLOWRING_HASHSIZE
It was bumping BRCMF_FLOWRING_HASHSIZE that caused problems on my
BCM43602 device back then. Please note BCM43602 wasn't affected by
flowings overflows because it wasn't using more than 255 of them:
brcmfmac: brcmf_pcie_init_ringbuffers Nr of flowrings is 132
The story is different with my BCM4366. I didn't try it with bumping
BRCMF_FLOWRING_HASHSIZE but it suffers from overflows in flowrings as
it seems to be independent issue. It's crucial that BCM4366 uses more
than 255 flowrings:
brcmfmac: brcmf_pcie_init_ringbuffers Nr of flowrings is 264
I want Hante to review your patch, but indeed this would be 4.5 material
and probably stable.
I just realized BCM4366 support went into 4.4 not 4.5, so Cc-ing
stable for 4.4+ is probably a good idea.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
