All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] b43: Increase number of RX DMA slots
@ 2013-02-18  3:01 ` Larry Finger
  0 siblings, 0 replies; 25+ messages in thread
From: Larry Finger @ 2013-02-18  3:01 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry Finger, netdev, Bastian Bittorf, Stable

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 <Larry.Finger@lwfinger.net>
Cc: Bastian Bittorf <bittorf@bluebottle.com>
Cc: Stable <stable@vger.kernel.org>
---
---
 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


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH] b43: Increase number of RX DMA slots
@ 2013-02-18  3:01 ` Larry Finger
  0 siblings, 0 replies; 25+ messages in thread
From: Larry Finger @ 2013-02-18  3:01 UTC (permalink / raw)
  To: linville-2XuSBdqkA4R54TAoqtyWWQ
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA, Larry Finger,
	netdev-u79uwXL29TY76Z2rM5mHXA, Bastian Bittorf, Stable

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 <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Cc: Bastian Bittorf <bittorf-yxHw+vPQXZDZJqsBc5GL+g@public.gmane.org>
Cc: Stable <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
@ 2013-02-18 16:18   ` Rafał Miłecki
  0 siblings, 0 replies; 25+ messages in thread
From: Rafał Miłecki @ 2013-02-18 16:18 UTC (permalink / raw)
  To: Larry Finger; +Cc: linville, linux-wireless, netdev, Bastian Bittorf, Stable

2013/2/18 Larry Finger <Larry.Finger@lwfinger.net>:
> 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ł

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
@ 2013-02-18 16:18   ` Rafał Miłecki
  0 siblings, 0 replies; 25+ messages in thread
From: Rafał Miłecki @ 2013-02-18 16:18 UTC (permalink / raw)
  To: Larry Finger
  Cc: linville-2XuSBdqkA4R54TAoqtyWWQ,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Bastian Bittorf, Stable

2013/2/18 Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>:
> 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ł
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
@ 2013-02-18 16:55     ` Bastian Bittorf
  0 siblings, 0 replies; 25+ messages in thread
From: Bastian Bittorf @ 2013-02-18 16:55 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Larry Finger, linville, linux-wireless, netdev, Bastian Bittorf, Stable

* Rafał Miłecki <zajec5@gmail.com> [18.02.2013 17:54]:
> 2013/2/18 Larry Finger <Larry.Finger@lwfinger.net>:
> 
> 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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
@ 2013-02-18 16:55     ` Bastian Bittorf
  0 siblings, 0 replies; 25+ messages in thread
From: Bastian Bittorf @ 2013-02-18 16:55 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Larry Finger, linville-2XuSBdqkA4R54TAoqtyWWQ,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Bastian Bittorf, Stable

* Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [18.02.2013 17:54]:
> 2013/2/18 Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>:
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
  2013-02-18 16:55     ` Bastian Bittorf
  (?)
@ 2013-02-18 17:04     ` Larry Finger
  -1 siblings, 0 replies; 25+ messages in thread
From: Larry Finger @ 2013-02-18 17:04 UTC (permalink / raw)
  To: Bastian Bittorf
  Cc: Rafał Miłecki, linville, linux-wireless, netdev, Stable

On 02/18/2013 10:55 AM, Bastian Bittorf wrote:
> * Rafał Miłecki <zajec5@gmail.com> [18.02.2013 17:54]:
>> 2013/2/18 Larry Finger <Larry.Finger@lwfinger.net>:
>>
>> 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



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
  2013-02-18 16:18   ` Rafał Miłecki
  (?)
  (?)
@ 2013-02-18 20:10   ` Larry Finger
  2013-02-19 20:01       ` Bastian Bittorf
  -1 siblings, 1 reply; 25+ messages in thread
From: Larry Finger @ 2013-02-18 20:10 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linville, linux-wireless, netdev, Bastian Bittorf, Stable

On 02/18/2013 10:18 AM, Rafał Miłecki wrote:
> 2013/2/18 Larry Finger <Larry.Finger@lwfinger.net>:
>> 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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
@ 2013-02-19  5:52   ` David Miller
  0 siblings, 0 replies; 25+ messages in thread
From: David Miller @ 2013-02-19  5:52 UTC (permalink / raw)
  To: Larry.Finger; +Cc: linville, linux-wireless, netdev, bittorf, stable

From: Larry Finger <Larry.Finger@lwfinger.net>
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 <Larry.Finger@lwfinger.net>

Applied.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
@ 2013-02-19  5:52   ` David Miller
  0 siblings, 0 replies; 25+ messages in thread
From: David Miller @ 2013-02-19  5:52 UTC (permalink / raw)
  To: Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ
  Cc: linville-2XuSBdqkA4R54TAoqtyWWQ,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, bittorf-yxHw+vPQXZDZJqsBc5GL+g,
	stable-u79uwXL29TY76Z2rM5mHXA

From: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
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 <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] b43: Increase number of RX DMA slots
  2013-02-19  5:52   ` David Miller
  (?)
@ 2013-02-19  9:42   ` David Laight
  2013-02-19  9:57     ` Rafał Miłecki
  2013-02-19 17:57     ` Larry Finger
  -1 siblings, 2 replies; 25+ messages in thread
From: David Laight @ 2013-02-19  9:42 UTC (permalink / raw)
  To: David Miller, Larry.Finger
  Cc: linville, linux-wireless, netdev, bittorf, stable

> > 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




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
  2013-02-19  9:42   ` David Laight
@ 2013-02-19  9:57     ` Rafał Miłecki
  2013-02-19 17:57     ` Larry Finger
  1 sibling, 0 replies; 25+ messages in thread
From: Rafał Miłecki @ 2013-02-19  9:57 UTC (permalink / raw)
  To: David Laight
  Cc: David Miller, Larry.Finger, linville, linux-wireless, netdev,
	bittorf, stable

2013/2/19 David Laight <David.Laight@aculab.com>:
>> > 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ł

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
  2013-02-19  9:42   ` David Laight
  2013-02-19  9:57     ` Rafał Miłecki
@ 2013-02-19 17:57     ` Larry Finger
  2013-02-19 18:15       ` David Miller
  1 sibling, 1 reply; 25+ messages in thread
From: Larry Finger @ 2013-02-19 17:57 UTC (permalink / raw)
  To: David Laight
  Cc: David Miller, linville, linux-wireless, netdev, bittorf, stable

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



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
  2013-02-19 17:57     ` Larry Finger
@ 2013-02-19 18:15       ` David Miller
  2013-02-19 18:28         ` Larry Finger
  0 siblings, 1 reply; 25+ messages in thread
From: David Miller @ 2013-02-19 18:15 UTC (permalink / raw)
  To: Larry.Finger
  Cc: David.Laight, linville, linux-wireless, netdev, bittorf, stable

From: Larry Finger <Larry.Finger@lwfinger.net>
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.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
  2013-02-19 18:15       ` David Miller
@ 2013-02-19 18:28         ` Larry Finger
  2013-02-20  0:47           ` Julian Calaby
  0 siblings, 1 reply; 25+ messages in thread
From: Larry Finger @ 2013-02-19 18:28 UTC (permalink / raw)
  To: David Miller
  Cc: David.Laight, linville, linux-wireless, netdev, bittorf, stable

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



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
  2013-02-18 20:10   ` Larry Finger
@ 2013-02-19 20:01       ` Bastian Bittorf
  0 siblings, 0 replies; 25+ messages in thread
From: Bastian Bittorf @ 2013-02-19 20:01 UTC (permalink / raw)
  To: Larry Finger, openwrt-devel
  Cc: Rafał Miłecki, linville, linux-wireless, netdev, Stable

* Larry Finger <Larry.Finger@lwfinger.net> [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.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
@ 2013-02-19 20:01       ` Bastian Bittorf
  0 siblings, 0 replies; 25+ messages in thread
From: Bastian Bittorf @ 2013-02-19 20:01 UTC (permalink / raw)
  To: Larry Finger, openwrt-devel; +Cc: netdev, linux-wireless, linville, Stable

* Larry Finger <Larry.Finger@lwfinger.net> [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.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
  2013-02-19 18:28         ` Larry Finger
@ 2013-02-20  0:47           ` Julian Calaby
  2013-02-20  2:42             ` Larry Finger
  0 siblings, 1 reply; 25+ messages in thread
From: Julian Calaby @ 2013-02-20  0:47 UTC (permalink / raw)
  To: Larry Finger
  Cc: David Miller, David.Laight, linville, linux-wireless, netdev,
	bittorf, stable

Hi Larry,

On Wed, Feb 20, 2013 at 5:28 AM, Larry Finger <Larry.Finger@lwfinger.net> 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: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
  2013-02-20  0:47           ` Julian Calaby
@ 2013-02-20  2:42             ` Larry Finger
  2013-02-20  6:26                 ` Gábor Stefanik
  0 siblings, 1 reply; 25+ messages in thread
From: Larry Finger @ 2013-02-20  2:42 UTC (permalink / raw)
  To: Julian Calaby
  Cc: David Miller, David.Laight, linville, linux-wireless, netdev,
	bittorf, stable

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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
@ 2013-02-20  6:26                 ` Gábor Stefanik
  0 siblings, 0 replies; 25+ messages in thread
From: Gábor Stefanik @ 2013-02-20  6:26 UTC (permalink / raw)
  To: Larry Finger
  Cc: Julian Calaby, David Miller, David.Laight, linville,
	linux-wireless, netdev, bittorf, stable

On Wed, Feb 20, 2013 at 3:42 AM, Larry Finger <Larry.Finger@lwfinger.net> 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.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
@ 2013-02-20  6:26                 ` Gábor Stefanik
  0 siblings, 0 replies; 25+ messages in thread
From: Gábor Stefanik @ 2013-02-20  6:26 UTC (permalink / raw)
  To: Larry Finger
  Cc: Julian Calaby, David Miller, David.Laight-JxhZ9S5GRejQT0dZR+AlfA,
	linville-2XuSBdqkA4R54TAoqtyWWQ,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, bittorf-yxHw+vPQXZDZJqsBc5GL+g,
	stable-u79uwXL29TY76Z2rM5mHXA

On Wed, Feb 20, 2013 at 3:42 AM, Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
  2013-02-20  6:26                 ` Gábor Stefanik
  (?)
@ 2013-02-20  7:15                 ` Larry Finger
  2013-02-20  8:15                     ` Bastian Bittorf
  -1 siblings, 1 reply; 25+ messages in thread
From: Larry Finger @ 2013-02-20  7:15 UTC (permalink / raw)
  To: Gábor Stefanik
  Cc: Julian Calaby, David Miller, David.Laight, linville,
	linux-wireless, netdev, bittorf, stable

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




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
@ 2013-02-20  8:15                     ` Bastian Bittorf
  0 siblings, 0 replies; 25+ messages in thread
From: Bastian Bittorf @ 2013-02-20  8:15 UTC (permalink / raw)
  To: Larry Finger
  Cc: Gábor Stefanik, Julian Calaby, David Miller, David.Laight,
	linville, linux-wireless, netdev, bittorf, stable, openwrt-devel

* Larry Finger <Larry.Finger@lwfinger.net> [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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
@ 2013-02-20  8:15                     ` Bastian Bittorf
  0 siblings, 0 replies; 25+ messages in thread
From: Bastian Bittorf @ 2013-02-20  8:15 UTC (permalink / raw)
  To: Larry Finger
  Cc: Gábor Stefanik, Julian Calaby, David Miller,
	David.Laight-JxhZ9S5GRejQT0dZR+AlfA,
	linville-2XuSBdqkA4R54TAoqtyWWQ,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, bittorf-yxHw+vPQXZDZJqsBc5GL+g,
	stable-u79uwXL29TY76Z2rM5mHXA,
	openwrt-devel-ZwoEplunGu0+xA9t4nZiwti2O/JbrIOy

* Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org> [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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] b43: Increase number of RX DMA slots
  2013-02-20  8:15                     ` Bastian Bittorf
  (?)
@ 2013-02-20 15:49                     ` Larry Finger
  -1 siblings, 0 replies; 25+ messages in thread
From: Larry Finger @ 2013-02-20 15:49 UTC (permalink / raw)
  To: Gábor Stefanik, Julian Calaby, David Miller, David.Laight,
	linville, linux-wireless, netdev, stable, openwrt-devel

[-- Attachment #1: Type: text/plain, Size: 1693 bytes --]

On 02/20/2013 02:15 AM, Bastian Bittorf wrote:
> * Larry Finger <Larry.Finger@lwfinger.net> [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


[-- Attachment #2: b43_rename_B43_DMA64_RXSTAT --]
[-- Type: text/plain, Size: 1968 bytes --]

Index: wireless-testing-new/drivers/net/wireless/b43/dma.c
===================================================================
--- wireless-testing-new.orig/drivers/net/wireless/b43/dma.c
+++ wireless-testing-new/drivers/net/wireless/b43/dma.c
@@ -476,7 +476,7 @@ static int b43_dmacontroller_rx_reset(st
 				break;
 			}
 		} else {
-			value &= B43_DMA32_RXSTATE;
+			value &= B43_DMA32_RXSTAT;
 			if (value == B43_DMA32_RXSTAT_DISABLED) {
 				i = -1;
 				break;
@@ -513,7 +513,7 @@ static int b43_dmacontroller_tx_reset(st
 			    value == B43_DMA64_TXSTAT_STOPPED)
 				break;
 		} else {
-			value &= B43_DMA32_TXSTATE;
+			value &= B43_DMA32_TXSTAT;
 			if (value == B43_DMA32_TXSTAT_DISABLED ||
 			    value == B43_DMA32_TXSTAT_IDLEWAIT ||
 			    value == B43_DMA32_TXSTAT_STOPPED)
@@ -534,7 +534,7 @@ static int b43_dmacontroller_tx_reset(st
 				break;
 			}
 		} else {
-			value &= B43_DMA32_TXSTATE;
+			value &= B43_DMA32_TXSTAT;
 			if (value == B43_DMA32_TXSTAT_DISABLED) {
 				i = -1;
 				break;
Index: wireless-testing-new/drivers/net/wireless/b43/dma.h
===================================================================
--- wireless-testing-new.orig/drivers/net/wireless/b43/dma.h
+++ wireless-testing-new/drivers/net/wireless/b43/dma.h
@@ -27,7 +27,7 @@
 #define B43_DMA32_TXINDEX				0x08
 #define B43_DMA32_TXSTATUS				0x0C
 #define		B43_DMA32_TXDPTR			0x00000FFF
-#define		B43_DMA32_TXSTATE			0x0000F000
+#define		B43_DMA32_TXSTAT			0x0000F000
 #define			B43_DMA32_TXSTAT_DISABLED	0x00000000
 #define			B43_DMA32_TXSTAT_ACTIVE	0x00001000
 #define			B43_DMA32_TXSTAT_IDLEWAIT	0x00002000
@@ -52,7 +52,7 @@
 #define B43_DMA32_RXINDEX				0x18
 #define B43_DMA32_RXSTATUS				0x1C
 #define		B43_DMA32_RXDPTR			0x00000FFF
-#define		B43_DMA32_RXSTATE			0x0000F000
+#define		B43_DMA32_RXSTAT			0x0000F000
 #define			B43_DMA32_RXSTAT_DISABLED	0x00000000
 #define			B43_DMA32_RXSTAT_ACTIVE	0x00001000
 #define			B43_DMA32_RXSTAT_IDLEWAIT	0x00002000

[-- Attachment #3: b43_workaround_RX_buffer_overflow --]
[-- Type: text/plain, Size: 4684 bytes --]

    Index: drivers/net/wireless/b43/dma.c
    ===================================================================
    --- a/drivers/net/wireless/b43/dma.c
    +++ b/drivers/net/wireless/b43/dma.c
    @@ -1689,6 +1692,31 @@
             sync_descbuffer_for_device(ring, dmaaddr, ring->rx_buffersize);
     }
     
    +static int dma_rx_check_overflow(struct b43_dmaring *ring)
    +{
    +        u32 state;
    +        u32 rxctl;
    +
    +        if (ring->type != B43_DMA_32BIT)
    +                return 0;
    +
    +        state = b43_dma_read(ring, B43_DMA32_RXSTATUS) & B43_DMA32_RXSTATE;
    +        if (state != B43_DMA32_RXSTAT_IDLEWAIT)
    +                return 0;
    +
    +        rxctl = b43_dma_read(ring, B43_DMA32_RXCTL);
    +        b43_dmacontroller_rx_reset(ring->dev, ring->mmio_base, ring->type);
    +
    +        b43_dma_write(ring, B43_DMA32_RXCTL, rxctl);
    +        b43_dma_write(ring, B43_DMA32_RXINDEX, ring->nr_slots *
    +                      sizeof(struct b43_dmadesc32));
    +        ring->current_slot = 0;
    +
    +        printk("DMA RX reset due to overflow\n");
    +
    +        return 1;
    +}
    +
     void b43_dma_rx(struct b43_dmaring *ring)
     {
             const struct b43_dma_ops *ops = ring->ops;
    @@ -1700,6 +1728,18 @@
             B43_WARN_ON(!(current_slot >= 0 && current_slot < ring->nr_slots));
     
             slot = ring->current_slot;
    +
    +        /* XXX: BRCM4318(?) dirty workaround:
    +         *      it seems sometimes the RX ring overflows due to interrupt latencies; 
    +         *      i.e. skb allocations are slow on routers with high CPU load
    +         *      and tight memory constraints */
    +        if (slot == current_slot) {
    +                /* Try to reset the RX channel, will cost us few lost frames,
    +                 * but will recover from an eternal stall */
    +                if (dma_rx_check_overflow(ring))
    +                        return;         
    +        }
    +        
             for (; slot != current_slot; slot = next_slot(ring, slot)) {
                     dma_rx(ring, &slot);
                     update_max_used_slots(ring, ++used_slots);


Index: wireless-testing/drivers/net/wireless/b43/dma.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/dma.c
+++ wireless-testing/drivers/net/wireless/b43/dma.c
@@ -1692,6 +1692,50 @@ drop_recycle_buffer:
 	sync_descbuffer_for_device(ring, dmaaddr, ring->rx_buffersize);
 }
 
+/* check for overflow of the RX descriptor ring. If found, reset the DMA
+ * controller and return true.
+ */
+static bool dma_rx_check_overflow(struct b43_dmaring *ring)
+{
+	if (ring->type == B43_DMA_64BIT) {
+		u64 state;
+		u64 rxctl;
+
+		state = b43_dma_read(ring, B43_DMA64_RXSTATUS) &
+			B43_DMA64_RXSTAT;
+		if (state != B43_DMA64_RXSTAT_IDLEWAIT)
+			return false;
+		rxctl = b43_dma_read(ring, B43_DMA64_RXCTL);
+		b43_dmacontroller_rx_reset(ring->dev, ring->mmio_base,
+					   ring->type);
+
+		b43_dma_write(ring, B43_DMA64_RXCTL, rxctl);
+		b43_dma_write(ring, B43_DMA64_RXINDEX, ring->nr_slots *
+			      sizeof(struct b43_dmadesc64));
+	} else {
+		u32 state;
+		u32 rxctl;
+
+		state = b43_dma_read(ring, B43_DMA32_RXSTATUS) &
+				     B43_DMA32_RXSTAT;
+		if (state != B43_DMA32_RXSTAT_IDLEWAIT)
+			return false;
+
+		rxctl = b43_dma_read(ring, B43_DMA32_RXCTL);
+		b43_dmacontroller_rx_reset(ring->dev, ring->mmio_base,
+					   ring->type);
+
+		b43_dma_write(ring, B43_DMA32_RXCTL, rxctl);
+		b43_dma_write(ring, B43_DMA32_RXINDEX, ring->nr_slots *
+			      sizeof(struct b43_dmadesc32));
+	}
+	ring->current_slot = 0;
+
+	b43err(ring->dev->wl, "DMA RX reset due to overflow\n");
+
+	return true;
+}
+
 void b43_dma_rx(struct b43_dmaring *ring)
 {
 	const struct b43_dma_ops *ops = ring->ops;
@@ -1703,7 +1747,21 @@ void b43_dma_rx(struct b43_dmaring *ring
 	B43_WARN_ON(!(current_slot >= 0 && current_slot < ring->nr_slots));
 
 	slot = ring->current_slot;
-	for (; slot != current_slot; slot = next_slot(ring, slot)) {
+
+	/* XXX: BRCM4318(?) dirty workaround:
+	 *	it seems sometimes the RX ring overflows due to interrupt
+	 *	latencies; particularly for systems with slow CPUs and tight
+	 *	memory constraints
+	 */
+	if (slot == current_slot) {
+		/* Try to reset the RX channel, will cost us few lost frames,
+		 * but will recover from an eternal stall
+		 */
+		if (dma_rx_check_overflow(ring))
+			return; /* exit on overflow and reset */
+	}
+
+        for (; slot != current_slot; slot = next_slot(ring, slot)) {
 		dma_rx(ring, &slot);
 		update_max_used_slots(ring, ++used_slots);
 	}

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2013-02-20 15:49 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-18  3:01 [PATCH] b43: Increase number of RX DMA slots Larry Finger
2013-02-18  3:01 ` Larry Finger
2013-02-18 16:18 ` Rafał Miłecki
2013-02-18 16:18   ` Rafał Miłecki
2013-02-18 16:55   ` Bastian Bittorf
2013-02-18 16:55     ` Bastian Bittorf
2013-02-18 17:04     ` Larry Finger
2013-02-18 20:10   ` Larry Finger
2013-02-19 20:01     ` Bastian Bittorf
2013-02-19 20:01       ` Bastian Bittorf
2013-02-19  5:52 ` David Miller
2013-02-19  5:52   ` David Miller
2013-02-19  9:42   ` David Laight
2013-02-19  9:57     ` Rafał Miłecki
2013-02-19 17:57     ` Larry Finger
2013-02-19 18:15       ` David Miller
2013-02-19 18:28         ` Larry Finger
2013-02-20  0:47           ` Julian Calaby
2013-02-20  2:42             ` Larry Finger
2013-02-20  6:26               ` Gábor Stefanik
2013-02-20  6:26                 ` Gábor Stefanik
2013-02-20  7:15                 ` Larry Finger
2013-02-20  8:15                   ` Bastian Bittorf
2013-02-20  8:15                     ` Bastian Bittorf
2013-02-20 15:49                     ` Larry Finger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.