When running a monitor interface with ath5k, the FCS of ACK
frames is two bytes short. The reason is that unlike the
payload, the FCS doesn't seem to aligned to a 4 byte boundary
by the chip, the attempt to remove the padding corrupts it.
This patch checks whether there actually is any payload after
the header (besides the FCS) before removing the padding.
This might not be fully correct or not handle other cases where
this can occur, but it doesn't seem too hackish and fixes the
problem for me :)
http://bugzilla.kernel.org/show_bug.cgi?id=12101
On Wed, Dec 10, 2008 at 8:43 AM, Sujith <[email protected]> wrote:
>> 957 /* remove FCS before passing up to protocol stack */
>> 958 skb_trim(skb, (skb->len - FCS_LEN));
>>
>
> I remember removing this piece of code...
Hmm, you're right. I must've been looking at 2.6.27. Sorry for the noise.
The padding still gets removed regardless of whether there is a payload;
does that truncate the FCS for ACK/CTS frames on ath9k?
>> 952 if (hdrlen & 3) {
>> 953 padsize = hdrlen % 4;
While we're at it, (for ath5k also) -- hdrlen % 4 can just be hdrlen & 3
again.
--
Bob Copeland %% http://www.bobcopeland.com
On Wed, 2008-12-10 at 08:24 -0500, Bob Copeland wrote:
> It seems very plausible to me. Though, why doesn't ath9k also have this
> problem? Luis, it looks like in that case ath9k could trim two extra
> bytes if ath9k hw behaves the same.
>
> main.c:
>
> 951 /* see if any padding is done by the hw and remove it */
> 952 if (hdrlen & 3) {
> 953 padsize = hdrlen % 4;
> 954 memmove(skb->data + padsize, skb->data, hdrlen);
> 955 skb_pull(skb, padsize);
> 956 }
>
> 957 /* remove FCS before passing up to protocol stack */
> 958 skb_trim(skb, (skb->len - FCS_LEN));
It also shouldn't remove the FCS, sometimes you want to see that on
wireshark and mac80211 handles removing the FCS itself just fine, just
set the HW flag.
johannes
On Wed, Dec 10, 2008 at 8:24 AM, Bob Copeland <[email protected]> wrote:
> On Wed, Dec 10, 2008 at 06:04:05AM +0100, Patrick McHardy wrote:
>> This might not be fully correct or not handle other cases where
>> this can occur, but it doesn't seem too hackish and fixes the
>> problem for me :)
>
>> - if (hdrlen & 3) {
>> + if (hdrlen & 3 && hdrlen != rs.rs_datalen - FCS_LEN) {
>> pad = hdrlen % 4;
>> memmove(skb->data + pad, skb->data, hdrlen);
>> skb_pull(skb, pad);
>
> It seems very plausible to me. Though, why doesn't ath9k also have this
> problem?
So, I guess ath9k did too. This patch gets my ack, though we should probably
do the hdrlen check the same way as Jouni's patch for consistency, and switch
to hdrlen & 3 when computing pad.
--
Bob Copeland %% http://www.bobcopeland.com
On Wed, Dec 10, 2008 at 06:04:05AM +0100, Patrick McHardy wrote:
> This might not be fully correct or not handle other cases where
> this can occur, but it doesn't seem too hackish and fixes the
> problem for me :)
> - if (hdrlen & 3) {
> + if (hdrlen & 3 && hdrlen != rs.rs_datalen - FCS_LEN) {
> pad = hdrlen % 4;
> memmove(skb->data + pad, skb->data, hdrlen);
> skb_pull(skb, pad);
It seems very plausible to me. Though, why doesn't ath9k also have this
problem? Luis, it looks like in that case ath9k could trim two extra
bytes if ath9k hw behaves the same.
main.c:
951 /* see if any padding is done by the hw and remove it */
952 if (hdrlen & 3) {
953 padsize = hdrlen % 4;
954 memmove(skb->data + padsize, skb->data, hdrlen);
955 skb_pull(skb, padsize);
956 }
957 /* remove FCS before passing up to protocol stack */
958 skb_trim(skb, (skb->len - FCS_LEN));
--
Bob Copeland %% http://www.bobcopeland.com
Bob Copeland wrote:
> 951 /* see if any padding is done by the hw and remove it */
> 952 if (hdrlen & 3) {
> 953 padsize = hdrlen % 4;
> 954 memmove(skb->data + padsize, skb->data, hdrlen);
> 955 skb_pull(skb, padsize);
> 956 }
>
> 957 /* remove FCS before passing up to protocol stack */
> 958 skb_trim(skb, (skb->len - FCS_LEN));
>
I remember removing this piece of code...
Sujith