On Wed, Dec 2, 2009 at 10:54 AM, Benoit PAPILLAULT <benoit.papillault@xxxxxxx> wrote: > Gertjan van Wingerde a écrit : >> >> On 12/01/09 00:46, Benoit PAPILLAULT wrote: >> >>> >>> Gertjan van Wingerde a écrit : >>> >>>> >>>> The L2 padding fixes patch has grown a bit and now consists of 4 >>>> separate >>>> patches to clean the L2 padding code up and to fix a number of bugs at >>>> the >>>> same time. >>>> >>>> 1. rt2x00: Further L2 padding fixes. >>>> 2. rt2x00: Remove SKBDESC_L2_PADDED flag. >>>> 3. rt2x00: Reorganize L2 padding inserting function. >>>> 4. rt2x00: Only remove L2 padding in received frames if there is >>>> payload. >>>> >>>> --- >>>> Gertjan. >>>> >>> >>> Thanks Gertjan for the patches, I know that padding is a bit of >>> nightware. I've testing your tree and here are the results: >>> >>> 1. On TX : It fails for control frames with hdrlen=10 since it will >>> produce l2pad = 2 in your case, where it should be 0. In all other >>> cases, it works! >>> >>> >> >> Hmm. That's strange, as patch 3 of the series should have handled this. >> Basically this patch >> checks if a frame actually has payload, and refrains from inserting l2pad >> and payload alignment >> in that case. >> >> How did you detect this failure? >> > > I sent every possible frame type in monitor and compared with what ath9k in > monitor mode received. >> >> What did you observe on the "wire", i.e. how was the frame malformed >> because of this? >> > > For instance, i sent : > c400c400c40000000000 (10 bytes) > > and I received : > c400ca00c400000000000000 (12 bytes) > > You can see the 2 bytes added at the end (bytes 2-3 are modified by the HW > since it contains the duration_id) OK. Thanks for the example. I think I already know where the problem is. The rt2x00queue_insert_l2pad function doesn't trim down the skb properly in case the frame has no payload. I'll provide a patch, but that may take until Friday (when I get back home). >> >> Also, would it be possible to get some of the skb details of a frame that >> fails? >> > > I'll redo the test. What details do you want? Never mind. I think I already know where the problem is. >> >> >>> >>> Solution : padding is only needed for data frames for rt28x devices, so >>> I think it's better to something like rt2xqueue_padpos >>> >>> [http://git.popipo.fr/?p=rt2x00.git;a=commitdiff;h=95ddf076c13062d1026025d97ba511f880a1792d] >>> >>> >> >> I'm still not fond of using detailed ieee 802.11 frame format knowledge to >> do this. I prefer >> to use standard mac80211 functions with a bit of detection on the actual >> skb. >> > > If you want to use mac80211 functions, we can do something like that : /* > * Determine the L2 padding size based on the 802.11 header length > * > * Exact formula is (4 - (header_length%4))%4 but since header_length is > * always a multiple of 2, we could simplify to (header_length&3). Current formula is (-header_length) & 3, which works even for header_lengths that are not a multiple of 2. > * > * L2PAD is only present for data frame and an easy way to check for that is > * to compare header_length with 24 bytes. > */ > #define L2PAD_SIZE(__header) \ > ((__header)<24 ? 0 : ((4 - ((__header)%4))%4)) > That depends on what the purpose of the L2PAD_SIZE macro is going to be. At the moment the intention is to have L2PAD_SIZE compute the number of l2pad bytes necessary, if a payload is present. Detection on whether actually a payload is present and whether the outcome of this macro should be used should be at the call-sites of this macro. --- Gertjan. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
