On Mar 30, 2009, at 11:35 PM, Francesco Gringoli wrote:

>
> On Mar 28, 2009, at 12:41 AM, Michael Buesch wrote:
>
>> The RX buffer poison needs to be refreshed, if we recycle an RX
>> buffer,
>> because it might be (partially) overwritten by some DMA operations.
>>
>> Cc: sta...@kernel.org
>> Cc: Francesco Gringoli <francesco.gring...@ing.unibs.it>
>> Signed-off-by: Michael Buesch <m...@bu3sch.de>
>>
>> ---
>>
>> Francesco, please stresstest this on top of the other patch that
>> adds poisoning.
> Hi Michael,
>
> great work! No more crashes with the two patches. I will continue
> stress testing anyway but it seems stable.
Hi Michael,

I run the patched kernel for some time and, though it is stable and  
never crashes, there are still (few) some strange rx events. I will  
refer to them as Wrong Frame Type I, Type II, and III. Please note  
however, that we can observe them ONLY when the FCSFAIL bit is set in  
the mac_ctrl_high (at least in my setup).

- Type I: plcp length mismatch
These frames have a plcp that seems to be correct (I mean, the first  
two bytes appear to be taken from a valid plcp), the padding reported  
by the firmware in this cases is correct too (e.g. the padding always  
point to the byte where the valid plcp seems to start). HOWEVER, the  
length of such frames is different than the length encoded in their  
plcp. In all these frames the B43_RX_MAC_FCSERR bit is not set, though  
these frames will fail a crc check. We should check the plcp matches  
the skb length and manually set the RX_FLAG_FAILED_FCS_CRC bit in the  
status field so that mac80211 will skip these frames.

- Type II: fcs is wrong
These frames have a correct plcp that matches their skb length. Their  
FCS however is wrong! but the B43_RX_MAC_FCSERR is not set. Again we  
should manually set the RX_FLAG_FAILED_FCS_CRC bit in the status field  
so that mac80211 will skip these frames. I believe that these frames  
are nothing more than Type I but the broken length collided with their  
actual length.

- Type III: plcp is not a... plcp
These frames have a plcp that is not a plcp, in the sense that the  
first two bytes (with both padding 0 or 2) does not represent any  
possible plcp.

I attach a patch to correctly set the RX_FLAG_FAILED_FCS_CRC bit in  
the status field on such situations so that such frames are not passed  
to the upper layers.

Cheers,
-FG

Index: wireless-testing/drivers/net/wireless/b43/xmit.c
===================================================================
--- drivers/net/wireless/b43/xmit.c     2009-03-26 19:41:53.000000000 +0100
+++ drivers/net/wireless/b43/xmit.c     2009-03-27 20:55:31.000000000 +0100
@@ -27,6 +27,7 @@

  */

+#include <linux/crc32.h>
  #include "xmit.h"
  #include "phy_common.h"
  #include "dma.h"
@@ -560,6 +562,67 @@
                goto drop;
        }
        plcp = (struct b43_plcp_hdr6 *)(skb->data + padding);
+
+       if ((dev->wl->filter_flags & FIF_FCSFAIL) && !(macstat &  
B43_RX_MAC_FCSERR)) {
+               int mismatch = 0;
+               int skb_len = skb->len - 6 - padding;
+               u8* plcp_data = (u8*) plcp;
+               if (phystat0 & B43_RX_PHYST0_OFDM) {
+                       int pkt_len = plcp_data[0] |
+                               plcp_data[1] <<  8 |
+                               plcp_data[2] << 16 |
+                               plcp_data[3] << 24;
+                       pkt_len >>= 5;
+                       pkt_len &= 0xFFF;
+                       if(pkt_len != skb_len) {
+                               mismatch = 1;
+                       }
+               }
+               else {
+                       int speed;
+                       int len1 = (plcp_data[3] << 8) | plcp_data[2];
+                       int len2;
+                       switch(plcp_data[0]) {
+                       case 0x0A:
+                               speed = 2;
+                               break;
+                       case 0x14:
+                               speed = 4;
+                               break;
+                       case 0x37:
+                               speed = 11;
+                               break;
+                       case 0x6E:
+                               speed = 22;
+                               break;
+                       default:
+                               speed = 1;
+                               break;
+                       }
+                       len2 = skb_len * 16 / speed;
+                       if((skb_len * 16 % speed) > 0)
+                               len2++;
+
+                       if(len1 != len2) {
+                               mismatch = 1;
+                       }
+               }
+               if(mismatch) {
+                       dev->wl->ieee_stats.dot11FCSErrorCount++;
+                       macstat |= B43_RX_MAC_FCSERR;
+                       status.flag |= RX_FLAG_FAILED_FCS_CRC;
+                       plcp_data = (u8*) (struct b43_plcp_hdr6 *)(skb->data);
+                       b43dbg(dev->wl, "RX: padding or plcp mismatch, setting 
FCSFAIL and  
keeping frame (" \
+                               "%02X %02X %02X %02X padding = %d, len = %d)\n",
+                               plcp_data[0],
+                               plcp_data[1],
+                               plcp_data[2],
+                               plcp_data[3],
+                               padding,
+                               skb->len);
+               }
+        }
+
        skb_pull(skb, sizeof(struct b43_plcp_hdr6) + padding);
        /* The skb contains the Wireless Header + payload data now */
        if (unlikely(skb->len < (2 + 2 + 6 /*minimum hdr */  + FCS_LEN))) {
@@ -569,6 +648,23 @@
        wlhdr = (struct ieee80211_hdr *)(skb->data);
        fctl = wlhdr->frame_control;

+       /* if we keep bad frames we should compute again the FCS as fw can  
skip marking */
+        if ((dev->wl->filter_flags & FIF_FCSFAIL) && !(macstat &  
B43_RX_MAC_FCSERR)) {
+               u32 fcs1, fcs2;
+               u8* fcs_ptr = skb->data + skb->len - FCS_LEN;
+               fcs1 = fcs_ptr[0] |
+                       fcs_ptr[1] << 8  |
+                       fcs_ptr[2] << 16 |
+                       fcs_ptr[3] << 24;
+               fcs2 = ~ether_crc_le(skb->len - FCS_LEN, skb->data);
+               if(fcs1 != fcs2) {
+                       b43dbg(dev->wl, "RX: FCS wrong not reported, setting 
FCSFAIL and  
keeping\n");
+                       macstat |= B43_RX_MAC_FCSERR;
+                       dev->wl->ieee_stats.dot11FCSErrorCount++;
+                       status.flag |= RX_FLAG_FAILED_FCS_CRC;
+               }
+       }
+
        if (macstat & B43_RX_MAC_DEC) {
                unsigned int keyidx;
                int wlhdr_len;
@@ -617,6 +721,10 @@
                 * Drop the frame, if we are not interested in corrupted 
frames. */
                if (!(dev->wl->filter_flags & FIF_PLCPFAIL))
                        goto drop;
+               else {
+                       b43dbg(dev->wl, "RX: PLCP corrupted, discarding\n");
+                       goto drop;
+               }
        }
        status.antenna = !!(phystat0 & B43_RX_PHYST0_ANT);


_______________________________________________
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev

Reply via email to