2013-02-18 03:01:35

by Larry Finger

[permalink] [raw]
Subject: [PATCH] b43: Increase number of RX DMA slots

Bastian Bittorf reported that some of the silent freezes on a Linksys WRT54G
were due to overflow of the RX DMA ring buffer, which was created with 64
slots. That finding reminded me that I was seeing similar crashed on a netbook,
which also has a relatively slow processor. After increasing the number of
slots to 128, runs on the netbook that previously failed now worked; however,
I found that 109 slots had been used in one test. For that reason, the number
of slots is being increased to 256.

Signed-off-by: Larry Finger <[email protected]>
Cc: Bastian Bittorf <[email protected]>
Cc: Stable <[email protected]>
---
---
drivers/net/wireless/b43/dma.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/b43/dma.h b/drivers/net/wireless/b43/dma.h
index 315b96e..9fdd198 100644
--- a/drivers/net/wireless/b43/dma.h
+++ b/drivers/net/wireless/b43/dma.h
@@ -169,7 +169,7 @@ struct b43_dmadesc_generic {

/* DMA engine tuning knobs */
#define B43_TXRING_SLOTS 256
-#define B43_RXRING_SLOTS 64
+#define B43_RXRING_SLOTS 256
#define B43_DMA0_RX_FW598_BUFSIZE (B43_DMA0_RX_FW598_FO + IEEE80211_MAX_FRAME_LEN)
#define B43_DMA0_RX_FW351_BUFSIZE (B43_DMA0_RX_FW351_FO + IEEE80211_MAX_FRAME_LEN)

--
1.8.1.1



2013-02-20 08:15:23

by Bastian Bittorf

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

* Larry Finger <[email protected]> [20.02.2013 08:32]:
> On 02/20/2013 12:26 AM, G?bor Stefanik wrote:
> >
> >Is this an issue that vendor drivers are also vulnerable to? If it is
> >a firmware issue, I would certainly think so.
>
> I also think so, at least if they are using the firmware version that
> Bastian is using. His logs don't have that info in them, but I
> certainly saw the problem on my BCM4312 using firmware 508.154 from
> 2009.

Another test this morning with heavy downloading (but tcp only)
show slot usage auf max 204/256. we are using firmware

"version 666.2 (2011-02-23 01:15:07)" which is OpenWrt's default
for b43. see here the full logs, including minstrel output and dmesg:

http://intercity-vpn.de/files/openwrt/b43test2.dmesg.txt

if a slot needs ~2500 bytes, so 256 slot are only 640kb which seems
ok to me. ofcourse it raises the memory consumption by 500kb, but now
the router is useful 8-)

thank you! bye, bastian


2013-02-20 06:26:23

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

On Wed, Feb 20, 2013 at 3:42 AM, Larry Finger <[email protected]> wrote:
> On 02/19/2013 06:47 PM, Julian Calaby wrote:
>>
>>
>> Is it be possible to increase the number of slots at runtime? Maybe an
>> even better solution would be to keep the existing number of slots,
>> and if they run out, reset and increase incrementally to some sensible
>> maximum value.
>
>
> The number could be increased a bit, but on systems with 32-bit DMA such as
> the BCM4318, the maximum size of the ring buffer is 4KB. Even more
> importantly, each slot is allocated an skb of 2390 bytes. Even at 256 slots,
> the memory allocation is pretty large.
>
> Larry
>

Is this an issue that vendor drivers are also vulnerable to? If it is
a firmware issue, I would certainly think so.

2013-02-19 18:28:17

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

On 02/19/2013 12:15 PM, David Miller wrote:

> I understand your constraints, but this is a trivially remotely
> DoS'able condition even on slow CPU atom laptops.
>
> Send an "expansive" full sized frame followed by 300 or so 64-byte UDP
> packets --> instant hang.

Thanks for the suggestion for a test. I think the reset solution should survive
with only some packet loss, but we will find out.

Larry



2013-02-20 02:42:08

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

On 02/19/2013 06:47 PM, Julian Calaby wrote:
>
> Is it be possible to increase the number of slots at runtime? Maybe an
> even better solution would be to keep the existing number of slots,
> and if they run out, reset and increase incrementally to some sensible
> maximum value.

The number could be increased a bit, but on systems with 32-bit DMA such as the
BCM4318, the maximum size of the ring buffer is 4KB. Even more importantly, each
slot is allocated an skb of 2390 bytes. Even at 256 slots, the memory allocation
is pretty large.

Larry


2013-02-20 15:49:29

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

On 02/20/2013 02:15 AM, Bastian Bittorf wrote:
> * Larry Finger <[email protected]> [20.02.2013 08:32]:
>> On 02/20/2013 12:26 AM, G?bor Stefanik wrote:
>>>
>>> Is this an issue that vendor drivers are also vulnerable to? If it is
>>> a firmware issue, I would certainly think so.
>>
>> I also think so, at least if they are using the firmware version that
>> Bastian is using. His logs don't have that info in them, but I
>> certainly saw the problem on my BCM4312 using firmware 508.154 from
>> 2009.
>
> Another test this morning with heavy downloading (but tcp only)
> show slot usage auf max 204/256. we are using firmware
>
> "version 666.2 (2011-02-23 01:15:07)" which is OpenWrt's default
> for b43. see here the full logs, including minstrel output and dmesg:
>
> http://intercity-vpn.de/files/openwrt/b43test2.dmesg.txt
>
> if a slot needs ~2500 bytes, so 256 slot are only 640kb which seems
> ok to me. ofcourse it raises the memory consumption by 500kb, but now
> the router is useful 8-)

Thanks for the testing and the report. The skb associated with each slot is
allocated at 2390 bytes, but I think each allocation is a minimum of one page.
In any case, using extra memory is much better than having the device freeze
without explanation. I do not think there is any newer firmware for the 4318
than the version you are using.

I have reworked the patch that resets on overflow, and added the section for
64-bit DMA. I still need to test that part, but I am sending two patches to you
for testing on the WRT54G. The first renames a couple of register names to make
32- and 64-bit naming to only differ in the number. The second is the reset code
patch.

Larry


Attachments:
b43_rename_B43_DMA64_RXSTAT (1.92 kB)
b43_workaround_RX_buffer_overflow (4.57 kB)
Download all attachments

2013-02-18 16:55:26

by Bastian Bittorf

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

* Rafał Miłecki <[email protected]> [18.02.2013 17:54]:
> 2013/2/18 Larry Finger <[email protected]>:
>
> So probably ideal solution is to use 128 *and* fix the driver's
> failing on overflow ;)

i will run a test tomorrow an report, keep calm.

bye, bastian

2013-02-18 20:10:58

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

On 02/18/2013 10:18 AM, Rafał Miłecki wrote:
> 2013/2/18 Larry Finger <[email protected]>:
>> Bastian Bittorf reported that some of the silent freezes on a Linksys WRT54G
>> were due to overflow of the RX DMA ring buffer, which was created with 64
>> slots. That finding reminded me that I was seeing similar crashed on a netbook,
>> which also has a relatively slow processor. After increasing the number of
>> slots to 128, runs on the netbook that previously failed now worked; however,
>> I found that 109 slots had been used in one test. For that reason, the number
>> of slots is being increased to 256.
>
> So probably ideal solution is to use 128 *and* fix the driver's
> failing on overflow ;)
>
> Did you try it on some old device? Just for sure firmware&DMA will
> handle it correctly.

I tested on BCM4318 (which is pretty old), and two different BCM4312 (14e4:4315)
units. I think the firmware and DMA can handle it. After all, all the TX rings
have 256 slots. There is, however, a question of the memory. TX only acquires
the buffers when needed, but RX has to get them in advance, thus 256 slots there
will waste a lot of memory.

I agree that there be two patches, depending on Bastian's testing. The slot size
change can be backported to stable.

Larry


2013-02-19 05:52:08

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

From: Larry Finger <[email protected]>
Date: Sun, 17 Feb 2013 21:01:20 -0600

> Bastian Bittorf reported that some of the silent freezes on a Linksys WRT54G
> were due to overflow of the RX DMA ring buffer, which was created with 64
> slots. That finding reminded me that I was seeing similar crashed on a netbook,
> which also has a relatively slow processor. After increasing the number of
> slots to 128, runs on the netbook that previously failed now worked; however,
> I found that 109 slots had been used in one test. For that reason, the number
> of slots is being increased to 256.
>
> Signed-off-by: Larry Finger <[email protected]>

Applied.

2013-02-19 09:57:55

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

2013/2/19 David Laight <[email protected]>:
>> > Bastian Bittorf reported that some of the silent freezes on a Linksys WRT54G
>> > were due to overflow of the RX DMA ring buffer, which was created with 64
>> > slots. That finding reminded me that I was seeing similar crashed on a netbook,
>> > which also has a relatively slow processor. After increasing the number of
>> > slots to 128, runs on the netbook that previously failed now worked; however,
>> > I found that 109 slots had been used in one test. For that reason, the number
>> > of slots is being increased to 256.
>
> Surely the driver should work even if all the RX buffers get filled?
> Increasing the number of buffers is just hiding the issue.
> A burst of 300 back to back small packets probably fills the 256 slots.
>
> I realise that dropping frames isn't ideal, and that small numbers
> of buffers can make it impossible to receive long fragmented IP
> messages. but increasing the number of buffers doesn't seem to
> be the best fix for a 'silent freeze'.
>
> It might be that the driver would be more robust if it only ever
> put rx buffers into all but one of the ring slots.

That's what I said ;) I have this on my TODO, but I need to resolve my
issues with ethernet first.

--
Rafał

2013-02-18 16:18:37

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

2013/2/18 Larry Finger <[email protected]>:
> Bastian Bittorf reported that some of the silent freezes on a Linksys WRT54G
> were due to overflow of the RX DMA ring buffer, which was created with 64
> slots. That finding reminded me that I was seeing similar crashed on a netbook,
> which also has a relatively slow processor. After increasing the number of
> slots to 128, runs on the netbook that previously failed now worked; however,
> I found that 109 slots had been used in one test. For that reason, the number
> of slots is being increased to 256.

So probably ideal solution is to use 128 *and* fix the driver's
failing on overflow ;)

Did you try it on some old device? Just for sure firmware&DMA will
handle it correctly.

--
Rafał

2013-02-19 18:15:55

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

From: Larry Finger <[email protected]>
Date: Tue, 19 Feb 2013 11:57:19 -0600

> The real problem is that some (perhaps all) versions of the firmware,
> which manages the 'in' pointer of the FIFO ring, appears to fail to
> detect the ring full condition. That is the real cause of the freeze;
> however, we do not have access to the firmware source. We don't even
> have the right to redistribute it, which is why we have the
> b43-fwcutter work around.

I understand your constraints, but this is a trivially remotely
DoS'able condition even on slow CPU atom laptops.

Send an "expansive" full sized frame followed by 300 or so 64-byte UDP
packets --> instant hang.

2013-02-19 20:01:24

by Bastian Bittorf

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

* Larry Finger <[email protected]> [18.02.2013 21:17]:

> (14e4:4315) units. I think the firmware and DMA can handle it. After
> all, all the TX rings have 256 slots. There is, however, a question
> of the memory. TX only acquires the buffers when needed, but RX has
> to get them in advance, thus 256 slots there will waste a lot of
> memory.

I did some testing with 256 slots and the
"b43-workaround-rx-fifo-overflow.patch" on a very slow board,
an i'am sure, that 128 slots are not enough. kernel-log is here:

http://intercity-vpn.de/files/openwrt/b43test.dmesg.txt

this was a normal download (100mb @ 1 megabyte/s), but if you
do some udp-magic (netperf) you run soon into trouble.

thanks for your work!

bye, bastian / wireless.subsignal.org / weimarnetz e.V.

2013-02-18 17:04:04

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

On 02/18/2013 10:55 AM, Bastian Bittorf wrote:
> * Rafał Miłecki <[email protected]> [18.02.2013 17:54]:
>> 2013/2/18 Larry Finger <[email protected]>:
>>
>> So probably ideal solution is to use 128 *and* fix the driver's
>> failing on overflow ;)
>
> i will run a test tomorrow an report, keep calm.

Do you have debugging turned on for b43? If so, the slot usage line at module
unload would be useful.

Larry



2013-02-20 07:16:01

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

On 02/20/2013 12:26 AM, G?bor Stefanik wrote:
>
> Is this an issue that vendor drivers are also vulnerable to? If it is
> a firmware issue, I would certainly think so.

I also think so, at least if they are using the firmware version that Bastian is
using. His logs don't have that info in them, but I certainly saw the problem on
my BCM4312 using firmware 508.154 from 2009.

Larry




2013-02-20 00:48:17

by Julian Calaby

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

Hi Larry,

On Wed, Feb 20, 2013 at 5:28 AM, Larry Finger <[email protected]> wrote:
> On 02/19/2013 12:15 PM, David Miller wrote:
>
>> I understand your constraints, but this is a trivially remotely
>> DoS'able condition even on slow CPU atom laptops.
>>
>> Send an "expansive" full sized frame followed by 300 or so 64-byte UDP
>> packets --> instant hang.
>
>
> Thanks for the suggestion for a test. I think the reset solution should
> survive with only some packet loss, but we will find out.

Is it be possible to increase the number of slots at runtime? Maybe an
even better solution would be to keep the existing number of slots,
and if they run out, reset and increase incrementally to some sensible
maximum value.

Thanks,

--
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

2013-02-19 09:43:21

by David Laight

[permalink] [raw]
Subject: RE: [PATCH] b43: Increase number of RX DMA slots

> > Bastian Bittorf reported that some of the silent freezes on a Linksys WRT54G
> > were due to overflow of the RX DMA ring buffer, which was created with 64
> > slots. That finding reminded me that I was seeing similar crashed on a netbook,
> > which also has a relatively slow processor. After increasing the number of
> > slots to 128, runs on the netbook that previously failed now worked; however,
> > I found that 109 slots had been used in one test. For that reason, the number
> > of slots is being increased to 256.

Surely the driver should work even if all the RX buffers get filled?
Increasing the number of buffers is just hiding the issue.
A burst of 300 back to back small packets probably fills the 256 slots.

I realise that dropping frames isn't ideal, and that small numbers
of buffers can make it impossible to receive long fragmented IP
messages. but increasing the number of buffers doesn't seem to
be the best fix for a 'silent freeze'.

It might be that the driver would be more robust if it only ever
put rx buffers into all but one of the ring slots.

David




2013-02-19 17:57:22

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] b43: Increase number of RX DMA slots

On 02/19/2013 03:42 AM, David Laight wrote:
> Surely the driver should work even if all the RX buffers get filled?
> Increasing the number of buffers is just hiding the issue.
> A burst of 300 back to back small packets probably fills the 256 slots.
>
> I realise that dropping frames isn't ideal, and that small numbers
> of buffers can make it impossible to receive long fragmented IP
> messages. but increasing the number of buffers doesn't seem to
> be the best fix for a 'silent freeze'.
>
> It might be that the driver would be more robust if it only ever
> put rx buffers into all but one of the ring slots.

The real problem is that some (perhaps all) versions of the firmware, which
manages the 'in' pointer of the FIFO ring, appears to fail to detect the ring
full condition. That is the real cause of the freeze; however, we do not have
access to the firmware source. We don't even have the right to redistribute it,
which is why we have the b43-fwcutter work around.

I just reviewed about 8 months of logs on my laptop and discovered that my 2.0
GHz dual CPU system once used 59 of 64 slots. On an netbook with an Atom running
at 1.6 GHz, 109 slots were used. Clearly, the much slower CPU in a Linksys
WRT54G needs more than 64, but testing to determine how many is in progress.

Current thinking is that we will change the number of slots to 128, and add code
to the driver to detect the overflow and reset the device when it occurs. The
increased memory usage should be manageable, most systems will never hit the
condition, and the packet loss will be minimal for those that do.

Larry