On Apr 5, 2009, at 8:58 PM, Michael Buesch wrote:
> On Sunday 05 April 2009 20:01:22 Francesco Gringoli wrote:
>> On Mar 30, 2009, at 11:35 PM, Francesco Gringoli wrote:
>>
>>>
>>> On Mar 28, 2009, at 12:41 AM, Michael Buesch wrote:
>>>
>>>>
Hi Michael,
>
> I think this really is completely getting overengineered by now.
>
> I'm not going to apply such a patch unless you tell me why it's
> needed.
> Having such an incredible mechanism for an absolute corner case that
> happens
> once in a billion frames but doesn't harm anybody is not really
> acceptable.
No problem :-) I simply sent the patch I'm using in my test
environment where I get this behavior for the 0.1% of the received
frames when FCSFAIL is set. Note that here we collect traces for
experiments with 802.11 protocol, so we need this kind of patches.
I understand that very few of us are doing such kind of experiments
and users are not, I simply sent a comment about these devices. It may
improve knowledge about them.
Cheers,
-FG
>
>
> If you have FCSFAIL set, you're expected to receive crap. It's as
> simple as that.
> If you don't want crap, don't set the bit.
>
>
>> 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);
>>
>>
>>
>>
>
>
>
> --
> Greetings, Michael.
-------
Francesco Gringoli, PhD - Assistant Professor
Dept. of Electrical Engineering for Automation
University of Brescia
via Branze, 38
25123 Brescia
ITALY
Ph: ++39.030.3715843
FAX: ++39.030.380014
WWW: http://www.ing.unibs.it/~gringoli
_______________________________________________
Bcm43xx-dev mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
