On Mon, 2012-11-12 at 13:52 -0800, Thomas Pedersen wrote: > On Mon, Nov 12, 2012 at 12:42 PM, Johannes Berg > <johannes@xxxxxxxxxxxxxxxx> wrote: > > On Mon, 2012-11-12 at 10:38 -0800, Thomas Pedersen wrote: > > > >> + if (ieee80211_have_rx_timestamp(rx_status)) > >> + /* time when timestamp field was received */ > >> + t_r = ieee80211_calculate_rx_timestamp(local, rx_status, > >> + 24 + 12 + > >> + elems->total_len + 4, > >> + 24); > > > > This doesn't seem quite right, the FCS isn't accounted for correctly? > > That's what the +4 is for. Is this wrong? No, ok, I guess that's correct then. I didn't do the calculations to check. Why not use skb->len though, rather than 24+12+...? Btw, I think there's a define for FCS_LEN somewhere, that might be worthwhile to use just for documentation. > > I think it might be wortwhile to pass the SKB to the function instead of > > rx_status though, then it could get skb->len and check whether or not > > FCS was before the timestamp > > How would you check this? Never mind, you can't know. > > (which, btw, you should mention in the documentation for the new flag!) > > I'll make it clear MACTIME_END includes the FCS. Ok. > > And then I suspect the check for IEEE80211_HW_RX_INCLUDES_FCS is also > > incorrect -- even if the driver didn't report the FCS maybe the > > timestamping semantic was such that it was after the FCS? > > Right, if the driver doesn't report the FCS, the radiotap code will > add an extra 4 bytes to account for this. Oh, yes, I misread the code, sorry. johannes -- 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
