On Feb 18, 2009, at 11:15 AM, Michael Buesch wrote:
> On Wednesday 18 February 2009 03:33:01 Francesco Gringoli wrote:
>> On Feb 18, 2009, at 12:23 AM, Michael Buesch wrote:
>>
>>> On Wednesday 18 February 2009 00:07:56 Bo Han wrote:
>>>> I think I am working on the first sanity check in the driver, but
>>>> still
>>>> cannot see any FCS error frames. Is setting B43_MACCTL_KEEP_BAD
>>>> the only
>>>> thing we need to do for the firmware?
>>>
>>> No. I suggest you don't touch that flag anyway and change the
>>> corresponding
>>> mac80211 filter flag. You most likely can do that through cfg80211/
>>> nl80211/iw.
>>> It will take care to set the b43 flag.
>>
>> Michael is right, the new iw interface eases all this stuff.
>>
>> There are however a few points that should be discussed
>>
>> - why b43_rx(...) (in xmit.c) does not mark the status with
>> RX_FLAG_FAILED_FCS_CRC when the firmware reports a B43_RX_MAC_FCSERR:
>> IMHO this should be done to prevent mac80211 to be corrupted and
>> crash
>> the pc when a _very_ noisy frame arrives
>
> Well, I think the mac80211 flag was invented afterwards. Remember
> that b43
> was the first mac80211 driver ever and quite a few things didn't
> exist when
> I ported the stuff.
>
> So do you care to send a patch?
ok, I will send.
>
>
> Anyway, could you elaborate why you think it would _crash_? I think
> it shouldn't
> crash in any case.
No. But I have to analyze more because I think there is a problem with
the firmware too (all firmwares) that not always sets up correctly the
corrupted frame flag when the KEEP_BAD_FRAME bit is activated. I will
check this and what happens when KEEP_BAD_FRAME is set and we have a
mistake even at the plcp sublayer (see below).
However, if RX_FLAG_FAILED_FCS_CRC is not set up in the status field,
mac80211 will process this frame: should not be something related to
the skb, probably somewhere it is shortened without checking its
actual size, as mac80211 trusts the driver that should already had
checked that the skb size correspond to something written in some
header, well, just speculating, I'm confused about.
>> - is that correct to have status.rate_idx filled by functions
>> b43_plcp_get_bitrate_idx_ofdm and b43_plcp_get_bitrate_idx_cck that
>> compute those values reading the plcp?
>
> Yes I think so. This seems to be the best way to do it.
After some thoughts I'm wondering how these fields could be wrong
since the PLCP is protected on its own by another checksum external to
MPDU. Probably the firmware keep also frames whose PLCP is wrong,
while b43_rx only check for FCS problems within the MPDU. Will
investigate.
>
>
>> When a frame is ok, values are
>> correct and mac80211 uses them without problems. If instead the frame
>> is not ok, then mac80211 can warn a lot of message because values are
>> out of range.
>
> If the PLCP header is corrupt you're completely fucked anyway.
> It is a basic and safe assumption that the PLCP header is correct.
> But it shouldn't _crash_ if it's not correct. But I think it doesn't.
Well, I think that capturing noisy frames is interesting for all those
guys willing to use data for research purposes without buying a
network analyzer that basically do the same...
> So if you crank up the flags to pass PLCP corrupted frames it's kind
> of
> expected that they are dropped somewhere in the driver, because we
> cannot
> trust the contents of the frame anyway.
> How do we know the PLCP length field is still correct for a
> corrupted PLCP?
> So we don't even know if the frame would have the correct length.
>
>> Should not we parse these values when FCS is bad and
>> sanitize them if out of range so that dmesg does not get filled with
>> warnings?
>
> These warnings were removed some time ago. Please update your kernel.
Uhm... I updated it before writing this mail to check about this
possibility but I believed these warns were still there
[taken from __ieee80211_rx() pulled from git yesterday night]
if (status->flag & RX_FLAG_HT) {
/* rate_idx is MCS index */
if (WARN_ON(status->rate_idx < 0 ||
status->rate_idx >= 76))
return;
/* HT rates are not in the table - use the highest
legacy rate
* for now since other parts of mac80211 may not yet
be fully
* MCS aware. */
rate = &sband->bitrates[sband->n_bitrates - 1];
} else {
if (WARN_ON(status->rate_idx < 0 ||
status->rate_idx >= sband->n_bitrates))
return;
rate = &sband->bitrates[status->rate_idx];
}
>
>
>> Or probably there is another method to get those values,
>> e.g., the firmware can provide them reading from the radio instead of
>> computing them reading fields from the plcp?
>
> I don't see why this is necessary and how it would be possible.
You are right, we have simply to trash real noise (frames whose plcp
was corrupted) so to avoid to decode the Microwave Owen as you were
saying a few days ago :-)
>
>
>> - I noticed that when in monitor mode and when set up to keep bad
>> frames, the radiotap header is not reporting FCS wrong for malformed
>> or corrupted frame as it should be, is that correct? Should not the
>> radiotap header be built on the status filled by first the driver
>> then
>> mac80211?
>
> I neither know nor care. If you do, please send patches.
Ok.
Cheers,
-FG
>
>
> --
> 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
