Re: [rt2x00-users] [PATCH v2] rt2x00: Further L2 padding fixes.

[Benoit PAPILLAULT <benoit.papillault@free.fr>]

Gertjan van Wingerde a écrit :
>> I'll sent a patch for unpadding on the RX path that I have tested with
>> rt2870 usb (I'm still working on the TX path). My feeling is that we are
>>  not doing the correct computation.
> 
> The current code indeed isn't correct, as it doesn't work for header lengths that don't
> need L2 padding. However see above on the problem with your suggested formula.

Indeed. 802.11 header length is always even (10, 16, 24, 26, 30, 32 ..).
  We can keep the exact formula anyway since it's correct.

> 
>> l2pad is the number of padding bytes added by the HW. It is computed
>> here based on the header_length. However, doing so, we missed two points  :
>>
>> 1. ACK frame is 10 bytes. So header_length will be 10 and then l2pad is
>> 2. However, ACK frames do not need padding, only DATA frames needs
>> padding. The above formula is then wrong.
> 
> I gathered as much as well, after a bit of rethinking. However, this is true
> because the frame doesn't have a payload at all. That is the part we have to
> take into account, not the fact that it is an ACK frame.

Correct. I just use ACK frame as an example.

> 
>> 2. If we receive a QoS DATA frame whose header_length is 26 according to
>> frame_control, but skb->len = 20, then ieee80211_get_hdrlen_from_skb()
>> will returns hdrlen = 0 based on the value of skb->len. However, this
>> function does not know if skb->len includes said padding or not, or some
>> other padding! (On RX, rt2870 usb frames are also padded at the end!).
> 
> But, in that case we have received an invalid frame anyway, so the padding 
> doesn't really matter.

I fully disagree here. It's a bit of chicken-egg problem. I'm using
monitor mode to debug other wireless drivers, so I need a tool that
gives me the frame as it appears on the radio medium, be it invalid or
not. And I do see lots of invalid 802.11 frames in real life that are
generated by bogus drivers or intended to be bogus in order to crash
wireless drivers.

> 
>> For those reason, I think it's better to have a common function that
>> computes the padding position (based on frame_control). Once we know the
>> padding position, we can compute padding size (= padding position & 3,
>> like above) and we can also compare with the real frame size (contained
>> in RXWI_W0_MPDU_TOTAL_BYTE_COUNT for instance).
>>
> 
> See above, I don't think we need to have this detailed knowledge of the frame
> formats in the driver.

Futher testing will tell us. So far, I don't need it either.

Regards,
Benoit