| Message ID | 1361156480-32566-1-git-send-email-Larry.Finger@lwfinger.net |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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ł 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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > 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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* 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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* 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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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)
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(-)