On Thu, Nov 19, 2009 at 1:19 PM, Benoit Papillault
<benoit.papillault@xxxxxxx> wrote:
> From: Benoit PAPILLAULT <benoit@benoit-laptop.(none)>
>
> It has been tested with a 802.11 frame generator and by checking the FCS field
> of each received frame with the value reported by the Atheros hardware. This
> patch is useful if you are trying to analyze non standard 802.11 frame going
> over the air.
Thank you for your patch! But can you please elaborate on your commit
log entry? This just tells me that you've tested it and how its useful
but it in no way tells me what you found was wrong and also does not
explain how you fixed it.
> Signed-off-by: Benoit PAPILLAULT <benoit@benoit-laptop.(none)>
> ---
> drivers/net/wireless/ath/ath9k/common.c | 19 ++++++++++++++-----
> 1 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/common.c b/drivers/net/wireless/ath/ath9k/common.c
> index 2f1e161..4a13632 100644
> --- a/drivers/net/wireless/ath/ath9k/common.c
> +++ b/drivers/net/wireless/ath/ath9k/common.c
> @@ -231,26 +231,35 @@ void ath9k_cmn_rx_skb_postprocess(struct ath_common *common,
> {
> struct ath_hw *ah = common->ah;
> struct ieee80211_hdr *hdr;
> - int hdrlen, padsize;
> + int hdrlen, padpos, padsize;
> u8 keyix;
> __le16 fc;
>
> /* see if any padding is done by the hw and remove it */
> hdr = (struct ieee80211_hdr *) skb->data;
> hdrlen = ieee80211_get_hdrlen_from_skb(skb);
> + padpos = 24;
> fc = hdr->frame_control;
> + if ((fc & cpu_to_le16(IEEE80211_FCTL_FROMDS|IEEE80211_FCTL_TODS)) ==
> + cpu_to_le16(IEEE80211_FCTL_FROMDS|IEEE80211_FCTL_TODS)) {
> + padpos += 6; /* ETH_ALEN */
> + }
How about just using ETH_ALEN then?
> + if ((fc & cpu_to_le16(IEEE80211_STYPE_QOS_DATA|IEEE80211_FCTL_FTYPE)) ==
> + cpu_to_le16(IEEE80211_STYPE_QOS_DATA|IEEE80211_FTYPE_DATA)) {
> + padpos += 2;
> + }
>
> /* The MAC header is padded to have 32-bit boundary if the
> * packet payload is non-zero. The general calculation for
> * padsize would take into account odd header lengths:
> - * padsize = (4 - hdrlen % 4) % 4; However, since only
> + * padsize = (4 - padpos % 4) % 4; However, since only
> * even-length headers are used, padding can only be 0 or 2
> * bytes and we can optimize this a bit. In addition, we must
> * not try to remove padding from short control frames that do
> * not have payload. */
> - padsize = hdrlen & 3;
> - if (padsize && hdrlen >= 24) {
> - memmove(skb->data + padsize, skb->data, hdrlen);
> + padsize = padpos & 3;
> + if (padsize && skb->len>=padpos+padsize+FCS_LEN) {
> + memmove(skb->data + padsize, skb->data, padpos);
> skb_pull(skb, padsize);
> }
If the skb->len would have been short ieee80211_get_hdrlen_from_skb()
would have picked up on this and 0 would have been used for hdrlen
therefore skipping this operation. With this patch even if skb->len
was 0 your padsize is always based on some static value. Additionally
its unclear to me how and why you substitute
ieee80211_get_hdrlen_from_skb() to a static 24 + something.
Also the possible static values for padpos are either (24 + 2) or (24
+ 6) right? Well these & 3 will always give true. So you are always
padding and this changes the way this was being implemented.
Unless I'm missing something.
Luis
--
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
